All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/5] hmp,qmp: Add some commands to introspect virtio devices
@ 2020-04-02 10:02 Laurent Vivier
  2020-04-02 10:02 ` [RFC 1/5] qmp: add QMP command query-virtio Laurent Vivier
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Laurent Vivier @ 2020-04-02 10:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, Dr. David Alan Gilbert, Michael Roth,
	Paolo Bonzini

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

1. Main command

 HMP Only:

     virtio [subcommand]

   Example:

    List all sub-commands:

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

2. List available virtio devices in the machine

  HMP Form:

    virtio query

  Example:

    (qemu) 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': 'query-virtio', 'returns': ['VirtioInfo'] }

  Example:

  -> { "execute": "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:

    virtio status <path>

  Example:

    (qemu) virtio status /machine/peripheral-anon/device[3]/virtio-backend
    /machine/peripheral-anon/device[3]/virtio-backend:
      Device Id:        1
      Guest features:   0x0000000130afffa7
      Host features:    0x0000000179bfffe7
      Backend features: 0x0000000000000000
      Endianness:       little
      VirtQueues:       3

  QMP Form:

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

  Example:

  -> { "execute": "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
       }
     }

4. Display status of a given virtio queue

  HMP Form:

    virtio queue-status <path> <queue>

  Example:

    (qemu) 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': 'virtio-queue-status',
      'data': { 'path': 'str', 'queue': 'uint16' },
      'returns': 'VirtQueueStatus'
    }

  Example:

  -> { "execute": "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:

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

  Example:

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

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

      (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': 'virtio-queue-element',
      'data': { 'path': 'str', 'queue': 'uint16', '*index': 'uint16' },
      'returns': 'VirtioQueueElement'
    }

  Example:

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

Laurent Vivier (5):
  qmp: add QMP command query-virtio
  qmp: add QMP command virtio-status
  qmp: add QMP command virtio-queue-status
  qmp: add QMP command virtio-queue-element
  hmp: add virtio commands

 Makefile                   |   2 +-
 Makefile.target            |   7 +-
 docs/system/monitor.rst    |   2 +
 hmp-commands-virtio.hx     | 148 ++++++++++++++++++
 hmp-commands.hx            |  10 ++
 hw/virtio/Makefile.objs    |   2 +
 hw/virtio/virtio-stub.c    |  32 ++++
 hw/virtio/virtio.c         | 313 +++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio.h |   1 +
 include/monitor/hmp.h      |   4 +
 monitor/misc.c             |  17 ++
 qapi/Makefile.objs         |   2 +-
 qapi/qapi-schema.json      |   1 +
 qapi/virtio.json           | 302 +++++++++++++++++++++++++++++++++++
 tests/qtest/qmp-cmd-test.c |   1 +
 15 files changed, 840 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.25.1



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

* [RFC 1/5] qmp: add QMP command query-virtio
  2020-04-02 10:02 [RFC 0/5] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
@ 2020-04-02 10:02 ` Laurent Vivier
  2020-04-02 14:23   ` Eric Blake
  2020-04-02 10:02 ` [RFC 2/5] qmp: add QMP command virtio-status Laurent Vivier
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2020-04-02 10:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, Dr. David Alan Gilbert, Michael Roth,
	Paolo Bonzini

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         | 27 +++++++++++++++++++
 include/hw/virtio/virtio.h |  1 +
 qapi/Makefile.objs         |  2 +-
 qapi/qapi-schema.json      |  1 +
 qapi/virtio.json           | 54 ++++++++++++++++++++++++++++++++++++++
 tests/qtest/qmp-cmd-test.c |  1 +
 8 files changed, 101 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..d9e4a815ecf1
--- /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_query_virtio(Error **errp)
+{
+    return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index b6c8ef5bc025..3cebc3d169c8 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,25 @@ bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
     return virtio_bus_ioeventfd_enabled(vbus);
 }
 
+VirtioInfoList *qmp_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->type = g_strdup(vdev->name);
+        node->value->path = g_strdup(dev->canonical_path);
+        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..2a95d08a8b9e
--- /dev/null
+++ b/qapi/virtio.json
@@ -0,0 +1,54 @@
+##
+# = Virtio devices
+##
+
+##
+# @VirtioInfo:
+#
+# Information about a given VirtIODevice
+#
+# @type: VirtIO device type.
+#
+# @path: VirtIO device canonical path.
+#
+# Since: 5.1
+#
+##
+{ 'struct': 'VirtioInfo',
+  'data': {
+    'path': 'str',
+    'type': 'str'
+  }
+}
+
+##
+# @query-virtio:
+#
+# Return the list of all VirtIO devices
+#
+# Returns: list of @VirtioInfo
+#
+# Since: 5.1
+#
+# Example:
+#
+# -> { "execute": "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': 'query-virtio', 'returns': ['VirtioInfo'] }
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index 9f5228cd9951..c0342c3a3075 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 */
+        "query-virtio",           /* CONFIG_VIRTIO */
         /* Success depends on launching SEV guest */
         "query-sev-launch-measure",
         /* Success depends on Host or Hypervisor SEV support */
-- 
2.25.1



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

* [RFC 2/5] qmp: add QMP command virtio-status
  2020-04-02 10:02 [RFC 0/5] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
  2020-04-02 10:02 ` [RFC 1/5] qmp: add QMP command query-virtio Laurent Vivier
@ 2020-04-02 10:02 ` Laurent Vivier
  2020-04-02 14:27   ` Eric Blake
  2020-04-02 10:03 ` [RFC 3/5] qmp: add QMP command virtio-queue-status Laurent Vivier
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2020-04-02 10:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, Dr. David Alan Gilbert, Michael Roth,
	Paolo Bonzini

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      | 51 ++++++++++++++++++++++++++++++++
 qapi/virtio.json        | 65 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+)

diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
index d9e4a815ecf1..8fe2d6cd8892 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -12,3 +12,8 @@ VirtioInfoList *qmp_query_virtio(Error **errp)
 {
     return qmp_virtio_unsupported(errp);
 }
