Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Bjorn Helgaas]

On Monday 20 April 2009 07:35:40 pm Yinghai Lu wrote:
> it wil be overwriten later if _CRS is used, so don't bother to set it.
> 
> [ Impact: cleanup ]
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  arch/x86/pci/amd_bus.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> Index: linux-2.6/arch/x86/pci/amd_bus.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/pci/amd_bus.c
> +++ linux-2.6/arch/x86/pci/amd_bus.c
> @@ -100,6 +100,10 @@ void x86_pci_root_bus_res_quirks(struct
>  	int j;
>  	struct pci_root_info *info;
>  
> +	/* don't go for it if _CRS is used */
> +	if (pci_probe & PCI_USE__CRS)
> +		return;
> +
>  	/* if only one root bus, don't need to anything */
>  	if (pci_root_num < 2)
>  		return;

This isn't a comment on this patch per se.

I am concerned about the fact that "pci=use_crs" is not the default.
>From the changelog of 62f420f8282, it sounds like you have to boot an
IBM x3850 with "pci=use_crs" to make hot-plug work, even though ACPI
tells us everything we need to know.  That's backwards.

We shouldn't need an option to tell Linux that the firmware is
trustworthy.  We should have an option to *ignore* it for the times
when we trip over something broken and haven't figured out a way to
work around it yet.

Bjorn

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Jesse Barnes]

On Mon, 27 Apr 2009 13:44:01 -0600
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Monday 20 April 2009 07:35:40 pm Yinghai Lu wrote:
> > it wil be overwriten later if _CRS is used, so don't bother to set
> > it.
> > 
> > [ Impact: cleanup ]
> > 
> > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> > 
> > ---
> >  arch/x86/pci/amd_bus.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > Index: linux-2.6/arch/x86/pci/amd_bus.c
> > ===================================================================
> > --- linux-2.6.orig/arch/x86/pci/amd_bus.c
> > +++ linux-2.6/arch/x86/pci/amd_bus.c
> > @@ -100,6 +100,10 @@ void x86_pci_root_bus_res_quirks(struct
> >  	int j;
> >  	struct pci_root_info *info;
> >  
> > +	/* don't go for it if _CRS is used */
> > +	if (pci_probe & PCI_USE__CRS)
> > +		return;
> > +
> >  	/* if only one root bus, don't need to anything */
> >  	if (pci_root_num < 2)
> >  		return;
> 
> This isn't a comment on this patch per se.
> 
> I am concerned about the fact that "pci=use_crs" is not the default.
> From the changelog of 62f420f8282, it sounds like you have to boot an
> IBM x3850 with "pci=use_crs" to make hot-plug work, even though ACPI
> tells us everything we need to know.  That's backwards.
> 
> We shouldn't need an option to tell Linux that the firmware is
> trustworthy.  We should have an option to *ignore* it for the times
> when we trip over something broken and haven't figured out a way to
> work around it yet.

Well, we could try using _CRS by default, but like many things ACPI we
can probably only trust firmwares after a certain date (i.e. the date
when Windows started relying on the data being correct in order to
boot).  Do we have a good cutoff for that?  Or should we try generally
enabling it early in 2.6.31 to see what happens?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Yinghai Lu]

Bjorn Helgaas wrote:
> On Monday 20 April 2009 07:35:40 pm Yinghai Lu wrote:
>> it wil be overwriten later if _CRS is used, so don't bother to set it.
>>
>> [ Impact: cleanup ]
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>
>> ---
>>  arch/x86/pci/amd_bus.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> Index: linux-2.6/arch/x86/pci/amd_bus.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/pci/amd_bus.c
>> +++ linux-2.6/arch/x86/pci/amd_bus.c
>> @@ -100,6 +100,10 @@ void x86_pci_root_bus_res_quirks(struct
>>  	int j;
>>  	struct pci_root_info *info;
>>  
>> +	/* don't go for it if _CRS is used */
>> +	if (pci_probe & PCI_USE__CRS)
>> +		return;
>> +
>>  	/* if only one root bus, don't need to anything */
>>  	if (pci_root_num < 2)
>>  		return;
> 
> This isn't a comment on this patch per se.
> 
> I am concerned about the fact that "pci=use_crs" is not the default.
> From the changelog of 62f420f8282, it sounds like you have to boot an
> IBM x3850 with "pci=use_crs" to make hot-plug work, even though ACPI
> tells us everything we need to know.  That's backwards.
> 
> We shouldn't need an option to tell Linux that the firmware is
> trustworthy.  We should have an option to *ignore* it for the times
> when we trip over something broken and haven't figured out a way to
> work around it yet.

other system may have broken _CRS.

maybe we could try to use DMI whitelist them?

YH

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Bjorn Helgaas]

On Monday 27 April 2009 02:15:33 pm Yinghai Lu wrote:
> Bjorn Helgaas wrote:
> > On Monday 20 April 2009 07:35:40 pm Yinghai Lu wrote:
> >> it wil be overwriten later if _CRS is used, so don't bother to set it.
> >>
> >> [ Impact: cleanup ]
> >>
> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >>
> >> ---
> >>  arch/x86/pci/amd_bus.c |    4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> Index: linux-2.6/arch/x86/pci/amd_bus.c
> >> ===================================================================
> >> --- linux-2.6.orig/arch/x86/pci/amd_bus.c
> >> +++ linux-2.6/arch/x86/pci/amd_bus.c
> >> @@ -100,6 +100,10 @@ void x86_pci_root_bus_res_quirks(struct
> >>  	int j;
> >>  	struct pci_root_info *info;
> >>  
> >> +	/* don't go for it if _CRS is used */
> >> +	if (pci_probe & PCI_USE__CRS)
> >> +		return;
> >> +
> >>  	/* if only one root bus, don't need to anything */
> >>  	if (pci_root_num < 2)
> >>  		return;
> > 
> > This isn't a comment on this patch per se.
> > 
> > I am concerned about the fact that "pci=use_crs" is not the default.
> > From the changelog of 62f420f8282, it sounds like you have to boot an
> > IBM x3850 with "pci=use_crs" to make hot-plug work, even though ACPI
> > tells us everything we need to know.  That's backwards.
> > 
> > We shouldn't need an option to tell Linux that the firmware is
> > trustworthy.  We should have an option to *ignore* it for the times
> > when we trip over something broken and haven't figured out a way to
> > work around it yet.
> 
> other system may have broken _CRS.

Do you have examples of problems here, or are you just worried that
there *may* be problems?

> maybe we could try to use DMI whitelist them?

I don't like a whitelist because it requires ongoing maintenance
for correctly-working machines.  A blacklist is nicer because it
only requires maintenance for *broken* machines.  A date-based
solution would be better from that point of view.

Bjorn

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Yinghai Lu]

On Mon, Apr 27, 2009 at 1:39 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

