[Qemu-devel] [RFC PATCH v2 00/12] iommu: add MemTxAttrs argument to IOMMU translate function

[Qemu-devel] [RFC PATCH v2 00/12] iommu: add MemTxAttrs argument to IOMMU translate function

[Peter Maydell]

This is an RFC patchset because it's a little bit unmotivated
and only lightly tested, but in principle it could be
committed, so half-RFC-half-not :-)

The Arm SMMU wants to know if the transaction it is handling
is secure/nonsecure and user/privileged, because the iommu
page tables can be configured by the guest to only allow
transactions which satisfy those criteria. At the moment
Eric's implementation ignores all that, because we don't
provide the IOMMUMemoryRegion translate function with any
memory transaction attribute information. This patchset fixes
that by plumbing through transaction attributes.

Most of the patchset is just starting at the leaves of the calltree
rooted at "flatview_do_translate()" and making callsites provide
attributes where appropriate or plumbing through existing attribute
information where it exists.  General principles of when I made a
caller pass MEMTXATTRS_UNSPECIFIED and when I had it take an
attrs value from further up:
 * dma_memory_* functions all assume UNSPECIFIED (matching
   the read/write/rw functions that already do that)
 * cpu_physical_memory_* also all assume UNSPECIFIED,
   following the pattern of existing functions in that family
 * address_space_* take an attributes argument, by analogy
   with existing functions in that family
 * endpoints like target-specific code or vhost has to
   provide attributes, but for all the targets affected here
   they don't care about attributes and can use UNSPECIFIED

As well as the SMMU, I'm also thinking about using the IOMMU
infrastructure for the v8M Memory Protection Controller
(though that is a bit trickier as I also need it to support
TCG execution in an IOMMU-controlled region, which is an
orthogonal bit of work to attribute support).

Based-on: <20180430122404.10741-1-peter.maydell@linaro.org>
("memory.h: Improve IOMMU related documentation") but
only for textual reasons.

v1->v2 changes: only adding the patch 1/12 that I forgot
to include in the v1 posting...

thanks
-- PMM

Peter Maydell (12):
  Make tb_invalidate_phys_addr() take a MemTxAttrs argument
  Make address_space_translate() take a MemTxAttrs argument
  Make address_space_map() take a MemTxAttrs argument
  Make address_space_access_valid() take a MemTxAttrs argument
  Make flatview_extend_translation() take a MemTxAttrs argument
  Make memory_region_access_valid() take a MemTxAttrs argument
  Make MemoryRegion valid.accepts callback take a MemTxAttrs argument
  Make flatview_access_valid() take a MemTxAttrs argument
  Make flatview_translate() take a MemTxAttrs argument
  Make address_space_get_iotlb_entry() take a MemTxAttrs argument
  Make flatview_do_translate() take a MemTxAttrs argument
  Add MemTxAttrs argument to IOMMU translate function

 include/exec/exec-all.h        |  5 ++-
 include/exec/memory-internal.h |  3 +-
 include/exec/memory.h          | 26 ++++++++----
 include/sysemu/dma.h           |  6 ++-
 accel/tcg/translate-all.c      |  4 +-
 exec.c                         | 75 ++++++++++++++++++++--------------
 hw/alpha/typhoon.c             |  3 +-
 hw/dma/rc4030.c                |  3 +-
 hw/hppa/dino.c                 |  3 +-
 hw/i386/amd_iommu.c            |  3 +-
 hw/i386/intel_iommu.c          |  3 +-
 hw/nvram/fw_cfg.c              | 12 ++++--
 hw/ppc/spapr_iommu.c           |  3 +-
 hw/s390x/s390-pci-bus.c        |  3 +-
 hw/s390x/s390-pci-inst.c       |  3 +-
 hw/scsi/esp.c                  |  3 +-
 hw/sparc/sun4m_iommu.c         |  3 +-
 hw/sparc64/sun4u_iommu.c       |  3 +-
 hw/vfio/common.c               |  3 +-
 hw/virtio/vhost.c              |  3 +-
 hw/xen/xen_pt_msi.c            |  3 +-
 memory.c                       | 15 ++++---
 memory_ldst.inc.c              | 18 ++++----
 target/ppc/mmu-hash64.c        |  3 +-
 target/riscv/helper.c          |  2 +-
 target/s390x/diag.c            |  6 ++-
 target/s390x/excp_helper.c     |  3 +-
 target/s390x/mmu_helper.c      |  3 +-
 target/s390x/sigp.c            |  3 +-
 target/xtensa/op_helper.c      |  3 +-
 30 files changed, 141 insertions(+), 88 deletions(-)

-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 01/12] Make tb_invalidate_phys_addr() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to tb_invalidate_phys_addr().
Its callers either have an attrs value to hand, or don't care
and can use MEMTXATTRS_UNSPECIFIED.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/exec-all.h   | 5 +++--
 accel/tcg/translate-all.c | 2 +-
 exec.c                    | 2 +-
 target/xtensa/op_helper.c | 3 ++-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index bd68328ed9..4d09eaba72 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -255,7 +255,7 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
 void tlb_set_page(CPUState *cpu, target_ulong vaddr,
                   hwaddr paddr, int prot,
                   int mmu_idx, target_ulong size);
-void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr);
+void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs);
 void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
                  uintptr_t retaddr);
 #else
@@ -303,7 +303,8 @@ static inline void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu,
                                                        uint16_t idxmap)
 {
 }
-static inline void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr)
+static inline void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr,
+                                           MemTxAttrs attrs)
 {
 }
 #endif
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index f409d42d54..f04a922ef7 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1672,7 +1672,7 @@ static TranslationBlock *tb_find_pc(uintptr_t tc_ptr)
 }
 
 #if !defined(CONFIG_USER_ONLY)
-void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr)
+void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs)
 {
     ram_addr_t ram_addr;
     MemoryRegion *mr;
diff --git a/exec.c b/exec.c
index c7fcefa851..df35e6dd85 100644
--- a/exec.c
+++ b/exec.c
@@ -863,7 +863,7 @@ static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
     if (phys != -1) {
         /* Locks grabbed by tb_invalidate_phys_addr */
         tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as,
-                                phys | (pc & ~TARGET_PAGE_MASK));
+                                phys | (pc & ~TARGET_PAGE_MASK), attrs);
     }
 }
 #endif
diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c
index e3bcbe10d6..8a8c763c63 100644
--- a/target/xtensa/op_helper.c
+++ b/target/xtensa/op_helper.c
@@ -105,7 +105,8 @@ static void tb_invalidate_virtual_addr(CPUXtensaState *env, uint32_t vaddr)
     int ret = xtensa_get_physical_addr(env, false, vaddr, 2, 0,
             &paddr, &page_size, &access);
     if (ret == 0) {
-        tb_invalidate_phys_addr(&address_space_memory, paddr);
+        tb_invalidate_phys_addr(&address_space_memory, paddr,
+                                MEMTXATTRS_UNSPECIFIED);
     }
 }
 
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 02/12] Make address_space_translate() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to address_space_translate().
Its callers either have an attrs value to hand, or don't care
and can use MEMTXATTRS_UNSPECIFIED.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/memory.h     |  4 +++-
 accel/tcg/translate-all.c |  2 +-
 exec.c                    |  6 ++++--
 hw/vfio/common.c          |  3 ++-
 memory_ldst.inc.c         | 18 +++++++++---------
 target/riscv/helper.c     |  2 +-
 6 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index e62965a0c8..f416d1e985 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1909,6 +1909,7 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
  * #MemoryRegion.
  * @len: pointer to length
  * @is_write: indicates the transfer direction
