Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4151 bytes --]

On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> This is something similar to MemoryRegionOps, it's just for address
> spaces to store arch-specific hooks.
> 
> The first hook I would like to introduce is iommu_get().
> 
> For systems that have IOMMUs, we will create a special address space per
> device which is different from system default address space for
> it (please refer to pci_device_iommu_address_space()). Normally when
> that happens, there will be one specific IOMMU (or say, translation
> unit) stands right behind that new address space.
> 
> This iommu_get() fetches that guy behind the address space. Here, the
> guy is defined as IOMMUObject, which is currently a (void *). In the
> future, maybe we can make it a better definition, but imho it's good
> enough for now, considering it's arch-dependent.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

This doesn't make sense to me.  It would be entirely possible for a
single address space to have different regions mapped by different
IOMMUs.  Or some regions mapped by IOMMUs and others direct mapped to
a device or memory block.

> ---
>  include/exec/memory.h | 30 ++++++++++++++++++++++++++++++
>  memory.c              |  8 ++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index e5707b3..0b0b58b 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -183,6 +183,15 @@ struct MemoryRegionOps {
>      const MemoryRegionMmio old_mmio;
>  };
>  
> +/*
> + * This stands for an IOMMU unit. Normally it should be exactly the
> + * IOMMU device, however this can also be actually anything which is
> + * related to that translation unit. What it is should be totally
> + * arch-dependent. Maybe one day we can have something better than a
> + * (void *) here.
> + */
> +typedef void *IOMMUObject;
> +
>  typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps;
>  
>  struct MemoryRegionIOMMUOps {
> @@ -282,6 +291,19 @@ struct MemoryListener {
>  };
>  
>  /**
> + * AddressSpaceOps: callbacks structure for address space specific operations
> + *
> + * @iommu_get: returns an IOMMU object that backs the address space.
> + *             Normally this should be NULL for generic address
> + *             spaces, and it's only used when there is one
> + *             translation unit behind this address space.
> + */
> +struct AddressSpaceOps {
> +    IOMMUObject *(*iommu_get)(AddressSpace *as);
> +};
> +typedef struct AddressSpaceOps AddressSpaceOps;
> +
> +/**
>   * AddressSpace: describes a mapping of addresses to #MemoryRegion objects
>   */
>  struct AddressSpace {
> @@ -302,6 +324,7 @@ struct AddressSpace {
>      MemoryListener dispatch_listener;
>      QTAILQ_HEAD(memory_listeners_as, MemoryListener) listeners;
>      QTAILQ_ENTRY(AddressSpace) address_spaces_link;
> +    AddressSpaceOps as_ops;
>  };
>  
>  /**
> @@ -1800,6 +1823,13 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
>      address_space_write(cache->as, cache->xlat + addr, MEMTXATTRS_UNSPECIFIED, buf, len);
>  }
>  
> +/**
> + * address_space_iommu_get: Get the backend IOMMU for the address space
> + *
> + * @as: the address space to fetch IOMMU from
> + */
> +IOMMUObject *address_space_iommu_get(AddressSpace *as);
> +
>  #endif
>  
>  #endif
> diff --git a/memory.c b/memory.c
> index 6af523e..6aaad45 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2500,6 +2500,14 @@ void address_space_destroy(AddressSpace *as)
>      call_rcu(as, do_address_space_destroy, rcu);
>  }
>  
> +IOMMUObject *address_space_iommu_get(AddressSpace *as)
> +{
> +    if (!as->as_ops.iommu_get) {
> +        return NULL;
> +    }
> +    return as->as_ops.iommu_get(as);
> +}
> +
>  static const char *memory_region_type(MemoryRegion *mr)
>  {
>      if (memory_region_is_ram_device(mr)) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]