On 5/5/2021 7:17 PM, Jason Wang wrote:
在 2021/5/1 上午6:32, Si-Wei Liu 写道:On 4/15/2021 1:04 AM, Jason Wang wrote:Shouldn't (hdev->vq_index + i) be used instead of i? or it's assumed to be single QP for vhost-vdpa for the moment?This patch implements the doorbell mapping support for vhost-vDPA. This is simply done by using mmap()/munmap() for the vhost-vDPA fd during device start/stop. For the device without doorbell support, we fall back to eventfd based notification gracefully. Signed-off-by: Jason Wang <jasow...@redhat.com> ---hw/virtio/vhost-vdpa.c | 85 ++++++++++++++++++++++++++++++++++include/hw/virtio/vhost-vdpa.h | 7 +++ 2 files changed, 92 insertions(+) diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index dd4321bac2..c3a3b7566f 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c@@ -285,12 +285,95 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque)return 0; } +static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev, + int queue_index) +{ + size_t page_size = qemu_real_host_page_size; + struct vhost_vdpa *v = dev->opaque; + VirtIODevice *vdev = dev->vdev; + VhostVDPAHostNotifier *n; + + n = &v->notifier[queue_index]; + + if (n->addr) {+ virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, false);+ object_unparent(OBJECT(&n->mr)); + munmap(n->addr, page_size); + n->addr = NULL; + } +} ++static void vhost_vdpa_host_notifiers_uninit(struct vhost_dev *dev, int n)+{ + int i; + + for (i = 0; i < n; i++) { + vhost_vdpa_host_notifier_uninit(dev, i); + } +} ++static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int queue_index)+{ + size_t page_size = qemu_real_host_page_size; + struct vhost_vdpa *v = dev->opaque; + VirtIODevice *vdev = dev->vdev; + VhostVDPAHostNotifier *n; + int fd = v->device_fd; + void *addr; + char *name; + + vhost_vdpa_host_notifier_uninit(dev, queue_index); + + n = &v->notifier[queue_index]; + + addr = mmap(NULL, page_size, PROT_WRITE, MAP_SHARED, fd, + queue_index * page_size); + if (addr == MAP_FAILED) { + goto err; + } + + name = g_strdup_printf("vhost-vdpa/host-notifier@%p mmaps[%d]", + v, queue_index); + memory_region_init_ram_device_ptr(&n->mr, OBJECT(vdev), name, + page_size, addr); + g_free(name); ++ if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {+ munmap(addr, page_size); + goto err; + } + n->addr = addr; + + return 0; + +err: + return -1; +} + +static void vhost_vdpa_host_notifiers_init(struct vhost_dev *dev) +{ + int i; + + for (i = 0; i < dev->nvqs; i++) { + if (vhost_vdpa_host_notifier_init(dev, i)) {Only single queue pair is supported, I'm working on the multiqueue support.
OK, I see.
If the latter, would be good to add some comment.I agree, and I think it's better to use vq_index + i to avoid future changes.
That'll be fine. I think that depends on the way how mq will be modeled for vhost-vdpa, i.e. it doesn't need to be 1:1 between struct vhost_dev and a queue pair, like what vhost-kernel is modeled after for mq.
I'm not sure if it is really the intent to leave other vqs behind - I presume that either none of them is mapped, or all mappable should be mapped. Why here just uninit the first unmappable vq?+ goto err; + } + } + + return; + +err: + vhost_vdpa_host_notifiers_uninit(dev, i);I'm not sure I get here, there's a loop in vhost_vdpa_host_notifiers_uninit(), so we either:1) map all doorbells or 2) no doorell is mapped
Oops, I missed the 's' in vhost_vdpa_host_notifiers_uninit() and thought it was vhost_vdpa_host_notifier_uninit(). Sorry for the false alarm. The error handling looks fine then.
Thanks! -Siwei
What this host_notifier_set is used for? Doesn't seem it's ever set or referenced?+ return; +} + static int vhost_vdpa_cleanup(struct vhost_dev *dev) { struct vhost_vdpa *v; assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); v = dev->opaque; trace_vhost_vdpa_cleanup(dev, v); + vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs); memory_listener_unregister(&v->listener); dev->opaque = NULL;@@ -467,6 +550,7 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)if (started) { uint8_t status = 0;memory_listener_register(&v->listener, &address_space_memory);+ vhost_vdpa_host_notifiers_init(dev); vhost_vdpa_set_vring_ready(dev); vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK); vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);@@ -476,6 +560,7 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)vhost_vdpa_reset_device(dev); vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER); + vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs); memory_listener_unregister(&v->listener); return 0;diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.hindex 9b81a409da..0f11ecff34 100644 --- a/include/hw/virtio/vhost-vdpa.h +++ b/include/hw/virtio/vhost-vdpa.h @@ -14,11 +14,18 @@ #include "hw/virtio/virtio.h" +typedef struct VhostVDPAHostNotifier { + MemoryRegion mr; + void *addr; +} VhostVDPAHostNotifier; + typedef struct vhost_vdpa { int device_fd; uint32_t msg_type; MemoryListener listener; struct vhost_dev *dev; + VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX]; + bool host_notifier_set;Right, will remove it. Thanks} VhostVDPA; extern AddressSpace address_space_memory;Thanks, -Siwei