+ * @attrs: memory attributes
  */
 MemoryRegion *flatview_translate(FlatView *fv,
                                  hwaddr addr, hwaddr *xlat,
@@ -1916,7 +1917,8 @@ MemoryRegion *flatview_translate(FlatView *fv,
 
 static inline MemoryRegion *address_space_translate(AddressSpace *as,
                                                     hwaddr addr, hwaddr *xlat,
-                                                    hwaddr *len, bool is_write)
+                                                    hwaddr *len, bool is_write,
+                                                    MemTxAttrs attrs)
 {
     return flatview_translate(address_space_to_flatview(as),
                               addr, xlat, len, is_write);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index f04a922ef7..52f7bd59a9 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1679,7 +1679,7 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs)
     hwaddr l = 1;
 
     rcu_read_lock();
-    mr = address_space_translate(as, addr, &addr, &l, false);
+    mr = address_space_translate(as, addr, &addr, &l, false, attrs);
     if (!(memory_region_is_ram(mr)
           || memory_region_is_romd(mr))) {
         rcu_read_unlock();
diff --git a/exec.c b/exec.c
index df35e6dd85..a0f27b7b8c 100644
--- a/exec.c
+++ b/exec.c
@@ -3287,7 +3287,8 @@ static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as,
     rcu_read_lock();
     while (len > 0) {
         l = len;
-        mr = address_space_translate(as, addr, &addr1, &l, true);
+        mr = address_space_translate(as, addr, &addr1, &l, true,
+                                     MEMTXATTRS_UNSPECIFIED);
 
         if (!(memory_region_is_ram(mr) ||
               memory_region_is_romd(mr))) {
@@ -3716,7 +3717,8 @@ bool cpu_physical_memory_is_io(hwaddr phys_addr)
 
     rcu_read_lock();
     mr = address_space_translate(&address_space_memory,
-                                 phys_addr, &phys_addr, &l, false);
+                                 phys_addr, &phys_addr, &l, false,
+                                 MEMTXATTRS_UNSPECIFIED);
 
     res = !(memory_region_is_ram(mr) || memory_region_is_romd(mr));
     rcu_read_unlock();
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 07ffa0ba10..8e57265edf 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -324,7 +324,8 @@ static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr,
      */
     mr = address_space_translate(&address_space_memory,
                                  iotlb->translated_addr,
-                                 &xlat, &len, writable);
+                                 &xlat, &len, writable,
+                                 MEMTXATTRS_UNSPECIFIED);
     if (!memory_region_is_ram(mr)) {
         error_report("iommu map to non memory area %"HWADDR_PRIx"",
                      xlat);
diff --git a/memory_ldst.inc.c b/memory_ldst.inc.c
index 5dbff9cef8..860ba31ac8 100644
--- a/memory_ldst.inc.c
+++ b/memory_ldst.inc.c
@@ -33,7 +33,7 @@ static inline uint32_t glue(address_space_ldl_internal, SUFFIX)(ARG1_DECL,
     bool release_lock = false;
 
     RCU_READ_LOCK();
-    mr = TRANSLATE(addr, &addr1, &l, false);
+    mr = TRANSLATE(addr, &addr1, &l, false, attrs);
     if (l < 4 || !IS_DIRECT(mr, false)) {
         release_lock |= prepare_mmio_access(mr);
 
@@ -127,7 +127,7 @@ static inline uint64_t glue(address_space_ldq_internal, SUFFIX)(ARG1_DECL,
     bool release_lock = false;
 
     RCU_READ_LOCK();
-    mr = TRANSLATE(addr, &addr1, &l, false);
+    mr = TRANSLATE(addr, &addr1, &l, false, attrs);
     if (l < 8 || !IS_DIRECT(mr, false)) {
         release_lock |= prepare_mmio_access(mr);
 
@@ -219,7 +219,7 @@ uint32_t glue(address_space_ldub, SUFFIX)(ARG1_DECL,
     bool release_lock = false;
 
     RCU_READ_LOCK();
-    mr = TRANSLATE(addr, &addr1, &l, false);
+    mr = TRANSLATE(addr, &addr1, &l, false, attrs);
     if (!IS_DIRECT(mr, false)) {
         release_lock |= prepare_mmio_access(mr);
 
@@ -261,7 +261,7 @@ static inline uint32_t glue(address_space_lduw_internal, SUFFIX)(ARG1_DECL,
     bool release_lock = false;
 
     RCU_READ_LOCK();
-    mr = TRANSLATE(addr, &addr1, &l, false);
+    mr = TRANSLATE(addr, &addr1, &l, false, attrs);
     if (l < 2 || !IS_DIRECT(mr, false)) {
         release_lock |= prepare_mmio_access(mr);
 
@@ -356,7 +356,7 @@ void glue(address_space_stl_notdirty, SUFFIX)(ARG1_DECL,
     bool release_lock = false;
 
     RCU_READ_LOCK();
-    mr = TRANSLATE(addr, &addr1, &l, true);
+    mr = TRANSLATE(addr, &addr1, &l, true, attrs);
     if (l < 4 || !IS_DIRECT(mr, true)) {
         release_lock |= prepare_mmio_access(mr);
 
@@ -399,7 +399,7 @@ static inline void glue(address_space_stl_internal, SUFFIX)(ARG1_DECL,
     bool release_lock = false;
 
     RCU_READ_LOCK();
-    mr = TRANSLATE(addr, &addr1, &l, true);
+    mr = TRANSLATE(addr, &addr1, &l, true, attrs);
     if (l < 4 || !IS_DIRECT(mr, true)) {
         release_lock |= prepare_mmio_access(mr);
 
@@ -489,7 +489,7 @@ void glue(address_space_stb, SUFFIX)(ARG1_DECL,
     bool release_lock = false;
 
     RCU_READ_LOCK();
-    mr = TRANSLATE(addr, &addr1, &l, true);
+    mr = TRANSLATE(addr, &addr1, &l, true, attrs);
     if (!IS_DIRECT(mr, true)) {
         release_lock |= prepare_mmio_access(mr);
         r = memory_region_dispatch_write(mr, addr1, val, 1, attrs);
@@ -528,7 +528,7 @@ static inline void glue(address_space_stw_internal, SUFFIX)(ARG1_DECL,
     bool release_lock = false;
 
     RCU_READ_LOCK();
-    mr = TRANSLATE(addr, &addr1, &l, true);
+    mr = TRANSLATE(addr, &addr1, &l, true, attrs);
     if (l < 2 || !IS_DIRECT(mr, true)) {
         release_lock |= prepare_mmio_access(mr);
 
@@ -619,7 +619,7 @@ static void glue(address_space_stq_internal, SUFFIX)(ARG1_DECL,
     bool release_lock = false;
 
     RCU_READ_LOCK();
-    mr = TRANSLATE(addr, &addr1, &l, true);
+    mr = TRANSLATE(addr, &addr1, &l, true, attrs);
     if (l < 8 || !IS_DIRECT(mr, true)) {
         release_lock |= prepare_mmio_access(mr);
 
diff --git a/target/riscv/helper.c b/target/riscv/helper.c
index 02cbcea2b7..d7023ad78e 100644
--- a/target/riscv/helper.c
+++ b/target/riscv/helper.c
@@ -210,7 +210,7 @@ restart:
                 MemoryRegion *mr;
                 hwaddr l = sizeof(target_ulong), addr1;
                 mr = address_space_translate(cs->as, pte_addr,
-                    &addr1, &l, false);
+                    &addr1, &l, false, MEMTXATTRS_UNSPECIFIED);
                 if (memory_access_is_direct(mr, true)) {
                     target_ulong *pte_pa =
                         qemu_map_ram_ptr(mr->ram_block, addr1);
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 03/12] Make address_space_map() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to address_space_map().
Its callers either have an attrs value to hand, or don't care
and can use MEMTXATTRS_UNSPECIFIED.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/memory.h   | 3 ++-
 include/sysemu/dma.h    | 3 ++-
 exec.c                  | 6 ++++--
 target/ppc/mmu-hash64.c | 3 ++-
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index f416d1e985..1af4e3cd5b 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1953,9 +1953,10 @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_
  * @addr: address within that address space
  * @plen: pointer to length of buffer; updated on return
  * @is_write: indicates the transfer direction
+ * @attrs: memory attributes
  */
 void *address_space_map(AddressSpace *as, hwaddr addr,
-                        hwaddr *plen, bool is_write);
+                        hwaddr *plen, bool is_write, MemTxAttrs attrs);
 
 /* address_space_unmap: Unmaps a memory region previously mapped by address_space_map()
  *
diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index c228c66513..0d73902634 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -132,7 +132,8 @@ static inline void *dma_memory_map(AddressSpace *as,
     hwaddr xlen = *len;
     void *p;
 
-    p = address_space_map(as, addr, &xlen, dir == DMA_DIRECTION_FROM_DEVICE);
+    p = address_space_map(as, addr, &xlen, dir == DMA_DIRECTION_FROM_DEVICE,
+                          MEMTXATTRS_UNSPECIFIED);
     *len = xlen;
     return p;
 }
diff --git a/exec.c b/exec.c
index a0f27b7b8c..eb6471abfe 100644
--- a/exec.c
+++ b/exec.c
@@ -3494,7 +3494,8 @@ flatview_extend_translation(FlatView *fv, hwaddr addr,
 void *address_space_map(AddressSpace *as,
                         hwaddr addr,
                         hwaddr *plen,
-                        bool is_write)
+                        bool is_write,
+                        MemTxAttrs attrs)
 {
     hwaddr len = *plen;
     hwaddr l, xlat;
@@ -3581,7 +3582,8 @@ void *cpu_physical_memory_map(hwaddr addr,
                               hwaddr *plen,
                               int is_write)
 {
-    return address_space_map(&address_space_memory, addr, plen, is_write);
+    return address_space_map(&address_space_memory, addr, plen, is_write,
+                             MEMTXATTRS_UNSPECIFIED);
 }
 
 void cpu_physical_memory_unmap(void *buffer, hwaddr len,
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 7e0adecfd9..4839dc22f0 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -431,7 +431,8 @@ const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
         return NULL;
     }
 
-    hptes = address_space_map(CPU(cpu)->as, base + pte_offset, &plen, false);
+    hptes = address_space_map(CPU(cpu)->as, base + pte_offset, &plen, false,
+                              MEMTXATTRS_UNSPECIFIED);
     if (plen < (n * HASH_PTE_SIZE_64)) {
         hw_error("%s: Unable to map all requested HPTEs\n", __func__);
     }
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 04/12] Make address_space_access_valid() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to address_space_access_valid().
Its callers either have an attrs value to hand, or don't care
and can use MEMTXATTRS_UNSPECIFIED.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/memory.h      | 4 +++-
 include/sysemu/dma.h       | 3 ++-
 exec.c                     | 3 ++-
 target/s390x/diag.c        | 6 ++++--
 target/s390x/excp_helper.c | 3 ++-
 target/s390x/mmu_helper.c  | 3 ++-
 target/s390x/sigp.c        | 3 ++-
 7 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1af4e3cd5b..eb1ceace27 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1938,8 +1938,10 @@ static inline MemoryRegion *address_space_translate(AddressSpace *as,
  * @addr: address within that address space
  * @len: length of the area to be checked
  * @is_write: indicates the transfer direction
+ * @attrs: memory attributes
  */
-bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_write);
+bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len,
+                                bool is_write, MemTxAttrs attrs);
 
 /* address_space_map: map a physical memory region into a host virtual address
  *
diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index 0d73902634..5da3c4e3c5 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -77,7 +77,8 @@ static inline bool dma_memory_valid(AddressSpace *as,
                                     DMADirection dir)
 {
     return address_space_access_valid(as, addr, len,
-                                      dir == DMA_DIRECTION_FROM_DEVICE);
+                                      dir == DMA_DIRECTION_FROM_DEVICE,
+                                      MEMTXATTRS_UNSPECIFIED);
 }
 
 static inline int dma_memory_rw_relaxed(AddressSpace *as, dma_addr_t addr,
diff --git a/exec.c b/exec.c
index eb6471abfe..87650dc7ed 100644
--- a/exec.c
+++ b/exec.c
@@ -3445,7 +3445,8 @@ static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
 }
 
 bool address_space_access_valid(AddressSpace *as, hwaddr addr,
-                                int len, bool is_write)
+                                int len, bool is_write,
+                                MemTxAttrs attrs)
 {
     FlatView *fv;
     bool result;
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index a755837ad5..6ab473e7b6 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -140,7 +140,8 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
             return;
         }
         if (!address_space_access_valid(&address_space_memory, addr,
-                                        sizeof(IplParameterBlock), false)) {
+                                        sizeof(IplParameterBlock), false,
+                                        MEMTXATTRS_UNSPECIFIED)) {
             s390_program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO, ra);
             return;
         }
@@ -169,7 +170,8 @@ out:
             return;
         }
         if (!address_space_access_valid(&address_space_memory, addr,
-                                        sizeof(IplParameterBlock), true)) {
+                                        sizeof(IplParameterBlock), true,
+                                        MEMTXATTRS_UNSPECIFIED)) {
             s390_program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO, ra);
             return;
         }
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index dfee221111..f0ce60cff2 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -120,7 +120,8 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size,
 
     /* check out of RAM access */
     if (!address_space_access_valid(&address_space_memory, raddr,
-                                    TARGET_PAGE_SIZE, rw)) {
+                                    TARGET_PAGE_SIZE, rw,
+                                    MEMTXATTRS_UNSPECIFIED)) {
         DPRINTF("%s: raddr %" PRIx64 " > ram_size %" PRIx64 "\n", __func__,
                 (uint64_t)raddr, (uint64_t)ram_size);
         trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO);
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index a25deef5dd..145b62a7ef 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -461,7 +461,8 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
             return ret;
         }
         if (!address_space_access_valid(&address_space_memory, pages[i],
-                                        TARGET_PAGE_SIZE, is_write)) {
+                                        TARGET_PAGE_SIZE, is_write,
+                                        MEMTXATTRS_UNSPECIFIED)) {
             trigger_access_exception(env, PGM_ADDRESSING, ILEN_AUTO, 0);
             return -EFAULT;
         }
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index aff1530c82..c1f9245797 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -280,7 +280,8 @@ static void sigp_set_prefix(CPUState *cs, run_on_cpu_data arg)
     cpu_synchronize_state(cs);
 
     if (!address_space_access_valid(&address_space_memory, addr,
-                                    sizeof(struct LowCore), false)) {
+                                    sizeof(struct LowCore), false,
+                                    MEMTXATTRS_UNSPECIFIED)) {
         set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
         return;
     }
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 05/12] Make flatview_extend_translation() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to flatview_extend_translation().
Its callers either have an attrs value to hand, or don't care
and can use MEMTXATTRS_UNSPECIFIED.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 exec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/exec.c b/exec.c
index 87650dc7ed..e56c3442c7 100644
--- a/exec.c
+++ b/exec.c
@@ -3460,9 +3460,9 @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr,
 
 static hwaddr
 flatview_extend_translation(FlatView *fv, hwaddr addr,
-                                 hwaddr target_len,
-                                 MemoryRegion *mr, hwaddr base, hwaddr len,
-                                 bool is_write)
+                            hwaddr target_len,
+                            MemoryRegion *mr, hwaddr base, hwaddr len,
+                            bool is_write, MemTxAttrs attrs)
 {
     hwaddr done = 0;
     hwaddr xlat;
@@ -3539,7 +3539,7 @@ void *address_space_map(AddressSpace *as,
 
     memory_region_ref(mr);
     *plen = flatview_extend_translation(fv, addr, len, mr, xlat,
-                                             l, is_write);
+                                        l, is_write, attrs);
     ptr = qemu_ram_ptr_length(mr->ram_block, xlat, plen, true);
     rcu_read_unlock();
 
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 06/12] Make memory_region_access_valid() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to memory_region_access_valid().
Its callers either have an attrs value to hand, or don't care
and can use MEMTXATTRS_UNSPECIFIED.

The callsite in flatview_access_valid() is part of a recursive
loop flatview_access_valid() -> memory_region_access_valid() ->
 subpage_accepts() -> flatview_access_valid(); we make it pass
MEMTXATTRS_UNSPECIFIED for now, until the next several commits
have plumbed an attrs parameter through the rest of the loop
and we can add an attrs parameter to flatview_access_valid().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/memory-internal.h | 3 ++-
 exec.c                         | 4 +++-
 hw/s390x/s390-pci-inst.c       | 3 ++-
 memory.c                       | 7 ++++---
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 6a5ee42d36..063dca0475 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -34,7 +34,8 @@ static inline AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as)
 extern const MemoryRegionOps unassigned_mem_ops;
 
 bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
-                                unsigned size, bool is_write);
+                                unsigned size, bool is_write,
+                                MemTxAttrs attrs);
 
 void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section);
 AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv);
diff --git a/exec.c b/exec.c
index e56c3442c7..57a984758e 100644
--- a/exec.c
+++ b/exec.c
@@ -3433,7 +3433,9 @@ static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
         mr = flatview_translate(fv, addr, &xlat, &l, is_write);
         if (!memory_access_is_direct(mr, is_write)) {
             l = memory_access_size(mr, l, addr);
-            if (!memory_region_access_valid(mr, xlat, l, is_write)) {
+            /* When our callers all have attrs we'll pass them through here */
+            if (!memory_region_access_valid(mr, xlat, l, is_write,
+                                            MEMTXATTRS_UNSPECIFIED)) {
                 return false;
             }
         }
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 3fcc330fe3..2e7b4068c0 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -770,7 +770,8 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
     mr = s390_get_subregion(mr, offset, len);
     offset -= mr->addr;
 
-    if (!memory_region_access_valid(mr, offset, len, true)) {
+    if (!memory_region_access_valid(mr, offset, len, true,
+                                    MEMTXATTRS_UNSPECIFIED)) {
         s390_program_interrupt(env, PGM_OPERAND, 6, ra);
         return 0;
     }
diff --git a/memory.c b/memory.c
index e70b64b8b9..0f8f37a57b 100644
--- a/memory.c
+++ b/memory.c
@@ -1347,7 +1347,8 @@ static const MemoryRegionOps ram_device_mem_ops = {
 bool memory_region_access_valid(MemoryRegion *mr,
                                 hwaddr addr,
                                 unsigned size,
-                                bool is_write)
+                                bool is_write,
+                                MemTxAttrs attrs)
 {
     int access_size_min, access_size_max;
     int access_size, i;
@@ -1416,7 +1417,7 @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
 {
     MemTxResult r;
 
-    if (!memory_region_access_valid(mr, addr, size, false)) {
+    if (!memory_region_access_valid(mr, addr, size, false, attrs)) {
         *pval = unassigned_mem_read(mr, addr, size);
         return MEMTX_DECODE_ERROR;
     }
@@ -1458,7 +1459,7 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
                                          unsigned size,
                                          MemTxAttrs attrs)
 {
-    if (!memory_region_access_valid(mr, addr, size, true)) {
+    if (!memory_region_access_valid(mr, addr, size, true, attrs)) {
         unassigned_mem_write(mr, addr, data, size);
         return MEMTX_DECODE_ERROR;
     }
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 07/12] Make MemoryRegion valid.accepts callback take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to the MemoryRegion valid.accepts
callback. We'll need this for subpage_accepts().

We could take the approach we used with the read and write
callbacks and add new a new _with_attrs version, but since there
are so few implementations of the accepts hook we just change
them all.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/memory.h |  3 ++-
 exec.c                |  9 ++++++---
 hw/hppa/dino.c        |  3 ++-
 hw/nvram/fw_cfg.c     | 12 ++++++++----
 hw/scsi/esp.c         |  3 ++-
 hw/xen/xen_pt_msi.c   |  3 ++-
 memory.c              |  5 +++--
 7 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index eb1ceace27..7c461b9718 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -166,7 +166,8 @@ struct MemoryRegionOps {
          * as a machine check exception).
          */
         bool (*accepts)(void *opaque, hwaddr addr,
-                        unsigned size, bool is_write);
+                        unsigned size, bool is_write,
+                        MemTxAttrs attrs);
     } valid;
     /* Internal implementation constraints: */
     struct {
diff --git a/exec.c b/exec.c
index 57a984758e..3ceeb0643f 100644
--- a/exec.c
+++ b/exec.c
@@ -2504,7 +2504,8 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
 }
 
 static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
-                                 unsigned size, bool is_write)
+                                 unsigned size, bool is_write,
+                                 MemTxAttrs attrs)
 {
     return is_write;
 }
@@ -2727,7 +2728,8 @@ static MemTxResult subpage_write(void *opaque, hwaddr addr,
 }
 
 static bool subpage_accepts(void *opaque, hwaddr addr,
-                            unsigned len, bool is_write)
+                            unsigned len, bool is_write,
+                            MemTxAttrs attrs)
 {
     subpage_t *subpage = opaque;
 #if defined(DEBUG_SUBPAGE)
@@ -2810,7 +2812,8 @@ static void readonly_mem_write(void *opaque, hwaddr addr,
 }
 
 static bool readonly_mem_accepts(void *opaque, hwaddr addr,
-                                 unsigned size, bool is_write)
+                                 unsigned size, bool is_write,
+                                 MemTxAttrs attrs)
 {
     return is_write;
 }
diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
index 15aefde09c..77463672a3 100644
--- a/hw/hppa/dino.c
+++ b/hw/hppa/dino.c
@@ -137,7 +137,8 @@ static void gsc_to_pci_forwarding(DinoState *s)
 }
 
 static bool dino_chip_mem_valid(void *opaque, hwaddr addr,
-                                unsigned size, bool is_write)
+                                unsigned size, bool is_write,
+                                MemTxAttrs attrs)
 {
     switch (addr) {
     case DINO_IAR0:
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 2a0739d0e9..b23e7f64a8 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -420,14 +420,16 @@ static void fw_cfg_dma_mem_write(void *opaque, hwaddr addr,
 }
 
 static bool fw_cfg_dma_mem_valid(void *opaque, hwaddr addr,
-                                  unsigned size, bool is_write)
+                                 unsigned size, bool is_write,
+                                 MemTxAttrs attrs)
 {
     return !is_write || ((size == 4 && (addr == 0 || addr == 4)) ||
                          (size == 8 && addr == 0));
 }
 
 static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr,
-                                  unsigned size, bool is_write)
+                                  unsigned size, bool is_write,
+                                  MemTxAttrs attrs)
 {
     return addr == 0;
 }
@@ -439,7 +441,8 @@ static void fw_cfg_ctl_mem_write(void *opaque, hwaddr addr,
 }
 
 static bool fw_cfg_ctl_mem_valid(void *opaque, hwaddr addr,
-                                 unsigned size, bool is_write)
+                                 unsigned size, bool is_write,
+                                 MemTxAttrs attrs)
 {
     return is_write && size == 2;
 }
@@ -458,7 +461,8 @@ static void fw_cfg_comb_write(void *opaque, hwaddr addr,
 }
 
 static bool fw_cfg_comb_valid(void *opaque, hwaddr addr,
-                                  unsigned size, bool is_write)
+                              unsigned size, bool is_write,
+                              MemTxAttrs attrs)
 {
     return (size == 1) || (is_write && size == 2);
 }
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 64ec285826..9ed9727744 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -564,7 +564,8 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val)
 }
 
 static bool esp_mem_accepts(void *opaque, hwaddr addr,
-                            unsigned size, bool is_write)
+                            unsigned size, bool is_write,
+                            MemTxAttrs attrs)
 {
     return (size == 1) || (is_write && size == 4);
 }
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index 6d1e3bdeb4..cc514f9157 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -498,7 +498,8 @@ static uint64_t pci_msix_read(void *opaque, hwaddr addr,
 }
 
 static bool pci_msix_accepts(void *opaque, hwaddr addr,
-                             unsigned size, bool is_write)
+                             unsigned size, bool is_write,
+                             MemTxAttrs attrs)
 {
     return !(addr & (size - 1));
 }
