Re: [Qemu-devel] [PATCH 1/2] pci: Automatically patch PCI device id in PCI ROM

[Stefan Weil <weil@mail.berlios.de>]

Hi,

Am 18.10.2010 13:54, schrieb Gerd Hoffmann:
>   Hi,
>
>> As far as I know there is no well-defined checksum offset.
>> The checksum is simply set by modifying any byte (which
>> normally should be unused).
>>
>> Etherboot has some unused bytes at the beginning of rom data
>> and always uses the same offset 6.
>
> Ah, so you don't actually update the checksum but change some unused 
> byte to make the checksum stay the same, right?

Right. The sum of all bytes modulo 255 must be 0.
Any byte can be modified to achieve this.

>
>> For other roms which also don't use the byte at offset 6, this approach
>> will work, too. If they store code or vital data at that location,
>> we destroy that data, so it won't work.
>>
>> The VGA bios roms have a sequence of several bytes of zero
>> starting at offset 6, so maybe this data is not important and
>> we may change the byte at offset 6, but that should be checked
>> before using this mechanism.
>
> From vgabios:
>
>     .org 0
>
>     vgabios_start:
>     .byte  0x55, 0xaa    /* BIOS signature */
>     .byte  0x40        /* BIOS extension length */
>
>     vgabios_entry_point:
>           jmp vgabios_init_func
>
> From seabios:
>
>     struct rom_header {
>         u16 signature;
>         u8 size;
>         u8 initVector[4];
>         u8 reserved[17];
>         u16 pcioffset;
>         u16 pnpoffset;
>     } PACKED;
>
> Hmm.  So offset 6 is the last byte of initVector.  If (and only if) 
> you happen to know that the jump instruction takes 3 bytes only it is 
> save to modify the unused 4th byte.  Seems to be true for both vgabios 
> and etherboot/gPXE.  We can't assume this in general, although it is 
> quite likely given that there hardly would be anything but a 16bit jump.

I agree. So it would work with vga bios, too.

It looks like vgabios uses the last byte to fix the checksum
(rom data ends with a sequence of 0xff, only last byte is different).


>
>> As long as the driver specifies the romfile name,
>> we get an implicitly defined behaviour: either the
>> rom matches and nothing special is done, or it doesn't
>> and the id(s) will be fixed.
>
>> So neither flag nor opt-in seems to be needed.
>
> When following this argumentation the vendor id sanity check shouldn't 
> be there in the first place ;)

The sanity check is simply there because I had no test case
which patches the vendor id. How could I test with vga bios?

>
> Note that romfile is a pci bus property, so it isn't fully under the 
> drivers control because it can be overridden from the command line for 
> every pci device.

Maybe this is an argument why the driver should not include any flags
for id patching. A user who overrides the rom name from the command line
should know what she/he does.

>
> cheers,
>   Gerd
>


Regards,
Stefan