Re: [PATCH 1/5]PCI: x86 MMCONFIG: introduce PCI_USING_MMCONF

Re: [PATCH 1/5]PCI: x86 MMCONFIG: introduce PCI_USING_MMCONF

[Greg KH]

On Wed, Dec 19, 2007 at 05:17:51PM -0500, tcamuso@redhat.com wrote:
> commit d1f2a573ca7083d237cfc354f42e089de9fa0f0b
> Author: Tony Camuso <tony.camuso@hp.com>
> Date:   Wed Dec 19 15:33:53 2007 -0500
> 
>     Introduces a flag bit to arch/x86/pci/pci.h to indicate that
>     MMCONFIG is the system-wide default PCI config access mechanisim.
>     
>     Function pci_mmcfg_init() in arch/x86/pci/mmconfig-shared.c sets
>     the bit in the pci_probe mask when MMCONFIG has been successfully
>     initialized for the platform.
>     
>     Signed-off-by: Tony Camuso tony.camuso@hp.com
> 
> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
> index 4df637e..fc0f866 100644
> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -274,7 +274,8 @@ void __init pci_mmcfg_init(int type)
>  			unreachable_devices();
>  		if (known_bridge)
>  			pci_mmcfg_insert_resources(IORESOURCE_BUSY);
> -		pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF;
> +		pci_probe = pci_probe & ~PCI_PROBE_MASK;
> +		pci_probe = pci_probe | PCI_PROBE_MMCONF | PCI_USING_MMCONF;
>  	} else {
>  		/*
>  		 * Signal not to attempt to insert mmcfg resources because
> diff --git a/arch/x86/pci/pci.h b/arch/x86/pci/pci.h
> index ac56d39..53f4051 100644
> --- a/arch/x86/pci/pci.h
> +++ b/arch/x86/pci/pci.h
> @@ -28,6 +28,7 @@
>  #define PCI_ASSIGN_ALL_BUSSES	0x4000
>  #define PCI_CAN_SKIP_ISA_ALIGN	0x8000
>  #define PCI_USE__CRS		0x10000
> +#define PCI_USING_MMCONF	0x20000
>  
>  extern unsigned int pci_probe;
>  extern unsigned long pirq_table_addr;
> @@ -39,6 +40,8 @@ enum pci_bf_sort_state {
>  	pci_dmi_bf,
>  };
>  
> +extern struct pci_ops pci_legacy_ops; 	/* direct.c */

This isn't needed in this patch at all, and might make the compiler
confused if you were to build with only this patch present :(

thanks,

greg k-h

Re: [PATCH 1/5]PCI: x86 MMCONFIG: introduce PCI_USING_MMCONF

[Tony Camuso]

Greg KH wrote:

> Please do not respond privately, please respond back on the list so that
> everyone can see.  That way I will respond, to do otherwise is rude to
> the rest of the community...
> 
> thanks,
> 
> greg k-h

I'm very sorry.

It is certainly not my intention to be rude.

Re: [PATCH 2/5]PCI: x86 MMCONFIG: add legacy pci conf functions

[Greg KH]

On Wed, Dec 19, 2007 at 05:17:56PM -0500, tcamuso@redhat.com wrote:
> commit 307e3aecde8060af3802590f8c5bffb5456fe22b
> Author: Tony Camuso <tony.camuso@hp.com>
> Date:   Wed Dec 19 15:38:31 2007 -0500
> 
>     Modifies arch/x86/pci/direct.c to add the Legacy PCI Config routines
>     that will be made available, even when MMCONFIG is the platform default
>     PCI Config access mechanism. It also provides logic for selecting the
>     correct Legacy PCI Config access mechanism and the corresponding boot log
>     messages.
>     
>     Signed-off-by: Tony Camuso tony.camuso@hp.com
> 
> diff --git a/arch/x86/pci/direct.c b/arch/x86/pci/direct.c
> index 431c9a5..4e78002 100644
> --- a/arch/x86/pci/direct.c
> +++ b/arch/x86/pci/direct.c
> @@ -175,6 +175,32 @@ static struct pci_raw_ops pci_direct_conf2 = {
>  	.write =	pci_conf2_write,
>  };
>  
> +/*
> + * Legacy PCI Config read and write routines for buses that can't use
> + * MMCONFIG accesses in systems where MMCONFIG is the default PCI config
> + * access mechanism.
> + */
> +static struct pci_raw_ops *pci_legacy_conf;
> +
> +static int pci_ops_legacy_read(struct pci_bus *bus, unsigned int devfn,
> +			      int where, int size, u32 *value)
> +{
> +	return pci_legacy_conf->read(0, bus->number, devfn, where,
> +				     size, value);
> +}
> +
> +static int pci_ops_legacy_write(struct pci_bus *bus, unsigned int devfn,
> +			       int where, int size, u32 value)
> +{
> +	return pci_legacy_conf->write(0, bus->number, devfn, where,
> +				      size, value);
> +}
> +
> +struct pci_ops pci_legacy_ops = {
> +	.read =		pci_ops_legacy_read,
> +	.write =	pci_ops_legacy_write,
> +};

This whole structure is never used in this patch, why add it now?

>  /*
>   * Before we decide to use direct hardware access mechanisms, we try to do some
> @@ -258,18 +284,28 @@ void __init pci_direct_init(int type)
>  {
>  	if (type == 0)
>  		return;
> -	printk(KERN_INFO "PCI: Using configuration type %d\n", type);
> -	if (type == 1)
> +	if (type == 1) {
>  		raw_pci_ops = &pci_direct_conf1;
> -	else
> +		pci_legacy_conf = &pci_direct_conf1;
> +	} else {
>  		raw_pci_ops = &pci_direct_conf2;
> +		pci_legacy_conf = &pci_direct_conf2;

Why have two pointers to the same thing?  What is pci_legacy_conf going
to be used for?  Why not just use raw_pci_ops?

thanks,

greg k-h

Re: [PATCH 4/5]PCI: x86 MMCONFIG: introduce pcibios_fix_bus_scan()

[Greg KH]

On Wed, Dec 19, 2007 at 05:18:06PM -0500, tcamuso@redhat.com wrote:
> commit ab28e1157e970f711c8451b66b3f940ec092db9d
> Author: Tony Camuso <tony.camuso@hp.com>
> Date:   Wed Dec 19 15:51:48 2007 -0500
> 
>     Introduces the x86 arch-specific routine that will determine whether
>     a device responds correctly to MMCONFIG accesses. This routine is
>     given the generic name pcibios_fix_bus_scan_quirk()
>     
>     The comment at the top of the routine explains its logic.
>     
>     Signed-off-by: Tony Camuso tony.camuso@hp.com
> 
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 8627463..9b1742d 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -525,3 +525,64 @@ struct pci_bus *pci_scan_bus_with_sysdata(int busno)
>  
>  	return bus;
>  }
> +
> +/**
> + * This routine traps devices not correctly responding to MMCONFIG access.
> + * For each device on the current bus, compare a mmconf read of the
> + * vendor/device dword with a legacy PCI config read. If they're not the same,
> + * the bus serving this device must use legacy PCI config accesses instead of
> + * mmconf, as must all buses descending from this bus.
> + */
> +
> +#define CHECK_MMCFG_STR_1 \
> +	"PCI: Device at %04x:%02x.%02x.%x is not MMCONFIG compliant.\n"
> +#define CHECK_MMCFG_STR_2 \
> +	"PCI: Bus %04x:%02x and its descendents cannot use MMCONFIG.\n"

Why define these if they are only used in one place?

Also, as you use dev_info(), I think you are duplicating some of the
information in the resulting printk(), right?

> +
> +void __devinit pcibios_fix_bus_scan_quirk(struct pci_bus *bus)
> +{
> +	int devfn;
> +	int fail;
> +	int found_nommconf_dev = 0;
> +	static int advised;
> +	u32 mcfg_vendev;
> +	u32 arch_vendev;
> +	struct pci_ops *save_ops = bus->ops;
> +
> +	if (bus->parent != NULL)
> +		if (bus->parent->ops == &pci_legacy_ops)
> +			return;
> +
> +	if (!advised) {
> +		pr_info("PCI: If a device isn't working, try \"pci=nommconf\". "
> +			"If that helps, please post a report.\n");

Post a report where?  Who is going to handle these reports?

The last time someone put a line like this in the kernel, I got a ton of
email and didn't know what to do with it.  If you really are trusting of
this patch, please put your email address in this printk(), so that you
can properly handle the resulting reports.  I sure don't want to :)

> +		advised = 1;
> +	}
> +	pr_debug("PCI: Checking bus %04x:%02x for MMCONFIG compliance.\n",
> +		 pci_domain_nr(bus), bus->number);
> +
> +	for (devfn = 0; devfn < 256; devfn++) {
> +		bus->ops = &pci_legacy_ops;
> +		fail = (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID,
> +						  &arch_vendev));

What's with the extra () around the function?

> +		if ((arch_vendev == 0xFFFFFFFF) || (arch_vendev == 0) || fail)
> +			continue;
> +
> +		bus->ops = save_ops;	/* Restore to original value */
> +		pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID,
> +					  &mcfg_vendev);
> +		if (mcfg_vendev != arch_vendev) {
> +			found_nommconf_dev = 1;
> +			break;
> +		}
> +	}
> +
> +	if (found_nommconf_dev) {
> +		pr_info(CHECK_MMCFG_STR_1, pci_domain_nr(bus), bus->number,
> +			PCI_SLOT(devfn), PCI_FUNC(devfn));
> +		pr_info(CHECK_MMCFG_STR_2, pci_domain_nr(bus), bus->number);
> +		bus->ops = &pci_legacy_ops;	/* Use Legace PCI Config */

Spelling check for your comments :)

thanks,

greg k-h

Re: [PATCH 4/5]PCI: x86 MMCONFIG: introduce pcibios_fix_bus_scan()

[Greg KH]

On Wed, Dec 19, 2007 at 05:18:06PM -0500, tcamuso@redhat.com wrote:
> commit ab28e1157e970f711c8451b66b3f940ec092db9d
> Author: Tony Camuso <tony.camuso@hp.com>
> Date:   Wed Dec 19 15:51:48 2007 -0500
> 
>     Introduces the x86 arch-specific routine that will determine whether
>     a device responds correctly to MMCONFIG accesses. This routine is
>     given the generic name pcibios_fix_bus_scan_quirk()
>     
>     The comment at the top of the routine explains its logic.
>     
>     Signed-off-by: Tony Camuso tony.camuso@hp.com
> 
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 8627463..9b1742d 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -525,3 +525,64 @@ struct pci_bus *pci_scan_bus_with_sysdata(int busno)
>  
>  	return bus;
>  }
> +
> +/**
> + * This routine traps devices not correctly responding to MMCONFIG access.
> + * For each device on the current bus, compare a mmconf read of the
> + * vendor/device dword with a legacy PCI config read. If they're not the same,
> + * the bus serving this device must use legacy PCI config accesses instead of
> + * mmconf, as must all buses descending from this bus.
> + */

