[PATCH v3 0/3] qmp, vhost-user: Remove virtio_list & update virtio introspection

[PATCH v3 0/3] qmp, vhost-user: Remove virtio_list & update virtio introspection

[Jonah Palmer]

These patches update a few things related to virtio introspection via.
QMP/HMP commands.

1. Remove 'virtio_list' and instead query the QOM composition tree to
find any active & realized virtio devices.

The 'virtio_list' was duplicating information about virtio devices that
was already available in the QOM composition tree, so there was no need
to keep this list.

2. Add new transport, protocol, and device features as well as support
to introspect vhost-user-gpio devices.

Vhost-user-gpio previously had no support for introspection. Support for
introspecting its vhost-user device is now available in these patches.

3. Move VhostUserProtocolFeature definition to its corresponding header
file (vhost-user.h). Cleanup previous definitions in other files.

VhostUserProtocolFeature was being defined in 3 separate files. Instead
of 3 separate definitions, use one instead and add it to the
vhost-user.h header file.

New virtio transport feature:
-----------------------------
 - VIRTIO_F_RING_RESET

New vhost-user protocol feature:
--------------------------------
 - VHOST_USER_PROTOCOL_F_STATUS

New virtio device features:
---------------------------
virtio-blk:
 - VIRTIO_BLK_F_SECURE_ERASE

virtio-net:
 - VIRTIO_NET_F_NOTF_COAL
 - VIRTIO_NET_F_GUEST_USO4
 - VIRTIO_NET_F_GUEST_USO6
 - VIRTIO_NET_F_HOST_USO

virtio/vhost-user-gpio:
 - VIRTIO_GPIO_F_IRQ
 - VHOST_F_LOG_ALL
 - VHOST_USER_F_PROTOCOL_FEATURES

v3: use recursion and type casting to find realized virtio devices
    remove virtio scmi & bluetooth feature mappings
    revert virtio scmi & bluetooth case changes in qmp_decode_features
    change config define for VIRTIO_GPIO to CONFIG_VHOST_USER_GPIO
    move VhostUserProtocolFeature definition to header file

v2: verify virtio devices via. 'TYPE_VIRTIO_DEVICES'
    verify path is a virtio device before checking if it's realized
    remove 'VIRTIO_BLK_F_ZONED' update (already exists)
    add cover letter

Jonah Palmer (3):
  qmp: remove virtio_list, search QOM tree instead
  qmp: update virtio feature maps, vhost-user-gpio introspection
  vhost-user: move VhostUserProtocolFeature definition to header file

 hw/scsi/vhost-user-scsi.c      |   4 -
 hw/virtio/vhost-user-gpio.c    |   7 ++
 hw/virtio/vhost-user.c         |  21 -----
 hw/virtio/virtio-qmp.c         | 154 ++++++++++++++++-----------------
 hw/virtio/virtio-qmp.h         |   7 --
 hw/virtio/virtio.c             |   6 --
 include/hw/virtio/vhost-user.h |  21 +++++
 7 files changed, 105 insertions(+), 115 deletions(-)

-- 
2.39.3

[PATCH v3 1/3] qmp: remove virtio_list, search QOM tree instead

[Jonah Palmer]

The virtio_list duplicates information about virtio devices that already
exist in the QOM composition tree. Instead of creating this list of
realized virtio devices, search the QOM composition tree instead.

This patch modifies the QMP command qmp_x_query_virtio to instead
recursively search the QOM composition tree for devices of type
'TYPE_VIRTIO_DEVICE'. The device is also checked to ensure it's
realized.

[Jonah: In the previous commit the qmp_x_query_virtio function was
 iterating through devices found via. qmp_qom_list and appending
 "/virtio-backend" to devices' paths to check if they were a virtio
 device.

 This method was messy and involved unneeded string manipulation.

 Instead, we can use recursion with object_get_root to iterate through
 all parent and child device paths to find virtio devices.

 The qmp_find_virtio_device function was also updated to simplify the
 method of determining if a path is to a valid and realized virtio
 device.]

Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
---
 hw/virtio/virtio-qmp.c | 96 ++++++++++++++++++------------------------
 hw/virtio/virtio-qmp.h |  7 ---
 hw/virtio/virtio.c     |  6 ---
 3 files changed, 40 insertions(+), 69 deletions(-)

diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index 3d32dbec8d..baec351c4f 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -665,70 +665,54 @@ VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap)
     return features;
 }
 
