[PATCH v6 4/6] qmp: add QMP command x-debug-virtio-queue-status
From: Laurent Vivier This new command shows internal status of a VirtQueue. (vrings and indexes). Signed-off-by: Laurent Vivier Signed-off-by: Jonah Palmer --- 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
Re: [PATCH v6 4/6] qmp: add QMP command x-debug-virtio-queue-status
在 2021/7/12 下午6:35, Jonah Palmer 写道: From: Laurent Vivier This new command shows internal status of a VirtQueue. (vrings and indexes). Signed-off-by: Laurent Vivier Signed-off-by: Jonah Palmer --- 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": { +# "si
Re: [PATCH v6 4/6] qmp: add QMP command x-debug-virtio-queue-status
On 7/13/21 10:37 PM, Jason Wang wrote: 在 2021/7/12 下午6:35, Jonah Palmer 写道: From: Laurent Vivier This new command shows internal status of a VirtQueue. (vrings and indexes). Signed-off-by: Laurent Vivier Signed-off-by: Jonah Palmer --- 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 VirtI
Re: [PATCH v6 4/6] qmp: add QMP command x-debug-virtio-queue-status
在 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 This new command shows internal status of a VirtQueue. (vrings and indexes). Signed-off-by: Laurent Vivier Signed-off-by: Jonah Palmer --- 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': 'u
Re: [PATCH v6 4/6] qmp: add QMP command x-debug-virtio-queue-status
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 This new command shows internal status of a VirtQueue. (vrings and indexes). Signed-off-by: Laurent Vivier Signed-off-by: Jonah Palmer --- 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/q
Re: [PATCH v6 4/6] qmp: add QMP command x-debug-virtio-queue-status
Jonah Palmer writes: > From: Laurent Vivier > > This new command shows internal status of a VirtQueue. > (vrings and indexes). > > Signed-off-by: Laurent Vivier > Signed-off-by: Jonah Palmer [...] > 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' > +}
Re: [PATCH v6 4/6] qmp: add QMP command x-debug-virtio-queue-status
On 8/7/21 8:45 AM, Markus Armbruster wrote: Jonah Palmer writes: From: Laurent Vivier This new command shows internal status of a VirtQueue. (vrings and indexes). Signed-off-by: Laurent Vivier Signed-off-by: Jonah Palmer [...] 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' +}
Re: [PATCH v6 4/6] qmp: add QMP command x-debug-virtio-queue-status
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 This new command shows internal status of a VirtQueue. (vrings and indexes). Signed-off-by: Laurent Vivier Signed-off-by: Jonah Palmer --- 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)
Re: [PATCH v6 4/6] qmp: add QMP command x-debug-virtio-queue-status
在 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 This new command shows internal status of a VirtQueue. (vrings and indexes). Signed-off-by: Laurent Vivier Signed-off-by: Jonah Palmer --- 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