Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Chuck Ebbert]

In-Reply-To: <p73ac8fqjix.fsf@verdi.suse.de>

On 15 Jun 2006 03:45:10 +0200, Andi Kleen wrote:

> Anyways I would say that if the BIOS can't get MCFG right then 
> it's likely not been validated on that board and shouldn't be used.

According to Petr Vandrovec:

 ... "What is important (and checked) is address of MMCONFIG reported by MCFG
 table...  Unfortunately code does not bother with printing that address :-(
 
 "Another problem is that code has hardcoded that MMCONFIG area is 256MB large. 
 Unfortunately for the code PCI specification allows any power of two between 2MB 
 and 256MB if vendor knows that such amount of busses (from 2 to 128) will be 
 sufficient for system.  With notebook it is quite possible that not full 8 bits 
 are implemented for MMCONFIG bus number."


So here is a patch.  Unfortunately my system still fails the test because
it doesn't reserve any part of the MMCONFIG area, but this may fix others.

Booted on x86_64, only compiled on i386.  x86_64 still remaps the max area
(256MB) even though only 2MB is checked... but 2.6.16 had no check at all
so it is still better.


PCI: reduce size of x86 MMCONFIG reserved area check

1.  Print the address of the MMCONFIG area when the test for that area
    being reserved fails.

2.  Only check if the first 2MB is reserved, as that is the minimum.

Signed-off-by: Chuck Ebbert <76306.1226@compuserve.com>

 arch/i386/pci/mmconfig.c   |    9 ++++++---
 arch/x86_64/pci/mmconfig.c |   13 +++++++++----
 2 files changed, 15 insertions(+), 7 deletions(-)

--- 2.6.17-rc6-64.orig/arch/i386/pci/mmconfig.c
+++ 2.6.17-rc6-64/arch/i386/pci/mmconfig.c
@@ -15,7 +15,9 @@
 #include <asm/e820.h>
 #include "pci.h"
 
-#define MMCONFIG_APER_SIZE (256*1024*1024)
+/* aperture is up to 256MB but BIOS may reserve less */
+#define MMCONFIG_APER_MIN	(2 * 1024*1024)
+#define MMCONFIG_APER_MAX	(256 * 1024*1024)
 
 /* Assume systems with more busses have correct MCFG */
 #define MAX_CHECK_BUS 16
@@ -197,9 +199,10 @@ void __init pci_mmcfg_init(void)
 		return;
 
 	if (!e820_all_mapped(pci_mmcfg_config[0].base_address,
-			pci_mmcfg_config[0].base_address + MMCONFIG_APER_SIZE,
+			pci_mmcfg_config[0].base_address + MMCONFIG_APER_MIN,
 			E820_RESERVED)) {
-		printk(KERN_ERR "PCI: BIOS Bug: MCFG area is not E820-reserved\n");
+		printk(KERN_ERR "PCI: BIOS Bug: MCFG area at %x is not E820-reserved\n",
+				pci_mmcfg_config[0].base_address);
 		printk(KERN_ERR "PCI: Not using MMCONFIG.\n");
 		return;
 	}
--- 2.6.17-rc6-64.orig/arch/x86_64/pci/mmconfig.c
+++ 2.6.17-rc6-64/arch/x86_64/pci/mmconfig.c
@@ -13,7 +13,10 @@
 
 #include "pci.h"
 
-#define MMCONFIG_APER_SIZE (256*1024*1024)
+/* aperture is up to 256MB but BIOS may reserve less */
+#define MMCONFIG_APER_MIN	(2 * 1024*1024)
+#define MMCONFIG_APER_MAX	(256 * 1024*1024)
+
 /* Verify the first 16 busses. We assume that systems with more busses
    get MCFG right. */
 #define MAX_CHECK_BUS 16
@@ -175,9 +178,10 @@ void __init pci_mmcfg_init(void)
 		return;
 
 	if (!e820_all_mapped(pci_mmcfg_config[0].base_address,
-			pci_mmcfg_config[0].base_address + MMCONFIG_APER_SIZE,
+			pci_mmcfg_config[0].base_address + MMCONFIG_APER_MIN,
 			E820_RESERVED)) {
-		printk(KERN_ERR "PCI: BIOS Bug: MCFG area is not E820-reserved\n");
+		printk(KERN_ERR "PCI: BIOS Bug: MCFG area at %x is not E820-reserved\n",
+				pci_mmcfg_config[0].base_address);
 		printk(KERN_ERR "PCI: Not using MMCONFIG.\n");
 		return;
 	}
@@ -190,7 +194,8 @@ void __init pci_mmcfg_init(void)
 	}
 	for (i = 0; i < pci_mmcfg_config_num; ++i) {
 		pci_mmcfg_virt[i].cfg = &pci_mmcfg_config[i];
-		pci_mmcfg_virt[i].virt = ioremap_nocache(pci_mmcfg_config[i].base_address, MMCONFIG_APER_SIZE);
+		pci_mmcfg_virt[i].virt = ioremap_nocache(pci_mmcfg_config[i].base_address,
+							 MMCONFIG_APER_MAX);
 		if (!pci_mmcfg_virt[i].virt) {
 			printk("PCI: Cannot map mmconfig aperture for segment %d\n",
 			       pci_mmcfg_config[i].pci_segment_group_number);
-- 
Chuck

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Arjan van de Ven]

Chuck Ebbert wrote:
> In-Reply-To: <p73ac8fqjix.fsf@verdi.suse.de>
> 
> On 15 Jun 2006 03:45:10 +0200, Andi Kleen wrote:
> 
>> Anyways I would say that if the BIOS can't get MCFG right then 
>> it's likely not been validated on that board and shouldn't be used.
> 
> According to Petr Vandrovec:
> 
>  ... "What is important (and checked) is address of MMCONFIG reported by MCFG
>  table...  Unfortunately code does not bother with printing that address :-(
>  
>  "Another problem is that code has hardcoded that MMCONFIG area is 256MB large. 
>  Unfortunately for the code PCI specification allows any power of two between 2MB 
>  and 256MB if vendor knows that such amount of busses (from 2 to 128) will be 
>  sufficient for system.  With notebook it is quite possible that not full 8 bits 
>  are implemented for MMCONFIG bus number."
> 
> 
> So here is a patch.  Unfortunately my system still fails the test because
> it doesn't reserve any part of the MMCONFIG area, but this may fix others.
> 
> Booted on x86_64, only compiled on i386.  x86_64 still remaps the max area
> (256MB) even though only 2MB is checked... but 2.6.16 had no check at all
> so it is still better.
> 
> 
> PCI: reduce size of x86 MMCONFIG reserved area check
> 
> 1.  Print the address of the MMCONFIG area when the test for that area
>     being reserved fails.
> 
> 2.  Only check if the first 2MB is reserved, as that is the minimum.
> 
> Signed-off-by: Chuck Ebbert <76306.1226@compuserve.com>

Acked-by: Arjan van de Ven <arjan@linux.intel.com>

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Barry Scott]

Arjan van de Ven wrote:
> Chuck Ebbert wrote:
>> In-Reply-To: <p73ac8fqjix.fsf@verdi.suse.de>
>>
>> On 15 Jun 2006 03:45:10 +0200, Andi Kleen wrote:
>>
>>> Anyways I would say that if the BIOS can't get MCFG right then it's 
>>> likely not been validated on that board and shouldn't be used.
>>
>> According to Petr Vandrovec:
>>
>>  ... "What is important (and checked) is address of MMCONFIG reported 
>> by MCFG
>>  table...  Unfortunately code does not bother with printing that 
>> address :-(
>>  
>>  "Another problem is that code has hardcoded that MMCONFIG area is 
>> 256MB large.  Unfortunately for the code PCI specification allows any 
>> power of two between 2MB  and 256MB if vendor knows that such amount 
>> of busses (from 2 to 128) will be  sufficient for system.  With 
>> notebook it is quite possible that not full 8 bits  are implemented 
>> for MMCONFIG bus number."
>>
>>
>> So here is a patch.  Unfortunately my system still fails the test 
>> because
>> it doesn't reserve any part of the MMCONFIG area, but this may fix 
>> others.
>>
>> Booted on x86_64, only compiled on i386.  x86_64 still remaps the max 
>> area
>> (256MB) even though only 2MB is checked... but 2.6.16 had no check at 
>> all
>> so it is still better.
>>
>>
>> PCI: reduce size of x86 MMCONFIG reserved area check
>>
>> 1.  Print the address of the MMCONFIG area when the test for that area
>>     being reserved fails.
>>
>> 2.  Only check if the first 2MB is reserved, as that is the minimum.
>>
>> Signed-off-by: Chuck Ebbert <76306.1226@compuserve.com>
>
> Acked-by: Arjan van de Ven <arjan@linux.intel.com>
>
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
I have a system that fails to boot reporting MCFG problems.
I've applied your patch and it allowed the HP dc7600U to boot past
the MCFG problem. (I still have LVM problems - but that nothing
to do with this patch).

My shuttle P4 system also boots with your patch applied.

Barry

[RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Brice Goglin]

Hi Arjan,

We have some machines here where MMCONFIG is disabled in 2.6.17 because
their MCFG area is not e820-reserved. It makes the extended PCI config
space unavailable to pci_read/write_config_foo(). I don't know if lots
of people out there use the extended config space, but at least we do in
our myri10ge driver.

What would you think of a patch implementing the following strategy:
1) if MMCONFIG works, always use it (no change)
2) if MMCONFIG is disabled and we are accessing the regular config
space, use direct conf (no change, should ensure that any machine will
still boot fine)
3) if MMCONFIG is disabled but we are accessing the _extended_ config
space, try mmconfig anyway since there's no other way to do it.

