[v8,13/17] vfio-user: handle DMA mappings

Commit Message

Jag Raman April 19, 2022, 8:44 p.m. UTC

Define and register callbacks to manage the RAM regions used for
device DMA

Signed-off-by: Elena Ufimtseva <[e-mail]>
Signed-off-by: John G Johnson <[e-mail]>
Signed-off-by: Jagannathan Raman <[e-mail]>
Reviewed-by: Stefan Hajnoczi <[e-mail]>
---
 hw/remote/machine.c       |  5 ++++
 hw/remote/vfio-user-obj.c | 55 +++++++++++++++++++++++++++++++++++++++
 hw/remote/trace-events    |  2 ++
 3 files changed, 62 insertions(+)

Comments

Stefan Hajnoczi April 25, 2022, 9:56 a.m. UTC | #1

On Tue, Apr 19, 2022 at 04:44:18PM -0400, Jagannathan Raman wrote:
> +static void dma_unregister(vfu_ctx_t *vfu_ctx, vfu_dma_info_t *info)
> +{
> +    VfuObject *o = vfu_get_private(vfu_ctx);
> +    AddressSpace *dma_as = NULL;
> +    MemoryRegion *mr = NULL;
> +    ram_addr_t offset;
> +
> +    mr = memory_region_from_host(info->vaddr, &offset);
> +    if (!mr) {
> +        return;
> +    }
> +
> +    dma_as = pci_device_iommu_address_space(o->pci_dev);
> +
> +    memory_region_del_subregion(dma_as->root, mr);
> +
> +    object_unparent((OBJECT(mr)));

Where is obj->parent set?

If it is not set then this call is a nop and mr is not freed:

  void object_unparent(Object *obj)
  {
      if (obj->parent) {
          object_property_del_child(obj->parent, obj);
      }
  }

Jag Raman April 25, 2022, 5:34 p.m. UTC | #2

> On Apr 25, 2022, at 5:56 AM, Stefan Hajnoczi <[e-mail]> wrote:
> 
> On Tue, Apr 19, 2022 at 04:44:18PM -0400, Jagannathan Raman wrote:
>> +static void dma_unregister(vfu_ctx_t *vfu_ctx, vfu_dma_info_t *info)
>> +{
>> +    VfuObject *o = vfu_get_private(vfu_ctx);
>> +    AddressSpace *dma_as = NULL;
>> +    MemoryRegion *mr = NULL;
>> +    ram_addr_t offset;
>> +
>> +    mr = memory_region_from_host(info->vaddr, &offset);
>> +    if (!mr) {
>> +        return;
>> +    }
>> +
>> +    dma_as = pci_device_iommu_address_space(o->pci_dev);
>> +
>> +    memory_region_del_subregion(dma_as->root, mr);
>> +
>> +    object_unparent((OBJECT(mr)));
> 
> Where is obj->parent set?

Yeah, it should be object_unref().

Thank you!
--
Jag

> 
> If it is not set then this call is a nop and mr is not freed:
> 
>  void object_unparent(Object *obj)
>  {
>      if (obj->parent) {
>          object_property_del_child(obj->parent, obj);
>      }
>  }

Jag Raman April 26, 2022, 7:53 p.m. UTC | #3

> On Apr 25, 2022, at 1:34 PM, Jag Raman <[e-mail]> wrote:
> 
> 
> 
>> On Apr 25, 2022, at 5:56 AM, Stefan Hajnoczi <[e-mail]> wrote:
>> 
>> On Tue, Apr 19, 2022 at 04:44:18PM -0400, Jagannathan Raman wrote:
>>> +static void dma_unregister(vfu_ctx_t *vfu_ctx, vfu_dma_info_t *info)
>>> +{
>>> + VfuObject *o = vfu_get_private(vfu_ctx);
>>> + AddressSpace *dma_as = NULL;
>>> + MemoryRegion *mr = NULL;
>>> + ram_addr_t offset;
>>> +
>>> + mr = memory_region_from_host(info->vaddr, &offset);
>>> + if (!mr) {
>>> + return;
>>> + }
>>> +
>>> + dma_as = pci_device_iommu_address_space(o->pci_dev);
>>> +
>>> + memory_region_del_subregion(dma_as->root, mr);
>>> +
>>> + object_unparent((OBJECT(mr)));
>> 
>> Where is obj->parent set?
> 
> Yeah, it should be object_unref().

I think object_unparent() is the appropriate way to finalize the
MemoryRegion in this case.

‘mr’ is an owner-less MemoryRegion created by dma_register(). owner-less
MemoryRegions initialized using memory_region_init* functions are children
of the “/unattached” object.

The parent is set by dma_register() -> memory_region_init_ram_ptr() ->
memory_region_init() -> memory_region_do_init() ->
object_property_add_child() -> object_property_try_add_child().

Thank you!
--
Jag

> 
> Thank you!
> --
> Jag
> 
>> 
>> If it is not set then this call is a nop and mr is not freed:
>> 
>> void object_unparent(Object *obj)
>> {
>> if (obj->parent) {
>> object_property_del_child(obj->parent, obj);
>> }
>> }

Patch

diff --git a/hw/remote/machine.c b/hw/remote/machine.c
index cca5d25f50..7002d46980 100644
--- a/hw/remote/machine.c
+++ b/hw/remote/machine.c
@@ -23,6 +23,7 @@ 
 #include "hw/remote/iohub.h"
 #include "hw/remote/iommu.h"
 #include "hw/qdev-core.h"
+#include "hw/remote/iommu.h"
 
 static void remote_machine_init(MachineState *machine)
 {
@@ -52,6 +53,10 @@  static void remote_machine_init(MachineState *machine)
 
     pci_host = PCI_HOST_BRIDGE(rem_host);
 
+    if (s->vfio_user) {
+        remote_iommu_setup(pci_host->bus);
+    }
+
     remote_iohub_init(&s->iohub);
 
     pci_bus_irqs(pci_host->bus, remote_iohub_set_irq, remote_iohub_map_irq,
diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index 7b863dec4f..425e45e8b2 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -276,6 +276,54 @@  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)
+{
+    VfuObject *o = vfu_get_private(vfu_ctx);
+    AddressSpace *dma_as = NULL;
+    MemoryRegion *subregion = NULL;
+    g_autofree char *name = NULL;
+    struct iovec *iov = &info->iova;
+
+    if (!info->vaddr) {
+        return;
+    }
+
+    name = g_strdup_printf("mem-%s-%"PRIx64"", o->device,
+                           (uint64_t)info->vaddr);
+
+    subregion = g_new0(MemoryRegion, 1);
+
+    memory_region_init_ram_ptr(subregion, NULL, name,
+                               iov->iov_len, info->vaddr);
+
+    dma_as = pci_device_iommu_address_space(o->pci_dev);
+
+    memory_region_add_subregion(dma_as->root, (hwaddr)iov->iov_base, subregion);
+
+    trace_vfu_dma_register((uint64_t)iov->iov_base, iov->iov_len);
+}
+
+static void dma_unregister(vfu_ctx_t *vfu_ctx, vfu_dma_info_t *info)
+{
+    VfuObject *o = vfu_get_private(vfu_ctx);
+    AddressSpace *dma_as = NULL;
+    MemoryRegion *mr = NULL;
+    ram_addr_t offset;
+
+    mr = memory_region_from_host(info->vaddr, &offset);
+    if (!mr) {
+        return;
+    }
+
+    dma_as = pci_device_iommu_address_space(o->pci_dev);
+
+    memory_region_del_subregion(dma_as->root, mr);
+
+    object_unparent((OBJECT(mr)));
+
+    trace_vfu_dma_unregister((uint64_t)info->iova.iov_base);
+}
+
 /*
  * TYPE_VFU_OBJECT depends on the availability of the 'socket' and 'device'
  * properties. It also depends on devices instantiated in QEMU. These
@@ -365,6 +413,13 @@  static void vfu_object_init_ctx(VfuObject *o, Error **errp)
         goto fail;
     }
 
+    ret = vfu_setup_device_dma(o->vfu_ctx, &dma_register, &dma_unregister);
+    if (ret < 0) {
+        error_setg(errp, "vfu: Failed to setup DMA handlers for %s",
+                   o->device);
+        goto fail;
+    }
+
     ret = vfu_realize_ctx(o->vfu_ctx);
     if (ret < 0) {
         error_setg(errp, "vfu: Failed to realize device %s- %s",
diff --git a/hw/remote/trace-events b/hw/remote/trace-events
index 2ef7884346..f945c7e33b 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""