Don't user kerneldoc comments for functions that are not really in
kerneldoc format.  Just a simple /* at the start will be fine, otherwise
the tools get confused.

So what happens today with these "broken" devices?  Does simply reading
the vendor id not work properly?  What specific hardware does not work
today that this is trying to fix?

I think this patch set needs a bit of reworking at the very least...

thanks,

greg k-h

Re: [PATCH 0/5]PCI: x86 MMCONFIG

[Greg KH]

On Wed, Dec 19, 2007 at 05:17:46PM -0500, tcamuso@redhat.com wrote:
> OVERVIEW
> =======
> 
> The patches should be applied in sequence to obviate any
> possible build problems.
> 
> The patch-set was built against 2.6.24-rc5
> 
> Description
> ===========
> 
> There exist devices that do not respond correctly to PCI
> MMCONFIG accesses in x86 platforms.

What devices are these?  Do you have reports of them somewhere?

> This patch-set detects the problem by comparing an MMCONFIG
> read to a Legacy PCI config read of the vendor/device dword
> of every device discovered during the PCI probing sequence.
> 
> A miscompare means that a device does not correctly respond
> to MMCONFIG accesses. When the patch code detects this condition,
> the bus that serves this device, and all subordinate buses, will
> be programmed to use Legacy PCI Config accesses.
> 
> This patch-set DOES NOT detect devices that generate machine
> checks against MMCONFIG accesses. For such systems,
> "pci=nommconf" is required in the boot command.

That sounds like this patchset can cause bad side affects on hardware
that currently works just fine.  That is not a good thing to be adding
to the kernel, right?

thanks,

greg k-h

Re: [PATCH 0/5]PCI: x86 MMCONFIG

[Robert Hancock]

Greg KH wrote:
> On Wed, Dec 19, 2007 at 05:17:46PM -0500, tcamuso@redhat.com wrote:
>> OVERVIEW
>> =======
>>
>> The patches should be applied in sequence to obviate any
>> possible build problems.
>>
>> The patch-set was built against 2.6.24-rc5
>>
>> Description
>> ===========
>>
>> There exist devices that do not respond correctly to PCI
>> MMCONFIG accesses in x86 platforms.
> 
> What devices are these?  Do you have reports of them somewhere?
> 
>> This patch-set detects the problem by comparing an MMCONFIG
>> read to a Legacy PCI config read of the vendor/device dword
>> of every device discovered during the PCI probing sequence.
>>
>> A miscompare means that a device does not correctly respond
>> to MMCONFIG accesses. When the patch code detects this condition,
>> the bus that serves this device, and all subordinate buses, will
>> be programmed to use Legacy PCI Config accesses.
>>
>> This patch-set DOES NOT detect devices that generate machine
>> checks against MMCONFIG accesses. For such systems,
>> "pci=nommconf" is required in the boot command.
> 
> That sounds like this patchset can cause bad side affects on hardware
> that currently works just fine.  That is not a good thing to be adding
> to the kernel, right?

I think we need more details on why this patch is needed. Also, we 
already have something like this in arch/x86/pci/mmconfig-shared.c, in 
the unreachable_devices function. This attempts to detect devices where 
MMCONFIG cannot access the configuration space (one of these would be at 
least one device in the AMD K8 built-in northbridge). If this is not 
sufficient, I would suggest expanding that mechanism instead of adding 
all this new code.

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

[Fwd: Re: [PATCH 0/5]PCI: x86 MMCONFIG]

[Tony Camuso]

-------- Original Message --------
Subject: Re: [PATCH 0/5]PCI: x86 MMCONFIG
Date: Wed, 19 Dec 2007 19:33:45 -0500
From: Tony Camuso <tcamuso@redhat.com>
Reply-To: tcamuso@redhat.com
To: Greg KH <gregkh@suse.de>
References: 
<20071219221746.20362.39243.sendpatchset@dhcp83-188.boston.redhat.com> 
<20071219231609.GE24219@suse.de>

Greg KH wrote:
> On Wed, Dec 19, 2007 at 05:17:46PM -0500, tcamuso@redhat.com wrote:
>> There exist devices that do not respond correctly to PCI
>> MMCONFIG accesses in x86 platforms.
> 
> What devices are these?  Do you have reports of them somewhere?
> 
There are the AMD 8131 and 8132, the Serverworks HT1000 bridge chips
and the 830M/MG graphics. Not all versions of these chips present
this pathology, but there are perhaps tens of thousands of systems
out there that have the broken versions of these chipsets.

RedHat have been maintaining a blacklist of systems having these
devices. Systems in the blacklist are confined to legacy PCI
access.

However, some of these systems, high volume ones at that, also have
PCI express buses in them. By limiting the whole platform to legacy
PCI access, we put PCI express extended config capabilities out of
reach. IIRC, the PCIe spec requires platforms to access extended
PCI config space.

> That sounds like this patchset can cause bad side affects on hardware
> that currently works just fine.  That is not a good thing to be adding
> to the kernel, right?
> 
No, the patch set tries to obviate this without requiring endusers to
write customized scripts with "pci=nommconf" and without requiring the
RH folks to add another platform (usually belatedly) to the blacklist.

If a device is going to machine check when you touch it with an mmconfig
access, it will happen with or without this patch-set.

However, the patch-set does cover most of the devices that don't respond
well to mmconfig access. Such devices almost alway7s return garbage when
you read from them.

The one device we know about that throws exceptions is the 830M/MG
graphics chip. This chip passes the read-compare test, so the code
merrily advances to bus sizing. When the bus sizing code writes the
BAR at offset 0x18 in this device, the system hangs.

I am thinking about a machine check handler that can catch these things,
or at least the exceptions. Aborts are not recoverable, according to
intel lore. However, the one device that we know croaks HARD seems to
throw an exception, since it happens in the same exact place every
time. I think we might be able to recover from that.

But that's in the future.

Re: [Fwd: Re: [PATCH 0/5]PCI: x86 MMCONFIG]

[Greg KH]

On Thu, Dec 20, 2007 at 07:28:00AM -0500, Tony Camuso wrote:
>
>
> -------- Original Message --------
> Subject: Re: [PATCH 0/5]PCI: x86 MMCONFIG
> Date: Wed, 19 Dec 2007 19:33:45 -0500
> From: Tony Camuso <tcamuso@redhat.com>
> Reply-To: tcamuso@redhat.com
> To: Greg KH <gregkh@suse.de>
> References: 
> <20071219221746.20362.39243.sendpatchset@dhcp83-188.boston.redhat.com> 
> <20071219231609.GE24219@suse.de>
>
> Greg KH wrote:
>> On Wed, Dec 19, 2007 at 05:17:46PM -0500, tcamuso@redhat.com wrote:
>>> There exist devices that do not respond correctly to PCI
>>> MMCONFIG accesses in x86 platforms.
>> What devices are these?  Do you have reports of them somewhere?
> There are the AMD 8131 and 8132, the Serverworks HT1000 bridge chips
> and the 830M/MG graphics. Not all versions of these chips present
> this pathology, but there are perhaps tens of thousands of systems
> out there that have the broken versions of these chipsets.

Why haven't we gotten reports about this before if this is a common
problem?

And why hasn't the vendor fixed the bios on these to work properly?

> RedHat have been maintaining a blacklist of systems having these
> devices. Systems in the blacklist are confined to legacy PCI
> access.

Do you have a pointer to this blacklist anywhere so that everyone can
benifit from this knowledge?

>> That sounds like this patchset can cause bad side affects on hardware
>> that currently works just fine.  That is not a good thing to be adding
>> to the kernel, right?
> No, the patch set tries to obviate this without requiring endusers to
> write customized scripts with "pci=nommconf" and without requiring the
> RH folks to add another platform (usually belatedly) to the blacklist.
>
> If a device is going to machine check when you touch it with an mmconfig
> access, it will happen with or without this patch-set.
>
> However, the patch-set does cover most of the devices that don't respond
> well to mmconfig access. Such devices almost alway7s return garbage when
> you read from them.
>
> The one device we know about that throws exceptions is the 830M/MG
> graphics chip. This chip passes the read-compare test, so the code
> merrily advances to bus sizing. When the bus sizing code writes the
> BAR at offset 0x18 in this device, the system hangs.

So it doesn't work at all, with or without this patch?  Does the vendor
know about this?

thanks,

greg k-h

Re: [Fwd: Re: [PATCH 0/5]PCI: x86 MMCONFIG]

[Tony Camuso]

Greg KH wrote:

> Why haven't we gotten reports about this before if this is a common
> problem?
> 
> And why hasn't the vendor fixed the bios on these to work properly?
>
I can't really answer either of these questions. All I know is the
problem exists, and we need to deal with it.

That's why the unreachable_devices() routine exists in the first place.

But that routine is limited to only the first 16 buses on segment-0.
You would like to think that hardware designers would confine legacy
hardware, or problematic hardware, to that area, but they don't.

As I said before, expanding that routine to cover more buses would
adversely impact mmconfig pci access, since the mmconfig access
code does a lookup of that bitmap for every request. I don't think
we want that bitmap and the accompanying in-line lookup to increase
enough to encompass the pci configuration of some of these larger
systems.

> Do you have a pointer to this blacklist anywhere so that everyone can
> benifit from this knowledge?
>

Appended below is a code snippet embedded in the RHEL version of mmconfig.c,
for both i386 and x86_64. It does not encompass all the systems that have
(or will have) problems with mmconf. Only HP platforms are listed, but I
believe there are others.

The reason the HP platforms got caught is they put these devices beyond
bus 16, where they would have been trapped, and the problem would have
been avoided.

static int __devinit disable_mmconf(struct dmi_system_id *d)
{
         pci_probe &= ~PCI_PROBE_MMCONF;
         printk(KERN_INFO "%s detected: disabling PCI MMCONFIG\n", d->ident);
         return 0;
}


/*
  * Systems which cannot use PCI MMCONFIG at this time...
  */
