| Message ID | 1261862362-2530-5-git-send-email-nathan@parenthephobia.org.uk |
|---|---|
| State | New |
| Headers | show |
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'.
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.
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.
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),
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(-)