+
+VirtioStatus *qmp_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 3cebc3d169c8..7f6e21e4ab2c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3839,6 +3839,57 @@ VirtioInfoList *qmp_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_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 = g_strdup("little");
+        break;
+    case VIRTIO_DEVICE_ENDIAN_BIG:
+        status->device_endian = g_strdup("big");
+        break;
+    case VIRTIO_DEVICE_ENDIAN_UNKNOWN:
+    default:
+        status->device_endian = g_strdup("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 2a95d08a8b9e..2af4d95b9893 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -52,3 +52,68 @@
 ##
 
 { 'command': 'query-virtio', 'returns': ['VirtioInfo'] }
+
+##
+# @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': 'str',
+    'guest_features': 'uint64',
+    'host_features': 'uint64',
+    'backend_features': 'uint64',
+    'num_vqs': 'uint16'
+  }
+}
+
+##
+# @virtio-status:
+#
+# Return the status of virtio device
+#
+# @path: QOBject path of the VirtIODevice
+#
+# Returns: status of the VirtIODevice
+#
+# Since: 5.1
+#
+# Example:
+#
+# -> { "execute": "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': 'virtio-status',
+  'data': { 'path': 'str' },
+  'returns': 'VirtioStatus'
+}
-- 
2.25.1



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

* [RFC 3/5] qmp: add QMP command virtio-queue-status
  2020-04-02 10:02 [RFC 0/5] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
  2020-04-02 10:02 ` [RFC 1/5] qmp: add QMP command query-virtio Laurent Vivier
  2020-04-02 10:02 ` [RFC 2/5] qmp: add QMP command virtio-status Laurent Vivier
@ 2020-04-02 10:03 ` Laurent Vivier
  2020-04-02 14:30   ` Eric Blake
  2020-04-02 10:03 ` [RFC 4/5] qmp: add QMP command virtio-queue-element Laurent Vivier
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2020-04-02 10:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, Dr. David Alan Gilbert, Michael Roth,
	Paolo Bonzini

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 8fe2d6cd8892..5b4ed6fd531e 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -17,3 +17,9 @@ VirtioStatus *qmp_virtio_status(const char* path, Error **errp)
 {
     return qmp_virtio_unsupported(errp);
 }
+
+VirtQueueStatus *qmp_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 7f6e21e4ab2c..271d4ca3417f 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3855,6 +3855,41 @@ static VirtIODevice *virtio_device_find(const char *path)
     return NULL;
 }
 
+VirtQueueStatus *qmp_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;
+}
+
 VirtioStatus *qmp_virtio_status(const char* path, Error **errp)
 {
     VirtIODevice *vdev;
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 2af4d95b9893..ab70500d919b 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -117,3 +117,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'
+  }
+}
+
+##
+# @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": "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': 'virtio-queue-status',
+  'data': { 'path': 'str', 'queue': 'uint16' },
+  'returns': 'VirtQueueStatus'
+}
-- 
2.25.1



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

* [RFC 4/5] qmp: add QMP command virtio-queue-element
  2020-04-02 10:02 [RFC 0/5] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
                   ` (2 preceding siblings ...)
  2020-04-02 10:03 ` [RFC 3/5] qmp: add QMP command virtio-queue-status Laurent Vivier
@ 2020-04-02 10:03 ` Laurent Vivier
  2020-04-02 14:31   ` Eric Blake
  2020-04-02 14:58   ` Dr. David Alan Gilbert
  2020-04-02 10:03 ` [RFC 5/5] hmp: add virtio commands Laurent Vivier
  2020-04-02 14:33 ` [RFC 0/5] hmp, qmp: Add some commands to introspect virtio devices Marc-André Lureau
  5 siblings, 2 replies; 14+ messages in thread
From: Laurent Vivier @ 2020-04-02 10:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, Dr. David Alan Gilbert, Michael Roth,
	Paolo Bonzini

This new command shows the information of a VirtQueue element.

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

diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
index 5b4ed6fd531e..693f5eac409f 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -23,3 +23,10 @@ VirtQueueStatus *qmp_virtio_queue_status(const char *path, uint16_t queue,
 {
     return qmp_virtio_unsupported(errp);
 }
+
+VirtioQueueElement *qmp_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 271d4ca3417f..28848b9e64cf 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3925,6 +3925,91 @@ VirtioStatus *qmp_virtio_status(const char* path, Error **errp)
     return status;
 }
 
+VirtioQueueElement *qmp_virtio_queue_element(const char* path, uint16_t queue,
+                                             bool has_index, uint16_t index,
+                                             Error **errp)
+{
+    VirtIODevice *vdev;
+    VirtQueue *vq;
+    VirtioQueueElement *element;
+
+    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 *desc_cache;
+        VRingDesc desc;
+
+        RCU_READ_LOCK_GUARD();
+        if (virtio_queue_empty_rcu(vq)) {
+            error_setg(errp, "Queue is empty");
+            return NULL;
+        }
+        /*
+         * Needed after virtio_queue_empty(), see comment in
+         * virtqueue_num_heads().
+         */
+        smp_rmb();
+
+        max = vq->vring.num;
+
+        if (vq->inuse >= vq->vring.num) {
+            error_setg(errp, "Queue size exceeded");
+            return NULL;
+        }
+
+        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) {
+            error_setg(errp, "Unsupported indirect buffer feature");
+            return NULL;
+        }
+
+        element = g_new0(VirtioQueueElement, 1);
+        element->index = head;
+        element->ndescs = 1;
+        element->descs = g_new0(VirtioRingDescList, 1);
+        element->descs->value = g_new0(VirtioRingDesc, 1);
+        element->descs->value->addr = desc.addr;
+        element->descs->value->len = desc.len;
+        element->descs->value->flags = desc.flags;
+    }
+
+    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 ab70500d919b..3e8865511217 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -215,3 +215,88 @@
   'data': { 'path': 'str', 'queue': 'uint16' },
   'returns': 'VirtQueueStatus'
 }
+
+##
+# @VirtioRingDesc:
+#
+# @addr: guest physical address of the descriptor data
+#
+# @len: length of the descriptor data
+#
+# @flags: descriptor flags (write-only, read-only, ...)
+#
+# Since: 5.1
+#
+##
+
+{ 'struct': 'VirtioRingDesc',
+  'data': {
+    'addr': 'uint64',
+    'len': 'uint32',
+    'flags': 'uint16'
+  }
+}
+
+##
+# @VirtioQueueElement:
+#
+# @index: index of the element in the queue
+#
+# @len: length of the element data
+#
+# @ndescs: number of descriptors
+#
+# @descs: list of the descriptors
+#
+# Since: 5.1
+#
+##
+
+{ 'struct': 'VirtioQueueElement',
+  'data': {
+    'index': 'uint32',
+    'len': 'uint32',
+    'ndescs': 'uint32',
+    'descs': ['VirtioRingDesc']
+  }
+}
+
+##
+# @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": "virtio-queue-element",
+#      "arguments": {
+#          "path": "/machine/peripheral-anon/device[3]/virtio-backend",
+#          "queue": 0
+#      }
+#   }
+# -> { "return": {
+#         "index": 109,
+#         "len": 0,
+#         "ndescs": 1,
+#         "descs": [
+#             { "flags": 2, "len": 2048, "addr": 853145600 }
+#         ]
+#      }
+#   }
+#
+##
+
+{ 'command': 'virtio-queue-element',
+  'data': { 'path': 'str', 'queue': 'uint16', '*index': 'uint16' },
+  'returns': 'VirtioQueueElement'
+}
-- 
2.25.1



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

