On Thu, 14 Feb 2008, Bjorn Helgaas wrote:
That makes sense, but ... there are BIOSes that actually list nested resources, e.g., http://bugzilla.kernel.org/show_bug.cgi?id=9514#c29 :
[000000290 - 000000294] Motherboard resources [000000290 - 00000029F] Motherboard resources
Heh.
Not that I can really see us ever being confused by this in practice. Sure, we'd skip the second resource (because it's busy), but that's what we used to do before anyway, wasn't it? And even if we do, there's very little reason we'd ever actually try to reserve that 294-29f range.
In practice, the only resources we actually tend to end up really allocating tends to be things like Cardbus (or a whole PCI bus either due to hotplug or due to the BIOS not bothering), which tend want a big window for the bus window, so the "off-by-one" kinds of things wouldn't really worry me. Very seldom do we actually have small resources to worry about on a PC platform, since they all tend to be pre-allocated by the BIOS anyway.
That said, if we really want to handle this, we could certainly add a whole new ioresource flag bit that says "allow inserting resources into this", and we could set that bit not just for the PnP reservations themselves, but in PCI bus resources too.
Basically, there are two different ways of inserting a resource:
- the "register this subresource" thing that "request_resource()" does, which just works on the one given resource and honors the BUSY bit.
- the "insert this resource into the tree" (insert_resource()) that starts from the root and tries to find the right location. It honors the BUSY bit too, but we could extend it to _only_ extend into resources that allow it.
Here's the trivial patch to add the infrastructure for this, but I did *not* do the required "mark PCI bus resources as IORESOURCE_INSERT" etc (nor obviously the markers to make PnP resources themselves be marked as "insert").
But if you want to play around with it, I think this is at least *conceptually* a sane idea. Another way to see it is that a resource with the IORESOURCE_INSERT bit is "subdivisible", and then it's very obvious that a normal PCI device resource should not have it set: it is a "leaf" resources that can not be split up.
So the only way to populate "leaf" resources is by explicitly specifying them and doing a "request_resource()" on the resource, while the non-leaf IORESOURCE_INSERT resources can be populated from below with ioresrouce_insert().
Is this over-designed? I dunno. The _implementation_ on a resource level is certainly trivial.
Oh, and if it wasn't obvious: the patch is untested. It compiles. That's all I'm going to say about it, especially since without marking any resources with the new IORESOURCE_INSERT bit, the only thing you can do with "insert_resource()" is to insert into the root level (because we don't check the level we are passing into)
Linus
--- include/linux/ioport.h | 1 + kernel/resource.c | 2 ++ 2 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/include/linux/ioport.h b/include/linux/ioport.h index 605d237..0a56410 100644 --- a/include/linux/ioport.h +++ b/include/linux/ioport.h @@ -45,6 +45,7 @@ struct resource_list { #define IORESOURCE_RANGELENGTH 0x00008000 #define IORESOURCE_SHADOWABLE 0x00010000 #define IORESOURCE_BUS_HAS_VGA 0x00080000 +#define IORESOURCE_INSERT 0x00100000 /* Allow insert_resource() */
#define IORESOURCE_DISABLED 0x10000000 #define IORESOURCE_UNSET 0x20000000 diff --git a/kernel/resource.c b/kernel/resource.c index 82aea81..d6da786 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -397,6 +397,8 @@ int insert_resource(struct resource *parent, struct resource *new) result = -EBUSY; if (first == parent) goto out; + if (!(first->flags & IORESOURCE_INSERT)) + goto out;
if ((first->start > new->start) || (first->end < new->end)) break;