diff --git a/memory.c b/memory.c
index 0f8f37a57b..a729c29862 100644
--- a/memory.c
+++ b/memory.c
@@ -1269,7 +1269,8 @@ static void unassigned_mem_write(void *opaque, hwaddr addr,
 }
 
 static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
-                                   unsigned size, bool is_write)
+                                   unsigned size, bool is_write,
+                                   MemTxAttrs attrs)
 {
     return false;
 }
@@ -1374,7 +1375,7 @@ bool memory_region_access_valid(MemoryRegion *mr,
     access_size = MAX(MIN(size, access_size_max), access_size_min);
     for (i = 0; i < size; i += access_size) {
         if (!mr->ops->valid.accepts(mr->opaque, addr + i, access_size,
-                                    is_write)) {
+                                    is_write, attrs)) {
             return false;
         }
     }
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 08/12] Make flatview_access_valid() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to flatview_access_valid().
Its callers now all have an attrs value to hand, so we can
correct our earlier temporary use of MEMTXATTRS_UNSPECIFIED.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 exec.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index 3ceeb0643f..0eef4702a5 100644
--- a/exec.c
+++ b/exec.c
@@ -2662,7 +2662,7 @@ static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
 static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
                                   const uint8_t *buf, int len);
 static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
-                                  bool is_write);
+                                  bool is_write, MemTxAttrs attrs);
 
 static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
                                 unsigned len, MemTxAttrs attrs)
