2007-08-26 01:56:14

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH] Fix boot-time hang on G31/G33 PC


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 <[email protected]>

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."


2007-08-26 04:25:26

by Robert Hancock

[permalink] [raw]
Subject: Re: [PATCH] Fix boot-time hang on G31/G33 PC

Matthew Wilcox wrote:
> 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 <[email protected]>
>

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
makes the disable-decode change more critical.

> 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)
>

2007-08-26 12:55:53

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] Fix boot-time hang on G31/G33 PC

On Sat, Aug 25, 2007 at 10:24:57PM -0600, Robert Hancock wrote:
> We've already got a patch for this in Greg's PCI tree, hopefully it
> should go in for 2.6.24.

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
about the same patch.

> Are you getting MMCONFIG enabled on your system with 2.6.23?

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."

2007-08-26 14:07:54

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] Fix boot-time hang on G31/G33 PC

On Sun, Aug 26, 2007 at 06:55:42AM -0600, Matthew Wilcox wrote:
> On Sat, Aug 25, 2007 at 10:24:57PM -0600, Robert Hancock wrote:
> > We've already got a patch for this in Greg's PCI tree, hopefully it
> > should go in for 2.6.24.
>
> 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
> about the same patch.

I found
http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f=pci/pci-disable-decode-of-io-memory-during-bar-sizing.patch;h=958ef4e837733206a80f058aee236847eec5fbd8;hb=HEAD

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."

2007-08-26 18:00:45

by Robert Hancock

[permalink] [raw]
Subject: Re: [PATCH] Fix boot-time hang on G31/G33 PC

Matthew Wilcox wrote:
> On Sun, Aug 26, 2007 at 06:55:42AM -0600, Matthew Wilcox wrote:
>> On Sat, Aug 25, 2007 at 10:24:57PM -0600, Robert Hancock wrote:
>>> We've already got a patch for this in Greg's PCI tree, hopefully it
>>> should go in for 2.6.24.
>> 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
>> about the same patch.
>
> I found
> http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f=pci/pci-disable-decode-of-io-memory-during-bar-sizing.patch;h=958ef4e837733206a80f058aee236847eec5fbd8;hb=HEAD
>
> 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.

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 <[email protected]>

2007-08-28 17:27:18

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH] Fix boot-time hang on G31/G33 PC

> > Greg, please drop Robert's patch and put mine in instead.
>
> 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 <[email protected]>

Yeah, thanks Matthew.

Acked-by: Jesse Barnes <[email protected]>

2007-08-28 17:59:38

by Grant Grundler

[permalink] [raw]
Subject: Re: [PATCH] Fix boot-time hang on G31/G33 PC

On Sat, Aug 25, 2007 at 07:55:56PM -0600, Matthew Wilcox wrote:
>
> 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.

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,
grant

>
> Signed-off-by: Matthew Wilcox <[email protected]>
>
> 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."

2007-08-28 18:29:18

by Grant Grundler

[permalink] [raw]
Subject: Re: [PATCH] Fix boot-time hang on G31/G33 PC

On Tue, Aug 28, 2007 at 11:59:08AM -0600, Grant Grundler wrote:
> On Sat, Aug 25, 2007 at 07:55:56PM -0600, Matthew Wilcox wrote:
> >
> > 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.
>
> This issue has come up before:
> http://www.uwsg.indiana.edu/hypermail/linux/kernel/0212.2/0232.html

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

2007-09-26 21:23:43

by Greg KH

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

Due to the issues surrounding this patch, I'm dropping it from my repo.

thanks,

greg k-h


On Sat, Aug 25, 2007 at 07:55:56PM -0600, Matthew Wilcox wrote:
>
>
> 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 <[email protected]>
> Acked-by: Robert Hancock <[email protected]>
> Acked-by: Jesse Barnes <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ---
> drivers/pci/probe.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -180,11 +180,26 @@ static inline int is_64bit_memory(u32 ma
> 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_de
> }
> }
> }
> +
> + if ((dev->class >> 8) != PCI_CLASS_BRIDGE_HOST)
> + pci_write_config_word(dev, PCI_COMMAND, orig_cmd);
> }
>
> void pci_read_bridge_bases(struct pci_bus *child)

2007-09-26 21:57:40

by Jesse Barnes

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

On Wednesday, September 26, 2007 2:18 pm Greg KH wrote:
> Due to the issues surrounding this patch, I'm dropping it from my
> repo.