static struct dmi_system_id __devinitdata nommconf_dmi_table[] = {
         {
                 .callback = disable_mmconf,
                 .ident = "HP xw9300 Workstation",
                 .matches = {
                         DMI_MATCH(DMI_PRODUCT_NAME, "HP xw9300 Workstation"),
                 },
         },
         {
                 .callback = disable_mmconf,
                 .ident = "HP xw9400 Workstation",
                 .matches = {
                         DMI_MATCH(DMI_PRODUCT_NAME, "HP xw9400 Workstation"),
                 },
         },
         {
                 .callback = disable_mmconf,
                 .ident = "ProLiant DL585 G2",
                 .matches = {
                         DMI_MATCH(DMI_PRODUCT_NAME, "ProLiant DL585 G2"),
                 },
         },
         {
                 .callback = disable_mmconf,
                 .ident = "HP Compaq dc5700 Microtower",
                 .matches = {
                         DMI_MATCH(DMI_PRODUCT_NAME,
                                 "HP Compaq dc5700 Microtower"),
                 },
         },

         {}
};

>> The one device we know about that throws exceptions is the 830M/MG
>> graphics chip. This chip passes the read-compare test, so the code
>> merrily advances to bus sizing. When the bus sizing code writes the
>> BAR at offset 0x18 in this device, the system hangs.
> 
> So it doesn't work at all, with or without this patch?  Does the vendor
> know about this?
> 
> thanks,
> 
> greg k-h