@@ -2738,7 +2738,7 @@ static bool subpage_accepts(void *opaque, hwaddr addr,
 #endif
 
     return flatview_access_valid(subpage->fv, addr + subpage->base,
-                                 len, is_write);
+                                 len, is_write, attrs);
 }
 
 static const MemoryRegionOps subpage_ops = {
@@ -3426,7 +3426,7 @@ static void cpu_notify_map_clients(void)
 }
 
 static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
-                                  bool is_write)
+                                  bool is_write, MemTxAttrs attrs)
 {
     MemoryRegion *mr;
     hwaddr l, xlat;
@@ -3437,8 +3437,7 @@ static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
         if (!memory_access_is_direct(mr, is_write)) {
             l = memory_access_size(mr, l, addr);
             /* When our callers all have attrs we'll pass them through here */
-            if (!memory_region_access_valid(mr, xlat, l, is_write,
-                                            MEMTXATTRS_UNSPECIFIED)) {
+            if (!memory_region_access_valid(mr, xlat, l, is_write, attrs)) {
                 return false;
             }
         }
@@ -3458,7 +3457,7 @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr,
 
     rcu_read_lock();
     fv = address_space_to_flatview(as);
-    result = flatview_access_valid(fv, addr, len, is_write);
+    result = flatview_access_valid(fv, addr, len, is_write, attrs);
     rcu_read_unlock();
     return result;
 }
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 09/12] Make flatview_translate() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to flatview_translate(); all its
callers now have attrs available.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/memory.h |  7 ++++---
 exec.c                | 17 +++++++++--------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7c461b9718..bd50424804 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1914,7 +1914,8 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
  */
 MemoryRegion *flatview_translate(FlatView *fv,
                                  hwaddr addr, hwaddr *xlat,
-                                 hwaddr *len, bool is_write);
+                                 hwaddr *len, bool is_write,
+                                 MemTxAttrs attrs);
 
 static inline MemoryRegion *address_space_translate(AddressSpace *as,
                                                     hwaddr addr, hwaddr *xlat,
@@ -1922,7 +1923,7 @@ static inline MemoryRegion *address_space_translate(AddressSpace *as,
                                                     MemTxAttrs attrs)
 {
     return flatview_translate(address_space_to_flatview(as),
-                              addr, xlat, len, is_write);
+                              addr, xlat, len, is_write, attrs);
 }
 
 /* address_space_access_valid: check for validity of accessing an address
@@ -2024,7 +2025,7 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
             rcu_read_lock();
             fv = address_space_to_flatview(as);
             l = len;
-            mr = flatview_translate(fv, addr, &addr1, &l, false);
+            mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
             if (len == l && memory_access_is_direct(mr, false)) {
                 ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
                 memcpy(buf, ptr, len);
diff --git a/exec.c b/exec.c
index 0eef4702a5..41f7a7f5c4 100644
--- a/exec.c
+++ b/exec.c
@@ -583,7 +583,8 @@ iotlb_fail:
 
 /* Called from RCU critical section */
 MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
