Re: PCI memory allocation bug with CONFIG_HIGHMEM

[Eric W. Biederman]

Linus Torvalds <torvalds@xxxxxxxx> writes:

> On Wed, 7 Jan 2004, Eric W. Biederman wrote:
> > 
> > However insert_resource does not quite match what I think needs to
> > happen.  After a pci quirk applies insert_resource I will get
> > something like:
> > 
> > fff0000-ffffffff : BIOS ROM Window
> >   ffff0000-ffffffff : reserved
> > 
> > With the reserved region still present and marked as BUSY.
> 
> I would suggest ignoring it. Not only because being overly complicated is 
> bad, but simply because nobody should care. 
> 
> At some point adding extra regions is _purely_ for "documentation" 
> reasons, and while that may be nice, it's not worth worrying about. The 
> only thing you really want from a _correctness_ standpoint is to make sure 
> that nobody else will try to allocate their stuff in that area, and your 
> "BIOS ROM Window" resource should do that already. 

Right it is a documentation thing.  The case that causes me to pull
my hair are Itanium boards.  Typically they have 6 or 7 1MB rom chips,
for their firmware.  My goal with going down this road last was so
user space could figure out which rom chip is at which address and
how those correspond to mtd devices.

Using the existing interfaces to export this information looked like
the cleanest way to make certain that information was available until 
I ran into snags like the above.  And once I replace the BIOS I can
fix these things at the source, but...

> > Would it be reasonable to write a variant of request_resource that just
> > drops BIOS resources.
> 
> It would not be impossible to just have a "force_resource()" that would
> simply override _any_ existing resource, but quite frankly, I'd be more
> nervous about that.

Same here.
 
> We could also mark the e820 non-RAM resources with some special
> IORESOURCE_TENTATIVE flag, and allow just overriding those. 
>
> But even the simple "insert_resource()" has some potential problems: if 
> the BIOS has allocated the minimal window for itself (64kB at 0xffff0000), 
> and has allocated some _other_ chip at 0xfffe0000 that the kernel doesn't 
> know about yet, your insert_resource() would do the wrong thing and claim 
> the whole area for the BIOS writing. 
>
> Maybe that doesn't happen, but it's something to think about.

Agreed.  In practice it does not happen, but it is worth thinking
about.

The important thing to maintain is that nothing else grabs the
area the BIOS reserves with a dynamic resource.  So as long as
there is a resource over that area the kernel is safe, even
if I do grab it with insert_resources it does not really matter
to the rest of the kernel because someone has it.

The ROM chips actually have ID's so I can always positively identify
those, I just don't always know their count.  The worst case would be
the rom chip probe causing problems.  And that can be avoided by
simply not loading the driver so I think we are fairly safe.

In the case where I open up the decoder beyond the size it is
currently set for I can test for conflicts, from other devices.
The only reason I would not see another device at that point
is if either (a) there are ordering problems in the kernel or
(b) a SMM bios is doing truly stupid things.  The case where
there is a device there and we aren't using it is not a problem
because I am just reserving a region of the address space.

Now that I have thought about it some more I think the right
was to do with IORESOURCE_TENATIVE it instead of removing tenative
resources to just push them aside.  So in my terrible case I would
get:

fff0000-ffffffff : BIOS ROM Window
fffffff-ffffffff : reserved

And that cleans up all of the structure freeing problems.  I guess I
can do that right now with ____request_resource after I find the
conflict and confirm it has the name "reserved".  I still like the
tenative idea because then any one else who needs the same
functionality would not need to reimplement it.
 
> At some point, the _correct_ answer may be: don't do complex things, and
> write a bootable floppy (without any OS at all, or a really minimal one)  
> to do BIOS rom updates.

That works to some extent.  But it actually a lot more dangerous because
you have to be there in person to verify everything is working fine, and to
insert the floppy.  Doing it from Linux I can update the entire an
entire cluster in a minute, and verify everything automatically.  And
it happens faster because I can load it all over the network.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/