This patch, loosely based on a patch from Robert Hancock, which was in

turn based on a patch from Jesse Barnes, fixes a boot-time hang on my

shiny new PC.  The 'conflict' mentioned in the patch in my case happens

to be between mmconfig and the graphics card, but it could easily be

between any pair of devices if they are left enabled by the BIOS and

mappen in the wrong place.
Signed-off-by: Matthew Wilcox <matthew@wil.cx>
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c

index 34b8dae..51ef450 100644

--- a/drivers/pci/probe.c

+++ b/drivers/pci/probe.c

@@ -180,11 +180,26 @@ static inline int is_64bit_memory(u32 mask)

 	return 0;

 }
+/*

+ * Sizing PCI BARs requires us to disable decoding, otherwise we may run

+ * into conflicts with other devices while trying to size the BAR.  Normally

+ * this isn't a problem, but it happens on some machines normally, and can

+ * happen on others during PCI device hotplug.  Don't disable BARs for host

+ * bridges, though.  Some of them do silly things like disable accesses to

+ * RAM from the CPU

+ */

 static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)

 {

 	unsigned int pos, reg, next;

 	u32 l, sz;

 	struct resource *res;

+	u16 orig_cmd;

+

+	if ((dev->class >> 8) != PCI_CLASS_BRIDGE_HOST) {

+		pci_read_config_word(dev, PCI_COMMAND, &orig_cmd);

+		pci_write_config_word(dev, PCI_COMMAND,

+			orig_cmd & ~(PCI_COMMAND_MEMORY | PCI_COMMAND_IO));

+	}
 	for(pos=0; pos<howmany; pos = next) {

 		u64 l64;

@@ -283,6 +298,9 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)

 			}

 		}

 	}

+

+	if ((dev->class >> 8) != PCI_CLASS_BRIDGE_HOST)

+		pci_write_config_word(dev, PCI_COMMAND, orig_cmd);

 }
 void __devinit pci_read_bridge_bases(struct pci_bus *child)
--

"Bill, look, we understand that you're interested in selling us this

operating system, but compare it to ours.  We can't possibly take such

a retrograde step."

-

Due to the issues surrounding this patch, I'm dropping it from my repo.
thanks,
greg k-h
-

What issues?  Is it causing problems for people?
Jesse

-

I thought this was the patch that Ivan objected to.
thanks,
greg k-h

-

Yeah, Ivan objected to this, but incorrectly I think.
Ivan, your concern is about disabling things like interrupt controllers

and power management chips during probe right?  You're right that doing

that could cause problems if we get and interrupt or PMU event at just

the wrong time, but that could just as easily happen if decode was

still enabled but the BAR had a bogus address programmed (as it would

during probing).
Ultimately, I don't care much one way or another as long as we can get

the desktop platforms fixed somehow.  I think disabling decode is the

most correct way of doing this, but I'm open to other solutions (this

is the only patch I've seen though that's been tested to solve the

problem).
Jesse

-

Yes, nobody is arguing that moving the BAR around is unsafe, but generally

it's the less of two evils.
The major problem here is that with IO and MEM bits cleared in the command

register you disable *all* address decoders on the device, not just ranges

that have respective BARs. At least this behaviour is required by PCI spec.

Examples:

- legacy VGA IO and memory (no corresponding BARs);

- base/limit registers of P2P bridge;

- PMU and SMBus registers (sort of normal BARs, but hidden elsewhere

  in the config space);

- IDE legacy mode registers;

- IO-APIC registers (typically sort of read-only BAR).
For all of these address ranges our current BAR probe is effectively
There are two other solutions: one is to disable decode selectively,

only on devices or systems where it's necessary and known to be safe.

I've posted a patch which introduces "disable_while_probe" pci_dev field

for that purpose.

Another one is to delay mmconfig probe until after the PCI probe is done,

as Matthew suggested, and Robert confirmed that it's feasible.
Ivan.

-

for everyone who's using this quirk or has the same boot issue: I just confirmed

that the new dg33tl bios update v0287 (released 9/20) fixes the boot issue for my

systems. I encourage everyone to update their BIOS image and see if this works.
Cheers,
Auke

