* Re: [RFC v3 1/6] qmp: add QMP command x-debug-query-virtio [not found] ` <20200507114927.6733-2-lvivier@redhat.com> @ 2020-05-07 15:38 ` Eric Blake 0 siblings, 0 replies; 8+ messages in thread From: Eric Blake @ 2020-05-07 15:38 UTC (permalink / raw) To: Laurent Vivier, qemu-devel Cc: Fam Zheng, Kevin Wolf, Thomas Huth, qemu-block, David Hildenbrand, Markus Armbruster, Jason Wang, Michael S. Tsirkin, Michael Roth, Amit Shah, Max Reitz, Eric Auger, Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini, Dr. David Alan Gilbert On 5/7/20 6:49 AM, Laurent Vivier wrote: > This new command lists all the instances of VirtIODevice with > their path and virtio type > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > +++ b/qapi/virtio.json > @@ -0,0 +1,68 @@ > +## > +# = Virtio devices > +## > + > +## > +# @VirtioType: > +# > +# An enumeration of Virtio device types. > +# > +# Since: 5.1.0 Inconsistent on x.y.z vs. > +## > +{ 'enum': 'VirtioType', > + 'data': [ 'unknown', 'virtio-9p', 'virtio-blk', 'virtio-serial', > + 'virtio-gpu', 'virtio-input', 'virtio-net', 'virtio-scsi', > + 'vhost-user-fs', 'vhost-vsock', 'virtio-balloon', 'virtio-crypto', > + 'virtio-iommu', 'virtio-pmem', 'virtio-rng' ] > +} > + > +## > +# @VirtioInfo: > +# > +# Information about a given VirtIODevice > +# > +# @path: VirtIO device canonical path. > +# > +# @type: VirtIO device type. > +# > +# Since: 5.1 x.y. Most of our QAPI seems to have settled on just x.y these days. The x-debug- prefix is nice; it gives us more flexibility to mess with this later as needed. Otherwise: Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20200507114927.6733-4-lvivier@redhat.com>]
* Re: [RFC v3 3/6] qmp: decode feature bits in virtio-status [not found] ` <20200507114927.6733-4-lvivier@redhat.com> @ 2020-05-07 15:47 ` Eric Blake 2020-05-08 2:54 ` Jason Wang 1 sibling, 0 replies; 8+ messages in thread From: Eric Blake @ 2020-05-07 15:47 UTC (permalink / raw) To: Laurent Vivier, qemu-devel Cc: Fam Zheng, Kevin Wolf, Thomas Huth, qemu-block, David Hildenbrand, Markus Armbruster, Jason Wang, Michael S. Tsirkin, Michael Roth, Amit Shah, Max Reitz, Eric Auger, Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini, Dr. David Alan Gilbert On 5/7/20 6:49 AM, Laurent Vivier wrote: > Display feature names instead of a features bitmap for host, guest > and backend. > > Decode features according device type, transport features are > on the first line. Undecoded bits (if any) are stored in a separate > field. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- I didn't closely review the code, but for the QAPI parts: > +++ b/qapi/virtio.json > +## > +# @VirtioBlkFeature: > +# > +# An enumeration of Virtio block features > +# > +# Since: 5.1 > +## > + > +{ 'enum': 'VirtioBlkFeature', > + 'data': [ 'size-max', 'seg-max', 'geometry', 'ro', 'blk-size', 'topology', 'mq', 'discard', 'write-zeroes', 'barrier', 'scsi', 'flush', Missing newline. > +## > +# @VirtioDeviceFeatures: > +# > +# An union to define the list of features for a virtio device s/An/A/ (in English, 'an' goes with soft u, 'a' goes with pronounced u. You can remember with "a unicorn gets an umbrella") > +# > +# Since: 5.1 > +## > + > +{ 'union': 'VirtioDeviceFeatures', > + 'data': { > + 'virtio-serial': [ 'VirtioSerialFeature' ], > + 'virtio-blk': [ 'VirtioBlkFeature' ], > + 'virtio-gpu': [ 'VirtioGpuFeature' ], > + 'virtio-net': [ 'VirtioNetFeature' ], > + 'virtio-scsi': [ 'VirtioScsiFeature' ], > + 'virtio-balloon': [ 'VirtioBalloonFeature' ], > + 'virtio-iommu': [ 'VirtioIommuFeature' ] > + } > +} This is a legacy union rather than a flat union, which results in more {} in the QMP wire format. Is it worth trying to make this a flat union, by labeling an appropriate member as 'discriminator'? > + > +## > +# @VirtioStatusFeatures: > +# > +# @transport: the list of transport features of the virtio device > +# > +# @device: the list of the virtio device specific features > +# > +# @unknown: virtio bitmap of the undecoded features > +# > +# Since: 5.1 > +## > + > +{ 'struct': 'VirtioStatusFeatures', > + 'data': { 'transport': [ 'VirtioTransportFeature' ], > + 'device': 'VirtioDeviceFeatures', > + 'unknown': 'uint64' } > +} > + > ## > # @VirtioStatus: > # > @@ -101,9 +245,9 @@ > 'data': { > 'device-id': 'int', > 'device-endian': 'VirtioStatusEndianness', > - 'guest-features': 'uint64', > - 'host-features': 'uint64', > - 'backend-features': 'uint64', > + 'guest-features': 'VirtioStatusFeatures', > + 'host-features': 'VirtioStatusFeatures', > + 'backend-features': 'VirtioStatusFeatures', This is intra-series churn. Should we be trying to get the right types in place from the get-go? Or at least clarify in the commit message of the earlier patch that the format will be incrementally improved later? > 'num-vqs': 'uint16' > } > } > @@ -123,18 +267,40 @@ > # > # -> { "execute": "x-debug-virtio-status", > # "arguments": { > -# "path": "/machine/peripheral-anon/device[3]/virtio-backend" > +# "path": "/machine/peripheral-anon/device[1]/virtio-backend" > # } > # } > # <- { "return": { > -# "backend-features": 0, > -# "guest-features": 5111807911, > -# "num-vqs": 3, > -# "host-features": 6337593319, > # "device-endian": "little", > -# "device-id": 1 > +# "device-id": 3, > +# "backend-features": { > +# "device": { > +# "type": "virtio-serial", > +# "data": [] > +# }, > +# "unknown": 0, > +# "transport": [] > +# }, If we use a flat union, this could look like: "backend-feature": { "type": "virtio-serial", "features": [], "unknown": 0, "transport": [] }, Should 'unknown' be optional so it can be omitted when zero? Should it be named 'unknown-features'? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC v3 3/6] qmp: decode feature bits in virtio-status [not found] ` <20200507114927.6733-4-lvivier@redhat.com> 2020-05-07 15:47 ` [RFC v3 3/6] qmp: decode feature bits in virtio-status Eric Blake @ 2020-05-08 2:54 ` Jason Wang 1 sibling, 0 replies; 8+ messages in thread From: Jason Wang @ 2020-05-08 2:54 UTC (permalink / raw) To: Laurent Vivier, qemu-devel Cc: Fam Zheng, Kevin Wolf, Thomas Huth, qemu-block, David Hildenbrand, Markus Armbruster, Michael S. Tsirkin, Michael Roth, Amit Shah, Max Reitz, Eric Auger, Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini, Dr. David Alan Gilbert On 2020/5/7 下午7:49, Laurent Vivier wrote: > Display feature names instead of a features bitmap for host, guest > and backend. > > Decode features according device type, transport features are > on the first line. Undecoded bits (if any) are stored in a separate > field. > > Signed-off-by: Laurent Vivier<lvivier@redhat.com> This is really useful. I wonder maybe we need something similar in driver side, current sysfs can only display magic binary numbers. Thanks ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20200507114927.6733-7-lvivier@redhat.com>]
* Re: [RFC v3 6/6] hmp: add x-debug-virtio commands [not found] ` <20200507114927.6733-7-lvivier@redhat.com> @ 2020-05-07 15:51 ` Eric Blake 0 siblings, 0 replies; 8+ messages in thread From: Eric Blake @ 2020-05-07 15:51 UTC (permalink / raw) To: Laurent Vivier, qemu-devel Cc: Fam Zheng, Kevin Wolf, Thomas Huth, qemu-block, David Hildenbrand, Markus Armbruster, Jason Wang, Michael S. Tsirkin, Michael Roth, Amit Shah, Max Reitz, Eric Auger, Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini, Dr. David Alan Gilbert On 5/7/20 6:49 AM, Laurent Vivier wrote: > This patch implements HMP version of the virtio QMP commands > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> Most HMP commands do not use '-' in their name. Also, HMP doesn't promise api compatibility; so we could just name this 'info virtio' or 'debug_virtio' without an x- prefix, with no real loss. > --- > Makefile | 2 +- > Makefile.target | 7 +- > docs/system/monitor.rst | 2 + > hmp-commands-virtio.hx | 160 +++++++++++++++++++++++++++++++++ > hmp-commands.hx | 10 +++ > hw/virtio/virtio.c | 193 +++++++++++++++++++++++++++++++++++++++- > include/monitor/hmp.h | 4 + > monitor/misc.c | 17 ++++ > 8 files changed, 391 insertions(+), 4 deletions(-) > create mode 100644 hmp-commands-virtio.hx > > +SRST > +``x-debug-virtio`` *subcommand* > + Show various information about virtio. > + > + Example: > + > + List all sub-commands:: > + > + (qemu) x-debug-virtio > + x-debug-virtio query -- List all available virtio devices > + x-debug-virtio status path -- Display status of a given virtio device > + x-debug-virtio queue-status path queue -- Display status of a given virtio queue > + x-debug-virtio queue-element path queue [index] -- Display element of a given virtio queue Oh, you're introducing it as a metacommand (like 'info' already is) with several subcommands. Still, naming it 'virtio' instead of 'x-debug-virtio' would make sense to me, even though the underlying QMP commands are (correctly) in the x-debug- namespace. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20200507114927.6733-5-lvivier@redhat.com>]
* Re: [RFC v3 4/6] qmp: add QMP command x-debug-virtio-queue-status [not found] ` <20200507114927.6733-5-lvivier@redhat.com> @ 2020-05-08 2:57 ` Jason Wang 2020-05-15 15:16 ` Laurent Vivier 0 siblings, 1 reply; 8+ messages in thread From: Jason Wang @ 2020-05-08 2:57 UTC (permalink / raw) To: Laurent Vivier, qemu-devel Cc: Fam Zheng, Kevin Wolf, Thomas Huth, qemu-block, David Hildenbrand, Markus Armbruster, Michael S. Tsirkin, Michael Roth, Amit Shah, Max Reitz, Eric Auger, Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini, Dr. David Alan Gilbert On 2020/5/7 下午7:49, Laurent Vivier wrote: > This new command shows internal status of a VirtQueue. > (vrings and indexes). > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> It looks to me that packed virtqueue is not supported. It's better to add them in the future. > --- > hw/virtio/virtio-stub.c | 6 +++ > hw/virtio/virtio.c | 35 +++++++++++++++ > qapi/virtio.json | 98 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 139 insertions(+) > > diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c > index ddb592f72eee..3c1bf172acf6 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 59bf6ef651a6..57552bf05014 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -3877,6 +3877,41 @@ 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->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; This might not be correct when vhost is used. We may consider to sync last_avail_idx from vhost backends here? Thanks > + 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 69dd107d0c9b..43c234a9fc69 100644 > --- a/qapi/virtio.json > +++ b/qapi/virtio.json > @@ -308,3 +308,101 @@ > 'data': { 'path': 'str' }, > 'returns': 'VirtioStatus' > } > + > +## > +# @VirtQueueStatus: > +# > +# Status of a VirtQueue > +# > +# @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: 5.1 > +# > +## > + > +{ 'struct': 'VirtQueueStatus', > + 'data': { > + '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: 5.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-desc": 864411648, > +# "vring-num-default": 256, > +# "signalled-used-valid": 1, > +# "vring-avail": 864415744, > +# "last-avail-idx": 373, > +# "queue-index": 0, > +# "vring-used": 864416320, > +# "shadow-avail-idx": 619, > +# "used-idx": 373, > +# "vring-num": 256, > +# "vring-align": 4096 > +# } > +# } > +# > +## > + > +{ 'command': 'x-debug-virtio-queue-status', > + 'data': { 'path': 'str', 'queue': 'uint16' }, > + 'returns': 'VirtQueueStatus' > +} ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC v3 4/6] qmp: add QMP command x-debug-virtio-queue-status 2020-05-08 2:57 ` [RFC v3 4/6] qmp: add QMP command x-debug-virtio-queue-status Jason Wang @ 2020-05-15 15:16 ` Laurent Vivier 2020-05-18 3:31 ` Jason Wang 0 siblings, 1 reply; 8+ messages in thread From: Laurent Vivier @ 2020-05-15 15:16 UTC (permalink / raw) To: Jason Wang, qemu-devel Cc: Fam Zheng, Kevin Wolf, Thomas Huth, qemu-block, David Hildenbrand, Markus Armbruster, Michael S. Tsirkin, Michael Roth, Amit Shah, Max Reitz, Eric Auger, Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini, Dr. David Alan Gilbert On 08/05/2020 04:57, Jason Wang wrote: > > On 2020/5/7 下午7:49, Laurent Vivier wrote: >> This new command shows internal status of a VirtQueue. >> (vrings and indexes). >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> > > > It looks to me that packed virtqueue is not supported. It's better to > add them in the future. I agree, it's why the series still remains an "RFC". ... >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> index 59bf6ef651a6..57552bf05014 100644 >> --- a/hw/virtio/virtio.c >> +++ b/hw/virtio/virtio.c >> @@ -3877,6 +3877,41 @@ 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->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; > > > This might not be correct when vhost is used. > > We may consider to sync last_avail_idx from vhost backends here? Yes, but I don't know how to do that. Where can I find the information? Thanks, Laurent ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC v3 4/6] qmp: add QMP command x-debug-virtio-queue-status 2020-05-15 15:16 ` Laurent Vivier @ 2020-05-18 3:31 ` Jason Wang 0 siblings, 0 replies; 8+ messages in thread From: Jason Wang @ 2020-05-18 3:31 UTC (permalink / raw) To: Laurent Vivier, qemu-devel Cc: Fam Zheng, Kevin Wolf, Thomas Huth, qemu-block, David Hildenbrand, Markus Armbruster, Michael S. Tsirkin, Michael Roth, Amit Shah, Max Reitz, Eric Auger, Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini, Dr. David Alan Gilbert On 2020/5/15 下午11:16, Laurent Vivier wrote: > On 08/05/2020 04:57, Jason Wang wrote: >> On 2020/5/7 下午7:49, Laurent Vivier wrote: >>> This new command shows internal status of a VirtQueue. >>> (vrings and indexes). >>> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> >> It looks to me that packed virtqueue is not supported. It's better to >> add them in the future. > I agree, it's why the series still remains an "RFC". > > ... >>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >>> index 59bf6ef651a6..57552bf05014 100644 >>> --- a/hw/virtio/virtio.c >>> +++ b/hw/virtio/virtio.c >>> @@ -3877,6 +3877,41 @@ 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->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; >> >> This might not be correct when vhost is used. >> >> We may consider to sync last_avail_idx from vhost backends here? > Yes, but I don't know how to do that. Where can I find the information? It could be synced through vhost ops vhost_get_vring_base(), see vhost_virtqueue_stop(). Thanks > > Thanks, > Laurent ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20200507114927.6733-6-lvivier@redhat.com>]
* Re: [RFC v3 5/6] qmp: add QMP command x-debug-virtio-queue-element [not found] ` <20200507114927.6733-6-lvivier@redhat.com> @ 2020-05-08 3:03 ` Jason Wang 0 siblings, 0 replies; 8+ messages in thread From: Jason Wang @ 2020-05-08 3:03 UTC (permalink / raw) To: Laurent Vivier, qemu-devel Cc: Fam Zheng, Kevin Wolf, Thomas Huth, qemu-block, David Hildenbrand, Markus Armbruster, Michael S. Tsirkin, Michael Roth, Amit Shah, Max Reitz, Eric Auger, Gerd Hoffmann, Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini, Dr. David Alan Gilbert On 2020/5/7 下午7:49, Laurent Vivier wrote: > This new command shows the information of a VirtQueue element. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > hw/virtio/virtio-stub.c | 9 +++ > hw/virtio/virtio.c | 130 ++++++++++++++++++++++++++++++++++++++++ > qapi/virtio.json | 94 +++++++++++++++++++++++++++++ > 3 files changed, 233 insertions(+) > > diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c > index 3c1bf172acf6..8275e31430e7 100644 > --- a/hw/virtio/virtio-stub.c > +++ b/hw/virtio/virtio-stub.c > @@ -23,3 +23,12 @@ VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path, > { > return qmp_virtio_unsupported(errp); > } > + > +VirtioQueueElement *qmp_x_debug_virtio_queue_element(const char* path, > + uint16_t queue, > + bool has_index, > + uint16_t index, > + Error **errp) > +{ > + return qmp_virtio_unsupported(errp); > +} > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 57552bf05014..66dc2cef1b39 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -4033,6 +4033,136 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp) > return status; > } > > +static VirtioRingDescFlagsList *qmp_decode_vring_desc_flags(uint16_t flags) > +{ > + VirtioRingDescFlagsList *list = NULL; > + VirtioRingDescFlagsList *node; > + int i; > + struct { > + uint16_t flag; > + VirtioRingDescFlags value; > + } map[] = { > + { VRING_DESC_F_NEXT, VIRTIO_RING_DESC_FLAGS_NEXT }, > + { VRING_DESC_F_WRITE, VIRTIO_RING_DESC_FLAGS_WRITE }, > + { VRING_DESC_F_INDIRECT, VIRTIO_RING_DESC_FLAGS_INDIRECT }, > + { 1 << VRING_PACKED_DESC_F_AVAIL, VIRTIO_RING_DESC_FLAGS_AVAIL }, > + { 1 << VRING_PACKED_DESC_F_USED, VIRTIO_RING_DESC_FLAGS_USED }, > + { 0, -1 } > + }; > + > + for (i = 0; map[i].flag; i++) { > + if ((map[i].flag & flags) == 0) { > + continue; > + } > + node = g_malloc0(sizeof(VirtioRingDescFlagsList)); > + node->value = map[i].value; > + node->next = list; > + list = node; > + } > + > + return list; > +} > + > +VirtioQueueElement *qmp_x_debug_virtio_queue_element(const char* path, > + uint16_t queue, > + bool has_index, > + uint16_t index, > + Error **errp) > +{ > + VirtIODevice *vdev; > + VirtQueue *vq; > + VirtioQueueElement *element = NULL; > + > + 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; > + } > + vq = &vdev->vq[queue]; > + > + if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) { > + error_setg(errp, "Packed ring not supported"); > + return NULL; > + } else { > + unsigned int head, i, max; > + VRingMemoryRegionCaches *caches; > + MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID; > + MemoryRegionCache *desc_cache; > + VRingDesc desc; > + VirtioRingDescList *list = NULL; > + VirtioRingDescList *node; > + int rc; > + > + RCU_READ_LOCK_GUARD(); > + > + max = vq->vring.num; > + > + if (!has_index) { > + head = vring_avail_ring(vq, vq->last_avail_idx % vq->vring.num); > + } else { > + head = vring_avail_ring(vq, index % vq->vring.num); > + } > + i = head; > + > + caches = vring_get_region_caches(vq); > + if (!caches) { > + error_setg(errp, "Region caches not initialized"); > + return NULL; > + } > + > + if (caches->desc.len < max * sizeof(VRingDesc)) { > + error_setg(errp, "Cannot map descriptor ring"); > + return NULL; > + } > + > + desc_cache = &caches->desc; > + vring_split_desc_read(vdev, &desc, desc_cache, i); > + if (desc.flags & VRING_DESC_F_INDIRECT) { > + int64_t len; > + > + len = address_space_cache_init(&indirect_desc_cache, vdev->dma_as, > + desc.addr, desc.len, false); > + desc_cache = &indirect_desc_cache; > + if (len < desc.len) { > + error_setg(errp, "Cannot map indirect buffer"); > + goto done; > + } > + i = 0; > + vring_split_desc_read(vdev, &desc, desc_cache, i); > + } > + > + element = g_new0(VirtioQueueElement, 1); > + element->index = head; > + element->ndescs = 0; > + > + do { > + node = g_new0(VirtioRingDescList, 1); > + node->value = g_new0(VirtioRingDesc, 1); > + node->value->addr = desc.addr; > + node->value->len = desc.len; > + node->value->flags = qmp_decode_vring_desc_flags(desc.flags); > + node->next = list; > + list = node; > + > + element->ndescs++; > + > + rc = virtqueue_split_read_next_desc(vdev, &desc, desc_cache, > + max, &i); > + } while (rc == VIRTQUEUE_READ_DESC_MORE); It's better to limit the maximum number of iterations here to e.g vring.num. A buggy driver may produce a infinite loop here. Thanks > + > + element->descs = list; > +done: > + address_space_cache_destroy(&indirect_desc_cache); > + } > + > + return element; > +} > + > static const TypeInfo virtio_device_info = { > .name = TYPE_VIRTIO_DEVICE, > .parent = TYPE_DEVICE, > diff --git a/qapi/virtio.json b/qapi/virtio.json > index 43c234a9fc69..a55103dca780 100644 > --- a/qapi/virtio.json > +++ b/qapi/virtio.json > @@ -406,3 +406,97 @@ > 'data': { 'path': 'str', 'queue': 'uint16' }, > 'returns': 'VirtQueueStatus' > } > + > +## > +# @VirtioRingDescFlags: > +# > +# An enumeration of the virtio ring descriptor flags > +# > +# Since: 5.1 > +# > +## > + > +{ 'enum': 'VirtioRingDescFlags', > + 'data': [ 'next', 'write', 'indirect', 'avail', 'used' ] > +} > + > +## > +# @VirtioRingDesc: > +# > +# @addr: guest physical address of the descriptor data > +# > +# @len: length of the descriptor data > +# > +# @flags: descriptor flags > +# > +# Since: 5.1 > +# > +## > + > +{ 'struct': 'VirtioRingDesc', > + 'data': { > + 'addr': 'uint64', > + 'len': 'uint32', > + 'flags': [ 'VirtioRingDescFlags' ] > + } > +} > + > +## > +# @VirtioQueueElement: > +# > +# @index: index of the element in the queue > +# > +# @ndescs: number of descriptors > +# > +# @descs: list of the descriptors > +# > +# Since: 5.1 > +# > +## > + > +{ 'struct': 'VirtioQueueElement', > + 'data': { > + 'index': 'uint32', > + 'ndescs': 'uint32', > + 'descs': ['VirtioRingDesc'] > + } > +} > + > +## > +# @x-debug-virtio-queue-element: > +# > +# Return the information about an element queue (by default head) > +# > +# @path: QOBject path of the VirtIODevice > +# > +# @queue: queue number to examine > +# > +# @index: the index in the queue, by default head > +# > +# Returns: the element information > +# > +# Since: 5.1 > +# > +# Example: > +# > +# -> { "execute": "x-debug-virtio-queue-element", > +# "arguments": { > +# "path": "/machine/peripheral-anon/device[3]/virtio-backend", > +# "queue": 0 > +# } > +# } > +# -> { "return": { > +# "index": 24, > +# "ndescs": 1, > +# "descs": [ > +# { "flags": ["write"], "len": 1536, "addr": 2027557376 } > +# ] > +# } > +# } > +# > +## > + > +{ 'command': 'x-debug-virtio-queue-element', > + 'data': { 'path': 'str', 'queue': 'uint16', '*index': 'uint16' }, > + 'returns': 'VirtioQueueElement' > +} ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-18 3:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200507114927.6733-1-lvivier@redhat.com> [not found] ` <20200507114927.6733-2-lvivier@redhat.com> 2020-05-07 15:38 ` [RFC v3 1/6] qmp: add QMP command x-debug-query-virtio Eric Blake [not found] ` <20200507114927.6733-4-lvivier@redhat.com> 2020-05-07 15:47 ` [RFC v3 3/6] qmp: decode feature bits in virtio-status Eric Blake 2020-05-08 2:54 ` Jason Wang [not found] ` <20200507114927.6733-7-lvivier@redhat.com> 2020-05-07 15:51 ` [RFC v3 6/6] hmp: add x-debug-virtio commands Eric Blake [not found] ` <20200507114927.6733-5-lvivier@redhat.com> 2020-05-08 2:57 ` [RFC v3 4/6] qmp: add QMP command x-debug-virtio-queue-status Jason Wang 2020-05-15 15:16 ` Laurent Vivier 2020-05-18 3:31 ` Jason Wang [not found] ` <20200507114927.6733-6-lvivier@redhat.com> 2020-05-08 3:03 ` [RFC v3 5/6] qmp: add QMP command x-debug-virtio-queue-element Jason Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).