[04/11] qdev: pcibus_dev_info

Submitted by Nathan Baum on Dec. 26, 2009, 9:19 p.m.

Details

Message ID 1261862362-2530-5-git-send-email-nathan@parenthephobia.org.uk
State New
Headers show

Commit Message

Nathan Baum Dec. 26, 2009, 9:19 p.m.
This returns a QObject detailing the PCI-specific data about the device.

Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk>
---
 hw/pci.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)

Comments

Luiz Capitulino Dec. 29, 2009, 5:08 p.m.
On Sat, 26 Dec 2009 21:19:15 +0000
Nathan Baum <nathan@parenthephobia.org.uk> wrote:

> +static QObject *pcibus_dev_info(Monitor *mon, DeviceState *dev)
> +{
> +    PCIDevice *d = (PCIDevice *)dev;
> +    const pci_class_desc *desc;
> +    PCIIORegion *r;
> +    int i, class;
> +    QObject *retval;
> +    QList *regions;
> +    
> +    retval = qobject_from_jsonf("{ 'addr': { 'bus' : %d, 'slot' : %d, 'func': %d }, "
> +                                "  'device': { 'vendor': %d, 'id': %d }, "
> +                                "  'subsystem': { 'vendor': %d, 'id': %d } "
> +                                "}",

 You can have 'address' here.

> +    class = pci_get_word(d->config + PCI_CLASS_DEVICE);
> +    desc = pci_class_descriptions;
> +    while (desc->desc && class != desc->class)
> +        desc++;
> +    if (desc->desc) {
> +        qdict_put(qobject_to_qdict(retval), "class", qstring_from_str(desc->desc));
> +    } else {
> +        qdict_put(qobject_to_qdict(retval), "class", qint_from_int(class));
> +    }

 I think it's not good to return different data types for the same key, as
it will make clients more complex, what you can do here is to make 'class'
a QDict with members 'desc' and 'number'.
Nathan Baum Dec. 29, 2009, 8:02 p.m.
On Tue, 2009-12-29 at 15:08 -0200, Luiz Capitulino wrote:

> > +    class = pci_get_word(d->config + PCI_CLASS_DEVICE);
> > +    desc = pci_class_descriptions;
> > +    while (desc->desc && class != desc->class)
> > +        desc++;
> > +    if (desc->desc) {
> > +        qdict_put(qobject_to_qdict(retval), "class", qstring_from_str(desc->desc));
> > +    } else {
> > +        qdict_put(qobject_to_qdict(retval), "class", qint_from_int(class));
> > +    }
> 
>  I think it's not good to return different data types for the same key, as
> it will make clients more complex, what you can do here is to make 'class'
> a QDict with members 'desc' and 'number'.

That's reasonable. It also means we can tell clients the class number
even when the description is known, which might be useful.
Luiz Capitulino Dec. 31, 2009, 12:55 p.m.
On Sat, 26 Dec 2009 21:19:15 +0000
Nathan Baum <nathan@parenthephobia.org.uk> wrote:

> This returns a QObject detailing the PCI-specific data about the device.

 Back to this again, turns out I'm converting pci_info() and I think we
should agree on some standards for PCI keys.

> +static QObject *pcibus_dev_info(Monitor *mon, DeviceState *dev)
> +{
> +    PCIDevice *d = (PCIDevice *)dev;
> +    const pci_class_desc *desc;
> +    PCIIORegion *r;
> +    int i, class;
> +    QObject *retval;
> +    QList *regions;
> +    
> +    retval = qobject_from_jsonf("{ 'addr': { 'bus' : %d, 'slot' : %d, 'func': %d }, "
> +                                "  'device': { 'vendor': %d, 'id': %d }, "
> +                                "  'subsystem': { 'vendor': %d, 'id': %d } "
> +                                "}",

 I'm using 'function' instead of 'func' and I have a dict called 'id' with keys
'device' and 'vendor'. Maybe in this case you could call it 'device_info' as
you have subsystem as well.

 These are only general suggestions, it's not always easy to choose them.

> +    class = pci_get_word(d->config + PCI_CLASS_DEVICE);
> +    desc = pci_class_descriptions;
> +    while (desc->desc && class != desc->class)
> +        desc++;
> +    if (desc->desc) {
> +        qdict_put(qobject_to_qdict(retval), "class", qstring_from_str(desc->desc));
> +    } else {
> +        qdict_put(qobject_to_qdict(retval), "class", qint_from_int(class));
> +    }

 We can share code here, I've added a key called 'class_info' for this, it's
a dict with exact the same info ('desc' and 'class').

> +    
> +    regions = qlist_new();
> +    qdict_put(qobject_to_qdict(retval), "regions", regions);
> +
> +    for (i = 0; i < PCI_NUM_REGIONS; i++) {
> +        r = &d->io_regions[i];
> +        if (!r->size)
> +            continue;
> +        qlist_append_obj(regions,
> +                         qobject_from_jsonf("{'type':%s,'addr':%d,'size':%d}",
> +                                            r->type & PCI_BASE_ADDRESS_SPACE_IO ? "i/o" : "mem",
> +                                            (int) r->addr,
> +                                            (int) r->size));

 You should never cast to int, you should use PRId64 like this example:

obj = qobject_from_jsonf("{ 'BAR': %d, 'type': 'io', "
			"'address': %" PRId64 ", "
			"'current': %" PRId64 " }",
			i, r->addr, r->addr + r->size - 1);

 Also, I'm using 'io' and 'memory' for the 'type' key, usually I prefer the
full word if it's something simple.

 /me remember to write a 'dict best' practices doc.

Patch hide | download patch | download mbox

diff --git a/hw/pci.c b/hw/pci.c
index 9722fce..8688d8a 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -27,6 +27,8 @@ 
 #include "net.h"
 #include "sysemu.h"
 #include "loader.h"
+#include "qjson.h"
+#include "qint.h"
 
 //#define DEBUG_PCI
 #ifdef DEBUG_PCI
@@ -1585,6 +1587,52 @@  static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
     }
 }
 
+static QObject *pcibus_dev_info(Monitor *mon, DeviceState *dev)
+{
+    PCIDevice *d = (PCIDevice *)dev;
+    const pci_class_desc *desc;
+    PCIIORegion *r;
+    int i, class;
+    QObject *retval;
+    QList *regions;
+    
+    retval = qobject_from_jsonf("{ 'addr': { 'bus' : %d, 'slot' : %d, 'func': %d }, "
+                                "  'device': { 'vendor': %d, 'id': %d }, "
+                                "  'subsystem': { 'vendor': %d, 'id': %d } "
+                                "}",
+                                d->config[PCI_SECONDARY_BUS],
+                                PCI_SLOT(d->devfn),
+                                PCI_FUNC(d->devfn),
+                                pci_get_word(d->config + PCI_VENDOR_ID),
+                                pci_get_word(d->config + PCI_DEVICE_ID),
+                                pci_get_word(d->config + PCI_SUBSYSTEM_VENDOR_ID),
+                                pci_get_word(d->config + PCI_SUBSYSTEM_ID));
+    class = pci_get_word(d->config + PCI_CLASS_DEVICE);
+    desc = pci_class_descriptions;
+    while (desc->desc && class != desc->class)
+        desc++;
+    if (desc->desc) {
+        qdict_put(qobject_to_qdict(retval), "class", qstring_from_str(desc->desc));
+    } else {
+        qdict_put(qobject_to_qdict(retval), "class", qint_from_int(class));
+    }
+    
+    regions = qlist_new();
+    qdict_put(qobject_to_qdict(retval), "regions", regions);
+
+    for (i = 0; i < PCI_NUM_REGIONS; i++) {
+        r = &d->io_regions[i];
+        if (!r->size)
+            continue;
+        qlist_append_obj(regions,
+                         qobject_from_jsonf("{'type':%s,'addr':%d,'size':%d}",
+                                            r->type & PCI_BASE_ADDRESS_SPACE_IO ? "i/o" : "mem",
+                                            (int) r->addr,
+                                            (int) r->size));
+    }
+    return retval;
+}
+
 static PCIDeviceInfo bridge_info = {
     .qdev.name    = "pci-bridge",
     .qdev.size    = sizeof(PCIBridge),