diff mbox

[1/7] pci: move ids of config space into PCIDeviceInfo

Message ID 744e7922dc2a3259fd2fd371c1de3077c63c0a5a.1302266896.git.yamahata@valinux.co.jp
State New
Headers show

Commit Message

Isaku Yamahata April 8, 2011, 12:53 p.m. UTC 
vender id/device id... in configuration space are read-only registers
which are commonly defined for all pci devices.
So move those initialization into common place.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci.c |   46 ++++++++++++++++++++++++++++++++--------------
 hw/pci.h |    9 +++++++++
 2 files changed, 41 insertions(+), 14 deletions(-)

Comments

Michael S. Tsirkin May 5, 2011, 12:48 p.m. UTC | #1 
So the benefit as I see it would be that qemu will be able to list
supported devices by vendor id etc.
lspci has a database of readable vendor/device strings,
maybe we can import that.
And we could sort by device type, that's also helpful.

header type/prog interface  - not so sure.

On Fri, Apr 08, 2011 at 09:53:00PM +0900, Isaku Yamahata wrote:
> diff --git a/hw/pci.h b/hw/pci.h
> index c6a6eb6..f945798 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -433,6 +433,15 @@ typedef struct {
>      PCIConfigReadFunc *config_read;
>      PCIConfigWriteFunc *config_write;
>  
> +    uint16_t vendor_id;
> +    uint16_t device_id;
> +    uint8_t revision;

This is good.

> +    uint8_t prog_interface;

Not sure about this one. What is wrong

> +    uint16_t class_id;

This is good.

> +    uint8_t header_type;

We have a flag for bridge already, right?
Let's fill this in automatically then.

> +    uint16_t subsystem_vendor_id;       /* only for header type = 0 */
> +    uint16_t subsystem_id;              /* only for header type = 0 */

add an assert then?

> +
>      /*
>       * pci-to-pci bridge or normal device.
>       * This doesn't mean pci host switch.
> -- 
> 1.7.1.1
diff mbox

Patch

diff --git a/hw/pci.c b/hw/pci.c
index 410b67b..a3fd391 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -726,10 +726,12 @@  static void pci_config_free(PCIDevice *pci_dev)
 /* -1 for devfn means auto assign */
 static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
                                          const char *name, int devfn,
-                                         PCIConfigReadFunc *config_read,
-                                         PCIConfigWriteFunc *config_write,
-                                         bool is_bridge)
+                                         const PCIDeviceInfo *info)
 {
+    PCIConfigReadFunc *config_read = info->config_read;
+    PCIConfigWriteFunc *config_write = info->config_write;
+    uint8_t header_type = info->header_type & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
+
     if (devfn < 0) {
         for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
             devfn += PCI_FUNC_MAX) {
@@ -750,13 +752,27 @@  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     pci_dev->irq_state = 0;
     pci_config_alloc(pci_dev);
 
-    if (!is_bridge) {
-        pci_set_default_subsystem_id(pci_dev);
+    pci_config_set_vendor_id(pci_dev->config, info->vendor_id);
+    pci_config_set_device_id(pci_dev->config, info->device_id);
+    pci_config_set_revision(pci_dev->config, info->revision);
+    pci_config_set_prog_interface(pci_dev->config, info->prog_interface);
+    pci_config_set_class(pci_dev->config, info->class_id);
+    pci_dev->config[PCI_HEADER_TYPE] = header_type;
+
+    if (!info->is_bridge) {
+        if (info->subsystem_vendor_id) {
+            pci_set_word(pci_dev->config + PCI_SUBSYSTEM_VENDOR_ID,
+                         info->subsystem_vendor_id);
+            pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID,
+                         info->subsystem_id);
+        } else {
+            pci_set_default_subsystem_id(pci_dev);
+        }
     }
     pci_init_cmask(pci_dev);
     pci_init_wmask(pci_dev);
     pci_init_w1cmask(pci_dev);
-    if (is_bridge) {
+    if (info->is_bridge) {
         pci_init_wmask_bridge(pci_dev);
     }
     if (pci_init_multifunction(bus, pci_dev)) {
@@ -783,17 +799,21 @@  static void do_pci_unregister_device(PCIDevice *pci_dev)
     pci_config_free(pci_dev);
 }
 
+/* TODO: obsolete. eliminate this once all pci devices are qdevifed. */
 PCIDevice *pci_register_device(PCIBus *bus, const char *name,
                                int instance_size, int devfn,
                                PCIConfigReadFunc *config_read,
                                PCIConfigWriteFunc *config_write)
 {
     PCIDevice *pci_dev;
+    PCIDeviceInfo info = {
+        .config_read = config_read,
+        .config_write = config_write,
+        .header_type = PCI_HEADER_TYPE_NORMAL
+    };
 
     pci_dev = qemu_mallocz(instance_size);
-    pci_dev = do_pci_register_device(pci_dev, bus, name, devfn,
-                                     config_read, config_write,
-                                     PCI_HEADER_TYPE_NORMAL);
+    pci_dev = do_pci_register_device(pci_dev, bus, name, devfn, &info);
     if (pci_dev == NULL) {
         hw_error("PCI: can't register device\n");
     }
@@ -1642,7 +1662,7 @@  static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
     PCIDevice *pci_dev = (PCIDevice *)qdev;
     PCIDeviceInfo *info = container_of(base, PCIDeviceInfo, qdev);
     PCIBus *bus;
-    int devfn, rc;
+    int rc;
     bool is_default_rom;
 
     /* initialize cap_present for pci_is_express() and pci_config_size() */
@@ -1651,10 +1671,8 @@  static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
     }
 
     bus = FROM_QBUS(PCIBus, qdev_get_parent_bus(qdev));
-    devfn = pci_dev->devfn;
-    pci_dev = do_pci_register_device(pci_dev, bus, base->name, devfn,
-                                     info->config_read, info->config_write,
-                                     info->is_bridge);
+    pci_dev = do_pci_register_device(pci_dev, bus, base->name,
+                                     pci_dev->devfn, info);
     if (pci_dev == NULL)
         return -1;
     if (qdev->hotplugged && info->no_hotplug) {
diff --git a/hw/pci.h b/hw/pci.h
index c6a6eb6..f945798 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -433,6 +433,15 @@  typedef struct {
     PCIConfigReadFunc *config_read;
     PCIConfigWriteFunc *config_write;
 
+    uint16_t vendor_id;
+    uint16_t device_id;
+    uint8_t revision;
+    uint8_t prog_interface;
+    uint16_t class_id;
+    uint8_t header_type;
+    uint16_t subsystem_vendor_id;       /* only for header type = 0 */
+    uint16_t subsystem_id;              /* only for header type = 0 */
+
     /*
      * pci-to-pci bridge or normal device.
      * This doesn't mean pci host switch.