-VirtioInfoList *qmp_x_query_virtio(Error **errp)
+static int query_dev_child(Object *child, void *opaque)
 {
-    VirtioInfoList *list = NULL;
-    VirtioInfo *node;
-    VirtIODevice *vdev;
+    VirtioInfoList **vdevs = opaque;
+    Object *dev = object_dynamic_cast(child, TYPE_VIRTIO_DEVICE);
+    if (dev != NULL && DEVICE(dev)->realized) {
+        VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+
+        VirtioInfo *info = g_new(VirtioInfo, 1);
+
+        /* Get canonical path of device */
+        gchar *path = object_get_canonical_path(dev);
+
+        info->path = g_strdup(path);
+        info->name = g_strdup(vdev->name);
+        QAPI_LIST_PREPEND(*vdevs, info);
 
-    QTAILQ_FOREACH(vdev, &virtio_list, next) {
-        DeviceState *dev = DEVICE(vdev);
-        Error *err = NULL;
-        QObject *obj = qmp_qom_get(dev->canonical_path, "realized", &err);
-
-        if (err == NULL) {
-            GString *is_realized = qobject_to_json_pretty(obj, true);
-            /* virtio device is NOT realized, remove it from list */
-            if (!strncmp(is_realized->str, "false", 4)) {
-                QTAILQ_REMOVE(&virtio_list, vdev, next);
-            } else {
-                node = g_new(VirtioInfo, 1);
-                node->path = g_strdup(dev->canonical_path);
-                node->name = g_strdup(vdev->name);
-                QAPI_LIST_PREPEND(list, node);
-            }
-           g_string_free(is_realized, true);
-        }
-        qobject_unref(obj);
+        g_free(path);
+    } else {
+        object_unref(dev);
     }
 
-    return list;
+    return 0;
 }
 
-VirtIODevice *qmp_find_virtio_device(const char *path)
+VirtioInfoList *qmp_x_query_virtio(Error **errp)
 {
-    VirtIODevice *vdev;
+    VirtioInfoList *vdevs = NULL;
 
-    QTAILQ_FOREACH(vdev, &virtio_list, next) {
-        DeviceState *dev = DEVICE(vdev);
-
-        if (strcmp(dev->canonical_path, path) != 0) {
-            continue;
-        }
-
-        Error *err = NULL;
-        QObject *obj = qmp_qom_get(dev->canonical_path, "realized", &err);
-        if (err == NULL) {
-            GString *is_realized = qobject_to_json_pretty(obj, true);
-            /* virtio device is NOT realized, remove it from list */
-            if (!strncmp(is_realized->str, "false", 4)) {
-                g_string_free(is_realized, true);
-                qobject_unref(obj);
-                QTAILQ_REMOVE(&virtio_list, vdev, next);
-                return NULL;
-            }
-            g_string_free(is_realized, true);
-        } else {
-            /* virtio device doesn't exist in QOM tree */
-            QTAILQ_REMOVE(&virtio_list, vdev, next);
-            qobject_unref(obj);
-            return NULL;
-        }
-        /* device exists in QOM tree & is realized */
-        qobject_unref(obj);
-        return vdev;
+    /* Query the QOM composition tree recursively for virtio devices */
+    object_child_foreach_recursive(object_get_root(), query_dev_child, &vdevs);
+    if (vdevs == NULL) {
+        error_setg(errp, "No virtio devices found");
+        return NULL;
     }
-    return NULL;
+    return vdevs;
+}
+
+VirtIODevice *qmp_find_virtio_device(const char *path)
+{
+    /* Verify the canonical path is a realized virtio device */
+    Object *dev = object_dynamic_cast(object_resolve_path(path, NULL),
+                                      TYPE_VIRTIO_DEVICE);
+    if (!dev || !DEVICE(dev)->realized) {
+        object_unref(dev);
+        return NULL;
+    }
+
+    return VIRTIO_DEVICE(dev);
 }
 
 VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
@@ -738,7 +722,7 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
 
     vdev = qmp_find_virtio_device(path);
     if (vdev == NULL) {
-        error_setg(errp, "Path %s is not a VirtIODevice", path);
+        error_setg(errp, "Path %s is not a realized VirtIODevice", path);
         return NULL;
     }
 
diff --git a/hw/virtio/virtio-qmp.h b/hw/virtio/virtio-qmp.h
index 8af5f5e65a..245a446a56 100644
--- a/hw/virtio/virtio-qmp.h
+++ b/hw/virtio/virtio-qmp.h
@@ -15,13 +15,6 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/vhost.h"
 
-#include "qemu/queue.h"
-
-typedef QTAILQ_HEAD(QmpVirtIODeviceList, VirtIODevice) QmpVirtIODeviceList;
-
-/* QAPI list of realized VirtIODevices */
-extern QmpVirtIODeviceList virtio_list;
-
 VirtIODevice *qmp_find_virtio_device(const char *path);
 VirtioDeviceStatus *qmp_decode_status(uint8_t bitmap);
 VhostDeviceProtocols *qmp_decode_protocols(uint64_t bitmap);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 295a603e58..83c5db3d26 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -45,8 +45,6 @@
 #include "standard-headers/linux/virtio_mem.h"
 #include "standard-headers/linux/virtio_vsock.h"
 
-QmpVirtIODeviceList virtio_list;
-
 /*
  * Maximum size of virtio device config space
  */
@@ -3616,7 +3614,6 @@ static void virtio_device_realize(DeviceState *dev, Error **errp)
     vdev->listener.commit = virtio_memory_listener_commit;
     vdev->listener.name = "virtio";
     memory_listener_register(&vdev->listener, vdev->dma_as);
-    QTAILQ_INSERT_TAIL(&virtio_list, vdev, next);
 }
 
 static void virtio_device_unrealize(DeviceState *dev)
@@ -3631,7 +3628,6 @@ static void virtio_device_unrealize(DeviceState *dev)
         vdc->unrealize(dev);
     }
 
-    QTAILQ_REMOVE(&virtio_list, vdev, next);
     g_free(vdev->bus_name);
     vdev->bus_name = NULL;
 }
@@ -3805,8 +3801,6 @@ 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)
-- 
2.39.3

Re: [PATCH v3 1/3] qmp: remove virtio_list, search QOM tree instead

[Daniel P. Berrangé]