* [RFC 5/5] hmp: add virtio commands
  2020-04-02 10:02 [RFC 0/5] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
                   ` (3 preceding siblings ...)
  2020-04-02 10:03 ` [RFC 4/5] qmp: add QMP command virtio-queue-element Laurent Vivier
@ 2020-04-02 10:03 ` Laurent Vivier
  2020-04-02 15:08   ` Dr. David Alan Gilbert
  2020-04-02 14:33 ` [RFC 0/5] hmp, qmp: Add some commands to introspect virtio devices Marc-André Lureau
  5 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2020-04-02 10:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, Dr. David Alan Gilbert, Michael Roth,
	Paolo Bonzini

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  | 148 ++++++++++++++++++++++++++++++++++++++++
 hmp-commands.hx         |  10 +++
 hw/virtio/virtio.c      | 115 +++++++++++++++++++++++++++++++
 include/monitor/hmp.h   |   4 ++
 monitor/misc.c          |  17 +++++
 8 files changed, 302 insertions(+), 3 deletions(-)
 create mode 100644 hmp-commands-virtio.hx

diff --git a/Makefile b/Makefile
index 84ef88160006..5f70ea16a566 100644
--- a/Makefile
+++ b/Makefile
@@ -1100,7 +1100,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..a8d49f0b2b46
--- /dev/null
+++ b/hmp-commands-virtio.hx
@@ -0,0 +1,148 @@
+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 "virtio cmd"
+HXCOMM appears inside the documentation list item for the top level
+HXCOMM "virtio" documentation entry. The exception is the first SRST
+HXCOMM fragment that defines that top level entry.
+
+SRST
+``virtio`` *subcommand*
+  Show various information about virtio.
+
+ERST
+
+    {
+        .name       = "query",
+        .args_type  = "",
+        .params     = "",
+        .help       = "List all available virtio devices",
+        .cmd        = hmp_virtio_query,
+        .flags      = "p",
+    },
+
+SRST
+  ``virtio query``
+    List all available virtio devices
+
+    Example:
+
+    List all available virtio devices in the machine::
+
+      (qemu) 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_virtio_status,
+        .flags      = "p",
+    },
+
+SRST
+  ``virtio status`` *path*
+    Display status of a given virtio device
+
+    Example:
+
+    Dump the status of the first virtio device::
+
+      (qemu) virtio status /machine/peripheral-anon/device[3]/virtio-backend
+      /machine/peripheral-anon/device[3]/virtio-backend:
+        Device Id:        1
+        Guest features:   0x0000000130afffa7
+        Host features:    0x0000000179bfffe7
+        Backend features: 0x0000000000000000
+        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_virtio_queue_status,
+        .flags      = "p",
+    },
+
+SRST
+  ``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) 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_virtio_queue_element,
+        .flags      = "p",
+    },
+
+SRST
+  ``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) virtio queue-element/machine/peripheral-anon/device[3]/virtio-backend 0
+      index:  67
+      ndescs: 1
+      descs:  addr 0x6fe69800 len 1536 (write-only)
+
+      (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..14568b406dbc 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       = "virtio",
+        .args_type  = "name:S?",
+        .params     = "[cmd]",
+        .help       = "show various information about virtio",
+        .cmd        = hmp_virtio_help,
+        .sub_table  = hmp_virtio_cmds,
+        .flags      = "p",
+    },
+
     {
         .name       = "info",
         .args_type  = "item:s?",
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 28848b9e64cf..d52d41169fb4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -29,6 +29,9 @@
 #include "hw/virtio/virtio-access.h"
 #include "sysemu/dma.h"
 #include "sysemu/runstate.h"
+#include "monitor/hmp.h"
+#include "monitor/monitor.h"
+#include "qapi/qmp/qdict.h"
 
 static QTAILQ_HEAD(, VirtIODevice) virtio_list;
 
@@ -3839,6 +3842,29 @@ VirtioInfoList *qmp_query_virtio(Error **errp)
     return list;
 }
 
+void hmp_virtio_query(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    VirtioInfoList *l = qmp_query_virtio(&err);
+
+    if (err != NULL) {
+        hmp_handle_error(mon, err);
+        return;
+    }
+
+    if (l == NULL) {
+        monitor_printf(mon, "No VirtIO devices\n");
+        return;
+    }
+
+    while (l) {
+        monitor_printf(mon, "%s [%s]\n", l->value->path, l->value->type);
+        l = l->next;
+    }
+
+    qapi_free_VirtioInfoList(l);
+}
+
 static VirtIODevice *virtio_device_find(const char *path)
 {
     VirtIODevice *vdev;
@@ -3890,6 +3916,36 @@ VirtQueueStatus *qmp_virtio_queue_status(const char *path, uint16_t queue,
     return status;
 }
 
+void hmp_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_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);
+}
+
 VirtioStatus *qmp_virtio_status(const char* path, Error **errp)
 {
     VirtIODevice *vdev;
@@ -3925,6 +3981,31 @@ VirtioStatus *qmp_virtio_status(const char* path, Error **errp)
     return status;
 }
 
+void hmp_virtio_status(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    const char *path = qdict_get_try_str(qdict, "path");
+    VirtioStatus *s = qmp_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:   0x%016"PRIx64"\n",
+                   s->guest_features);
+    monitor_printf(mon, "  Host features:    0x%016"PRIx64"\n",
+                   s->host_features);
+    monitor_printf(mon, "  Backend features: 0x%016"PRIx64"\n",
+                   s->backend_features);
+    monitor_printf(mon, "  Endianness:       %s\n", s->device_endian);
+    monitor_printf(mon, "  VirtQueues:       %d\n", s->num_vqs);
+
+    qapi_free_VirtioStatus(s);
+}
+
 VirtioQueueElement *qmp_virtio_queue_element(const char* path, uint16_t queue,
                                              bool has_index, uint16_t index,
                                              Error **errp)
@@ -4010,6 +4091,40 @@ VirtioQueueElement *qmp_virtio_queue_element(const char* path, uint16_t queue,
     return element;
 }
 
+void hmp_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_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 %s", list->value->addr,
+                       list->value->len, list->value->flags &
+                       VRING_DESC_F_WRITE ? "(write-only)" : "(read-only)");
+        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..9f1c118dde31 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_virtio_query(Monitor *mon, const QDict *qdict);
+void hmp_virtio_status(Monitor *mon, const QDict *qdict);
+void hmp_virtio_queue_status(Monitor *mon, const QDict *qdict);
+void hmp_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 6c45fa490ff5..5eacfa7079fc 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_virtio_help(Monitor *mon, const QDict *qdict)
+{
+#if defined(CONFIG_VIRTIO)
+    help_cmd(mon, "virtio");
+#else
+    monitor_printf(mon, "Virtio is disabled\n");
+#endif
+}
+
 static void monitor_init_qmp_commands(void)
 {
     /*
@@ -1683,6 +1693,13 @@ static HMPCommand hmp_info_cmds[] = {
     { NULL, NULL, },
 };
 
+static HMPCommand hmp_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.25.1



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

* Re: [RFC 1/5] qmp: add QMP command query-virtio
  2020-04-02 10:02 ` [RFC 1/5] qmp: add QMP command query-virtio Laurent Vivier