Actually, this problem seems to target nVidia chipsets, and we actually
know how to check this chipset's registers to be sure whether MMCONFIG
works. So it might be good to improve the current MMCONFIG disabling
code by adding some chipset-specific hacks (having nVidia and Intel
chipsets there may cover most of the cases). I don't think there's any
way to do that with PCI quirks. We might end up having these hacks in
the MMCONFIG initialization code.

Thanks,
Brice

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Arjan van de Ven]

Brice Goglin wrote:er.
> 
> What would you think of a patch implementing the following strategy:
> 1) if MMCONFIG works, always use it (no change)
> 2) if MMCONFIG is disabled and we are accessing the regular config
> space, use direct conf (no change, should ensure that any machine will
> still boot fine)
> 3) if MMCONFIG is disabled but we are accessing the _extended_ config
> space, try mmconfig anyway since there's no other way to do it.

an OS isn't allowed to mix old and new access methods realistically so I don't think
this is a viable good solution...

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Andi Kleen]

Arjan van de Ven <arjan@linux.intel.com> writes:

> Brice Goglin wrote:er.
> > What would you think of a patch implementing the following strategy:
> > 1) if MMCONFIG works, always use it (no change)

One problem in your proposal Bryan is that just trying to access
bogus hardware mappings might cause nasty effects like machine
checks or trigger chipset errata.