On Thu, Aug 03, 2023 at 10:54:58AM -0400, Jonah Palmer wrote:
> The virtio_list duplicates information about virtio devices that already
> exist in the QOM composition tree. Instead of creating this list of
> realized virtio devices, search the QOM composition tree instead.
> 
> This patch modifies the QMP command qmp_x_query_virtio to instead
> recursively search the QOM composition tree for devices of type
> 'TYPE_VIRTIO_DEVICE'. The device is also checked to ensure it's
> realized.
> 
> [Jonah: In the previous commit the qmp_x_query_virtio function was
>  iterating through devices found via. qmp_qom_list and appending
>  "/virtio-backend" to devices' paths to check if they were a virtio
>  device.
> 
>  This method was messy and involved unneeded string manipulation.
> 
>  Instead, we can use recursion with object_get_root to iterate through
>  all parent and child device paths to find virtio devices.
> 
>  The qmp_find_virtio_device function was also updated to simplify the
>  method of determining if a path is to a valid and realized virtio
>  device.]

FWIW, this "history" would typically go after the '---' but before
the diffstat, as it is relevant to reviewers of this new v3, but
doesn't need to get into the permanent git log once merged.

> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> ---
>  hw/virtio/virtio-qmp.c | 96 ++++++++++++++++++------------------------
>  hw/virtio/virtio-qmp.h |  7 ---
>  hw/virtio/virtio.c     |  6 ---
>  3 files changed, 40 insertions(+), 69 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 1/3] qmp: remove virtio_list, search QOM tree instead

[Jonah Palmer]

[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]

On 8/3/23 11:05, Daniel P. Berrangé wrote:

> On Thu, Aug 03, 2023 at 10:54:58AM -0400, Jonah Palmer wrote:
>> The virtio_list duplicates information about virtio devices that already
>> exist in the QOM composition tree. Instead of creating this list of
>> realized virtio devices, search the QOM composition tree instead.
>>
>> This patch modifies the QMP command qmp_x_query_virtio to instead
>> recursively search the QOM composition tree for devices of type
>> 'TYPE_VIRTIO_DEVICE'. The device is also checked to ensure it's
>> realized.
>>
>> [Jonah: In the previous commit the qmp_x_query_virtio function was
>>   iterating through devices found via. qmp_qom_list and appending
>>   "/virtio-backend" to devices' paths to check if they were a virtio
>>   device.
>>
>>   This method was messy and involved unneeded string manipulation.
>>
>>   Instead, we can use recursion with object_get_root to iterate through
>>   all parent and child device paths to find virtio devices.
>>
>>   The qmp_find_virtio_device function was also updated to simplify the
>>   method of determining if a path is to a valid and realized virtio
>>   device.]
> FWIW, this "history" would typically go after the '---' but before
> the diffstat, as it is relevant to reviewers of this new v3, but
> doesn't need to get into the permanent git log once merged.

Yes, you're right, my apologies. I'm still familiarizing myself with
these kinds of things. I will move this comment to the correct
location in v4 so it won't show up in the git log.

Thank you for the clarification!

>> Signed-off-by: Jonah Palmer<jonah.palmer@oracle.com>
>> ---
>>   hw/virtio/virtio-qmp.c | 96 ++++++++++++++++++------------------------
>>   hw/virtio/virtio-qmp.h |  7 ---
>>   hw/virtio/virtio.c     |  6 ---
>>   3 files changed, 40 insertions(+), 69 deletions(-)
> Reviewed-by: Daniel P. Berrangé<berrange@redhat.com>
>
>
> With regards,
> Daniel

