[PATCH 19/40] vdpa: should avoid map flushing with persistent iotlb

Si-Wei Liu posted 40 patches 11 months, 3 weeks ago
[PATCH 19/40] vdpa: should avoid map flushing with persistent iotlb
Posted by Si-Wei Liu 11 months, 3 weeks ago
Today memory listener is unregistered in vhost_vdpa_reset_status
unconditionally, due to which all the maps will be flushed away
from the iotlb. However, map flush is not always needed, and
doing it from performance hot path may have innegligible latency
impact that affects VM reboot time or brown out period during
live migration.

Leverage the IOTLB_PERSIST backend featuae, which ensures durable
iotlb maps and not disappearing even across reset. When it is
supported, we may conditionally keep the maps for cases where the
guest memory mapping doesn't change. Prepare a function so that
the next patch will be able to use it to keep the maps.

Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 hw/virtio/trace-events |  1 +
 hw/virtio/vhost-vdpa.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 77905d1..9725d44 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -66,6 +66,7 @@ vhost_vdpa_set_owner(void *dev) "dev: %p"
 vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
 vhost_vdpa_get_iova_range(void *dev, uint64_t first, uint64_t last) "dev: %p first: 0x%"PRIx64" last: 0x%"PRIx64
 vhost_vdpa_set_config_call(void *dev, int fd)"dev: %p fd: %d"
+vhost_vdpa_maybe_flush_map(void *dev, bool reg, bool flush, bool persist) "dev: %p registered: %d flush_map: %d iotlb_persistent: %d"
 
 # virtio.c
 virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index ea2dfc8..31e0a55 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1471,6 +1471,26 @@ out_stop:
     return ok ? 0 : -1;
 }
 
+static void vhost_vdpa_maybe_flush_map(struct vhost_dev *dev)
+{
+    struct vhost_vdpa *v = dev->opaque;
+
+    trace_vhost_vdpa_maybe_flush_map(dev, v->shared->listener_registered,
+                                     v->shared->flush_map,
+                                     !!(dev->backend_cap &
+                                     BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST)));
+
+    if (!v->shared->listener_registered) {
+        return;
+    }
+
+    if (!(dev->backend_cap & BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST)) ||
+        v->shared->flush_map) {
+        memory_listener_unregister(&v->shared->listener);
+        v->shared->listener_registered = false;
+    }
+}
+
 static void vhost_vdpa_reset_status(struct vhost_dev *dev)
 {
     struct vhost_vdpa *v = dev->opaque;
-- 
1.8.3.1
Re: [PATCH 19/40] vdpa: should avoid map flushing with persistent iotlb
Posted by Jason Wang 10 months, 2 weeks ago
On Fri, Dec 8, 2023 at 2:50 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> Today memory listener is unregistered in vhost_vdpa_reset_status
> unconditionally, due to which all the maps will be flushed away
> from the iotlb. However, map flush is not always needed, and
> doing it from performance hot path may have innegligible latency
> impact that affects VM reboot time or brown out period during
> live migration.
>
> Leverage the IOTLB_PERSIST backend featuae, which ensures durable
> iotlb maps and not disappearing even across reset. When it is
> supported, we may conditionally keep the maps for cases where the
> guest memory mapping doesn't change. Prepare a function so that
> the next patch will be able to use it to keep the maps.
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  hw/virtio/trace-events |  1 +
>  hw/virtio/vhost-vdpa.c | 20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 77905d1..9725d44 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -66,6 +66,7 @@ vhost_vdpa_set_owner(void *dev) "dev: %p"
>  vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
>  vhost_vdpa_get_iova_range(void *dev, uint64_t first, uint64_t last) "dev: %p first: 0x%"PRIx64" last: 0x%"PRIx64
>  vhost_vdpa_set_config_call(void *dev, int fd)"dev: %p fd: %d"
> +vhost_vdpa_maybe_flush_map(void *dev, bool reg, bool flush, bool persist) "dev: %p registered: %d flush_map: %d iotlb_persistent: %d"
>
>  # virtio.c
>  virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index ea2dfc8..31e0a55 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1471,6 +1471,26 @@ out_stop:
>      return ok ? 0 : -1;
>  }
>
> +static void vhost_vdpa_maybe_flush_map(struct vhost_dev *dev)

Nit: Not a native speaker, but it looks like
vhost_vdpa_may_flush_map() is better.

> +{
> +    struct vhost_vdpa *v = dev->opaque;
> +
> +    trace_vhost_vdpa_maybe_flush_map(dev, v->shared->listener_registered,
> +                                     v->shared->flush_map,
> +                                     !!(dev->backend_cap &
> +                                     BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST)));
> +
> +    if (!v->shared->listener_registered) {
> +        return;
> +    }
> +
> +    if (!(dev->backend_cap & BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST)) ||
> +        v->shared->flush_map) {
> +        memory_listener_unregister(&v->shared->listener);
> +        v->shared->listener_registered = false;
> +    }

Others look good.

Thanks

> +}
> +
>  static void vhost_vdpa_reset_status(struct vhost_dev *dev)
>  {
>      struct vhost_vdpa *v = dev->opaque;
> --
> 1.8.3.1
>