> > 2) if MMCONFIG is disabled and we are accessing the regular config
> > space, use direct conf (no change, should ensure that any machine will
> > still boot fine)

That's already the case.

> > 3) if MMCONFIG is disabled but we are accessing the _extended_ config
> > space, try mmconfig anyway since there's no other way to do it.

(3) sounds dangerous to me.

> an OS isn't allowed to mix old and new access methods realistically so I don't think
> this is a viable good solution...

Why is it not allowed? We already do it in some cases.

Anyways I would say that if the BIOS can't get MCFG right then 
it's likely not been validated on that board and shouldn't be used.

-Andi

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Brice Goglin]

Arjan van de Ven wrote:
> Brice Goglin wrote:er.
>>
>> What would you think of a patch implementing the following strategy:
>> 1) if MMCONFIG works, always use it (no change)
>> 2) if MMCONFIG is disabled and we are accessing the regular config
>> space, use direct conf (no change, should ensure that any machine will
>> still boot fine)
>> 3) if MMCONFIG is disabled but we are accessing the _extended_ config
>> space, try mmconfig anyway since there's no other way to do it.
>
> an OS isn't allowed to mix old and new access methods realistically so
> I don't think
> this is a viable good solution...

Well, we are talking about using a different method to access the
extended config space only. This space is independent from the legacy
config space.
I don't see how mixing the old and new methods like this could lead to
any problem, we are not going to mix them to access the same registers.
But I agree with Andi about the possible dangerousness.

We need to improve this "mmconfig disabled" anyhow. Having the extended
config space unavailable on lots of machines is also far from a viable
solution :) If you still do not like this first proposal, what do you
think of my other one ? (having chipset-specific checks in
pci_mmcfg_init to find out for sure whether mmconfig will work)

Thanks,
Brice

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Arjan van de Ven]

> 
> Well, we are talking about using a different method to access the
> extended config space only. This space is independent from the legacy
> config space.

