Ingo suggest to use sticky resource type to record fixed resource.
so could check that BAR and avoid update it after request_resource failed.
and we could remove tricky code about insert some resource with late_initcall.
Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
---
arch/x86/kernel/acpi/boot.c | 17 +-----------
arch/x86/kernel/apic.c | 30 +++++----------------
arch/x86/kernel/io_apic.c | 26 +-----------------
arch/x86/pci/i386.c | 38 ++++----------------------
arch/x86/pci/mmconfig-shared.c | 49 ++--------------------------------
include/linux/ioport.h | 3 ++
kernel/resource.c | 58 +++++++++++++++++++++++++++++++++++++++++
7 files changed, 83 insertions(+), 138 deletions(-)
Index: linux-2.6/arch/x86/kernel/acpi/boot.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/boot.c
+++ linux-2.6/arch/x86/kernel/acpi/boot.c
@@ -702,30 +702,17 @@ static int __init acpi_parse_hpet(struct
hpet_res = alloc_bootmem(sizeof(*hpet_res) + HPET_RESOURCE_NAME_SIZE);
hpet_res->name = (void *)&hpet_res[1];
- hpet_res->flags = IORESOURCE_MEM;
+ hpet_res->flags = IORESOURCE_MEM | IORESOURCE_STICKY;
snprintf((char *)hpet_res->name, HPET_RESOURCE_NAME_SIZE, "HPET %u",
hpet_tbl->sequence);
hpet_res->start = hpet_address;
hpet_res->end = hpet_address + (1 * 1024) - 1;
+ insert_resource(&iomem_resource, hpet_res);
return 0;
}
-/*
- * hpet_insert_resource inserts the HPET resources used into the resource
- * tree.
- */
-static __init int hpet_insert_resource(void)
-{
- if (!hpet_res)
- return 1;
-
- return insert_resource(&iomem_resource, hpet_res);
-}
-
-late_initcall(hpet_insert_resource);
-
#else
#define acpi_parse_hpet NULL
#endif
Index: linux-2.6/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic.c
+++ ...very nice patch! I've split your patch into two, and applied the
kernel/resource.c and include/linux/ioresource.h core bits to the
tip/core/resources topic branch - see the first patch below. (i renamed
the new API to check_sticky_resource())
Then i've merged this topic branch into irq/sparseirq and applied the
x86 bits that use this new facility to irq/sparseirq [on which it has
many dependencies, io_apic.c and apic.c got unified, etc.]. See the
second patch below.
Especially the second patch shows how much fragile resource code we are
able to remove this way:
5 files changed, 22 insertions(+), 138 deletions(-)
nice work.
Does anyone have any suggestions of how to improve this some more? (or
do it differently)
Ingo
----------------------->
From 0fec91fc00059401dc756ad635975bea8f813309 Mon Sep 17 00:00:00 2001
From: Yinghai Lu <yhlu.kernel@gmail.com>
Date: Wed, 27 Aug 2008 18:56:23 -0700
Subject: [PATCH] IO resources: add sticky resource type
Ingo suggested to use sticky resource type to record fixed resource.
That way we could check that BAR in the PCI init code and avoid updating
it after request_resource failed.
That way we could remove some tricky code about insert some platform
resources with late_initcall.
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
include/linux/ioport.h | 3 ++
kernel/resource.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 0 deletions(-)
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 22d2115..db53613 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -48,6 +48,8 @@ struct resource_list {
#define IORESOURCE_SIZEALIGN 0x00020000 /* size indicates alignment */
#define IORESOURCE_STARTALIGN 0x00040000 /* start field is alignment */
+#define IORESOURCE_STICKY 0x08000000
+
#define IORESOURCE_DISABLED 0x10000000
#define IORESOURCE_UNSET 0x20000000
#define IORESOURCE_AUTO 0x40000000
@@ -109,6 +111,7 @@ extern ...As far as I can see, THIS IS TOTALLY BROKEN. There's a reason why we add the broken resources LATE. There's a reason we _have_ to add them late. Trying to come up with these braindamaged schemes to avoid doing it right is wrong. Don't do it. The reason? Those bogus resources that the BIOS reports are simply NOT TRUSTWORTHY. They may actually be in all the wrong places, including covering a resource half-way, or crossing two real resources. Yes, it's rare. But it happens. Which is why we should not do these broken things that _incorrectly_ assume that the BIOS resources will only ever be totally contained within a BAR, or will totally contain one. Think about the partial overlap case. Guys, until you learn that the BIOS resources are _crap_ and at most random guesses, you shouldn't touch this code! And it has nothing to do with writing "clean" code, or making things "simple", because quite frankly, things simply ARE NOT simple! The fact is, the only reliable way to handle these things has _always_ been to ask the hardware first. Add the broken resources from ACPI and other BIOS tables _later_. If they conflict, it is the ACPI/BIOS tables that should be removed. Why do I have to tell this to people over and over again? Linus --
i fully agree with that principle, i just messed up implementing it. 'Sticky resources' tried to be exactly the kind of 'untrusted, possibly wrong' resources, which should not prevent existing PCI resources from being registered - they would at most prevent new PCI resources from being allocated over them. (the free space is large enough for us to take the small/untrusted hint from the BIOS where not to allocate to) I missed the possibility of a sticky resource not being wide enough and preventing a BAR from being registered, due to partial overlap. That was not intended. I guess this whole patchset has to become a lot wider and a lot more involved. Ingo --
