* [PATCH 0/2] vhost-vDPA: doorbell mapping support @ 2021-04-15 8:04 Jason Wang 2021-04-15 8:04 ` [PATCH 1/2] vhost-vdpa: skip ram device from the IOTLB mapping Jason Wang ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Jason Wang @ 2021-04-15 8:04 UTC (permalink / raw) To: mst, qemu-devel; +Cc: elic, Jason Wang Hi All: This series tries to implement doorbell mapping support for vhost-vDPA. Tested with virtio-pci vDPA driver. Please review. Jason Wang (2): vhost-vdpa: skip ram device from the IOTLB mapping vhost-vdpa: doorbell mapping support hw/virtio/vhost-vdpa.c | 97 ++++++++++++++++++++++++++++++---- include/hw/virtio/vhost-vdpa.h | 7 +++ 2 files changed, 94 insertions(+), 10 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] vhost-vdpa: skip ram device from the IOTLB mapping 2021-04-15 8:04 [PATCH 0/2] vhost-vDPA: doorbell mapping support Jason Wang @ 2021-04-15 8:04 ` Jason Wang 2021-04-15 8:04 ` [PATCH 2/2] vhost-vdpa: doorbell mapping support Jason Wang 2021-05-04 8:38 ` [PATCH 0/2] vhost-vDPA: " Michael S. Tsirkin 2 siblings, 0 replies; 8+ messages in thread From: Jason Wang @ 2021-04-15 8:04 UTC (permalink / raw) To: mst, qemu-devel; +Cc: elic, Jason Wang vDPA is not tie to any specific hardware, for safety and simplicity, vhost-vDPA doesn't allow MMIO area to be mapped via IOTLB. Only the doorbell could be mapped via mmap(). So this patch exclude skip the ram device from the IOTLB mapping. Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/virtio/vhost-vdpa.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 01d2101d09..dd4321bac2 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -27,6 +27,8 @@ static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) { return (!memory_region_is_ram(section->mr) && !memory_region_is_iommu(section->mr)) || + /* vhost-vDPA doesn't allow MMIO to be mapped */ + memory_region_is_ram_device(section->mr) || /* * Sizing an enabled 64-bit BAR can cause spurious mappings to * addresses in the upper part of the 64-bit address space. These @@ -171,22 +173,12 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener, vaddr, section->readonly); if (ret) { error_report("vhost vdpa map fail!"); - if (memory_region_is_ram_device(section->mr)) { - /* Allow unexpected mappings not to be fatal for RAM devices */ - error_report("map ram fail!"); - return ; - } goto fail; } return; fail: - if (memory_region_is_ram_device(section->mr)) { - error_report("failed to vdpa_dma_map. pci p2p may not work"); - return; - - } /* * On the initfn path, store the first error in the container so we * can gracefully fail. Runtime, there's not much we can do other -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] vhost-vdpa: doorbell mapping support 2021-04-15 8:04 [PATCH 0/2] vhost-vDPA: doorbell mapping support Jason Wang 2021-04-15 8:04 ` [PATCH 1/2] vhost-vdpa: skip ram device from the IOTLB mapping Jason Wang @ 2021-04-15 8:04 ` Jason Wang 2021-04-30 22:32 ` Si-Wei Liu 2021-05-04 8:38 ` [PATCH 0/2] vhost-vDPA: " Michael S. Tsirkin 2 siblings, 1 reply; 8+ messages in thread From: Jason Wang @ 2021-04-15 8:04 UTC (permalink / raw) To: mst, qemu-devel; +Cc: elic, Jason Wang 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 <jasowang@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)) { + goto err; + } + } + + return; + +err: + vhost_vdpa_host_notifiers_uninit(dev, i); + 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.h index 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; } VhostVDPA; extern AddressSpace address_space_memory; -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] vhost-vdpa: doorbell mapping support 2021-04-15 8:04 ` [PATCH 2/2] vhost-vdpa: doorbell mapping support Jason Wang @ 2021-04-30 22:32 ` Si-Wei Liu 2021-05-06 2:17 ` Jason Wang 0 siblings, 1 reply; 8+ messages in thread From: Si-Wei Liu @ 2021-04-30 22:32 UTC (permalink / raw) To: Jason Wang, mst, qemu-devel; +Cc: elic On 4/15/2021 1:04 AM, Jason Wang wrote: > 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 <jasowang@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)) { 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? If the latter, would be good to add some comment. > + goto err; > + } > + } > + > + return; > + > +err: > + vhost_vdpa_host_notifiers_uninit(dev, i); 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? > + 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.h > index 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; What this host_notifier_set is used for? Doesn't seem it's ever set or referenced? > } VhostVDPA; > > extern AddressSpace address_space_memory; Thanks, -Siwei ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] vhost-vdpa: doorbell mapping support 2021-04-30 22:32 ` Si-Wei Liu @ 2021-05-06 2:17 ` Jason Wang 2021-05-06 21:15 ` Si-Wei Liu 0 siblings, 1 reply; 8+ messages in thread From: Jason Wang @ 2021-05-06 2:17 UTC (permalink / raw) To: Si-Wei Liu, mst, qemu-devel; +Cc: elic 在 2021/5/1 上午6:32, Si-Wei Liu 写道: > > > On 4/15/2021 1:04 AM, Jason Wang wrote: >> 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 <jasowang@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)) { > 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? Only single queue pair is supported, I'm working on the multiqueue support. > 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. >> + goto err; >> + } >> + } >> + >> + return; >> + >> +err: >> + vhost_vdpa_host_notifiers_uninit(dev, i); > 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? 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 > >> + 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.h >> index 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; > What this host_notifier_set is used for? Doesn't seem it's ever set or > referenced? Right, will remove it. Thanks > >> } VhostVDPA; >> extern AddressSpace address_space_memory; > Thanks, > -Siwei > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] vhost-vdpa: doorbell mapping support 2021-05-06 2:17 ` Jason Wang @ 2021-05-06 21:15 ` Si-Wei Liu 0 siblings, 0 replies; 8+ messages in thread From: Si-Wei Liu @ 2021-05-06 21:15 UTC (permalink / raw) To: Jason Wang, mst, qemu-devel; +Cc: elic 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: >>> 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 <jasowang@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)) { >> 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? > > > 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. > > >>> + goto err; >>> + } >>> + } >>> + >>> + return; >>> + >>> +err: >>> + vhost_vdpa_host_notifiers_uninit(dev, i); >> 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? > > > 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 > > >> >>> + 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.h >>> index 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; >> What this host_notifier_set is used for? Doesn't seem it's ever set >> or referenced? > > > Right, will remove it. > > Thanks > > >> >>> } VhostVDPA; >>> extern AddressSpace address_space_memory; >> Thanks, >> -Siwei >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] vhost-vDPA: doorbell mapping support 2021-04-15 8:04 [PATCH 0/2] vhost-vDPA: doorbell mapping support Jason Wang 2021-04-15 8:04 ` [PATCH 1/2] vhost-vdpa: skip ram device from the IOTLB mapping Jason Wang 2021-04-15 8:04 ` [PATCH 2/2] vhost-vdpa: doorbell mapping support Jason Wang @ 2021-05-04 8:38 ` Michael S. Tsirkin 2021-05-06 7:20 ` Jason Wang 2 siblings, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2021-05-04 8:38 UTC (permalink / raw) To: Jason Wang; +Cc: elic, qemu-devel On Thu, Apr 15, 2021 at 04:04:42PM +0800, Jason Wang wrote: > Hi All: > > This series tries to implement doorbell mapping support for > vhost-vDPA. Tested with virtio-pci vDPA driver. BTW I'd rather we used the kick/call terminology from the virtio spec. doorbell terminology seems to be originating from e1000 which has a register named like this. > Please review. > > Jason Wang (2): > vhost-vdpa: skip ram device from the IOTLB mapping > vhost-vdpa: doorbell mapping support > > hw/virtio/vhost-vdpa.c | 97 ++++++++++++++++++++++++++++++---- > include/hw/virtio/vhost-vdpa.h | 7 +++ > 2 files changed, 94 insertions(+), 10 deletions(-) > > -- > 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] vhost-vDPA: doorbell mapping support 2021-05-04 8:38 ` [PATCH 0/2] vhost-vDPA: " Michael S. Tsirkin @ 2021-05-06 7:20 ` Jason Wang 0 siblings, 0 replies; 8+ messages in thread From: Jason Wang @ 2021-05-06 7:20 UTC (permalink / raw) To: qemu-devel 在 2021/5/4 下午4:38, Michael S. Tsirkin 写道: > On Thu, Apr 15, 2021 at 04:04:42PM +0800, Jason Wang wrote: >> Hi All: >> >> This series tries to implement doorbell mapping support for >> vhost-vDPA. Tested with virtio-pci vDPA driver. > BTW I'd rather we used the kick/call terminology from the virtio > spec. doorbell terminology seems to be originating from e1000 > which has a register named like this. Ok, will rename in next version. Thanks > >> Please review. >> >> Jason Wang (2): >> vhost-vdpa: skip ram device from the IOTLB mapping >> vhost-vdpa: doorbell mapping support >> >> hw/virtio/vhost-vdpa.c | 97 ++++++++++++++++++++++++++++++---- >> include/hw/virtio/vhost-vdpa.h | 7 +++ >> 2 files changed, 94 insertions(+), 10 deletions(-) >> >> -- >> 2.25.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-05-06 21:16 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-15 8:04 [PATCH 0/2] vhost-vDPA: doorbell mapping support Jason Wang 2021-04-15 8:04 ` [PATCH 1/2] vhost-vdpa: skip ram device from the IOTLB mapping Jason Wang 2021-04-15 8:04 ` [PATCH 2/2] vhost-vdpa: doorbell mapping support Jason Wang 2021-04-30 22:32 ` Si-Wei Liu 2021-05-06 2:17 ` Jason Wang 2021-05-06 21:15 ` Si-Wei Liu 2021-05-04 8:38 ` [PATCH 0/2] vhost-vDPA: " Michael S. Tsirkin 2021-05-06 7:20 ` Jason Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.