I have talked to intel about this, but they haven't gotten back to me.

All I know at this point is that a mmconf write to the BAR at offset 0x18
of this device hangs the system.

Re: [Fwd: Re: [PATCH 0/5]PCI: x86 MMCONFIG]

[Greg KH]

On Thu, Dec 20, 2007 at 01:25:57PM -0500, Tony Camuso wrote:
> Appended below is a code snippet embedded in the RHEL version of 
> mmconfig.c,
> for both i386 and x86_64. It does not encompass all the systems that have
> (or will have) problems with mmconf. Only HP platforms are listed, but I
> believe there are others.
>
> The reason the HP platforms got caught is they put these devices beyond
> bus 16, where they would have been trapped, and the problem would have
> been avoided.

Any reason why these changes were never submitted to the upstream kernel
versions?  Or do you all just want to keep patching your newer releases
with this information forever?  :)

Please send these kinds of changes upstream...

>>> The one device we know about that throws exceptions is the 830M/MG
>>> graphics chip. This chip passes the read-compare test, so the code
>>> merrily advances to bus sizing. When the bus sizing code writes the
>>> BAR at offset 0x18 in this device, the system hangs.
>> So it doesn't work at all, with or without this patch?  Does the vendor
>> know about this?
>> thanks,
>> greg k-h
>
> I have talked to intel about this, but they haven't gotten back to me.

