Re: [Qemu-ppc] [PATCH 10/11] memory: Add iommu map/unmap notifiers

[Alex Williamson]

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 <address@hidden>
> ---
>  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;