Patchwork [14/40] memory: iommu support

login
register
mail settings
Submitter Paolo Bonzini
Date May 7, 2013, 2:16 p.m.
Message ID <1367936238-12196-15-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/242384/
State New
Headers show

Comments

Paolo Bonzini - May 7, 2013, 2:16 p.m.
From: Avi Kivity <avi.kivity@gmail.com>

Add a new memory region type that translates addresses it is given,
then forwards them to a target address space.  This is similar to
an alias, except that the mapping is more flexible than a linear
translation and trucation, and also less efficient since the
translation happens at runtime.

The implementation uses an AddressSpace mapping the target region to
avoid hierarchical dispatch all the way to the resolved region; only
iommu regions are looked up dynamically.

Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
[Modified to put translation in address_space_translate - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                |   35 +++++++++++++++++++++++++++++------
 include/exec/memory.h |   44 ++++++++++++++++++++++++++++++++++++++++++++
 memory.c              |   21 +++++++++++++++++++++
 3 files changed, 94 insertions(+), 6 deletions(-)
Peter Maydell - May 7, 2013, 6:15 p.m.
On 7 May 2013 15:16, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Avi Kivity <avi.kivity@gmail.com>
>
> Add a new memory region type that translates addresses it is given,
> then forwards them to a target address space.  This is similar to
> an alias, except that the mapping is more flexible than a linear
> translation and trucation, and also less efficient since the
> translation happens at runtime.
>
> The implementation uses an AddressSpace mapping the target region to
> avoid hierarchical dispatch all the way to the resolved region; only
> iommu regions are looked up dynamically.
>
> Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
> [Modified to put translation in address_space_translate - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  exec.c                |   35 +++++++++++++++++++++++++++++------
>  include/exec/memory.h |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  memory.c              |   21 +++++++++++++++++++++
>  3 files changed, 94 insertions(+), 6 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 6efad6d..7dab2fa 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -208,15 +208,38 @@ MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
>                                               hwaddr *xlat, hwaddr *plen,
>                                               bool is_write)
>  {
> +    IOMMUTLBEntry iotlb;
>      MemoryRegionSection *section;
> +    hwaddr len = *plen;
>
> -    section = address_space_lookup_region(as, addr);
> -    /* Compute offset with MemoryRegionSection */
> -    addr -= section->offset_within_address_space;
> -    *plen = MIN(section->size - addr, *plen);
> +    for (;;) {
> +        section = address_space_lookup_region(as, addr);
> +
> +        /* Compute offset with MemoryRegionSection */
> +        addr -= section->offset_within_address_space;
> +        len = MIN(section->size - addr, len);
> +
> +        /* Compute offset with MemoryRegion */
> +        addr += section->offset_within_region;
> +
> +        if (!section->mr->iommu_ops) {
> +            break;
> +        }
> +
> +        iotlb = section->mr->iommu_ops->translate(section->mr, addr);
> +        addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
> +                | (addr & iotlb.addr_mask));
> +        len = MIN(len, (addr | iotlb.addr_mask) - addr + 1);
> +        if (!iotlb.perm[is_write]) {
> +            section = &phys_sections[phys_section_unassigned];

Why is "unwritable" handled the same as "no memory"? They are
likely to result in different fault information, surely?

> +            break;
> +        }
> +
> +        as = section->mr->iommu_target_as;
> +    }
>
> -    /* Compute offset with MemoryRegion */
> -    *xlat = addr + section->offset_within_region;
> +    *plen = len;
> +    *xlat = addr;
>      return section;
>  }
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 914f5d4..e05296b 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -112,12 +112,27 @@ struct MemoryRegionOps {
>      const MemoryRegionMmio old_mmio;
>  };
>
> +typedef struct IOMMUTLBEntry IOMMUTLBEntry;
> +typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps;
> +
> +struct IOMMUTLBEntry {
> +    hwaddr translated_addr;
> +    hwaddr addr_mask;  /* 0xfff = 4k translation */
> +    bool perm[2]; /* permissions, [0] for read, [1] for write */
> +};

This could use a proper doc comment.