>>
>> other system may have broken _CRS.
>
> Do you have examples of problems here, or are you just worried that
> there *may* be problems?
one system with three chains... with pci=use_crs
[    9.365669] pci_bus 0000:00: resource 0 io:  [0x00-0x3af]
[    9.371065] pci_bus 0000:00: resource 1 io:  [0x3e0-0xcf7]
[    9.376551] pci_bus 0000:00: resource 2 io:  [0x3b0-0x3bb]
[    9.382028] pci_bus 0000:00: resource 3 io:  [0x3c0-0x3df]
[    9.387513] pci_bus 0000:00: resource 4 io:  [0xd00-0xefff]
[    9.393077] pci_bus 0000:00: resource 5 mem: [0x0a0000-0x0bffff]
[    9.399084] pci_bus 0000:00: resource 6 mem: [0x0d0000-0x0dffff]
[    9.405089] pci_bus 0000:00: resource 7 mem: [0xdd000000-0xdfffffff]
[    9.505332] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
[    9.510991] pci_bus 0000:40: resource 1 mem: [0xdb000000-0xdcffffff]
[    9.553378] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
[    9.559036] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]

without that: amd_bus.c will read that from pci conf space
[    9.310965] pci_bus 0000:00: resource 0 io:  [0x9000-0xefff]
[    9.316621] pci_bus 0000:00: resource 1 io:  [0x00-0xfff]
[    9.322020] pci_bus 0000:00: resource 2 mem: [0xdd000000-0xdfffffff]
[    9.328373] pci_bus 0000:00: resource 3 mem: [0x0a0000-0x0bffff]
[    9.334378] pci_bus 0000:00: resource 4 mem: [0xc0000000-0xd9ffffff]
[    9.340731] pci_bus 0000:00: resource 5 mem: [0xf0000000-0xffffffff]
[    9.347084] pci_bus 0000:00: resource 6 mem: [0x840000000-0xfcffffffff]
[    9.444440] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
[    9.450099] pci_bus 0000:40: resource 1 io:  [0xf000-0xffff]
[    9.455757] pci_bus 0000:40: resource 2 mem: [0xdb000000-0xdcffffff]
[    9.498118] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
[    9.503777] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]

>
>> maybe we could try to use DMI whitelist them?
>
> I don't like a whitelist because it requires ongoing maintenance
> for correctly-working machines.  A blacklist is nicer because it
> only requires maintenance for *broken* machines.  A date-based
> solution would be better from that point of view.

could try apply that in development cycle like -rcX, and disable that
formal release.
so could find more broken system.

YH

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Bjorn Helgaas]

On Monday 27 April 2009 03:00:16 pm Yinghai Lu wrote:
> On Mon, Apr 27, 2009 at 1:39 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> >> other system may have broken _CRS.
> >
> > Do you have examples of problems here, or are you just worried that
> > there *may* be problems?
> one system with three chains... with pci=use_crs
> [    9.365669] pci_bus 0000:00: resource 0 io:  [0x00-0x3af]
> [    9.371065] pci_bus 0000:00: resource 1 io:  [0x3e0-0xcf7]
> [    9.376551] pci_bus 0000:00: resource 2 io:  [0x3b0-0x3bb]
> [    9.382028] pci_bus 0000:00: resource 3 io:  [0x3c0-0x3df]
> [    9.387513] pci_bus 0000:00: resource 4 io:  [0xd00-0xefff]
> [    9.393077] pci_bus 0000:00: resource 5 mem: [0x0a0000-0x0bffff]
> [    9.399084] pci_bus 0000:00: resource 6 mem: [0x0d0000-0x0dffff]
> [    9.405089] pci_bus 0000:00: resource 7 mem: [0xdd000000-0xdfffffff]
> [    9.505332] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
> [    9.510991] pci_bus 0000:40: resource 1 mem: [0xdb000000-0xdcffffff]
> [    9.553378] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
> [    9.559036] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]
> 
> without that: amd_bus.c will read that from pci conf space
> [    9.310965] pci_bus 0000:00: resource 0 io:  [0x9000-0xefff]
> [    9.316621] pci_bus 0000:00: resource 1 io:  [0x00-0xfff]
> [    9.322020] pci_bus 0000:00: resource 2 mem: [0xdd000000-0xdfffffff]
> [    9.328373] pci_bus 0000:00: resource 3 mem: [0x0a0000-0x0bffff]
> [    9.334378] pci_bus 0000:00: resource 4 mem: [0xc0000000-0xd9ffffff]
> [    9.340731] pci_bus 0000:00: resource 5 mem: [0xf0000000-0xffffffff]
> [    9.347084] pci_bus 0000:00: resource 6 mem: [0x840000000-0xfcffffffff]
> [    9.444440] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
> [    9.450099] pci_bus 0000:40: resource 1 io:  [0xf000-0xffff]
> [    9.455757] pci_bus 0000:40: resource 2 mem: [0xdb000000-0xdcffffff]
> [    9.498118] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
> [    9.503777] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]

It's interesting that many of the differences involve the legacy
VGA I/O ports in the 0x3b0-0x3df range.  My guess is that the AMD
chipset has special routing for those ranges.  If it didn't, it
would be difficult to support VGA devices under the other two
root bridges.  Maybe that VGA routing doesn't show up in the
bridge's PCI config space.  Can you tell from the ASL whether the
root bridge _SRS/_PRS/_CRS methods handle the VGA ranges specially?

One of the differences is that PCI config space shows a 64-bit region
(bus 0000:00 mem 0x840000000-0xfcffffffff) that doesn't show up in
the _CRS info.  But the _CRS parsing depends on acpi_resource_to_address64(),
which doesn't know about the ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64
descriptors added in ACPI 3.0.  So this difference could be a result
of that Linux bug.  It'd be interesting to see whether the test patch
below makes a difference.

The PCI config space region 0xf0000000-0xffffffff, also on bus 0000:00,
looks suspicious to me.  I thought that area contained a bunch of
BIOS-y things like reset vectors and local APICs.

Bjorn



diff --git a/drivers/acpi/acpica/rsxface.c b/drivers/acpi/acpica/rsxface.c
index 69a2aa5..dc2c5cb 100644
--- a/drivers/acpi/acpica/rsxface.c
+++ b/drivers/acpi/acpica/rsxface.c
@@ -62,8 +62,7 @@ ACPI_MODULE_NAME("rsxface")
 	ACPI_COPY_FIELD(out, in, minimum);                   \
 	ACPI_COPY_FIELD(out, in, maximum);                   \
 	ACPI_COPY_FIELD(out, in, translation_offset);        \
-	ACPI_COPY_FIELD(out, in, address_length);            \
-	ACPI_COPY_FIELD(out, in, resource_source);
+	ACPI_COPY_FIELD(out, in, address_length);
 /* Local prototypes */
 static acpi_status
 acpi_rs_match_vendor_resource(struct acpi_resource *resource, void *context);