not really. In many ways it's the same space.

> I don't see how mixing the old and new methods like this could lead to
> any problem, we are not going to mix them to access the same registers.

the spec simply doesn't allow it. Sure it may work on your machine today,
but that doesn't make it a good idea ;)

> We need to improve this "mmconfig disabled" anyhow. Having the extended
> config space unavailable on lots of machines is also far from a viable
> solution :)

it's unlikely to be many machines though.

  If you still do not like this first proposal, what do you
> think of my other one ? (having chipset-specific checks in
> pci_mmcfg_init to find out for sure whether mmconfig will work)

I'm all in favor of a more detailed test; just we HAVE to have a test
for this since it's simply broken too often. What the test needs to do
is check if the MCFG entry actually points to a working mmconfig area,
so 1) that it actually points to an mmconfig area and not to garbage, and 2)
that accesses to it actually work ;)

the current approach doesn't test 2) realistically, only 1), but if you weaken
the 1) test as you propose you really ought to substitute it with a 2) test...

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Rajesh Shah]

On Wed, Jun 14, 2006 at 11:47:57PM -0700, Arjan van de Ven wrote:
> 
> > We need to improve this "mmconfig disabled" anyhow. Having the extended
> > config space unavailable on lots of machines is also far from a viable
> > solution :)
> 
> it's unlikely to be many machines though.
> 
I just noticed today - this check killed PCI Express on 3 of the 4
machines I normally use for testing. Digging a bit deeper, I found
this in the PCI firmware spec (v 3.0):

If the operating system does not natively comprehend reserving the
MMCFG region, the MMCFG region must be reserved by firmware. The
address range reported in the MCFG table or by _CBA method (see
Section 4.1.3) must be reserved by declaring a motherboard resource.
For most systems, the motherboard resource would appear at the root
of the ACPI namespace (under \_SB) in a node with a _HID of EISAID
(PNP0C02), and the resources in this case should not be claimed in
the root PCI bus.s _CRS. The resources can optionally be returned in
Int15 E820 or EFIGetMemoryMap as reserved memory but must always be
reported through ACPI as a motherboard resource

Sure enough, the ACPI namespace for the "broken" machines lists
the MMCFG resources as indicated above, and PCI Express works fine
otherwise. I haven't looked yet whether it's possible to add this
check in the code, have you looked into that option? I understand
the PCI firmware spec is not necessarily the final authority on
this, but a _lot_ of BIOS developers read that to figure out what
to do...

Rajesh

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Andi Kleen]

On Thursday 22 June 2006 00:19, Rajesh Shah wrote:
> On Wed, Jun 14, 2006 at 11:47:57PM -0700, Arjan van de Ven wrote:
> > 
> > > We need to improve this "mmconfig disabled" anyhow. Having the extended
> > > config space unavailable on lots of machines is also far from a viable
> > > solution :)
> > 
> > it's unlikely to be many machines though.
> > 
> I just noticed today - this check killed PCI Express on 3 of the 4
> machines I normally use for testing.

What do you mean with killed PCI Express? PCI Express should
work even without extended config space, except error handling.

Error handling seems to be still a quite obscure feature,
not used by many people - so booting is more important than
supporting it. Still would be good to support it of course.

You're saying that you have lots of machines where the mmconfig
aperture is not fully reserved in e820?

Is it partially reserved (not for all busses) or not at all?


> Sure enough, the ACPI namespace for the "broken" machines lists
> the MMCFG resources as indicated above, and PCI Express works fine
> otherwise. I haven't looked yet whether it's possible to add this
> check in the code, have you looked into that option? I understand
> the PCI firmware spec is not necessarily the final authority on
> this, but a _lot_ of BIOS developers read that to figure out what
> to do...

If someone does a patch to double check it against the ACPI name space
I'm not opposed to let it overrule the e820 heuristic.

The point of this code is to pragmatically detect BIOS with obviously 
broken setups. It's not about standards lawyering.

-Andi
 

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Rajesh Shah]

