Re: [RFC] PCI: Unassigned Expansion ROM BARs

From: Yinghai Lu
Date: Fri Sep 25 2015 - 20:04:46 EST


On Fri, Sep 25, 2015 at 9:18 AM, Alex Williamson
<alex.williamson@xxxxxxxxxx> wrote:
>> > > Or do we want to keep a white list to say which device should have
>> > > ROM bar as mush have, and other is optional to have ?
>> >
>> > Subject: [RFC PATCH] PCI: Add pci_dev_need_rom_bar()
>> >
>> > Only set that for
>> > 1. if BIOS/firmware already set ROM bar.
>> > 2. via quirks for some devices.
>> >
>> > We assign those needed ROM bar as required
>> > and other ROM bars as optional resources.
>>
>> I'd rather not have a whitelist if we can avoid it. We'd be
>> continually adding new devices to it, and it makes the system harder
>> to understand because there's no consistent rule about how ROMs are
>> handled.

I don't like that whitelist way, and hope we can find better way to
handle all the
cases elegantly.

>>
>> Alex mentioned the idea of ripping the ROM, and I'd like to explore
>> that idea a little more. What if we could temporarily assign space
>> for the ROM during enumeration, read the contents, cache the contents
>> somewhere, then deallocate the actual BAR space? We could hang onto
>> the cache and give it to anybody who later needs the ROM.
>
> That sounds pretty good, so long as we can consider the ROM to be
> perfectly static. I don't know if anything relies on an in-place update
> or if there are ROMs that are less read-only than others. Maybe it's
> safe to assume that or at least safe to assume that if the BIOS didn't
> leave room for them, then we can consider them static. It might be
> interesting to strace some of the userspace firmware update programs for
> add-in cards to see if they re-read the ROM to determine if it has
> changed.

Should cover most cases. But there is some driver like
drivers/mtd/maps/pci.c::drivers/mtd/maps/pci.c do have write operation
to the mmio range.

>
> We already sort of do this for VGA ROMs. When a driver tries to map the
> boot VGA ROM they actually map the shadow copy at 0xC0000 (iirc) rather
> than the one on the device. This actually sort of sucks because this
> particular shadow copy certainly is not read-only and in all the glory
> that is VGA, the shadow sometimes gets modified by the execution of the
> VGA ROM itself and we no longer have access to the pristine device
> version of it (bad for device assignment of primary graphics).
>
> Now, if we want to make our own shadow copies for all ROMs, it seems
> pretty clear that we first need to get access to the ROM, which means we
> need to figure out if the BIOS mapped it. If the ROM BAR is outside of
> the bridge aperture (catching both 0 and 0xFFF..000) or overlaps a
> standard ROM BAR, we can consider it unprogrammed. In that case we need
> to try to do the trick above with unmapping standard BARs to get the
> shadow copy. Otherwise we should be able to get the shadow copy in
> place (maybe a question of which we prefer to use in this case). I
> would be willing to risk that if the BIOS didn't leave room for the ROM
> and we can't map it into the space used by other BARs or it doesn't fit
> the bridge aperture, we can spit out a printk warning and skip it. I
> expect a very low failure rate.

Maybe we can combine two methods together:
1. have NEED_ROM_BAR, and it is set
a) if BIOS allocate resource to yet (maybe not, so we leave space
for MMIO bars)
b) via limited whitelist that will not support static copy.
2. kernel will try to allocate resource to ROM bar with
NEED_ROM_BAR as required, and others
as optional
3. for ROM bar that can not get assigned, kernel try to borrow mmio range
from other MMIO bar on the device, and save a copy and expose that
via /sys/.../rom
That should happen FINAL_FIXUP stage before driver get
loaded.

--- There is risk on it, some add-on card firmware would
stop working if kernel ever try to change MMIO bar.
then we will need blacklist to skip BAR on those devices.
--
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/