@@ -328,6 +327,7 @@ acpi_resource_to_address64(struct acpi_resource *resource,
 {
 	struct acpi_resource_address16 *address16;
 	struct acpi_resource_address32 *address32;
+	struct acpi_resource_extended_address64 *address64;
 
 	if (!resource || !out) {
 		return (AE_BAD_PARAMETER);
@@ -356,6 +356,13 @@ acpi_resource_to_address64(struct acpi_resource *resource,
 			    sizeof(struct acpi_resource_address64));
 		break;
 
+	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
+
+		address64 = (struct acpi_resource_extended_address64 *)
+			&resource->data;
+		ACPI_COPY_ADDRESS(out, address64);
+		break;
+
 	default:
 		return (AE_BAD_PARAMETER);
 	}

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Yinghai Lu]

On Mon, Apr 27, 2009 at 3:24 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Monday 27 April 2009 03:00:16 pm Yinghai Lu wrote:
>> On Mon, Apr 27, 2009 at 1:39 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
>> >> other system may have broken _CRS.
>> >
>> > Do you have examples of problems here, or are you just worried that
>> > there *may* be problems?
>> one system with three chains... with pci=use_crs
>> [    9.365669] pci_bus 0000:00: resource 0 io:  [0x00-0x3af]
>> [    9.371065] pci_bus 0000:00: resource 1 io:  [0x3e0-0xcf7]
>> [    9.376551] pci_bus 0000:00: resource 2 io:  [0x3b0-0x3bb]
>> [    9.382028] pci_bus 0000:00: resource 3 io:  [0x3c0-0x3df]
>> [    9.387513] pci_bus 0000:00: resource 4 io:  [0xd00-0xefff]
>> [    9.393077] pci_bus 0000:00: resource 5 mem: [0x0a0000-0x0bffff]
>> [    9.399084] pci_bus 0000:00: resource 6 mem: [0x0d0000-0x0dffff]
>> [    9.405089] pci_bus 0000:00: resource 7 mem: [0xdd000000-0xdfffffff]
>> [    9.505332] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
>> [    9.510991] pci_bus 0000:40: resource 1 mem: [0xdb000000-0xdcffffff]
>> [    9.553378] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
>> [    9.559036] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]
>>
>> without that: amd_bus.c will read that from pci conf space
>> [    9.310965] pci_bus 0000:00: resource 0 io:  [0x9000-0xefff]
>> [    9.316621] pci_bus 0000:00: resource 1 io:  [0x00-0xfff]
>> [    9.322020] pci_bus 0000:00: resource 2 mem: [0xdd000000-0xdfffffff]
>> [    9.328373] pci_bus 0000:00: resource 3 mem: [0x0a0000-0x0bffff]
>> [    9.334378] pci_bus 0000:00: resource 4 mem: [0xc0000000-0xd9ffffff]
>> [    9.340731] pci_bus 0000:00: resource 5 mem: [0xf0000000-0xffffffff]
>> [    9.347084] pci_bus 0000:00: resource 6 mem: [0x840000000-0xfcffffffff]
>> [    9.444440] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
>> [    9.450099] pci_bus 0000:40: resource 1 io:  [0xf000-0xffff]
>> [    9.455757] pci_bus 0000:40: resource 2 mem: [0xdb000000-0xdcffffff]
>> [    9.498118] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
>> [    9.503777] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]
>
> It's interesting that many of the differences involve the legacy
> VGA I/O ports in the 0x3b0-0x3df range.  My guess is that the AMD
> chipset has special routing for those ranges.  If it didn't, it
> would be difficult to support VGA devices under the other two
> root bridges.  Maybe that VGA routing doesn't show up in the
> bridge's PCI config space.  Can you tell from the ASL whether the
> root bridge _SRS/_PRS/_CRS methods handle the VGA ranges specially?
>
> One of the differences is that PCI config space shows a 64-bit region
> (bus 0000:00 mem 0x840000000-0xfcffffffff) that doesn't show up in
> the _CRS info.  But the _CRS parsing depends on acpi_resource_to_address64(),
> which doesn't know about the ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64
> descriptors added in ACPI 3.0.  So this difference could be a result
> of that Linux bug.  It'd be interesting to see whether the test patch
> below makes a difference.
will check it.
>
> The PCI config space region 0xf0000000-0xffffffff, also on bus 0000:00,
> looks suspicious to me.  I thought that area contained a bunch of
> BIOS-y things like reset vectors and local APICs.

in the amd_bus.c, will put left over resource to def HT chain.

YH

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Bjorn Helgaas]

On Monday 27 April 2009 08:07:01 pm Yinghai Lu wrote:
> On Mon, Apr 27, 2009 at 3:24 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Monday 27 April 2009 03:00:16 pm Yinghai Lu wrote:
> >> On Mon, Apr 27, 2009 at 1:39 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> >> >> other system may have broken _CRS.
> >> >
> >> > Do you have examples of problems here, or are you just worried that
> >> > there *may* be problems?
> >> one system with three chains... with pci=use_crs
> >> [    9.365669] pci_bus 0000:00: resource 0 io:  [0x00-0x3af]
> >> [    9.371065] pci_bus 0000:00: resource 1 io:  [0x3e0-0xcf7]
> >> [    9.376551] pci_bus 0000:00: resource 2 io:  [0x3b0-0x3bb]
> >> [    9.382028] pci_bus 0000:00: resource 3 io:  [0x3c0-0x3df]
> >> [    9.387513] pci_bus 0000:00: resource 4 io:  [0xd00-0xefff]
> >> [    9.393077] pci_bus 0000:00: resource 5 mem: [0x0a0000-0x0bffff]
> >> [    9.399084] pci_bus 0000:00: resource 6 mem: [0x0d0000-0x0dffff]
> >> [    9.405089] pci_bus 0000:00: resource 7 mem: [0xdd000000-0xdfffffff]
> >> [    9.505332] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
> >> [    9.510991] pci_bus 0000:40: resource 1 mem: [0xdb000000-0xdcffffff]
> >> [    9.553378] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
> >> [    9.559036] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]
> >>
> >> without that: amd_bus.c will read that from pci conf space
> >> [    9.310965] pci_bus 0000:00: resource 0 io:  [0x9000-0xefff]
> >> [    9.316621] pci_bus 0000:00: resource 1 io:  [0x00-0xfff]
> >> [    9.322020] pci_bus 0000:00: resource 2 mem: [0xdd000000-0xdfffffff]
> >> [    9.328373] pci_bus 0000:00: resource 3 mem: [0x0a0000-0x0bffff]
> >> [    9.334378] pci_bus 0000:00: resource 4 mem: [0xc0000000-0xd9ffffff]
> >> [    9.340731] pci_bus 0000:00: resource 5 mem: [0xf0000000-0xffffffff]
> >> [    9.347084] pci_bus 0000:00: resource 6 mem: [0x840000000-0xfcffffffff]
> >> [    9.444440] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
> >> [    9.450099] pci_bus 0000:40: resource 1 io:  [0xf000-0xffff]
> >> [    9.455757] pci_bus 0000:40: resource 2 mem: [0xdb000000-0xdcffffff]
> >> [    9.498118] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
> >> [    9.503777] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]
> >
> > It's interesting that many of the differences involve the legacy
> > VGA I/O ports in the 0x3b0-0x3df range.  My guess is that the AMD
> > chipset has special routing for those ranges.  If it didn't, it
> > would be difficult to support VGA devices under the other two
> > root bridges.  Maybe that VGA routing doesn't show up in the
> > bridge's PCI config space.  Can you tell from the ASL whether the
> > root bridge _SRS/_PRS/_CRS methods handle the VGA ranges specially?
> >
> > One of the differences is that PCI config space shows a 64-bit region
> > (bus 0000:00 mem 0x840000000-0xfcffffffff) that doesn't show up in
> > the _CRS info.  But the _CRS parsing depends on acpi_resource_to_address64(),
> > which doesn't know about the ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64
> > descriptors added in ACPI 3.0.  So this difference could be a result
> > of that Linux bug.  It'd be interesting to see whether the test patch
> > below makes a difference.
> will check it.

Did you learn anything about this?  I have a PNPACPI patch to parse
these new descriptors, but I don't have any machines where I can test
it.  If your box uses that descriptor, it'd be nice to test the patch
there.