@ 2020-04-02 14:23   ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2020-04-02 14:23 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel
  Cc: Thomas Huth, Michael Roth, Michael S. Tsirkin,
	Dr. David Alan Gilbert, Markus Armbruster, Paolo Bonzini

On 4/2/20 5:02 AM, Laurent Vivier wrote:
> This new command lists all the instances of VirtIODevice with
> their path and virtio type
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

> +++ b/qapi/virtio.json
> @@ -0,0 +1,54 @@
> +##
> +# = Virtio devices
> +##
> +
> +##
> +# @VirtioInfo:
> +#
> +# Information about a given VirtIODevice
> +#
> +# @type: VirtIO device type.
> +#
> +# @path: VirtIO device canonical path.

One order here,

> +#
> +# Since: 5.1
> +#
> +##
> +{ 'struct': 'VirtioInfo',
> +  'data': {
> +    'path': 'str',
> +    'type': 'str'

and another here.  I don't think our doc generator cares, but it is nice 
to document things in the same order they are declared.

Why are we using open-coded 'type':'str' instead of an enum type? 
Aren't there only a finite number of virtio types, in which case an enum 
is a better representation (not to mention making it introspectible when 
we add a new virtio type)?

> +  }
> +}
> +
> +##
> +# @query-virtio:
> +#
> +# Return the list of all VirtIO devices
> +#
> +# Returns: list of @VirtioInfo
> +#
> +# Since: 5.1
> +#
> +# Example:
> +#
> +# -> { "execute": "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"
> +#        }
> +#      ]

The resulting QMP will still be the same with an enum type (as the 
on-the-wire format is still the string name of the enum constant), but 
using an enum will have knock-on effects to the C code; it may be better 
to convert to an enum in one patch, then add the query-virtio command in 
a second.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [RFC 2/5] qmp: add QMP command virtio-status
  2020-04-02 10:02 ` [RFC 2/5] qmp: add QMP command virtio-status Laurent Vivier
@ 2020-04-02 14:27   ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2020-04-02 14:27 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel
  Cc: Thomas Huth, Michael Roth, Michael S. Tsirkin,
	Dr. David Alan Gilbert, Markus Armbruster, Paolo Bonzini

On 4/2/20 5:02 AM, Laurent Vivier wrote:
> This new command shows the status of a VirtIODevice
> (features, endianness and number of virtqueues)
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

> +++ b/qapi/virtio.json
> @@ -52,3 +52,68 @@
>   ##
>   
>   { 'command': 'query-virtio', 'returns': ['VirtioInfo'] }
> +
> +##
> +# @VirtioStatus:
> +#
> +# @device_id: VirtIODevice status

As this is a new API, it should stick to our naming conventions:

device-id

> +#
> +# @device_endian: VirtIODevice device_endian

device-endian

> +#
> +# @guest_features: VirtIODevice guest_features

guest-features

> +#
> +# @host_features: VirtIODevice host_features

host-features

> +#
> +# @backend_features: VirtIODevice backend_features

backend-features

> +#
> +# @num_vqs: number of VirtIODevice queues

num-vqs

> +#
> +# Since: 5.1
> +#
> +##
> +
> +{ 'struct': 'VirtioStatus',
> +  'data': {
> +    'device_id': 'int',
> +    'device_endian': 'str',
> +    'guest_features': 'uint64',
> +    'host_features': 'uint64',
> +    'backend_features': 'uint64',

A bare int requires subsequent decoding.  Can this instead be an array 
of enum values, with enum values naming each enabled feature?

> +    'num_vqs': 'uint16'
> +  }
> +}
> +
> +##
> +# @virtio-status:
> +#
> +# Return the status of virtio device
> +#
> +# @path: QOBject path of the VirtIODevice
> +#
> +# Returns: status of the VirtIODevice
> +#
> +# Since: 5.1
> +#
> +# Example:
> +#
> +# -> { "execute": "virtio-status",
> +#      "arguments": {
> +#          "path": "/machine/peripheral-anon/device[3]/virtio-backend"
> +#      }
> +#   }
> +# <- { "return": {
> +#          "backend_features": 0,
> +#          "guest_features": 5111807911,

again, this means nothing to me.
   "guest-features": ["feature-a","feature-b"]
is vastly more usable if I'm inspecting the query output, compared to 
decoding the decimal number back into bits then doing a lookup into the 
documentation of which bits mean which features.

> +#          "num_vqs": 3,
> +#          "host_features": 6337593319,
> +#          "device_endian": "little",
> +#          "device_id": 1
> +#      }
> +#    }
> +#
> +##
> +
> +{ 'command': 'virtio-status',
> +  'data': { 'path': 'str' },
> +  'returns': 'VirtioStatus'
> +}
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [RFC 3/5] qmp: add QMP command virtio-queue-status
  2020-04-02 10:03 ` [RFC 3/5] qmp: add QMP command virtio-queue-status Laurent Vivier
