Re: [PATCH 2/2] device-assignment: Allow PCI to manage the option ROM

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 2010-10-07 at 19:18 +0200, Michael S. Tsirkin wrote:
> On Mon, Oct 04, 2010 at 03:26:30PM -0600, Alex Williamson wrote:
> > --- a/hw/device-assignment.c
> > +++ b/hw/device-assignment.c
...
> > @@ -1644,58 +1621,64 @@ void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices)
> >   */
> >  static void assigned_dev_load_option_rom(AssignedDevice *dev)
> >  {
> > -    int size, len, ret;
> > -    void *buf;
> > +    char name[32], rom_file[64];
> >      FILE *fp;
> > -    uint8_t i = 1;
> > -    char rom_file[64];
> > +    uint8_t val;
> > +    struct stat st;
> > +    void *ptr;
> > +
> > +    /* If loading ROM from file, pci handles it */
> > +    if (dev->dev.romfile || !dev->dev.rom_bar)
> > +        return;
> >  
> >      snprintf(rom_file, sizeof(rom_file),
> >               "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
> >               dev->host.seg, dev->host.bus, dev->host.dev, dev->host.func);
> >  
> > -    if (access(rom_file, F_OK))
> > +    if (stat(rom_file, &st)) {
> >          return;
> > +    }
> >  
> 
> Just a note that stat on the ROM sysfs file returns window size,
> not the ROM size. So this allocates more ram than really necessary for
> ROM. Real size is returned by fread.
> 
> Do we care?

That was my intention with using stat.  I thought that by default the
ROM BAR should match physical hardware, so even if the contents could be
rounded down to a smaller size, we maintain the size of the physical
device.  To use the minimum size, the contents could be extracted using
pci-sysfs and passed with the romfile option, or the ROM could be
disabled altogether with the rombar=0 option.  Sound reasonable?
Thanks,

Alex