From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=43324 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P7pZn-0001dL-BN for qemu-devel@nongnu.org; Mon, 18 Oct 2010 09:16:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P7pZm-00084r-8g for qemu-devel@nongnu.org; Mon, 18 Oct 2010 09:16:31 -0400 Received: from moutng.kundenserver.de ([212.227.17.8]:55691) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P7pZl-00084U-U5 for qemu-devel@nongnu.org; Mon, 18 Oct 2010 09:16:30 -0400 Message-ID: <4CBC48A0.3030701@mail.berlios.de> Date: Mon, 18 Oct 2010 15:16:16 +0200 From: Stefan Weil MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/2] pci: Automatically patch PCI device id in PCI ROM References: <1287175867-7757-1-git-send-email-weil@mail.berlios.de> <4CBC1BA8.7030604@redhat.com> <4CBC2C7A.4000803@mail.berlios.de> <4CBC3572.6000908@redhat.com> In-Reply-To: <4CBC3572.6000908@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: Markus Armbruster , QEMU Developers , "Michael S. Tsirkin" 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