Try poking them harder, they are usually very responsive to Linux kernel
issues.

thanks,

greg k-h

Re: [Fwd: Re: [PATCH 0/5]PCI: x86 MMCONFIG]

[Tony Camuso]

Greg KH wrote:
> On Thu, Dec 20, 2007 at 01:25:57PM -0500, Tony Camuso wrote:
> Any reason why these changes were never submitted to the upstream kernel
> versions?  Or do you all just want to keep patching your newer releases
> with this information forever?  :)
> 

I really don't know why these changes were never submitted to the
upstream kernel versions".

I was brought on the scene about six months ago as HP's on-site engineer
at RH, and this was one of the things they wanted me to do.

We wanted a solution that was more generic and could manage this
problem preemptively, rather than using blacklists. Maintenance of
blacklists is a bother and almost always done after a new system
with this problem is discovered.

Furthermore, blacklisting whole platforms to use legacy pci config
penalizes any mmconfig-friendly buses in those platforms, particularly
the pci express buses, and causes such platforms to be non-compliant
with the pci expres spec.

 > Please send these kinds of changes upstream...
 >
As you wish.
:)

>> I have talked to intel about this, but they haven't gotten back to me.
> 
> Try poking them harder, they are usually very responsive to Linux kernel
> issues.
> 
> thanks,
> 
> greg k-h