On Thu, Jun 22, 2006 at 12:32:19AM +0200, Andi Kleen wrote:
> On Thursday 22 June 2006 00:19, Rajesh Shah wrote:
> > On Wed, Jun 14, 2006 at 11:47:57PM -0700, Arjan van de Ven wrote:
> > > 
> > > > We need to improve this "mmconfig disabled" anyhow. Having the extended
> > > > config space unavailable on lots of machines is also far from a viable
> > > > solution :)
> > > 
> > > it's unlikely to be many machines though.
> > > 
> > I just noticed today - this check killed PCI Express on 3 of the 4
> > machines I normally use for testing.
> 
> What do you mean with killed PCI Express? PCI Express should
> work even without extended config space, except error handling.
> 
Yes, I meant it killed extended config space. Note that we are
about to send out code that enables PCI Express error handling,
so this will become more visible now.

> You're saying that you have lots of machines where the mmconfig
> aperture is not fully reserved in e820?
> 
Yes, I saw this in 3 out of 4 systems I checked. I'll go check
some more systems too.

> Is it partially reserved (not for all busses) or not at all?
> 
The MMCFG resources are not listed at all in the BIOS provided
memory map.

> 
> If someone does a patch to double check it against the ACPI name space
> I'm not opposed to let it overrule the e820 heuristic.
> 
> The point of this code is to pragmatically detect BIOS with obviously 
> broken setups. It's not about standards lawyering.
> 
Oh I agree with you that booting is more important. My point with
the spec statement was that most BIOS developers may not even know
they are doing something "wrong" by not listing these resources in
the int15 E820 table, since the document they normally refer to
doesn't say so. I suspect there are many more systems out there
which do the same thing and will fail the check, but we never notice
since most users don't try to ever access the extended space today.

Rajesh

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Arjan van de Ven]

Rajesh Shah wrote:
> On Thu, Jun 22, 2006 at 12:32:19AM +0200, Andi Kleen wrote:
>> On Thursday 22 June 2006 00:19, Rajesh Shah wrote:
>>> On Wed, Jun 14, 2006 at 11:47:57PM -0700, Arjan van de Ven wrote:
>>>>> We need to improve this "mmconfig disabled" anyhow. Having the extended
>>>>> config space unavailable on lots of machines is also far from a viable
>>>>> solution :)
>>>> it's unlikely to be many machines though.
>>>>
>>> I just noticed today - this check killed PCI Express on 3 of the 4
>>> machines I normally use for testing.
>> What do you mean with killed PCI Express? PCI Express should
>> work even without extended config space, except error handling.
>>
> Yes, I meant it killed extended config space. Note that we are
> about to send out code that enables PCI Express error handling,
> so this will become more visible now.
> 
>> You're saying that you have lots of machines where the mmconfig
>> aperture is not fully reserved in e820?
>>
> Yes, I saw this in 3 out of 4 systems I checked. I'll go check
> some more systems too.
> 
>> Is it partially reserved (not for all busses) or not at all?
>>
> The MMCFG resources are not listed at all in the BIOS provided
> memory map.
> 
>> If someone does a patch to double check it against the ACPI name space
>> I'm not opposed to let it overrule the e820 heuristic.
>>
>> The point of this code is to pragmatically detect BIOS with obviously 
>> broken setups. It's not about standards lawyering.
>>
> Oh I agree with you that booting is more important. My point with
> the spec statement was that most BIOS developers may not even know
> they are doing something "wrong" by not listing these resources in
> the int15 E820 table, since the document they normally refer to
> doesn't say so. I suspect there are many more systems out there
> which do the same thing and will fail the check, but we never notice
> since most users don't try to ever access the extended space today.

well... it's sort of common sense though.. if you want non-ACPI OSes to 
work properly (like the older 2.4 based distros...)

Re: [RFC] PCI extended conf space when MMCONFIG disabled because of e820

[Rajesh Shah]

On Thu, Jun 22, 2006 at 11:27:59AM +0200, Arjan van de Ven wrote:
> Rajesh Shah wrote:
> > Oh I agree with you that booting is more important. My point with
> > the spec statement was that most BIOS developers may not even know
> > they are doing something "wrong" by not listing these resources in
> > the int15 E820 table, since the document they normally refer to
> > doesn't say so. I suspect there are many more systems out there
> > which do the same thing and will fail the check, but we never notice
> > since most users don't try to ever access the extended space today.
> 
> well... it's sort of common sense though.. if you want non-ACPI OSes to 
> work properly (like the older 2.4 based distros...)
> 
In this case we already have an ACPI dependence, since the MCFG
table is listed in ACPI. In any case, I have a patch that got me
my extended config space back on the one machine I've tested so
far. I'll test it out on the remaining 2 where I saw this problem
and send it out.

Rajesh