All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices
@ 2020-05-07 13:47 Laurent Vivier
  2020-05-07 13:47 ` [RFC v4 1/6] qmp: add QMP command x-debug-query-virtio Laurent Vivier
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Laurent Vivier @ 2020-05-07 13:47 UTC (permalink / raw)
  To: lvivier, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	Markus Armbruster, Dr. David Alan Gilbert, Eric Auger,
	Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau,
	Paolo Bonzini, Max Reitz

This series introduces new QMP/HMP commands to dump the status
of a a virtio device at different levels.

1. Main command

 HMP Only:

     x-debug-virtio [subcommand]

   Example:

    List all sub-commands:

    (qemu) x-debug-virtio
    x-debug-virtio query  -- List all available virtio devices
    x-debug-virtio status path -- Display status of a given virtio device
    x-debug-virtio queue-status path queue -- Display status of a given virtio queue
    x-debug-virtio queue-element path queue [index] -- Display element of a given virtio queue

2. List available virtio devices in the machine

  HMP Form:

    x-debug-virtio query

  Example:

    (qemu) x-debug-virtio query
    /machine/peripheral-anon/device[3]/virtio-backend [virtio-net]
    /machine/peripheral-anon/device[1]/virtio-backend [virtio-serial]
    /machine/peripheral-anon/device[0]/virtio-backend [virtio-blk]

  QMP Form:

    { 'command': 'x-debug-query-virtio', 'returns': ['VirtioInfo'] }

  Example:

  -> { "execute": "x-debug-query-virtio" }
  <- { "return": [
         {
             "path": "/machine/peripheral-anon/device[3]/virtio-backend",
             "type": "virtio-net"
         },
         {
             "path": "/machine/peripheral-anon/device[1]/virtio-backend",
             "type": "virtio-serial"
         },
         {
             "path": "/machine/peripheral-anon/device[0]/virtio-backend",
             "type": "virtio-blk"
         }
       ]
     }

3. Display status of a given virtio device

  HMP Form:

    x-debug-virtio status <path>

  Example:

    (qemu) x-debug-virtio status /machine/peripheral-anon/device[3]/virtio-backend
    /machine/peripheral-anon/device[3]/virtio-backend:
      Device Id:        1
      Guest features:   event-idx, indirect-desc, version-1
                        ctrl-mac-addr, guest-announce, ctrl-vlan, ctrl-rx, ctrl-vq, status, mrg-rxbuf, host-ufo, host-ecn, host-tso6, host-tso4, guest-ufo, guest-ecn, guest-tso6, guest-tso4, mac, ctrl-guest-offloads, guest-csum, csum
      Host features:    event-idx, indirect-desc, bad-feature, version-1, any-layout, notify-on-empty
                        gso, ctrl-mac-addr, guest-announce, ctrl-rx-extra, ctrl-vlan, ctrl-rx, ctrl-vq, status, mrg-rxbuf, host-ufo, host-ecn, host-tso6, host-tso4, guest-ufo, guest-ecn, guest-tso6, guest-tso4, mac, ctrl-guest-offloads, guest-csum, csum
      Backend features:
      Endianness:       little
      VirtQueues:       3

  QMP Form:

    { 'command': 'x-debug-virtio-status',
      'data': { 'path': 'str' },
      'returns': 'VirtioStatus'
    }

  Example:

  -> { "execute": "x-debug-virtio-status",
       "arguments": {
           "path": "/machine/peripheral-anon/device[3]/virtio-backend"
       }
    }
  <- { "return": {
       "device-endian": "little",
       "device-id": 1,
       "backend-features": {
         "device": {
           "type": "virtio-net",
           "data": []
         },
         "unknown": 0,
         "transport": []
       },
       "num-vqs": 3,
       "guest-features": {
         "device": {
           "type": "virtio-net",
           "data": [ "ctrl-mac-addr", "guest-announce", "ctrl-vlan", "ctrl-rx",
                     "ctrl-vq", "status", "mrg-rxbuf", "host-ufo", "host-ecn",
                     "host-tso6", "host-tso4", "guest-ufo", "guest-ecn",
                     "guest-tso6", "guest-tso4", "mac", "ctrl-guest-offloads",
                     "guest-csum", "csum" ]
         },
         "unknown": 0,
         "transport": [ "event-idx", "indirect-desc", "version-1" ]
       },
       "host-features": {
         "device": {
           "type": "virtio-net",
           "data": [ "gso", "ctrl-mac-addr", "guest-announce", "ctrl-rx-extra",
                     "ctrl-vlan", "ctrl-rx", "ctrl-vq", "status", "mrg-rxbuf",
                     "host-ufo", "host-ecn", "host-tso6", "host-tso4",
                     "guest-ufo", "guest-ecn", "guest-tso6", "guest-tso4",
                     "mac", "ctrl-guest-offloads", "guest-csum", "csum" ]
           },
           "unknown": 0,
           "transport": [ "event-idx", "indirect-desc", "bad-feature",
                          "version-1", "any-layout", "notify-on-empty" ]
         }
       }
     }

4. Display status of a given virtio queue

  HMP Form:

    x-debug-virtio queue-status <path> <queue>

  Example:

    (qemu) x-debug-virtio queue-status /machine/peripheral-anon/device[3]/virtio-backend 0
    /machine/peripheral-anon/device[3]/virtio-backend:
      index:                0
      inuse:                0
      last_avail_idx:       61
      shadow_avail_idx:     292
      signalled_used:       61
      signalled_used_valid: 1
      VRing:
        num:         256
        num_default: 256
        align:       4096
        desc:        0x000000006c352000
        avail:       0x000000006c353000
        used:        0x000000006c353240

  QMP Form:

    { 'command': 'x-debug-virtio-queue-status',
      'data': { 'path': 'str', 'queue': 'uint16' },
      'returns': 'VirtQueueStatus'
    }

  Example:

  -> { "execute": "x-debug-virtio-queue-status",
       "arguments": {
           "path": "/machine/peripheral-anon/device[3]/virtio-backend",
           "queue": 0
       }
    }
  <- { "return": {
       "signalled_used": 373,
       "inuse": 0,
       "vring_desc": 864411648,
       "vring_num_default": 256,
       "signalled_used_valid": 1,
       "vring_avail": 864415744,
       "last_avail_idx": 373,
       "queue_index": 0,
       "vring_used": 864416320,
       "shadow_avail_idx": 619,
       "used_idx": 373,
       "vring_num": 256,
       "vring_align": 4096
       }
     }

5. Display element of a given virtio queue

  HMP Form:

    x-debug-virtio queue-element <path> <queue> [index]

  Example:

    Dump the information of the head element of the first queue of
    the first virtio device::

      (qemu) x-debug-virtio queue-element/machine/peripheral-anon/device[3]/virtio-backend 0
      index:  67
      ndescs: 1
      descs:  addr 0x6fe69800 len 1536 (write)

      (qemu) xp/128bx 0x6fe69800
      000000006fe69800: 0x02 0x00 0x00 0x00 0x00 0x00 0x00 0x00
      000000006fe69808: 0x00 0x00 0x01 0x00 0x52 0x54 0x00 0x12
      000000006fe69810: 0x34 0x56 0x52 0x54 0x00 0x09 0x51 0xde
      000000006fe69818: 0x08 0x00 0x45 0x00 0x00 0x4c 0x8f 0x32

    device[3] is a virtio-net device and we can see in the element buffer the
    MAC address of the card::

      [root@localhost ~]# ip link show ens4
      2: ens4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP m0
          link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff

    and the MAC address of the gateway::

      [root@localhost ~]# arp -a
      _gateway (192.168.122.1) at 52:54:00:09:51:de [ether] on ens4

  QMP Form:

    { 'command': 'x-debug-virtio-queue-element',
      'data': { 'path': 'str', 'queue': 'uint16', '*index': 'uint16' },
      'returns': 'VirtioQueueElement'
    }

  Example:

  -> { "execute": "x-debug-virtio-queue-element",
       "arguments": {
           "path": "/machine/peripheral-anon/device[3]/virtio-backend",
           "queue": 0
       }
    }
  -> { "return": {
          "index": 109,
          "ndescs": 1,
          "descs": [
              { "flags": [ "write" ], "len": 1536, "addr": 853145600 }
          ]
       }
    }

v4: re-send series as v3 didn't reach qemu-devel

v3: use qapi_free_VirtioInfoList() on the head of the list, not on the tail
    prefix the commands with x-debug-

v2: introduce VirtioType enum
    use an enum for the endianness
    change field names to stick to naming conventions (s/_/-/)
    add a patch to decode feature bits
    don't check if the queue is empty to allow to displa old elements
    use enum for desc flags
    manage indirect desc
    decode device features in the HMP command

Laurent Vivier (6):
  qmp: add QMP command x-debug-query-virtio
  qmp: add QMP command x-debug-virtio-status
  qmp: decode feature bits in virtio-status
  qmp: add QMP command x-debug-virtio-queue-status
  qmp: add QMP command x-debug-virtio-queue-element
  hmp: add x-debug-virtio commands

 Makefile                     |   2 +-
 Makefile.target              |   7 +-
 docs/system/monitor.rst      |   2 +
 hmp-commands-virtio.hx       | 160 +++++++++++
 hmp-commands.hx              |  10 +
 hw/block/virtio-blk.c        |  23 ++
 hw/char/virtio-serial-bus.c  |  11 +
 hw/display/virtio-gpu-base.c |  10 +
 hw/net/virtio-net.c          |  35 +++
 hw/scsi/virtio-scsi.c        |  12 +
 hw/virtio/Makefile.objs      |   2 +
 hw/virtio/virtio-balloon.c   |  13 +
 hw/virtio/virtio-iommu.c     |  14 +
 hw/virtio/virtio-stub.c      |  34 +++
 hw/virtio/virtio.c           | 542 +++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio.h   |  14 +
 include/monitor/hmp.h        |   4 +
 monitor/misc.c               |  17 ++
 qapi/Makefile.objs           |   2 +-
 qapi/qapi-schema.json        |   1 +
 qapi/virtio.json             | 502 ++++++++++++++++++++++++++++++++
 tests/qtest/qmp-cmd-test.c   |   1 +
 22 files changed, 1414 insertions(+), 4 deletions(-)
 create mode 100644 hmp-commands-virtio.hx
 create mode 100644 hw/virtio/virtio-stub.c
 create mode 100644 qapi/virtio.json

-- 
2.26.2




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC v4 1/6] qmp: add QMP command x-debug-query-virtio
  2020-05-07 13:47 [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
@ 2020-05-07 13:47 ` Laurent Vivier
  2020-05-07 13:47 ` [RFC v4 2/6] qmp: add QMP command x-debug-virtio-status Laurent Vivier
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Laurent Vivier @ 2020-05-07 13:47 UTC (permalink / raw)
  To: lvivier, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	Markus Armbruster, Dr. David Alan Gilbert, Eric Auger,
	Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau,
	Paolo Bonzini, Max Reitz

This new command lists all the instances of VirtIODevice with
their path and virtio type

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/virtio/Makefile.objs    |  2 ++
 hw/virtio/virtio-stub.c    | 14 ++++++++
 hw/virtio/virtio.c         | 28 ++++++++++++++++
 include/hw/virtio/virtio.h |  1 +
 qapi/Makefile.objs         |  2 +-
 qapi/qapi-schema.json      |  1 +
 qapi/virtio.json           | 68 ++++++++++++++++++++++++++++++++++++++
 tests/qtest/qmp-cmd-test.c |  1 +
 8 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 hw/virtio/virtio-stub.c
 create mode 100644 qapi/virtio.json

diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index 4e4d39a0a48f..0b649f120044 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -38,6 +38,8 @@ obj-$(CONFIG_VIRTIO_SERIAL) += virtio-serial-pci.o
 endif
 else
 common-obj-y += vhost-stub.o
+common-obj-y += virtio-stub.o
 endif
 
 common-obj-$(CONFIG_ALL) += vhost-stub.o
+common-obj-$(CONFIG_ALL) += virtio-stub.o
diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
new file mode 100644
index 000000000000..d4a88f5753a9
--- /dev/null
+++ b/hw/virtio/virtio-stub.c
@@ -0,0 +1,14 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-virtio.h"
+
+static void *qmp_virtio_unsupported(Error **errp)
+{
+    error_setg(errp, "Virtio is disabled");
+    return NULL;
+}
+
+VirtioInfoList *qmp_x_debug_query_virtio(Error **errp)
+{
+    return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index b6c8ef5bc025..05b640bcc267 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -13,6 +13,8 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qapi/qapi-commands-virtio.h"
+#include "qapi/qapi-visit-virtio.h"
 #include "cpu.h"
 #include "trace.h"
 #include "exec/address-spaces.h"
@@ -28,6 +30,8 @@
 #include "sysemu/dma.h"
 #include "sysemu/runstate.h"
 
+static QTAILQ_HEAD(, VirtIODevice) virtio_list;
+
 /*
  * The alignment to use between consumer and producer parts of vring.
  * x86 pagesize again. This is the default, used by transports like PCI
@@ -3628,6 +3632,7 @@ static void virtio_device_realize(DeviceState *dev, Error **errp)
 
     vdev->listener.commit = virtio_memory_listener_commit;
     memory_listener_register(&vdev->listener, vdev->dma_as);
+    QTAILQ_INSERT_TAIL(&virtio_list, vdev, next);
 }
 
 static void virtio_device_unrealize(DeviceState *dev, Error **errp)
@@ -3646,6 +3651,7 @@ static void virtio_device_unrealize(DeviceState *dev, Error **errp)
         }
     }
 
+    QTAILQ_REMOVE(&virtio_list, vdev, next);
     g_free(vdev->bus_name);
     vdev->bus_name = NULL;
 }
@@ -3802,6 +3808,8 @@ static void virtio_device_class_init(ObjectClass *klass, void *data)
     vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl;
 
     vdc->legacy_features |= VIRTIO_LEGACY_FEATURES;
+
+    QTAILQ_INIT(&virtio_list);
 }
 
 bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
@@ -3812,6 +3820,26 @@ bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
     return virtio_bus_ioeventfd_enabled(vbus);
 }
 
+VirtioInfoList *qmp_x_debug_query_virtio(Error **errp)
+{
+    VirtioInfoList *list = NULL;
+    VirtioInfoList *node;
+    VirtIODevice *vdev;
+
+    QTAILQ_FOREACH(vdev, &virtio_list, next) {
+        DeviceState *dev = DEVICE(vdev);
+        node = g_new0(VirtioInfoList, 1);
+        node->value = g_new(VirtioInfo, 1);
+        node->value->path = g_strdup(dev->canonical_path);
+        node->value->type = qapi_enum_parse(&VirtioType_lookup, vdev->name,
+                                            VIRTIO_TYPE_UNKNOWN, NULL);
+        node->next = list;
+        list = node;
+    }
+
+    return list;
+}
+
 static const TypeInfo virtio_device_info = {
     .name = TYPE_VIRTIO_DEVICE,
     .parent = TYPE_DEVICE,
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b69d51749635..65adce680188 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -111,6 +111,7 @@ struct VirtIODevice
     bool use_guest_notifier_mask;
     AddressSpace *dma_as;
     QLIST_HEAD(, VirtQueue) *vector_queues;
+    QTAILQ_ENTRY(VirtIODevice) next;
 };
 
 typedef struct VirtioDeviceClass {
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 4673ab7490df..4fae2e37cfc9 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -8,7 +8,7 @@ util-obj-y += qapi-util.o
 QAPI_COMMON_MODULES = audio authz block-core block char common control crypto
 QAPI_COMMON_MODULES += dump error introspect job machine migration misc
 QAPI_COMMON_MODULES += net pragma qdev qom rdma rocker run-state sockets tpm
-QAPI_COMMON_MODULES += trace transaction ui
+QAPI_COMMON_MODULES += trace transaction ui virtio
 QAPI_TARGET_MODULES = machine-target misc-target
 QAPI_MODULES = $(QAPI_COMMON_MODULES) $(QAPI_TARGET_MODULES)
 
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 43b0ba0dea22..189f5a0a7383 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -84,3 +84,4 @@
 { 'include': 'misc.json' }
 { 'include': 'misc-target.json' }
 { 'include': 'audio.json' }
+{ 'include': 'virtio.json' }
diff --git a/qapi/virtio.json b/qapi/virtio.json
new file mode 100644
index 000000000000..da59a118dbfb
--- /dev/null
+++ b/qapi/virtio.json
@@ -0,0 +1,68 @@
+##
+# = Virtio devices
+##
+
+##
+# @VirtioType:
+#
+# An enumeration of Virtio device types.
+#
+# Since: 5.1.0
+##
+{ 'enum': 'VirtioType',
+  'data': [ 'unknown', 'virtio-9p', 'virtio-blk', 'virtio-serial',
+            'virtio-gpu', 'virtio-input', 'virtio-net', 'virtio-scsi',
+            'vhost-user-fs', 'vhost-vsock', 'virtio-balloon', 'virtio-crypto',
+            'virtio-iommu', 'virtio-pmem', 'virtio-rng' ]
+}
+
+##
+# @VirtioInfo:
+#
+# Information about a given VirtIODevice
+#
+# @path: VirtIO device canonical path.
+#
+# @type: VirtIO device type.
+#
+# Since: 5.1
+#
+##
+{ 'struct': 'VirtioInfo',
+  'data': {
+    'path': 'str',
+    'type': 'VirtioType'
+  }
+}
+
+##
+# @x-debug-query-virtio:
+#
+# Return the list of all VirtIO devices
+#
+# Returns: list of @VirtioInfo
+#
+# Since: 5.1
+#
+# Example:
+#
+# -> { "execute": "x-debug-query-virtio" }
+# <- { "return": [
+#        {
+#            "path": "/machine/peripheral-anon/device[3]/virtio-backend",
+#            "type": "virtio-net"
+#        },
+#        {
+#            "path": "/machine/peripheral-anon/device[1]/virtio-backend",
+#            "type": "virtio-serial"
+#        },
+#        {
+#            "path": "/machine/peripheral-anon/device[0]/virtio-backend",
+#            "type": "virtio-blk"
+#        }
+#      ]
+#    }
+#
+##
+
+{ 'command': 'x-debug-query-virtio', 'returns': ['VirtioInfo'] }
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index 9f5228cd9951..a7bf8aab6357 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -92,6 +92,7 @@ static bool query_is_blacklisted(const char *cmd)
         "query-gic-capabilities", /* arm */
         /* Success depends on target-specific build configuration: */
         "query-pci",              /* CONFIG_PCI */
+        "x-debug-query-virtio",   /* CONFIG_VIRTIO */
         /* Success depends on launching SEV guest */
         "query-sev-launch-measure",
         /* Success depends on Host or Hypervisor SEV support */
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v4 2/6] qmp: add QMP command x-debug-virtio-status
  2020-05-07 13:47 [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
  2020-05-07 13:47 ` [RFC v4 1/6] qmp: add QMP command x-debug-query-virtio Laurent Vivier
