From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=58080 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P7uh0-0004bK-Vy for qemu-devel@nongnu.org; Mon, 18 Oct 2010 14:44:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P7ugz-0003nt-SP for qemu-devel@nongnu.org; Mon, 18 Oct 2010 14:44:18 -0400 Received: from mail-qy0-f180.google.com ([209.85.216.180]:33089) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P7ugz-0003no-Pb for qemu-devel@nongnu.org; Mon, 18 Oct 2010 14:44:17 -0400 Received: by qyk1 with SMTP id 1so1317288qyk.4 for ; Mon, 18 Oct 2010 11:44:17 -0700 (PDT) Message-ID: <4CBC957D.3040007@codemonkey.ws> Date: Mon, 18 Oct 2010 13:44:13 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/2] pci: Automatically patch PCI vendor id and device id in PCI ROM References: <4CBC6CDB.109@redhat.com> <1287424511-22021-1-git-send-email-weil@mail.berlios.de> In-Reply-To: <1287424511-22021-1-git-send-email-weil@mail.berlios.de> 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: Stefan Weil Cc: Markus Armbruster , Gerd Hoffmann , QEMU Developers , "Michael S. Tsirkin" On 10/18/2010 12:55 PM, Stefan Weil wrote: > PCI devices with different vendor or device ids sometimes share > the same rom code. Only the ids and the checksum > differs in a boot rom for such devices. > > The i825xx ethernet controller family is a typical example > which is implemented in hw/eepro100.c. It uses at least > 3 different device ids, so normally 3 boot roms would be needed. > > By automatically patching vendor id and device id (and the checksum) > in qemu, all emulated family members can share the same boot rom. > > VGA bios roms are another example with different vendor and device ids. > > v2: > > * Patch also the vendor id (and remove the sanity check for vendor id). > > Cc: Gerd Hoffmann > Cc: Markus Armbruster > Cc: Michael S. Tsirkin > Signed-off-by: Stefan Weil > I get very nervous about patching a ROM. Who's to say that the ROM doesn't somehow depend on the contents of its header? Maybe it has an internal CRC built into it or something like that. Regards, Anthony Liguori > --- > hw/pci.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 58 insertions(+), 0 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 1280d4d..139eb24 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1797,6 +1797,62 @@ static void pci_map_option_rom(PCIDevice *pdev, int region_num, pcibus_t addr, p > cpu_register_physical_memory(addr, size, pdev->rom_offset); > } > > +/* Patch the PCI vendor and device ids in a PCI rom image if necessary. > + This is needed for an option rom which is used for more than one device. */ > +static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size) > +{ > + uint16_t vendor_id; > + uint16_t device_id; > + uint16_t rom_vendor_id; > + uint16_t rom_device_id; > + uint16_t rom_magic; > + uint16_t pcir_offset; > + uint8_t checksum; > + > + /* Words in rom data are little endian (like in PCI configuration), > + so they can be read / written with pci_get_word / pci_set_word. */ > + > + /* Only a valid rom will be patched. */ > + rom_magic = pci_get_word(ptr); > + if (rom_magic != 0xaa55) { > + PCI_DPRINTF("Bad ROM magic %04x\n", rom_magic); > + return; > + } > + pcir_offset = pci_get_word(ptr + 0x18); > + if (pcir_offset + 8>= size || memcmp(ptr + pcir_offset, "PCIR", 4)) { > + PCI_DPRINTF("Bad PCIR offset 0x%x or signature\n", pcir_offset); > + return; > + } > + > + vendor_id = pci_get_word(pdev->config + PCI_VENDOR_ID); > + device_id = pci_get_word(pdev->config + PCI_DEVICE_ID); > + rom_vendor_id = pci_get_word(ptr + pcir_offset + 4); > + rom_device_id = pci_get_word(ptr + pcir_offset + 6); > + > + PCI_DPRINTF("ROM id %04x%04x / PCI id %04x%04x\n", > + vendor_id, device_id, rom_vendor_id, rom_device_id); > + > + checksum = ptr[6]; > + > + if (vendor_id != rom_vendor_id) { > + /* Patch vendor id and checksum (at offset 6 for etherboot roms). */ > + checksum += (uint8_t)rom_vendor_id + (uint8_t)(rom_vendor_id>> 8); > + checksum -= (uint8_t)vendor_id + (uint8_t)(vendor_id>> 8); > + PCI_DPRINTF("ROM checksum %02x / %02x\n", ptr[6], checksum); > + ptr[6] = checksum; > + pci_set_word(ptr + pcir_offset + 4, vendor_id); > + } > + > + if (device_id != rom_device_id) { > + /* Patch device id and checksum (at offset 6 for etherboot roms). */ > + checksum += (uint8_t)rom_device_id + (uint8_t)(rom_device_id>> 8); > + checksum -= (uint8_t)device_id + (uint8_t)(device_id>> 8); > + PCI_DPRINTF("ROM checksum %02x / %02x\n", ptr[6], checksum); > + ptr[6] = checksum; > + pci_set_word(ptr + pcir_offset + 6, device_id); > + } > +} > + > /* Add an option rom for the device */ > static int pci_add_option_rom(PCIDevice *pdev) > { > @@ -1849,6 +1905,8 @@ static int pci_add_option_rom(PCIDevice *pdev) > load_image(path, ptr); > qemu_free(path); > > + pci_patch_ids(pdev, ptr, size); > + > pci_register_bar(pdev, PCI_ROM_SLOT, size, > 0, pci_map_option_rom); > >