RE: [PATCH RFC server 07/11] vfio-user: handle DMA mappings

[Thanos Makatos]

> -----Original Message-----
> From: Jagannathan Raman <jag.raman@oracle.com>
> Sent: 19 July 2021 21:00
> To: qemu-devel@nongnu.org
> Cc: stefanha@redhat.com; alex.williamson@redhat.com;
> elena.ufimtseva@oracle.com; John Levon <john.levon@nutanix.com>;
> john.g.johnson@oracle.com; Thanos Makatos
> <thanos.makatos@nutanix.com>; Swapnil Ingle
> <swapnil.ingle@nutanix.com>; jag.raman@oracle.com
> Subject: [PATCH RFC server 07/11] vfio-user: handle DMA mappings
> 
> Define and register callbacks to manage the RAM regions used for
> device DMA
> 
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>  hw/remote/vfio-user-obj.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++++
>  hw/remote/trace-events    |  2 ++
>  2 files changed, 60 insertions(+)
> 
> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
> index 60d9fa8..d158a7f 100644
> --- a/hw/remote/vfio-user-obj.c
> +++ b/hw/remote/vfio-user-obj.c
> @@ -161,6 +161,57 @@ static ssize_t vfu_object_cfg_access(vfu_ctx_t
> *vfu_ctx, char * const buf,
>      return count;
>  }
> 
> +static void dma_register(vfu_ctx_t *vfu_ctx, vfu_dma_info_t *info)
> +{
> +    MemoryRegion *subregion = NULL;
> +    g_autofree char *name = NULL;
> +    static unsigned int suffix;
> +    struct iovec *iov = &info->iova;
> +
> +    if (!info->vaddr) {
> +        return;
> +    }

This shouldn't happen, you can replace it with an assert if you want.

> +
> +    name = g_strdup_printf("remote-mem-%u", suffix++);
> +
> +    subregion = g_new0(MemoryRegion, 1);
> +
> +    qemu_mutex_lock_iothread();
> +
> +    memory_region_init_ram_ptr(subregion, NULL, name,
> +                               iov->iov_len, info->vaddr);
> +
> +    memory_region_add_subregion(get_system_memory(), (hwaddr)iov-
> >iov_base,
> +                                subregion);
> +
> +    qemu_mutex_unlock_iothread();
> +
> +    trace_vfu_dma_register((uint64_t)iov->iov_base, iov->iov_len);
> +}
> +
> +static int dma_unregister(vfu_ctx_t *vfu_ctx, vfu_dma_info_t *info)
> +{
> +    MemoryRegion *mr = NULL;
> +    ram_addr_t offset;
> +
> +    mr = memory_region_from_host(info->vaddr, &offset);
> +    if (!mr) {

Is this expected? If not then should we at least log something?

> +        return 0;
> +    }
> +
> +    qemu_mutex_lock_iothread();
> +
> +    memory_region_del_subregion(get_system_memory(), mr);
> +
> +    object_unparent((OBJECT(mr)));
> +
> +    qemu_mutex_unlock_iothread();
> +
> +    trace_vfu_dma_unregister((uint64_t)info->iova.iov_base);
> +
> +    return 0;
> +}
> +
>  static void vfu_object_machine_done(Notifier *notifier, void *data)
>  {
>      VfuObject *o = container_of(notifier, VfuObject, machine_done);
> @@ -208,6 +259,13 @@ static void vfu_object_machine_done(Notifier
> *notifier, void *data)
>          return;
>      }
> 
> +    ret = vfu_setup_device_dma(o->vfu_ctx, &dma_register,
> &dma_unregister);
> +    if (ret < 0) {
> +        error_setg(&error_abort, "vfu: Failed to setup DMA handlers for %s",
> +                   o->devid);
> +        return;
> +    }
> +
>      qemu_thread_create(&o->vfu_ctx_thread, "VFU ctx runner",
> vfu_object_ctx_run,
>                         o, QEMU_THREAD_JOINABLE);
>  }
> diff --git a/hw/remote/trace-events b/hw/remote/trace-events
> index 2ef7884..f945c7e 100644
> --- a/hw/remote/trace-events
> +++ b/hw/remote/trace-events
> @@ -7,3 +7,5 @@ mpqemu_recv_io_error(int cmd, int size, int nfds) "failed
> to receive %d size %d,
>  vfu_prop(const char *prop, const char *val) "vfu: setting %s as %s"
>  vfu_cfg_read(uint32_t offset, uint32_t val) "vfu: cfg: 0x%u -> 0x%x"
>  vfu_cfg_write(uint32_t offset, uint32_t val) "vfu: cfg: 0x%u <- 0x%x"
> +vfu_dma_register(uint64_t gpa, size_t len) "vfu: registering GPA
> 0x%"PRIx64", %zu bytes"
> +vfu_dma_unregister(uint64_t gpa) "vfu: unregistering GPA 0x%"PRIx64""
> --
> 1.8.3.1