> +
> +struct MemoryRegionIOMMUOps {
> +    /* Returns a TLB entry that contains a given address. */
> +    IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr);
> +};
> +
>  typedef struct CoalescedMemoryRange CoalescedMemoryRange;
>  typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
>
>  struct MemoryRegion {
>      /* All fields are private - violators will be prosecuted */
>      const MemoryRegionOps *ops;
> +    const MemoryRegionIOMMUOps *iommu_ops;
>      void *opaque;
>      MemoryRegion *parent;
>      Int128 size;
> @@ -144,6 +159,7 @@ struct MemoryRegion {
>      uint8_t dirty_log_mask;
>      unsigned ioeventfd_nb;
>      MemoryRegionIoeventfd *ioeventfds;
> +    struct AddressSpace *iommu_target_as;
>  };
>
>  struct MemoryRegionPortio {
> @@ -329,6 +345,25 @@ void memory_region_init_rom_device(MemoryRegion *mr,
>  void memory_region_init_reservation(MemoryRegion *mr,
>                                      const char *name,
>                                      uint64_t size);
> +
> +/**
> + * memory_region_init_iommu: Initialize a memory region that translates addresses
> + *
> + * An IOMMU region translates addresses and forwards accesses to a target memory region.
> + *
> + * @mr: the #MemoryRegion to be initialized
> + * @ops: a function that translates addresses into the @target region

This is an ops pointer, not a function pointer.

> + * @target_as: the #AddressSpace that will be used to satisfy accesses to translated
> + *          addresses
> + * @name: used for debugging; not visible to the user or ABI
> + * @size: size of the region.
> + */
> +void memory_region_init_iommu(MemoryRegion *mr,
> +                              MemoryRegionIOMMUOps *ops,
> +                              AddressSpace *target_as,
> +                              const char *name,
> +                              uint64_t size);
> +
>  /**
>   * memory_region_destroy: Destroy a memory region and reclaim all resources.
>   *
> @@ -368,6 +403,15 @@ static inline bool memory_region_is_romd(MemoryRegion *mr)
>  }
>
>  /**
> + * memory_region_is_iommu: check whether a memory region is an iommu
> + *
> + * Returns %true is a memory region is an iommu.

"%true if this memory region is an iommu"

(should we be consistently capitalising IOMMU?)

> + *
> + * @mr: the memory region being queried
> + */
> +bool memory_region_is_iommu(MemoryRegion *mr);
> +
> +/**
>   * memory_region_name: get a memory region's name
>   *
>   * Returns the string that was used to initialize the memory region.
> diff --git a/memory.c b/memory.c
> index a8929aa..b0d5e33 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -787,6 +787,7 @@ void memory_region_init(MemoryRegion *mr,
>                          uint64_t size)
>  {
>      mr->ops = NULL;
> +    mr->iommu_ops = NULL;
>      mr->parent = NULL;
>      mr->size = int128_make64(size);
>      if (size == UINT64_MAX) {
> @@ -978,6 +979,21 @@ void memory_region_init_rom_device(MemoryRegion *mr,
>      mr->ram_addr = qemu_ram_alloc(size, mr);
>  }
>
> +void memory_region_init_iommu(MemoryRegion *mr,
> +                              MemoryRegionIOMMUOps *ops,
> +                              AddressSpace *target_as,
> +                              const char *name,
> +                              uint64_t size)
> +{
> +    memory_region_init(mr, name, size);
> +    mr->ops = NULL;
> +    mr->iommu_ops = ops,
> +    mr->opaque = mr;
> +    mr->terminates = true;  /* then re-forwards */
> +    mr->destructor = memory_region_destructor_none;
> +    mr->iommu_target_as = target_as;
> +}
> +
>  static uint64_t invalid_read(void *opaque, hwaddr addr,
>                               unsigned size)
>  {
> @@ -1052,6 +1068,11 @@ bool memory_region_is_rom(MemoryRegion *mr)
>      return mr->ram && mr->readonly;
>  }
>
> +bool memory_region_is_iommu(MemoryRegion *mr)
> +{
> +    return mr->iommu_ops;
> +}
> +
>  void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
>  {
>      uint8_t mask = 1 << client;
> --
> 1.7.1
>

thanks
-- PMM

Patch

diff --git a/exec.c b/exec.c
index 6efad6d..7dab2fa 100644
--- a/exec.c
+++ b/exec.c
@@ -208,15 +208,38 @@  MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
                                              hwaddr *xlat, hwaddr *plen,
                                              bool is_write)
 {
+    IOMMUTLBEntry iotlb;
     MemoryRegionSection *section;
+    hwaddr len = *plen;
 
-    section = address_space_lookup_region(as, addr);
-    /* Compute offset with MemoryRegionSection */
-    addr -= section->offset_within_address_space;
-    *plen = MIN(section->size - addr, *plen);
+    for (;;) {
+        section = address_space_lookup_region(as, addr);
+
+        /* Compute offset with MemoryRegionSection */
+        addr -= section->offset_within_address_space;
+        len = MIN(section->size - addr, len);
+
+        /* Compute offset with MemoryRegion */
+        addr += section->offset_within_region;
+
+        if (!section->mr->iommu_ops) {
+            break;
+        }
+
+        iotlb = section->mr->iommu_ops->translate(section->mr, addr);
+        addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
+                | (addr & iotlb.addr_mask));
+        len = MIN(len, (addr | iotlb.addr_mask) - addr + 1);
+        if (!iotlb.perm[is_write]) {
+            section = &phys_sections[phys_section_unassigned];
+            break;
+        }
+
+        as = section->mr->iommu_target_as;
+    }
 
-    /* Compute offset with MemoryRegion */
-    *xlat = addr + section->offset_within_region;
+    *plen = len;
+    *xlat = addr;
     return section;
 }
 
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 914f5d4..e05296b 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -112,12 +112,27 @@  struct MemoryRegionOps {
     const MemoryRegionMmio old_mmio;
 };
 
+typedef struct IOMMUTLBEntry IOMMUTLBEntry;
+typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps;
+
+struct IOMMUTLBEntry {
+    hwaddr translated_addr;
+    hwaddr addr_mask;  /* 0xfff = 4k translation */
+    bool perm[2]; /* permissions, [0] for read, [1] for write */
+};
+
+struct MemoryRegionIOMMUOps {
+    /* Returns a TLB entry that contains a given address. */
+    IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr);
+};
+
 typedef struct CoalescedMemoryRange CoalescedMemoryRange;
 typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
 
 struct MemoryRegion {
     /* All fields are private - violators will be prosecuted */
     const MemoryRegionOps *ops;
+    const MemoryRegionIOMMUOps *iommu_ops;
     void *opaque;
     MemoryRegion *parent;
     Int128 size;
@@ -144,6 +159,7 @@  struct MemoryRegion {
     uint8_t dirty_log_mask;
     unsigned ioeventfd_nb;
     MemoryRegionIoeventfd *ioeventfds;
+    struct AddressSpace *iommu_target_as;
 };
 
 struct MemoryRegionPortio {
@@ -329,6 +345,25 @@  void memory_region_init_rom_device(MemoryRegion *mr,
 void memory_region_init_reservation(MemoryRegion *mr,
                                     const char *name,
                                     uint64_t size);
+
+/**
+ * memory_region_init_iommu: Initialize a memory region that translates addresses
+ *
+ * An IOMMU region translates addresses and forwards accesses to a target memory region.
+ *
+ * @mr: the #MemoryRegion to be initialized
+ * @ops: a function that translates addresses into the @target region
+ * @target_as: the #AddressSpace that will be used to satisfy accesses to translated
+ *          addresses
+ * @name: used for debugging; not visible to the user or ABI
+ * @size: size of the region.
+ */
+void memory_region_init_iommu(MemoryRegion *mr,
+                              MemoryRegionIOMMUOps *ops,
+                              AddressSpace *target_as,
+                              const char *name,
+                              uint64_t size);
+
 /**
  * memory_region_destroy: Destroy a memory region and reclaim all resources.
  *
@@ -368,6 +403,15 @@  static inline bool memory_region_is_romd(MemoryRegion *mr)
 }
 
 /**
+ * memory_region_is_iommu: check whether a memory region is an iommu
+ *
+ * Returns %true is a memory region is an iommu.
+ *
+ * @mr: the memory region being queried
+ */
+bool memory_region_is_iommu(MemoryRegion *mr);
+
+/**
  * memory_region_name: get a memory region's name
  *
  * Returns the string that was used to initialize the memory region.
diff --git a/memory.c b/memory.c
index a8929aa..b0d5e33 100644
--- a/memory.c
+++ b/memory.c
@@ -787,6 +787,7 @@  void memory_region_init(MemoryRegion *mr,
                         uint64_t size)
 {
     mr->ops = NULL;
+    mr->iommu_ops = NULL;
     mr->parent = NULL;
     mr->size = int128_make64(size);
     if (size == UINT64_MAX) {
@@ -978,6 +979,21 @@  void memory_region_init_rom_device(MemoryRegion *mr,
     mr->ram_addr = qemu_ram_alloc(size, mr);
 }
 
+void memory_region_init_iommu(MemoryRegion *mr,
+                              MemoryRegionIOMMUOps *ops,
+                              AddressSpace *target_as,
+                              const char *name,
+                              uint64_t size)
+{
+    memory_region_init(mr, name, size);
+    mr->ops = NULL;
+    mr->iommu_ops = ops,
+    mr->opaque = mr;
+    mr->terminates = true;  /* then re-forwards */
+    mr->destructor = memory_region_destructor_none;
+    mr->iommu_target_as = target_as;
+}
+
 static uint64_t invalid_read(void *opaque, hwaddr addr,
                              unsigned size)
 {
@@ -1052,6 +1068,11 @@  bool memory_region_is_rom(MemoryRegion *mr)
     return mr->ram && mr->readonly;
 }
 
+bool memory_region_is_iommu(MemoryRegion *mr)
+{
+    return mr->iommu_ops;
+}
+
 void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
 {
     uint8_t mask = 1 << client;