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

[Jason Wang]

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
>