> > The PCI config space region 0xf0000000-0xffffffff, also on bus 0000:00,
> > looks suspicious to me.  I thought that area contained a bunch of
> > BIOS-y things like reset vectors and local APICs.
> 
> in the amd_bus.c, will put left over resource to def HT chain.
> 
> YH
> 

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Bjorn Helgaas]

On Wednesday 29 April 2009 05:08:51 pm Bjorn Helgaas wrote:
> On Monday 27 April 2009 08:07:01 pm Yinghai Lu wrote:
> > On Mon, Apr 27, 2009 at 3:24 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > On Monday 27 April 2009 03:00:16 pm Yinghai Lu wrote:
> > >> On Mon, Apr 27, 2009 at 1:39 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > >> >> other system may have broken _CRS.
> > >> >
> > >> > Do you have examples of problems here, or are you just worried that
> > >> > there *may* be problems?
> > >> one system with three chains... with pci=use_crs
> > >> [    9.365669] pci_bus 0000:00: resource 0 io:  [0x00-0x3af]
> > >> [    9.371065] pci_bus 0000:00: resource 1 io:  [0x3e0-0xcf7]
> > >> [    9.376551] pci_bus 0000:00: resource 2 io:  [0x3b0-0x3bb]
> > >> [    9.382028] pci_bus 0000:00: resource 3 io:  [0x3c0-0x3df]
> > >> [    9.387513] pci_bus 0000:00: resource 4 io:  [0xd00-0xefff]
> > >> [    9.393077] pci_bus 0000:00: resource 5 mem: [0x0a0000-0x0bffff]
> > >> [    9.399084] pci_bus 0000:00: resource 6 mem: [0x0d0000-0x0dffff]
> > >> [    9.405089] pci_bus 0000:00: resource 7 mem: [0xdd000000-0xdfffffff]
> > >> [    9.505332] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
> > >> [    9.510991] pci_bus 0000:40: resource 1 mem: [0xdb000000-0xdcffffff]
> > >> [    9.553378] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
> > >> [    9.559036] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]
> > >>
> > >> without that: amd_bus.c will read that from pci conf space
> > >> [    9.310965] pci_bus 0000:00: resource 0 io:  [0x9000-0xefff]
> > >> [    9.316621] pci_bus 0000:00: resource 1 io:  [0x00-0xfff]
> > >> [    9.322020] pci_bus 0000:00: resource 2 mem: [0xdd000000-0xdfffffff]
> > >> [    9.328373] pci_bus 0000:00: resource 3 mem: [0x0a0000-0x0bffff]
> > >> [    9.334378] pci_bus 0000:00: resource 4 mem: [0xc0000000-0xd9ffffff]
> > >> [    9.340731] pci_bus 0000:00: resource 5 mem: [0xf0000000-0xffffffff]
> > >> [    9.347084] pci_bus 0000:00: resource 6 mem: [0x840000000-0xfcffffffff]
> > >> [    9.444440] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
> > >> [    9.450099] pci_bus 0000:40: resource 1 io:  [0xf000-0xffff]
> > >> [    9.455757] pci_bus 0000:40: resource 2 mem: [0xdb000000-0xdcffffff]
> > >> [    9.498118] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
> > >> [    9.503777] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]
> > >
> > > It's interesting that many of the differences involve the legacy
> > > VGA I/O ports in the 0x3b0-0x3df range.  My guess is that the AMD
> > > chipset has special routing for those ranges.  If it didn't, it
> > > would be difficult to support VGA devices under the other two
> > > root bridges.  Maybe that VGA routing doesn't show up in the
> > > bridge's PCI config space.  Can you tell from the ASL whether the
> > > root bridge _SRS/_PRS/_CRS methods handle the VGA ranges specially?
> > >
> > > One of the differences is that PCI config space shows a 64-bit region
> > > (bus 0000:00 mem 0x840000000-0xfcffffffff) that doesn't show up in
> > > the _CRS info.  But the _CRS parsing depends on acpi_resource_to_address64(),
> > > which doesn't know about the ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64
> > > descriptors added in ACPI 3.0.  So this difference could be a result
> > > of that Linux bug.  It'd be interesting to see whether the test patch
> > > below makes a difference.
> > will check it.
> 
> Did you learn anything about this?  I have a PNPACPI patch to parse
> these new descriptors, but I don't have any machines where I can test
> it.  If your box uses that descriptor, it'd be nice to test the patch
> there.

Oops, I should have just attached the PNPACPI patch in case anybody
has a box where it can be tested.  One way to test it would be to
compare the output of "grep . /sys/devices/pnp*/*/{id,resources,options}"
before and after the patch.  If a BIOS uses the new descriptors, we
should see some new resources after the patch.


PNPACPI: parse Extended Address Space Descriptors

From: Bjorn Helgaas <bjorn.helgaas@hp.com>

Extended Address Space Descriptors are new in ACPI 3.0 and allow the
BIOS to communicate device resource cacheability attributes (write-back,
write-through, uncacheable, etc) to the OS.

Previously, PNPACPI ignored these descriptors, so if a BIOS used them,
a device could be responding at addresses the OS doesn't know about.
This patch adds support for these descriptors in _CRS and _PRS.  We
don't attempt to encode them for _SRS (just like we don't attempt to
encode the existing 16-, 32-, and 64-bit Address Space Descriptors).

Unfortunately, I don't have a way to test this.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/pnp/pnpacpi/rsparser.c |   45 ++++++++++++++++++++++++++++++++++++++--
 1 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
index adf1785..0864242 100644
--- a/drivers/pnp/pnpacpi/rsparser.c
+++ b/drivers/pnp/pnpacpi/rsparser.c
@@ -287,6 +287,25 @@ static void pnpacpi_parse_allocated_address_space(struct pnp_dev *dev,
 				ACPI_DECODE_16);
 }
 
+static void pnpacpi_parse_allocated_ext_address_space(struct pnp_dev *dev,
+						      struct acpi_resource *res)
+{
+	struct acpi_resource_extended_address64 *p = &res->data.ext_address64;
+
+	if (p->producer_consumer == ACPI_PRODUCER)
+		return;
+
+	if (p->resource_type == ACPI_MEMORY_RANGE)
+		pnpacpi_parse_allocated_memresource(dev,
+			p->minimum, p->address_length,
+			p->info.mem.write_protect);
+	else if (p->resource_type == ACPI_IO_RANGE)
+		pnpacpi_parse_allocated_ioresource(dev,
+			p->minimum, p->address_length,
+			p->granularity == 0xfff ? ACPI_DECODE_10 :
+				ACPI_DECODE_16);
+}
+
 static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
 					      void *data)
 {
@@ -400,8 +419,7 @@ static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
-		if (res->data.ext_address64.producer_consumer == ACPI_PRODUCER)
-			return AE_OK;
+		pnpacpi_parse_allocated_ext_address_space(dev, res);
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
@@ -630,6 +648,28 @@ static __init void pnpacpi_parse_address_option(struct pnp_dev *dev,
 					   IORESOURCE_IO_FIXED);
 }
 