-

Thanks for letting us know.  So, another reason to drop this patch :)
thanks,
greg k-h

-

No, it still doesn't work even with a latest BIOS verstion. I have a computer

with Intel DQ35MP motherboard. I upgraded BIOS to rev. 0696, released

Oct 1. But kernel (2.6.22.5) still hangs up. Kernel with this fix patch boots 
--

Thanks,

Vitaliy Gusev
-

please note that your BIOS is completely different than the one posted above. This

may be a clue.
interestingly enough I can attest (somewhat) to this. After the BIOS upgrade

2.6.22 booted just fine without pci=nommconf, but I've had several boot lockups

with 2.6.23-rc8.
So, the issue is still open and Matthew/Jesse's suggested fix becomes much more

attractive to me now.
Greg, I suggest putting this patch back in the "grey" zone
Auke

-

I agree, I've yet to see a single report of an actual problem that

disabling the decode causes, nor even a theoretical problem which

couldn't already happen due to the possibility of the device being

accessed during BAR sizing, which just shouldn't be allowed since it

can't work with or without this patch.
Until and unless we have something better that fixes the real and

serious boot-time problems that we need this patch to fix, I would say

it should stay in.
--

Robert Hancock      Saskatoon, SK, Canada

To email, remove "nospam" from hancockr@nospamshaw.ca

Home Page: http://www.roberthancock.com/
-

This issue has come up before:

    http://www.uwsg.indiana.edu/hypermail/linux/kernel/0212.2/0232.html
and Ivan Kokshaysky suggested a very similar patch:

    http://www.uwsg.indiana.edu/hypermail/linux/kernel/0212.2/0324.html
cheers,

-

Ok...finally found the thread I was looking for:

    http://www.ussg.iu.edu/hypermail/linux/kernel/0212.2/0978.html
or look at the "by Thread" page and search for "disable BAR":

    http://www.ussg.iu.edu/hypermail/linux/kernel/0212.2/index.html
Main difference now is not disabling anything on any sort of Bridge.
Summary: Sizing BARs has never been a very safe operation. We have to

mitigate as best we can and then live with the remaining risks.
cheers,

grant

-

We've already got a patch for this in Greg's PCI tree, hopefully it

should go in for 2.6.24.
Are you getting MMCONFIG enabled on your system with 2.6.23? If not this

problem shouldn't matter. In the cases I've seen that have caused

problems in the past (Intel boards mainly), where the MMCONFIG area

overlaps with where the graphics card BAR ends up during BAR sizing, the

BIOS happened to not reserve the MMCONFIG table in the E820 memory map,

so current mainline will turn off MMCONFIG. However, it's quite possible

that some systems will pass the old E820 validation check and turn on

MMCONFIG where the overlap happens..
There's a patch in Andi's tree (also hopefully for 2.6.24) to loosen the

MMCONFIG validation to check against ACPI reservations instead of the

E820 map (which isn't required to have a reservation for MMCONFIG). This

-

I haven't seen it.  I guess it wasn't sent to the PCI mailing list.
Your patch had two or three problems with it; assuming we're talking
Yes, I have to pass pci=nommconf to make it boot.
--

"Bill, look, we understand that you're interested in selling us this

operating system, but compare it to ours.  We can't possibly take such

a retrograde step."

-

I found

http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f=pc...
which has two problems:
 - With a 64-bit BAR, it checks to see if the upper 32 bits represent IO

   or memory.  You can't do that to the upper 32 bits.

 - It does a lot of additional writes to the cmd register; my patch does two

   writes per device; yours does two per BAR
It's also a lot more complex than my patch, IMO.
Greg, please drop Robert's patch and put mine in instead.
--

"Bill, look, we understand that you're interested in selling us this

operating system, but compare it to ours.  We can't possibly take such

a retrograde step."

-

Based on looking at your patch it seems OK. The first problem you

mentioned with what Jesse and I had there is definitely a valid one.
You can add my:
Acked-by: Robert Hancock <hancockr@shaw.ca>
-

Yeah, thanks Matthew.
Acked-by:  Jesse Barnes <jesse.barnes@intel.com>

-