[-- Attachment #2: Type: text/html, Size: 2780 bytes --]

Re: [PATCH v3 1/3] qmp: remove virtio_list, search QOM tree instead

[Manos Pitsidianakis]

On Thu, 03 Aug 2023 17:54, Jonah Palmer <jonah.palmer@oracle.com> wrote:
>-VirtioInfoList *qmp_x_query_virtio(Error **errp)
>+static int query_dev_child(Object *child, void *opaque)
> {
>-    VirtioInfoList *list = NULL;
>-    VirtioInfo *node;
>-    VirtIODevice *vdev;
>+    VirtioInfoList **vdevs = opaque;
>+    Object *dev = object_dynamic_cast(child, TYPE_VIRTIO_DEVICE);
>+    if (dev != NULL && DEVICE(dev)->realized) {
>+        VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>+
>+        VirtioInfo *info = g_new(VirtioInfo, 1);
>+
>+        /* Get canonical path of device */
>+        gchar *path = object_get_canonical_path(dev);

(You can use g_autofree char * here)

>+
>+        info->path = g_strdup(path);
>+        info->name = g_strdup(vdev->name);
>+        QAPI_LIST_PREPEND(*vdevs, info);
> 
>-    QTAILQ_FOREACH(vdev, &virtio_list, next) {
>-        DeviceState *dev = DEVICE(vdev);
>-        Error *err = NULL;
>-        QObject *obj = qmp_qom_get(dev->canonical_path, "realized", &err);
>-
>-        if (err == NULL) {
>-            GString *is_realized = qobject_to_json_pretty(obj, true);
>-            /* virtio device is NOT realized, remove it from list */
>-            if (!strncmp(is_realized->str, "false", 4)) {
>-                QTAILQ_REMOVE(&virtio_list, vdev, next);
>-            } else {
>-                node = g_new(VirtioInfo, 1);
>-                node->path = g_strdup(dev->canonical_path);
>-                node->name = g_strdup(vdev->name);
>-                QAPI_LIST_PREPEND(list, node);
>-            }
>-           g_string_free(is_realized, true);
>-        }
>-        qobject_unref(obj);
>+        g_free(path);
>+    } else {
>+        object_unref(dev);
>     }

The object_unref should not happen only in the else branch, no? Though 
it's not clear to me where the ref count was previously incremented.

>+    object_child_foreach_recursive(object_get_root(), query_dev_child, 
>&vdevs);
>+    if (vdevs == NULL) {
>+        error_setg(errp, "No virtio devices found");
>+        return NULL;

(No need for early return here)

>     }
>-    return NULL;
>+    return vdevs;
>+}
>+
>+VirtIODevice *qmp_find_virtio_device(const char *path)
>+{
>+    /* Verify the canonical path is a realized virtio device */
>+    Object *dev = object_dynamic_cast(object_resolve_path(path, NULL),
>+                                      TYPE_VIRTIO_DEVICE);
>+    if (!dev || !DEVICE(dev)->realized) {
>+        object_unref(dev);

Same as before with object refs

Re: [PATCH v3 1/3] qmp: remove virtio_list, search QOM tree instead

[Jonah Palmer]

[-- Attachment #1: Type: text/plain, Size: 3371 bytes --]


On 8/3/23 12:40, Manos Pitsidianakis wrote:
> On Thu, 03 Aug 2023 17:54, Jonah Palmer <jonah.palmer@oracle.com> wrote:
>> -VirtioInfoList *qmp_x_query_virtio(Error **errp)
>> +static int query_dev_child(Object *child, void *opaque)
>> {
>> -    VirtioInfoList *list = NULL;
>> -    VirtioInfo *node;
>> -    VirtIODevice *vdev;
>> +    VirtioInfoList **vdevs = opaque;
>> +    Object *dev = object_dynamic_cast(child, TYPE_VIRTIO_DEVICE);
>> +    if (dev != NULL && DEVICE(dev)->realized) {
>> +        VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>> +
>> +        VirtioInfo *info = g_new(VirtioInfo, 1);
>> +
>> +        /* Get canonical path of device */
>> +        gchar *path = object_get_canonical_path(dev);
>
> (You can use g_autofree char * here)
>
Got it, thanks. I'll use this and remove the g_free() call
as well.

>> +
>> +        info->path = g_strdup(path);
>> +        info->name = g_strdup(vdev->name);
>> +        QAPI_LIST_PREPEND(*vdevs, info);
>>
>> -    QTAILQ_FOREACH(vdev, &virtio_list, next) {
>> -        DeviceState *dev = DEVICE(vdev);
>> -        Error *err = NULL;
>> -        QObject *obj = qmp_qom_get(dev->canonical_path, "realized", 
>> &err);
>> -
>> -        if (err == NULL) {
>> -            GString *is_realized = qobject_to_json_pretty(obj, true);
>> -            /* virtio device is NOT realized, remove it from list */
>> -            if (!strncmp(is_realized->str, "false", 4)) {
>> -                QTAILQ_REMOVE(&virtio_list, vdev, next);
>> -            } else {
>> -                node = g_new(VirtioInfo, 1);
>> -                node->path = g_strdup(dev->canonical_path);
>> -                node->name = g_strdup(vdev->name);
>> -                QAPI_LIST_PREPEND(list, node);
>> -            }
>> -           g_string_free(is_realized, true);
>> -        }
>> -        qobject_unref(obj);
>> +        g_free(path);
>> +    } else {
>> +        object_unref(dev);
>>     }
>
> The object_unref should not happen only in the else branch, no? Though 
> it's not clear to me where the ref count was previously incremented.
>
There is no reference count being incremented, my apologies.
Therefore, there's no need to have this object_unref being
called.

I'll remove this and the one in the qmp_find_virtio_device
function below. Thanks for pointing this out.

>> + object_child_foreach_recursive(object_get_root(), query_dev_child, 
>> &vdevs);
>> +    if (vdevs == NULL) {
>> +        error_setg(errp, "No virtio devices found");
>> +        return NULL;
>
> (No need for early return here)
>
Good catch. Will remove. Thanks!

>>     }
>> -    return NULL;
>> +    return vdevs;
>> +}
>> +
>> +VirtIODevice *qmp_find_virtio_device(const char *path)
>> +{
>> +    /* Verify the canonical path is a realized virtio device */
>> +    Object *dev = object_dynamic_cast(object_resolve_path(path, NULL),
>> +                                      TYPE_VIRTIO_DEVICE);
>> +    if (!dev || !DEVICE(dev)->realized) {
>> +        object_unref(dev);
>
> Same as before with object refs

[-- Attachment #2: Type: text/html, Size: 7691 bytes --]

[PATCH v3 2/3] qmp: update virtio feature maps, vhost-user-gpio introspection

[Jonah Palmer]

Add new virtio transport feature to transport feature map:
 - VIRTIO_F_RING_RESET

Add new vhost-user protocol feature to vhost-user protocol feature map
and enumeration:
 - VHOST_USER_PROTOCOL_F_STATUS

Add new virtio device features for several virtio devices to their
respective feature mappings:

virtio-blk:
 - VIRTIO_BLK_F_SECURE_ERASE

virtio-net:
 - VIRTIO_NET_F_NOTF_COAL
 - VIRTIO_NET_F_GUEST_USO4
 - VIRTIO_NET_F_GUEST_USO6
 - VIRTIO_NET_F_HOST_USO

virtio/vhost-user-gpio:
 - VIRTIO_GPIO_F_IRQ
 - VHOST_F_LOG_ALL
 - VHOST_USER_F_PROTOCOL_FEATURES

Add support for introspection on vhost-user-gpio devices.

Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
---
 hw/virtio/vhost-user-gpio.c |  7 +++++++
 hw/virtio/virtio-qmp.c      | 38 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
index 3b013f2d0f..3d7fae3984 100644
--- a/hw/virtio/vhost-user-gpio.c
+++ b/hw/virtio/vhost-user-gpio.c
@@ -205,6 +205,12 @@ static void vu_gpio_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
     vhost_virtqueue_mask(&gpio->vhost_dev, vdev, idx, mask);
 }
 
+static struct vhost_dev *vu_gpio_get_vhost(VirtIODevice *vdev)
+{
+    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
+    return &gpio->vhost_dev;
+}
+
 static void do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserGPIO *gpio)
 {
     virtio_delete_queue(gpio->command_vq);
@@ -413,6 +419,7 @@ static void vu_gpio_class_init(ObjectClass *klass, void *data)
     vdc->get_config = vu_gpio_get_config;
     vdc->set_status = vu_gpio_set_status;
     vdc->guest_notifier_mask = vu_gpio_guest_notifier_mask;
+    vdc->get_vhost = vu_gpio_get_vhost;
 }
 
 static const TypeInfo vu_gpio_info = {
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index baec351c4f..a38b49af8a 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -30,6 +30,7 @@
 #include "standard-headers/linux/virtio_iommu.h"
 #include "standard-headers/linux/virtio_mem.h"
 #include "standard-headers/linux/virtio_vsock.h"
+#include "standard-headers/linux/virtio_gpio.h"
 
 #include CONFIG_DEVICES
 
@@ -53,6 +54,7 @@ enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
     VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
     VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
+    VHOST_USER_PROTOCOL_F_STATUS = 16,
     VHOST_USER_PROTOCOL_F_MAX
 };
 
@@ -79,6 +81,8 @@ static const qmp_virtio_feature_map_t virtio_transport_map[] = {
             "VIRTIO_F_ORDER_PLATFORM: Memory accesses ordered by platform"),
     FEATURE_ENTRY(VIRTIO_F_SR_IOV, \
             "VIRTIO_F_SR_IOV: Device supports single root I/O virtualization"),
+    FEATURE_ENTRY(VIRTIO_F_RING_RESET, \
+            "VIRTIO_F_RING_RESET: Driver can reset individual VQs"),
     /* Virtio ring transport features */
     FEATURE_ENTRY(VIRTIO_RING_F_INDIRECT_DESC, \
             "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported"),
@@ -134,6 +138,9 @@ static const qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
     FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS, \
             "VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS: Configuration for "
             "memory slots supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_STATUS, \
+            "VHOST_USER_PROTOCOL_F_STATUS: Querying and notifying back-end "
+            "device status supported"),
     { -1, "" }
 };
 
@@ -176,6 +183,8 @@ static const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
             "VIRTIO_BLK_F_DISCARD: Discard command supported"),
     FEATURE_ENTRY(VIRTIO_BLK_F_WRITE_ZEROES, \
             "VIRTIO_BLK_F_WRITE_ZEROES: Write zeroes command supported"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_SECURE_ERASE, \
+            "VIRTIO_BLK_F_SECURE_ERASE: Secure erase supported"),
     FEATURE_ENTRY(VIRTIO_BLK_F_ZONED, \
             "VIRTIO_BLK_F_ZONED: Zoned block devices"),
 #ifndef VIRTIO_BLK_NO_LEGACY
@@ -299,6 +308,14 @@ static const qmp_virtio_feature_map_t virtio_net_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_NET_F_CTRL_MAC_ADDR, \
             "VIRTIO_NET_F_CTRL_MAC_ADDR: MAC address set through control "
             "channel"),
+    FEATURE_ENTRY(VIRTIO_NET_F_NOTF_COAL, \
+            "VIRTIO_NET_F_NOTF_COAL: Device supports coalescing notifications"),
+    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_USO4, \
+            "VIRTIO_NET_F_GUEST_USO4: Driver can receive USOv4"),
+    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_USO6, \
+            "VIRTIO_NET_F_GUEST_USO4: Driver can receive USOv6"),
+    FEATURE_ENTRY(VIRTIO_NET_F_HOST_USO, \
+            "VIRTIO_NET_F_HOST_USO: Device can receive USO"),
     FEATURE_ENTRY(VIRTIO_NET_F_HASH_REPORT, \
             "VIRTIO_NET_F_HASH_REPORT: Hash reporting supported"),
     FEATURE_ENTRY(VIRTIO_NET_F_RSS, \
@@ -469,6 +486,20 @@ static const qmp_virtio_feature_map_t virtio_rng_feature_map[] = {
 };
 #endif
 
+/* virtio/vhost-gpio features mapping */
+#ifdef CONFIG_VHOST_USER_GPIO
+static const qmp_virtio_feature_map_t virtio_gpio_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_GPIO_F_IRQ, \
+            "VIRTIO_GPIO_F_IRQ: Device supports interrupts on GPIO lines"),
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+#endif
+
 #define CONVERT_FEATURES(type, map, is_status, bitmap)   \
     ({                                                   \
         type *list = NULL;                               \
@@ -625,6 +656,12 @@ VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap)
         features->dev_features =
             CONVERT_FEATURES(strList, virtio_rng_feature_map, 0, bitmap);
         break;
+#endif
+#ifdef CONFIG_VHOST_USER_GPIO
+    case VIRTIO_ID_GPIO:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_gpio_feature_map, 0, bitmap);
+        break;
 #endif
     /* No features */
     case VIRTIO_ID_9P:
@@ -651,7 +688,6 @@ VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap)
     case VIRTIO_ID_DMABUF:
     case VIRTIO_ID_PARAM_SERV:
     case VIRTIO_ID_AUDIO_POLICY:
