[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] qmp: update virtio feature maps, vhost-user-gpio inst
|
From: |
Michael S. Tsirkin |
|
Subject: |
Re: [PATCH v2 2/2] qmp: update virtio feature maps, vhost-user-gpio instrospection |
|
Date: |
Fri, 23 Jun 2023 01:43:06 -0400 |
On Fri, Jun 09, 2023 at 09:20:40AM -0400, Jonah Palmer 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
>
> virtio-bt:
> - VIRTIO_BT_F_VND_HCI
> - VIRTIO_BT_F_MSFT_EXT
> - VIRTIO_BT_F_AOSP_EXT
> - VIRTIO_BT_F_CONFIG_V2
>
> virtio-scmi:
> - VIRTIO_SCMI_F_P2A_CHANNELS
> - VIRTIO_SCMI_F_SHARED_MEMORY
>
> Add support for introspection on vhost-user-gpio devices.
>
> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
Thanks for the patch! Some comments:
> ---
> hw/virtio/vhost-user-gpio.c | 7 ++++
> hw/virtio/virtio-qmp.c | 79 +++++++++++++++++++++++++++++++++++--
> 2 files changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
> index d6927b610a..e88ca5370f 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 e936cc8ce5..140c420d87 100644
> --- a/hw/virtio/virtio-qmp.c
> +++ b/hw/virtio/virtio-qmp.c
> @@ -53,6 +53,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
> };
>
OMG I just realized that by now we have accumulated each value
in 4 places! This is really badly asking to be moved
to a header. Not sure what to do about the document yet
but that will at least get us down to two.
> @@ -79,6 +80,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 +137,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 statuses supported"),
status - there's only one per device
> { -1, "" }
> };
>
> @@ -176,6 +182,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 +307,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 +485,48 @@ static const qmp_virtio_feature_map_t
> virtio_rng_feature_map[] = {
> };
> #endif
>
> +/* virtio/vhost-gpio features mapping */
> +#ifdef CONFIG_VIRTIO_GPIO
Where's this coming from?
> +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
> +
> +/* virtio-bluetooth features mapping */
> +#ifdef CONFIG_VIRTIO_BT
Where's this coming from?
> +static const qmp_virtio_feature_map_t virtio_bt_feature_map[] = {
> + FEATURE_ENTRY(VIRTIO_BT_F_VND_HCI, \
> + "VIRTIO_BT_F_VND_HCI: Vendor command supported"),
> + FEATURE_ENTRY(VIRTIO_BT_F_MSFT_EXT, \
> + "VIRTIO_BT_F_MSFT_EXT: MSFT vendor supported"),
> + FEATURE_ENTRY(VIRTIO_BT_F_AOSP_EXT, \
> + "VIRTIO_BT_F_AOSP_EXT: AOSP vendor supported"),
> + FEATURE_ENTRY(VIRTIO_BT_F_CONFIG_V2, \
> + "VIRTIO_BT_F_CONFIG_V2: Using v2 configuration"),
> + { -1, "" }
> +};
> +#endif
> +
> +/* virtio-scmi features mapping */
> +#ifdef CONFIG_VIRTIO_SCMI
Where's this coming from?
> +static const qmp_virtio_feature_map_t virtio_scmi_feature_map[] = {
> + FEATURE_ENTRY(VIRTIO_SCMI_F_P2A_CHANNELS, \
> + "VIRTIO_SCMI_F_P2A_CHANNELS: SCMI notifications or delayed "
> + "responses implemented"),
> + FEATURE_ENTRY(VIRTIO_SCMI_F_SHARED_MEMORY, \
> + "VIRTIO_SCMI_F_SHARED_MEMORY: SCMI shared memory region
> statistics "
> + "implemented"),
> + { -1, "" }
> +};
> +#endif
> +
> #define CONVERT_FEATURES(type, map, is_status, bitmap) \
> ({ \
> type *list = NULL; \
> @@ -625,6 +683,24 @@ 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_VIRTIO_GPIO
> + case VIRTIO_ID_GPIO:
> + features->dev_features =
> + CONVERT_FEATURES(strList, virtio_gpio_feature_map, 0, bitmap);
> + break;
> +#endif
> +#ifdef CONFIG_VIRTIO_BT
> + case VIRTIO_ID_BT:
> + features->dev_features =
> + CONVERT_FEATURES(strList, virtio_bt_feature_map, 0, bitmap);
> + break;
> +#endif
> +#ifdef CONFIG_VIRTIO_SCMI
> + case VIRTIO_ID_SCMI:
> + features->dev_features =
> + CONVERT_FEATURES(strList, virtio_scmi_feature_map, 0, bitmap);
> + break;
> #endif
> /* No features */
> case VIRTIO_ID_9P:
> @@ -640,18 +716,15 @@ VirtioDeviceFeatures *qmp_decode_features(uint16_t
> device_id, uint64_t bitmap)
> case VIRTIO_ID_SIGNAL_DIST:
> case VIRTIO_ID_PSTORE:
> case VIRTIO_ID_SOUND:
> - case VIRTIO_ID_BT:
> case VIRTIO_ID_RPMB:
> case VIRTIO_ID_VIDEO_ENCODER:
> case VIRTIO_ID_VIDEO_DECODER:
> - case VIRTIO_ID_SCMI:
> case VIRTIO_ID_NITRO_SEC_MOD:
> case VIRTIO_ID_WATCHDOG:
> case VIRTIO_ID_CAN:
> 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