What issues? Is it causing problems for people?

Jesse

2007-09-26 22:02:14

by Greg KH

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

On Wed, Sep 26, 2007 at 02:55:55PM -0700, Jesse Barnes wrote:
> On Wednesday, September 26, 2007 2:18 pm Greg KH wrote:
> > Due to the issues surrounding this patch, I'm dropping it from my
> > repo.
>
> What issues? Is it causing problems for people?

I thought this was the patch that Ivan objected to.

thanks,

greg k-h

2007-09-26 22:22:19

by Jesse Barnes

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

On Wednesday, September 26, 2007 2:56 pm Greg KH wrote:
> On Wed, Sep 26, 2007 at 02:55:55PM -0700, Jesse Barnes wrote:
> > On Wednesday, September 26, 2007 2:18 pm Greg KH wrote:
> > > Due to the issues surrounding this patch, I'm dropping it from my
> > > repo.
> >
> > What issues? Is it causing problems for people?
>
> I thought this was the patch that Ivan objected to.

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

2007-09-26 23:05:28

by Robert Hancock

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

Jesse Barnes wrote:
> On Wednesday, September 26, 2007 2:56 pm Greg KH wrote:
>> On Wed, Sep 26, 2007 at 02:55:55PM -0700, Jesse Barnes wrote:
>>> On Wednesday, September 26, 2007 2:18 pm Greg KH wrote:
>>>> Due to the issues surrounding this patch, I'm dropping it from my
>>>> repo.
>>> What issues? Is it causing problems for people?
>> I thought this was the patch that Ivan objected to.
>
> 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).

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 [email protected]
Home Page: http://www.roberthancock.com/

2007-09-27 14:31:35

by Ivan Kokshaysky

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

On Wed, Sep 26, 2007 at 03:20:40PM -0700, Jesse Barnes wrote:
> 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).

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
a no-op, but disable/re-enable clearly isn't.

> 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).

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.

2007-09-27 18:36:52

by Kok, Auke

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

Ivan Kokshaysky wrote:
> On Wed, Sep 26, 2007 at 03:20:40PM -0700, Jesse Barnes wrote:
>> 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).
>
> 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
> a no-op, but disable/re-enable clearly isn't.
>
>> 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).
>
> 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.


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

2007-09-28 17:16:49

by Greg KH

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

On Thu, Sep 27, 2007 at 11:36:32AM -0700, Kok, Auke wrote:
> Ivan Kokshaysky wrote:
> > On Wed, Sep 26, 2007 at 03:20:40PM -0700, Jesse Barnes wrote:
> >> 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).
> >
> > 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
> > a no-op, but disable/re-enable clearly isn't.
> >
> >> 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).
> >
> > 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.
>
>
> 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.

Thanks for letting us know. So, another reason to drop this patch :)

thanks,

greg k-h

2007-10-12 14:29:40

by Vitaliy Gusev

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

On the 28 September 2007 03:13 Greg KH, wrote:
> On Thu, Sep 27, 2007 at 11:36:32AM -0700, Kok, Auke wrote:
> > Ivan Kokshaysky wrote:
> > > On Wed, Sep 26, 2007 at 03:20:40PM -0700, Jesse Barnes wrote:
> > >> 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).
> > >
> > > 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
> > > a no-op, but disable/re-enable clearly isn't.
> > >
> > >> 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).
> > >
> > > 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.
> >
> > 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.
>

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
and works fine.

> Thanks for letting us know. So, another reason to drop this patch :)
>
> thanks,
>
> greg k-h
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Thanks,
Vitaliy Gusev

2007-10-12 17:09:12

by Kok, Auke

[permalink] [raw]
Subject: Re: PCI: Fix boot-time hang on G31/G33 PC

Vitaliy Gusev wrote:
> On the 28 September 2007 03:13 Greg KH, wrote:
>> On Thu, Sep 27, 2007 at 11:36:32AM -0700, Kok, Auke wrote:
>>> Ivan Kokshaysky wrote:
>>>> On Wed, Sep 26, 2007 at 03:20:40PM -0700, Jesse Barnes wrote:
>>>>> 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).
>>>> 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
>>>> a no-op, but disable/re-enable clearly isn't.
>>>>
>>>>> 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).
>>>> 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.
>>> 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.
>
> 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
> and works fine.

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