[Qemu-devel] Re: [PATCH] piix: use pci_config_set_prog_interface()

[Sebastian Herbszt]

Michael S. Tsirkin wrote:

On Mon, Dec 20, 2010 at 10:18:01PM +0100, Sebastian Herbszt wrote:

Use pci_config_set_prog_interface().

Signed-off-by: Sebastian Herbszt <herb...@gmx.de>

Since I was asked explicitly - I don't have a problem
with these changes: both class and prog interface.
However, they aren't all that useful in themselves.

For class, what I would like to see is a system where
the device class is put in the qdev info table,
and where -device ?
(and hopefully the legacy -help, -nic etc as well)
use this information.

I am not sure if you mean something like the patch below.

pci_config_set_prog_interface can then have an assert to
verify that value.
Maybe we can even make e.g. -device nic work.

In a similar way, pci_config_set_prog_interface
would really become useful if we put the handling
for the legacy classes in a central place
(e.g. pci_class.c)

Which are "legacy classes"?

Any chance of doing something like the above?
I'd be happy to apply such patches.

Sebastian

diff --git a/hw/e1000.c b/hw/e1000.c
index af101bd..88ace8f 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -1117,7 +1117,6 @@ static int pci_e1000_init(PCIDevice *pci_dev)
    /* TODO: we have no capabilities, so why is this bit set? */
    pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_CAP_LIST);
    pci_conf[PCI_REVISION_ID] = 0x03;
-    pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
    /* TODO: RST# value should be 0, PCI spec 6.2.4 */
    pci_conf[PCI_CACHE_LINE_SIZE] = 0x10;

@@ -1169,6 +1168,7 @@ static PCIDeviceInfo e1000_info = {
    .init       = pci_e1000_init,
    .exit       = pci_e1000_uninit,
    .romfile    = "pxe-e1000.bin",
+    .class      = PCI_CLASS_NETWORK_ETHERNET,
    .qdev.props = (Property[]) {
        DEFINE_NIC_PROPERTIES(E1000State, conf),
        DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/pci.c b/hw/pci.c
index ef00d20..de0038c 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1620,6 +1620,11 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo 
*base)
                                     info->is_bridge);
    if (pci_dev == NULL)
        return -1;
+
+    if (info->class) {
+        pci_config_set_class(pci_dev->config, info->class);
+    }
+
    rc = info->init(pci_dev);
    if (rc != 0) {
        do_pci_unregister_device(pci_dev);
diff --git a/hw/pci.h b/hw/pci.h
index 17744dc..29b9280 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -436,6 +436,9 @@ typedef struct {

    /* rom bar */
    const char *romfile;
+
+    /* class */
+    uint16_t class;
} PCIDeviceInfo;

void pci_qdev_register(PCIDeviceInfo *info);