Re: [Qemu-devel] [PATCH] Define PCI vendor and device ID in pci.h

[Anthony Liguori]

Stuart Brady wrote:
> This patch defines PPC vendor and device IDs in pci.h, and uses those
> definitions where appropriate.
>
> Signed-off-by: Stuart Brady <address@hidden>
>
> Index: hw/cirrus_vga.c
> ===================================================================
> --- hw/cirrus_vga.c     (revision 6364)
> +++ hw/cirrus_vga.c     (working copy)
> @@ -173,8 +173,7 @@
>  #define CIRRUS_MMIO_LINEDRAW_MODE     0x39     // byte
>  #define CIRRUS_MMIO_BLTSTATUS         0x40     // byte
>  
> -// PCI 0x00: vendor, 0x02: device
> -#define PCI_VENDOR_CIRRUS             0x1013
> +// PCI 0x02: device
>  #define PCI_DEVICE_CLGD5462           0x00d0
>  #define PCI_DEVICE_CLGD5465           0x00d6
>  
> @@ -3354,10 +3353,8 @@
>                                                   sizeof(PCICirrusVGAState),
>                                                   -1, NULL, NULL);
>      pci_conf = d->dev.config;
> -    pci_conf[0x00] = (uint8_t) (PCI_VENDOR_CIRRUS & 0xff);
> -    pci_conf[0x01] = (uint8_t) (PCI_VENDOR_CIRRUS >> 8);
> -    pci_conf[0x02] = (uint8_t) (device_id & 0xff);
> -    pci_conf[0x03] = (uint8_t) (device_id >> 8);
> +    *(uint16_t *)&pci_conf[0x00] = cpu_to_le16(PCI_VENDOR_ID_CIRRUS);
> +    *(uint16_t *)&pci_conf[0x02] = cpu_to_le16(device_id);
>      pci_conf[0x04] = PCI_COMMAND_IOACCESS | PCI_COMMAND_MEMACCESS;
>      pci_conf[0x0a] = PCI_CLASS_SUB_VGA;
>      pci_conf[0x0b] = PCI_CLASS_BASE_DISPLAY;
> Index: hw/usb-uhci.c
> ===================================================================
> --- hw/usb-uhci.c       (revision 6364)
> +++ hw/usb-uhci.c       (working copy)
> @@ -1080,10 +1080,8 @@
>                                          "USB-UHCI", sizeof(UHCIState),
>                                          devfn, NULL, NULL);
>      pci_conf = s->dev.config;
> -    pci_conf[0x00] = 0x86;
> -    pci_conf[0x01] = 0x80;
> -    pci_conf[0x02] = 0x20;
> -    pci_conf[0x03] = 0x70;
> +    *(uint16_t *)&pci_conf[0x00] = cpu_to_le16(PCI_VENDOR_ID_INTEL);
> +    *(uint16_t *)&pci_conf[0x02] = cpu_to_le16(PCI_DEVICE_ID_INTEL_82371SB_2);
>   
You should use cpu_to_le16uw here.  Even better yet, you should 
introduce some PCI space accessors to remove all of the open coded 
offset access.

You could start with just 
pci_config_set_vendor_id()/pci_config_set_device_id().

Regards,

Anthony Liguori