From: Laurent Vivier <lvivier@redhat.com>
This new command shows internal status of a VirtQueue.
(vrings and indexes).
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
---
hw/virtio/virtio-stub.c | 6 +++
hw/virtio/virtio.c | 37 ++++++++++++++++++
qapi/virtio.json | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 145 insertions(+)
[Jonah: Added 'device-type' field to VirtQueueStatus and
qmp command x-debug-virtio-queue-status.]
diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
index ddb592f..3c1bf17 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -17,3 +17,9 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
{
return qmp_virtio_unsupported(errp);
}
+
+VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
+ uint16_t queue, Error **errp)
+{
+ return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 81a0ee8..ccd4371 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3935,6 +3935,43 @@ static VirtIODevice *virtio_device_find(const char *path)
return NULL;
}
+VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
+ uint16_t queue, Error **errp)
+{
+ VirtIODevice *vdev;
+ VirtQueueStatus *status;
+
+ vdev = virtio_device_find(path);
+ if (vdev == NULL) {
+ error_setg(errp, "Path %s is not a VirtIO device", path);
+ return NULL;
+ }
+
+ if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) {
+ error_setg(errp, "Invalid virtqueue number %d", queue);
+ return NULL;
+ }
+
+ status = g_new0(VirtQueueStatus, 1);
+ status->device_type = qapi_enum_parse(&VirtioType_lookup, vdev->name,
+ VIRTIO_TYPE_UNKNOWN, NULL);
+ status->queue_index = vdev->vq[queue].queue_index;
+ status->inuse = vdev->vq[queue].inuse;
+ status->vring_num = vdev->vq[queue].vring.num;
+ status->vring_num_default = vdev->vq[queue].vring.num_default;
+ status->vring_align = vdev->vq[queue].vring.align;
+ status->vring_desc = vdev->vq[queue].vring.desc;
+ status->vring_avail = vdev->vq[queue].vring.avail;
+ status->vring_used = vdev->vq[queue].vring.used;
+ status->last_avail_idx = vdev->vq[queue].last_avail_idx;
+ status->shadow_avail_idx = vdev->vq[queue].shadow_avail_idx;
+ status->used_idx = vdev->vq[queue].used_idx;
+ status->signalled_used = vdev->vq[queue].signalled_used;
+ status->signalled_used_valid = vdev->vq[queue].signalled_used_valid;
+
+ return status;
+}
+
#define CONVERT_FEATURES(type, map) \
({ \
type *list = NULL; \
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 78873cd..7007e0c 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -406,3 +406,105 @@
'data': { 'path': 'str' },
'returns': 'VirtioStatus'
}
+
+##
+# @VirtQueueStatus:
+#
+# Status of a VirtQueue
+#
+# @device-type: VirtIO device type
+#
+# @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: 6.1
+#
+##
+
+{ 'struct': 'VirtQueueStatus',
+ 'data': {
+ 'device-type': 'VirtioType',
+ 'queue-index': 'uint16',
+ 'inuse': 'uint32',
+ 'vring-num': 'int',
+ 'vring-num-default': 'int',
+ 'vring-align': 'int',
+ 'vring-desc': 'uint64',
+ 'vring-avail': 'uint64',
+ 'vring-used': 'uint64',
+ 'last-avail-idx': 'uint16',
+ 'shadow-avail-idx': 'uint16',
+ 'used-idx': 'uint16',
+ 'signalled-used': 'uint16',
+ 'signalled-used-valid': 'uint16'
+ }
+}
+
+##
+# @x-debug-virtio-queue-status:
+#
+# Return the status of a given VirtQueue
+#
+# @path: QOBject path of the VirtIODevice
+#
+# @queue: queue number to examine
+#
+# Returns: Status of the VirtQueue
+#
+# Since: 6.1
+#
+# Example:
+#
+# -> { "execute": "x-debug-virtio-queue-status",
+# "arguments": {
+# "path": "/machine/peripheral-anon/device[3]/virtio-backend",
+# "queue": 0
+# }
+# }
+# <- { "return": {
+# "signalled-used": 373,
+# "inuse": 0,
+# "vring-align": 4096,
+# "vring-desc": 864411648,
+# "signalled-used-valid": 0,
+# "vring-num-default": 256,
+# "vring-avail": 864415744,
+# "queue-index": 0,
+# "last-avail-idx": 373,
+# "vring-used": 864416320,
+# "used-idx": 373,
+# "device-type": "virtio-net",
+# "shadow-avail-idx": 619,
+# "vring-num": 256
+# }
+# }
+#
+##
+
+{ 'command': 'x-debug-virtio-queue-status',
+ 'data': { 'path': 'str', 'queue': 'uint16' },
+ 'returns': 'VirtQueueStatus'
+}
--
1.8.3.1
在 2021/7/12 下午6:35, Jonah Palmer 写道:
> From: Laurent Vivier <lvivier@redhat.com>
>
> This new command shows internal status of a VirtQueue.
> (vrings and indexes).
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> ---
> hw/virtio/virtio-stub.c | 6 +++
> hw/virtio/virtio.c | 37 ++++++++++++++++++
> qapi/virtio.json | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 145 insertions(+)
>
> [Jonah: Added 'device-type' field to VirtQueueStatus and
> qmp command x-debug-virtio-queue-status.]
>
> diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
> index ddb592f..3c1bf17 100644
> --- a/hw/virtio/virtio-stub.c
> +++ b/hw/virtio/virtio-stub.c
> @@ -17,3 +17,9 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
> {
> return qmp_virtio_unsupported(errp);
> }
> +
> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
> + uint16_t queue, Error **errp)
> +{
> + return qmp_virtio_unsupported(errp);
> +}
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 81a0ee8..ccd4371 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3935,6 +3935,43 @@ static VirtIODevice *virtio_device_find(const char *path)
> return NULL;
> }
>
> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
> + uint16_t queue, Error **errp)
> +{
> + VirtIODevice *vdev;
> + VirtQueueStatus *status;
> +
> + vdev = virtio_device_find(path);
> + if (vdev == NULL) {
> + error_setg(errp, "Path %s is not a VirtIO device", path);
> + return NULL;
> + }
> +
> + if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) {
> + error_setg(errp, "Invalid virtqueue number %d", queue);
> + return NULL;
> + }
> +
> + status = g_new0(VirtQueueStatus, 1);
> + status->device_type = qapi_enum_parse(&VirtioType_lookup, vdev->name,
> + VIRTIO_TYPE_UNKNOWN, NULL);
> + 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;
As mentioned in previous versions. We need add vhost support otherwise
the value here is wrong.
> + status->shadow_avail_idx = vdev->vq[queue].shadow_avail_idx;
The shadow index is something that is implementation specific e.g in the
case of vhost it's kind of meaningless.
Thanks
> + status->used_idx = vdev->vq[queue].used_idx;
> + status->signalled_used = vdev->vq[queue].signalled_used;
> + status->signalled_used_valid = vdev->vq[queue].signalled_used_valid;
> +
> + return status;
> +}
> +
> #define CONVERT_FEATURES(type, map) \
> ({ \
> type *list = NULL; \
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 78873cd..7007e0c 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -406,3 +406,105 @@
> 'data': { 'path': 'str' },
> 'returns': 'VirtioStatus'
> }
> +
> +##
> +# @VirtQueueStatus:
> +#
> +# Status of a VirtQueue
> +#
> +# @device-type: VirtIO device type
> +#
> +# @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: 6.1
> +#
> +##
> +
> +{ 'struct': 'VirtQueueStatus',
> + 'data': {
> + 'device-type': 'VirtioType',
> + 'queue-index': 'uint16',
> + 'inuse': 'uint32',
> + 'vring-num': 'int',
> + 'vring-num-default': 'int',
> + 'vring-align': 'int',
> + 'vring-desc': 'uint64',
> + 'vring-avail': 'uint64',
> + 'vring-used': 'uint64',
> + 'last-avail-idx': 'uint16',
> + 'shadow-avail-idx': 'uint16',
> + 'used-idx': 'uint16',
> + 'signalled-used': 'uint16',
> + 'signalled-used-valid': 'uint16'
> + }
> +}
> +
> +##
> +# @x-debug-virtio-queue-status:
> +#
> +# Return the status of a given VirtQueue
> +#
> +# @path: QOBject path of the VirtIODevice
> +#
> +# @queue: queue number to examine
> +#
> +# Returns: Status of the VirtQueue
> +#
> +# Since: 6.1
> +#
> +# Example:
> +#
> +# -> { "execute": "x-debug-virtio-queue-status",
> +# "arguments": {
> +# "path": "/machine/peripheral-anon/device[3]/virtio-backend",
> +# "queue": 0
> +# }
> +# }
> +# <- { "return": {
> +# "signalled-used": 373,
> +# "inuse": 0,
> +# "vring-align": 4096,
> +# "vring-desc": 864411648,
> +# "signalled-used-valid": 0,
> +# "vring-num-default": 256,
> +# "vring-avail": 864415744,
> +# "queue-index": 0,
> +# "last-avail-idx": 373,
> +# "vring-used": 864416320,
> +# "used-idx": 373,
> +# "device-type": "virtio-net",
> +# "shadow-avail-idx": 619,
> +# "vring-num": 256
> +# }
> +# }
> +#
> +##
> +
> +{ 'command': 'x-debug-virtio-queue-status',
> + 'data': { 'path': 'str', 'queue': 'uint16' },
> + 'returns': 'VirtQueueStatus'
> +}
On 7/13/21 10:37 PM, Jason Wang wrote:
>
> 在 2021/7/12 下午6:35, Jonah Palmer 写道:
>> From: Laurent Vivier <lvivier@redhat.com>
>>
>> This new command shows internal status of a VirtQueue.
>> (vrings and indexes).
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
>> ---
>> hw/virtio/virtio-stub.c | 6 +++
>> hw/virtio/virtio.c | 37 ++++++++++++++++++
>> qapi/virtio.json | 102
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 145 insertions(+)
>>
>> [Jonah: Added 'device-type' field to VirtQueueStatus and
>> qmp command x-debug-virtio-queue-status.]
>>
>> diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
>> index ddb592f..3c1bf17 100644
>> --- a/hw/virtio/virtio-stub.c
>> +++ b/hw/virtio/virtio-stub.c
>> @@ -17,3 +17,9 @@ VirtioStatus *qmp_x_debug_virtio_status(const char*
>> path, Error **errp)
>> {
>> return qmp_virtio_unsupported(errp);
>> }
>> +
>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>> + uint16_t queue,
>> Error **errp)
>> +{
>> + return qmp_virtio_unsupported(errp);
>> +}
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 81a0ee8..ccd4371 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -3935,6 +3935,43 @@ static VirtIODevice *virtio_device_find(const
>> char *path)
>> return NULL;
>> }
>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>> + uint16_t queue,
>> Error **errp)
>> +{
>> + VirtIODevice *vdev;
>> + VirtQueueStatus *status;
>> +
>> + vdev = virtio_device_find(path);
>> + if (vdev == NULL) {
>> + error_setg(errp, "Path %s is not a VirtIO device", path);
>> + return NULL;
>> + }
>> +
>> + if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev,
>> queue)) {
>> + error_setg(errp, "Invalid virtqueue number %d", queue);
>> + return NULL;
>> + }
>> +
>> + status = g_new0(VirtQueueStatus, 1);
>> + status->device_type = qapi_enum_parse(&VirtioType_lookup,
>> vdev->name,
>> + VIRTIO_TYPE_UNKNOWN, NULL);
>> + 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;
>
>
> As mentioned in previous versions. We need add vhost support otherwise
> the value here is wrong.
Got it. I'll add a case to determine if vhost is active for a given device.
So, in the case that vhost is active, should I just not print out the value or would I substitute it with
another value (whatever that might be)? Same question for shadow_avail_idx below as well.
Jonah
>
>
>> + status->shadow_avail_idx = vdev->vq[queue].shadow_avail_idx;
>
>
> The shadow index is something that is implementation specific e.g in
> the case of vhost it's kind of meaningless.
>
> Thanks
>
>
>> + status->used_idx = vdev->vq[queue].used_idx;
>> + status->signalled_used = vdev->vq[queue].signalled_used;
>> + status->signalled_used_valid =
>> vdev->vq[queue].signalled_used_valid;
>> +
>> + return status;
>> +}
>> +
>> #define CONVERT_FEATURES(type, map) \
>> ({ \
>> type *list = NULL; \
>> diff --git a/qapi/virtio.json b/qapi/virtio.json
>> index 78873cd..7007e0c 100644
>> --- a/qapi/virtio.json
>> +++ b/qapi/virtio.json
>> @@ -406,3 +406,105 @@
>> 'data': { 'path': 'str' },
>> 'returns': 'VirtioStatus'
>> }
>> +
>> +##
>> +# @VirtQueueStatus:
>> +#
>> +# Status of a VirtQueue
>> +#
>> +# @device-type: VirtIO device type
>> +#
>> +# @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: 6.1
>> +#
>> +##
>> +
>> +{ 'struct': 'VirtQueueStatus',
>> + 'data': {
>> + 'device-type': 'VirtioType',
>> + 'queue-index': 'uint16',
>> + 'inuse': 'uint32',
>> + 'vring-num': 'int',
>> + 'vring-num-default': 'int',
>> + 'vring-align': 'int',
>> + 'vring-desc': 'uint64',
>> + 'vring-avail': 'uint64',
>> + 'vring-used': 'uint64',
>> + 'last-avail-idx': 'uint16',
>> + 'shadow-avail-idx': 'uint16',
>> + 'used-idx': 'uint16',
>> + 'signalled-used': 'uint16',
>> + 'signalled-used-valid': 'uint16'
>> + }
>> +}
>> +
>> +##
>> +# @x-debug-virtio-queue-status:
>> +#
>> +# Return the status of a given VirtQueue
>> +#
>> +# @path: QOBject path of the VirtIODevice
>> +#
>> +# @queue: queue number to examine
>> +#
>> +# Returns: Status of the VirtQueue
>> +#
>> +# Since: 6.1
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "x-debug-virtio-queue-status",
>> +# "arguments": {
>> +# "path": "/machine/peripheral-anon/device[3]/virtio-backend",
>> +# "queue": 0
>> +# }
>> +# }
>> +# <- { "return": {
>> +# "signalled-used": 373,
>> +# "inuse": 0,
>> +# "vring-align": 4096,
>> +# "vring-desc": 864411648,
>> +# "signalled-used-valid": 0,
>> +# "vring-num-default": 256,
>> +# "vring-avail": 864415744,
>> +# "queue-index": 0,
>> +# "last-avail-idx": 373,
>> +# "vring-used": 864416320,
>> +# "used-idx": 373,
>> +# "device-type": "virtio-net",
>> +# "shadow-avail-idx": 619,
>> +# "vring-num": 256
>> +# }
>> +# }
>> +#
>> +##
>> +
>> +{ 'command': 'x-debug-virtio-queue-status',
>> + 'data': { 'path': 'str', 'queue': 'uint16' },
>> + 'returns': 'VirtQueueStatus'
>> +}
>
在 2021/7/21 下午4:59, Jonah Palmer 写道:
>
>
> On 7/13/21 10:37 PM, Jason Wang wrote:
>>
>> 在 2021/7/12 下午6:35, Jonah Palmer 写道:
>>> From: Laurent Vivier <lvivier@redhat.com>
>>>
>>> This new command shows internal status of a VirtQueue.
>>> (vrings and indexes).
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
>>> ---
>>> hw/virtio/virtio-stub.c | 6 +++
>>> hw/virtio/virtio.c | 37 ++++++++++++++++++
>>> qapi/virtio.json | 102
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 145 insertions(+)
>>>
>>> [Jonah: Added 'device-type' field to VirtQueueStatus and
>>> qmp command x-debug-virtio-queue-status.]
>>>
>>> diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
>>> index ddb592f..3c1bf17 100644
>>> --- a/hw/virtio/virtio-stub.c
>>> +++ b/hw/virtio/virtio-stub.c
>>> @@ -17,3 +17,9 @@ VirtioStatus *qmp_x_debug_virtio_status(const
>>> char* path, Error **errp)
>>> {
>>> return qmp_virtio_unsupported(errp);
>>> }
>>> +
>>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>>> + uint16_t queue,
>>> Error **errp)
>>> +{
>>> + return qmp_virtio_unsupported(errp);
>>> +}
>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>> index 81a0ee8..ccd4371 100644
>>> --- a/hw/virtio/virtio.c
>>> +++ b/hw/virtio/virtio.c
>>> @@ -3935,6 +3935,43 @@ static VirtIODevice *virtio_device_find(const
>>> char *path)
>>> return NULL;
>>> }
>>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>>> + uint16_t queue,
>>> Error **errp)
>>> +{
>>> + VirtIODevice *vdev;
>>> + VirtQueueStatus *status;
>>> +
>>> + vdev = virtio_device_find(path);
>>> + if (vdev == NULL) {
>>> + error_setg(errp, "Path %s is not a VirtIO device", path);
>>> + return NULL;
>>> + }
>>> +
>>> + if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev,
>>> queue)) {
>>> + error_setg(errp, "Invalid virtqueue number %d", queue);
>>> + return NULL;
>>> + }
>>> +
>>> + status = g_new0(VirtQueueStatus, 1);
>>> + status->device_type = qapi_enum_parse(&VirtioType_lookup,
>>> vdev->name,
>>> + VIRTIO_TYPE_UNKNOWN, NULL);
>>> + 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;
>>
>>
>> As mentioned in previous versions. We need add vhost support
>> otherwise the value here is wrong.
> Got it. I'll add a case to determine if vhost is active for a given device.
> So, in the case that vhost is active, should I just not print out the value or would I substitute it with
> another value (whatever that might be)?
You can query the vhost for those index.
(vhost_get_vring_base())
> Same question for shadow_avail_idx below as well.
It's an implementation specific. I think we can simply not show it if
vhost is enabled.
Thanks
>
> Jonah
>>
>>
>>> + status->shadow_avail_idx = vdev->vq[queue].shadow_avail_idx;
>>
>>
>> The shadow index is something that is implementation specific e.g in
>> the case of vhost it's kind of meaningless.
>>
>> Thanks
>>
>>
>>> + status->used_idx = vdev->vq[queue].used_idx;
>>> + status->signalled_used = vdev->vq[queue].signalled_used;
>>> + status->signalled_used_valid =
>>> vdev->vq[queue].signalled_used_valid;
>>> +
>>> + return status;
>>> +}
>>> +
>>> #define CONVERT_FEATURES(type, map) \
>>> ({ \
>>> type *list = NULL; \
>>> diff --git a/qapi/virtio.json b/qapi/virtio.json
>>> index 78873cd..7007e0c 100644
>>> --- a/qapi/virtio.json
>>> +++ b/qapi/virtio.json
>>> @@ -406,3 +406,105 @@
>>> 'data': { 'path': 'str' },
>>> 'returns': 'VirtioStatus'
>>> }
>>> +
>>> +##
>>> +# @VirtQueueStatus:
>>> +#
>>> +# Status of a VirtQueue
>>> +#
>>> +# @device-type: VirtIO device type
>>> +#
>>> +# @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: 6.1
>>> +#
>>> +##
>>> +
>>> +{ 'struct': 'VirtQueueStatus',
>>> + 'data': {
>>> + 'device-type': 'VirtioType',
>>> + 'queue-index': 'uint16',
>>> + 'inuse': 'uint32',
>>> + 'vring-num': 'int',
>>> + 'vring-num-default': 'int',
>>> + 'vring-align': 'int',
>>> + 'vring-desc': 'uint64',
>>> + 'vring-avail': 'uint64',
>>> + 'vring-used': 'uint64',
>>> + 'last-avail-idx': 'uint16',
>>> + 'shadow-avail-idx': 'uint16',
>>> + 'used-idx': 'uint16',
>>> + 'signalled-used': 'uint16',
>>> + 'signalled-used-valid': 'uint16'
>>> + }
>>> +}
>>> +
>>> +##
>>> +# @x-debug-virtio-queue-status:
>>> +#
>>> +# Return the status of a given VirtQueue
>>> +#
>>> +# @path: QOBject path of the VirtIODevice
>>> +#
>>> +# @queue: queue number to examine
>>> +#
>>> +# Returns: Status of the VirtQueue
>>> +#
>>> +# Since: 6.1
>>> +#
>>> +# Example:
>>> +#
>>> +# -> { "execute": "x-debug-virtio-queue-status",
>>> +# "arguments": {
>>> +# "path":
>>> "/machine/peripheral-anon/device[3]/virtio-backend",
>>> +# "queue": 0
>>> +# }
>>> +# }
>>> +# <- { "return": {
>>> +# "signalled-used": 373,
>>> +# "inuse": 0,
>>> +# "vring-align": 4096,
>>> +# "vring-desc": 864411648,
>>> +# "signalled-used-valid": 0,
>>> +# "vring-num-default": 256,
>>> +# "vring-avail": 864415744,
>>> +# "queue-index": 0,
>>> +# "last-avail-idx": 373,
>>> +# "vring-used": 864416320,
>>> +# "used-idx": 373,
>>> +# "device-type": "virtio-net",
>>> +# "shadow-avail-idx": 619,
>>> +# "vring-num": 256
>>> +# }
>>> +# }
>>> +#
>>> +##
>>> +
>>> +{ 'command': 'x-debug-virtio-queue-status',
>>> + 'data': { 'path': 'str', 'queue': 'uint16' },
>>> + 'returns': 'VirtQueueStatus'
>>> +}
>>
On 7/22/21 5:22 AM, Jason Wang wrote:
>
> 在 2021/7/21 下午4:59, Jonah Palmer 写道:
>>
>>
>> On 7/13/21 10:37 PM, Jason Wang wrote:
>>>
>>> 在 2021/7/12 下午6:35, Jonah Palmer 写道:
>>>> From: Laurent Vivier <lvivier@redhat.com>
>>>>
>>>> This new command shows internal status of a VirtQueue.
>>>> (vrings and indexes).
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
>>>> ---
>>>> hw/virtio/virtio-stub.c | 6 +++
>>>> hw/virtio/virtio.c | 37 ++++++++++++++++++
>>>> qapi/virtio.json | 102
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 145 insertions(+)
>>>>
>>>> [Jonah: Added 'device-type' field to VirtQueueStatus and
>>>> qmp command x-debug-virtio-queue-status.]
>>>>
>>>> diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
>>>> index ddb592f..3c1bf17 100644
>>>> --- a/hw/virtio/virtio-stub.c
>>>> +++ b/hw/virtio/virtio-stub.c
>>>> @@ -17,3 +17,9 @@ VirtioStatus *qmp_x_debug_virtio_status(const
>>>> char* path, Error **errp)
>>>> {
>>>> return qmp_virtio_unsupported(errp);
>>>> }
>>>> +
>>>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>>>> + uint16_t queue,
>>>> Error **errp)
>>>> +{
>>>> + return qmp_virtio_unsupported(errp);
>>>> +}
>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>>> index 81a0ee8..ccd4371 100644
>>>> --- a/hw/virtio/virtio.c
>>>> +++ b/hw/virtio/virtio.c
>>>> @@ -3935,6 +3935,43 @@ static VirtIODevice
>>>> *virtio_device_find(const char *path)
>>>> return NULL;
>>>> }
>>>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>>>> + uint16_t queue,
>>>> Error **errp)
>>>> +{
>>>> + VirtIODevice *vdev;
>>>> + VirtQueueStatus *status;
>>>> +
>>>> + vdev = virtio_device_find(path);
>>>> + if (vdev == NULL) {
>>>> + error_setg(errp, "Path %s is not a VirtIO device", path);
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev,
>>>> queue)) {
>>>> + error_setg(errp, "Invalid virtqueue number %d", queue);
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + status = g_new0(VirtQueueStatus, 1);
>>>> + status->device_type = qapi_enum_parse(&VirtioType_lookup,
>>>> vdev->name,
>>>> + VIRTIO_TYPE_UNKNOWN, NULL);
>>>> + 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;
>>>
>>>
>>> As mentioned in previous versions. We need add vhost support
>>> otherwise the value here is wrong.
>> Got it. I'll add a case to determine if vhost is active for a given
>> device.
>> So, in the case that vhost is active, should I just not print out the
>> value or would I substitute it with
>> another value (whatever that might be)?
>
>
> You can query the vhost for those index.
>
> (vhost_get_vring_base())
>
>
>> Same question for shadow_avail_idx below as well.
>
>
> It's an implementation specific. I think we can simply not show it if
> vhost is enabled.
>
> Thanks
Ah I see, thank you!
So, it appears to me that it's not very easy to get the struct vhost_dev
pointer from struct VirtIODevice to indicate whether or not vhost is
active, e.g. there's no virtio class-independent way to get struct
vhost_dev.
I was thinking of adding an op/callback function to struct
VirtioDeviceClass, e.g. bool has_vhost(VirtIODevice *vdev), and
implement it for each virtio class (net, scsi, blk, etc.).
For example, for virtio-net, maybe it'd be something like:
bool has_vhost(VirtIODevice *vdev) {
VirtIONet *n = VIRTIO_NET(vdev);
NetClientState *nc = qemu_get_queue(n->nic);
return nc->peer ? get_vhost_net(nc->peer) : false;
}
Also, for getting the last_avail_idx, I was also thinking of adding
another op/callback to struct VirtioDeviceClass, e.g. unsigned int
get_last_avail_idx(VirtIODevice *vdev, unsigned int vq_idx) that finds
if vhost is active or not and either gets last_avail_idx from virtio
directly or through vhost (e.g.
vhost_dev->vhost_ops->vhost_get_vring_base()).
I wanted to run this by you and get your opinion on this before I
started implementing it in code. Let me know what you think about this.
Jonah
>
>
>>
>> Jonah
>>>
>>>
>>>> + status->shadow_avail_idx = vdev->vq[queue].shadow_avail_idx;
>>>
>>>
>>> The shadow index is something that is implementation specific e.g in
>>> the case of vhost it's kind of meaningless.
>>>
>>> Thanks
>>>
>>>
>>>> + status->used_idx = vdev->vq[queue].used_idx;
>>>> + status->signalled_used = vdev->vq[queue].signalled_used;
>>>> + status->signalled_used_valid =
>>>> vdev->vq[queue].signalled_used_valid;
>>>> +
>>>> + return status;
>>>> +}
>>>> +
>>>> #define CONVERT_FEATURES(type, map) \
>>>> ({ \
>>>> type *list = NULL; \
>>>> diff --git a/qapi/virtio.json b/qapi/virtio.json
>>>> index 78873cd..7007e0c 100644
>>>> --- a/qapi/virtio.json
>>>> +++ b/qapi/virtio.json
>>>> @@ -406,3 +406,105 @@
>>>> 'data': { 'path': 'str' },
>>>> 'returns': 'VirtioStatus'
>>>> }
>>>> +
>>>> +##
>>>> +# @VirtQueueStatus:
>>>> +#
>>>> +# Status of a VirtQueue
>>>> +#
>>>> +# @device-type: VirtIO device type
>>>> +#
>>>> +# @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: 6.1
>>>> +#
>>>> +##
>>>> +
>>>> +{ 'struct': 'VirtQueueStatus',
>>>> + 'data': {
>>>> + 'device-type': 'VirtioType',
>>>> + 'queue-index': 'uint16',
>>>> + 'inuse': 'uint32',
>>>> + 'vring-num': 'int',
>>>> + 'vring-num-default': 'int',
>>>> + 'vring-align': 'int',
>>>> + 'vring-desc': 'uint64',
>>>> + 'vring-avail': 'uint64',
>>>> + 'vring-used': 'uint64',
>>>> + 'last-avail-idx': 'uint16',
>>>> + 'shadow-avail-idx': 'uint16',
>>>> + 'used-idx': 'uint16',
>>>> + 'signalled-used': 'uint16',
>>>> + 'signalled-used-valid': 'uint16'
>>>> + }
>>>> +}
>>>> +
>>>> +##
>>>> +# @x-debug-virtio-queue-status:
>>>> +#
>>>> +# Return the status of a given VirtQueue
>>>> +#
>>>> +# @path: QOBject path of the VirtIODevice
>>>> +#
>>>> +# @queue: queue number to examine
>>>> +#
>>>> +# Returns: Status of the VirtQueue
>>>> +#
>>>> +# Since: 6.1
>>>> +#
>>>> +# Example:
>>>> +#
>>>> +# -> { "execute": "x-debug-virtio-queue-status",
>>>> +# "arguments": {
>>>> +# "path":
>>>> "/machine/peripheral-anon/device[3]/virtio-backend",
>>>> +# "queue": 0
>>>> +# }
>>>> +# }
>>>> +# <- { "return": {
>>>> +# "signalled-used": 373,
>>>> +# "inuse": 0,
>>>> +# "vring-align": 4096,
>>>> +# "vring-desc": 864411648,
>>>> +# "signalled-used-valid": 0,
>>>> +# "vring-num-default": 256,
>>>> +# "vring-avail": 864415744,
>>>> +# "queue-index": 0,
>>>> +# "last-avail-idx": 373,
>>>> +# "vring-used": 864416320,
>>>> +# "used-idx": 373,
>>>> +# "device-type": "virtio-net",
>>>> +# "shadow-avail-idx": 619,
>>>> +# "vring-num": 256
>>>> +# }
>>>> +# }
>>>> +#
>>>> +##
>>>> +
>>>> +{ 'command': 'x-debug-virtio-queue-status',
>>>> + 'data': { 'path': 'str', 'queue': 'uint16' },
>>>> + 'returns': 'VirtQueueStatus'
>>>> +}
>>>
>
Hi Jason, could I get your thoughts on this implementation question below?
I'm not too sure on how I should proceed determining if vhost is active
or not.
Thank you!
Jonah
On 7/26/21 5:33 AM, Jonah Palmer wrote:
>
>
> On 7/22/21 5:22 AM, Jason Wang wrote:
>>
>> 在 2021/7/21 下午4:59, Jonah Palmer 写道:
>>>
>>>
>>> On 7/13/21 10:37 PM, Jason Wang wrote:
>>>>
>>>> 在 2021/7/12 下午6:35, Jonah Palmer 写道:
>>>>> From: Laurent Vivier <lvivier@redhat.com>
>>>>>
>>>>> This new command shows internal status of a VirtQueue.
>>>>> (vrings and indexes).
>>>>>
>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
>>>>> ---
>>>>> hw/virtio/virtio-stub.c | 6 +++
>>>>> hw/virtio/virtio.c | 37 ++++++++++++++++++
>>>>> qapi/virtio.json | 102
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> 3 files changed, 145 insertions(+)
>>>>>
>>>>> [Jonah: Added 'device-type' field to VirtQueueStatus and
>>>>> qmp command x-debug-virtio-queue-status.]
>>>>>
>>>>> diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
>>>>> index ddb592f..3c1bf17 100644
>>>>> --- a/hw/virtio/virtio-stub.c
>>>>> +++ b/hw/virtio/virtio-stub.c
>>>>> @@ -17,3 +17,9 @@ VirtioStatus *qmp_x_debug_virtio_status(const
>>>>> char* path, Error **errp)
>>>>> {
>>>>> return qmp_virtio_unsupported(errp);
>>>>> }
>>>>> +
>>>>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>>>>> + uint16_t queue,
>>>>> Error **errp)
>>>>> +{
>>>>> + return qmp_virtio_unsupported(errp);
>>>>> +}
>>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>>>> index 81a0ee8..ccd4371 100644
>>>>> --- a/hw/virtio/virtio.c
>>>>> +++ b/hw/virtio/virtio.c
>>>>> @@ -3935,6 +3935,43 @@ static VirtIODevice
>>>>> *virtio_device_find(const char *path)
>>>>> return NULL;
>>>>> }
>>>>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>>>>> + uint16_t queue,
>>>>> Error **errp)
>>>>> +{
>>>>> + VirtIODevice *vdev;
>>>>> + VirtQueueStatus *status;
>>>>> +
>>>>> + vdev = virtio_device_find(path);
>>>>> + if (vdev == NULL) {
>>>>> + error_setg(errp, "Path %s is not a VirtIO device", path);
>>>>> + return NULL;
>>>>> + }
>>>>> +
>>>>> + if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev,
>>>>> queue)) {
>>>>> + error_setg(errp, "Invalid virtqueue number %d", queue);
>>>>> + return NULL;
>>>>> + }
>>>>> +
>>>>> + status = g_new0(VirtQueueStatus, 1);
>>>>> + status->device_type = qapi_enum_parse(&VirtioType_lookup,
>>>>> vdev->name,
>>>>> + VIRTIO_TYPE_UNKNOWN, NULL);
>>>>> + 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;
>>>>
>>>>
>>>> As mentioned in previous versions. We need add vhost support
>>>> otherwise the value here is wrong.
>>> Got it. I'll add a case to determine if vhost is active for a given
>>> device.
>>> So, in the case that vhost is active, should I just not print out
>>> the value or would I substitute it with
>>> another value (whatever that might be)?
>>
>>
>> You can query the vhost for those index.
>>
>> (vhost_get_vring_base())
>>
>>
>>> Same question for shadow_avail_idx below as well.
>>
>>
>> It's an implementation specific. I think we can simply not show it if
>> vhost is enabled.
>>
>> Thanks
>
> Ah I see, thank you!
>
> So, it appears to me that it's not very easy to get the struct
> vhost_dev pointer from struct VirtIODevice to indicate whether or not
> vhost is active, e.g. there's no virtio class-independent way to get
> struct vhost_dev.
>
> I was thinking of adding an op/callback function to struct
> VirtioDeviceClass, e.g. bool has_vhost(VirtIODevice *vdev), and
> implement it for each virtio class (net, scsi, blk, etc.).
>
> For example, for virtio-net, maybe it'd be something like:
>
> bool has_vhost(VirtIODevice *vdev) {
> VirtIONet *n = VIRTIO_NET(vdev);
> NetClientState *nc = qemu_get_queue(n->nic);
> return nc->peer ? get_vhost_net(nc->peer) : false;
> }
>
> Also, for getting the last_avail_idx, I was also thinking of adding
> another op/callback to struct VirtioDeviceClass, e.g. unsigned int
> get_last_avail_idx(VirtIODevice *vdev, unsigned int vq_idx) that finds
> if vhost is active or not and either gets last_avail_idx from virtio
> directly or through vhost (e.g.
> vhost_dev->vhost_ops->vhost_get_vring_base()).
>
> I wanted to run this by you and get your opinion on this before I
> started implementing it in code. Let me know what you think about this.
>
>
> Jonah
>
>>
>>
>>>
>>> Jonah
>>>>
>>>>
>>>>> + status->shadow_avail_idx = vdev->vq[queue].shadow_avail_idx;
>>>>
>>>>
>>>> The shadow index is something that is implementation specific e.g
>>>> in the case of vhost it's kind of meaningless.
>>>>
>>>> Thanks
>>>>
>>>>
>>>>> + status->used_idx = vdev->vq[queue].used_idx;
>>>>> + status->signalled_used = vdev->vq[queue].signalled_used;
>>>>> + status->signalled_used_valid =
>>>>> vdev->vq[queue].signalled_used_valid;
>>>>> +
>>>>> + return status;
>>>>> +}
>>>>> +
>>>>> #define CONVERT_FEATURES(type, map) \
>>>>> ({ \
>>>>> type *list = NULL; \
>>>>> diff --git a/qapi/virtio.json b/qapi/virtio.json
>>>>> index 78873cd..7007e0c 100644
>>>>> --- a/qapi/virtio.json
>>>>> +++ b/qapi/virtio.json
>>>>> @@ -406,3 +406,105 @@
>>>>> 'data': { 'path': 'str' },
>>>>> 'returns': 'VirtioStatus'
>>>>> }
>>>>> +
>>>>> +##
>>>>> +# @VirtQueueStatus:
>>>>> +#
>>>>> +# Status of a VirtQueue
>>>>> +#
>>>>> +# @device-type: VirtIO device type
>>>>> +#
>>>>> +# @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: 6.1
>>>>> +#
>>>>> +##
>>>>> +
>>>>> +{ 'struct': 'VirtQueueStatus',
>>>>> + 'data': {
>>>>> + 'device-type': 'VirtioType',
>>>>> + 'queue-index': 'uint16',
>>>>> + 'inuse': 'uint32',
>>>>> + 'vring-num': 'int',
>>>>> + 'vring-num-default': 'int',
>>>>> + 'vring-align': 'int',
>>>>> + 'vring-desc': 'uint64',
>>>>> + 'vring-avail': 'uint64',
>>>>> + 'vring-used': 'uint64',
>>>>> + 'last-avail-idx': 'uint16',
>>>>> + 'shadow-avail-idx': 'uint16',
>>>>> + 'used-idx': 'uint16',
>>>>> + 'signalled-used': 'uint16',
>>>>> + 'signalled-used-valid': 'uint16'
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +##
>>>>> +# @x-debug-virtio-queue-status:
>>>>> +#
>>>>> +# Return the status of a given VirtQueue
>>>>> +#
>>>>> +# @path: QOBject path of the VirtIODevice
>>>>> +#
>>>>> +# @queue: queue number to examine
>>>>> +#
>>>>> +# Returns: Status of the VirtQueue
>>>>> +#
>>>>> +# Since: 6.1
>>>>> +#
>>>>> +# Example:
>>>>> +#
>>>>> +# -> { "execute": "x-debug-virtio-queue-status",
>>>>> +# "arguments": {
>>>>> +# "path":
>>>>> "/machine/peripheral-anon/device[3]/virtio-backend",
>>>>> +# "queue": 0
>>>>> +# }
>>>>> +# }
>>>>> +# <- { "return": {
>>>>> +# "signalled-used": 373,
>>>>> +# "inuse": 0,
>>>>> +# "vring-align": 4096,
>>>>> +# "vring-desc": 864411648,
>>>>> +# "signalled-used-valid": 0,
>>>>> +# "vring-num-default": 256,
>>>>> +# "vring-avail": 864415744,
>>>>> +# "queue-index": 0,
>>>>> +# "last-avail-idx": 373,
>>>>> +# "vring-used": 864416320,
>>>>> +# "used-idx": 373,
>>>>> +# "device-type": "virtio-net",
>>>>> +# "shadow-avail-idx": 619,
>>>>> +# "vring-num": 256
>>>>> +# }
>>>>> +# }
>>>>> +#
>>>>> +##
>>>>> +
>>>>> +{ 'command': 'x-debug-virtio-queue-status',
>>>>> + 'data': { 'path': 'str', 'queue': 'uint16' },
>>>>> + 'returns': 'VirtQueueStatus'
>>>>> +}
>>>>
>>
在 2021/8/26 下午2:25, Jonah Palmer 写道:
>
> Hi Jason, could I get your thoughts on this implementation question below?
>
> I'm not too sure on how I should proceed determining if vhost is
> active or not.
>
> Thank you!
>
>
> Jonah
>
> On 7/26/21 5:33 AM, Jonah Palmer wrote:
>>
>>
>> On 7/22/21 5:22 AM, Jason Wang wrote:
>>>
>>> 在 2021/7/21 下午4:59, Jonah Palmer 写道:
>>>>
>>>>
>>>> On 7/13/21 10:37 PM, Jason Wang wrote:
>>>>>
>>>>> 在 2021/7/12 下午6:35, Jonah Palmer 写道:
>>>>>> From: Laurent Vivier <lvivier@redhat.com>
>>>>>>
>>>>>> This new command shows internal status of a VirtQueue.
>>>>>> (vrings and indexes).
>>>>>>
>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>>> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
>>>>>> ---
>>>>>> hw/virtio/virtio-stub.c | 6 +++
>>>>>> hw/virtio/virtio.c | 37 ++++++++++++++++++
>>>>>> qapi/virtio.json | 102
>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> 3 files changed, 145 insertions(+)
>>>>>>
>>>>>> [Jonah: Added 'device-type' field to VirtQueueStatus and
>>>>>> qmp command x-debug-virtio-queue-status.]
>>>>>>
>>>>>> diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
>>>>>> index ddb592f..3c1bf17 100644
>>>>>> --- a/hw/virtio/virtio-stub.c
>>>>>> +++ b/hw/virtio/virtio-stub.c
>>>>>> @@ -17,3 +17,9 @@ VirtioStatus *qmp_x_debug_virtio_status(const
>>>>>> char* path, Error **errp)
>>>>>> {
>>>>>> return qmp_virtio_unsupported(errp);
>>>>>> }
>>>>>> +
>>>>>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
>>>>>> + uint16_t queue, Error **errp)
>>>>>> +{
>>>>>> + return qmp_virtio_unsupported(errp);
>>>>>> +}
>>>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>>>>> index 81a0ee8..ccd4371 100644
>>>>>> --- a/hw/virtio/virtio.c
>>>>>> +++ b/hw/virtio/virtio.c
>>>>>> @@ -3935,6 +3935,43 @@ static VirtIODevice
>>>>>> *virtio_device_find(const char *path)
>>>>>> return NULL;
>>>>>> }
>>>>>> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char
>>>>>> *path,
>>>>>> + uint16_t queue, Error **errp)
>>>>>> +{
>>>>>> + VirtIODevice *vdev;
>>>>>> + VirtQueueStatus *status;
>>>>>> +
>>>>>> + vdev = virtio_device_find(path);
>>>>>> + if (vdev == NULL) {
>>>>>> + error_setg(errp, "Path %s is not a VirtIO device", path);
>>>>>> + return NULL;
>>>>>> + }
>>>>>> +
>>>>>> + if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev,
>>>>>> queue)) {
>>>>>> + error_setg(errp, "Invalid virtqueue number %d", queue);
>>>>>> + return NULL;
>>>>>> + }
>>>>>> +
>>>>>> + status = g_new0(VirtQueueStatus, 1);
>>>>>> + status->device_type = qapi_enum_parse(&VirtioType_lookup,
>>>>>> vdev->name,
>>>>>> + VIRTIO_TYPE_UNKNOWN, NULL);
>>>>>> + 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;
>>>>>
>>>>>
>>>>> As mentioned in previous versions. We need add vhost support
>>>>> otherwise the value here is wrong.
>>>> Got it. I'll add a case to determine if vhost is active for a given
>>>> device.
>>>> So, in the case that vhost is active, should I just not print out
>>>> the value or would I substitute it with
>>>> another value (whatever that might be)?
>>>
>>>
>>> You can query the vhost for those index.
>>>
>>> (vhost_get_vring_base())
>>>
>>>
>>>> Same question for shadow_avail_idx below as well.
>>>
>>>
>>> It's an implementation specific. I think we can simply not show it
>>> if vhost is enabled.
>>>
>>> Thanks
>>
>> Ah I see, thank you!
>>
>> So, it appears to me that it's not very easy to get the struct
>> vhost_dev pointer from struct VirtIODevice to indicate whether or not
>> vhost is active, e.g. there's no virtio class-independent way to get
>> struct vhost_dev.
>>
>> I was thinking of adding an op/callback function to struct
>> VirtioDeviceClass, e.g. bool has_vhost(VirtIODevice *vdev), and
>> implement it for each virtio class (net, scsi, blk, etc.).
>>
>> For example, for virtio-net, maybe it'd be something like:
>>
>> bool has_vhost(VirtIODevice *vdev) {
>> VirtIONet *n = VIRTIO_NET(vdev);
>> NetClientState *nc = qemu_get_queue(n->nic);
>> return nc->peer ? get_vhost_net(nc->peer) : false;
>> }
Something like this, yes.
>> Also, for getting the last_avail_idx, I was also thinking of adding
>> another op/callback to struct VirtioDeviceClass, e.g. unsigned int
>> get_last_avail_idx(VirtIODevice *vdev, unsigned int vq_idx) that
>> finds if vhost is active or not and either gets last_avail_idx from
>> virtio directly or through vhost (e.g.
>> vhost_dev->vhost_ops->vhost_get_vring_base()).
>>
So I think instead of has_vhost, we probably need get_vhost() to have a
pointer to vhost_dev. Then we can do anything we want other than a
dedicated interface just for avail index.
Thanks
>> I wanted to run this by you and get your opinion on this before I
>> started implementing it in code. Let me know what you think about this.
>>
>>
>> Jonah
>>
>>>
>>>
>>>>
>>>> Jonah
>>>>>
>>>>>
>>>>>> + status->shadow_avail_idx = vdev->vq[queue].shadow_avail_idx;
>>>>>
>>>>>
>>>>> The shadow index is something that is implementation specific e.g
>>>>> in the case of vhost it's kind of meaningless.
>>>>>
>>>>> Thanks
>>>>>
>>>>>
>>>>>> + status->used_idx = vdev->vq[queue].used_idx;
>>>>>> + status->signalled_used = vdev->vq[queue].signalled_used;
>>>>>> + status->signalled_used_valid =
>>>>>> vdev->vq[queue].signalled_used_valid;
>>>>>> +
>>>>>> + return status;
>>>>>> +}
>>>>>> +
>>>>>> #define CONVERT_FEATURES(type, map) \
>>>>>> ({ \
>>>>>> type *list = NULL; \
>>>>>> diff --git a/qapi/virtio.json b/qapi/virtio.json
>>>>>> index 78873cd..7007e0c 100644
>>>>>> --- a/qapi/virtio.json
>>>>>> +++ b/qapi/virtio.json
>>>>>> @@ -406,3 +406,105 @@
>>>>>> 'data': { 'path': 'str' },
>>>>>> 'returns': 'VirtioStatus'
>>>>>> }
>>>>>> +
>>>>>> +##
>>>>>> +# @VirtQueueStatus:
>>>>>> +#
>>>>>> +# Status of a VirtQueue
>>>>>> +#
>>>>>> +# @device-type: VirtIO device type
>>>>>> +#
>>>>>> +# @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: 6.1
>>>>>> +#
>>>>>> +##
>>>>>> +
>>>>>> +{ 'struct': 'VirtQueueStatus',
>>>>>> + 'data': {
>>>>>> + 'device-type': 'VirtioType',
>>>>>> + 'queue-index': 'uint16',
>>>>>> + 'inuse': 'uint32',
>>>>>> + 'vring-num': 'int',
>>>>>> + 'vring-num-default': 'int',
>>>>>> + 'vring-align': 'int',
>>>>>> + 'vring-desc': 'uint64',
>>>>>> + 'vring-avail': 'uint64',
>>>>>> + 'vring-used': 'uint64',
>>>>>> + 'last-avail-idx': 'uint16',
>>>>>> + 'shadow-avail-idx': 'uint16',
>>>>>> + 'used-idx': 'uint16',
>>>>>> + 'signalled-used': 'uint16',
>>>>>> + 'signalled-used-valid': 'uint16'
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> +##
>>>>>> +# @x-debug-virtio-queue-status:
>>>>>> +#
>>>>>> +# Return the status of a given VirtQueue
>>>>>> +#
>>>>>> +# @path: QOBject path of the VirtIODevice
>>>>>> +#
>>>>>> +# @queue: queue number to examine
>>>>>> +#
>>>>>> +# Returns: Status of the VirtQueue
>>>>>> +#
>>>>>> +# Since: 6.1
>>>>>> +#
>>>>>> +# Example:
>>>>>> +#
>>>>>> +# -> { "execute": "x-debug-virtio-queue-status",
>>>>>> +# "arguments": {
>>>>>> +# "path":
>>>>>> "/machine/peripheral-anon/device[3]/virtio-backend",
>>>>>> +# "queue": 0
>>>>>> +# }
>>>>>> +# }
>>>>>> +# <- { "return": {
>>>>>> +# "signalled-used": 373,
>>>>>> +# "inuse": 0,
>>>>>> +# "vring-align": 4096,
>>>>>> +# "vring-desc": 864411648,
>>>>>> +# "signalled-used-valid": 0,
>>>>>> +# "vring-num-default": 256,
>>>>>> +# "vring-avail": 864415744,
>>>>>> +# "queue-index": 0,
>>>>>> +# "last-avail-idx": 373,
>>>>>> +# "vring-used": 864416320,
>>>>>> +# "used-idx": 373,
>>>>>> +# "device-type": "virtio-net",
>>>>>> +# "shadow-avail-idx": 619,
>>>>>> +# "vring-num": 256
>>>>>> +# }
>>>>>> +# }
>>>>>> +#
>>>>>> +##
>>>>>> +
>>>>>> +{ 'command': 'x-debug-virtio-queue-status',
>>>>>> + 'data': { 'path': 'str', 'queue': 'uint16' },
>>>>>> + 'returns': 'VirtQueueStatus'
>>>>>> +}
>>>>>
>>>
Jonah Palmer <jonah.palmer@oracle.com> writes:
> From: Laurent Vivier <lvivier@redhat.com>
>
> This new command shows internal status of a VirtQueue.
> (vrings and indexes).
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
[...]
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 78873cd..7007e0c 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -406,3 +406,105 @@
> 'data': { 'path': 'str' },
> 'returns': 'VirtioStatus'
> }
> +
> +##
> +# @VirtQueueStatus:
> +#
> +# Status of a VirtQueue
> +#
> +# @device-type: VirtIO device type
> +#
> +# @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: 6.1
> +#
> +##
> +
> +{ 'struct': 'VirtQueueStatus',
> + 'data': {
> + 'device-type': 'VirtioType',
> + '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'
> + }
> +}
I can't check the member types like I did for VirtioStatus in PATCH 2
right now. Please double-check them.
> +
> +##
> +# @x-debug-virtio-queue-status:
> +#
> +# Return the status of a given VirtQueue
> +#
> +# @path: QOBject path of the VirtIODevice
> +#
> +# @queue: queue number to examine
> +#
> +# Returns: Status of the VirtQueue
> +#
> +# Since: 6.1
> +#
> +# Example:
> +#
> +# -> { "execute": "x-debug-virtio-queue-status",
> +# "arguments": {
> +# "path": "/machine/peripheral-anon/device[3]/virtio-backend",
> +# "queue": 0
> +# }
> +# }
> +# <- { "return": {
> +# "signalled-used": 373,
> +# "inuse": 0,
> +# "vring-align": 4096,
> +# "vring-desc": 864411648,
> +# "signalled-used-valid": 0,
> +# "vring-num-default": 256,
> +# "vring-avail": 864415744,
> +# "queue-index": 0,
> +# "last-avail-idx": 373,
> +# "vring-used": 864416320,
> +# "used-idx": 373,
> +# "device-type": "virtio-net",
> +# "shadow-avail-idx": 619,
> +# "vring-num": 256
> +# }
> +# }
> +#
> +##
> +
> +{ 'command': 'x-debug-virtio-queue-status',
> + 'data': { 'path': 'str', 'queue': 'uint16' },
> + 'returns': 'VirtQueueStatus'
> +}
On 8/7/21 8:45 AM, Markus Armbruster wrote:
> Jonah Palmer <jonah.palmer@oracle.com> writes:
>
>> From: Laurent Vivier <lvivier@redhat.com>
>>
>> This new command shows internal status of a VirtQueue.
>> (vrings and indexes).
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> [...]
>
>> diff --git a/qapi/virtio.json b/qapi/virtio.json
>> index 78873cd..7007e0c 100644
>> --- a/qapi/virtio.json
>> +++ b/qapi/virtio.json
>> @@ -406,3 +406,105 @@
>> 'data': { 'path': 'str' },
>> 'returns': 'VirtioStatus'
>> }
>> +
>> +##
>> +# @VirtQueueStatus:
>> +#
>> +# Status of a VirtQueue
>> +#
>> +# @device-type: VirtIO device type
>> +#
>> +# @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: 6.1
>> +#
>> +##
>> +
>> +{ 'struct': 'VirtQueueStatus',
>> + 'data': {
>> + 'device-type': 'VirtioType',
>> + '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'
>> + }
>> +}
> I can't check the member types like I did for VirtioStatus in PATCH 2
> right now. Please double-check them.
Double-checked these; 'vring.num', 'vring.num_default', and 'vring.align'
are all of type 'unsigned int'. I think these should be of type 'uint32' here.
Also, 'signalled_used_valid' is of type bool, so I'll change it to 'bool'
here as well.
>
>> +
>> +##
>> +# @x-debug-virtio-queue-status:
>> +#
>> +# Return the status of a given VirtQueue
>> +#
>> +# @path: QOBject path of the VirtIODevice
>> +#
>> +# @queue: queue number to examine
>> +#
>> +# Returns: Status of the VirtQueue
>> +#
>> +# Since: 6.1
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "x-debug-virtio-queue-status",
>> +# "arguments": {
>> +# "path": "/machine/peripheral-anon/device[3]/virtio-backend",
>> +# "queue": 0
>> +# }
>> +# }
>> +# <- { "return": {
>> +# "signalled-used": 373,
>> +# "inuse": 0,
>> +# "vring-align": 4096,
>> +# "vring-desc": 864411648,
>> +# "signalled-used-valid": 0,
>> +# "vring-num-default": 256,
>> +# "vring-avail": 864415744,
>> +# "queue-index": 0,
>> +# "last-avail-idx": 373,
>> +# "vring-used": 864416320,
>> +# "used-idx": 373,
>> +# "device-type": "virtio-net",
>> +# "shadow-avail-idx": 619,
>> +# "vring-num": 256
>> +# }
>> +# }
>> +#
>> +##
>> +
>> +{ 'command': 'x-debug-virtio-queue-status',
>> + 'data': { 'path': 'str', 'queue': 'uint16' },
>> + 'returns': 'VirtQueueStatus'
>> +}
© 2016 - 2025 Red Hat, Inc.