From: "Michael S. Tsirkin" <mst@redhat.com>
To: Paolo Abeni <pabeni@redhat.com>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
Dmitry Fleytman <dmitry.fleytman@gmail.com>,
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>,
Jason Wang <jasowang@redhat.com>,
Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>,
Stefano Garzarella <sgarzare@redhat.com>,
Cornelia Huck <cohuck@redhat.com>,
Luigi Rizzo <lrizzo@google.com>,
Giuseppe Lettieri <g.lettieri@iet.unipi.it>,
Vincenzo Maffione <v.maffione@gmail.com>,
Eric Blake <eblake@redhat.com>,
Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH v6 09/14] qmp: update virtio features map to support extended features
Date: Mon, 22 Sep 2025 08:55:00 -0400 [thread overview]
Message-ID: <20250922085341-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <bef09bed78378da9c32d9913eefb83bd42ba8808.1757676218.git.pabeni@redhat.com>
On Fri, Sep 12, 2025 at 03:07:00PM +0200, Paolo Abeni wrote:
> Extend the VirtioDeviceFeatures struct with an additional u64
> to track unknown features in the 64-127 bit range and decode
> the full virtio features spaces for vhost and virtio devices.
>
> Also add entries for the soon-to-be-supported virtio net GSO over
> UDP features.
>
> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Acked-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v3 -> v4:
> - cleanup unknown features init
> - update QMP example and doc accordingly
> - use new virtio_features macro names
>
> v2 -> v3:
> - unknown-dev-features-dword2 -> unknown-dev-features2
> - _array -> _ex
> - fixed typos in entries description
>
> v1 -> v2:
> - uint128_t -> uint64_t[]
> ---
> hw/virtio/virtio-hmp-cmds.c | 3 +-
> hw/virtio/virtio-qmp.c | 91 +++++++++++++++++++++++++------------
> hw/virtio/virtio-qmp.h | 3 +-
> qapi/virtio.json | 9 +++-
> 4 files changed, 74 insertions(+), 32 deletions(-)
>
> diff --git a/hw/virtio/virtio-hmp-cmds.c b/hw/virtio/virtio-hmp-cmds.c
> index 7d8677bcf0..1daae482d3 100644
> --- a/hw/virtio/virtio-hmp-cmds.c
> +++ b/hw/virtio/virtio-hmp-cmds.c
> @@ -74,7 +74,8 @@ static void hmp_virtio_dump_features(Monitor *mon,
> }
>
> if (features->has_unknown_dev_features) {
> - monitor_printf(mon, " unknown-features(0x%016"PRIx64")\n",
> + monitor_printf(mon, " unknown-features(0x%016"PRIx64"%016"PRIx64")\n",
> + features->unknown_dev_features2,
> features->unknown_dev_features);
> }
> }
> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> index 3b6377cf0d..502c9ae930 100644
> --- a/hw/virtio/virtio-qmp.c
> +++ b/hw/virtio/virtio-qmp.c
> @@ -325,6 +325,20 @@ static const qmp_virtio_feature_map_t virtio_net_feature_map[] = {
> FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
> "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
> "negotiation supported"),
> + FEATURE_ENTRY(VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO, \
> + "VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO: Driver can receive GSO over "
> + "UDP tunnel packets"),
> + FEATURE_ENTRY(VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM, \
> + "VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO: Driver can receive GSO over "
This really should be VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM.
Given they all seem to start with repeating the feature name,
why not just add it to the string automatically by the macro?
> + "UDP tunnel packets requiring checksum offload for the outer "
> + "header"),
> + FEATURE_ENTRY(VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO, \
> + "VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO: Device can receive GSO over "
> + "UDP tunnel packets"),
> + FEATURE_ENTRY(VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM, \
> + "VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO: Device can receive GSO over "
> + "UDP tunnel packets requiring checksum offload for the outer "
> + "header"),
> { -1, "" }
> };
> #endif
> @@ -510,6 +524,24 @@ static const qmp_virtio_feature_map_t virtio_gpio_feature_map[] = {
> list; \
> })
>
> +#define CONVERT_FEATURES_EX(type, map, bitmap) \
> + ({ \
> + type *list = NULL; \
> + type *node; \
> + for (i = 0; map[i].virtio_bit != -1; i++) { \
> + bit = map[i].virtio_bit; \
> + if (!virtio_has_feature_ex(bitmap, bit)) { \
> + continue; \
> + } \
> + node = g_new0(type, 1); \
> + node->value = g_strdup(map[i].feature_desc); \
> + node->next = list; \
> + list = node; \
> + virtio_clear_feature_ex(bitmap, bit); \
> + } \
> + list; \
> + })
> +
> VirtioDeviceStatus *qmp_decode_status(uint8_t bitmap)
> {
> VirtioDeviceStatus *status;
> @@ -545,109 +577,112 @@ VhostDeviceProtocols *qmp_decode_protocols(uint64_t bitmap)
> return vhu_protocols;
> }
>
> -VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap)
> +VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id,
> + const uint64_t *bmap)
> {
> + uint64_t bitmap[VIRTIO_FEATURES_NU64S];
> VirtioDeviceFeatures *features;
> uint64_t bit;
> int i;
>
> + virtio_features_copy(bitmap, bmap);
> features = g_new0(VirtioDeviceFeatures, 1);
> features->has_dev_features = true;
>
> /* transport features */
> - features->transports = CONVERT_FEATURES(strList, virtio_transport_map, 0,
> - bitmap);
> + features->transports = CONVERT_FEATURES_EX(strList, virtio_transport_map,
> + bitmap);
>
> /* device features */
> switch (device_id) {
> #ifdef CONFIG_VIRTIO_SERIAL
> case VIRTIO_ID_CONSOLE:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_serial_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_serial_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_BLK
> case VIRTIO_ID_BLOCK:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_blk_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_blk_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_GPU
> case VIRTIO_ID_GPU:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_gpu_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_gpu_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_NET
> case VIRTIO_ID_NET:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_net_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_net_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_SCSI
> case VIRTIO_ID_SCSI:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_scsi_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_scsi_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_BALLOON
> case VIRTIO_ID_BALLOON:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_balloon_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_balloon_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_IOMMU
> case VIRTIO_ID_IOMMU:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_iommu_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_iommu_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_INPUT
> case VIRTIO_ID_INPUT:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_input_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_input_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VHOST_USER_FS
> case VIRTIO_ID_FS:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_fs_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_fs_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VHOST_VSOCK
> case VIRTIO_ID_VSOCK:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_vsock_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_vsock_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_CRYPTO
> case VIRTIO_ID_CRYPTO:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_crypto_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_crypto_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_MEM
> case VIRTIO_ID_MEM:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_mem_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_mem_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_I2C_ADAPTER
> case VIRTIO_ID_I2C_ADAPTER:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_i2c_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_i2c_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VIRTIO_RNG
> case VIRTIO_ID_RNG:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_rng_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_rng_feature_map, bitmap);
> break;
> #endif
> #ifdef CONFIG_VHOST_USER_GPIO
> case VIRTIO_ID_GPIO:
> features->dev_features =
> - CONVERT_FEATURES(strList, virtio_gpio_feature_map, 0, bitmap);
> + CONVERT_FEATURES_EX(strList, virtio_gpio_feature_map, bitmap);
> break;
> #endif
> /* No features */
> @@ -680,10 +715,9 @@ VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap)
> g_assert_not_reached();
> }
>
> - features->has_unknown_dev_features = bitmap != 0;
> - if (features->has_unknown_dev_features) {
> - features->unknown_dev_features = bitmap;
> - }
> + features->has_unknown_dev_features = !virtio_features_empty(bitmap);
> + features->unknown_dev_features = bitmap[0];
> + features->unknown_dev_features2 = bitmap[1];
>
> return features;
> }
> @@ -743,11 +777,11 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
> status->device_id = vdev->device_id;
> status->vhost_started = vdev->vhost_started;
> status->guest_features = qmp_decode_features(vdev->device_id,
> - vdev->guest_features);
> + vdev->guest_features_ex);
> status->host_features = qmp_decode_features(vdev->device_id,
> - vdev->host_features);
> + vdev->host_features_ex);
> status->backend_features = qmp_decode_features(vdev->device_id,
> - vdev->backend_features);
> + vdev->backend_features_ex);
>
> switch (vdev->device_endian) {
> case VIRTIO_DEVICE_ENDIAN_LITTLE:
> @@ -785,11 +819,12 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
> status->vhost_dev->nvqs = hdev->nvqs;
> status->vhost_dev->vq_index = hdev->vq_index;
> status->vhost_dev->features =
> - qmp_decode_features(vdev->device_id, hdev->features);
> + qmp_decode_features(vdev->device_id, hdev->features_ex);
> status->vhost_dev->acked_features =
> - qmp_decode_features(vdev->device_id, hdev->acked_features);
> + qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
> status->vhost_dev->backend_features =
> - qmp_decode_features(vdev->device_id, hdev->backend_features);
> + qmp_decode_features(vdev->device_id, hdev->backend_features_ex);
> +
> status->vhost_dev->protocol_features =
> qmp_decode_protocols(hdev->protocol_features);
> status->vhost_dev->max_queues = hdev->max_queues;
> diff --git a/hw/virtio/virtio-qmp.h b/hw/virtio/virtio-qmp.h
> index 245a446a56..e0a1e49035 100644
> --- a/hw/virtio/virtio-qmp.h
> +++ b/hw/virtio/virtio-qmp.h
> @@ -18,6 +18,7 @@
> VirtIODevice *qmp_find_virtio_device(const char *path);
> VirtioDeviceStatus *qmp_decode_status(uint8_t bitmap);
> VhostDeviceProtocols *qmp_decode_protocols(uint64_t bitmap);
> -VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap);
> +VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id,
> + const uint64_t *bitmap);
>
> #endif
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 9d652fe4a8..05295ab665 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -247,6 +247,7 @@
> # },
> # "host-features": {
> # "unknown-dev-features": 1073741824,
> +# "unknown-dev-features2": 0,
> # "dev-features": [],
> # "transports": [
> # "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled",
> @@ -490,14 +491,18 @@
> # unique features)
> #
> # @unknown-dev-features: Virtio device features bitmap that have not
> -# been decoded
> +# been decoded (bits 0-63)
> +#
> +# @unknown-dev-features2: Virtio device features bitmap that have not
> +# been decoded (bits 64-127) (since 10.2)
> #
> # Since: 7.2
> ##
> { 'struct': 'VirtioDeviceFeatures',
> 'data': { 'transports': [ 'str' ],
> '*dev-features': [ 'str' ],
> - '*unknown-dev-features': 'uint64' } }
> + '*unknown-dev-features': 'uint64',
> + '*unknown-dev-features2': 'uint64' } }
>
> ##
> # @VirtQueueStatus:
> --
> 2.51.0
next prev parent reply other threads:[~2025-09-22 12:55 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-12 13:06 [PATCH v6 00/14] virtio: introduce support for GSO over UDP tunnel Paolo Abeni
2025-09-12 13:06 ` [PATCH v6 01/14] net: bundle all offloads in a single struct Paolo Abeni
2025-09-12 13:06 ` [PATCH v6 02/14] linux-headers: deal with counted_by annotation Paolo Abeni
2025-09-17 12:10 ` Stefano Garzarella
2025-09-12 13:06 ` [PATCH v6 03/14] linux-headers: Update to Linux v6.17-rc1 Paolo Abeni
2025-09-12 13:06 ` [PATCH v6 04/14] virtio: introduce extended features type Paolo Abeni
2025-09-17 12:12 ` Stefano Garzarella
2025-09-17 12:24 ` Stefano Garzarella
2025-09-12 13:06 ` [PATCH v6 05/14] virtio: serialize extended features state Paolo Abeni
2025-09-17 12:13 ` Stefano Garzarella
2025-09-12 13:06 ` [PATCH v6 06/14] virtio: add support for negotiating extended features Paolo Abeni
2025-09-17 12:19 ` Stefano Garzarella
2025-09-12 13:06 ` [PATCH v6 07/14] virtio-pci: implement support for " Paolo Abeni
2025-09-17 12:25 ` Stefano Garzarella
2025-09-12 13:06 ` [PATCH v6 08/14] vhost: add support for negotiating " Paolo Abeni
2025-09-17 12:29 ` Stefano Garzarella
2025-09-12 13:07 ` [PATCH v6 09/14] qmp: update virtio features map to support " Paolo Abeni
2025-09-12 13:42 ` Markus Armbruster
2025-09-12 13:57 ` Paolo Abeni
2025-09-12 14:04 ` Markus Armbruster
2025-09-17 12:35 ` Stefano Garzarella
2025-09-22 12:55 ` Michael S. Tsirkin [this message]
2025-09-22 13:10 ` Paolo Abeni
2025-09-22 13:15 ` Michael S. Tsirkin
2025-09-22 13:28 ` Paolo Abeni
2025-09-12 13:07 ` [PATCH v6 10/14] vhost-backend: implement extended features support Paolo Abeni
2025-09-17 12:29 ` Stefano Garzarella
2025-09-12 13:07 ` [PATCH v6 11/14] vhost-net: " Paolo Abeni
2025-09-12 13:07 ` [PATCH v6 12/14] virtio-net: " Paolo Abeni
2025-09-12 13:07 ` [PATCH v6 13/14] net: implement tunnel probing Paolo Abeni
2025-09-17 12:36 ` Stefano Garzarella
2025-09-12 13:07 ` [PATCH v6 14/14] net: implement UDP tunnel features offloading Paolo Abeni
2025-09-15 14:39 ` [PATCH v6 00/14] virtio: introduce support for GSO over UDP tunnel Lei Yang
2025-09-19 9:09 ` Paolo Abeni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250922085341-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=armbru@redhat.com \
--cc=cohuck@redhat.com \
--cc=dmitry.fleytman@gmail.com \
--cc=eblake@redhat.com \
--cc=g.lettieri@iet.unipi.it \
--cc=jasowang@redhat.com \
--cc=lrizzo@google.com \
--cc=odaki@rsg.ci.i.u-tokyo.ac.jp \
--cc=pabeni@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=sgarzare@redhat.com \
--cc=sriram.yagnaraman@ericsson.com \
--cc=v.maffione@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.