-    case VIRTIO_ID_GPIO:
         break;
     default:
         g_assert_not_reached();
-- 
2.39.3

Re: [PATCH v3 2/3] qmp: update virtio feature maps, vhost-user-gpio introspection

[Manos Pitsidianakis]

On Thu, 03 Aug 2023 17:54, Jonah Palmer <jonah.palmer@oracle.com> wrote:
>Add new virtio transport feature to transport feature map:
> - VIRTIO_F_RING_RESET
>
>Add new vhost-user protocol feature to vhost-user protocol feature map
>and enumeration:
> - VHOST_USER_PROTOCOL_F_STATUS
>
>Add new virtio device features for several virtio devices to their
>respective feature mappings:
>
>virtio-blk:
> - VIRTIO_BLK_F_SECURE_ERASE
>
>virtio-net:
> - VIRTIO_NET_F_NOTF_COAL
> - VIRTIO_NET_F_GUEST_USO4
> - VIRTIO_NET_F_GUEST_USO6
> - VIRTIO_NET_F_HOST_USO
>
>virtio/vhost-user-gpio:
> - VIRTIO_GPIO_F_IRQ
> - VHOST_F_LOG_ALL
> - VHOST_USER_F_PROTOCOL_FEATURES
>
>Add support for introspection on vhost-user-gpio devices.
>
>Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>