+static __init void pnpacpi_parse_ext_address_option(struct pnp_dev *dev,
+						    unsigned int option_flags,
+						    struct acpi_resource *r)
+{
+	struct acpi_resource_extended_address64 *p = &r->data.ext_address64;
+	unsigned char flags = 0;
+
+	if (p->address_length == 0)
+		return;
+
+	if (p->resource_type == ACPI_MEMORY_RANGE) {
+		if (p->info.mem.write_protect == ACPI_READ_WRITE_MEMORY)
+			flags = IORESOURCE_MEM_WRITEABLE;
+		pnp_register_mem_resource(dev, option_flags, p->minimum,
+					  p->minimum, 0, p->address_length,
+					  flags);
+	} else if (p->resource_type == ACPI_IO_RANGE)
+		pnp_register_port_resource(dev, option_flags, p->minimum,
+					   p->minimum, 0, p->address_length,
+					   IORESOURCE_IO_FIXED);
+}
+
 struct acpipnp_parse_option_s {
 	struct pnp_dev *dev;
 	unsigned int option_flags;
@@ -711,6 +751,7 @@ static __init acpi_status pnpacpi_option_resource(struct acpi_resource *res,
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
+		pnpacpi_parse_ext_address_option(dev, option_flags, res);
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Bjorn Helgaas]

On Thursday 30 April 2009 09:14:21 am Bjorn Helgaas wrote:
> On Wednesday 29 April 2009 05:08:51 pm Bjorn Helgaas wrote:
> > On Monday 27 April 2009 08:07:01 pm Yinghai Lu wrote:
> > > On Mon, Apr 27, 2009 at 3:24 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > > On Monday 27 April 2009 03:00:16 pm Yinghai Lu wrote:
> > > >> On Mon, Apr 27, 2009 at 1:39 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > >> >> other system may have broken _CRS.
> > > >> >
> > > >> > Do you have examples of problems here, or are you just worried that
> > > >> > there *may* be problems?
> > > >> one system with three chains... with pci=use_crs
> > > >> [    9.365669] pci_bus 0000:00: resource 0 io:  [0x00-0x3af]
> > > >> [    9.371065] pci_bus 0000:00: resource 1 io:  [0x3e0-0xcf7]
> > > >> [    9.376551] pci_bus 0000:00: resource 2 io:  [0x3b0-0x3bb]
> > > >> [    9.382028] pci_bus 0000:00: resource 3 io:  [0x3c0-0x3df]
> > > >> [    9.387513] pci_bus 0000:00: resource 4 io:  [0xd00-0xefff]
> > > >> [    9.393077] pci_bus 0000:00: resource 5 mem: [0x0a0000-0x0bffff]
> > > >> [    9.399084] pci_bus 0000:00: resource 6 mem: [0x0d0000-0x0dffff]
> > > >> [    9.405089] pci_bus 0000:00: resource 7 mem: [0xdd000000-0xdfffffff]
> > > >> [    9.505332] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
> > > >> [    9.510991] pci_bus 0000:40: resource 1 mem: [0xdb000000-0xdcffffff]
> > > >> [    9.553378] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
> > > >> [    9.559036] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]
> > > >>
> > > >> without that: amd_bus.c will read that from pci conf space
> > > >> [    9.310965] pci_bus 0000:00: resource 0 io:  [0x9000-0xefff]
> > > >> [    9.316621] pci_bus 0000:00: resource 1 io:  [0x00-0xfff]
> > > >> [    9.322020] pci_bus 0000:00: resource 2 mem: [0xdd000000-0xdfffffff]
> > > >> [    9.328373] pci_bus 0000:00: resource 3 mem: [0x0a0000-0x0bffff]
> > > >> [    9.334378] pci_bus 0000:00: resource 4 mem: [0xc0000000-0xd9ffffff]
> > > >> [    9.340731] pci_bus 0000:00: resource 5 mem: [0xf0000000-0xffffffff]
> > > >> [    9.347084] pci_bus 0000:00: resource 6 mem: [0x840000000-0xfcffffffff]
> > > >> [    9.444440] pci_bus 0000:40: resource 0 io:  [0x5000-0x8fff]
> > > >> [    9.450099] pci_bus 0000:40: resource 1 io:  [0xf000-0xffff]
> > > >> [    9.455757] pci_bus 0000:40: resource 2 mem: [0xdb000000-0xdcffffff]
> > > >> [    9.498118] pci_bus 0000:80: resource 0 io:  [0x1000-0x4fff]
> > > >> [    9.503777] pci_bus 0000:80: resource 1 mem: [0xda000000-0xdaffffff]
> > > >
> > > > It's interesting that many of the differences involve the legacy
> > > > VGA I/O ports in the 0x3b0-0x3df range.  My guess is that the AMD
> > > > chipset has special routing for those ranges.  If it didn't, it
> > > > would be difficult to support VGA devices under the other two
> > > > root bridges.  Maybe that VGA routing doesn't show up in the
> > > > bridge's PCI config space.  Can you tell from the ASL whether the
> > > > root bridge _SRS/_PRS/_CRS methods handle the VGA ranges specially?
> > > >
> > > > One of the differences is that PCI config space shows a 64-bit region
> > > > (bus 0000:00 mem 0x840000000-0xfcffffffff) that doesn't show up in
> > > > the _CRS info.  But the _CRS parsing depends on acpi_resource_to_address64(),
> > > > which doesn't know about the ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64
> > > > descriptors added in ACPI 3.0.  So this difference could be a result
> > > > of that Linux bug.  It'd be interesting to see whether the test patch
> > > > below makes a difference.
> > > will check it.
> > 
> > Did you learn anything about this?  I have a PNPACPI patch to parse
> > these new descriptors, but I don't have any machines where I can test
> > it.  If your box uses that descriptor, it'd be nice to test the patch
> > there.
> 
> Oops, I should have just attached the PNPACPI patch in case anybody
> has a box where it can be tested.  One way to test it would be to
> compare the output of "grep . /sys/devices/pnp*/*/{id,resources,options}"
> before and after the patch.  If a BIOS uses the new descriptors, we
> should see some new resources after the patch.

Did anything happen with this?

The longer we wait to make "use_crs" the default, the harder it
will be, so I'd like to push ahead.

Bjorn