@ 2020-04-02 14:30   ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2020-04-02 14:30 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel
  Cc: Thomas Huth, Michael Roth, Michael S. Tsirkin,
	Dr. David Alan Gilbert, Markus Armbruster, Paolo Bonzini

On 4/2/20 5:03 AM, Laurent Vivier wrote:
> 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(+)
> 

> +++ b/qapi/virtio.json
> @@ -117,3 +117,101 @@
>     'data': { 'path': 'str' },
>     'returns': 'VirtioStatus'
>   }
> +
> +##
> +# @VirtQueueStatus:
> +#
> +# Status of a VirtQueue
> +#
> +# @queue_index: VirtQueue queue_index

More naming conventions.  queue-index.  I'll quit pointing them out, but 
the whole series should be scrubbed.


> +# Example:
> +#
> +# -> { "execute": "virtio-queue-status",
> +#      "arguments": {
> +#          "path": "/machine/peripheral-anon/device[3]/virtio-backend",
> +#          "queue": 0
> +#      }
> +#   }
> +# <- { "return": {
> +#      "signalled_used": 373,

Of course, if you fix the naming conventions, you'll also need to 
regenerate the examples.

Hmm, it's a bit annoying that our current QMP output is 
non-deterministic.  It doesn't affect correctness, but it would be nice 
if our output was in the same order as our declaration.  Not the fault 
of your patch, nor something you need to worry about.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [RFC 4/5] qmp: add QMP command virtio-queue-element
  2020-04-02 10:03 ` [RFC 4/5] qmp: add QMP command virtio-queue-element Laurent Vivier
@ 2020-04-02 14:31   ` Eric Blake
  2020-04-02 14:58   ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 14+ messages in thread
From: Eric Blake @ 2020-04-02 14:31 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel
  Cc: Thomas Huth, Michael Roth, Michael S. Tsirkin,
	Dr. David Alan Gilbert, Markus Armbruster, Paolo Bonzini

On 4/2/20 5:03 AM, Laurent Vivier wrote:
> This new command shows the information of a VirtQueue element.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   hw/virtio/virtio-stub.c |  7 ++++
>   hw/virtio/virtio.c      | 85 +++++++++++++++++++++++++++++++++++++++++
>   qapi/virtio.json        | 85 +++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 177 insertions(+)
> 