Reviewed-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>

[PATCH v3 3/3] vhost-user: move VhostUserProtocolFeature definition to header file

[Jonah Palmer]

Move the definition of VhostUserProtocolFeature to
include/hw/virtio/vhost-user.h.

Remove previous definitions in hw/scsi/vhost-user-scsi.c,
hw/virtio/vhost-user.c, and hw/virtio/virtio-qmp.c.

Previously there were 3 separate definitions of this over 3 different
files. Now only 1 definition of this will be present for these 3 files.

Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
---
 hw/scsi/vhost-user-scsi.c      |  4 ----
 hw/virtio/vhost-user.c         | 21 ---------------------
 hw/virtio/virtio-qmp.c         | 22 +---------------------
 include/hw/virtio/vhost-user.h | 21 +++++++++++++++++++++
 4 files changed, 22 insertions(+), 46 deletions(-)

diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index ee99b19e7a..df6b66cc1a 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -39,10 +39,6 @@ static const int user_feature_bits[] = {
     VHOST_INVALID_FEATURE_BIT
 };
 
-enum VhostUserProtocolFeature {
-    VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
-};
-
 static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status)
 {
     VHostUserSCSI *s = (VHostUserSCSI *)vdev;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8dcf049d42..a096335921 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -56,27 +56,6 @@
  */
 #define VHOST_USER_MAX_CONFIG_SIZE 256
 
-enum VhostUserProtocolFeature {
-    VHOST_USER_PROTOCOL_F_MQ = 0,
-    VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
-    VHOST_USER_PROTOCOL_F_RARP = 2,
-    VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
-    VHOST_USER_PROTOCOL_F_NET_MTU = 4,
-    VHOST_USER_PROTOCOL_F_BACKEND_REQ = 5,
-    VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
-    VHOST_USER_PROTOCOL_F_CRYPTO_SESSION = 7,
-    VHOST_USER_PROTOCOL_F_PAGEFAULT = 8,
-    VHOST_USER_PROTOCOL_F_CONFIG = 9,
-    VHOST_USER_PROTOCOL_F_BACKEND_SEND_FD = 10,
-    VHOST_USER_PROTOCOL_F_HOST_NOTIFIER = 11,
-    VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD = 12,
-    VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
-    /* Feature 14 reserved for VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS. */
-    VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
-    VHOST_USER_PROTOCOL_F_STATUS = 16,
-    VHOST_USER_PROTOCOL_F_MAX
-};
-
 #define VHOST_USER_PROTOCOL_FEATURE_MASK ((1 << VHOST_USER_PROTOCOL_F_MAX) - 1)
 
 typedef enum VhostUserRequest {
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index a38b49af8a..69880fe7c5 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -17,6 +17,7 @@
 #include "qapi/qapi-commands-qom.h"
 #include "qapi/qmp/qobject.h"
 #include "qapi/qmp/qjson.h"
+#include "hw/virtio/vhost-user.h"
 
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/vhost_types.h"
@@ -37,27 +38,6 @@
 #define FEATURE_ENTRY(name, desc) (qmp_virtio_feature_map_t) \
     { .virtio_bit = name, .feature_desc = desc }
 
-enum VhostUserProtocolFeature {
-    VHOST_USER_PROTOCOL_F_MQ = 0,
-    VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
-    VHOST_USER_PROTOCOL_F_RARP = 2,
-    VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
-    VHOST_USER_PROTOCOL_F_NET_MTU = 4,
-    VHOST_USER_PROTOCOL_F_BACKEND_REQ = 5,
-    VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
-    VHOST_USER_PROTOCOL_F_CRYPTO_SESSION = 7,
-    VHOST_USER_PROTOCOL_F_PAGEFAULT = 8,
-    VHOST_USER_PROTOCOL_F_CONFIG = 9,
-    VHOST_USER_PROTOCOL_F_BACKEND_SEND_FD = 10,
-    VHOST_USER_PROTOCOL_F_HOST_NOTIFIER = 11,
-    VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD = 12,
-    VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
-    VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
-    VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
-    VHOST_USER_PROTOCOL_F_STATUS = 16,
-    VHOST_USER_PROTOCOL_F_MAX
-};
-
 /* Virtio transport features mapping */
 static const qmp_virtio_feature_map_t virtio_transport_map[] = {
     /* Virtio device transport features */
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index 191216a74f..80e2b4a463 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -11,6 +11,27 @@
 #include "chardev/char-fe.h"
 #include "hw/virtio/virtio.h"
 
+enum VhostUserProtocolFeature {
+    VHOST_USER_PROTOCOL_F_MQ = 0,
+    VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
+    VHOST_USER_PROTOCOL_F_RARP = 2,
+    VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
+    VHOST_USER_PROTOCOL_F_NET_MTU = 4,
+    VHOST_USER_PROTOCOL_F_BACKEND_REQ = 5,
+    VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
+    VHOST_USER_PROTOCOL_F_CRYPTO_SESSION = 7,
+    VHOST_USER_PROTOCOL_F_PAGEFAULT = 8,
+    VHOST_USER_PROTOCOL_F_CONFIG = 9,
+    VHOST_USER_PROTOCOL_F_BACKEND_SEND_FD = 10,
+    VHOST_USER_PROTOCOL_F_HOST_NOTIFIER = 11,
+    VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD = 12,
+    VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
+    VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
+    VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
+    VHOST_USER_PROTOCOL_F_STATUS = 16,
+    VHOST_USER_PROTOCOL_F_MAX
+};
+
 /**
  * VhostUserHostNotifier - notifier information for one queue
  * @rcu: rcu_head for cleanup
-- 
2.39.3

Re: [PATCH v3 3/3] vhost-user: move VhostUserProtocolFeature definition to header file

[Manos Pitsidianakis]

On Thu, 03 Aug 2023 17:55, Jonah Palmer <jonah.palmer@oracle.com> wrote:
>Move the definition of VhostUserProtocolFeature to
>include/hw/virtio/vhost-user.h.
>
>Remove previous definitions in hw/scsi/vhost-user-scsi.c,
>hw/virtio/vhost-user.c, and hw/virtio/virtio-qmp.c.
>
>Previously there were 3 separate definitions of this over 3 different
>files. Now only 1 definition of this will be present for these 3 files.
>
>Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>

Reviewed-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>

Re: [PATCH v3 0/3] qmp,vhost-user: Remove virtio_list & update virtio introspection

[Michael S. Tsirkin]

On Thu, Aug 03, 2023 at 10:54:57AM -0400, Jonah Palmer wrote:
> These patches update a few things related to virtio introspection via.
> QMP/HMP commands.
> 
> 1. Remove 'virtio_list' and instead query the QOM composition tree to
> find any active & realized virtio devices.
> 
> The 'virtio_list' was duplicating information about virtio devices that
> was already available in the QOM composition tree, so there was no need
> to keep this list.
> 
> 2. Add new transport, protocol, and device features as well as support
> to introspect vhost-user-gpio devices.
> 
> Vhost-user-gpio previously had no support for introspection. Support for
> introspecting its vhost-user device is now available in these patches.
> 
> 3. Move VhostUserProtocolFeature definition to its corresponding header
> file (vhost-user.h). Cleanup previous definitions in other files.
> 
> VhostUserProtocolFeature was being defined in 3 separate files. Instead
> of 3 separate definitions, use one instead and add it to the
> vhost-user.h header file.
> 
> New virtio transport feature:
> -----------------------------
>  - VIRTIO_F_RING_RESET
> 
> New vhost-user protocol feature:
> --------------------------------
>  - VHOST_USER_PROTOCOL_F_STATUS
> 
> New virtio device features:
> ---------------------------
> virtio-blk:
>  - VIRTIO_BLK_F_SECURE_ERASE
> 
> virtio-net:
>  - VIRTIO_NET_F_NOTF_COAL
>  - VIRTIO_NET_F_GUEST_USO4
>  - VIRTIO_NET_F_GUEST_USO6
>  - VIRTIO_NET_F_HOST_USO
> 
> virtio/vhost-user-gpio:
>  - VIRTIO_GPIO_F_IRQ
>  - VHOST_F_LOG_ALL
>  - VHOST_USER_F_PROTOCOL_FEATURES

VHOST_F_LOG_ALL likely does not make sense.

> v3: use recursion and type casting to find realized virtio devices
>     remove virtio scmi & bluetooth feature mappings
>     revert virtio scmi & bluetooth case changes in qmp_decode_features
>     change config define for VIRTIO_GPIO to CONFIG_VHOST_USER_GPIO
>     move VhostUserProtocolFeature definition to header file
> 
> v2: verify virtio devices via. 'TYPE_VIRTIO_DEVICES'
>     verify path is a virtio device before checking if it's realized
>     remove 'VIRTIO_BLK_F_ZONED' update (already exists)
>     add cover letter
> 
> Jonah Palmer (3):
>   qmp: remove virtio_list, search QOM tree instead
>   qmp: update virtio feature maps, vhost-user-gpio introspection
>   vhost-user: move VhostUserProtocolFeature definition to header file
> 
>  hw/scsi/vhost-user-scsi.c      |   4 -
>  hw/virtio/vhost-user-gpio.c    |   7 ++
>  hw/virtio/vhost-user.c         |  21 -----
>  hw/virtio/virtio-qmp.c         | 154 ++++++++++++++++-----------------
>  hw/virtio/virtio-qmp.h         |   7 --
>  hw/virtio/virtio.c             |   6 --
>  include/hw/virtio/vhost-user.h |  21 +++++
>  7 files changed, 105 insertions(+), 115 deletions(-)
> 
> -- 
> 2.39.3

Re: [PATCH v3 0/3] qmp, vhost-user: Remove virtio_list & update virtio introspection

[Jonah Palmer]

[-- Attachment #1: Type: text/plain, Size: 3230 bytes --]

On 8/3/23 15:50, Michael S. Tsirkin wrote:

> On Thu, Aug 03, 2023 at 10:54:57AM -0400, Jonah Palmer wrote:
>> These patches update a few things related to virtio introspection via.
>> QMP/HMP commands.
>>
>> 1. Remove 'virtio_list' and instead query the QOM composition tree to
>> find any active & realized virtio devices.
>>
>> The 'virtio_list' was duplicating information about virtio devices that
>> was already available in the QOM composition tree, so there was no need
>> to keep this list.
>>
>> 2. Add new transport, protocol, and device features as well as support
>> to introspect vhost-user-gpio devices.
>>
>> Vhost-user-gpio previously had no support for introspection. Support for
>> introspecting its vhost-user device is now available in these patches.
>>
>> 3. Move VhostUserProtocolFeature definition to its corresponding header
>> file (vhost-user.h). Cleanup previous definitions in other files.
>>
>> VhostUserProtocolFeature was being defined in 3 separate files. Instead
>> of 3 separate definitions, use one instead and add it to the
>> vhost-user.h header file.
>>
>> New virtio transport feature:
>> -----------------------------
>>   - VIRTIO_F_RING_RESET
>>
>> New vhost-user protocol feature:
>> --------------------------------
>>   - VHOST_USER_PROTOCOL_F_STATUS
>>
>> New virtio device features:
>> ---------------------------
>> virtio-blk:
>>   - VIRTIO_BLK_F_SECURE_ERASE
>>
>> virtio-net:
>>   - VIRTIO_NET_F_NOTF_COAL
>>   - VIRTIO_NET_F_GUEST_USO4
>>   - VIRTIO_NET_F_GUEST_USO6
>>   - VIRTIO_NET_F_HOST_USO
>>
>> virtio/vhost-user-gpio:
>>   - VIRTIO_GPIO_F_IRQ
>>   - VHOST_F_LOG_ALL
>>   - VHOST_USER_F_PROTOCOL_FEATURES
> VHOST_F_LOG_ALL likely does not make sense.
>
Ah, okay. I can remove this in v4.

I'm not very familiar with virtio/vhost-user-gpio other than that
it's a virtual general-purpose I/O controller that maps native
GPIOs to a VM, allowing the VM to perform GPIO operations via. it.

I wasn't sure if the vhost-user version of it would make use of
this feature.

>> v3: use recursion and type casting to find realized virtio devices
>>      remove virtio scmi & bluetooth feature mappings
>>      revert virtio scmi & bluetooth case changes in qmp_decode_features
>>      change config define for VIRTIO_GPIO to CONFIG_VHOST_USER_GPIO
>>      move VhostUserProtocolFeature definition to header file
>>
>> v2: verify virtio devices via. 'TYPE_VIRTIO_DEVICES'
>>      verify path is a virtio device before checking if it's realized
>>      remove 'VIRTIO_BLK_F_ZONED' update (already exists)
>>      add cover letter
>>
>> Jonah Palmer (3):
>>    qmp: remove virtio_list, search QOM tree instead
>>    qmp: update virtio feature maps, vhost-user-gpio introspection
>>    vhost-user: move VhostUserProtocolFeature definition to header file
>>
>>   hw/scsi/vhost-user-scsi.c      |   4 -
>>   hw/virtio/vhost-user-gpio.c    |   7 ++
>>   hw/virtio/vhost-user.c         |  21 -----
>>   hw/virtio/virtio-qmp.c         | 154 ++++++++++++++++-----------------
>>   hw/virtio/virtio-qmp.h         |   7 --
>>   hw/virtio/virtio.c             |   6 --
>>   include/hw/virtio/vhost-user.h |  21 +++++
>>   7 files changed, 105 insertions(+), 115 deletions(-)
>>
>> -- 
>> 2.39.3

[-- Attachment #2: Type: text/html, Size: 3773 bytes --]