qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Stefan Weil <weil@mail.berlios.de>
Cc: Markus Armbruster <armbru@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/2] pci: Automatically patch PCI vendor id and device id in PCI ROM
Date: Mon, 18 Oct 2010 13:44:13 -0500	[thread overview]
Message-ID: <4CBC957D.3040007@codemonkey.ws> (raw)
In-Reply-To: <1287424511-22021-1-git-send-email-weil@mail.berlios.de>

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<kraxel@redhat.com>
> Cc: Markus Armbruster<armbru@redhat.com>
> Cc: Michael S. Tsirkin<mst@redhat.com>
> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
>    

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);
>
>    

  parent reply	other threads:[~2010-10-18 18:44 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-12 12:41 [Qemu-devel] Where's gpxe-eepro100-80862449.rom ? Markus Armbruster
2010-10-12 16:18 ` [Qemu-devel] " Stefan Weil
2010-10-13  7:13   ` Markus Armbruster
2010-10-15 20:51     ` [Qemu-devel] [PATCH 1/2] pci: Automatically patch PCI device id in PCI ROM Stefan Weil
2010-10-15 21:05       ` Anthony Liguori
2010-10-18 10:09         ` Gerd Hoffmann
2010-10-18 18:39           ` Anthony Liguori
2010-10-21 10:09             ` [SeaBIOS] " Avi Kivity
2010-10-21 13:14               ` Anthony Liguori
2010-10-21 15:34                 ` Avi Kivity
2010-10-18 10:04       ` Gerd Hoffmann
2010-10-18 11:16         ` Stefan Weil
2010-10-18 11:54           ` Gerd Hoffmann
2010-10-18 13:16             ` Stefan Weil
2010-10-18 13:30               ` Gerd Hoffmann
2010-10-18 15:50                 ` Gerd Hoffmann
2010-10-18 17:54                   ` Stefan Weil
2010-10-18 17:55                   ` [Qemu-devel] [PATCH 1/2] pci: Automatically patch PCI vendor id and " Stefan Weil
2010-10-18 17:58                     ` [Qemu-devel] " Michael S. Tsirkin
2010-10-18 18:42                       ` Anthony Liguori
2010-10-18 19:03                         ` Michael S. Tsirkin
2010-10-18 19:36                           ` Stefan Weil
2010-10-18 19:59                             ` Anthony Liguori
2010-10-19  6:40                             ` Gerd Hoffmann
2010-10-18 19:56                           ` Anthony Liguori
2010-10-18 18:44                     ` Anthony Liguori [this message]
2010-10-18 18:53                       ` [Qemu-devel] " Anthony Liguori
2010-10-18 19:11                         ` Stefan Weil
2010-10-19  8:37                           ` Michael S. Tsirkin
2010-10-19 21:15                             ` Stefan Weil
2010-10-19 21:22                               ` Anthony Liguori
2010-10-19 21:25                                 ` Michael S. Tsirkin
2010-10-19 21:08                         ` Stefan Weil
2010-10-20  7:19                           ` [Qemu-devel] " Gerd Hoffmann
2010-10-20 20:30                             ` Stefan Weil
2010-11-22  6:29                           ` Michael S. Tsirkin
2010-10-15 20:51     ` [Qemu-devel] [PATCH 2/2] eepro100: Use a single rom file for all i825xx devices Stefan Weil
2010-11-22  6:29       ` [Qemu-devel] " Michael S. Tsirkin
2010-10-15 21:03     ` [Qemu-devel] Re: Where's gpxe-eepro100-80862449.rom ? Stefan Weil
2010-10-25 12:11       ` Markus Armbruster
2010-10-25 16:23         ` Stefan Weil
2010-10-25 16:54           ` Michael S. Tsirkin
2010-11-15 17:06             ` Stefan Weil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4CBC957D.3040007@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=armbru@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=weil@mail.berlios.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).