-                                 hwaddr *plen, bool is_write)
+                                 hwaddr *plen, bool is_write,
+                                 MemTxAttrs attrs)
 {
     MemoryRegion *mr;
     MemoryRegionSection section;
@@ -3117,7 +3118,7 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
         }
 
         l = len;
-        mr = flatview_translate(fv, addr, &addr1, &l, true);
+        mr = flatview_translate(fv, addr, &addr1, &l, true, attrs);
     }
 
     return result;
@@ -3133,7 +3134,7 @@ static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
     MemTxResult result = MEMTX_OK;
 
     l = len;
-    mr = flatview_translate(fv, addr, &addr1, &l, true);
+    mr = flatview_translate(fv, addr, &addr1, &l, true, attrs);
     result = flatview_write_continue(fv, addr, attrs, buf, len,
                                      addr1, l, mr);
 
@@ -3204,7 +3205,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
         }
 
         l = len;
-        mr = flatview_translate(fv, addr, &addr1, &l, false);
+        mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
     }
 
     return result;
@@ -3219,7 +3220,7 @@ static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
     MemoryRegion *mr;
 
     l = len;
-    mr = flatview_translate(fv, addr, &addr1, &l, false);
+    mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
     return flatview_read_continue(fv, addr, attrs, buf, len,
                                   addr1, l, mr);
 }