> PNPACPI: parse Extended Address Space Descriptors
> 
> From: Bjorn Helgaas <bjorn.helgaas@hp.com>
> 
> Extended Address Space Descriptors are new in ACPI 3.0 and allow the
> BIOS to communicate device resource cacheability attributes (write-back,
> write-through, uncacheable, etc) to the OS.
> 
> Previously, PNPACPI ignored these descriptors, so if a BIOS used them,
> a device could be responding at addresses the OS doesn't know about.
> This patch adds support for these descriptors in _CRS and _PRS.  We
> don't attempt to encode them for _SRS (just like we don't attempt to
> encode the existing 16-, 32-, and 64-bit Address Space Descriptors).
> 
> Unfortunately, I don't have a way to test this.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> ---
>  drivers/pnp/pnpacpi/rsparser.c |   45 ++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
> index adf1785..0864242 100644
> --- a/drivers/pnp/pnpacpi/rsparser.c
> +++ b/drivers/pnp/pnpacpi/rsparser.c
> @@ -287,6 +287,25 @@ static void pnpacpi_parse_allocated_address_space(struct pnp_dev *dev,
>  				ACPI_DECODE_16);
>  }
>  
> +static void pnpacpi_parse_allocated_ext_address_space(struct pnp_dev *dev,
> +						      struct acpi_resource *res)
> +{
> +	struct acpi_resource_extended_address64 *p = &res->data.ext_address64;
> +
> +	if (p->producer_consumer == ACPI_PRODUCER)
> +		return;
> +
> +	if (p->resource_type == ACPI_MEMORY_RANGE)
> +		pnpacpi_parse_allocated_memresource(dev,
> +			p->minimum, p->address_length,
> +			p->info.mem.write_protect);
> +	else if (p->resource_type == ACPI_IO_RANGE)
> +		pnpacpi_parse_allocated_ioresource(dev,
> +			p->minimum, p->address_length,
> +			p->granularity == 0xfff ? ACPI_DECODE_10 :
> +				ACPI_DECODE_16);
> +}
> +
>  static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
>  					      void *data)
>  {
> @@ -400,8 +419,7 @@ static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
>  		break;
>  
>  	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
> -		if (res->data.ext_address64.producer_consumer == ACPI_PRODUCER)
> -			return AE_OK;
> +		pnpacpi_parse_allocated_ext_address_space(dev, res);
>  		break;
>  
>  	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
> @@ -630,6 +648,28 @@ static __init void pnpacpi_parse_address_option(struct pnp_dev *dev,
>  					   IORESOURCE_IO_FIXED);
>  }
>  
> +static __init void pnpacpi_parse_ext_address_option(struct pnp_dev *dev,
> +						    unsigned int option_flags,
> +						    struct acpi_resource *r)
> +{
> +	struct acpi_resource_extended_address64 *p = &r->data.ext_address64;
> +	unsigned char flags = 0;
> +
> +	if (p->address_length == 0)
> +		return;
> +
> +	if (p->resource_type == ACPI_MEMORY_RANGE) {
> +		if (p->info.mem.write_protect == ACPI_READ_WRITE_MEMORY)
> +			flags = IORESOURCE_MEM_WRITEABLE;
> +		pnp_register_mem_resource(dev, option_flags, p->minimum,
> +					  p->minimum, 0, p->address_length,
> +					  flags);
> +	} else if (p->resource_type == ACPI_IO_RANGE)
> +		pnp_register_port_resource(dev, option_flags, p->minimum,
> +					   p->minimum, 0, p->address_length,
> +					   IORESOURCE_IO_FIXED);
> +}
> +
>  struct acpipnp_parse_option_s {
>  	struct pnp_dev *dev;
>  	unsigned int option_flags;
> @@ -711,6 +751,7 @@ static __init acpi_status pnpacpi_option_resource(struct acpi_resource *res,
>  		break;
>  
>  	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
> +		pnpacpi_parse_ext_address_option(dev, option_flags, res);
>  		break;
>  
>  	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Jesse Barnes]

On Fri, 8 May 2009 16:40:10 -0600
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> Did anything happen with this?
> 
> The longer we wait to make "use_crs" the default, the harder it
> will be, so I'd like to push ahead.

Here's a patch to make CRS the default.  If it looks ok I can push it
into my linux-next branch.  I'm all for using reliable data from the
BIOS.  I guess we'll find out fairly quickly if this stuff isn't...

-- 
Jesse Barnes, Intel Open Source Technology Center

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 90b3924..8d798bf 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1777,7 +1777,7 @@ and is between 256 and 4096 characters. It is defined in the file
 				IRQ routing is enabled.
 		noacpi		[X86] Do not use ACPI for IRQ routing
 				or for PCI scanning.
-		use_crs		[X86] Use _CRS for PCI resource
+		nocrs		[X86] Don't use _CRS for PCI resource
 				allocation.
 		routeirq	Do IRQ routing for all PCI devices.
 				This is normally done in pci_enable_device(),
diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index e60fd3e..010c08c 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -25,7 +25,7 @@
 #define PCI_BIOS_IRQ_SCAN	0x2000
 #define PCI_ASSIGN_ALL_BUSSES	0x4000
 #define PCI_CAN_SKIP_ISA_ALIGN	0x8000
-#define PCI_USE__CRS		0x10000
+#define PCI_NO_CRS		0x10000
 #define PCI_CHECK_ENABLE_AMD_MMCONF	0x20000
 #define PCI_HAS_IO_ECS		0x40000
 #define PCI_NOASSIGN_ROMS	0x80000
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index c0ecf25..9f562a3 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -217,7 +217,7 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_device *device, int do
 #endif
 	}
 
-	if (bus && (pci_probe & PCI_USE__CRS))
+	if (bus && !(pci_probe & PCI_NO_CRS))
 		get_current_resources(device, busnum, domain, bus);
 	return bus;
 }
diff --git a/arch/x86/pci/amd_bus.c b/arch/x86/pci/amd_bus.c
index f893d6a..cfed6b1 100644
--- a/arch/x86/pci/amd_bus.c
+++ b/arch/x86/pci/amd_bus.c
@@ -101,7 +101,7 @@ void x86_pci_root_bus_res_quirks(struct pci_bus *b)
 	struct pci_root_info *info;
 
 	/* don't go for it if _CRS is used */
-	if (pci_probe & PCI_USE__CRS)
+	if (!(pci_probe & PCI_NO_CRS)
 		return;
 
 	/* if only one root bus, don't need to anything */
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 2202b62..3e361c7 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -515,8 +515,8 @@ char * __devinit  pcibios_setup(char *str)
 	} else if (!strcmp(str, "assign-busses")) {
 		pci_probe |= PCI_ASSIGN_ALL_BUSSES;
 		return NULL;
-	} else if (!strcmp(str, "use_crs")) {
-		pci_probe |= PCI_USE__CRS;
+	} else if (!strcmp(str, "nocrs")) {
+		pci_probe |= PCI_NO_CRS;
 		return NULL;
 	} else if (!strcmp(str, "earlydump")) {
 		pci_early_dump_regs = 1;

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Yinghai Lu]

On Wed, May 20, 2009 at 4:49 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Fri, 8 May 2009 16:40:10 -0600
> Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
>> Did anything happen with this?
>>
>> The longer we wait to make "use_crs" the default, the harder it
>> will be, so I'd like to push ahead.
>
> Here's a patch to make CRS the default.  If it looks ok I can push it
> into my linux-next branch.  I'm all for using reliable data from the
> BIOS.  I guess we'll find out fairly quickly if this stuff isn't...
>
> --
> Jesse Barnes, Intel Open Source Technology Center
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 90b3924..8d798bf 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1777,7 +1777,7 @@ and is between 256 and 4096 characters. It is defined in the file
>                                IRQ routing is enabled.
>                noacpi          [X86] Do not use ACPI for IRQ routing
>                                or for PCI scanning.
> -               use_crs         [X86] Use _CRS for PCI resource
> +               nocrs           [X86] Don't use _CRS for PCI resource
>                                allocation.
>                routeirq        Do IRQ routing for all PCI devices.
>                                This is normally done in pci_enable_device(),
> diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
> index e60fd3e..010c08c 100644
> --- a/arch/x86/include/asm/pci_x86.h
> +++ b/arch/x86/include/asm/pci_x86.h
> @@ -25,7 +25,7 @@
>  #define PCI_BIOS_IRQ_SCAN      0x2000
>  #define PCI_ASSIGN_ALL_BUSSES  0x4000
>  #define PCI_CAN_SKIP_ISA_ALIGN 0x8000
> -#define PCI_USE__CRS           0x10000
> +#define PCI_NO_CRS             0x10000

PCI_NO_ROOT_CRS is more accurate ?

