[PATCH] PCI: resource address mismatch

On Tue, 21 Feb 2006, Ivan Kokshaysky wrote:
> There are two bogus entries in the BIOS memory map table which are
> conflicting with a prefetchable memory range of the AGP bridge:
>
>  BIOS-e820: 00000000fec00000 - 00000000fec01000 (reserved)
>  BIOS-e820: 00000000fee00000 - 00000000fee01000 (reserved)
>
> 0000:00:02.0 PCI bridge: Silicon Integrated Systems [SiS] Virtual PCI-to-PCI bridge (AGP) (prog-if 00 [Normal decode])
> 	Flags: bus master, fast devsel, latency 0
> 	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> 	I/O behind bridge: 0000c000-0000cfff
> 	Memory behind bridge: e7e00000-e7efffff
> 	Prefetchable memory behind bridge: fec00000-ffcfffff
> 					   ^^^^^^^^^^^^^^^^^

Yes. However, it's pretty clear that the e820 entries are there for a
reason. Probably they are a hack by the BIOS maintainers to keep Windows
from stomping/moving that region, exactly because they want to keep the
bridge where it is (or, it's actually for the BIOS itself - the BIOS
tables are a horrid mess, and BIOS engineers are pretty hacky people:
they'll add random entries to make their own broken algorithms do the
"right thing").

> Starting from 2.6.13, kernel tries to resolve that sort of conflicts,
> so that prefetch window of the bridge and the framebuffer memory behind
> it get moved to 0x10000000.

I think we could (and probably should) solve this another way: consider
the ACPI "reserved regions" from the e820 map exactly the same way that we
do other ACPI hints - they should restrict _new_ allocations, but not
impact stuff we figure out on our own.

Basically, right now we assign _unassigned_ resources at "fs_initcall"
time. If we were to add in the e820 "reserved region" stuff before that
(but after we've done PCI discovery), we'd probably do the right thing.

Right now we do the e820 reserved regions very early indeed: we call
"register_memory()" from setup_arch(). We could move at least part of it
(the part that registers the resources) down a bit.

Here's a test-patch. I'm not saying we should absolutely do this, but it
might be interesting to try...

Cc: "Antonino A. Daplas" <adaplas@pol.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: <bjk@luxsci.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

2 files changed, 18 insertions, 6 deletions

diff --git a/arch/i386/kernel/efi.c b/arch/i386/kernel/efi.c
index aeabb4196861..7ec6cfa01fb3 100644
--- a/arch/i386/kernel/efi.c
+++ b/arch/i386/kernel/efi.c
@@ -543,7 +543,7 @@ efi_initialize_iomem_resources(struct resource *code_resource,
                 if ((md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT)) >
                     0x100000000ULL)
                         continue;
-                res = alloc_bootmem_low(sizeof(struct resource));
+                res = kzalloc(sizeof(struct resource), GFP_ATOMIC);
                 switch (md->type) {
                 case EFI_RESERVED_TYPE:
                         res->name = "Reserved Memory";
diff --git a/arch/i386/kernel/setup.c b/arch/i386/kernel/setup.c
index 2d8782960f41..d313a11acafa 100644
--- a/arch/i386/kernel/setup.c
+++ b/arch/i386/kernel/setup.c
@@ -1288,7 +1288,7 @@ legacy_init_iomem_resources(struct resource *code_resource, struct resource *dat
                 struct resource *res;
                 if (e820.map[i].addr + e820.map[i].size > 0x100000000ULL)
                         continue;
-                res = alloc_bootmem_low(sizeof(struct resource));
+                res = kzalloc(sizeof(struct resource), GFP_ATOMIC);
                 switch (e820.map[i].type) {
                 case E820_RAM:  res->name = "System RAM"; break;
                 case E820_ACPI: res->name = "ACPI Tables"; break;
@@ -1316,13 +1316,15 @@ legacy_init_iomem_resources(struct resource *code_resource, struct resource *dat
 
 /*
  * Request address space for all standard resources
+ *
+ * This is called just before pcibios_assign_resources(), which is also
+ * an fs_initcall, but is linked in later (in arch/i386/pci/i386.c).
  */
-static void __init register_memory(void)
+static int __init request_standard_resources(void)
 {
-        unsigned long gapstart, gapsize, round;
-        unsigned long long last;
-        int           i;
+        int i;
 
+        printk("Setting up standard PCI resources\n");
         if (efi_enabled)
                 efi_initialize_iomem_resources(&code_resource, &data_resource);
         else
@@ -1334,6 +1336,16 @@ static void __init register_memory(void)
         /* request I/O space for devices used on all i[345]86 PCs */
         for (i = 0; i < STANDARD_IO_RESOURCES; i++)
                 request_resource(&ioport_resource, &standard_io_resources[i]);
+        return 0;
+}
+
+fs_initcall(request_standard_resources);
+
+static void __init register_memory(void)
+{
+        unsigned long gapstart, gapsize, round;
+        unsigned long long last;
+        int i;
 
         /*
          * Search for the bigest gap in the low 32 bits of the e820