@@ -3433,7 +3434,7 @@ static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
 
     while (len > 0) {
         l = len;
-        mr = flatview_translate(fv, addr, &xlat, &l, is_write);
+        mr = flatview_translate(fv, addr, &xlat, &l, is_write, attrs);
         if (!memory_access_is_direct(mr, is_write)) {
             l = memory_access_size(mr, l, addr);
             /* When our callers all have attrs we'll pass them through here */
@@ -3482,7 +3483,7 @@ flatview_extend_translation(FlatView *fv, hwaddr addr,
 
         len = target_len;
         this_mr = flatview_translate(fv, addr, &xlat,
-                                                   &len, is_write);
+                                     &len, is_write, attrs);
         if (this_mr != mr || xlat != base + done) {
             return done;
         }
@@ -3515,7 +3516,7 @@ void *address_space_map(AddressSpace *as,
     l = len;
     rcu_read_lock();
     fv = address_space_to_flatview(as);
-    mr = flatview_translate(fv, addr, &xlat, &l, is_write);
+    mr = flatview_translate(fv, addr, &xlat, &l, is_write, attrs);
 
     if (!memory_access_is_direct(mr, is_write)) {
         if (atomic_xchg(&bounce.in_use, true)) {
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 10/12] Make address_space_get_iotlb_entry() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to address_space_get_iotlb_entry().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/memory.h | 2 +-
 exec.c                | 2 +-
 hw/virtio/vhost.c     | 3 ++-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index bd50424804..16a82d9722 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1897,7 +1897,7 @@ void stq_be_phys_cached(MemoryRegionCache *cache, hwaddr addr, uint64_t val);
  * entry. Should be called from an RCU critical section.
  */
 IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
-                                            bool is_write);
+                                            bool is_write, MemTxAttrs attrs);
 
 /* address_space_translate: translate an address range into an address space
  * into a MemoryRegion and an address range into that section.  Should be
diff --git a/exec.c b/exec.c
index 41f7a7f5c4..c29bf47ce2 100644
--- a/exec.c
+++ b/exec.c
@@ -547,7 +547,7 @@ translate_fail:
 
 /* Called from RCU critical section */
 IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
-                                            bool is_write)
+                                            bool is_write, MemTxAttrs attrs)
 {
     MemoryRegionSection section;
     hwaddr xlat, page_mask;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 9d5850a7d7..48f4fd7cc9 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -895,7 +895,8 @@ int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write)
     rcu_read_lock();
 
     iotlb = address_space_get_iotlb_entry(dev->vdev->dma_as,
-                                          iova, write);
+                                          iova, write,
+                                          MEMTXATTRS_UNSPECIFIED);
     if (iotlb.target_as != NULL) {
         ret = vhost_memory_region_lookup(dev, iotlb.translated_addr,
                                          &uaddr, &len);
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 11/12] Make flatview_do_translate() take a MemTxAttrs argument

[Peter Maydell]

As part of plumbing MemTxAttrs down to the IOMMU translate method,
add MemTxAttrs as an argument to flatview_do_translate().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 exec.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index c29bf47ce2..9c6d9aae28 100644
--- a/exec.c
+++ b/exec.c
@@ -476,6 +476,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
  *            would tell. It can be @NULL if we don't care about it.
  * @is_write: whether the translation operation is for write
  * @is_mmio: whether this can be MMIO, set true if it can
+ * @attrs: memory transaction attributes
  *
  * This function is called from RCU critical section
  */
@@ -486,7 +487,8 @@ static MemoryRegionSection flatview_do_translate(FlatView *fv,
                                                  hwaddr *page_mask_out,
                                                  bool is_write,
                                                  bool is_mmio,
-                                                 AddressSpace **target_as)
+                                                 AddressSpace **target_as,
+                                                 MemTxAttrs attrs)
 {
     IOMMUTLBEntry iotlb;
     MemoryRegionSection *section;
@@ -557,7 +559,8 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
      * but page mask.
      */
     section = flatview_do_translate(address_space_to_flatview(as), addr, &xlat,
-                                    NULL, &page_mask, is_write, false, &as);
+                                    NULL, &page_mask, is_write, false, &as,
+                                    attrs);
 
     /* Illegal translation */
     if (section.mr == &io_mem_unassigned) {
@@ -592,7 +595,7 @@ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
 
     /* This can be MMIO, so setup MMIO bit. */
     section = flatview_do_translate(fv, addr, xlat, plen, NULL,
-                                    is_write, true, &as);
+                                    is_write, true, &as, attrs);
     mr = section.mr;
 
     if (xen_enabled() && memory_access_is_direct(mr, is_write)) {
-- 
2.17.0

[Qemu-devel] [RFC PATCH v2 12/12] Add MemTxAttrs argument to IOMMU translate function

[Peter Maydell]

Add a MemTxAttrs argument to the IOMMU translate function; this is
necessary for IOMMU implementations that care about transaction
attributes such as user/privileged or secure/nonsecure when
deciding whether a transaction is permitted.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/memory.h    | 3 ++-
 exec.c                   | 2 +-
 hw/alpha/typhoon.c       | 3 ++-
 hw/dma/rc4030.c          | 3 ++-
 hw/i386/amd_iommu.c      | 3 ++-
 hw/i386/intel_iommu.c    | 3 ++-
 hw/ppc/spapr_iommu.c     | 3 ++-
 hw/s390x/s390-pci-bus.c  | 3 ++-
 hw/sparc/sun4m_iommu.c   | 3 ++-
 hw/sparc64/sun4u_iommu.c | 3 ++-
 memory.c                 | 3 ++-
 11 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 16a82d9722..2c7dd4b373 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -221,9 +221,10 @@ typedef struct IOMMUMemoryRegionClass {
      * @iommu: the IOMMUMemoryRegion
      * @hwaddr: address to be translated within the memory region
      * @flag: requested access permissions
+     * @attrs: memory transaction attributes
      */
     IOMMUTLBEntry (*translate)(IOMMUMemoryRegion *iommu, hwaddr addr,
-                               IOMMUAccessFlags flag);
+                               IOMMUAccessFlags flag, MemTxAttrs attrs);
     /* Returns minimum supported page size in bytes.
      * If this method is not provided then the minimum is assumed to
      * be TARGET_PAGE_SIZE.
diff --git a/exec.c b/exec.c
index 9c6d9aae28..e346424172 100644
--- a/exec.c
+++ b/exec.c
@@ -513,7 +513,7 @@ static MemoryRegionSection flatview_do_translate(FlatView *fv,
         imrc = memory_region_get_iommu_class_nocheck(iommu_mr);
 
         iotlb = imrc->translate(iommu_mr, addr, is_write ?
-                                IOMMU_WO : IOMMU_RO);
+                                IOMMU_WO : IOMMU_RO, attrs);
         addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
                 | (addr & iotlb.addr_mask));
         page_mask &= iotlb.addr_mask;
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 6a40869488..49192ab24d 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -666,7 +666,8 @@ static bool window_translate(TyphoonWindow *win, hwaddr addr,
    Pchip and generate a machine check interrupt.  */
 static IOMMUTLBEntry typhoon_translate_iommu(IOMMUMemoryRegion *iommu,
                                              hwaddr addr,
-                                             IOMMUAccessFlags flag)
+                                             IOMMUAccessFlags flag,
+                                             MemTxAttrs attrs)
 {
     TyphoonPchip *pchip = container_of(iommu, TyphoonPchip, iommu);
     IOMMUTLBEntry ret;
diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index 5d4833eeca..89686ae7dc 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -491,7 +491,8 @@ static const MemoryRegionOps jazzio_ops = {
 };
 
 static IOMMUTLBEntry rc4030_dma_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
-                                          IOMMUAccessFlags flag)
+                                          IOMMUAccessFlags flag,
+                                          MemTxAttrs attrs)
 {
     rc4030State *s = container_of(iommu, rc4030State, dma_mr);
     IOMMUTLBEntry ret = {
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 63d46ff6ee..5f530b5fe6 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -991,7 +991,8 @@ static inline bool amdvi_is_interrupt_addr(hwaddr addr)
 }
 
 static IOMMUTLBEntry amdvi_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
-                                     IOMMUAccessFlags flag)
+                                     IOMMUAccessFlags flag,
+                                     MemTxAttrs attrs)
 {
     AMDVIAddressSpace *as = container_of(iommu, AMDVIAddressSpace, iommu);
     AMDVIState *s = as->iommu_state;
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index fb31de9416..483ff305f8 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2282,7 +2282,8 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
 }
 
 static IOMMUTLBEntry vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