Matthew Wilcox explained the problem to me. The hang that cannot be fixed
by my patch isn't the chipset's fault. The graphics device has a 64-bit
BAR asking for 256 MB of IO.

If I understand his explanation correctly, when the bus sizing code
writes the low dword of this BAR with 0xffffffff, the chip is then
programmed to claim every MMIO reference betweeen 0x00000000.f0000000 and
0x00000001.00000000

It just so happens that MMCONFIG space for the dc5700 (awa some others) is
mapped by its BIOS into that very region, so all MMCONFIG references are now
swallowed up by the graphics chip. At that point, no further boot progress can
be made.

Re: [Fwd: Re: [PATCH 0/5]PCI: x86 MMCONFIG]

[Greg KH]

On Thu, Dec 20, 2007 at 05:36:43PM -0500, Tony Camuso wrote:
> Greg KH wrote:
>> On Thu, Dec 20, 2007 at 01:25:57PM -0500, Tony Camuso wrote:
>> Any reason why these changes were never submitted to the upstream kernel
>> versions?  Or do you all just want to keep patching your newer releases
>> with this information forever?  :)
>
> I really don't know why these changes were never submitted to the
> upstream kernel versions".
>
> I was brought on the scene about six months ago as HP's on-site engineer
> at RH, and this was one of the things they wanted me to do.
>
> We wanted a solution that was more generic and could manage this
> problem preemptively, rather than using blacklists. Maintenance of
> blacklists is a bother and almost always done after a new system
> with this problem is discovered.
>
> Furthermore, blacklisting whole platforms to use legacy pci config
> penalizes any mmconfig-friendly buses in those platforms, particularly
> the pci express buses, and causes such platforms to be non-compliant
> with the pci expres spec.

