Re: Proper support for PCI-based option rom loading (was Re: [Qemu-devel] Re: qdev property bug?)

[Michael S. Tsirkin]

On Thu, Dec 17, 2009 at 10:45:45AM +0100, Gerd Hoffmann wrote:
> On 12/17/09 02:24, Kevin O'Connor wrote:
>> On Wed, Dec 16, 2009 at 05:22:41PM +0100, Gerd Hoffmann wrote:
>> The current "CBFS" mechanism looks for named "files" of the following
>> form:
>>
>> - pciVVVV,DDDD.rom - a rom associated with a PCI device with the given
>>    vendor and device id.
>
> Might be useful for backward compatibility with older qemu versions as  
> adding a rom bar to the pci devices is a guest-visible change in pci  
> config space.
>
>> - vgaroms/* - any "file" starting with the "vgaroms/" prefix is
>>    treated as a vga option rom not associated with any specific pci
>>    device.
>
> obvious place for vgabios.
>
>> - genroms/* - any "file" starting with the "genroms/" prefix is
>>    treated as a generic option rom not associated with any specific pci
>>    device.
>
> and this for the non-pci roms like extboot &
>
>> Associating a name to the fw_cfg entries and using the naming
>> convention above would allow SeaBIOS to reuse its existing code.
>
> Ok.
>
>>> I'd tend to have one fw_cfg entry which returns structed data, like this:
>>>
>>> struct fw_files {
>>>    u32 filecount;
>>>    struct fw_file {
>>>      u32  type;        /* vga, option, other? */
>>>      u32  size;        /* file size */
>>>      u32  select;      /* write this to 0x510 to read it */
>>>      u32  reserved;    /* you never know ;) */
>>>      char name[16];    /* maybe: filename */

Can't name simply be null terminated?

>>>    };
>>> };
>>>
>>> What do you think?
>>
>> It works for me.  The name would need to be larger (64 bytes?).
>
> I'd size name so it pads the struct to 32 or 64 bytes total.  32bit  
> size, 16bit select, 16bit reserved, leaves 24 bytes for the name with 32  
> bytes.  Hmm, better to go for a 64 byte struct, leaving 56 bytes for the  
> name, which should be enougth.
>
> I'll have patches ready later today.
>
> cheers,
>   Gerd

-- 
MST