-                                         IOMMUAccessFlags flag)
+                                         IOMMUAccessFlags flag,
+                                         MemTxAttrs attrs)
 {
     VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
     IntelIOMMUState *s = vtd_as->iommu_state;
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index aaa6010d5c..199612095a 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -112,7 +112,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(IOMMUMemoryRegion *iommu,
                                                hwaddr addr,
-                                               IOMMUAccessFlags flag)
+                                               IOMMUAccessFlags flag,
+                                               MemTxAttrs attrs)
 {
     sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu);
     uint64_t tce;
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 10da87458e..77588c2355 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -484,7 +484,8 @@ uint16_t s390_guest_io_table_walk(uint64_t g_iota, hwaddr addr,
 }
 
 static IOMMUTLBEntry s390_translate_iommu(IOMMUMemoryRegion *mr, hwaddr addr,
-                                          IOMMUAccessFlags flag)
+                                          IOMMUAccessFlags flag,
+                                          MemTxAttrs attrs)
 {
     S390PCIIOMMU *iommu = container_of(mr, S390PCIIOMMU, iommu_mr);
     S390IOTLBEntry *entry;
diff --git a/hw/sparc/sun4m_iommu.c b/hw/sparc/sun4m_iommu.c
index b677601fc6..f68bcade3c 100644
--- a/hw/sparc/sun4m_iommu.c
+++ b/hw/sparc/sun4m_iommu.c
@@ -282,7 +282,8 @@ static void iommu_bad_addr(IOMMUState *s, hwaddr addr,
 /* Called from RCU critical section */
 static IOMMUTLBEntry sun4m_translate_iommu(IOMMUMemoryRegion *iommu,
                                            hwaddr addr,
-                                           IOMMUAccessFlags flags)
+                                           IOMMUAccessFlags flags,
+                                           MemTxAttrs attrs)
 {
     IOMMUState *is = container_of(iommu, IOMMUState, iommu);
     hwaddr page, pa;
diff --git a/hw/sparc64/sun4u_iommu.c b/hw/sparc64/sun4u_iommu.c
index eb3aaa87e6..7a5a588aed 100644
--- a/hw/sparc64/sun4u_iommu.c
+++ b/hw/sparc64/sun4u_iommu.c
@@ -73,7 +73,8 @@
 /* Called from RCU critical section */
 static IOMMUTLBEntry sun4u_translate_iommu(IOMMUMemoryRegion *iommu,
                                            hwaddr addr,
-                                           IOMMUAccessFlags flag)
+                                           IOMMUAccessFlags flag,
+                                           MemTxAttrs attrs)
 {
     IOMMUState *is = container_of(iommu, IOMMUState, iommu);
     hwaddr baseaddr, offset;
diff --git a/memory.c b/memory.c
index a729c29862..dbb9718bea 100644
--- a/memory.c
+++ b/memory.c
@@ -1832,7 +1832,8 @@ void memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n)
     granularity = memory_region_iommu_get_min_page_size(iommu_mr);
 
     for (addr = 0; addr < memory_region_size(mr); addr += granularity) {
-        iotlb = imrc->translate(iommu_mr, addr, IOMMU_NONE);
+        iotlb = imrc->translate(iommu_mr, addr, IOMMU_NONE,
+                                MEMTXATTRS_UNSPECIFIED);
         if (iotlb.perm != IOMMU_NONE) {
             n->notify(n, &iotlb);
         }
-- 
2.17.0

Re: [Qemu-devel] [RFC PATCH v2 00/12] iommu: add MemTxAttrs argument to IOMMU translate function

[Paolo Bonzini]

On 01/05/2018 10:59, Peter Maydell wrote:
> As well as the SMMU, I'm also thinking about using the IOMMU
> infrastructure for the v8M Memory Protection Controller
> (though that is a bit trickier as I also need it to support
> TCG execution in an IOMMU-controlled region, which is an
> orthogonal bit of work to attribute support).
> 
> Based-on: <20180430122404.10741-1-peter.maydell@linaro.org>
> ("memory.h: Improve IOMMU related documentation") but
> only for textual reasons.
> 
> v1->v2 changes: only adding the patch 1/12 that I forgot
> to include in the v1 posting...

This is going to conflict with the MemoryRegionCache patches (review
welcome :)), as they add a new function address_space_translate_iommu.
But apart from this it seems like a no brainer.

Paolo

Re: [Qemu-devel] [RFC PATCH v2 00/12] iommu: add MemTxAttrs argument to IOMMU translate function

[Peter Maydell]

On 2 May 2018 at 10:25, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 01/05/2018 10:59, Peter Maydell wrote:
>> As well as the SMMU, I'm also thinking about using the IOMMU
>> infrastructure for the v8M Memory Protection Controller
>> (though that is a bit trickier as I also need it to support
>> TCG execution in an IOMMU-controlled region, which is an
>> orthogonal bit of work to attribute support).

> This is going to conflict with the MemoryRegionCache patches (review
> welcome :)), as they add a new function address_space_translate_iommu.
> But apart from this it seems like a no brainer.

(We talked about this on IRC, but I don't really have any
good ideas about the API changes needed, so I figured I'd
write it up here.)