YH

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Bjorn Helgaas]

On Wednesday 20 May 2009 5:49:10 pm Jesse Barnes wrote:
> On Fri, 8 May 2009 16:40:10 -0600
> Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > Did anything happen with this?
> > 
> > The longer we wait to make "use_crs" the default, the harder it
> > will be, so I'd like to push ahead.
> 
> Here's a patch to make CRS the default.  If it looks ok I can push it
> into my linux-next branch.  I'm all for using reliable data from the
> BIOS.  I guess we'll find out fairly quickly if this stuff isn't...

Thanks for following up on this, Jesse.  It was on my to-do list for
yesterday, but I didn't get to it.

Yinghai mentioned a specific box where we might have trouble, but we
never got enough details to really debug it.  So I think we might as
well give it a shot and fine-tune it as we need to.

Bjorn

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Gary Hade]

On Thu, May 21, 2009 at 08:46:17AM -0600, Bjorn Helgaas wrote:
> On Wednesday 20 May 2009 5:49:10 pm Jesse Barnes wrote:
> > On Fri, 8 May 2009 16:40:10 -0600
> > Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > Did anything happen with this?
> > > 
> > > The longer we wait to make "use_crs" the default, the harder it
> > > will be, so I'd like to push ahead.
> > 
> > Here's a patch to make CRS the default.  If it looks ok I can push it
> > into my linux-next branch.  I'm all for using reliable data from the
> > BIOS.  I guess we'll find out fairly quickly if this stuff isn't...
> 
> Thanks for following up on this, Jesse.  It was on my to-do list for
> yesterday, but I didn't get to it.
> 
> Yinghai mentioned a specific box where we might have trouble, but we
> never got enough details to really debug it.  So I think we might as
> well give it a shot and fine-tune it as we need to.

I just remembered that Andrew Morton once raised some concerns
related to the number of _CRS returned resources exceeding the
fixed resource array size -> http://lkml.org/lkml/2007/11/1/49

I believe PCI_BUS_NUM_RESOURCES was later increased but if 
that increase was insufficient to cover all systems that the
code will now be exposed to, it seems like the "And should we
really be silently ignoring this problem?  Should we at least
report it?" comment that was not addressed could become relevent.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Gary Hade]

On Thu, May 21, 2009 at 09:37:37AM -0700, Gary Hade wrote:
> On Thu, May 21, 2009 at 08:46:17AM -0600, Bjorn Helgaas wrote:
> > On Wednesday 20 May 2009 5:49:10 pm Jesse Barnes wrote:
> > > On Fri, 8 May 2009 16:40:10 -0600
> > > Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > > Did anything happen with this?
> > > > 
> > > > The longer we wait to make "use_crs" the default, the harder it
> > > > will be, so I'd like to push ahead.
> > > 
> > > Here's a patch to make CRS the default.  If it looks ok I can push it
> > > into my linux-next branch.  I'm all for using reliable data from the
> > > BIOS.  I guess we'll find out fairly quickly if this stuff isn't...
> > 
> > Thanks for following up on this, Jesse.  It was on my to-do list for
> > yesterday, but I didn't get to it.
> > 
> > Yinghai mentioned a specific box where we might have trouble, but we
> > never got enough details to really debug it.  So I think we might as
> > well give it a shot and fine-tune it as we need to.
> 
> I just remembered that Andrew Morton once raised some concerns
> related to the number of _CRS returned resources exceeding the
> fixed resource array size -> http://lkml.org/lkml/2007/11/1/49
> 
> I believe PCI_BUS_NUM_RESOURCES was later increased but if 
> that increase was insufficient to cover all systems that the
> code will now be exposed to, it seems like the "And should we
> really be silently ignoring this problem?  Should we at least
> report it?" comment that was not addressed could become relevent.

Not only do we not warn, if we did warn based on the current
    'if (info->res_num >= PCI_BUS_NUM_RESOURCES)'
resource array overrun avoidance criteria it would be incorrect
for a root bus that is connected to a subordinate bus with a
transparent bridge.  We need to avoid the last 3 slots of the
resource array since they do not get mapped to a transparent
bridge connected subordinate bus.  I believe the below patch
addresses these issues.

With PCI_BUS_NUM_RESOURCES currently at 16 we still have space
to accomodate 13 _CRS returned resource descriptors which will
hopefully be sufficient.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc


From: Gary Hade <garyhade@us.ibm.com>

Correct maximum allowed _CRS returned resources and warn if exceeded.

Issue a warning if _CRS returns too many resource descriptors
to be accommodated by the fixed size resource array instances.
If there is no transparent bridge on the root bus "too many"
is the PCI_BUS_NUM_RESOURCES size of the resource array.
Otherwise, the last 3 slots of the resource array must be
excluded making the maximum (PCI_BUS_NUM_RESOURCES - 3).

The current code:
 - is silent when _CRS returns too many resource descriptors and
 - incorrectly allows use of the last 3 slots of the resource array
   for a root bus with a transparent bridge

Signed-off-by: Gary Hade <garyhade@us.ibm.com>

---
 arch/x86/pci/acpi.c |   33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

Index: linux-2.6.28-rc7/arch/x86/pci/acpi.c
===================================================================
--- linux-2.6.28-rc7.orig/arch/x86/pci/acpi.c	2009-05-27 10:42:10.000000000 -0700
+++ linux-2.6.28-rc7/arch/x86/pci/acpi.c	2009-05-27 10:43:35.000000000 -0700
@@ -38,15 +38,26 @@ count_resource(struct acpi_resource *acp
 	struct acpi_resource_address64 addr;
 	acpi_status status;
 
-	if (info->res_num >= PCI_BUS_NUM_RESOURCES)
-		return AE_OK;
-
 	status = resource_to_addr(acpi_res, &addr);
 	if (ACPI_SUCCESS(status))
 		info->res_num++;
 	return AE_OK;
 }
 