> +##
> +# @VirtioRingDesc:
> +#
> +# @addr: guest physical address of the descriptor data
> +#
> +# @len: length of the descriptor data
> +#
> +# @flags: descriptor flags (write-only, read-only, ...)
> +#
> +# Since: 5.1
> +#
> +##
> +
> +{ 'struct': 'VirtioRingDesc',
> +  'data': {
> +    'addr': 'uint64',
> +    'len': 'uint32',
> +    'flags': 'uint16'

Again, flags should probably be an array of enum values, rather than a 
bare int.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [RFC 0/5] hmp, qmp: Add some commands to introspect virtio devices
  2020-04-02 10:02 [RFC 0/5] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
                   ` (4 preceding siblings ...)
  2020-04-02 10:03 ` [RFC 5/5] hmp: add virtio commands Laurent Vivier
@ 2020-04-02 14:33 ` Marc-André Lureau
  2020-04-02 15:23   ` Dr. David Alan Gilbert
  5 siblings, 1 reply; 14+ messages in thread
From: Marc-André Lureau @ 2020-04-02 14:33 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Thomas Huth, Michael S. Tsirkin, QEMU, Michael Roth,
	Markus Armbruster, Paolo Bonzini, Dr. David Alan Gilbert

Hi

On Thu, Apr 2, 2020 at 12:03 PM Laurent Vivier <lvivier@redhat.com> wrote:
>
> This series introduces new QMP/HMP commands to dump the status
> of a a virtio device at different levels.

Looks nice, but my feeling is that we are growing qemu with debugging
facilities in general with HMP/QMP.

If this is only for debugging purposes, why don't we compile it only
when --enable-debug?

A guest that would need to be debugged could have its state loaded in
a debug version of qemu.

Alternatively, I think most of the data you provide here could
probably be introspected via gdb scripts.

Just some thoughts

>
> 1. Main command
>
>  HMP Only:
>
>      virtio [subcommand]
>
>    Example:
>
>     List all sub-commands:
>
>     (qemu) virtio
>     virtio query  -- List all available virtio devices
>     virtio status path -- Display status of a given virtio device
>     virtio queue-status path queue -- Display status of a given virtio queue
>     virtio queue-element path queue [index] -- Display element of a given virtio queue
>
> 2. List available virtio devices in the machine
>
>   HMP Form:
>
>     virtio query
>
>   Example:
>
>     (qemu) 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': 'query-virtio', 'returns': ['VirtioInfo'] }
>
>   Example:
>
>   -> { "execute": "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:
>
>     virtio status <path>
>
>   Example:
>
>     (qemu) virtio status /machine/peripheral-anon/device[3]/virtio-backend
>     /machine/peripheral-anon/device[3]/virtio-backend:
>       Device Id:        1
>       Guest features:   0x0000000130afffa7
>       Host features:    0x0000000179bfffe7
>       Backend features: 0x0000000000000000
>       Endianness:       little
>       VirtQueues:       3
>
>   QMP Form:
>
>     { 'command': 'virtio-status',
>       'data': { 'path': 'str' },
>       'returns': 'VirtioStatus'
>     }
>
>   Example:
>
>   -> { "execute": "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
>        }
>      }
>
> 4. Display status of a given virtio queue
>
>   HMP Form:
>
>     virtio queue-status <path> <queue>
>
>   Example:
>
>     (qemu) 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': 'virtio-queue-status',
>       'data': { 'path': 'str', 'queue': 'uint16' },
>       'returns': 'VirtQueueStatus'
>     }
>
>   Example:
>
>   -> { "execute": "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:
>
>     virtio queue-element <path> <queue> [index]
>
>   Example:
>
>     Dump the information of the head element of the first queue of
>     the first virtio device::
>
>       (qemu) virtio queue-element/machine/peripheral-anon/device[3]/virtio-backend 0
>       index:  67
>       ndescs: 1
>       descs:  addr 0x6fe69800 len 1536 (write-only)
>
>       (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': 'virtio-queue-element',
>       'data': { 'path': 'str', 'queue': 'uint16', '*index': 'uint16' },
>       'returns': 'VirtioQueueElement'
>     }
>
>   Example:
>
>   -> { "execute": "virtio-queue-element",
>        "arguments": {
>            "path": "/machine/peripheral-anon/device[3]/virtio-backend",
>            "queue": 0
>        }
>     }
>   -> { "return": {
>           "index": 109,
>           "len": 0,
>           "ndescs": 1,
>           "descs": [
>               { "flags": 2, "len": 2048, "addr": 853145600 }
>           ]
>        }
>     }
>
> Laurent Vivier (5):
>   qmp: add QMP command query-virtio
>   qmp: add QMP command virtio-status
>   qmp: add QMP command virtio-queue-status
>   qmp: add QMP command virtio-queue-element
>   hmp: add virtio commands
>
>  Makefile                   |   2 +-
>  Makefile.target            |   7 +-
>  docs/system/monitor.rst    |   2 +
>  hmp-commands-virtio.hx     | 148 ++++++++++++++++++
>  hmp-commands.hx            |  10 ++
>  hw/virtio/Makefile.objs    |   2 +
>  hw/virtio/virtio-stub.c    |  32 ++++
>  hw/virtio/virtio.c         | 313 +++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio.h |   1 +
>  include/monitor/hmp.h      |   4 +
>  monitor/misc.c             |  17 ++
>  qapi/Makefile.objs         |   2 +-
>  qapi/qapi-schema.json      |   1 +
>  qapi/virtio.json           | 302 +++++++++++++++++++++++++++++++++++
>  tests/qtest/qmp-cmd-test.c |   1 +
>  15 files changed, 840 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.25.1
>
>


-- 
Marc-André Lureau


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

* Re: [RFC 4/5] qmp: add QMP command virtio-queue-element
  2020-04-02 10:03 ` [RFC 4/5] qmp: add QMP command virtio-queue-element Laurent Vivier
  2020-04-02 14:31   ` Eric Blake
@ 2020-04-02 14:58   ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2020-04-02 14:58 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Thomas Huth, Michael Roth, Michael S. Tsirkin, qemu-devel,
	Markus Armbruster, Paolo Bonzini

* Laurent Vivier (lvivier@redhat.com) wrote:
> This new command shows the information of a VirtQueue element.

Having had a few second play with this, I think I've always seen it say
that the ring is empty; is this pretty much always the case when the VM
is running and the device is consuming elements off the queue - so most
cases where this is useful is where the VM is paused?
> +        desc_cache = &caches->desc;
> +        vring_split_desc_read(vdev, &desc, desc_cache, i);
> +        if (desc.flags & VRING_DESC_F_INDIRECT) {
> +            error_setg(errp, "Unsupported indirect buffer feature");
> +            return NULL;
> +        }

I did trigger this in the case I was playing with.

Dave

> +        element = g_new0(VirtioQueueElement, 1);
> +        element->index = head;
> +        element->ndescs = 1;
> +        element->descs = g_new0(VirtioRingDescList, 1);
> +        element->descs->value = g_new0(VirtioRingDesc, 1);
> +        element->descs->value->addr = desc.addr;
> +        element->descs->value->len = desc.len;
> +        element->descs->value->flags = desc.flags;
> +    }
> +
> +    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 ab70500d919b..3e8865511217 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -215,3 +215,88 @@
>    'data': { 'path': 'str', 'queue': 'uint16' },
>    'returns': 'VirtQueueStatus'
>  }
> +
> +##
> +# @VirtioRingDesc:
> +#
> +# @addr: guest physical address of the descriptor data
> +#
> +# @len: length of the descriptor data
> +#
> +# @flags: descriptor flags (write-only, read-only, ...)
> +#
> +# Since: 5.1
> +#
> +##
> +
> +{ 'struct': 'VirtioRingDesc',
> +  'data': {
> +    'addr': 'uint64',
> +    'len': 'uint32',
> +    'flags': 'uint16'
> +  }
> +}
> +
> +##
> +# @VirtioQueueElement:
> +#
> +# @index: index of the element in the queue
> +#
> +# @len: length of the element data
> +#
> +# @ndescs: number of descriptors
> +#
> +# @descs: list of the descriptors
> +#
> +# Since: 5.1
> +#
> +##
> +
> +{ 'struct': 'VirtioQueueElement',
> +  'data': {
> +    'index': 'uint32',
> +    'len': 'uint32',
> +    'ndescs': 'uint32',
> +    'descs': ['VirtioRingDesc']
> +  }
> +}
> +
> +##
> +# @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": "virtio-queue-element",
> +#      "arguments": {
> +#          "path": "/machine/peripheral-anon/device[3]/virtio-backend",
> +#          "queue": 0
> +#      }
> +#   }
> +# -> { "return": {
> +#         "index": 109,
> +#         "len": 0,
> +#         "ndescs": 1,
> +#         "descs": [
> +#             { "flags": 2, "len": 2048, "addr": 853145600 }
> +#         ]
> +#      }
> +#   }
> +#
> +##
> +
> +{ 'command': 'virtio-queue-element',
> +  'data': { 'path': 'str', 'queue': 'uint16', '*index': 'uint16' },
> +  'returns': 'VirtioQueueElement'
> +}
> -- 
> 2.25.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [RFC 5/5] hmp: add virtio commands
  2020-04-02 10:03 ` [RFC 5/5] hmp: add virtio commands Laurent Vivier
@ 2020-04-02 15:08   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2020-04-02 15:08 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Thomas Huth, Michael Roth, Michael S. Tsirkin, qemu-devel,
	Markus Armbruster, Paolo Bonzini

* Laurent Vivier (lvivier@redhat.com) wrote:
> 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  | 148 ++++++++++++++++++++++++++++++++++++++++
>  hmp-commands.hx         |  10 +++
>  hw/virtio/virtio.c      | 115 +++++++++++++++++++++++++++++++
>  include/monitor/hmp.h   |   4 ++
>  monitor/misc.c          |  17 +++++
>  8 files changed, 302 insertions(+), 3 deletions(-)
>  create mode 100644 hmp-commands-virtio.hx
> 
> diff --git a/Makefile b/Makefile
> index 84ef88160006..5f70ea16a566 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1100,7 +1100,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)$@")
> +

(It would be great if these files moved into monitor at some point)

>  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..a8d49f0b2b46
> --- /dev/null
> +++ b/hmp-commands-virtio.hx
> @@ -0,0 +1,148 @@
> +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 "virtio cmd"
> +HXCOMM appears inside the documentation list item for the top level
> +HXCOMM "virtio" documentation entry. The exception is the first SRST
> +HXCOMM fragment that defines that top level entry.
> +
> +SRST
> +``virtio`` *subcommand*
> +  Show various information about virtio.
> +
> +ERST
> +
> +    {
> +        .name       = "query",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "List all available virtio devices",
> +        .cmd        = hmp_virtio_query,
> +        .flags      = "p",
> +    },
> +
> +SRST
> +  ``virtio query``
> +    List all available virtio devices
> +
> +    Example:
> +
> +    List all available virtio devices in the machine::
> +
> +      (qemu) 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_virtio_status,
> +        .flags      = "p",
> +    },
> +
> +SRST
> +  ``virtio status`` *path*
> +    Display status of a given virtio device
> +
> +    Example:
> +
> +    Dump the status of the first virtio device::
> +
> +      (qemu) virtio status /machine/peripheral-anon/device[3]/virtio-backend
> +      /machine/peripheral-anon/device[3]/virtio-backend:
> +        Device Id:        1
> +        Guest features:   0x0000000130afffa7
> +        Host features:    0x0000000179bfffe7
> +        Backend features: 0x0000000000000000
> +        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_virtio_queue_status,
> +        .flags      = "p",
> +    },
> +
> +SRST
> +  ``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) 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_virtio_queue_element,
> +        .flags      = "p",
> +    },
> +
> +SRST
> +  ``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) virtio queue-element/machine/peripheral-anon/device[3]/virtio-backend 0
> +      index:  67
> +      ndescs: 1
> +      descs:  addr 0x6fe69800 len 1536 (write-only)
> +
> +      (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..14568b406dbc 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       = "virtio",
> +        .args_type  = "name:S?",
> +        .params     = "[cmd]",
> +        .help       = "show various information about virtio",
> +        .cmd        = hmp_virtio_help,
> +        .sub_table  = hmp_virtio_cmds,
> +        .flags      = "p",
> +    },
> +
>      {
>          .name       = "info",
>          .args_type  = "item:s?",
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 28848b9e64cf..d52d41169fb4 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -29,6 +29,9 @@
>  #include "hw/virtio/virtio-access.h"
>  #include "sysemu/dma.h"
>  #include "sysemu/runstate.h"
> +#include "monitor/hmp.h"
> +#include "monitor/monitor.h"
> +#include "qapi/qmp/qdict.h"
>  
>  static QTAILQ_HEAD(, VirtIODevice) virtio_list;
>  
> @@ -3839,6 +3842,29 @@ VirtioInfoList *qmp_query_virtio(Error **errp)
>      return list;
>  }
>  
> +void hmp_virtio_query(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    VirtioInfoList *l = qmp_query_virtio(&err);
> +
> +    if (err != NULL) {
> +        hmp_handle_error(mon, err);
> +        return;
> +    }
> +
> +    if (l == NULL) {
> +        monitor_printf(mon, "No VirtIO devices\n");
> +        return;
> +    }
> +
> +    while (l) {
> +        monitor_printf(mon, "%s [%s]\n", l->value->path, l->value->type);
> +        l = l->next;
> +    }
> +
> +    qapi_free_VirtioInfoList(l);

I think you've made the same mistake there that I made in the vnc code;
you're walking 'l' along the list and then freeing l at the end, but l
is always null.

> +}
> +
>  static VirtIODevice *virtio_device_find(const char *path)
>  {
>      VirtIODevice *vdev;
> @@ -3890,6 +3916,36 @@ VirtQueueStatus *qmp_virtio_queue_status(const char *path, uint16_t queue,
>      return status;
>  }
>  
> +void hmp_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_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);
> +}
> +
>  VirtioStatus *qmp_virtio_status(const char* path, Error **errp)
>  {
>      VirtIODevice *vdev;
> @@ -3925,6 +3981,31 @@ VirtioStatus *qmp_virtio_status(const char* path, Error **errp)
>      return status;
>  }
>  
> +void hmp_virtio_status(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    const char *path = qdict_get_try_str(qdict, "path");
> +    VirtioStatus *s = qmp_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:   0x%016"PRIx64"\n",
> +                   s->guest_features);
> +    monitor_printf(mon, "  Host features:    0x%016"PRIx64"\n",
> +                   s->host_features);
> +    monitor_printf(mon, "  Backend features: 0x%016"PRIx64"\n",
> +                   s->backend_features);
> +    monitor_printf(mon, "  Endianness:       %s\n", s->device_endian);
> +    monitor_printf(mon, "  VirtQueues:       %d\n", s->num_vqs);
> +
> +    qapi_free_VirtioStatus(s);
> +}
> +
>  VirtioQueueElement *qmp_virtio_queue_element(const char* path, uint16_t queue,
>                                               bool has_index, uint16_t index,
>                                               Error **errp)
> @@ -4010,6 +4091,40 @@ VirtioQueueElement *qmp_virtio_queue_element(const char* path, uint16_t queue,
>      return element;
>  }
>  
> +void hmp_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_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 %s", list->value->addr,
> +                       list->value->len, list->value->flags &
> +                       VRING_DESC_F_WRITE ? "(write-only)" : "(read-only)");
> +        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..9f1c118dde31 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_virtio_query(Monitor *mon, const QDict *qdict);
> +void hmp_virtio_status(Monitor *mon, const QDict *qdict);
> +void hmp_virtio_queue_status(Monitor *mon, const QDict *qdict);
> +void hmp_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 6c45fa490ff5..5eacfa7079fc 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_virtio_help(Monitor *mon, const QDict *qdict)
> +{
> +#if defined(CONFIG_VIRTIO)
> +    help_cmd(mon, "virtio");
> +#else
> +    monitor_printf(mon, "Virtio is disabled\n");
> +#endif
> +}
> +
>  static void monitor_init_qmp_commands(void)
>  {
>      /*
> @@ -1683,6 +1693,13 @@ static HMPCommand hmp_info_cmds[] = {
>      { NULL, NULL, },
>  };
>  
> +static HMPCommand hmp_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.25.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [RFC 0/5] hmp, qmp: Add some commands to introspect virtio devices
  2020-04-02 14:33 ` [RFC 0/5] hmp, qmp: Add some commands to introspect virtio devices Marc-André Lureau