While working through this and the MPC changes, I realized that
if we pass transaction attributes through to the IOMMU translate
method then we also need to change the API of the IOMMU notifier
system. This is because a txattrs-aware IOMMU might return more
than one mapping for a particular input address (eg "use mapping
A if txattrs.secure, but mapping B if not"). So map/unmap events
become more complex than just "unmap for address X" and "map for
address X" -- you need to distinguish "here's the new map for
address X with txattrs XA".

Presumably we also want a way for notifier users like vfio to
detect "I'm dealing with an IOMMU that is txattrs aware in a
way I can't deal with" so they can usefully bail out rather than
not working.

Unfortunately I don't really know enough about our two current
users of notifiers (vhost and vfio) to know what they actually
need the iommu notifications for...

(For the notifier I need to add for TCG, the requirements are
easy: I just want to be able to get unmap events so I can
flush the TCG TLB when the info I've cached from translate
calls is stale.)

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v2 00/12] iommu: add MemTxAttrs argument to IOMMU translate function

[Paolo Bonzini]

On 15/05/2018 18:28, Peter Maydell wrote:
> 
> Presumably we also want a way for notifier users like vfio to
> detect "I'm dealing with an IOMMU that is txattrs aware in a
> way I can't deal with" so they can usefully bail out rather than
> not working.
> 
> Unfortunately I don't really know enough about our two current
> users of notifiers (vhost and vfio) to know what they actually
> need the iommu notifications for...

As you guessed on IRC, they basically establish a shadow IOMMU page
table or TLB.  My proposal would be to add two MemTxAttrs arguments to
the IOMMUNotify typedef and to memory_region_notify_one, respectively to
indicate which attributes matter (0 = indifferent, 1 = matter) and their
value.  So far so good.

Perhaps memory_region_register_iommu_notifier can also get an argument
with the supported attributes.  The function would then fail if there
are fewer bits set in that argument than what the IOMMU supports...

The only problem with that is that memory_region_register_iommu_notifier
is called late from VFIO, and in a place that really cannot fail (a
MemoryListener's region_add callback).  So I would not be sure about how
to deal with failure in the VFIO code.

Thanks,

Paolo

Re: [Qemu-devel] [RFC PATCH v2 00/12] iommu: add MemTxAttrs argument to IOMMU translate function

[Peter Maydell]

On 15 May 2018 at 17:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 15/05/2018 18:28, Peter Maydell wrote:
>>
>> Presumably we also want a way for notifier users like vfio to
>> detect "I'm dealing with an IOMMU that is txattrs aware in a
>> way I can't deal with" so they can usefully bail out rather than
>> not working.
>>
>> Unfortunately I don't really know enough about our two current
>> users of notifiers (vhost and vfio) to know what they actually
>> need the iommu notifications for...
>
> As you guessed on IRC, they basically establish a shadow IOMMU page
> table or TLB.  My proposal would be to add two MemTxAttrs arguments to
> the IOMMUNotify typedef and to memory_region_notify_one, respectively to
> indicate which attributes matter (0 = indifferent, 1 = matter) and their
> value.  So far so good.
>
> Perhaps memory_region_register_iommu_notifier can also get an argument
> with the supported attributes.  The function would then fail if there
> are fewer bits set in that argument than what the IOMMU supports...

So I had an idea this morning for a slightly different way to approach
this, which is that we add a concept of an iommu_idx, analogous to
our existing TCG mmu_idx. Basically an iommu_idx represents a
page table (kind of like what Arm calls a "translation regime"), so
that for any particular iommu index and input address the output
address and permissions are always the same. For the memory
protection controller there would be two iommu_idxes, one for
secure and one for non-secure.

The API changes would be something like:
 * new method get_num_indexes() which returns the number of iommu indexes
   this IOMMU implements (default implementation: return 1)
 * translate method takes an iommu index (and not the txattrs)
 * new method index_from_attrs() which takes a MemTxAttrs and
   returns the iommu index that should be used (default implementation:
   always return 0)
 * memory_region_register_iommu_notifier() takes an iommu index
 * the default 'replay' method does "for each supported index,
   for each address, call @translate"
 * vfio and vhost can register their notifiers using the index
   returned by index_from_attrs(MEMTXATTRS_UNSPECIFIED)
 * maybe they could call get_num_indexes() and bail out if the
   IOMMU has support for multiple indexes?
 * maybe they could be enhanced to support tracking multiple
   page tables if necessary in future

I haven't worked through the details yet, but this seems to me
more flexible than working directly with txattrs. It also means
it's harder to accidentally write an iommu implementation that
looks at more fields in the txattrs than its notifier interface
claims are significant to it.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v2 00/12] iommu: add MemTxAttrs argument to IOMMU translate function

[Paolo Bonzini]

On 17/05/2018 12:48, Peter Maydell wrote:
> So I had an idea this morning for a slightly different way to approach
> this, which is that we add a concept of an iommu_idx, analogous to
> our existing TCG mmu_idx. Basically an iommu_idx represents a
> page table (kind of like what Arm calls a "translation regime"), so
> that for any particular iommu index and input address the output
> address and permissions are always the same. For the memory
> protection controller there would be two iommu_idxes, one for
> secure and one for non-secure.
> 
> The API changes would be something like:
>  * new method get_num_indexes() which returns the number of iommu indexes
>    this IOMMU implements (default implementation: return 1)
>  * translate method takes an iommu index (and not the txattrs)
>  * new method index_from_attrs() which takes a MemTxAttrs and
>    returns the iommu index that should be used (default implementation:
>    always return 0)
>  * memory_region_register_iommu_notifier() takes an iommu index
>  * the default 'replay' method does "for each supported index,
>    for each address, call @translate"
>  * vfio and vhost can register their notifiers using the index
>    returned by index_from_attrs(MEMTXATTRS_UNSPECIFIED)
>  * maybe they could call get_num_indexes() and bail out if the
>    IOMMU has support for multiple indexes?
>  * maybe they could be enhanced to support tracking multiple
>    page tables if necessary in future
> 
> I haven't worked through the details yet, but this seems to me
> more flexible than working directly with txattrs. It also means
> it's harder to accidentally write an iommu implementation that
> looks at more fields in the txattrs than its notifier interface
> claims are significant to it.

Yes, this also sounds good.  It does have the same issue for VFIO that
get_num_indexes() would be called too late to fail (and again, in a
place where it's hard to fail).

Maybe the index count and the index-from/to-attrs translation should be
static (index-to-attrs could use the same pair of MemTxAttrs for "which
bits matter" and "what value should they have"), so that VFIO can
inspect it and decide if it's okay to proceed with e.g. the first iommu_idx?

Thanks,

Paolo

Re: [Qemu-devel] [RFC PATCH v2 00/12] iommu: add MemTxAttrs argument to IOMMU translate function

[Peter Maydell]

On 17 May 2018 at 13:46, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Yes, this also sounds good.  It does have the same issue for VFIO that
> get_num_indexes() would be called too late to fail (and again, in a
> place where it's hard to fail).
>
> Maybe the index count and the index-from/to-attrs translation should be
> static (index-to-attrs could use the same pair of MemTxAttrs for "which
> bits matter" and "what value should they have"), so that VFIO can
> inspect it and decide if it's okay to proceed with e.g. the first iommu_idx?

Well, they need to be static in the sense that the IOMMU can't
change its opinion later about what it has. So as long as you
have a pointer to the IOMMU you can ask it how many indexes it has.

thanks
-- PMM