qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [PATCH qemu v6] memory/iommu: QOM'fy IOMMU MemoryRegion


From: Peter Xu
Subject: Re: [Qemu-ppc] [PATCH qemu v6] memory/iommu: QOM'fy IOMMU MemoryRegion
Date: Tue, 9 May 2017 17:14:51 +0800
User-agent: Mutt/1.5.24 (2015-08-30)

On Fri, May 05, 2017 at 08:19:30PM +1000, Alexey Kardashevskiy wrote:
> This defines new QOM object - IOMMUMemoryRegion - with MemoryRegion
> as a parent.
> 
> This moves IOMMU-related fields from MR to IOMMU MR. However to avoid
> dymanic QOM casting in fast path (address_space_translate, etc),
> this adds an @is_iommu boolean flag to MR and provides new helper to
> do simple cast to IOMMU MR - memory_region_get_iommu. The flag
> is set in the instance init callback. This defines
> memory_region_is_iommu as memory_region_get_iommu()!=NULL.
> 
> This switches MemoryRegion to IOMMUMemoryRegion in most places except
> the ones where MemoryRegion may be an alias.
> 
> This defines memory_region_init_iommu_type() to allow creating
> IOMMUMemoryRegion subclasses. In order to support custom QOM type,
> this splits memory_region_init() to object_initialize() +
> memory_region_do_init.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> Changes:
> v6:
> * s/\<iommumr\>/iommu_mr/g
> 
> v5:
> * fixed sparc64, first time in many years did run "./configure" without
> --target-list :-D Sorry for the noise though :(
> 
> v4:
> * fixed alpha, mips64el and sparc
> 
> v3:
> * rebased on sha1 81b2d5ceb0
> 
> v2:
> * added mr->is_iommu
> * updated i386/x86_64/s390/sun
> ---
>  hw/s390x/s390-pci-bus.h       |   2 +-
>  include/exec/memory.h         |  77 +++++++++++++++++++-------
>  include/hw/i386/intel_iommu.h |   2 +-
>  include/hw/mips/mips.h        |   2 +-
>  include/hw/ppc/spapr.h        |   3 +-
>  include/hw/vfio/vfio-common.h |   2 +-
>  include/qemu/typedefs.h       |   1 +
>  exec.c                        |  16 +++---
>  hw/alpha/typhoon.c            |   8 ++-
>  hw/dma/rc4030.c               |   8 +--
>  hw/i386/amd_iommu.c           |   9 +--
>  hw/i386/intel_iommu.c         |  17 +++---
>  hw/mips/mips_jazz.c           |   2 +-
>  hw/pci-host/apb.c             |   6 +-
>  hw/ppc/spapr_iommu.c          |  20 ++++---
>  hw/s390x/s390-pci-bus.c       |   6 +-
>  hw/s390x/s390-pci-inst.c      |   8 +--
>  hw/vfio/common.c              |  12 ++--
>  hw/vfio/spapr.c               |   3 +-
>  memory.c                      | 124 
> +++++++++++++++++++++++++++++-------------
>  20 files changed, 213 insertions(+), 115 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
> index cf142a3e68..6a599ed353 100644
> --- a/hw/s390x/s390-pci-bus.h
> +++ b/hw/s390x/s390-pci-bus.h
> @@ -266,7 +266,7 @@ typedef struct S390PCIIOMMU {
>      S390PCIBusDevice *pbdev;
>      AddressSpace as;
>      MemoryRegion mr;
> -    MemoryRegion iommu_mr;
> +    IOMMUMemoryRegion iommu_mr;
>      bool enabled;
>      uint64_t g_iota;
>      uint64_t pba;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 99e0f54d86..3638089c1d 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -37,6 +37,10 @@
>  #define MEMORY_REGION(obj) \
>          OBJECT_CHECK(MemoryRegion, (obj), TYPE_MEMORY_REGION)
>  
> +#define TYPE_IOMMU_MEMORY_REGION "qemu:iommu-memory-region"
> +#define IOMMU_MEMORY_REGION(obj) \
> +        OBJECT_CHECK(IOMMUMemoryRegion, (obj), TYPE_IOMMU_MEMORY_REGION)
> +
>  typedef struct MemoryRegionOps MemoryRegionOps;
>  typedef struct MemoryRegionMmio MemoryRegionMmio;
>  
> @@ -186,15 +190,16 @@ typedef struct MemoryRegionIOMMUOps 
> MemoryRegionIOMMUOps;
>  
>  struct MemoryRegionIOMMUOps {
>      /* Return a TLB entry that contains a given address. */
> -    IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr, bool 
> is_write);
> +    IOMMUTLBEntry (*translate)(IOMMUMemoryRegion *iommu, hwaddr addr,
> +                               bool is_write);
>      /* Returns minimum supported page size */
> -    uint64_t (*get_min_page_size)(MemoryRegion *iommu);
> +    uint64_t (*get_min_page_size)(IOMMUMemoryRegion *iommu);
>      /* Called when IOMMU Notifier flag changed */
> -    void (*notify_flag_changed)(MemoryRegion *iommu,
> +    void (*notify_flag_changed)(IOMMUMemoryRegion *iommu,
>                                  IOMMUNotifierFlag old_flags,
>                                  IOMMUNotifierFlag new_flags);
>      /* Set this up to provide customized IOMMU replay function */
> -    void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
> +    void (*replay)(IOMMUMemoryRegion *iommu, IOMMUNotifier *notifier);
>  };
>  
>  typedef struct CoalescedMemoryRange CoalescedMemoryRange;
> @@ -216,7 +221,6 @@ struct MemoryRegion {
>      uint8_t dirty_log_mask;
>      RAMBlock *ram_block;
>      Object *owner;
> -    const MemoryRegionIOMMUOps *iommu_ops;
>  
>      const MemoryRegionOps *ops;
>      void *opaque;
> @@ -239,6 +243,13 @@ struct MemoryRegion {
>      const char *name;
>      unsigned ioeventfd_nb;
>      MemoryRegionIoeventfd *ioeventfds;
> +    bool is_iommu;
> +};
> +
> +struct IOMMUMemoryRegion {
> +    MemoryRegion parent_obj;
> +
> +    const MemoryRegionIOMMUOps *iommu_ops;
>      QLIST_HEAD(, IOMMUNotifier) iommu_notify;
>      IOMMUNotifierFlag iommu_notify_flags;
>  };
> @@ -579,19 +590,40 @@ static inline void 
> memory_region_init_reservation(MemoryRegion *mr,
>  }
>  
>  /**
> + * memory_region_init_iommu_type: Initialize a memory region of a custom type
> + * that translates addresses
> + *
> + * An IOMMU region translates addresses and forwards accesses to a target
> + * memory region.
> + *
> + * @typename: QOM class name
> + * @iommu_mr: the #IOMMUMemoryRegion to be initialized
> + * @owner: the object that tracks the region's reference count
> + * @ops: a function that translates addresses into the @target region
> + * @name: used for debugging; not visible to the user or ABI
> + * @size: size of the region.
> + */
> +void memory_region_init_iommu_type(const char *mrtypename,
> +                                   IOMMUMemoryRegion *iommu_mr,
> +                                   Object *owner,
> +                                   const MemoryRegionIOMMUOps *ops,
> +                                   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
> + * @iommu_mr: the #IOMMUMemoryRegion to be initialized
>   * @owner: the object that tracks the region's reference count
>   * @ops: a function that translates addresses into the @target region
>   * @name: used for debugging; not visible to the user or ABI
>   * @size: size of the region.
>   */
> -void memory_region_init_iommu(MemoryRegion *mr,
> +void memory_region_init_iommu(IOMMUMemoryRegion *iommu_mr,
>                                struct Object *owner,
>                                const MemoryRegionIOMMUOps *ops,
>                                const char *name,
> @@ -646,20 +678,25 @@ static inline bool memory_region_is_romd(MemoryRegion 
> *mr)
>  }
>  
>  /**
> - * memory_region_is_iommu: check whether a memory region is an iommu
> + * memory_region_get_iommu: check whether a memory region is an iommu
>   *
> - * Returns %true is a memory region is an iommu.
> + * Returns pointer to IOMMUMemoryRegion if a memory region is an iommu,
> + * otherwise NULL.
>   *
>   * @mr: the memory region being queried
>   */
> -static inline bool memory_region_is_iommu(MemoryRegion *mr)
> +static inline IOMMUMemoryRegion *memory_region_get_iommu(MemoryRegion *mr)
>  {
>      if (mr->alias) {
> -        return memory_region_is_iommu(mr->alias);
> +        return memory_region_get_iommu(mr->alias);
>      }
> -    return mr->iommu_ops;
> +    if (mr->is_iommu) {
> +        return (IOMMUMemoryRegion *) mr;
> +    }
> +    return NULL;
>  }
>  
> +#define memory_region_is_iommu(mr) (memory_region_get_iommu(mr) != NULL)
>  
>  /**
>   * memory_region_iommu_get_min_page_size: get minimum supported page size
> @@ -667,9 +704,9 @@ static inline bool memory_region_is_iommu(MemoryRegion 
> *mr)
>   *
>   * Returns minimum supported page size for an iommu.
>   *
> - * @mr: the memory region being queried
> + * @iommu_mr: the memory region being queried
>   */
> -uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr);
> +uint64_t memory_region_iommu_get_min_page_size(IOMMUMemoryRegion *iommu_mr);
>  
>  /**
>   * memory_region_notify_iommu: notify a change in an IOMMU translation entry.
> @@ -683,12 +720,12 @@ uint64_t 
> memory_region_iommu_get_min_page_size(MemoryRegion *mr);
>   * Note: for any IOMMU implementation, an in-place mapping change
>   * should be notified with an UNMAP followed by a MAP.
>   *
> - * @mr: the memory region that was changed
> + * @iommu_mr: the memory region that was changed
>   * @entry: the new entry in the IOMMU translation table.  The entry
>   *         replaces all old entries for the same virtual I/O address range.
>   *         Deleted entries have address@hidden == 0.
>   */
> -void memory_region_notify_iommu(MemoryRegion *mr,
> +void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr,
>                                  IOMMUTLBEntry entry);
>  
>  /**
> @@ -723,21 +760,21 @@ void memory_region_register_iommu_notifier(MemoryRegion 
> *mr,
>   * a notifier with the minimum page granularity returned by
>   * mr->iommu_ops->get_page_size().
>   *
> - * @mr: the memory region to observe
> + * @iommu_mr: the memory region to observe
>   * @n: the notifier to which to replay iommu mappings
>   * @is_write: Whether to treat the replay as a translate "write"
>   *     through the iommu
>   */
> -void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n,
> +void memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier 
> *n,
>                                  bool is_write);
>  
>  /**
>   * memory_region_iommu_replay_all: replay existing IOMMU translations
>   * to all the notifiers registered.
>   *
> - * @mr: the memory region to observe
> + * @iommu_mr: the memory region to observe
>   */
> -void memory_region_iommu_replay_all(MemoryRegion *mr);
> +void memory_region_iommu_replay_all(IOMMUMemoryRegion *iommu_mr);
>  
>  /**
>   * memory_region_unregister_iommu_notifier: unregister a notifier for
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 3e51876b75..45fba4ff97 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -83,7 +83,7 @@ struct VTDAddressSpace {
>      PCIBus *bus;
>      uint8_t devfn;
>      AddressSpace as;
> -    MemoryRegion iommu;
> +    IOMMUMemoryRegion iommu;
>      MemoryRegion root;
>      MemoryRegion sys_alias;
>      MemoryRegion iommu_ir;      /* Interrupt region: 0xfeeXXXXX */
> diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
> index e0065ce808..0af4c3d5d7 100644
> --- a/include/hw/mips/mips.h
> +++ b/include/hw/mips/mips.h
> @@ -18,6 +18,6 @@ typedef struct rc4030DMAState *rc4030_dma;
>  void rc4030_dma_read(void *dma, uint8_t *buf, int len);
>  void rc4030_dma_write(void *dma, uint8_t *buf, int len);
>  
> -DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr);
> +DeviceState *rc4030_init(rc4030_dma **dmas, IOMMUMemoryRegion **dma_mr);
>  
>  #endif
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5802f888c3..011858afb1 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -589,7 +589,8 @@ struct sPAPRTCETable {
>      bool bypass;
>      bool need_vfio;
>      int fd;
> -    MemoryRegion root, iommu;
> +    MemoryRegion root;
> +    IOMMUMemoryRegion iommu;
>      struct VIOsPAPRDevice *vdev; /* for @bypass migration compatibility only 
> */
>      QLIST_ENTRY(sPAPRTCETable) list;
>  };
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index c582de18c9..7a4135ae6f 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -94,7 +94,7 @@ typedef struct VFIOContainer {
>  
>  typedef struct VFIOGuestIOMMU {
>      VFIOContainer *container;
> -    MemoryRegion *iommu;
> +    IOMMUMemoryRegion *iommu;
>      hwaddr iommu_offset;
>      IOMMUNotifier n;
>      QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index f08d327aec..f9906b28fd 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -46,6 +46,7 @@ typedef struct MachineState MachineState;
>  typedef struct MemoryListener MemoryListener;
>  typedef struct MemoryMappingList MemoryMappingList;
>  typedef struct MemoryRegion MemoryRegion;
> +typedef struct IOMMUMemoryRegion IOMMUMemoryRegion;
>  typedef struct MemoryRegionCache MemoryRegionCache;
>  typedef struct MemoryRegionSection MemoryRegionSection;
>  typedef struct MigrationIncomingState MigrationIncomingState;
> diff --git a/exec.c b/exec.c
> index eac6085760..d30262c182 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -468,20 +468,20 @@ IOMMUTLBEntry 
> address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
>  {
>      IOMMUTLBEntry iotlb = {0};
>      MemoryRegionSection *section;
> -    MemoryRegion *mr;
> +    IOMMUMemoryRegion *iommu_mr;
>  
>      for (;;) {
>          AddressSpaceDispatch *d = atomic_rcu_read(&as->dispatch);
>          section = address_space_lookup_region(d, addr, false);
>          addr = addr - section->offset_within_address_space
>                 + section->offset_within_region;
> -        mr = section->mr;
>  
> -        if (!mr->iommu_ops) {
> +        iommu_mr = memory_region_get_iommu(section->mr);
> +        if (!iommu_mr) {
>              break;
>          }
>  
> -        iotlb = mr->iommu_ops->translate(mr, addr, is_write);
> +        iotlb = iommu_mr->iommu_ops->translate(iommu_mr, addr, is_write);
>          if (!(iotlb.perm & (1 << is_write))) {
>              iotlb.target_as = NULL;
>              break;
> @@ -503,17 +503,19 @@ MemoryRegion *address_space_translate(AddressSpace *as, 
> hwaddr addr,
>      IOMMUTLBEntry iotlb;
>      MemoryRegionSection *section;
>      MemoryRegion *mr;
> +    IOMMUMemoryRegion *iommu_mr;
>  
>      for (;;) {
>          AddressSpaceDispatch *d = atomic_rcu_read(&as->dispatch);
>          section = address_space_translate_internal(d, addr, &addr, plen, 
> true);
>          mr = section->mr;
>  
> -        if (!mr->iommu_ops) {
> +        iommu_mr = memory_region_get_iommu(mr);
> +        if (!iommu_mr) {
>              break;
>          }
>  
> -        iotlb = mr->iommu_ops->translate(mr, addr, is_write);
> +        iotlb = iommu_mr->iommu_ops->translate(iommu_mr, addr, is_write);
>          addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
>                  | (addr & iotlb.addr_mask));
>          *plen = MIN(*plen, (addr | iotlb.addr_mask) - addr + 1);
> @@ -544,7 +546,7 @@ address_space_translate_for_iotlb(CPUState *cpu, int 
> asidx, hwaddr addr,
>  
>      section = address_space_translate_internal(d, addr, xlat, plen, false);
>  
> -    assert(!section->mr->iommu_ops);
> +    assert(!memory_region_is_iommu(section->mr));
>      return section;
>  }
>  #endif
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index f50f5cf186..1ee9e7a969 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -41,7 +41,7 @@ typedef struct TyphoonPchip {
>      MemoryRegion reg_conf;
>  
>      AddressSpace iommu_as;
> -    MemoryRegion iommu;
> +    IOMMUMemoryRegion iommu;
>  
>      uint64_t ctl;
>      TyphoonWindow win[4];
> @@ -663,7 +663,8 @@ static bool window_translate(TyphoonWindow *win, hwaddr 
> addr,
>  /* Handle PCI-to-system address translation.  */
>  /* TODO: A translation failure here ought to set PCI error codes on the
>     Pchip and generate a machine check interrupt.  */
> -static IOMMUTLBEntry typhoon_translate_iommu(MemoryRegion *iommu, hwaddr 
> addr,
> +static IOMMUTLBEntry typhoon_translate_iommu(IOMMUMemoryRegion *iommu,
> +                                             hwaddr addr,
>                                               bool is_write)
>  {
>      TyphoonPchip *pchip = container_of(iommu, TyphoonPchip, iommu);
> @@ -893,7 +894,8 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus 
> **isa_bus,
>      /* Host memory as seen from the PCI side, via the IOMMU.  */
>      memory_region_init_iommu(&s->pchip.iommu, OBJECT(s), &typhoon_iommu_ops,
>                               "iommu-typhoon", UINT64_MAX);
> -    address_space_init(&s->pchip.iommu_as, &s->pchip.iommu, "pchip0-pci");
> +    address_space_init(&s->pchip.iommu_as, MEMORY_REGION(&s->pchip.iommu),
> +                       "pchip0-pci");
>      pci_setup_iommu(b, typhoon_pci_dma_iommu, s);
>  
>      /* Pchip0 PCI special/interrupt acknowledge, 0x801.F800.0000, 64MB.  */
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index 0080141905..16f13883f3 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -90,7 +90,7 @@ typedef struct rc4030State
>      qemu_irq jazz_bus_irq;
>  
>      /* whole DMA memory region, root of DMA address space */
> -    MemoryRegion dma_mr;
> +    IOMMUMemoryRegion dma_mr;
>      AddressSpace dma_as;
>  
>      MemoryRegion iomem_chipset;
> @@ -488,7 +488,7 @@ static const MemoryRegionOps jazzio_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -static IOMMUTLBEntry rc4030_dma_translate(MemoryRegion *iommu, hwaddr addr,
> +static IOMMUTLBEntry rc4030_dma_translate(IOMMUMemoryRegion *iommu, hwaddr 
> addr,
>                                            bool is_write)
>  {
>      rc4030State *s = container_of(iommu, rc4030State, dma_mr);
> @@ -679,7 +679,7 @@ static void rc4030_realize(DeviceState *dev, Error **errp)
>  
>      memory_region_init_iommu(&s->dma_mr, o, &rc4030_dma_ops,
>                               "rc4030.dma", UINT32_MAX);
> -    address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
> +    address_space_init(&s->dma_as, MEMORY_REGION(&s->dma_mr), "rc4030-dma");
>  }
>  
>  static void rc4030_unrealize(DeviceState *dev, Error **errp)
> @@ -717,7 +717,7 @@ static void rc4030_register_types(void)
>  
>  type_init(rc4030_register_types)
>  
> -DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr)
> +DeviceState *rc4030_init(rc4030_dma **dmas, IOMMUMemoryRegion **dma_mr)
>  {
>      DeviceState *dev;
>  
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index f86a40aa30..46ba34a733 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -51,7 +51,7 @@ struct AMDVIAddressSpace {
>      uint8_t bus_num;            /* bus number                           */
>      uint8_t devfn;              /* device function                      */
>      AMDVIState *iommu_state;    /* AMDVI - one per machine              */
> -    MemoryRegion iommu;         /* Device's address translation region  */
> +    IOMMUMemoryRegion iommu;    /* Device's address translation region  */
>      MemoryRegion iommu_ir;      /* Device's interrupt remapping region  */
>      AddressSpace as;            /* device's corresponding address space */
>  };
> @@ -986,7 +986,7 @@ static inline bool amdvi_is_interrupt_addr(hwaddr addr)
>      return addr >= AMDVI_INT_ADDR_FIRST && addr <= AMDVI_INT_ADDR_LAST;
>  }
>  
> -static IOMMUTLBEntry amdvi_translate(MemoryRegion *iommu, hwaddr addr,
> +static IOMMUTLBEntry amdvi_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>                                       bool is_write)
>  {
>      AMDVIAddressSpace *as = container_of(iommu, AMDVIAddressSpace, iommu);
> @@ -1045,7 +1045,8 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, 
> void *opaque, int devfn)
>  
>          memory_region_init_iommu(&iommu_as[devfn]->iommu, OBJECT(s),
>                                   &s->iommu_ops, "amd-iommu", UINT64_MAX);
> -        address_space_init(&iommu_as[devfn]->as, &iommu_as[devfn]->iommu,
> +        address_space_init(&iommu_as[devfn]->as,
> +                           MEMORY_REGION(&iommu_as[devfn]->iommu),
>                             "amd-iommu");
>      }
>      return &iommu_as[devfn]->as;
> @@ -1066,7 +1067,7 @@ static const MemoryRegionOps mmio_mem_ops = {
>      }
>  };
>  
> -static void amdvi_iommu_notify_flag_changed(MemoryRegion *iommu,
> +static void amdvi_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
>                                              IOMMUNotifierFlag old,
>                                              IOMMUNotifierFlag new)
>  {
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 02f047c8e3..adad2e7293 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1205,7 +1205,7 @@ static void vtd_iotlb_domain_invalidate(IntelIOMMUState 
> *s, uint16_t domain_id)
>  static int vtd_page_invalidate_notify_hook(IOMMUTLBEntry *entry,
>                                             void *private)
>  {
> -    memory_region_notify_iommu((MemoryRegion *)private, *entry);
> +    memory_region_notify_iommu((IOMMUMemoryRegion *)private, *entry);
>      return 0;
>  }
>  
> @@ -1373,9 +1373,9 @@ static void vtd_switch_address_space(VTDAddressSpace 
> *as)
>      /* Turn off first then on the other */
>      if (as->iommu_state->dmar_enabled) {
>          memory_region_set_enabled(&as->sys_alias, false);
> -        memory_region_set_enabled(&as->iommu, true);
> +        memory_region_set_enabled(MEMORY_REGION(&as->iommu), true);
>      } else {
> -        memory_region_set_enabled(&as->iommu, false);
> +        memory_region_set_enabled(MEMORY_REGION(&as->iommu), false);
>          memory_region_set_enabled(&as->sys_alias, true);
>      }
>  }
> @@ -2220,7 +2220,7 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
>      }
>  }
>  
> -static IOMMUTLBEntry vtd_iommu_translate(MemoryRegion *iommu, hwaddr addr,
> +static IOMMUTLBEntry vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr 
> addr,
>                                           bool is_write)
>  {
>      VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
> @@ -2252,7 +2252,7 @@ static IOMMUTLBEntry vtd_iommu_translate(MemoryRegion 
> *iommu, hwaddr addr,
>      return ret;
>  }
>  
> -static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
> +static void vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
>                                            IOMMUNotifierFlag old,
>                                            IOMMUNotifierFlag new)
>  {
> @@ -2695,7 +2695,8 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, 
> PCIBus *bus, int devfn)
>          memory_region_add_subregion_overlap(&vtd_dev_as->root, 0,
>                                              &vtd_dev_as->sys_alias, 1);
>          memory_region_add_subregion_overlap(&vtd_dev_as->root, 0,
> -                                            &vtd_dev_as->iommu, 1);
> +                                            
> MEMORY_REGION(&vtd_dev_as->iommu),
> +                                            1);
>          vtd_switch_address_space(vtd_dev_as);
>      }
>      return vtd_dev_as;
> @@ -2775,9 +2776,9 @@ static int vtd_replay_hook(IOMMUTLBEntry *entry, void 
> *private)
>      return 0;
>  }
>  
> -static void vtd_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n)
> +static void vtd_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n)
>  {
> -    VTDAddressSpace *vtd_as = container_of(mr, VTDAddressSpace, iommu);
> +    VTDAddressSpace *vtd_as = container_of(iommu_mr, VTDAddressSpace, iommu);
>      IntelIOMMUState *s = vtd_as->iommu_state;
>      uint8_t bus_n = pci_bus_num(vtd_as->bus);
>      VTDContextEntry ce;
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 1cef581878..1f69322c15 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -130,7 +130,7 @@ static void mips_jazz_init(MachineState *machine,
>      CPUMIPSState *env;
>      qemu_irq *i8259;
>      rc4030_dma *dmas;
> -    MemoryRegion *rc4030_dma_mr;
> +    IOMMUMemoryRegion *rc4030_dma_mr;
>      MemoryRegion *isa_mem = g_new(MemoryRegion, 1);
>      MemoryRegion *isa_io = g_new(MemoryRegion, 1);
>      MemoryRegion *rtc = g_new(MemoryRegion, 1);
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 653e711121..1029fb0e5a 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -123,7 +123,7 @@ do { printf("IOMMU: " fmt , ## __VA_ARGS__); } while (0)
>  
>  typedef struct IOMMUState {
>      AddressSpace iommu_as;
> -    MemoryRegion iommu;
> +    IOMMUMemoryRegion iommu;
>  
>      uint64_t regs[IOMMU_NREGS];
>  } IOMMUState;
> @@ -208,7 +208,7 @@ static AddressSpace *pbm_pci_dma_iommu(PCIBus *bus, void 
> *opaque, int devfn)
>  }
>  
>  /* Called from RCU critical section */
> -static IOMMUTLBEntry pbm_translate_iommu(MemoryRegion *iommu, hwaddr addr,
> +static IOMMUTLBEntry pbm_translate_iommu(IOMMUMemoryRegion *iommu, hwaddr 
> addr,
>                                           bool is_write)
>  {
>      IOMMUState *is = container_of(iommu, IOMMUState, iommu);
> @@ -699,7 +699,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
>  
>      memory_region_init_iommu(&is->iommu, OBJECT(dev), &pbm_iommu_ops,
>                               "iommu-apb", UINT64_MAX);
> -    address_space_init(&is->iommu_as, &is->iommu, "pbm-as");
> +    address_space_init(&is->iommu_as, MEMORY_REGION(&is->iommu), "pbm-as");
>      pci_setup_iommu(phb->bus, pbm_pci_dma_iommu, is);
>  
>      /* APB secondary busses */
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 29c80bb3c8..99732bf24d 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -110,7 +110,8 @@ static void spapr_tce_free_table(uint64_t *table, int fd, 
> uint32_t nb_table)
>  }
>  
>  /* Called from RCU critical section */
> -static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr 
> addr,
> +static IOMMUTLBEntry spapr_tce_translate_iommu(IOMMUMemoryRegion *iommu,
> +                                               hwaddr addr,
>                                                 bool is_write)
>  {
>      sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu);
> @@ -150,14 +151,14 @@ static void spapr_tce_table_pre_save(void *opaque)
>                                 tcet->bus_offset, tcet->page_shift);
>  }
>  
> -static uint64_t spapr_tce_get_min_page_size(MemoryRegion *iommu)
> +static uint64_t spapr_tce_get_min_page_size(IOMMUMemoryRegion *iommu)
>  {
>      sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu);
>  
>      return 1ULL << tcet->page_shift;
>  }
>  
> -static void spapr_tce_notify_flag_changed(MemoryRegion *iommu,
> +static void spapr_tce_notify_flag_changed(IOMMUMemoryRegion *iommu,
>                                            IOMMUNotifierFlag old,
>                                            IOMMUNotifierFlag new)
>  {
> @@ -265,7 +266,9 @@ static int spapr_tce_table_realize(DeviceState *dev)
>      memory_region_init(&tcet->root, tcetobj, tmp, UINT64_MAX);
>  
>      snprintf(tmp, sizeof(tmp), "tce-iommu-%x", tcet->liobn);
> -    memory_region_init_iommu(&tcet->iommu, tcetobj, &spapr_iommu_ops, tmp, 
> 0);
> +    memory_region_init_iommu_type(TYPE_IOMMU_MEMORY_REGION,
> +                                  &tcet->iommu, tcetobj, &spapr_iommu_ops,
> +                                  tmp, 0);

Could we use memory_region_init_iommu() directly here?

>  
>      QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
>  
> @@ -348,9 +351,10 @@ void spapr_tce_table_enable(sPAPRTCETable *tcet,
>                                          &tcet->fd,
>                                          tcet->need_vfio);
>  
> -    memory_region_set_size(&tcet->iommu,
> +    memory_region_set_size(MEMORY_REGION(&tcet->iommu),
>                             (uint64_t)tcet->nb_table << tcet->page_shift);
> -    memory_region_add_subregion(&tcet->root, tcet->bus_offset, &tcet->iommu);
> +    memory_region_add_subregion(&tcet->root, tcet->bus_offset,
> +                                MEMORY_REGION(&tcet->iommu));
>  }
>  
>  void spapr_tce_table_disable(sPAPRTCETable *tcet)
> @@ -359,8 +363,8 @@ void spapr_tce_table_disable(sPAPRTCETable *tcet)
>          return;
>      }
>  
> -    memory_region_del_subregion(&tcet->root, &tcet->iommu);
> -    memory_region_set_size(&tcet->iommu, 0);
> +    memory_region_del_subregion(&tcet->root, MEMORY_REGION(&tcet->iommu));
> +    memory_region_set_size(MEMORY_REGION(&tcet->iommu), 0);
>  
>      spapr_tce_free_table(tcet->table, tcet->fd, tcet->nb_table);
>      tcet->fd = -1;
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index a8a1bab50a..9bda89eb12 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -356,7 +356,7 @@ out:
>      return pte;
>  }
>  
> -static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *mr, hwaddr addr,
> +static IOMMUTLBEntry s390_translate_iommu(IOMMUMemoryRegion *mr, hwaddr addr,
>                                            bool is_write)
>  {
>      uint64_t pte;
> @@ -525,14 +525,14 @@ void s390_pci_iommu_enable(S390PCIIOMMU *iommu)
>      memory_region_init_iommu(&iommu->iommu_mr, OBJECT(&iommu->mr),
>                               &s390_iommu_ops, name, iommu->pal + 1);
>      iommu->enabled = true;
> -    memory_region_add_subregion(&iommu->mr, 0, &iommu->iommu_mr);
> +    memory_region_add_subregion(&iommu->mr, 0, 
> MEMORY_REGION(&iommu->iommu_mr));
>      g_free(name);
>  }
>  
>  void s390_pci_iommu_disable(S390PCIIOMMU *iommu)
>  {
>      iommu->enabled = false;
> -    memory_region_del_subregion(&iommu->mr, &iommu->iommu_mr);
> +    memory_region_del_subregion(&iommu->mr, MEMORY_REGION(&iommu->iommu_mr));
>      object_unparent(OBJECT(&iommu->iommu_mr));
>  }
>  
> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index 314a9cbad4..b82cab9f6c 100644
> --- a/hw/s390x/s390-pci-inst.c
> +++ b/hw/s390x/s390-pci-inst.c
> @@ -563,7 +563,7 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t 
> r2)
>      S390PCIIOMMU *iommu;
>      hwaddr start, end;
>      IOMMUTLBEntry entry;
> -    MemoryRegion *mr;
> +    IOMMUMemoryRegion *iommu_mr;
>  
>      cpu_synchronize_state(CPU(cpu));
>  
> @@ -622,9 +622,9 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t 
> r2)
>          goto out;
>      }
>  
> -    mr = &iommu->iommu_mr;
> +    iommu_mr = &iommu->iommu_mr;
>      while (start < end) {
> -        entry = mr->iommu_ops->translate(mr, start, 0);
> +        entry = iommu_mr->iommu_ops->translate(iommu_mr, start, 0);
>  
>          if (!entry.translated_addr) {
>              pbdev->state = ZPCI_FS_ERROR;
> @@ -635,7 +635,7 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t 
> r2)
>              goto out;
>          }
>  
> -        memory_region_notify_iommu(mr, entry);
> +        memory_region_notify_iommu(iommu_mr, entry);
>          start += entry.addr_mask + 1;
>      }
>  
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 6b33b9f55d..8689ab7535 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -465,6 +465,7 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>  
>      if (memory_region_is_iommu(section->mr)) {
>          VFIOGuestIOMMU *giommu;
> +        IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr);
>  
>          trace_vfio_listener_region_add_iommu(iova, end);
>          /*
> @@ -474,7 +475,7 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>           * device emulation the VFIO iommu handles to use).
>           */
>          giommu = g_malloc0(sizeof(*giommu));
> -        giommu->iommu = section->mr;
> +        giommu->iommu = iommu_mr;
>          giommu->iommu_offset = section->offset_within_address_space -
>                                 section->offset_within_region;
>          giommu->container = container;
> @@ -487,7 +488,7 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>                              int128_get64(llend));
>          QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
>  
> -        memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
> +        memory_region_register_iommu_notifier(section->mr, &giommu->n);
>          memory_region_iommu_replay(giommu->iommu, &giommu->n, false);
>  
>          return;
> @@ -555,9 +556,9 @@ static void vfio_listener_region_del(MemoryListener 
> *listener,
>          VFIOGuestIOMMU *giommu;
>  
>          QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
> -            if (giommu->iommu == section->mr &&
> +            if (MEMORY_REGION(giommu->iommu) == section->mr &&
>                  giommu->n.start == section->offset_within_region) {
> -                memory_region_unregister_iommu_notifier(giommu->iommu,
> +                memory_region_unregister_iommu_notifier(section->mr,
>                                                          &giommu->n);
>                  QLIST_REMOVE(giommu, giommu_next);
>                  g_free(giommu);
> @@ -1147,7 +1148,8 @@ static void vfio_disconnect_container(VFIOGroup *group)
>          QLIST_REMOVE(container, next);
>  
>          QLIST_FOREACH_SAFE(giommu, &container->giommu_list, giommu_next, 
> tmp) {
> -            memory_region_unregister_iommu_notifier(giommu->iommu, 
> &giommu->n);
> +            memory_region_unregister_iommu_notifier(
> +                    MEMORY_REGION(giommu->iommu), &giommu->n);
>              QLIST_REMOVE(giommu, giommu_next);
>              g_free(giommu);
>          }
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 4409bcc0d7..32fd6a9b54 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -143,7 +143,8 @@ int vfio_spapr_create_window(VFIOContainer *container,
>                               hwaddr *pgsize)
>  {
>      int ret;
> -    unsigned pagesize = memory_region_iommu_get_min_page_size(section->mr);
> +    IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr);
> +    unsigned pagesize = memory_region_iommu_get_min_page_size(iommu_mr);
>      unsigned entries, pages;
>      struct vfio_iommu_spapr_tce_create create = { .argsz = sizeof(create) };
>  
> diff --git a/memory.c b/memory.c
> index b727f5ec0e..630bfc51e1 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -975,12 +975,11 @@ static char *memory_region_escape_name(const char *name)
>      return escaped;
>  }
>  
> -void memory_region_init(MemoryRegion *mr,
> -                        Object *owner,
> -                        const char *name,
> -                        uint64_t size)
> +static void memory_region_do_init(MemoryRegion *mr,
> +                                  Object *owner,
> +                                  const char *name,
> +                                  uint64_t size)
>  {
> -    object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
>      mr->size = int128_make64(size);
>      if (size == UINT64_MAX) {
>          mr->size = int128_2_64();
> @@ -1004,6 +1003,15 @@ void memory_region_init(MemoryRegion *mr,
>      }
>  }
>  
> +void memory_region_init(MemoryRegion *mr,
> +                        Object *owner,
> +                        const char *name,
> +                        uint64_t size)
> +{
> +    object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
> +    memory_region_do_init(mr, owner, name, size);
> +}
> +
>  static void memory_region_get_addr(Object *obj, Visitor *v, const char *name,
>                                     void *opaque, Error **errp)
>  {
> @@ -1090,6 +1098,13 @@ static void memory_region_initfn(Object *obj)
>                          NULL, NULL, &error_abort);
>  }
>  
> +static void iommu_memory_region_initfn(Object *obj)
> +{
> +    MemoryRegion *mr = MEMORY_REGION(obj);
> +
> +    mr->is_iommu = true;
> +}
> +
>  static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
>                                      unsigned size)
>  {
> @@ -1473,17 +1488,33 @@ void memory_region_init_rom_device(MemoryRegion *mr,
>      mr->ram_block = qemu_ram_alloc(size, mr, errp);
>  }
>  
> -void memory_region_init_iommu(MemoryRegion *mr,
> -                              Object *owner,
> -                              const MemoryRegionIOMMUOps *ops,
> -                              const char *name,
> -                              uint64_t size)
> +void memory_region_init_iommu_type(const char *mrtypename,
> +                                   IOMMUMemoryRegion *iommu_mr,
> +                                   Object *owner,
> +                                   const MemoryRegionIOMMUOps *ops,
> +                                   const char *name,
> +                                   uint64_t size)
>  {
> -    memory_region_init(mr, owner, name, size);
> -    mr->iommu_ops = ops,
> +    struct MemoryRegion *mr;
> +    size_t instance_size = object_type_get_instance_size(mrtypename);
> +
> +    object_initialize(iommu_mr, instance_size, mrtypename);
> +    mr = MEMORY_REGION(iommu_mr);
> +    memory_region_do_init(mr, owner, name, size);
> +    iommu_mr->iommu_ops = ops,
>      mr->terminates = true;  /* then re-forwards */
> -    QLIST_INIT(&mr->iommu_notify);
> -    mr->iommu_notify_flags = IOMMU_NOTIFIER_NONE;
> +    QLIST_INIT(&iommu_mr->iommu_notify);
> +    iommu_mr->iommu_notify_flags = IOMMU_NOTIFIER_NONE;
> +}
> +
> +void memory_region_init_iommu(IOMMUMemoryRegion *iommu_mr,
> +                              Object *owner,
> +                              const MemoryRegionIOMMUOps *ops,
> +                              const char *name,
> +                              uint64_t size)
> +{
> +    memory_region_init_iommu_type(TYPE_IOMMU_MEMORY_REGION, iommu_mr,
> +                                  owner, ops, name, size);

I see that memory_region_init_iommu_type() is only used to create
TYPE_IOMMU_MEMORY_REGION typed MRs. Then would
memory_region_init_iommu() enough for us? Or do you have plan to add
new memory region types?

Thanks,

-- 
Peter Xu



reply via email to

[Prev in Thread] Current Thread [Next in Thread]