+static int
+bus_has_transparent_bridge(struct pci_bus *bus)
+{
+	struct pci_dev *dev;
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		u16 class = dev->class >> 8;
+
+		if (class == PCI_CLASS_BRIDGE_PCI && dev->transparent)
+			return true;
+	}
+	return false;
+}
+
 static acpi_status
 setup_resource(struct acpi_resource *acpi_res, void *data)
 {
@@ -56,9 +67,7 @@ setup_resource(struct acpi_resource *acp
 	acpi_status status;
 	unsigned long flags;
 	struct resource *root;
-
-	if (info->res_num >= PCI_BUS_NUM_RESOURCES)
-		return AE_OK;
+	int max_root_bus_resources = PCI_BUS_NUM_RESOURCES;
 
 	status = resource_to_addr(acpi_res, &addr);
 	if (!ACPI_SUCCESS(status))
@@ -82,6 +91,18 @@ setup_resource(struct acpi_resource *acp
 	res->end = res->start + addr.address_length - 1;
 	res->child = NULL;
 
+	if (bus_has_transparent_bridge(info->bus))
+		max_root_bus_resources -= 3;
+	if (info->res_num >= max_root_bus_resources) {
+		printk(KERN_WARNING "PCI: Failed to allocate 0x%lx-0x%lx "
+			"from %s for %s due to _CRS returning more than "
+			"%d resource descriptors\n", (unsigned long) res->start,
+			(unsigned long) res->end, root->name, info->name,
+			max_root_bus_resources);
+		info->res_num++;
+		return AE_OK;
+	}
+
 	if (insert_resource(root, res)) {
 		printk(KERN_ERR "PCI: Failed to allocate 0x%lx-0x%lx "
 			"from %s for %s\n", (unsigned long) res->start,

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Jesse Barnes]

On Wed, 27 May 2009 12:41:44 -0700
Gary Hade <garyhade@us.ibm.com> wrote:

> On Thu, May 21, 2009 at 09:37:37AM -0700, Gary Hade wrote:
> > On Thu, May 21, 2009 at 08:46:17AM -0600, Bjorn Helgaas wrote:
> > > On Wednesday 20 May 2009 5:49:10 pm Jesse Barnes wrote:
> > > > On Fri, 8 May 2009 16:40:10 -0600
> > > > Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > > > Did anything happen with this?
> > > > > 
> > > > > The longer we wait to make "use_crs" the default, the harder
> > > > > it will be, so I'd like to push ahead.
> > > > 
> > > > Here's a patch to make CRS the default.  If it looks ok I can
> > > > push it into my linux-next branch.  I'm all for using reliable
> > > > data from the BIOS.  I guess we'll find out fairly quickly if
> > > > this stuff isn't...
> > > 
> > > Thanks for following up on this, Jesse.  It was on my to-do list
> > > for yesterday, but I didn't get to it.
> > > 
> > > Yinghai mentioned a specific box where we might have trouble, but
> > > we never got enough details to really debug it.  So I think we
> > > might as well give it a shot and fine-tune it as we need to.
> > 
> > I just remembered that Andrew Morton once raised some concerns
> > related to the number of _CRS returned resources exceeding the
> > fixed resource array size -> http://lkml.org/lkml/2007/11/1/49
> > 
> > I believe PCI_BUS_NUM_RESOURCES was later increased but if 
> > that increase was insufficient to cover all systems that the
> > code will now be exposed to, it seems like the "And should we
> > really be silently ignoring this problem?  Should we at least
> > report it?" comment that was not addressed could become relevent.
> 
> Not only do we not warn, if we did warn based on the current
>     'if (info->res_num >= PCI_BUS_NUM_RESOURCES)'
> resource array overrun avoidance criteria it would be incorrect
> for a root bus that is connected to a subordinate bus with a
> transparent bridge.  We need to avoid the last 3 slots of the
> resource array since they do not get mapped to a transparent
> bridge connected subordinate bus.  I believe the below patch
> addresses these issues.
> 
> With PCI_BUS_NUM_RESOURCES currently at 16 we still have space
> to accomodate 13 _CRS returned resource descriptors which will
> hopefully be sufficient.

Ok, applied the patch (with Yinghai's suggested name change) to my
linux-next branch.  We'll see now it goes and revert it if there's
trouble.


-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Jesse Barnes]

On Wed, 27 May 2009 12:41:44 -0700
Gary Hade <garyhade@us.ibm.com> wrote:

> On Thu, May 21, 2009 at 09:37:37AM -0700, Gary Hade wrote:
> > On Thu, May 21, 2009 at 08:46:17AM -0600, Bjorn Helgaas wrote:
> > > On Wednesday 20 May 2009 5:49:10 pm Jesse Barnes wrote:
> > > > On Fri, 8 May 2009 16:40:10 -0600
> > > > Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > > > Did anything happen with this?
> > > > > 
> > > > > The longer we wait to make "use_crs" the default, the harder
> > > > > it will be, so I'd like to push ahead.
> > > > 
> > > > Here's a patch to make CRS the default.  If it looks ok I can
> > > > push it into my linux-next branch.  I'm all for using reliable
> > > > data from the BIOS.  I guess we'll find out fairly quickly if
> > > > this stuff isn't...
> > > 
> > > Thanks for following up on this, Jesse.  It was on my to-do list
> > > for yesterday, but I didn't get to it.
> > > 
> > > Yinghai mentioned a specific box where we might have trouble, but
> > > we never got enough details to really debug it.  So I think we
> > > might as well give it a shot and fine-tune it as we need to.
> > 
> > I just remembered that Andrew Morton once raised some concerns
> > related to the number of _CRS returned resources exceeding the
> > fixed resource array size -> http://lkml.org/lkml/2007/11/1/49
> > 
> > I believe PCI_BUS_NUM_RESOURCES was later increased but if 
> > that increase was insufficient to cover all systems that the
> > code will now be exposed to, it seems like the "And should we
> > really be silently ignoring this problem?  Should we at least
> > report it?" comment that was not addressed could become relevent.
> 
> Not only do we not warn, if we did warn based on the current
>     'if (info->res_num >= PCI_BUS_NUM_RESOURCES)'
> resource array overrun avoidance criteria it would be incorrect
> for a root bus that is connected to a subordinate bus with a
> transparent bridge.  We need to avoid the last 3 slots of the
> resource array since they do not get mapped to a transparent
> bridge connected subordinate bus.  I believe the below patch
> addresses these issues.
> 
> With PCI_BUS_NUM_RESOURCES currently at 16 we still have space
> to accomodate 13 _CRS returned resource descriptors which will
> hopefully be sufficient.

Ok, just applied this one to my linux-next tree, since we'll want it
now that _CRS is used by default.

Len, is this ok with you?  It touches x86 ACPI files rather than PCI
files, but it's pretty tightly related...

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] x86/pci: do assign root bus res if _CRS is used

[Gary Hade]

On Mon, Apr 27, 2009 at 01:44:01PM -0600, Bjorn Helgaas wrote:
> On Monday 20 April 2009 07:35:40 pm Yinghai Lu wrote:
> > it wil be overwriten later if _CRS is used, so don't bother to set it.
> > 
> > [ Impact: cleanup ]
> > 
> > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> > 
> > ---
> >  arch/x86/pci/amd_bus.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > Index: linux-2.6/arch/x86/pci/amd_bus.c
> > ===================================================================
> > --- linux-2.6.orig/arch/x86/pci/amd_bus.c
> > +++ linux-2.6/arch/x86/pci/amd_bus.c
> > @@ -100,6 +100,10 @@ void x86_pci_root_bus_res_quirks(struct
> >  	int j;
> >  	struct pci_root_info *info;
> >  
> > +	/* don't go for it if _CRS is used */
> > +	if (pci_probe & PCI_USE__CRS)
> > +		return;
> > +
> >  	/* if only one root bus, don't need to anything */
> >  	if (pci_root_num < 2)
> >  		return;
> 
> This isn't a comment on this patch per se.
> 
> I am concerned about the fact that "pci=use_crs" is not the default.
> >From the changelog of 62f420f8282, it sounds like you have to boot an
> IBM x3850 with "pci=use_crs" to make hot-plug work, even though ACPI
> tells us everything we need to know.  That's backwards.
> 
> We shouldn't need an option to tell Linux that the firmware is
> trustworthy.  We should have an option to *ignore* it for the times
> when we trip over something broken and haven't figured out a way to
> work around it yet.

Sorry, I am behind on my email and just noticed this.

When I posted the patches to add "pci=use_crs" it was only
needed to enable PCI hotplug on a subset of our systems.
At that time it was not apparent that others were interested. 
I was also concerned that real or anticipated breakage on
on other systems might delay or prevent acceptance.

As long as there is an option to disable it I am also in 
favor of trying to make it the default.

Thanks!

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc