Patchwork [10/11] memory: Add iommu map/unmap notifiers

login
register
mail settings
Submitter David Gibson
Date May 14, 2013, 9:13 a.m.
Message ID <1368522837-20747-11-git-send-email-david@gibson.dropbear.id.au>
Download mbox | patch
Permalink /patch/243638/
State New
Headers show

Comments

David Gibson - May 14, 2013, 9:13 a.m.
This patch adds a NotifierList to MemoryRegions which represent IOMMUs
allowing other parts of the code to register interest in mappings or
unmappings from the IOMMU.  All IOMMU implementations will need to call
memory_region_notify_iommu() to inform those waiting on the notifier list,
whenever an IOMMU mapping is made or removed.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_iommu.c  |    8 ++++++++
 include/exec/memory.h |    7 +++++++
 memory.c              |   18 ++++++++++++++++++
 3 files changed, 33 insertions(+)
Alex Williamson - May 14, 2013, 5:15 p.m.
On Tue, 2013-05-14 at 19:13 +1000, David Gibson wrote:
> This patch adds a NotifierList to MemoryRegions which represent IOMMUs
> allowing other parts of the code to register interest in mappings or
> unmappings from the IOMMU.  All IOMMU implementations will need to call
> memory_region_notify_iommu() to inform those waiting on the notifier list,
> whenever an IOMMU mapping is made or removed.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_iommu.c  |    8 ++++++++
>  include/exec/memory.h |    7 +++++++
>  memory.c              |   18 ++++++++++++++++++
>  3 files changed, 33 insertions(+)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 07a6307..6d40485 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -194,6 +194,7 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
>                                  target_ulong tce)
>  {
>      sPAPRTCE *tcep;
> +    IOMMUTLBEntry entry;
>  
>      if (ioba >= tcet->window_size) {
>          hcall_dprintf("spapr_vio_put_tce on out-of-bounds IOBA 0x"
> @@ -204,6 +205,13 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
>      tcep = tcet->table + (ioba >> SPAPR_TCE_PAGE_SHIFT);
>      tcep->tce = tce;
>  
> +    entry.iova = ioba & ~SPAPR_TCE_PAGE_MASK;
> +    entry.translated_addr = tce & ~SPAPR_TCE_PAGE_MASK;
> +    entry.addr_mask = SPAPR_TCE_PAGE_MASK;
> +    entry.perm[0] = !!(tce & SPAPR_TCE_RO);
> +    entry.perm[1] = !!(tce & SPAPR_TCE_WO);

I really wish these perms were addressed by #defines (ie.
perm[READ]/perm[WRITE]) or explicitly named.

> +    memory_region_notify_iommu(&tcet->iommu, entry);
> +
>      return H_SUCCESS;
>  }
>  
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index cd33439..024b511 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -25,6 +25,7 @@
>  #include "exec/iorange.h"
>  #include "exec/ioport.h"
>  #include "qemu/int128.h"
> +#include "qemu/notify.h"
>  
>  typedef struct MemoryRegionOps MemoryRegionOps;
>  typedef struct MemoryRegionPortio MemoryRegionPortio;
> @@ -160,6 +161,7 @@ struct MemoryRegion {
>      unsigned ioeventfd_nb;
>      MemoryRegionIoeventfd *ioeventfds;
>      struct AddressSpace *iommu_target_as;
> +    NotifierList iommu_notify;
>  };
>  
>  struct MemoryRegionPortio {
> @@ -462,6 +464,11 @@ static inline bool memory_region_is_romd(MemoryRegion *mr)
>   */
>  bool memory_region_is_iommu(MemoryRegion *mr);
>  
> +void memory_region_notify_iommu(MemoryRegion *mr,
> +                                IOMMUTLBEntry entry);
> +void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n);
> +void memory_region_unregister_iommu_notifier(Notifier *n);
> +
>  /**
>   * memory_region_name: get a memory region's name
>   *
> diff --git a/memory.c b/memory.c
> index c409ee5..b11ca9f 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1060,6 +1060,7 @@ void memory_region_init_iommu(MemoryRegion *mr,
>      mr->terminates = true;  /* then re-forwards */
>      mr->destructor = memory_region_destructor_none;
>      mr->iommu_target_as = target_as;
> +    notifier_list_init(&mr->iommu_notify);
>  }
>  
>  static uint64_t invalid_read(void *opaque, hwaddr addr,
> @@ -1175,6 +1176,23 @@ bool memory_region_is_iommu(MemoryRegion *mr)
>      return mr->iommu_ops;
>  }
>  
> +void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n)
> +{
> +    notifier_list_add(&mr->iommu_notify, n);
> +}
> +
> +void memory_region_unregister_iommu_notifier(Notifier *n)
> +{
> +    notifier_remove(n);
> +}
> +
> +void memory_region_notify_iommu(MemoryRegion *mr,
> +                                IOMMUTLBEntry entry)
> +{
> +    assert(memory_region_is_iommu(mr));
> +    notifier_list_notify(&mr->iommu_notify, &entry);
> +}
> +
>  void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
>  {
>      uint8_t mask = 1 << client;
Paolo Bonzini - May 14, 2013, 9:02 p.m.
Il 14/05/2013 19:15, Alex Williamson ha scritto:
> On Tue, 2013-05-14 at 19:13 +1000, David Gibson wrote:
>> > This patch adds a NotifierList to MemoryRegions which represent IOMMUs
>> > allowing other parts of the code to register interest in mappings or
>> > unmappings from the IOMMU.  All IOMMU implementations will need to call
>> > memory_region_notify_iommu() to inform those waiting on the notifier list,
>> > whenever an IOMMU mapping is made or removed.
>> > 
>> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> > ---
>> >  hw/ppc/spapr_iommu.c  |    8 ++++++++
>> >  include/exec/memory.h |    7 +++++++
>> >  memory.c              |   18 ++++++++++++++++++
>> >  3 files changed, 33 insertions(+)
>> > 
>> > diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> > index 07a6307..6d40485 100644
>> > --- a/hw/ppc/spapr_iommu.c
>> > +++ b/hw/ppc/spapr_iommu.c
>> > @@ -194,6 +194,7 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
>> >                                  target_ulong tce)
>> >  {
>> >      sPAPRTCE *tcep;
>> > +    IOMMUTLBEntry entry;
>> >  
>> >      if (ioba >= tcet->window_size) {
>> >          hcall_dprintf("spapr_vio_put_tce on out-of-bounds IOBA 0x"
>> > @@ -204,6 +205,13 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
>> >      tcep = tcet->table + (ioba >> SPAPR_TCE_PAGE_SHIFT);
>> >      tcep->tce = tce;
>> >  
>> > +    entry.iova = ioba & ~SPAPR_TCE_PAGE_MASK;
>> > +    entry.translated_addr = tce & ~SPAPR_TCE_PAGE_MASK;
>> > +    entry.addr_mask = SPAPR_TCE_PAGE_MASK;
>> > +    entry.perm[0] = !!(tce & SPAPR_TCE_RO);
>> > +    entry.perm[1] = !!(tce & SPAPR_TCE_WO);
> I really wish these perms were addressed by #defines (ie.
> perm[READ]/perm[WRITE]) or explicitly named.
> 

I will make it a 2-bit field.

Paolo
Paolo Bonzini - May 14, 2013, 9:23 p.m.
Il 14/05/2013 11:13, David Gibson ha scritto:
> This patch adds a NotifierList to MemoryRegions which represent IOMMUs
> allowing other parts of the code to register interest in mappings or
> unmappings from the IOMMU.  All IOMMU implementations will need to call
> memory_region_notify_iommu() to inform those waiting on the notifier list,
> whenever an IOMMU mapping is made or removed.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Split into various patches of my iommu tree, will push tomorrow.

Paolo

>  hw/ppc/spapr_iommu.c  |    8 ++++++++
>  include/exec/memory.h |    7 +++++++
>  memory.c              |   18 ++++++++++++++++++
>  3 files changed, 33 insertions(+)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 07a6307..6d40485 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -194,6 +194,7 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
>                                  target_ulong tce)
>  {
>      sPAPRTCE *tcep;
> +    IOMMUTLBEntry entry;
>  
>      if (ioba >= tcet->window_size) {
>          hcall_dprintf("spapr_vio_put_tce on out-of-bounds IOBA 0x"
> @@ -204,6 +205,13 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
>      tcep = tcet->table + (ioba >> SPAPR_TCE_PAGE_SHIFT);
>      tcep->tce = tce;
>  
> +    entry.iova = ioba & ~SPAPR_TCE_PAGE_MASK;
> +    entry.translated_addr = tce & ~SPAPR_TCE_PAGE_MASK;
> +    entry.addr_mask = SPAPR_TCE_PAGE_MASK;
> +    entry.perm[0] = !!(tce & SPAPR_TCE_RO);
> +    entry.perm[1] = !!(tce & SPAPR_TCE_WO);
> +    memory_region_notify_iommu(&tcet->iommu, entry);
> +
>      return H_SUCCESS;
>  }
>  
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index cd33439..024b511 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -25,6 +25,7 @@
>  #include "exec/iorange.h"
>  #include "exec/ioport.h"
>  #include "qemu/int128.h"
> +#include "qemu/notify.h"
>  
>  typedef struct MemoryRegionOps MemoryRegionOps;
>  typedef struct MemoryRegionPortio MemoryRegionPortio;
> @@ -160,6 +161,7 @@ struct MemoryRegion {
>      unsigned ioeventfd_nb;
>      MemoryRegionIoeventfd *ioeventfds;
>      struct AddressSpace *iommu_target_as;
> +    NotifierList iommu_notify;
>  };
>  
>  struct MemoryRegionPortio {
> @@ -462,6 +464,11 @@ static inline bool memory_region_is_romd(MemoryRegion *mr)
>   */
>  bool memory_region_is_iommu(MemoryRegion *mr);
>  
> +void memory_region_notify_iommu(MemoryRegion *mr,
> +                                IOMMUTLBEntry entry);
> +void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n);
> +void memory_region_unregister_iommu_notifier(Notifier *n);
> +
>  /**
>   * memory_region_name: get a memory region's name
>   *
> diff --git a/memory.c b/memory.c
> index c409ee5..b11ca9f 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1060,6 +1060,7 @@ void memory_region_init_iommu(MemoryRegion *mr,
>      mr->terminates = true;  /* then re-forwards */
>      mr->destructor = memory_region_destructor_none;
>      mr->iommu_target_as = target_as;
> +    notifier_list_init(&mr->iommu_notify);
>  }
>  
>  static uint64_t invalid_read(void *opaque, hwaddr addr,
> @@ -1175,6 +1176,23 @@ bool memory_region_is_iommu(MemoryRegion *mr)
>      return mr->iommu_ops;
>  }
>  
> +void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n)
> +{
> +    notifier_list_add(&mr->iommu_notify, n);
> +}
> +
> +void memory_region_unregister_iommu_notifier(Notifier *n)
> +{
> +    notifier_remove(n);
> +}
> +
> +void memory_region_notify_iommu(MemoryRegion *mr,
> +                                IOMMUTLBEntry entry)
> +{
> +    assert(memory_region_is_iommu(mr));
> +    notifier_list_notify(&mr->iommu_notify, &entry);
> +}
> +
>  void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
>  {
>      uint8_t mask = 1 << client;
>
David Gibson - May 15, 2013, 1:21 a.m.
On Tue, May 14, 2013 at 11:23:16PM +0200, Paolo Bonzini wrote:
> Il 14/05/2013 11:13, David Gibson ha scritto:
> > This patch adds a NotifierList to MemoryRegions which represent IOMMUs
> > allowing other parts of the code to register interest in mappings or
> > unmappings from the IOMMU.  All IOMMU implementations will need to call
> > memory_region_notify_iommu() to inform those waiting on the notifier list,
> > whenever an IOMMU mapping is made or removed.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> 
> Split into various patches of my iommu tree, will push tomorrow.

Uhhh... ok.  I would kind have preferred to hold off on this one until
the VFIO/iommu stuff is more polished.  I'm not totally convinced the
notifier structure doesn't need some tweaking.

Patch

diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index 07a6307..6d40485 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -194,6 +194,7 @@  static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
                                 target_ulong tce)
 {
     sPAPRTCE *tcep;
+    IOMMUTLBEntry entry;
 
     if (ioba >= tcet->window_size) {
         hcall_dprintf("spapr_vio_put_tce on out-of-bounds IOBA 0x"
@@ -204,6 +205,13 @@  static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
     tcep = tcet->table + (ioba >> SPAPR_TCE_PAGE_SHIFT);
     tcep->tce = tce;
 
+    entry.iova = ioba & ~SPAPR_TCE_PAGE_MASK;
+    entry.translated_addr = tce & ~SPAPR_TCE_PAGE_MASK;
+    entry.addr_mask = SPAPR_TCE_PAGE_MASK;
+    entry.perm[0] = !!(tce & SPAPR_TCE_RO);
+    entry.perm[1] = !!(tce & SPAPR_TCE_WO);
+    memory_region_notify_iommu(&tcet->iommu, entry);
+
     return H_SUCCESS;
 }
 
diff --git a/include/exec/memory.h b/include/exec/memory.h
index cd33439..024b511 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -25,6 +25,7 @@ 
 #include "exec/iorange.h"
 #include "exec/ioport.h"
 #include "qemu/int128.h"
+#include "qemu/notify.h"
 
 typedef struct MemoryRegionOps MemoryRegionOps;
 typedef struct MemoryRegionPortio MemoryRegionPortio;
@@ -160,6 +161,7 @@  struct MemoryRegion {
     unsigned ioeventfd_nb;
     MemoryRegionIoeventfd *ioeventfds;
     struct AddressSpace *iommu_target_as;
+    NotifierList iommu_notify;
 };
 
 struct MemoryRegionPortio {
@@ -462,6 +464,11 @@  static inline bool memory_region_is_romd(MemoryRegion *mr)
  */
 bool memory_region_is_iommu(MemoryRegion *mr);
 
+void memory_region_notify_iommu(MemoryRegion *mr,
+                                IOMMUTLBEntry entry);
+void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n);
+void memory_region_unregister_iommu_notifier(Notifier *n);
+
 /**
  * memory_region_name: get a memory region's name
  *
diff --git a/memory.c b/memory.c
index c409ee5..b11ca9f 100644
--- a/memory.c
+++ b/memory.c
@@ -1060,6 +1060,7 @@  void memory_region_init_iommu(MemoryRegion *mr,
     mr->terminates = true;  /* then re-forwards */
     mr->destructor = memory_region_destructor_none;
     mr->iommu_target_as = target_as;
+    notifier_list_init(&mr->iommu_notify);
 }
 
 static uint64_t invalid_read(void *opaque, hwaddr addr,
@@ -1175,6 +1176,23 @@  bool memory_region_is_iommu(MemoryRegion *mr)
     return mr->iommu_ops;
 }
 
+void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n)
+{
+    notifier_list_add(&mr->iommu_notify, n);
+}
+
+void memory_region_unregister_iommu_notifier(Notifier *n)
+{
+    notifier_remove(n);
+}
+
+void memory_region_notify_iommu(MemoryRegion *mr,
+                                IOMMUTLBEntry entry)
+{
+    assert(memory_region_is_iommu(mr));
+    notifier_list_notify(&mr->iommu_notify, &entry);
+}
+
 void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
 {
     uint8_t mask = 1 << client;