@ 2020-05-07 13:47 ` Laurent Vivier
  2020-05-07 13:47 ` [RFC v4 3/6] qmp: decode feature bits in virtio-status Laurent Vivier
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Laurent Vivier @ 2020-05-07 13:47 UTC (permalink / raw)
  To: lvivier, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	Markus Armbruster, Dr. David Alan Gilbert, Eric Auger,
	Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau,
	Paolo Bonzini, Max Reitz

This new command shows the status of a VirtIODevice
(features, endianness and number of virtqueues)

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/virtio/virtio-stub.c |  5 +++
 hw/virtio/virtio.c      | 50 +++++++++++++++++++++++++++
 qapi/virtio.json        | 76 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+)

diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
index d4a88f5753a9..ddb592f72eee 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -12,3 +12,8 @@ VirtioInfoList *qmp_x_debug_query_virtio(Error **errp)
 {
     return qmp_virtio_unsupported(errp);
 }
+
+VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
+{
+    return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 05b640bcc267..af69e874258d 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3840,6 +3840,56 @@ VirtioInfoList *qmp_x_debug_query_virtio(Error **errp)
     return list;
 }
 
+static VirtIODevice *virtio_device_find(const char *path)
+{
+    VirtIODevice *vdev;
+
+    QTAILQ_FOREACH(vdev, &virtio_list, next) {
+        DeviceState *dev = DEVICE(vdev);
+
+        if (strcmp(dev->canonical_path, path) != 0) {
+            continue;
+        }
+        return vdev;
+    }
+
+    return NULL;
+}
+
+VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
+{
+    VirtIODevice *vdev;
+    VirtioStatus *status;
+
+    vdev = virtio_device_find(path);
+    if (vdev == NULL) {
+        error_setg(errp, "Path %s is not a VirtIO device", path);
+        return NULL;
+    }
+
+    status = g_new0(VirtioStatus, 1);
+    status->guest_features = vdev->guest_features;
+    status->host_features = vdev->host_features;
+    status->backend_features = vdev->backend_features;
+    status->device_id = vdev->device_id;
+
+    switch (vdev->device_endian) {
+    case VIRTIO_DEVICE_ENDIAN_LITTLE:
+        status->device_endian = VIRTIO_STATUS_ENDIANNESS_LITTLE;
+        break;
+    case VIRTIO_DEVICE_ENDIAN_BIG:
+        status->device_endian = VIRTIO_STATUS_ENDIANNESS_BIG;
+        break;
+    default:
+        status->device_endian = VIRTIO_STATUS_ENDIANNESS_UNKNOWN;
+        break;
+    }
+
+    status->num_vqs = virtio_get_num_queues(vdev);
+
+    return status;
+}
+
 static const TypeInfo virtio_device_info = {
     .name = TYPE_VIRTIO_DEVICE,
     .parent = TYPE_DEVICE,
diff --git a/qapi/virtio.json b/qapi/virtio.json
index da59a118dbfb..9142818d198c 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -66,3 +66,79 @@
 ##
 
 { 'command': 'x-debug-query-virtio', 'returns': ['VirtioInfo'] }
+
+##
+# @VirtioStatusEndianness:
+#
+# Enumeration of endianness for VirtioDevice
+#
+# Since: 5.1
+##
+{ 'enum': 'VirtioStatusEndianness',
+  'data': [ 'unknown', 'little', 'big' ]
+}
+
+##
+# @VirtioStatus:
+#
+# @device-id: VirtIODevice status
+#
+# @device-endian: VirtIODevice device_endian
+#
+# @guest-features: VirtIODevice guest_features
+#
+# @host-features: VirtIODevice host_features
+#
+# @backend-features: VirtIODevice backend_features
+#
+# @num-vqs: number of VirtIODevice queues
+#
+# Since: 5.1
+#
+##
+
+{ 'struct': 'VirtioStatus',
+  'data': {
+    'device-id': 'int',
+    'device-endian': 'VirtioStatusEndianness',
+    'guest-features': 'uint64',
+    'host-features': 'uint64',
+    'backend-features': 'uint64',
+    'num-vqs': 'uint16'
+  }
+}
+
+##
+# @x-debug-virtio-status:
+#
+# Return the status of virtio device
+#
+# @path: QOBject path of the VirtIODevice
+#
+# Returns: status of the VirtIODevice
+#
+# Since: 5.1
+#
+# Example:
+#
+# -> { "execute": "x-debug-virtio-status",
+#      "arguments": {
+#          "path": "/machine/peripheral-anon/device[3]/virtio-backend"
+#      }
+#   }
+# <- { "return": {
+#          "backend-features": 0,
+#          "guest-features": 5111807911,
+#          "num-vqs": 3,
+#          "host-features": 6337593319,
+#          "device-endian": "little",
+#          "device-id": 1
+#      }
+#    }
+#
+##
+
+{ 'command': 'x-debug-virtio-status',
+  'data': { 'path': 'str' },
+  'returns': 'VirtioStatus'
+}
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v4 3/6] qmp: decode feature bits in virtio-status
  2020-05-07 13:47 [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
  2020-05-07 13:47 ` [RFC v4 1/6] qmp: add QMP command x-debug-query-virtio Laurent Vivier
  2020-05-07 13:47 ` [RFC v4 2/6] qmp: add QMP command x-debug-virtio-status Laurent Vivier
@ 2020-05-07 13:47 ` Laurent Vivier
  2020-05-07 13:47 ` [RFC v4 4/6] qmp: add QMP command x-debug-virtio-queue-status Laurent Vivier
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Laurent Vivier @ 2020-05-07 13:47 UTC (permalink / raw)
  To: lvivier, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	Markus Armbruster, Dr. David Alan Gilbert, Eric Auger,
	Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau,
	Paolo Bonzini, Max Reitz

Display feature names instead of a features bitmap for host, guest
and backend.

Decode features according device type, transport features are
on the first line. Undecoded bits (if any) are stored in a separate
field.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/block/virtio-blk.c        |  23 +++++
 hw/char/virtio-serial-bus.c  |  11 +++
 hw/display/virtio-gpu-base.c |  10 ++
 hw/net/virtio-net.c          |  35 +++++++
 hw/scsi/virtio-scsi.c        |  12 +++
 hw/virtio/virtio-balloon.c   |  13 +++
 hw/virtio/virtio-iommu.c     |  14 +++
 hw/virtio/virtio.c           | 114 ++++++++++++++++++++-
 include/hw/virtio/virtio.h   |  13 +++
 qapi/virtio.json             | 186 +++++++++++++++++++++++++++++++++--
 10 files changed, 418 insertions(+), 13 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 97ba8a218727..5bb3f70cbd1b 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qapi/qapi-visit-virtio.h"
 #include "qemu/iov.h"
 #include "qemu/module.h"
 #include "qemu/error-report.h"
@@ -48,6 +49,28 @@ static VirtIOFeature feature_sizes[] = {
     {}
 };
 
+qmp_virtio_feature_map_t blk_map[] = {
+#define FEATURE_ENTRY(name) \
+    { VIRTIO_BLK_F_##name, VIRTIO_BLK_FEATURE_##name }
+    FEATURE_ENTRY(SIZE_MAX),
+    FEATURE_ENTRY(SEG_MAX),
+    FEATURE_ENTRY(GEOMETRY),
+    FEATURE_ENTRY(RO),
+    FEATURE_ENTRY(BLK_SIZE),
+    FEATURE_ENTRY(TOPOLOGY),
+    FEATURE_ENTRY(MQ),
+    FEATURE_ENTRY(DISCARD),
+    FEATURE_ENTRY(WRITE_ZEROES),
+#ifndef VIRTIO_BLK_NO_LEGACY
+    FEATURE_ENTRY(BARRIER),
+    FEATURE_ENTRY(SCSI),
+    FEATURE_ENTRY(FLUSH),
+    FEATURE_ENTRY(CONFIG_WCE),
+#endif
+#undef FEATURE_ENTRY
+    { -1, -1 }
+};
+
 static void virtio_blk_set_config_size(VirtIOBlock *s, uint64_t host_features)
 {
     s->config_size = MAX(VIRTIO_BLK_CFG_SIZE,
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 99a65bab7fff..872ba2ccf49f 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -20,6 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qapi/qapi-visit-virtio.h"
 #include "qemu/iov.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
@@ -33,6 +34,16 @@
 #include "hw/virtio/virtio-serial.h"
 #include "hw/virtio/virtio-access.h"
 
+qmp_virtio_feature_map_t serial_map[] = {
+#define FEATURE_ENTRY(name) \
+    { VIRTIO_CONSOLE_F_##name, VIRTIO_SERIAL_FEATURE_##name }
+    FEATURE_ENTRY(SIZE),
+    FEATURE_ENTRY(MULTIPORT),
+    FEATURE_ENTRY(EMERG_WRITE),
+#undef FEATURE_ENTRY
+    { -1, -1 }
+};
+
 static struct VirtIOSerialDevices {
     QLIST_HEAD(, VirtIOSerial) devices;
 } vserdevices;
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 55e07995feec..da66325452df 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -16,9 +16,19 @@
 #include "hw/virtio/virtio-gpu.h"
 #include "migration/blocker.h"
 #include "qapi/error.h"
+#include "qapi/qapi-visit-virtio.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 
+qmp_virtio_feature_map_t gpu_map[] = {
+#define FEATURE_ENTRY(name) \
+    { VIRTIO_GPU_F_##name, VIRTIO_GPU_FEATURE_##name }
+    FEATURE_ENTRY(VIRGL),
+    FEATURE_ENTRY(EDID),
+#undef FEATURE_ENTRY
+    { -1, -1 }
+};
+
 void
 virtio_gpu_base_reset(VirtIOGPUBase *g)
 {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3301869d4f90..e4e4e8df32a4 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -35,6 +35,7 @@
 #include "hw/qdev-properties.h"
 #include "qapi/qapi-types-migration.h"
 #include "qapi/qapi-events-migration.h"
+#include "qapi/qapi-visit-virtio.h"
 #include "hw/virtio/virtio-access.h"
 #include "migration/misc.h"
 #include "standard-headers/linux/ethtool.h"
@@ -85,6 +86,40 @@
 
 #endif
 
+qmp_virtio_feature_map_t net_map[] = {
+#define FEATURE_ENTRY(name) \
+    { VIRTIO_NET_F_##name, VIRTIO_NET_FEATURE_##name }
+    FEATURE_ENTRY(CSUM),
+    FEATURE_ENTRY(GUEST_CSUM),
+    FEATURE_ENTRY(CTRL_GUEST_OFFLOADS),
+    FEATURE_ENTRY(MTU),
+    FEATURE_ENTRY(MAC),
+    FEATURE_ENTRY(GUEST_TSO4),
+    FEATURE_ENTRY(GUEST_TSO6),
+    FEATURE_ENTRY(GUEST_ECN),
+    FEATURE_ENTRY(GUEST_UFO),
+    FEATURE_ENTRY(HOST_TSO4),
+    FEATURE_ENTRY(HOST_TSO6),
+    FEATURE_ENTRY(HOST_ECN),
+    FEATURE_ENTRY(HOST_UFO),
+    FEATURE_ENTRY(MRG_RXBUF),
+    FEATURE_ENTRY(STATUS),
+    FEATURE_ENTRY(CTRL_VQ),
+    FEATURE_ENTRY(CTRL_RX),
+    FEATURE_ENTRY(CTRL_VLAN),
+    FEATURE_ENTRY(CTRL_RX_EXTRA),
+    FEATURE_ENTRY(GUEST_ANNOUNCE),
+    FEATURE_ENTRY(MQ),
+    FEATURE_ENTRY(CTRL_MAC_ADDR),
+    FEATURE_ENTRY(STANDBY),
+    FEATURE_ENTRY(SPEED_DUPLEX),
+#ifndef VIRTIO_NET_NO_LEGACY
+    FEATURE_ENTRY(GSO),
+#endif /* VIRTIO_NET_NO_LEGACY */
+#undef FEATURE_ENTRY
+    { -1, -1 }
+};
+
 static inline __virtio16 *virtio_net_rsc_ext_num_packets(
     struct virtio_net_hdr *hdr)
 {
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 472bbd233bf3..7f027fbacf56 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -15,6 +15,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qapi/qapi-visit-virtio.h"
 #include "standard-headers/linux/virtio_ids.h"
 #include "hw/virtio/virtio-scsi.h"
 #include "migration/qemu-file-types.h"
@@ -28,6 +29,17 @@
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
 
+qmp_virtio_feature_map_t scsi_map[] = {
+#define FEATURE_ENTRY(name) \
+    { VIRTIO_SCSI_F_##name, VIRTIO_SCSI_FEATURE_##name }
+    FEATURE_ENTRY(INOUT),
+    FEATURE_ENTRY(HOTPLUG),
+    FEATURE_ENTRY(CHANGE),
+    FEATURE_ENTRY(T10_PI),
+#undef FEATURE_ENTRY
+    { -1, -1 }
+};
+
 static inline int virtio_scsi_get_lun(uint8_t *lun)
 {
     return ((lun[2] << 8) | lun[3]) & 0x3FFF;
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index a4729f7fc930..3cd9b319e77a 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -26,6 +26,7 @@
 #include "qapi/error.h"
 #include "qapi/qapi-events-misc.h"
 #include "qapi/visitor.h"
+#include "qapi/qapi-visit-virtio.h"
 #include "trace.h"
 #include "qemu/error-report.h"
 #include "migration/misc.h"
@@ -33,6 +34,18 @@
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
 
+qmp_virtio_feature_map_t balloon_map[] = {
+#define FEATURE_ENTRY(name) \
+    { VIRTIO_BALLOON_F_##name, VIRTIO_BALLOON_FEATURE_##name }
+    FEATURE_ENTRY(MUST_TELL_HOST),
+    FEATURE_ENTRY(STATS_VQ),
+    FEATURE_ENTRY(DEFLATE_ON_OOM),
+    FEATURE_ENTRY(FREE_PAGE_HINT),
+    FEATURE_ENTRY(PAGE_POISON),
+#undef FEATURE_ENTRY
+    { -1, -1 }
+};
+
 #define BALLOON_PAGE_SIZE  (1 << VIRTIO_BALLOON_PFN_SHIFT)
 
 typedef struct PartiallyBalloonedPage {
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 22ba8848c2fe..89858321ef87 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -25,6 +25,7 @@
 #include "hw/virtio/virtio.h"
 #include "sysemu/kvm.h"
 #include "qapi/error.h"
+#include "qapi/qapi-visit-virtio.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 
@@ -36,6 +37,19 @@
 #include "hw/pci/pci_bus.h"
 #include "hw/pci/pci.h"
 
+qmp_virtio_feature_map_t iommu_map[] = {
+#define FEATURE_ENTRY(name) \
+    { VIRTIO_IOMMU_F_##name, VIRTIO_IOMMU_FEATURE_##name }
+    FEATURE_ENTRY(INPUT_RANGE),
+    FEATURE_ENTRY(DOMAIN_RANGE),
+    FEATURE_ENTRY(MAP_UNMAP),
+    FEATURE_ENTRY(BYPASS),
+    FEATURE_ENTRY(PROBE),
+    FEATURE_ENTRY(MMIO),
+#undef FEATURE_ENTRY
+    { -1, -1 }
+};
+
 /* Max size */
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index af69e874258d..59bf6ef651a6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -29,9 +29,30 @@
 #include "hw/virtio/virtio-access.h"
 #include "sysemu/dma.h"
 #include "sysemu/runstate.h"
+#include "config-devices.h"
 
 static QTAILQ_HEAD(, VirtIODevice) virtio_list;
 
+static qmp_virtio_feature_map_t transport_map[] = {
+#define FEATURE_ENTRY(name) \
+    { VIRTIO_F_##name, VIRTIO_TRANSPORT_FEATURE_##name }
+    FEATURE_ENTRY(NOTIFY_ON_EMPTY),
+    FEATURE_ENTRY(ANY_LAYOUT),
+    FEATURE_ENTRY(VERSION_1),
+    FEATURE_ENTRY(IOMMU_PLATFORM),
+    FEATURE_ENTRY(RING_PACKED),
+    FEATURE_ENTRY(ORDER_PLATFORM),
+    FEATURE_ENTRY(SR_IOV),
+    FEATURE_ENTRY(BAD_FEATURE),
+#undef FEATURE_ENTRY
+#define FEATURE_ENTRY(name) \
+    { VIRTIO_RING_F_##name, VIRTIO_TRANSPORT_FEATURE_##name }
+    FEATURE_ENTRY(INDIRECT_DESC),
+    FEATURE_ENTRY(EVENT_IDX),
+#undef FEATURE_ENTRY
+    { -1, -1 }
+};
+
 /*
  * The alignment to use between consumer and producer parts of vring.
  * x86 pagesize again. This is the default, used by transports like PCI
@@ -3856,6 +3877,90 @@ static VirtIODevice *virtio_device_find(const char *path)
     return NULL;
 }
 
+#define CONVERT_FEATURES(type, map)                \
+    ({                                           \
+        type *list = NULL;                         \
+        type *node;                                \
+        for (i = 0; map[i].virtio_bit != -1; i++) {\
+            bit = 1ULL << map[i].virtio_bit;       \
+            if ((bitmap & bit) == 0) {             \
+                continue;                          \
+            }                                      \
+            node = g_new0(type, 1);                \
+            node->value = map[i].qapi_virtio_enum; \
+            node->next = list;                     \
+            list = node;                           \
+            bitmap ^= bit;                         \
+        }                                          \
+        list;                                      \
+    })
+
+static VirtioStatusFeatures *qmp_decode_features(const char *name,
+                                                 uint64_t bitmap)
+{
+    VirtioStatusFeatures *features;
+    uint64_t bit;
+    int i;
+
+    features = g_new0(VirtioStatusFeatures, 1);
+
+    /* transport features */
+    features->transport = CONVERT_FEATURES(VirtioTransportFeatureList, \
+                                           transport_map);
+
+    /* device features */
+    features->device = g_new0(VirtioDeviceFeatures, 1);
+    features->device->type = qapi_enum_parse(&VirtioDeviceFeaturesKind_lookup,
+                                             name, -1, NULL);
+    switch (features->device->type) {
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SERIAL:
+        features->device->u.virtio_serial.data =
+                          CONVERT_FEATURES(VirtioSerialFeatureList, serial_map);
+        break;
+#ifdef CONFIG_VIRTIO_BLK
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BLK:
+        features->device->u.virtio_blk.data =
+                                CONVERT_FEATURES(VirtioBlkFeatureList, blk_map);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_GPU
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_GPU:
+        features->device->u.virtio_gpu.data =
+                                CONVERT_FEATURES(VirtioGpuFeatureList, gpu_map);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_NET
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_NET:
+        features->device->u.virtio_net.data =
+                                CONVERT_FEATURES(VirtioNetFeatureList, net_map);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_SCSI
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SCSI:
+        features->device->u.virtio_scsi.data =
+                              CONVERT_FEATURES(VirtioScsiFeatureList, scsi_map);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_BALLOON
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BALLOON:
+        features->device->u.virtio_balloon.data =
+                        CONVERT_FEATURES(VirtioBalloonFeatureList, balloon_map);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_IOMMU
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_IOMMU:
+        features->device->u.virtio_iommu.data =
+                            CONVERT_FEATURES(VirtioIommuFeatureList, iommu_map);
+        break;
+#endif
+    default:
+        g_assert_not_reached();
+    }
+    features->unknown = bitmap;
+
+    return features;
+}
+
 VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
 {
     VirtIODevice *vdev;
@@ -3868,9 +3973,12 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
     }
 
     status = g_new0(VirtioStatus, 1);
-    status->guest_features = vdev->guest_features;
-    status->host_features = vdev->host_features;
-    status->backend_features = vdev->backend_features;
+    status->guest_features = qmp_decode_features(vdev->name,
+                                                 vdev->guest_features);
+    status->host_features = qmp_decode_features(vdev->name,
+                                                vdev->host_features);
+    status->backend_features = qmp_decode_features(vdev->name,
+                                                   vdev->backend_features);
     status->device_id = vdev->device_id;
 
     switch (vdev->device_endian) {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 65adce680188..f10c3365e367 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -166,6 +166,19 @@ typedef struct VirtioDeviceClass {
     bool (*primary_unplug_pending)(void *opaque);
 } VirtioDeviceClass;
 
+typedef struct {
+    int virtio_bit;
+    int qapi_virtio_enum;
+} qmp_virtio_feature_map_t;
+
+extern qmp_virtio_feature_map_t serial_map[];
+extern qmp_virtio_feature_map_t blk_map[];
+extern qmp_virtio_feature_map_t gpu_map[];
+extern qmp_virtio_feature_map_t net_map[];
+extern qmp_virtio_feature_map_t scsi_map[];
+extern qmp_virtio_feature_map_t balloon_map[];
+extern qmp_virtio_feature_map_t iommu_map[];
+
 void virtio_instance_init_common(Object *proxy_obj, void *data,
                                  size_t vdev_size, const char *vdev_name);
 
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 9142818d198c..69dd107d0c9b 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -78,6 +78,150 @@
   'data': [ 'unknown', 'little', 'big' ]
 }
 
+##
+# @VirtioTransportFeature:
+#
+# An enumeration of Virtio device transport features, including virtio-ring
+#
+# Since: 5.1
+##
+
+{ 'enum': 'VirtioTransportFeature',
+  'data': [ 'notify-on-empty', 'any-layout', 'version-1', 'iommu-platform',
+            'ring-packed', 'order-platform', 'sr-iov', 'indirect-desc',
+            'event-idx', 'bad-feature' ]
+}
+
+##
+# @VirtioSerialFeature:
+#
+# An enumeration of Virtio serial features
+#
+# Since: 5.1
+##
+
+{ 'enum': 'VirtioSerialFeature',
+  'data': [ 'size', 'multiport', 'emerg-write' ]
+}
+
+##
+# @VirtioBlkFeature:
+#
+# An enumeration of Virtio block features
+#
+# Since: 5.1
+##
+
+{ 'enum': 'VirtioBlkFeature',
+  'data': [ 'size-max', 'seg-max', 'geometry', 'ro', 'blk-size', 'topology',              'mq', 'discard', 'write-zeroes', 'barrier', 'scsi', 'flush',
+            'config-wce' ]
+}
+
+##
+# @VirtioGpuFeature:
+#
+# An enumeration of Virtio gpu features
+#
+# Since: 5.1
+##
+
+{ 'enum': 'VirtioGpuFeature',
+  'data': [ 'virgl', 'edid' ]
+}
+
+##
+# @VirtioNetFeature:
+#
+# An enumeration of Virtio net features
+#
+# Since: 5.1
+##
+
+{ 'enum': 'VirtioNetFeature',
+  'data': [ 'csum', 'guest-csum', 'ctrl-guest-offloads', 'mtu', 'mac',
+            'guest-tso4', 'guest-tso6', 'guest-ecn', 'guest-ufo',
+            'host-tso4', 'host-tso6', 'host-ecn', 'host-ufo', 'mrg-rxbuf',
+            'status', 'ctrl-vq', 'ctrl-rx', 'ctrl-vlan', 'ctrl-rx-extra',
+            'guest-announce', 'mq', 'ctrl-mac-addr', 'standby',
+            'speed-duplex', 'gso' ]
+}
+
+##
+# @VirtioScsiFeature:
+#
+# An enumeration of Virtio scsi features
+#
+# Since: 5.1
+##
+
+{ 'enum': 'VirtioScsiFeature',
+  'data': [ 'inout', 'hotplug', 'change', 't10-pi' ]
+}
+
+##
+# @VirtioBalloonFeature:
+#
+# An enumeration of Virtio balloon features
+#
+# Since: 5.1
+##
+
+{ 'enum': 'VirtioBalloonFeature',
+  'data': [ 'must-tell-host', 'stats-vq', 'deflate-on-oom', 'free-page-hint',
+            'page-poison' ]
+}
+
+##
+# @VirtioIommuFeature:
+#
+# An enumeration of Virtio iommu features
+#
+# Since: 5.1
+##
+
+{ 'enum': 'VirtioIommuFeature',
+  'data': [ 'input-range', 'domain-range', 'map-unmap', 'bypass', 'probe',
+            'mmio' ]
+}
+
+##
+# @VirtioDeviceFeatures:
+#
+# An union to define the list of features for a virtio device
+#
+# Since: 5.1
+##
+
+{ 'union': 'VirtioDeviceFeatures',
+  'data': {
+    'virtio-serial': [ 'VirtioSerialFeature' ],
+    'virtio-blk': [ 'VirtioBlkFeature' ],
+    'virtio-gpu': [ 'VirtioGpuFeature' ],
+    'virtio-net': [ 'VirtioNetFeature' ],
+    'virtio-scsi': [ 'VirtioScsiFeature' ],
+    'virtio-balloon': [ 'VirtioBalloonFeature' ],
+    'virtio-iommu': [ 'VirtioIommuFeature' ]
+  }
+}
+
+##
+# @VirtioStatusFeatures:
+#
+# @transport: the list of transport features of the virtio device
+#
+# @device: the list of the virtio device specific features
+#
+# @unknown: virtio bitmap of the undecoded features
+#
+# Since: 5.1
+##
+
+{ 'struct': 'VirtioStatusFeatures',
+  'data': { 'transport': [ 'VirtioTransportFeature' ],
+            'device': 'VirtioDeviceFeatures',
+            'unknown': 'uint64' }
+}
+
 ##
 # @VirtioStatus:
 #
@@ -101,9 +245,9 @@
   'data': {
     'device-id': 'int',
     'device-endian': 'VirtioStatusEndianness',
-    'guest-features': 'uint64',
-    'host-features': 'uint64',
-    'backend-features': 'uint64',
+    'guest-features': 'VirtioStatusFeatures',
+    'host-features': 'VirtioStatusFeatures',
+    'backend-features': 'VirtioStatusFeatures',
     'num-vqs': 'uint16'
   }
 }
@@ -123,18 +267,40 @@
 #
 # -> { "execute": "x-debug-virtio-status",
 #      "arguments": {
-#          "path": "/machine/peripheral-anon/device[3]/virtio-backend"
+#          "path": "/machine/peripheral-anon/device[1]/virtio-backend"
 #      }
 #   }
 # <- { "return": {
-#          "backend-features": 0,
-#          "guest-features": 5111807911,
-#          "num-vqs": 3,
-#          "host-features": 6337593319,
 #          "device-endian": "little",
-#          "device-id": 1
+#          "device-id": 3,
+#          "backend-features": {
+#              "device": {
+#                  "type": "virtio-serial",
+#                  "data": []
+#              },
+#              "unknown": 0,
+#              "transport": []
+#          },
+#          "num-vqs": 64,
+#          "guest-features": {
+#              "device": {
+#                  "type": "virtio-serial",
+#                  "data": [ "multiport" ]
+#              },
+#              "unknown": 0,
+#              "transport": [ "event-idx", "indirect-desc", "version-1" ]
+#          },
+#          "host-features": {
+#              "device": {
+#                  "type": "virtio-serial",
+#                  "data": [ "emerg-write", "multiport" ]
+#              },
+#              "unknown": 0,
+#              "transport": [ "event-idx", "indirect-desc", "bad-feature",
+#                             "version-1", "any-layout", "notify-on-empty" ]
+#          }
 #      }
-#    }
+#  }
 #
 ##
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v4 4/6] qmp: add QMP command x-debug-virtio-queue-status
  2020-05-07 13:47 [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
                   ` (2 preceding siblings ...)
  2020-05-07 13:47 ` [RFC v4 3/6] qmp: decode feature bits in virtio-status Laurent Vivier
@ 2020-05-07 13:47 ` Laurent Vivier
  2020-05-07 13:47 ` [RFC v4 5/6] qmp: add QMP command x-debug-virtio-queue-element Laurent Vivier
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Laurent Vivier @ 2020-05-07 13:47 UTC (permalink / raw)
  To: lvivier, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	Markus Armbruster, Dr. David Alan Gilbert, Eric Auger,
	Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau,
	Paolo Bonzini, Max Reitz

This new command shows internal status of a VirtQueue.
(vrings and indexes).

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/virtio/virtio-stub.c |  6 +++
 hw/virtio/virtio.c      | 35 +++++++++++++++
 qapi/virtio.json        | 98 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+)

diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
index ddb592f72eee..3c1bf172acf6 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -17,3 +17,9 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
 {
     return qmp_virtio_unsupported(errp);
 }
+
+VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
+                                                 uint16_t queue, Error **errp)
+{
+    return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 59bf6ef651a6..57552bf05014 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3877,6 +3877,41 @@ static VirtIODevice *virtio_device_find(const char *path)
     return NULL;
 }
 
+VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
+                                                 uint16_t queue, Error **errp)
+{
+    VirtIODevice *vdev;
+    VirtQueueStatus *status;
+
+    vdev = virtio_device_find(path);
+    if (vdev == NULL) {
+        error_setg(errp, "Path %s is not a VirtIO device", path);
+        return NULL;
+    }
+
+    if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) {
+        error_setg(errp, "Invalid virtqueue number %d", queue);
+        return NULL;
+    }
+
+    status = g_new0(VirtQueueStatus, 1);
+    status->queue_index = vdev->vq[queue].queue_index;
+    status->inuse = vdev->vq[queue].inuse;
+    status->vring_num = vdev->vq[queue].vring.num;
+    status->vring_num_default = vdev->vq[queue].vring.num_default;
+    status->vring_align = vdev->vq[queue].vring.align;
+    status->vring_desc = vdev->vq[queue].vring.desc;
+    status->vring_avail = vdev->vq[queue].vring.avail;
+    status->vring_used = vdev->vq[queue].vring.used;
+    status->last_avail_idx = vdev->vq[queue].last_avail_idx;
+    status->shadow_avail_idx = vdev->vq[queue].shadow_avail_idx;
+    status->used_idx = vdev->vq[queue].used_idx;
+    status->signalled_used = vdev->vq[queue].signalled_used;
+    status->signalled_used_valid = vdev->vq[queue].signalled_used_valid;
+
+    return status;
+}
+
 #define CONVERT_FEATURES(type, map)                \
     ({                                           \
         type *list = NULL;                         \
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 69dd107d0c9b..43c234a9fc69 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -308,3 +308,101 @@
   'data': { 'path': 'str' },
   'returns': 'VirtioStatus'
 }
+
+##
+# @VirtQueueStatus:
+#
+# Status of a VirtQueue
+#
+# @queue-index: VirtQueue queue_index
+#
+# @inuse: VirtQueue inuse
+#
+# @vring-num: VirtQueue vring.num
+#
+# @vring-num-default: VirtQueue vring.num_default
+#
+# @vring-align: VirtQueue vring.align
+#
+# @vring-desc: VirtQueue vring.desc
+#
+# @vring-avail: VirtQueue vring.avail
+#
+# @vring-used: VirtQueue vring.used
+#
+# @last-avail-idx: VirtQueue last_avail_idx
+#
+# @shadow-avail-idx: VirtQueue shadow_avail_idx
+#
+# @used-idx: VirtQueue used_idx
+#
+# @signalled-used: VirtQueue signalled_used
+#
+# @signalled-used-valid: VirtQueue signalled_used_valid
+#
+# Since: 5.1
+#
+##
+
+{ 'struct': 'VirtQueueStatus',
+  'data': {
+    'queue-index': 'uint16',
+    'inuse': 'uint32',
+    'vring-num': 'int',
+    'vring-num-default': 'int',
+    'vring-align': 'int',
+    'vring-desc': 'uint64',
+    'vring-avail': 'uint64',
+    'vring-used': 'uint64',
+    'last-avail-idx': 'uint16',
+    'shadow-avail-idx': 'uint16',
+    'used-idx': 'uint16',
+    'signalled-used': 'uint16',
+    'signalled-used-valid': 'uint16'
+  }
+}
+
+##
+# @x-debug-virtio-queue-status:
+#
+# Return the status of a given VirtQueue
+#
+# @path: QOBject path of the VirtIODevice
+#
+# @queue: queue number to examine
+#
+# Returns: Status of the VirtQueue
+#
+# Since: 5.1
+#
+# Example:
+#
+# -> { "execute": "x-debug-virtio-queue-status",
+#      "arguments": {
+#          "path": "/machine/peripheral-anon/device[3]/virtio-backend",
+#          "queue": 0
+#      }
+#   }
+# <- { "return": {
+#      "signalled-used": 373,
+#      "inuse": 0,
+#      "vring-desc": 864411648,
+#      "vring-num-default": 256,
+#      "signalled-used-valid": 1,
+#      "vring-avail": 864415744,
+#      "last-avail-idx": 373,
+#      "queue-index": 0,
+#      "vring-used": 864416320,
+#      "shadow-avail-idx": 619,
+#      "used-idx": 373,
+#      "vring-num": 256,
+#      "vring-align": 4096
+#      }
+#    }
+#
+##
+
+{ 'command': 'x-debug-virtio-queue-status',
+  'data': { 'path': 'str', 'queue': 'uint16' },
+  'returns': 'VirtQueueStatus'
+}
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v4 5/6] qmp: add QMP command x-debug-virtio-queue-element
  2020-05-07 13:47 [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
                   ` (3 preceding siblings ...)
  2020-05-07 13:47 ` [RFC v4 4/6] qmp: add QMP command x-debug-virtio-queue-status Laurent Vivier
@ 2020-05-07 13:47 ` Laurent Vivier
  2020-05-07 13:48 ` [RFC v4 6/6] hmp: add x-debug-virtio commands Laurent Vivier
  2020-05-07 20:11 ` [RFC v4 0/6] hmp, qmp: Add some commands to introspect virtio devices no-reply
  6 siblings, 0 replies; 12+ messages in thread
From: Laurent Vivier @ 2020-05-07 13:47 UTC (permalink / raw)
  To: lvivier, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	Markus Armbruster, Dr. David Alan Gilbert, Eric Auger,
	Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau,
	Paolo Bonzini, Max Reitz

This new command shows the information of a VirtQueue element.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/virtio/virtio-stub.c |   9 +++
 hw/virtio/virtio.c      | 130 ++++++++++++++++++++++++++++++++++++++++
 qapi/virtio.json        |  94 +++++++++++++++++++++++++++++
 3 files changed, 233 insertions(+)

diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
index 3c1bf172acf6..8275e31430e7 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -23,3 +23,12 @@ VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
 {
     return qmp_virtio_unsupported(errp);
 }
+
+VirtioQueueElement *qmp_x_debug_virtio_queue_element(const char* path,
+                                                     uint16_t queue,
+                                                     bool has_index,
+                                                     uint16_t index,
+                                                     Error **errp)
+{
+    return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 57552bf05014..66dc2cef1b39 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -4033,6 +4033,136 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
     return status;
 }
 
+static VirtioRingDescFlagsList *qmp_decode_vring_desc_flags(uint16_t flags)
+{
+    VirtioRingDescFlagsList *list = NULL;
+    VirtioRingDescFlagsList *node;
+    int i;
+    struct {
+        uint16_t flag;
+        VirtioRingDescFlags value;
+    } map[] = {
+        { VRING_DESC_F_NEXT, VIRTIO_RING_DESC_FLAGS_NEXT },
+        { VRING_DESC_F_WRITE, VIRTIO_RING_DESC_FLAGS_WRITE },
+        { VRING_DESC_F_INDIRECT, VIRTIO_RING_DESC_FLAGS_INDIRECT },
+        { 1 << VRING_PACKED_DESC_F_AVAIL, VIRTIO_RING_DESC_FLAGS_AVAIL },
+        { 1 << VRING_PACKED_DESC_F_USED, VIRTIO_RING_DESC_FLAGS_USED },
+        { 0, -1 }
+    };
+
+    for (i = 0; map[i].flag; i++) {
+        if ((map[i].flag & flags) == 0) {
+            continue;
+        }
+        node = g_malloc0(sizeof(VirtioRingDescFlagsList));
+        node->value = map[i].value;
+        node->next = list;
+        list = node;
+    }
+
+    return list;
+}
+
+VirtioQueueElement *qmp_x_debug_virtio_queue_element(const char* path,
+                                                     uint16_t queue,
+                                                     bool has_index,
+                                                     uint16_t index,
+                                                     Error **errp)
+{
+    VirtIODevice *vdev;
+    VirtQueue *vq;
+    VirtioQueueElement *element = NULL;
+
+    vdev = virtio_device_find(path);
+    if (vdev == NULL) {
+        error_setg(errp, "Path %s is not a VirtIO device", path);
+        return NULL;
+    }
+
+    if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) {
+        error_setg(errp, "Invalid virtqueue number %d", queue);
+        return NULL;
+    }
+    vq = &vdev->vq[queue];
+
+    if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
+        error_setg(errp, "Packed ring not supported");
+        return NULL;
+    } else {
+        unsigned int head, i, max;
+        VRingMemoryRegionCaches *caches;
+        MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
+        MemoryRegionCache *desc_cache;
+        VRingDesc desc;
+        VirtioRingDescList *list = NULL;
+        VirtioRingDescList *node;
+        int rc;
+
+        RCU_READ_LOCK_GUARD();
+
+        max = vq->vring.num;
+
+        if (!has_index) {
+            head = vring_avail_ring(vq, vq->last_avail_idx % vq->vring.num);
+        } else {
+            head = vring_avail_ring(vq, index % vq->vring.num);
+        }
+        i = head;
+
+        caches = vring_get_region_caches(vq);
+        if (!caches) {
+            error_setg(errp, "Region caches not initialized");
+            return NULL;
+        }
+
+        if (caches->desc.len < max * sizeof(VRingDesc)) {
+            error_setg(errp, "Cannot map descriptor ring");
+            return NULL;
+        }
+
+        desc_cache = &caches->desc;
+        vring_split_desc_read(vdev, &desc, desc_cache, i);
+        if (desc.flags & VRING_DESC_F_INDIRECT) {
+            int64_t len;
+
+            len = address_space_cache_init(&indirect_desc_cache, vdev->dma_as,
+                                           desc.addr, desc.len, false);
+            desc_cache = &indirect_desc_cache;
+            if (len < desc.len) {
+                error_setg(errp, "Cannot map indirect buffer");
+                goto done;
+            }
+            i = 0;
+            vring_split_desc_read(vdev, &desc, desc_cache, i);
+        }
+
+        element = g_new0(VirtioQueueElement, 1);
+        element->index = head;
+        element->ndescs = 0;
+
+        do {
+            node = g_new0(VirtioRingDescList, 1);
+            node->value = g_new0(VirtioRingDesc, 1);
+            node->value->addr = desc.addr;
+            node->value->len = desc.len;
+            node->value->flags = qmp_decode_vring_desc_flags(desc.flags);
+            node->next = list;
+            list = node;
+
+            element->ndescs++;
+
+            rc = virtqueue_split_read_next_desc(vdev, &desc, desc_cache,
+                                                max, &i);
+        } while (rc == VIRTQUEUE_READ_DESC_MORE);
+
+        element->descs = list;
+done:
+        address_space_cache_destroy(&indirect_desc_cache);
+    }
+
+    return element;
+}
+
 static const TypeInfo virtio_device_info = {
     .name = TYPE_VIRTIO_DEVICE,
     .parent = TYPE_DEVICE,
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 43c234a9fc69..a55103dca780 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -406,3 +406,97 @@
   'data': { 'path': 'str', 'queue': 'uint16' },
   'returns': 'VirtQueueStatus'
 }
+
+##
+# @VirtioRingDescFlags:
+#
+# An enumeration of the virtio ring descriptor flags
+#
+# Since: 5.1
+#
+##
+
+{ 'enum': 'VirtioRingDescFlags',
+  'data': [ 'next', 'write', 'indirect', 'avail', 'used' ]
+}
+
+##
+# @VirtioRingDesc:
+#
+# @addr: guest physical address of the descriptor data
+#
+# @len: length of the descriptor data
+#
+# @flags: descriptor flags
+#
+# Since: 5.1
+#
+##
+
+{ 'struct': 'VirtioRingDesc',
+  'data': {
+    'addr': 'uint64',
+    'len': 'uint32',
+    'flags': [ 'VirtioRingDescFlags' ]
+  }
+}
+
+##
+# @VirtioQueueElement:
+#
+# @index: index of the element in the queue
+#
+# @ndescs: number of descriptors
+#
+# @descs: list of the descriptors
+#
+# Since: 5.1
+#
+##
+
+{ 'struct': 'VirtioQueueElement',
+  'data': {
+    'index': 'uint32',
+    'ndescs': 'uint32',
+    'descs': ['VirtioRingDesc']
+  }
+}
+
+##
+# @x-debug-virtio-queue-element:
+#
+# Return the information about an element queue (by default head)
+#
+# @path: QOBject path of the VirtIODevice
+#
+# @queue: queue number to examine
+#
+# @index: the index in the queue, by default head
+#
+# Returns: the element information
+#
+# Since: 5.1
+#
+# Example:
+#
+# -> { "execute": "x-debug-virtio-queue-element",
+#      "arguments": {
+#          "path": "/machine/peripheral-anon/device[3]/virtio-backend",
+#          "queue": 0
+#      }
+#   }
+# -> { "return": {
+#         "index": 24,
+#         "ndescs": 1,
+#         "descs": [
+#             { "flags": ["write"], "len": 1536, "addr": 2027557376 }
+#         ]
+#      }
+#   }
+#
+##
+
+{ 'command': 'x-debug-virtio-queue-element',
+  'data': { 'path': 'str', 'queue': 'uint16', '*index': 'uint16' },
+  'returns': 'VirtioQueueElement'
+}
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v4 6/6] hmp: add x-debug-virtio commands
  2020-05-07 13:47 [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
                   ` (4 preceding siblings ...)
  2020-05-07 13:47 ` [RFC v4 5/6] qmp: add QMP command x-debug-virtio-queue-element Laurent Vivier
@ 2020-05-07 13:48 ` Laurent Vivier
  2020-05-13 10:51   ` Dr. David Alan Gilbert
  2020-05-07 20:11 ` [RFC v4 0/6] hmp, qmp: Add some commands to introspect virtio devices no-reply
  6 siblings, 1 reply; 12+ messages in thread
From: Laurent Vivier @ 2020-05-07 13:48 UTC (permalink / raw)
  To: lvivier, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	Markus Armbruster, Dr. David Alan Gilbert, Eric Auger,
	Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau,
	Paolo Bonzini, Max Reitz

This patch implements HMP version of the virtio QMP commands

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 Makefile                |   2 +-
 Makefile.target         |   7 +-
 docs/system/monitor.rst |   2 +
 hmp-commands-virtio.hx  | 160 +++++++++++++++++++++++++++++++++
 hmp-commands.hx         |  10 +++
 hw/virtio/virtio.c      | 193 +++++++++++++++++++++++++++++++++++++++-
 include/monitor/hmp.h   |   4 +
 monitor/misc.c          |  17 ++++
 8 files changed, 391 insertions(+), 4 deletions(-)
 create mode 100644 hmp-commands-virtio.hx

diff --git a/Makefile b/Makefile
index 34275f57c9cb..feb300ebb2d4 100644
--- a/Makefile
+++ b/Makefile
@@ -1099,7 +1099,7 @@ $(MANUAL_BUILDDIR)/interop/index.html: $(call manual-deps,interop)
 $(MANUAL_BUILDDIR)/specs/index.html: $(call manual-deps,specs)
 	$(call build-manual,specs,html)
 
-$(MANUAL_BUILDDIR)/system/index.html: $(call manual-deps,system) $(SRC_PATH)/hmp-commands.hx $(SRC_PATH)/hmp-commands-info.hx $(SRC_PATH)/qemu-options.hx
+$(MANUAL_BUILDDIR)/system/index.html: $(call manual-deps,system) $(SRC_PATH)/hmp-commands.hx $(SRC_PATH)/hmp-commands-info.hx $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/hmp-commands-virtio.hx
 	$(call build-manual,system,html)
 
 $(MANUAL_BUILDDIR)/tools/index.html: $(call manual-deps,tools) $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/docs/qemu-option-trace.rst.inc
diff --git a/Makefile.target b/Makefile.target
index 8ed1eba95b9c..66d3ff9bc350 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -171,7 +171,7 @@ else
 obj-y += hw/$(TARGET_BASE_ARCH)/
 endif
 
-generated-files-y += hmp-commands.h hmp-commands-info.h
+generated-files-y += hmp-commands.h hmp-commands-info.h hmp-commands-virtio.h
 generated-files-y += config-devices.h
 
 endif # CONFIG_SOFTMMU
@@ -220,10 +220,13 @@ hmp-commands.h: $(SRC_PATH)/hmp-commands.hx $(SRC_PATH)/scripts/hxtool
 hmp-commands-info.h: $(SRC_PATH)/hmp-commands-info.hx $(SRC_PATH)/scripts/hxtool
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"GEN","$(TARGET_DIR)$@")
 
+hmp-commands-virtio.h: $(SRC_PATH)/hmp-commands-virtio.hx $(SRC_PATH)/scripts/hxtool
+	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"GEN","$(TARGET_DIR)$@")
+
 clean: clean-target
 	rm -f *.a *~ $(PROGS)
 	rm -f $(shell find . -name '*.[od]')
-	rm -f hmp-commands.h gdbstub-xml.c
+	rm -f hmp-commands.h hmp-commands-virtio.h gdbstub-xml.c
 	rm -f trace/generated-helpers.c trace/generated-helpers.c-timestamp
 ifdef CONFIG_TRACE_SYSTEMTAP
 	rm -f *.stp
diff --git a/docs/system/monitor.rst b/docs/system/monitor.rst
index 0bcd5da21644..985c3f51ffe7 100644
--- a/docs/system/monitor.rst
+++ b/docs/system/monitor.rst
@@ -21,6 +21,8 @@ The following commands are available:
 
 .. hxtool-doc:: hmp-commands.hx
 
+.. hxtool-doc:: hmp-commands-virtio.hx
+
 .. hxtool-doc:: hmp-commands-info.hx
 
 Integer expressions
diff --git a/hmp-commands-virtio.hx b/hmp-commands-virtio.hx
new file mode 100644
index 000000000000..14cb14bfcc70
--- /dev/null
+++ b/hmp-commands-virtio.hx
@@ -0,0 +1,160 @@
+HXCOMM Use DEFHEADING() to define headings in both help text and rST.
+HXCOMM Text between SRST and ERST is copied to the rST version and
+HXCOMM discarded from C version.
+HXCOMM DEF(command, args, callback, arg_string, help) is used to construct
+HXCOMM monitor info commands
+HXCOMM HXCOMM can be used for comments, discarded from both rST and C.
+HXCOMM
+HXCOMM In this file, generally SRST fragments should have two extra
+HXCOMM spaces of indent, so that the documentation list item for "x-debug-virtio cmd"
+HXCOMM appears inside the documentation list item for the top level
+HXCOMM "x-debug-virtio" documentation entry. The exception is the first SRST
+HXCOMM fragment that defines that top level entry.
+
+SRST
+``x-debug-virtio`` *subcommand*
+  Show various information about virtio.
+
+  Example:
+
+  List all sub-commands::
+
+    (qemu) x-debug-virtio
+    x-debug-virtio query  -- List all available virtio devices
+    x-debug-virtio status path -- Display status of a given virtio device
+    x-debug-virtio queue-status path queue -- Display status of a given virtio queue
+    x-debug-virtio queue-element path queue [index] -- Display element of a given virtio queue
+
+ERST
+
+    {
+        .name       = "query",
+        .args_type  = "",
+        .params     = "",
+        .help       = "List all available virtio devices",
+        .cmd        = hmp_x_debug_virtio_query,
+        .flags      = "p",
+    },
+
+SRST
+  ``x-debug-virtio query``
+    List all available virtio devices
+
+    Example:
+
+    List all available virtio devices in the machine::
+
+      (qemu) x-debug-virtio query
+      /machine/peripheral-anon/device[3]/virtio-backend [virtio-net]
+      /machine/peripheral-anon/device[1]/virtio-backend [virtio-serial]
+      /machine/peripheral-anon/device[0]/virtio-backend [virtio-blk]
+
+ERST
+
+    {
+        .name       = "status",
+        .args_type  = "path:s",
+        .params     = "path",
+        .help       = "Display status of a given virtio device",
+        .cmd        = hmp_x_debug_virtio_status,
+        .flags      = "p",
+    },
+
+SRST
+  ``x-debug-virtio status`` *path*
+    Display status of a given virtio device
+
+    Example:
+
+    Dump the status of the first virtio device::
+
+      (qemu) x-debug-virtio status /machine/peripheral-anon/device[3]/virtio-backend
+      /machine/peripheral-anon/device[3]/virtio-backend:
+        Device Id:        1
+        Guest features:   event-idx, indirect-desc, version-1
+                          ctrl-mac-addr, guest-announce, ctrl-vlan, ctrl-rx, ctrl-vq, status, mrg-rxbuf, host-ufo, host-ecn, host-tso6, host-tso4, guest-ufo, guest-ecn, guest-tso6, guest-tso4, mac, ctrl-guest-offloads, guest-csum, csum
+        Host features:    event-idx, indirect-desc, bad-feature, version-1, any-layout, notify-on-empty
+                          gso, ctrl-mac-addr, guest-announce, ctrl-rx-extra, ctrl-vlan, ctrl-rx, ctrl-vq, status, mrg-rxbuf, host-ufo, host-ecn, host-tso6, host-tso4, guest-ufo, guest-ecn, guest-tso6, guest-tso4, mac, ctrl-guest-offloads, guest-csum, csum
+        Backend features:
+        Endianness:       little
+        VirtQueues:       3
+
+ERST
+
+    {
+        .name       = "queue-status",
+        .args_type  = "path:s,queue:i",
+        .params     = "path queue",
+        .help       = "Display status of a given virtio queue",
+        .cmd        = hmp_x_debug_virtio_queue_status,
+        .flags      = "p",
+    },
+
+SRST
+  ``x-debug-virtio queue-status`` *path* *queue*
+    Display status of a given virtio queue
+
+    Example:
+
+    Dump the status of the first queue of the first virtio device::
+
+      (qemu) x-debug-virtio queue-status /machine/peripheral-anon/device[3]/virtio-backend 0
+      /machine/peripheral-anon/device[3]/virtio-backend:
+        index:                0
+        inuse:                0
+        last_avail_idx:       61
+        shadow_avail_idx:     292
+        signalled_used:       61
+        signalled_used_valid: 1
+        VRing:
+          num:         256
+          num_default: 256
+          align:       4096
+          desc:        0x000000006c352000
+          avail:       0x000000006c353000
+          used:        0x000000006c353240
+
+ERST
+
+    {
+        .name       = "queue-element",
+        .args_type  = "path:s,queue:i,index:i?",
+        .params     = "path queue [index]",
+        .help       = "Display element of a given virtio queue",
+        .cmd        = hmp_x_debug_virtio_queue_element,
+        .flags      = "p",
+    },
+
+SRST
+  ``x-debug-virtio queue-element`` *path* *queue* [*index*]
+    Display element of a given virtio queue
+
+    Example:
+
+    Dump the information of the head element of the first queue of
+    the first virtio device::
+
+      (qemu) x-debug-virtio queue-element/machine/peripheral-anon/device[3]/virtio-backend 0
+      index:  67
+      ndescs: 1
+      descs:  addr 0x6fe69800 len 1536 (write)
+
+      (qemu) xp/128bx 0x6fe69800
+      000000006fe69800: 0x02 0x00 0x00 0x00 0x00 0x00 0x00 0x00
+      000000006fe69808: 0x00 0x00 0x01 0x00 0x52 0x54 0x00 0x12
+      000000006fe69810: 0x34 0x56 0x52 0x54 0x00 0x09 0x51 0xde
+      000000006fe69818: 0x08 0x00 0x45 0x00 0x00 0x4c 0x8f 0x32
+
+    device[3] is a virtio-net device and we can see in the element buffer the
+    MAC address of the card::
+
+      [root@localhost ~]# ip link show ens4
+      2: ens4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP m0
+          link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
+
+    and the MAC address of the gateway::
+
+      [root@localhost ~]# arp -a
+      _gateway (192.168.122.1) at 52:54:00:09:51:de [ether] on ens4
+
+ERST
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 7f0f3974ad90..777761dc48d7 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1804,6 +1804,16 @@ SRST
   Set QOM property *property* of object at location *path* to value *value*
 ERST
 
+    {
+        .name       = "x-debug-virtio",
+        .args_type  = "name:S?",
+        .params     = "[cmd]",
+        .help       = "show various information about virtio",
+        .cmd        = hmp_x_debug_virtio_help,
+        .sub_table  = hmp_x_debug_virtio_cmds,
+        .flags      = "p",
+    },
+
     {
         .name       = "info",
         .args_type  = "item:s?",
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 66dc2cef1b39..c3d6b783417e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -30,6 +30,9 @@
 #include "sysemu/dma.h"
 #include "sysemu/runstate.h"
 #include "config-devices.h"
+#include "monitor/hmp.h"
+#include "monitor/monitor.h"
+#include "qapi/qmp/qdict.h"
 
 static QTAILQ_HEAD(, VirtIODevice) virtio_list;
 
@@ -3861,6 +3864,32 @@ VirtioInfoList *qmp_x_debug_query_virtio(Error **errp)
     return list;
 }
 
+void hmp_x_debug_virtio_query(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    VirtioInfoList *list = qmp_x_debug_query_virtio(&err);
+    VirtioInfoList *node;
+
+    if (err != NULL) {
+        hmp_handle_error(mon, err);
+        return;
+    }
+
+    if (list == NULL) {
+        monitor_printf(mon, "No VirtIO devices\n");
+        return;
+    }
+
+    node = list;
+    while (node) {
+        monitor_printf(mon, "%s [%s]\n", node->value->path,
+                       VirtioType_str(node->value->type));
+        node = node->next;
+    }
+
+    qapi_free_VirtioInfoList(list);
+}
+
 static VirtIODevice *virtio_device_find(const char *path)
 {
     VirtIODevice *vdev;
@@ -3912,8 +3941,38 @@ VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
     return status;
 }
 
+void hmp_x_debug_virtio_queue_status(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    const char *path = qdict_get_try_str(qdict, "path");
+    int queue = qdict_get_int(qdict, "queue");
+    VirtQueueStatus *s = qmp_x_debug_virtio_queue_status(path, queue, &err);
+
+    if (err != NULL) {
+        hmp_handle_error(mon, err);
+        return;
+    }
+    monitor_printf(mon, "%s:\n", path);
+    monitor_printf(mon, "  index:                %d\n", s->queue_index);
+    monitor_printf(mon, "  inuse:                %d\n", s->inuse);
+    monitor_printf(mon, "  last_avail_idx:       %d\n", s->last_avail_idx);
+    monitor_printf(mon, "  shadow_avail_idx:     %d\n", s->shadow_avail_idx);
+    monitor_printf(mon, "  signalled_used:       %d\n", s->signalled_used);
+    monitor_printf(mon, "  signalled_used_valid: %d\n",
+                   s->signalled_used_valid);
+    monitor_printf(mon, "  VRing:\n");
+    monitor_printf(mon, "    num:         %"PRId64"\n", s->vring_num);
+    monitor_printf(mon, "    num_default: %"PRId64"\n", s->vring_num_default);
+    monitor_printf(mon, "    align:       %"PRId64"\n", s->vring_align);
+    monitor_printf(mon, "    desc:        0x%016"PRIx64"\n", s->vring_desc);
+    monitor_printf(mon, "    avail:       0x%016"PRIx64"\n", s->vring_avail);
+    monitor_printf(mon, "    used:        0x%016"PRIx64"\n", s->vring_used);
+
+    qapi_free_VirtQueueStatus(s);
+}
+
 #define CONVERT_FEATURES(type, map)                \
-    ({                                           \
+    ({                                             \
         type *list = NULL;                         \
         type *node;                                \
         for (i = 0; map[i].virtio_bit != -1; i++) {\
@@ -4033,6 +4092,92 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
     return status;
 }
 
+#define DUMP_FEATURES(type, field)                                         \
+    do {                                                                   \
+        type##FeatureList *list = features->device->u.field.data;          \
+        if (list) {                                                        \
+            monitor_printf(mon, "                    ");                   \
+            while (list) {                                                 \
+                monitor_printf(mon, "%s", type##Feature_str(list->value)); \
+                list = list->next;                                         \
+                if (list != NULL) {                                        \
+                    monitor_printf(mon, ", ");                             \
+                }                                                          \
+            }                                                              \
+            monitor_printf(mon, "\n");                                     \
+        }                                                                  \
+    } while (0)
+
+static void hmp_virtio_dump_features(Monitor *mon,
+                                     VirtioStatusFeatures *features)
+{
+    VirtioTransportFeatureList *transport_list = features->transport;
+    while (transport_list) {
+        monitor_printf(mon, "%s",
+                       VirtioTransportFeature_str(transport_list->value));
+        transport_list = transport_list->next;
+        if (transport_list != NULL) {
+            monitor_printf(mon, ", ");
+        }
+    }
+    monitor_printf(mon, "\n");
+    switch (features->device->type) {
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SERIAL:
+        DUMP_FEATURES(VirtioSerial, virtio_serial);
+        break;
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BLK:
+        DUMP_FEATURES(VirtioBlk, virtio_blk);
+        break;
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_GPU:
+        DUMP_FEATURES(VirtioGpu, virtio_gpu);
+        break;
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_NET:
+        DUMP_FEATURES(VirtioNet, virtio_net);
+        break;
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SCSI:
+        DUMP_FEATURES(VirtioScsi, virtio_scsi);
+        break;
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BALLOON:
+        DUMP_FEATURES(VirtioBalloon, virtio_balloon);
+        break;
+    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_IOMMU:
+        DUMP_FEATURES(VirtioIommu, virtio_iommu);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+    if (features->unknown) {
+        monitor_printf(mon, "                    unknown(0x%016"PRIx64")\n", \
+                       features->unknown);
+    }
+}
+
+void hmp_x_debug_virtio_status(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    const char *path = qdict_get_try_str(qdict, "path");
+    VirtioStatus *s = qmp_x_debug_virtio_status(path, &err);
+
+    if (err != NULL) {
+        hmp_handle_error(mon, err);
+        return;
+    }
+
+    monitor_printf(mon, "%s:\n", path);
+    monitor_printf(mon, "  Device Id:        %"PRId64"\n", s->device_id);
+    monitor_printf(mon, "  Guest features:   ");
+    hmp_virtio_dump_features(mon, s->guest_features);
+    monitor_printf(mon, "  Host features:    ");
+    hmp_virtio_dump_features(mon, s->host_features);
+    monitor_printf(mon, "  Backend features: ");
+    hmp_virtio_dump_features(mon, s->backend_features);
+    monitor_printf(mon, "  Endianness:       %s\n",
+                   VirtioStatusEndianness_str(s->device_endian));
+    monitor_printf(mon, "  VirtQueues:       %d\n", s->num_vqs);
+
+    qapi_free_VirtioStatus(s);
+}
+
 static VirtioRingDescFlagsList *qmp_decode_vring_desc_flags(uint16_t flags)
 {
     VirtioRingDescFlagsList *list = NULL;
@@ -4163,6 +4308,52 @@ done:
     return element;
 }
 
+void hmp_x_debug_virtio_queue_element(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    const char *path = qdict_get_try_str(qdict, "path");
+    int queue = qdict_get_int(qdict, "queue");
+    int index = qdict_get_try_int(qdict, "index", -1);
+    VirtioQueueElement *element;
+    VirtioRingDescList *list;
+
+    element = qmp_x_debug_virtio_queue_element(path, queue, index != -1,
+                                               index, &err);
+    if (err != NULL) {
+        hmp_handle_error(mon, err);
+        return;
+    }
+
+    monitor_printf(mon, "index:  %d\n", element->index);
+    monitor_printf(mon, "ndescs: %d\n", element->ndescs);
+    monitor_printf(mon, "descs:  ");
+
+    list = element->descs;
+    while (list) {
+        monitor_printf(mon, "addr 0x%"PRIx64" len %d", list->value->addr,
+                       list->value->len);
+        if (list->value->flags) {
+            VirtioRingDescFlagsList *flag = list->value->flags;
+            monitor_printf(mon, " (");
+            while (flag) {
+                monitor_printf(mon, "%s", VirtioRingDescFlags_str(flag->value));
+                flag = flag->next;
+                if (flag) {
+                    monitor_printf(mon, ", ");
+                }
+            }
+            monitor_printf(mon, ")");
+        }
+        list = list->next;
+        if (list) {
+            monitor_printf(mon, ", ");
+        }
+    }
+    monitor_printf(mon, "\n");
+
+    qapi_free_VirtioQueueElement(element);
+}
+
 static const TypeInfo virtio_device_info = {
     .name = TYPE_VIRTIO_DEVICE,
     .parent = TYPE_DEVICE,
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index e33ca5a911a5..f07509985254 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -98,6 +98,10 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict);
 void hmp_qom_list(Monitor *mon, const QDict *qdict);
 void hmp_qom_set(Monitor *mon, const QDict *qdict);
 void hmp_info_qom_tree(Monitor *mon, const QDict *dict);
+void hmp_x_debug_virtio_query(Monitor *mon, const QDict *qdict);
+void hmp_x_debug_virtio_status(Monitor *mon, const QDict *qdict);
+void hmp_x_debug_virtio_queue_status(Monitor *mon, const QDict *qdict);
+void hmp_x_debug_virtio_queue_element(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/monitor/misc.c b/monitor/misc.c
index 9723b466cda1..1a179829250d 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "config-devices.h"
 #include "monitor-internal.h"
 #include "cpu.h"
 #include "monitor/qdev.h"
@@ -232,6 +233,15 @@ static void hmp_info_help(Monitor *mon, const QDict *qdict)
     help_cmd(mon, "info");
 }
 
+static void hmp_x_debug_virtio_help(Monitor *mon, const QDict *qdict)
+{
+#if defined(CONFIG_VIRTIO)
+    help_cmd(mon, "x-debug-virtio");
+#else
+    monitor_printf(mon, "Virtio is disabled\n");
+#endif
+}
+
 static void monitor_init_qmp_commands(void)
 {
     /*
@@ -1681,6 +1691,13 @@ static HMPCommand hmp_info_cmds[] = {
     { NULL, NULL, },
 };
 
+static HMPCommand hmp_x_debug_virtio_cmds[] = {
+#if defined(CONFIG_VIRTIO)
+#include "hmp-commands-virtio.h"
+#endif
+    { NULL, NULL, },
+};
+
 /* hmp_cmds and hmp_info_cmds would be sorted at runtime */
 HMPCommand hmp_cmds[] = {
 #include "hmp-commands.h"
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC v4 0/6] hmp, qmp: Add some commands to introspect virtio devices
  2020-05-07 13:47 [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
                   ` (5 preceding siblings ...)
  2020-05-07 13:48 ` [RFC v4 6/6] hmp: add x-debug-virtio commands Laurent Vivier
@ 2020-05-07 20:11 ` no-reply
  6 siblings, 0 replies; 12+ messages in thread
From: no-reply @ 2020-05-07 20:11 UTC (permalink / raw)
  To: lvivier
  Cc: lvivier, kwolf, thuth, qemu-block, amit, armbru, jasowang, mst,
	david, qemu-devel, mdroth, eric.auger, kraxel, stefanha,
	pbonzini, marcandre.lureau, fam, mreitz, dgilbert

Patchew URL: https://patchew.org/QEMU/20200507134800.10837-1-lvivier@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200507134800.10837-1-lvivier@redhat.com
Subject: [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
   3c7adbc..1b8c458  master     -> master
Switched to a new branch 'test'
e9975e2 hmp: add x-debug-virtio commands
5092215 qmp: add QMP command x-debug-virtio-queue-element
7572b3f qmp: add QMP command x-debug-virtio-queue-status
d7e8edc qmp: decode feature bits in virtio-status
ee6f64a qmp: add QMP command x-debug-virtio-status
09ea838 qmp: add QMP command x-debug-query-virtio

=== OUTPUT BEGIN ===
1/6 Checking commit 09ea838d52de (qmp: add QMP command x-debug-query-virtio)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#27: 
new file mode 100644

total: 0 errors, 1 warnings, 180 lines checked

Patch 1/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/6 Checking commit ee6f64a61066 (qmp: add QMP command x-debug-virtio-status)
3/6 Checking commit d7e8edc95598 (qmp: decode feature bits in virtio-status)
4/6 Checking commit 7572b3f9ba58 (qmp: add QMP command x-debug-virtio-queue-status)
5/6 Checking commit 50922154ae8c (qmp: add QMP command x-debug-virtio-queue-element)
6/6 Checking commit e9975e214541 (hmp: add x-debug-virtio commands)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#67: 
new file mode 100644

ERROR: spaces required around that '*' (ctx:WxV)
#346: FILE: hw/virtio/virtio.c:4097:
+        type##FeatureList *list = features->device->u.field.data;          \
                           ^

total: 1 errors, 1 warnings, 483 lines checked

Patch 6/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200507134800.10837-1-lvivier@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 6/6] hmp: add x-debug-virtio commands
  2020-05-07 13:48 ` [RFC v4 6/6] hmp: add x-debug-virtio commands Laurent Vivier
@ 2020-05-13 10:51   ` Dr. David Alan Gilbert
  2020-05-15 15:20     ` Laurent Vivier
  0 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2020-05-13 10:51 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	qemu-devel, Markus Armbruster, Eric Auger, Gerd Hoffmann,
	Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
	Max Reitz

* Laurent Vivier (lvivier@redhat.com) wrote:
> This patch implements HMP version of the virtio QMP commands
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

With a thought below....

> ---
>  Makefile                |   2 +-
>  Makefile.target         |   7 +-
>  docs/system/monitor.rst |   2 +
>  hmp-commands-virtio.hx  | 160 +++++++++++++++++++++++++++++++++
>  hmp-commands.hx         |  10 +++
>  hw/virtio/virtio.c      | 193 +++++++++++++++++++++++++++++++++++++++-
>  include/monitor/hmp.h   |   4 +
>  monitor/misc.c          |  17 ++++
>  8 files changed, 391 insertions(+), 4 deletions(-)
>  create mode 100644 hmp-commands-virtio.hx
> 
> diff --git a/Makefile b/Makefile
> index 34275f57c9cb..feb300ebb2d4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1099,7 +1099,7 @@ $(MANUAL_BUILDDIR)/interop/index.html: $(call manual-deps,interop)
>  $(MANUAL_BUILDDIR)/specs/index.html: $(call manual-deps,specs)
>  	$(call build-manual,specs,html)
>  
> -$(MANUAL_BUILDDIR)/system/index.html: $(call manual-deps,system) $(SRC_PATH)/hmp-commands.hx $(SRC_PATH)/hmp-commands-info.hx $(SRC_PATH)/qemu-options.hx
> +$(MANUAL_BUILDDIR)/system/index.html: $(call manual-deps,system) $(SRC_PATH)/hmp-commands.hx $(SRC_PATH)/hmp-commands-info.hx $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/hmp-commands-virtio.hx
>  	$(call build-manual,system,html)
>  
>  $(MANUAL_BUILDDIR)/tools/index.html: $(call manual-deps,tools) $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/docs/qemu-option-trace.rst.inc
> diff --git a/Makefile.target b/Makefile.target
> index 8ed1eba95b9c..66d3ff9bc350 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -171,7 +171,7 @@ else
>  obj-y += hw/$(TARGET_BASE_ARCH)/
>  endif
>  
> -generated-files-y += hmp-commands.h hmp-commands-info.h
> +generated-files-y += hmp-commands.h hmp-commands-info.h hmp-commands-virtio.h
>  generated-files-y += config-devices.h
>  
>  endif # CONFIG_SOFTMMU
> @@ -220,10 +220,13 @@ hmp-commands.h: $(SRC_PATH)/hmp-commands.hx $(SRC_PATH)/scripts/hxtool
>  hmp-commands-info.h: $(SRC_PATH)/hmp-commands-info.hx $(SRC_PATH)/scripts/hxtool
>  	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"GEN","$(TARGET_DIR)$@")
>  
> +hmp-commands-virtio.h: $(SRC_PATH)/hmp-commands-virtio.hx $(SRC_PATH)/scripts/hxtool
> +	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"GEN","$(TARGET_DIR)$@")
> +
>  clean: clean-target
>  	rm -f *.a *~ $(PROGS)
>  	rm -f $(shell find . -name '*.[od]')
> -	rm -f hmp-commands.h gdbstub-xml.c
> +	rm -f hmp-commands.h hmp-commands-virtio.h gdbstub-xml.c
>  	rm -f trace/generated-helpers.c trace/generated-helpers.c-timestamp
>  ifdef CONFIG_TRACE_SYSTEMTAP
>  	rm -f *.stp
> diff --git a/docs/system/monitor.rst b/docs/system/monitor.rst
> index 0bcd5da21644..985c3f51ffe7 100644
> --- a/docs/system/monitor.rst
> +++ b/docs/system/monitor.rst
> @@ -21,6 +21,8 @@ The following commands are available:
>  
>  .. hxtool-doc:: hmp-commands.hx
>  
> +.. hxtool-doc:: hmp-commands-virtio.hx
> +
>  .. hxtool-doc:: hmp-commands-info.hx
>  
>  Integer expressions
> diff --git a/hmp-commands-virtio.hx b/hmp-commands-virtio.hx
> new file mode 100644
> index 000000000000..14cb14bfcc70
> --- /dev/null
> +++ b/hmp-commands-virtio.hx
> @@ -0,0 +1,160 @@
> +HXCOMM Use DEFHEADING() to define headings in both help text and rST.
> +HXCOMM Text between SRST and ERST is copied to the rST version and
> +HXCOMM discarded from C version.
> +HXCOMM DEF(command, args, callback, arg_string, help) is used to construct
> +HXCOMM monitor info commands
> +HXCOMM HXCOMM can be used for comments, discarded from both rST and C.
> +HXCOMM
> +HXCOMM In this file, generally SRST fragments should have two extra
> +HXCOMM spaces of indent, so that the documentation list item for "x-debug-virtio cmd"
> +HXCOMM appears inside the documentation list item for the top level
> +HXCOMM "x-debug-virtio" documentation entry. The exception is the first SRST
> +HXCOMM fragment that defines that top level entry.
> +
> +SRST
> +``x-debug-virtio`` *subcommand*
> +  Show various information about virtio.
> +
> +  Example:
> +
> +  List all sub-commands::
> +
> +    (qemu) x-debug-virtio
> +    x-debug-virtio query  -- List all available virtio devices
> +    x-debug-virtio status path -- Display status of a given virtio device
> +    x-debug-virtio queue-status path queue -- Display status of a given virtio queue
> +    x-debug-virtio queue-element path queue [index] -- Display element of a given virtio queue
> +
> +ERST
> +
> +    {
> +        .name       = "query",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "List all available virtio devices",
> +        .cmd        = hmp_x_debug_virtio_query,
> +        .flags      = "p",
> +    },
> +
> +SRST
> +  ``x-debug-virtio query``
> +    List all available virtio devices
> +
> +    Example:
> +
> +    List all available virtio devices in the machine::
> +
> +      (qemu) x-debug-virtio query
> +      /machine/peripheral-anon/device[3]/virtio-backend [virtio-net]
> +      /machine/peripheral-anon/device[1]/virtio-backend [virtio-serial]
> +      /machine/peripheral-anon/device[0]/virtio-backend [virtio-blk]
> +
> +ERST
> +
> +    {
> +        .name       = "status",
> +        .args_type  = "path:s",
> +        .params     = "path",
> +        .help       = "Display status of a given virtio device",
> +        .cmd        = hmp_x_debug_virtio_status,
> +        .flags      = "p",
> +    },
> +
> +SRST
> +  ``x-debug-virtio status`` *path*
> +    Display status of a given virtio device
> +
> +    Example:
> +
> +    Dump the status of the first virtio device::
> +
> +      (qemu) x-debug-virtio status /machine/peripheral-anon/device[3]/virtio-backend
> +      /machine/peripheral-anon/device[3]/virtio-backend:
> +        Device Id:        1
> +        Guest features:   event-idx, indirect-desc, version-1
> +                          ctrl-mac-addr, guest-announce, ctrl-vlan, ctrl-rx, ctrl-vq, status, mrg-rxbuf, host-ufo, host-ecn, host-tso6, host-tso4, guest-ufo, guest-ecn, guest-tso6, guest-tso4, mac, ctrl-guest-offloads, guest-csum, csum
> +        Host features:    event-idx, indirect-desc, bad-feature, version-1, any-layout, notify-on-empty
> +                          gso, ctrl-mac-addr, guest-announce, ctrl-rx-extra, ctrl-vlan, ctrl-rx, ctrl-vq, status, mrg-rxbuf, host-ufo, host-ecn, host-tso6, host-tso4, guest-ufo, guest-ecn, guest-tso6, guest-tso4, mac, ctrl-guest-offloads, guest-csum, csum
> +        Backend features:
> +        Endianness:       little
> +        VirtQueues:       3
> +
> +ERST
> +
> +    {
> +        .name       = "queue-status",
> +        .args_type  = "path:s,queue:i",
> +        .params     = "path queue",
> +        .help       = "Display status of a given virtio queue",
> +        .cmd        = hmp_x_debug_virtio_queue_status,
> +        .flags      = "p",
> +    },
> +
> +SRST
> +  ``x-debug-virtio queue-status`` *path* *queue*
> +    Display status of a given virtio queue
> +
> +    Example:
> +
> +    Dump the status of the first queue of the first virtio device::
> +
> +      (qemu) x-debug-virtio queue-status /machine/peripheral-anon/device[3]/virtio-backend 0
> +      /machine/peripheral-anon/device[3]/virtio-backend:
> +        index:                0
> +        inuse:                0
> +        last_avail_idx:       61
> +        shadow_avail_idx:     292
> +        signalled_used:       61
> +        signalled_used_valid: 1
> +        VRing:
> +          num:         256
> +          num_default: 256
> +          align:       4096
> +          desc:        0x000000006c352000
> +          avail:       0x000000006c353000
> +          used:        0x000000006c353240
> +
> +ERST
> +
> +    {
> +        .name       = "queue-element",
> +        .args_type  = "path:s,queue:i,index:i?",
> +        .params     = "path queue [index]",
> +        .help       = "Display element of a given virtio queue",
> +        .cmd        = hmp_x_debug_virtio_queue_element,
> +        .flags      = "p",
> +    },
> +
> +SRST
> +  ``x-debug-virtio queue-element`` *path* *queue* [*index*]
> +    Display element of a given virtio queue
> +
> +    Example:
> +
> +    Dump the information of the head element of the first queue of
> +    the first virtio device::
> +
> +      (qemu) x-debug-virtio queue-element/machine/peripheral-anon/device[3]/virtio-backend 0
> +      index:  67
> +      ndescs: 1
> +      descs:  addr 0x6fe69800 len 1536 (write)
> +
> +      (qemu) xp/128bx 0x6fe69800
> +      000000006fe69800: 0x02 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> +      000000006fe69808: 0x00 0x00 0x01 0x00 0x52 0x54 0x00 0x12
> +      000000006fe69810: 0x34 0x56 0x52 0x54 0x00 0x09 0x51 0xde
> +      000000006fe69818: 0x08 0x00 0x45 0x00 0x00 0x4c 0x8f 0x32
> +
> +    device[3] is a virtio-net device and we can see in the element buffer the
> +    MAC address of the card::
> +
> +      [root@localhost ~]# ip link show ens4
> +      2: ens4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP m0
> +          link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
> +
> +    and the MAC address of the gateway::
> +
> +      [root@localhost ~]# arp -a
> +      _gateway (192.168.122.1) at 52:54:00:09:51:de [ether] on ens4
> +
> +ERST
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 7f0f3974ad90..777761dc48d7 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1804,6 +1804,16 @@ SRST
>    Set QOM property *property* of object at location *path* to value *value*
>  ERST
>  
> +    {
> +        .name       = "x-debug-virtio",
> +        .args_type  = "name:S?",
> +        .params     = "[cmd]",
> +        .help       = "show various information about virtio",
> +        .cmd        = hmp_x_debug_virtio_help,
> +        .sub_table  = hmp_x_debug_virtio_cmds,
> +        .flags      = "p",
> +    },
> +
>      {
>          .name       = "info",
>          .args_type  = "item:s?",
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 66dc2cef1b39..c3d6b783417e 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -30,6 +30,9 @@
>  #include "sysemu/dma.h"
>  #include "sysemu/runstate.h"
>  #include "config-devices.h"
> +#include "monitor/hmp.h"
> +#include "monitor/monitor.h"
> +#include "qapi/qmp/qdict.h"
>  
>  static QTAILQ_HEAD(, VirtIODevice) virtio_list;
>  
> @@ -3861,6 +3864,32 @@ VirtioInfoList *qmp_x_debug_query_virtio(Error **errp)
>      return list;
>  }
>  
> +void hmp_x_debug_virtio_query(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    VirtioInfoList *list = qmp_x_debug_query_virtio(&err);
> +    VirtioInfoList *node;
> +
> +    if (err != NULL) {
> +        hmp_handle_error(mon, err);
> +        return;
> +    }
> +
> +    if (list == NULL) {
> +        monitor_printf(mon, "No VirtIO devices\n");
> +        return;
> +    }
> +
> +    node = list;
> +    while (node) {
> +        monitor_printf(mon, "%s [%s]\n", node->value->path,
> +                       VirtioType_str(node->value->type));
> +        node = node->next;
> +    }
> +
> +    qapi_free_VirtioInfoList(list);
> +}
> +
>  static VirtIODevice *virtio_device_find(const char *path)
>  {
>      VirtIODevice *vdev;
> @@ -3912,8 +3941,38 @@ VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>      return status;
>  }
>  
> +void hmp_x_debug_virtio_queue_status(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    const char *path = qdict_get_try_str(qdict, "path");
> +    int queue = qdict_get_int(qdict, "queue");
> +    VirtQueueStatus *s = qmp_x_debug_virtio_queue_status(path, queue, &err);
> +
> +    if (err != NULL) {
> +        hmp_handle_error(mon, err);
> +        return;
> +    }
> +    monitor_printf(mon, "%s:\n", path);
> +    monitor_printf(mon, "  index:                %d\n", s->queue_index);
> +    monitor_printf(mon, "  inuse:                %d\n", s->inuse);
> +    monitor_printf(mon, "  last_avail_idx:       %d\n", s->last_avail_idx);
> +    monitor_printf(mon, "  shadow_avail_idx:     %d\n", s->shadow_avail_idx);
> +    monitor_printf(mon, "  signalled_used:       %d\n", s->signalled_used);
> +    monitor_printf(mon, "  signalled_used_valid: %d\n",
> +                   s->signalled_used_valid);
> +    monitor_printf(mon, "  VRing:\n");
> +    monitor_printf(mon, "    num:         %"PRId64"\n", s->vring_num);
> +    monitor_printf(mon, "    num_default: %"PRId64"\n", s->vring_num_default);
> +    monitor_printf(mon, "    align:       %"PRId64"\n", s->vring_align);
> +    monitor_printf(mon, "    desc:        0x%016"PRIx64"\n", s->vring_desc);
> +    monitor_printf(mon, "    avail:       0x%016"PRIx64"\n", s->vring_avail);
> +    monitor_printf(mon, "    used:        0x%016"PRIx64"\n", s->vring_used);
> +
> +    qapi_free_VirtQueueStatus(s);
> +}
> +
>  #define CONVERT_FEATURES(type, map)                \
> -    ({                                           \
> +    ({                                             \
>          type *list = NULL;                         \
>          type *node;                                \
>          for (i = 0; map[i].virtio_bit != -1; i++) {\
> @@ -4033,6 +4092,92 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
>      return status;
>  }
>  
> +#define DUMP_FEATURES(type, field)                                         \
> +    do {                                                                   \
> +        type##FeatureList *list = features->device->u.field.data;          \
> +        if (list) {                                                        \
> +            monitor_printf(mon, "                    ");                   \
> +            while (list) {                                                 \
> +                monitor_printf(mon, "%s", type##Feature_str(list->value)); \
> +                list = list->next;                                         \
> +                if (list != NULL) {                                        \
> +                    monitor_printf(mon, ", ");                             \
> +                }                                                          \
> +            }                                                              \
> +            monitor_printf(mon, "\n");                                     \
> +        }                                                                  \
> +    } while (0)

It feels like you should be able to have an array of Feature_str's
indexed by VIRTIO_DEVICE_FEATURE_KIND_ enum, so that when a new
VIRTIO_DEVICE_FEATURE_KIND is added you don't need to fix this up.

Dave

> +
> +static void hmp_virtio_dump_features(Monitor *mon,
> +                                     VirtioStatusFeatures *features)
> +{
> +    VirtioTransportFeatureList *transport_list = features->transport;
> +    while (transport_list) {
> +        monitor_printf(mon, "%s",
> +                       VirtioTransportFeature_str(transport_list->value));
> +        transport_list = transport_list->next;
> +        if (transport_list != NULL) {
> +            monitor_printf(mon, ", ");
> +        }
> +    }
> +    monitor_printf(mon, "\n");
> +    switch (features->device->type) {
> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SERIAL:
> +        DUMP_FEATURES(VirtioSerial, virtio_serial);
> +        break;
> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BLK:
> +        DUMP_FEATURES(VirtioBlk, virtio_blk);
> +        break;
> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_GPU:
> +        DUMP_FEATURES(VirtioGpu, virtio_gpu);
> +        break;
> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_NET:
> +        DUMP_FEATURES(VirtioNet, virtio_net);
> +        break;
> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SCSI:
> +        DUMP_FEATURES(VirtioScsi, virtio_scsi);
> +        break;
> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BALLOON:
> +        DUMP_FEATURES(VirtioBalloon, virtio_balloon);
> +        break;
> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_IOMMU:
> +        DUMP_FEATURES(VirtioIommu, virtio_iommu);
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +    if (features->unknown) {
> +        monitor_printf(mon, "                    unknown(0x%016"PRIx64")\n", \
> +                       features->unknown);
> +    }
> +}
> +
> +void hmp_x_debug_virtio_status(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    const char *path = qdict_get_try_str(qdict, "path");
> +    VirtioStatus *s = qmp_x_debug_virtio_status(path, &err);
> +
> +    if (err != NULL) {
> +        hmp_handle_error(mon, err);
> +        return;
> +    }
> +
> +    monitor_printf(mon, "%s:\n", path);
> +    monitor_printf(mon, "  Device Id:        %"PRId64"\n", s->device_id);
> +    monitor_printf(mon, "  Guest features:   ");
> +    hmp_virtio_dump_features(mon, s->guest_features);
> +    monitor_printf(mon, "  Host features:    ");
> +    hmp_virtio_dump_features(mon, s->host_features);
> +    monitor_printf(mon, "  Backend features: ");
> +    hmp_virtio_dump_features(mon, s->backend_features);
> +    monitor_printf(mon, "  Endianness:       %s\n",
> +                   VirtioStatusEndianness_str(s->device_endian));
> +    monitor_printf(mon, "  VirtQueues:       %d\n", s->num_vqs);
> +
> +    qapi_free_VirtioStatus(s);
> +}
> +
>  static VirtioRingDescFlagsList *qmp_decode_vring_desc_flags(uint16_t flags)
>  {
>      VirtioRingDescFlagsList *list = NULL;
> @@ -4163,6 +4308,52 @@ done:
>      return element;
>  }
>  
> +void hmp_x_debug_virtio_queue_element(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    const char *path = qdict_get_try_str(qdict, "path");
> +    int queue = qdict_get_int(qdict, "queue");
> +    int index = qdict_get_try_int(qdict, "index", -1);
> +    VirtioQueueElement *element;
> +    VirtioRingDescList *list;
> +
> +    element = qmp_x_debug_virtio_queue_element(path, queue, index != -1,
> +                                               index, &err);
> +    if (err != NULL) {
> +        hmp_handle_error(mon, err);
> +        return;
> +    }
> +
> +    monitor_printf(mon, "index:  %d\n", element->index);
> +    monitor_printf(mon, "ndescs: %d\n", element->ndescs);
> +    monitor_printf(mon, "descs:  ");
> +
> +    list = element->descs;
> +    while (list) {
> +        monitor_printf(mon, "addr 0x%"PRIx64" len %d", list->value->addr,
> +                       list->value->len);
> +        if (list->value->flags) {
> +            VirtioRingDescFlagsList *flag = list->value->flags;
> +            monitor_printf(mon, " (");
> +            while (flag) {
> +                monitor_printf(mon, "%s", VirtioRingDescFlags_str(flag->value));
> +                flag = flag->next;
> +                if (flag) {
> +                    monitor_printf(mon, ", ");
> +                }
> +            }
> +            monitor_printf(mon, ")");
> +        }
> +        list = list->next;
> +        if (list) {
> +            monitor_printf(mon, ", ");
> +        }
> +    }
> +    monitor_printf(mon, "\n");
> +
> +    qapi_free_VirtioQueueElement(element);
> +}
> +
>  static const TypeInfo virtio_device_info = {
>      .name = TYPE_VIRTIO_DEVICE,
>      .parent = TYPE_DEVICE,
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index e33ca5a911a5..f07509985254 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -98,6 +98,10 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict);
>  void hmp_qom_list(Monitor *mon, const QDict *qdict);
>  void hmp_qom_set(Monitor *mon, const QDict *qdict);
>  void hmp_info_qom_tree(Monitor *mon, const QDict *dict);
> +void hmp_x_debug_virtio_query(Monitor *mon, const QDict *qdict);
> +void hmp_x_debug_virtio_status(Monitor *mon, const QDict *qdict);
> +void hmp_x_debug_virtio_queue_status(Monitor *mon, const QDict *qdict);
> +void hmp_x_debug_virtio_queue_element(Monitor *mon, const QDict *qdict);
>  void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
>  void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
>  void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 9723b466cda1..1a179829250d 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "config-devices.h"
>  #include "monitor-internal.h"
>  #include "cpu.h"
>  #include "monitor/qdev.h"
> @@ -232,6 +233,15 @@ static void hmp_info_help(Monitor *mon, const QDict *qdict)
>      help_cmd(mon, "info");
>  }
>  
> +static void hmp_x_debug_virtio_help(Monitor *mon, const QDict *qdict)
> +{
> +#if defined(CONFIG_VIRTIO)
> +    help_cmd(mon, "x-debug-virtio");
> +#else
> +    monitor_printf(mon, "Virtio is disabled\n");
> +#endif
> +}
> +
>  static void monitor_init_qmp_commands(void)
>  {
>      /*
> @@ -1681,6 +1691,13 @@ static HMPCommand hmp_info_cmds[] = {
>      { NULL, NULL, },
>  };
>  
> +static HMPCommand hmp_x_debug_virtio_cmds[] = {
> +#if defined(CONFIG_VIRTIO)
> +#include "hmp-commands-virtio.h"
> +#endif
> +    { NULL, NULL, },
> +};
> +
>  /* hmp_cmds and hmp_info_cmds would be sorted at runtime */
>  HMPCommand hmp_cmds[] = {
>  #include "hmp-commands.h"
> -- 
> 2.26.2
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 6/6] hmp: add x-debug-virtio commands
  2020-05-13 10:51   ` Dr. David Alan Gilbert
@ 2020-05-15 15:20     ` Laurent Vivier
  2020-05-15 15:45       ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Vivier @ 2020-05-15 15:20 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	qemu-devel, Markus Armbruster, Eric Auger, Gerd Hoffmann,
	Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
	Max Reitz

On 13/05/2020 12:51, Dr. David Alan Gilbert wrote:
> * Laurent Vivier (lvivier@redhat.com) wrote:
>> This patch implements HMP version of the virtio QMP commands
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> With a thought below....
> 
>> ---
>>  Makefile                |   2 +-
>>  Makefile.target         |   7 +-
>>  docs/system/monitor.rst |   2 +
>>  hmp-commands-virtio.hx  | 160 +++++++++++++++++++++++++++++++++
>>  hmp-commands.hx         |  10 +++
>>  hw/virtio/virtio.c      | 193 +++++++++++++++++++++++++++++++++++++++-
>>  include/monitor/hmp.h   |   4 +
>>  monitor/misc.c          |  17 ++++
>>  8 files changed, 391 insertions(+), 4 deletions(-)
>>  create mode 100644 hmp-commands-virtio.hx
>>
...
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 66dc2cef1b39..c3d6b783417e 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
...
>> @@ -4033,6 +4092,92 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
>>      return status;
>>  }
>>  
>> +#define DUMP_FEATURES(type, field)                                         \
>> +    do {                                                                   \
>> +        type##FeatureList *list = features->device->u.field.data;          \
>> +        if (list) {                                                        \
>> +            monitor_printf(mon, "                    ");                   \
>> +            while (list) {                                                 \
>> +                monitor_printf(mon, "%s", type##Feature_str(list->value)); \
>> +                list = list->next;                                         \
>> +                if (list != NULL) {                                        \
>> +                    monitor_printf(mon, ", ");                             \
>> +                }                                                          \
>> +            }                                                              \
>> +            monitor_printf(mon, "\n");                                     \
>> +        }                                                                  \
>> +    } while (0)
> 
> It feels like you should be able to have an array of Feature_str's
> indexed by VIRTIO_DEVICE_FEATURE_KIND_ enum, so that when a new
> VIRTIO_DEVICE_FEATURE_KIND is added you don't need to fix this up.

I don't understand what you mean here.

>> +
>> +static void hmp_virtio_dump_features(Monitor *mon,
>> +                                     VirtioStatusFeatures *features)
>> +{
>> +    VirtioTransportFeatureList *transport_list = features->transport;
>> +    while (transport_list) {
>> +        monitor_printf(mon, "%s",
>> +                       VirtioTransportFeature_str(transport_list->value));
>> +        transport_list = transport_list->next;
>> +        if (transport_list != NULL) {
>> +            monitor_printf(mon, ", ");
>> +        }
>> +    }
>> +    monitor_printf(mon, "\n");
>> +    switch (features->device->type) {
>> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SERIAL:
>> +        DUMP_FEATURES(VirtioSerial, virtio_serial);
>> +        break;
>> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BLK:
>> +        DUMP_FEATURES(VirtioBlk, virtio_blk);
>> +        break;
>> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_GPU:
>> +        DUMP_FEATURES(VirtioGpu, virtio_gpu);
>> +        break;
>> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_NET:
>> +        DUMP_FEATURES(VirtioNet, virtio_net);
>> +        break;
>> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SCSI:
>> +        DUMP_FEATURES(VirtioScsi, virtio_scsi);
>> +        break;
>> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BALLOON:
>> +        DUMP_FEATURES(VirtioBalloon, virtio_balloon);
>> +        break;
>> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_IOMMU:
>> +        DUMP_FEATURES(VirtioIommu, virtio_iommu);
>> +        break;
>> +    default:
>> +        g_assert_not_reached();
>> +    }
>> +    if (features->unknown) {
>> +        monitor_printf(mon, "                    unknown(0x%016"PRIx64")\n", \
>> +                       features->unknown);
>> +    }
>> +}
...

Thanks,
Laurent



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 6/6] hmp: add x-debug-virtio commands
  2020-05-15 15:20     ` Laurent Vivier
@ 2020-05-15 15:45       ` Dr. David Alan Gilbert
  2020-05-15 15:57         ` Laurent Vivier
  0 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2020-05-15 15:45 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	qemu-devel, Markus Armbruster, Eric Auger, Gerd Hoffmann,
	Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
	Max Reitz

* Laurent Vivier (lvivier@redhat.com) wrote:
> On 13/05/2020 12:51, Dr. David Alan Gilbert wrote:
> > * Laurent Vivier (lvivier@redhat.com) wrote:
> >> This patch implements HMP version of the virtio QMP commands
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > 
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > 
> > With a thought below....
> > 
> >> ---
> >>  Makefile                |   2 +-
> >>  Makefile.target         |   7 +-
> >>  docs/system/monitor.rst |   2 +
> >>  hmp-commands-virtio.hx  | 160 +++++++++++++++++++++++++++++++++
> >>  hmp-commands.hx         |  10 +++
> >>  hw/virtio/virtio.c      | 193 +++++++++++++++++++++++++++++++++++++++-
> >>  include/monitor/hmp.h   |   4 +
> >>  monitor/misc.c          |  17 ++++
> >>  8 files changed, 391 insertions(+), 4 deletions(-)
> >>  create mode 100644 hmp-commands-virtio.hx
> >>
> ...
> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >> index 66dc2cef1b39..c3d6b783417e 100644
> >> --- a/hw/virtio/virtio.c
> >> +++ b/hw/virtio/virtio.c
> ...
> >> @@ -4033,6 +4092,92 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
> >>      return status;
> >>  }
> >>  
> >> +#define DUMP_FEATURES(type, field)                                         \
> >> +    do {                                                                   \
> >> +        type##FeatureList *list = features->device->u.field.data;          \
> >> +        if (list) {                                                        \
> >> +            monitor_printf(mon, "                    ");                   \
> >> +            while (list) {                                                 \
> >> +                monitor_printf(mon, "%s", type##Feature_str(list->value)); \
> >> +                list = list->next;                                         \
> >> +                if (list != NULL) {                                        \
> >> +                    monitor_printf(mon, ", ");                             \
> >> +                }                                                          \
> >> +            }                                                              \
> >> +            monitor_printf(mon, "\n");                                     \
> >> +        }                                                                  \
> >> +    } while (0)
> > 
> > It feels like you should be able to have an array of Feature_str's
> > indexed by VIRTIO_DEVICE_FEATURE_KIND_ enum, so that when a new
> > VIRTIO_DEVICE_FEATURE_KIND is added you don't need to fix this up.
> 
> I don't understand what you mean here.

Instead of the switch below, I'm thinking you could have something like:

    if (features->device->type < something_MAX) {
        features_str = anarray[features->device->type];

        ....
        monitor_printf(mon, "%s", features_str(list->value));
        ....
    }

with 'anarray' somewhere more central, so we don't have to keep
these switch structures and macros spread around.

Dave

> >> +
> >> +static void hmp_virtio_dump_features(Monitor *mon,
> >> +                                     VirtioStatusFeatures *features)
> >> +{
> >> +    VirtioTransportFeatureList *transport_list = features->transport;
> >> +    while (transport_list) {
> >> +        monitor_printf(mon, "%s",
> >> +                       VirtioTransportFeature_str(transport_list->value));
> >> +        transport_list = transport_list->next;
> >> +        if (transport_list != NULL) {
> >> +            monitor_printf(mon, ", ");
> >> +        }
> >> +    }
> >> +    monitor_printf(mon, "\n");
> >> +    switch (features->device->type) {
> >> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SERIAL:
> >> +        DUMP_FEATURES(VirtioSerial, virtio_serial);
> >> +        break;
> >> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BLK:
> >> +        DUMP_FEATURES(VirtioBlk, virtio_blk);
> >> +        break;
> >> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_GPU:
> >> +        DUMP_FEATURES(VirtioGpu, virtio_gpu);
> >> +        break;
> >> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_NET:
> >> +        DUMP_FEATURES(VirtioNet, virtio_net);
> >> +        break;
> >> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_SCSI:
> >> +        DUMP_FEATURES(VirtioScsi, virtio_scsi);
> >> +        break;
> >> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_BALLOON:
> >> +        DUMP_FEATURES(VirtioBalloon, virtio_balloon);
> >> +        break;
> >> +    case VIRTIO_DEVICE_FEATURES_KIND_VIRTIO_IOMMU:
> >> +        DUMP_FEATURES(VirtioIommu, virtio_iommu);
> >> +        break;
> >> +    default:
> >> +        g_assert_not_reached();
> >> +    }
> >> +    if (features->unknown) {
> >> +        monitor_printf(mon, "                    unknown(0x%016"PRIx64")\n", \
> >> +                       features->unknown);
> >> +    }
> >> +}
> ...
> 
> Thanks,
> Laurent
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 6/6] hmp: add x-debug-virtio commands
  2020-05-15 15:45       ` Dr. David Alan Gilbert
@ 2020-05-15 15:57         ` Laurent Vivier
  0 siblings, 0 replies; 12+ messages in thread
From: Laurent Vivier @ 2020-05-15 15:57 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: Kevin Wolf, Fam Zheng, Thomas Huth, Michael Roth, qemu-block,
	Amit Shah, Jason Wang, Michael S. Tsirkin, David Hildenbrand,
	qemu-devel, Markus Armbruster, Eric Auger, Gerd Hoffmann,
	Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
	Max Reitz

On 15/05/2020 17:45, Dr. David Alan Gilbert wrote:
> * Laurent Vivier (lvivier@redhat.com) wrote:
>> On 13/05/2020 12:51, Dr. David Alan Gilbert wrote:
>>> * Laurent Vivier (lvivier@redhat.com) wrote:
>>>> This patch implements HMP version of the virtio QMP commands
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>
>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>
>>> With a thought below....
>>>
>>>> ---
>>>>  Makefile                |   2 +-
>>>>  Makefile.target         |   7 +-
>>>>  docs/system/monitor.rst |   2 +
>>>>  hmp-commands-virtio.hx  | 160 +++++++++++++++++++++++++++++++++
>>>>  hmp-commands.hx         |  10 +++
>>>>  hw/virtio/virtio.c      | 193 +++++++++++++++++++++++++++++++++++++++-
>>>>  include/monitor/hmp.h   |   4 +
>>>>  monitor/misc.c          |  17 ++++
>>>>  8 files changed, 391 insertions(+), 4 deletions(-)
>>>>  create mode 100644 hmp-commands-virtio.hx
>>>>
>> ...
>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>>> index 66dc2cef1b39..c3d6b783417e 100644
>>>> --- a/hw/virtio/virtio.c
>>>> +++ b/hw/virtio/virtio.c
>> ...
>>>> @@ -4033,6 +4092,92 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
>>>>      return status;
>>>>  }
>>>>  
>>>> +#define DUMP_FEATURES(type, field)                                         \
>>>> +    do {                                                                   \
>>>> +        type##FeatureList *list = features->device->u.field.data;          \
>>>> +        if (list) {                                                        \
>>>> +            monitor_printf(mon, "                    ");                   \
>>>> +            while (list) {                                                 \
>>>> +                monitor_printf(mon, "%s", type##Feature_str(list->value)); \
>>>> +                list = list->next;                                         \
>>>> +                if (list != NULL) {                                        \
>>>> +                    monitor_printf(mon, ", ");                             \
>>>> +                }                                                          \
>>>> +            }                                                              \
>>>> +            monitor_printf(mon, "\n");                                     \
>>>> +        }                                                                  \
>>>> +    } while (0)
>>>
>>> It feels like you should be able to have an array of Feature_str's
>>> indexed by VIRTIO_DEVICE_FEATURE_KIND_ enum, so that when a new
>>> VIRTIO_DEVICE_FEATURE_KIND is added you don't need to fix this up.
>>
>> I don't understand what you mean here.
> 
> Instead of the switch below, I'm thinking you could have something like:
> 
>     if (features->device->type < something_MAX) {
>         features_str = anarray[features->device->type];
> 
>         ....
>         monitor_printf(mon, "%s", features_str(list->value));
>         ....
>     }
> 
> with 'anarray' somewhere more central, so we don't have to keep
> these switch structures and macros spread around.

OK, I tried that, but in fact we need to know the type of the list to be
able to scan it (the "type##FeatureList": VirtoSerialFeatureList,
VirtioBlkFeatureList, ...), except if we introduce a generic feature
list node (for "next " and "value"). But it becomes more complicated,
because we also need to generate the "anarray" somewhere.

[Note: I've changed the legacy enum to a flat enum as proposed by Eric
in v3]

Thanks,
Laurent



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-05-15 15:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 13:47 [RFC v4 0/6] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
2020-05-07 13:47 ` [RFC v4 1/6] qmp: add QMP command x-debug-query-virtio Laurent Vivier
2020-05-07 13:47 ` [RFC v4 2/6] qmp: add QMP command x-debug-virtio-status Laurent Vivier
2020-05-07 13:47 ` [RFC v4 3/6] qmp: decode feature bits in virtio-status Laurent Vivier
2020-05-07 13:47 ` [RFC v4 4/6] qmp: add QMP command x-debug-virtio-queue-status Laurent Vivier
2020-05-07 13:47 ` [RFC v4 5/6] qmp: add QMP command x-debug-virtio-queue-element Laurent Vivier
2020-05-07 13:48 ` [RFC v4 6/6] hmp: add x-debug-virtio commands Laurent Vivier
2020-05-13 10:51   ` Dr. David Alan Gilbert
2020-05-15 15:20     ` Laurent Vivier
2020-05-15 15:45       ` Dr. David Alan Gilbert
2020-05-15 15:57         ` Laurent Vivier
2020-05-07 20:11 ` [RFC v4 0/6] hmp, qmp: Add some commands to introspect virtio devices no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.