2006-08-22 02:43:14

by Tim Hockin

[permalink] [raw]
Subject: PCI MMCONFIG aperture size

I'm not sure who's responsible for this piece of code, so sorry for the
extra email if it's not you.

I've got a system that has an MMCONFIG region that is 32 MB. Not the
expected 256 MB. Careful reading of the PCI firmware spec says:

"The size of the memory mapped configuration region is indicated
by the start and end bus number fields in the Memory mapped
Enhanced configuration space base address allocation
structure..."
and
"...configuration access method, the base address of the memory
mapped configuration space always corresponds to bus number 0
(regardless of the start bus number decoded by the host
bridge)..."

This says to me that (as long as the MCFG table has an End Bus Number of
31) a 32 MB decode area (32 MB aligned, too) is valid.

Would something like the below patch be accepted? It makes my system
work...

Also, why are we forcing 32 bit base addresses? ACPI defines it to be a
64 bit base...

Tim



--- ./arch/x86_64/pci/mmconfig.c.orig 2006-08-18 15:35:30.000000000 -0700
+++ ./arch/x86_64/pci/mmconfig.c 2006-08-21 19:36:52.000000000 -0700
@@ -13,7 +13,6 @@

#include "pci.h"

-#define MMCONFIG_APER_SIZE (256*1024*1024)
/* Verify the first 16 busses. We assume that systems with more busses
get MCFG right. */
#define MAX_CHECK_BUS 16
@@ -164,6 +163,7 @@
void __init pci_mmcfg_init(void)
{
int i;
+ u32 start;

if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return;
@@ -174,8 +174,9 @@
(pci_mmcfg_config[0].base_address == 0))
return;

- if (!e820_all_mapped(pci_mmcfg_config[0].base_address,
- pci_mmcfg_config[0].base_address + MMCONFIG_APER_SIZE,
+ start = pci_mmcfg_config[0].base_address;
+ if (!e820_all_mapped(start,
+ start+((pci_mmcfg_config[0].end_bus_number+1)*1024*1024),
E820_RESERVED)) {
printk(KERN_ERR "PCI: BIOS Bug: MCFG area is not E820-reserved\n");
printk(KERN_ERR "PCI: Not using MMCONFIG.\n");
@@ -190,7 +191,9 @@
}
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,
+ (pci_mmcfg_config[i].end_bus_number+1)*1024*1024);
if (!pci_mmcfg_virt[i].virt) {
printk("PCI: Cannot map mmconfig aperture for segment %d\n",
pci_mmcfg_config[i].pci_segment_group_number);
--- ./arch/i386/pci/mmconfig.c.orig 2006-08-21 19:19:12.000000000 -0700
+++ ./arch/i386/pci/mmconfig.c 2006-08-21 19:35:04.000000000 -0700
@@ -15,8 +15,6 @@
#include <asm/e820.h>
#include "pci.h"

-#define MMCONFIG_APER_SIZE (256*1024*1024)
-
/* Assume systems with more busses have correct MCFG */
#define MAX_CHECK_BUS 16

@@ -187,6 +185,8 @@

void __init pci_mmcfg_init(void)
{
+ u32 start;
+
if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return;

@@ -196,8 +196,9 @@
(pci_mmcfg_config[0].base_address == 0))
return;

- if (!e820_all_mapped(pci_mmcfg_config[0].base_address,
- pci_mmcfg_config[0].base_address + MMCONFIG_APER_SIZE,
+ start = pci_mmcfg_config[0].base_address;
+ if (!e820_all_mapped(start
+ start+((pci_mmcfg_config[0].end_bus_number+1)*1024*1024),
E820_RESERVED)) {
printk(KERN_ERR "PCI: BIOS Bug: MCFG area is not E820-reserved\n");
printk(KERN_ERR "PCI: Not using MMCONFIG.\n");


2006-08-22 08:01:57

by Andi Kleen

[permalink] [raw]
Subject: Re: PCI MMCONFIG aperture size


> This says to me that (as long as the MCFG table has an End Bus Number of
> 31) a 32 MB decode area (32 MB aligned, too) is valid.
>
> Would something like the below patch be accepted? It makes my system
> work...


I already got a patch to remove the complete e820 validation code because
it broke far more than it fixed. That should fix your problem too.

> Also, why are we forcing 32 bit base addresses? ACPI defines it to be a
> 64 bit base...

Where do you think we do that?

-Andi

2006-08-22 14:58:33

by Tim Hockin

[permalink] [raw]
Subject: Re: PCI MMCONFIG aperture size

On Tue, Aug 22, 2006 at 09:55:31AM +0200, Andi Kleen wrote:
>
> > This says to me that (as long as the MCFG table has an End Bus Number of
> > 31) a 32 MB decode area (32 MB aligned, too) is valid.
> >
> > Would something like the below patch be accepted? It makes my system
> > work...
>
> I already got a patch to remove the complete e820 validation code because
> it broke far more than it fixed. That should fix your problem too.

Great! Coming in 2.6.18?

> > Also, why are we forcing 32 bit base addresses? ACPI defines it to be a
> > 64 bit base...
>
> Where do you think we do that?

Looking at 2.6.17, we always have u32 base_address and u32
base_reserved. base_address is the only one ever referenced, that I can
see. I guess I should grab 2.6.18 pre-releases and recheck.

Thanks

2006-08-22 15:01:06

by Andi Kleen

[permalink] [raw]
Subject: Re: PCI MMCONFIG aperture size

On Tuesday 22 August 2006 16:58, Tim Hockin wrote:
> On Tue, Aug 22, 2006 at 09:55:31AM +0200, Andi Kleen wrote:
> >
> > > This says to me that (as long as the MCFG table has an End Bus Number of
> > > 31) a 32 MB decode area (32 MB aligned, too) is valid.
> > >
> > > Would something like the below patch be accepted? It makes my system
> > > work...
> >
> > I already got a patch to remove the complete e820 validation code because
> > it broke far more than it fixed. That should fix your problem too.
>
> Great! Coming in 2.6.18?

Yes.

>
> > > Also, why are we forcing 32 bit base addresses? ACPI defines it to be a
> > > 64 bit base...
> >
> > Where do you think we do that?
>
> Looking at 2.6.17, we always have u32 base_address and u32
> base_reserved. base_address is the only one ever referenced, that I can
> see. I guess I should grab 2.6.18 pre-releases and recheck.

True. Please submit a patch.

-Andi