@ 2020-04-02 15:23   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2020-04-02 15:23 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, QEMU,
	Markus Armbruster, Michael Roth, Paolo Bonzini

* Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> Hi
> 
> On Thu, Apr 2, 2020 at 12:03 PM Laurent Vivier <lvivier@redhat.com> wrote:
> >
> > This series introduces new QMP/HMP commands to dump the status
> > of a a virtio device at different levels.
> 
> Looks nice, but my feeling is that we are growing qemu with debugging
> facilities in general with HMP/QMP.
> 
> If this is only for debugging purposes, why don't we compile it only
> when --enable-debug?
> 
> A guest that would need to be debugged could have its state loaded in
> a debug version of qemu.
> 
> Alternatively, I think most of the data you provide here could
> probably be introspected via gdb scripts.
> 
> Just some thoughts

I kind of agree, but virtio is bread-and-butter, so having it available
all the time isn't a bad idea; however having said that hte same
argument would be for why having gdb scripts to examine your virtio
queues would be useful if you're dealing with a core.

Dave

> 
> >
> > 1. Main command
> >
> >  HMP Only:
> >
> >      virtio [subcommand]
> >
> >    Example:
> >
> >     List all sub-commands:
> >
> >     (qemu) virtio
> >     virtio query  -- List all available virtio devices
> >     virtio status path -- Display status of a given virtio device
> >     virtio queue-status path queue -- Display status of a given virtio queue
> >     virtio queue-element path queue [index] -- Display element of a given virtio queue
> >
> > 2. List available virtio devices in the machine
> >
> >   HMP Form:
> >
> >     virtio query
> >
> >   Example:
> >
> >     (qemu) 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': 'query-virtio', 'returns': ['VirtioInfo'] }
> >
> >   Example:
> >
> >   -> { "execute": "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:
> >
> >     virtio status <path>
> >
> >   Example:
> >
> >     (qemu) virtio status /machine/peripheral-anon/device[3]/virtio-backend
> >     /machine/peripheral-anon/device[3]/virtio-backend:
> >       Device Id:        1
> >       Guest features:   0x0000000130afffa7
> >       Host features:    0x0000000179bfffe7
> >       Backend features: 0x0000000000000000
> >       Endianness:       little
> >       VirtQueues:       3
> >
> >   QMP Form:
> >
> >     { 'command': 'virtio-status',
> >       'data': { 'path': 'str' },
> >       'returns': 'VirtioStatus'
> >     }
> >
> >   Example:
> >
> >   -> { "execute": "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
> >        }
> >      }
> >
> > 4. Display status of a given virtio queue
> >
> >   HMP Form:
> >
> >     virtio queue-status <path> <queue>
> >
> >   Example:
> >
> >     (qemu) 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': 'virtio-queue-status',
> >       'data': { 'path': 'str', 'queue': 'uint16' },
> >       'returns': 'VirtQueueStatus'
> >     }
> >
> >   Example:
> >
> >   -> { "execute": "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:
> >
> >     virtio queue-element <path> <queue> [index]
> >
> >   Example:
> >
> >     Dump the information of the head element of the first queue of
> >     the first virtio device::
> >
> >       (qemu) virtio queue-element/machine/peripheral-anon/device[3]/virtio-backend 0
> >       index:  67
> >       ndescs: 1
> >       descs:  addr 0x6fe69800 len 1536 (write-only)
> >
> >       (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': 'virtio-queue-element',
> >       'data': { 'path': 'str', 'queue': 'uint16', '*index': 'uint16' },
> >       'returns': 'VirtioQueueElement'
> >     }
> >
> >   Example:
> >
> >   -> { "execute": "virtio-queue-element",
> >        "arguments": {
> >            "path": "/machine/peripheral-anon/device[3]/virtio-backend",
> >            "queue": 0
> >        }
> >     }
> >   -> { "return": {
> >           "index": 109,
> >           "len": 0,
> >           "ndescs": 1,
> >           "descs": [
> >               { "flags": 2, "len": 2048, "addr": 853145600 }
> >           ]
> >        }
> >     }
> >
> > Laurent Vivier (5):
> >   qmp: add QMP command query-virtio
> >   qmp: add QMP command virtio-status
> >   qmp: add QMP command virtio-queue-status
> >   qmp: add QMP command virtio-queue-element
> >   hmp: add virtio commands
> >
> >  Makefile                   |   2 +-
> >  Makefile.target            |   7 +-
> >  docs/system/monitor.rst    |   2 +
> >  hmp-commands-virtio.hx     | 148 ++++++++++++++++++
> >  hmp-commands.hx            |  10 ++
> >  hw/virtio/Makefile.objs    |   2 +
> >  hw/virtio/virtio-stub.c    |  32 ++++
> >  hw/virtio/virtio.c         | 313 +++++++++++++++++++++++++++++++++++++
> >  include/hw/virtio/virtio.h |   1 +
> >  include/monitor/hmp.h      |   4 +
> >  monitor/misc.c             |  17 ++
> >  qapi/Makefile.objs         |   2 +-
> >  qapi/qapi-schema.json      |   1 +
> >  qapi/virtio.json           | 302 +++++++++++++++++++++++++++++++++++
> >  tests/qtest/qmp-cmd-test.c |   1 +
> >  15 files changed, 840 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.25.1
> >
> >
> 
> 
> -- 
> Marc-André Lureau
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

end of thread, other threads:[~2020-04-02 15:24 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-02 10:02 [RFC 0/5] hmp,qmp: Add some commands to introspect virtio devices Laurent Vivier
2020-04-02 10:02 ` [RFC 1/5] qmp: add QMP command query-virtio Laurent Vivier
2020-04-02 14:23   ` Eric Blake
2020-04-02 10:02 ` [RFC 2/5] qmp: add QMP command virtio-status Laurent Vivier
2020-04-02 14:27   ` Eric Blake
2020-04-02 10:03 ` [RFC 3/5] qmp: add QMP command virtio-queue-status Laurent Vivier
2020-04-02 14:30   ` Eric Blake
2020-04-02 10:03 ` [RFC 4/5] qmp: add QMP command virtio-queue-element Laurent Vivier
2020-04-02 14:31   ` Eric Blake
2020-04-02 14:58   ` Dr. David Alan Gilbert
2020-04-02 10:03 ` [RFC 5/5] hmp: add virtio commands Laurent Vivier
2020-04-02 15:08   ` Dr. David Alan Gilbert
2020-04-02 14:33 ` [RFC 0/5] hmp, qmp: Add some commands to introspect virtio devices Marc-André Lureau
2020-04-02 15:23   ` Dr. David Alan Gilbert

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.