On Wed, May 18, 2011 at 07:55:17PM +0900, Isaku Yamahata wrote: > On Wed, May 18, 2011 at 12:17:46PM +0300, Michael S. Tsirkin wrote: > > On Wed, May 18, 2011 at 01:55:17AM +0900, Isaku Yamahata wrote: > > > vender id/device id... in pci configuration space are read-only registers > > > which are commonly defined for all pci devices. > > > So initialize them in common code and it simplifies the initialization a > > > bit. > > > I didn't converted virtio-pci and qxl because it determines ids > > > dynaically. > > > So I'll leave those conversion (or not to convert) to the authors. > > > > Hmm, virtio doesn't: > > static PCIDeviceInfo virtio_info[] = { > > ... > > } > > > > has the array of devices. > > Okay. I missed it somehow. I get the following, And I'll leave > qxl convection to Gerd. > The remaining issue is, should I adopt/drop prog_interface conversion?
Drop it I think. I don't see what it buys us. The point with all this work IMO is to be able to sort devices by type and to show device vendor/type/revision in -help as well as to identify device by vendor/device/revision instead of the arbitrary qemu names on the monitor. We can use numeric values, as well as parse pci.ids if present on system for symbolic ones. revision is sometimes 0 and initialized later by devices, so if it's 0 we don't really know what it is, but it's a bit less important I guess, so while I'm not 100% we should have it in the table, I'm not sure we shouldn't either. However none of this seems to apply to prog_interface which is useful for the OS but not that useful for the user. > >From c9834234c73eb03d764a3c999cbd34f4814a5553 Mon Sep 17 00:00:00 2001 > Message-Id: > <c9834234c73eb03d764a3c999cbd34f4814a5553.1305715603.git.yamah...@valinux.co.jp> > In-Reply-To: <cover.1305715602.git.yamah...@valinux.co.jp> > References: <cover.1305715602.git.yamah...@valinux.co.jp> > From: Isaku Yamahata <yamah...@valinux.co.jp> > Date: Wed, 18 May 2011 19:46:04 +0900 > Subject: [PATCH] virtio-pci.c: convert to PCIDEviceInfo to initialize ids > > use PCIDeviceInfo to initialize ids. > > Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp> > --- > hw/virtio-pci.c | 69 ++++++++++++++++++++++++------------------------------ > 1 files changed, 31 insertions(+), 38 deletions(-) > > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index c19629d..270e2c7 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -669,9 +669,7 @@ static const VirtIOBindings virtio_pci_bindings = { > .vmstate_change = virtio_pci_vmstate_change, > }; > > -static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, > - uint16_t vendor, uint16_t device, > - uint16_t class_code, uint8_t pif) > +static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev) > { > uint8_t *config; > uint32_t size; > @@ -679,19 +677,12 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, > VirtIODevice *vdev, > proxy->vdev = vdev; > > config = proxy->pci_dev.config; > - pci_config_set_vendor_id(config, vendor); > - pci_config_set_device_id(config, device); > - > - config[0x08] = VIRTIO_PCI_ABI_VERSION; > - > - config[0x09] = pif; > - pci_config_set_class(config, class_code); > - > - config[0x2c] = vendor & 0xFF; > - config[0x2d] = (vendor >> 8) & 0xFF; > - config[0x2e] = vdev->device_id & 0xFF; > - config[0x2f] = (vdev->device_id >> 8) & 0xFF; > > + if (proxy->class_code) { > + pci_config_set_class(config, proxy->class_code); > + } > + pci_set_word(config + 0x2c, pci_get_word(config + PCI_VENDOR_ID)); > + pci_set_word(config + 0x2e, vdev->device_id); > config[0x3d] = 1; > > if (vdev->nvectors && !msix_init(&proxy->pci_dev, vdev->nvectors, 1, 0)) > { > @@ -735,10 +726,7 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev) > return -1; > } > vdev->nvectors = proxy->nvectors; > - virtio_init_pci(proxy, vdev, > - PCI_VENDOR_ID_REDHAT_QUMRANET, > - PCI_DEVICE_ID_VIRTIO_BLOCK, > - proxy->class_code, 0x00); > + virtio_init_pci(proxy, vdev); > /* make the actual value visible */ > proxy->nvectors = vdev->nvectors; > return 0; > @@ -776,10 +764,7 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev) > vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED > ? proxy->serial.max_virtserial_ports > + 1 > : proxy->nvectors; > - virtio_init_pci(proxy, vdev, > - PCI_VENDOR_ID_REDHAT_QUMRANET, > - PCI_DEVICE_ID_VIRTIO_CONSOLE, > - proxy->class_code, 0x00); > + virtio_init_pci(proxy, vdev); > proxy->nvectors = vdev->nvectors; > return 0; > } > @@ -801,11 +786,7 @@ static int virtio_net_init_pci(PCIDevice *pci_dev) > vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, &proxy->net); > > vdev->nvectors = proxy->nvectors; > - virtio_init_pci(proxy, vdev, > - PCI_VENDOR_ID_REDHAT_QUMRANET, > - PCI_DEVICE_ID_VIRTIO_NET, > - PCI_CLASS_NETWORK_ETHERNET, > - 0x00); > + virtio_init_pci(proxy, vdev); > > /* make the actual value visible */ > proxy->nvectors = vdev->nvectors; > @@ -827,11 +808,7 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev) > VirtIODevice *vdev; > > vdev = virtio_balloon_init(&pci_dev->qdev); > - virtio_init_pci(proxy, vdev, > - PCI_VENDOR_ID_REDHAT_QUMRANET, > - PCI_DEVICE_ID_VIRTIO_BALLOON, > - PCI_CLASS_MEMORY_RAM, > - 0x00); > + virtio_init_pci(proxy, vdev); > return 0; > } > > @@ -843,11 +820,7 @@ static int virtio_9p_init_pci(PCIDevice *pci_dev) > > vdev = virtio_9p_init(&pci_dev->qdev, &proxy->fsconf); > vdev->nvectors = proxy->nvectors; > - virtio_init_pci(proxy, vdev, > - PCI_VENDOR_ID_REDHAT_QUMRANET, > - 0x1009, > - 0x2, > - 0x00); > + virtio_init_pci(proxy, vdev); > /* make the actual value visible */ > proxy->nvectors = vdev->nvectors; > return 0; > @@ -861,6 +834,10 @@ static PCIDeviceInfo virtio_info[] = { > .qdev.size = sizeof(VirtIOPCIProxy), > .init = virtio_blk_init_pci, > .exit = virtio_blk_exit_pci, > + .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, > + .device_id = PCI_DEVICE_ID_VIRTIO_BLOCK, > + .revision = VIRTIO_PCI_ABI_VERSION, > + .class_id = PCI_CLASS_STORAGE_SCSI, > .qdev.props = (Property[]) { > DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), > DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, block), > @@ -878,6 +855,10 @@ static PCIDeviceInfo virtio_info[] = { > .init = virtio_net_init_pci, > .exit = virtio_net_exit_pci, > .romfile = "pxe-virtio.rom", > + .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, > + .device_id = PCI_DEVICE_ID_VIRTIO_NET, > + .revision = VIRTIO_PCI_ABI_VERSION, > + .class_id = PCI_CLASS_NETWORK_ETHERNET, > .qdev.props = (Property[]) { > DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false), > @@ -898,6 +879,10 @@ static PCIDeviceInfo virtio_info[] = { > .qdev.size = sizeof(VirtIOPCIProxy), > .init = virtio_serial_init_pci, > .exit = virtio_serial_exit_pci, > + .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, > + .device_id = PCI_DEVICE_ID_VIRTIO_CONSOLE, > + .revision = VIRTIO_PCI_ABI_VERSION, > + .class_id = PCI_CLASS_COMMUNICATION_OTHER, > .qdev.props = (Property[]) { > DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > @@ -916,6 +901,10 @@ static PCIDeviceInfo virtio_info[] = { > .qdev.size = sizeof(VirtIOPCIProxy), > .init = virtio_balloon_init_pci, > .exit = virtio_exit_pci, > + .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, > + .device_id = PCI_DEVICE_ID_VIRTIO_BALLOON, > + .revision = VIRTIO_PCI_ABI_VERSION, > + .class_id = PCI_CLASS_MEMORY_RAM, > .qdev.props = (Property[]) { > DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_PROP_END_OF_LIST(), > @@ -927,6 +916,10 @@ static PCIDeviceInfo virtio_info[] = { > .qdev.alias = "virtio-9p", > .qdev.size = sizeof(VirtIOPCIProxy), > .init = virtio_9p_init_pci, > + .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, > + .device_id = 0x1009, > + .revision = VIRTIO_PCI_ABI_VERSION, > + .class_id = 0x2, > .qdev.props = (Property[]) { > DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), > DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > -- > 1.7.1.1 > > > -- > yamahata