Sure, I realize this, but it solves the problem in one way for broken
hardware, such that it at least allows it to work, right?  It also
provides a better incentive for the manufacturer to fix their bios,
which as you are on-site at HP, it would seem odd that they would just
not do that instead of trying to work around this in the kernel...

thanks,

greg k-h

Re: [PATCH 0/5]PCI: x86 MMCONFIG

[Tony Camuso]

Greg,

Have you given this patch-set any more consideration?

I've submitted the changes you requested. 

Regards,
Tony

Re: [PATCH 0/5]PCI: x86 MMCONFIG

[Greg KH]

On Mon, Jan 07, 2008 at 10:20:23PM -0500, Tony Camuso wrote:
> Greg,
> 
> Have you given this patch-set any more consideration?

Which patch-set, there have been a number of them :)

> I've submitted the changes you requested. 

Care to respin them all so I'm not confused?

thanks,

greg k-h

Re: [PATCH 0/5]PCI: x86 MMCONFIG

[Tony Camuso]

I'll respin the whole kit with [PATCH ?/5][V2]PCI: x86 MMCONFIG *
in the subject line, where the ? is replaced by the number of the
patch within the set and the * is replaced by a brief description,
if that's acceptable.

I can have it ready in a few hours. 

On Mon, 2008-01-07 at 20:56 -0800, Greg KH wrote:
> On Mon, Jan 07, 2008 at 10:20:23PM -0500, Tony Camuso wrote:
> > Greg,
> > 
> > Have you given this patch-set any more consideration?
> 
> Which patch-set, there have been a number of them :)
> 
> > I've submitted the changes you requested. 
> 
> Care to respin them all so I'm not confused?
> 
> thanks,
> 
> greg k-h

Re: [PATCH 0/5]PCI: x86 MMCONFIG

[Greg KH]

On Tue, Jan 08, 2008 at 08:14:22AM -0500, Tony Camuso wrote:
> I'll respin the whole kit with [PATCH ?/5][V2]PCI: x86 MMCONFIG *
> in the subject line, where the ? is replaced by the number of the
> patch within the set and the * is replaced by a brief description,
> if that's acceptable.

That sounds great.

> I can have it ready in a few hours. 

I'll be on a plane for a few hours, so don't feel the need to rush :)

thanks,

greg k-h

Re: [PATCH 0/5]PCI: x86 MMCONFIG

[Tony Camuso]

Thanks, Greg.

Have a safe flight!

On Tue, 2008-01-08 at 05:36 -0800, Greg KH wrote:
> On Tue, Jan 08, 2008 at 08:14:22AM -0500, Tony Camuso wrote:
> > I'll respin the whole kit with [PATCH ?/5][V2]PCI: x86 MMCONFIG *
> > in the subject line, where the ? is replaced by the number of the
> > patch within the set and the * is replaced by a brief description,
> > if that's acceptable.
> 
> That sounds great.
> 
> > I can have it ready in a few hours. 
> 
> I'll be on a plane for a few hours, so don't feel the need to rush :)
> 
> thanks,
> 
> greg k-h