All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection
@ 2013-05-24 17:05 Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram Paolo Bonzini
                   ` (22 more replies)
  0 siblings, 23 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

This is part 2 of the memory/IOMMU patches.  These reorganize the
handling of unassigned accesses so that they are propagated as
errors during I/O dispatch.  In the end, a return value is added to
address_space_rw/read/write.  This is particularly useful when an IOMMU
is available, because it lets devices detect faulting accesses.

Compared to v1, there is no special casing of the "unassigned" dummy
section and subpages are handled correctly.  Most of the patches are new.

The updated full series, including the IOMMU and ref/unref patches, is
available at refs/heads/iommu on my github repository.

Paolo

Paolo Bonzini (22):
  exec: eliminate io_mem_ram
  exec: drop useless #if
  cputlb: simplify tlb_set_page
  exec: make io_mem_unassigned private
  exec: do not use error_mem_read
  memory: dispatch unassigned accesses based on .valid.accepts
  memory: add address_space_translate
  memory: move unassigned_mem_ops to memory.c
  memory: assign MemoryRegionOps to all regions
  exec: expect mr->ops to be initialized for ROM
  exec: introduce memory_access_is_direct
  exec: introduce memory_access_size
  memory: export memory_region_access_valid to exec.c
  exec: implement .valid.accepts for subpages
  memory: add address_space_access_valid
  memory: accept mismatching sizes in memory_region_access_valid
  memory: add big endian support to access_with_adjusted_size
  memory: split accesses even when the old MMIO callbacks are used
  memory: correctly handle endian-swapped 64-bit accesses
  exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses
  memory: propagate errors on I/O dispatch
  memory: add return value to address_space_rw/read/write

 cputlb.c                        |  31 ++-
 dma-helpers.c                   |   5 +
 exec.c                          | 412 ++++++++++++++++++++--------------------
 include/exec/cpu-common.h       |   2 -
 include/exec/cputlb.h           |  12 +-
 include/exec/exec-all.h         |   6 +-
 include/exec/memory-internal.h  |   5 +
 include/exec/memory.h           |  58 ++++--
 include/exec/softmmu_template.h |  36 +---
 include/sysemu/dma.h            |   3 +-
 memory.c                        | 215 +++++++++++++--------
 translate-all.c                 |   6 +-
 12 files changed, 419 insertions(+), 372 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 02/22] exec: drop useless #if Paolo Bonzini
                   ` (21 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

It is never used, the IOTLB always goes through io_mem_notdirty.

In fact in softmmu_template.h, if it were, QEMU would crash just
below the tests, as soon as io_mem_read/write dispatches to
error_mem_read/write.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                          | 18 ++----------------
 include/exec/cpu-common.h       |  1 -
 include/exec/softmmu_template.h |  4 ++--
 3 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/exec.c b/exec.c
index 3a9ddcb..b720be5 100644
--- a/exec.c
+++ b/exec.c
@@ -66,7 +66,7 @@ AddressSpace address_space_io;
 AddressSpace address_space_memory;
 DMAContext dma_context_memory;
 
-MemoryRegion io_mem_ram, io_mem_rom, io_mem_unassigned, io_mem_notdirty;
+MemoryRegion io_mem_rom, io_mem_unassigned, io_mem_notdirty;
 static MemoryRegion io_mem_subpage_ram;
 
 #endif
@@ -200,8 +200,7 @@ MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index)
 
 bool memory_region_is_unassigned(MemoryRegion *mr)
 {
-    return mr != &io_mem_ram && mr != &io_mem_rom
-        && mr != &io_mem_notdirty && !mr->rom_device
+    return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device
         && mr != &io_mem_watch;
 }
 #endif
@@ -1419,18 +1418,6 @@ static uint64_t error_mem_read(void *opaque, hwaddr addr,
     abort();
 }
 
-static void error_mem_write(void *opaque, hwaddr addr,
-                            uint64_t value, unsigned size)
-{
-    abort();
-}
-
-static const MemoryRegionOps error_mem_ops = {
-    .read = error_mem_read,
-    .write = error_mem_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
 static const MemoryRegionOps rom_mem_ops = {
     .read = error_mem_read,
     .write = unassigned_mem_write,
@@ -1691,7 +1678,6 @@ MemoryRegion *iotlb_to_region(hwaddr index)
 
 static void io_mem_init(void)
 {
-    memory_region_init_io(&io_mem_ram, &error_mem_ops, NULL, "ram", UINT64_MAX);
     memory_region_init_io(&io_mem_rom, &rom_mem_ops, NULL, "rom", UINT64_MAX);
     memory_region_init_io(&io_mem_unassigned, &unassigned_mem_ops, NULL,
                           "unassigned", UINT64_MAX);
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index af5258d..1686b8f 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -110,7 +110,6 @@ void stq_phys(hwaddr addr, uint64_t val);
 void cpu_physical_memory_write_rom(hwaddr addr,
                                    const uint8_t *buf, int len);
 
-extern struct MemoryRegion io_mem_ram;
 extern struct MemoryRegion io_mem_rom;
 extern struct MemoryRegion io_mem_unassigned;
 extern struct MemoryRegion io_mem_notdirty;
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index b219191..4501dac 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -68,7 +68,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
 
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
     env->mem_io_pc = retaddr;
-    if (mr != &io_mem_ram && mr != &io_mem_rom
+    if (mr != &io_mem_rom
         && mr != &io_mem_unassigned
         && mr != &io_mem_notdirty
             && !can_do_io(env)) {
@@ -218,7 +218,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env,
     MemoryRegion *mr = iotlb_to_region(physaddr);
 
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
-    if (mr != &io_mem_ram && mr != &io_mem_rom
+    if (mr != &io_mem_rom
         && mr != &io_mem_unassigned
         && mr != &io_mem_notdirty
             && !can_do_io(env)) {
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 02/22] exec: drop useless #if
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 03/22] cputlb: simplify tlb_set_page Paolo Bonzini
                   ` (20 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

This code is only compiled for softmmu targets.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/exec.c b/exec.c
index b720be5..7728ea3 100644
--- a/exec.c
+++ b/exec.c
@@ -1430,10 +1430,8 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
     int dirty_flags;
     dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr);
     if (!(dirty_flags & CODE_DIRTY_FLAG)) {
-#if !defined(CONFIG_USER_ONLY)
         tb_invalidate_phys_page_fast(ram_addr, size);
         dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr);
-#endif
     }
     switch (size) {
     case 1:
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 03/22] cputlb: simplify tlb_set_page
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 02/22] exec: drop useless #if Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 04/22] exec: make io_mem_unassigned private Paolo Bonzini
                   ` (19 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The same "if" condition is repeated twice.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cputlb.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index aba7e44..b56bc01 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -262,17 +262,14 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
 #endif
 
     address = vaddr;
-    if (!(memory_region_is_ram(section->mr) ||
-          memory_region_is_romd(section->mr))) {
-        /* IO memory case (romd handled later) */
+    if (!memory_region_is_ram(section->mr) && !memory_region_is_romd(section->mr)) {
+        /* IO memory case */
         address |= TLB_MMIO;
-    }
-    if (memory_region_is_ram(section->mr) ||
-        memory_region_is_romd(section->mr)) {
+        addend = 0;
+    } else {
+        /* TLB_MMIO for rom/romd handled below */
         addend = (uintptr_t)memory_region_get_ram_ptr(section->mr)
         + memory_region_section_addr(section, paddr);
-    } else {
-        addend = 0;
     }
 
     code_address = address;
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 04/22] exec: make io_mem_unassigned private
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (2 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 03/22] cputlb: simplify tlb_set_page Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 05/22] exec: do not use error_mem_read Paolo Bonzini
                   ` (18 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

There is no reason to avoid a recompile before accessing unassigned
memory.  In the end it will be treated as MMIO anyway.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                          |  4 ++--
 include/exec/cpu-common.h       |  1 -
 include/exec/softmmu_template.h | 10 ++--------
 3 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/exec.c b/exec.c
index 7728ea3..7e22980 100644
--- a/exec.c
+++ b/exec.c
@@ -66,8 +66,8 @@ AddressSpace address_space_io;
 AddressSpace address_space_memory;
 DMAContext dma_context_memory;
 
-MemoryRegion io_mem_rom, io_mem_unassigned, io_mem_notdirty;
-static MemoryRegion io_mem_subpage_ram;
+MemoryRegion io_mem_rom, io_mem_notdirty;
+static MemoryRegion io_mem_unassigned, io_mem_subpage_ram;
 
 #endif
 
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 1686b8f..e061e21 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -111,7 +111,6 @@ void cpu_physical_memory_write_rom(hwaddr addr,
                                    const uint8_t *buf, int len);
 
 extern struct MemoryRegion io_mem_rom;
-extern struct MemoryRegion io_mem_unassigned;
 extern struct MemoryRegion io_mem_notdirty;
 
 #endif
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index 4501dac..ca91fd0 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -68,10 +68,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
 
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
     env->mem_io_pc = retaddr;
-    if (mr != &io_mem_rom
-        && mr != &io_mem_unassigned
-        && mr != &io_mem_notdirty
-            && !can_do_io(env)) {
+    if (mr != &io_mem_rom && mr != &io_mem_notdirty && !can_do_io(env)) {
         cpu_io_recompile(env, retaddr);
     }
 
@@ -218,10 +215,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env,
     MemoryRegion *mr = iotlb_to_region(physaddr);
 
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
-    if (mr != &io_mem_rom
-        && mr != &io_mem_unassigned
-        && mr != &io_mem_notdirty
-            && !can_do_io(env)) {
+    if (mr != &io_mem_rom && mr != &io_mem_notdirty && !can_do_io(env)) {
         cpu_io_recompile(env, retaddr);
     }
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 05/22] exec: do not use error_mem_read
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (3 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 04/22] exec: make io_mem_unassigned private Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts Paolo Bonzini
                   ` (17 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

We will soon reach this case when doing (unaligned) accesses that
partly span past the end of memory.  We do not want to crash in
that case.

unassigned_mem_ops and rom_mem_ops are now the same.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/exec.c b/exec.c
index 7e22980..785eeeb 100644
--- a/exec.c
+++ b/exec.c
@@ -1412,18 +1412,6 @@ static const MemoryRegionOps unassigned_mem_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static uint64_t error_mem_read(void *opaque, hwaddr addr,
-                               unsigned size)
-{
-    abort();
-}
-
-static const MemoryRegionOps rom_mem_ops = {
-    .read = error_mem_read,
-    .write = unassigned_mem_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
 static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
                                uint64_t val, unsigned size)
 {
@@ -1455,7 +1443,7 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
 }
 
 static const MemoryRegionOps notdirty_mem_ops = {
-    .read = error_mem_read,
+    .read = unassigned_mem_read,
     .write = notdirty_mem_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
@@ -1676,7 +1664,7 @@ MemoryRegion *iotlb_to_region(hwaddr index)
 
 static void io_mem_init(void)
 {
-    memory_region_init_io(&io_mem_rom, &rom_mem_ops, NULL, "rom", UINT64_MAX);
+    memory_region_init_io(&io_mem_rom, &unassigned_mem_ops, NULL, "rom", UINT64_MAX);
     memory_region_init_io(&io_mem_unassigned, &unassigned_mem_ops, NULL,
                           "unassigned", UINT64_MAX);
     memory_region_init_io(&io_mem_notdirty, &notdirty_mem_ops, NULL,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (4 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 05/22] exec: do not use error_mem_read Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 07/22] memory: add address_space_translate Paolo Bonzini
                   ` (16 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

This provides the basics for detecting accesses to unassigned memory
as soon as they happen, and also for a simple implementation of
address_space_access_valid.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c   | 36 ++++++++++++------------------------
 memory.c | 28 ++++++++++++++++++++++++++--
 2 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/exec.c b/exec.c
index 785eeeb..c5100d6 100644
--- a/exec.c
+++ b/exec.c
@@ -50,7 +50,6 @@
 
 #include "exec/memory-internal.h"
 
-//#define DEBUG_UNASSIGNED
 //#define DEBUG_SUBPAGE
 
 #if !defined(CONFIG_USER_ONLY)
@@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
     return ram_addr;
 }
 
-static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
-                                    unsigned size)
+static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
+                                   unsigned size, bool is_write)
 {
-#ifdef DEBUG_UNASSIGNED
-    printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
-#endif
-#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
-    cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
-#endif
-    return 0;
-}
-
-static void unassigned_mem_write(void *opaque, hwaddr addr,
-                                 uint64_t val, unsigned size)
-{
-#ifdef DEBUG_UNASSIGNED
-    printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
-#endif
-#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
-    cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
-#endif
+    return false;
 }
 
-static const MemoryRegionOps unassigned_mem_ops = {
-    .read = unassigned_mem_read,
-    .write = unassigned_mem_write,
+const MemoryRegionOps unassigned_mem_ops = {
+    .valid.accepts = unassigned_mem_accepts,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
@@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
         tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
 }
 
+static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
+                                 unsigned size, bool is_write)
+{
+    return is_write;
+}
+
 static const MemoryRegionOps notdirty_mem_ops = {
-    .read = unassigned_mem_read,
     .write = notdirty_mem_write,
+    .valid.accepts = notdirty_mem_accepts,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
diff --git a/memory.c b/memory.c
index 99f046d..15da877 100644
--- a/memory.c
+++ b/memory.c
@@ -22,6 +22,8 @@
 
 #include "exec/memory-internal.h"
 
+//#define DEBUG_UNASSIGNED
+
 static unsigned memory_region_transaction_depth;
 static bool memory_region_update_pending;
 static bool global_dirty_log = false;
@@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
     mr->flush_coalesced_mmio = false;
 }
 
+static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
+                                    unsigned size)
+{
+#ifdef DEBUG_UNASSIGNED
+    printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
+#endif
+#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
+    cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
+#endif
+    return 0;
+}
+
+static void unassigned_mem_write(void *opaque, hwaddr addr,
+                                 uint64_t val, unsigned size)
+{
+#ifdef DEBUG_UNASSIGNED
+    printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
+#endif
+#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
+    cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
+#endif
+}
+
 static bool memory_region_access_valid(MemoryRegion *mr,
                                        hwaddr addr,
                                        unsigned size,
@@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
     uint64_t data = 0;
 
     if (!memory_region_access_valid(mr, addr, size, false)) {
-        return -1U; /* FIXME: better signalling */
+        return unassigned_mem_read(mr, addr, size);
     }
 
     if (!mr->ops->read) {
@@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
                                          unsigned size)
 {
     if (!memory_region_access_valid(mr, addr, size, true)) {
-        return; /* FIXME: better signalling */
+        unassigned_mem_write(mr, addr, data, size);
+        return;
     }
 
     adjust_endianness(mr, &data, size);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 07/22] memory: add address_space_translate
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (5 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 08/22] memory: move unassigned_mem_ops to memory.c Paolo Bonzini
                   ` (15 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Using phys_page_find to translate an AddressSpace to a MemoryRegionSection
is unwieldy.  It requires to pass the page index rather than the address,
and later memory_region_section_addr has to be called.  Replace
memory_region_section_addr with a function that does all of it: call
phys_page_find, compute the offset within the region, and check how
big the current mapping is.  This way, a large flat region can be written
with a single lookup rather than a page at a time.

address_space_translate will also provide a single point where IOMMU
forwarding is implemented.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cputlb.c              |  20 +++---
 exec.c                | 189 +++++++++++++++++++++++++-------------------------
 include/exec/cputlb.h |  12 ++--
 include/exec/memory.h |  31 ++++-----
 translate-all.c       |   6 +-
 5 files changed, 128 insertions(+), 130 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index b56bc01..86666c8 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -248,13 +248,18 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
     target_ulong code_address;
     uintptr_t addend;
     CPUTLBEntry *te;
-    hwaddr iotlb;
+    hwaddr iotlb, xlat, sz;
 
     assert(size >= TARGET_PAGE_SIZE);
     if (size != TARGET_PAGE_SIZE) {
         tlb_add_large_page(env, vaddr, size);
     }
-    section = phys_page_find(address_space_memory.dispatch, paddr >> TARGET_PAGE_BITS);
+
+    sz = size;
+    section = address_space_translate(&address_space_memory, paddr, &xlat, &sz,
+                                      false);
+    assert(sz >= TARGET_PAGE_SIZE);
+
 #if defined(DEBUG_TLB)
     printf("tlb_set_page: vaddr=" TARGET_FMT_lx " paddr=0x" TARGET_FMT_plx
            " prot=%x idx=%d pd=0x%08lx\n",
@@ -268,13 +273,12 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
         addend = 0;
     } else {
         /* TLB_MMIO for rom/romd handled below */
-        addend = (uintptr_t)memory_region_get_ram_ptr(section->mr)
-        + memory_region_section_addr(section, paddr);
+        addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat;
     }
 
     code_address = address;
-    iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, prot,
-                                            &address);
+    iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, xlat,
+                                            prot, &address);
 
     index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     env->iotlb[mmu_idx][index] = iotlb - vaddr;
@@ -297,9 +301,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
             /* Write access calls the I/O callback.  */
             te->addr_write = address | TLB_MMIO;
         } else if (memory_region_is_ram(section->mr)
-                   && !cpu_physical_memory_is_dirty(
-                           section->mr->ram_addr
-                           + memory_region_section_addr(section, paddr))) {
+                   && !cpu_physical_memory_is_dirty(section->mr->ram_addr + xlat)) {
             te->addr_write = address | TLB_NOTDIRTY;
         } else {
             te->addr_write = address;
diff --git a/exec.c b/exec.c
index c5100d6..5111327 100644
--- a/exec.c
+++ b/exec.c
@@ -182,7 +182,7 @@ static void phys_page_set(AddressSpaceDispatch *d,
     phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS - 1);
 }
 
-MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index)
+static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index)
 {
     PhysPageEntry lp = d->phys_map;
     PhysPageEntry *p;
@@ -203,6 +203,22 @@ bool memory_region_is_unassigned(MemoryRegion *mr)
     return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device
         && mr != &io_mem_watch;
 }
+
+MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
+                                             hwaddr *xlat, hwaddr *plen,
+                                             bool is_write)
+{
+    MemoryRegionSection *section;
+
+    section = phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS);
+    /* Compute offset within MemoryRegionSection */
+    addr -= section->offset_within_address_space;
+    *plen = MIN(section->size - addr, *plen);
+
+    /* Compute offset within MemoryRegion */
+    *xlat = addr + section->offset_within_region;
+    return section;
+}
 #endif
 
 void cpu_exec_init_all(void)
@@ -615,11 +631,11 @@ static int cpu_physical_memory_set_dirty_tracking(int enable)
 }
 
 hwaddr memory_region_section_get_iotlb(CPUArchState *env,
-                                                   MemoryRegionSection *section,
-                                                   target_ulong vaddr,
-                                                   hwaddr paddr,
-                                                   int prot,
-                                                   target_ulong *address)
+                                       MemoryRegionSection *section,
+                                       target_ulong vaddr,
+                                       hwaddr paddr, hwaddr xlat,
+                                       int prot,
+                                       target_ulong *address)
 {
     hwaddr iotlb;
     CPUWatchpoint *wp;
@@ -627,7 +643,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
     if (memory_region_is_ram(section->mr)) {
         /* Normal RAM.  */
         iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
-            + memory_region_section_addr(section, paddr);
+            + xlat;
         if (!section->readonly) {
             iotlb |= phys_section_notdirty;
         } else {
@@ -635,7 +651,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
         }
     } else {
         iotlb = section - phys_sections;
-        iotlb += memory_region_section_addr(section, paddr);
+        iotlb += xlat;
     }
 
     /* Make accesses to pages with watchpoints go via the
@@ -1852,24 +1868,18 @@ static void invalidate_and_set_dirty(hwaddr addr,
 void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                       int len, bool is_write)
 {
-    AddressSpaceDispatch *d = as->dispatch;
-    int l;
+    hwaddr l;
     uint8_t *ptr;
     uint32_t val;
-    hwaddr page;
+    hwaddr addr1;
     MemoryRegionSection *section;
 
     while (len > 0) {
-        page = addr & TARGET_PAGE_MASK;
-        l = (page + TARGET_PAGE_SIZE) - addr;
-        if (l > len)
-            l = len;
-        section = phys_page_find(d, page >> TARGET_PAGE_BITS);
+        l = len;
+        section = address_space_translate(as, addr, &addr1, &l, is_write);
 
         if (is_write) {
             if (!memory_region_is_ram(section->mr)) {
-                hwaddr addr1;
-                addr1 = memory_region_section_addr(section, addr);
                 /* XXX: could force cpu_single_env to NULL to avoid
                    potential bugs */
                 if (l >= 4 && ((addr1 & 3) == 0)) {
@@ -1889,9 +1899,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                     l = 1;
                 }
             } else if (!section->readonly) {
-                ram_addr_t addr1;
-                addr1 = memory_region_get_ram_addr(section->mr)
-                    + memory_region_section_addr(section, addr);
+                addr1 += memory_region_get_ram_addr(section->mr);
                 /* RAM case */
                 ptr = qemu_get_ram_ptr(addr1);
                 memcpy(ptr, buf, l);
@@ -1900,9 +1908,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
         } else {
             if (!(memory_region_is_ram(section->mr) ||
                   memory_region_is_romd(section->mr))) {
-                hwaddr addr1;
                 /* I/O case */
-                addr1 = memory_region_section_addr(section, addr);
                 if (l >= 4 && ((addr1 & 3) == 0)) {
                     /* 32 bit read access */
                     val = io_mem_read(section->mr, addr1, 4);
@@ -1921,9 +1927,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                 }
             } else {
                 /* RAM case */
-                ptr = qemu_get_ram_ptr(section->mr->ram_addr
-                                       + memory_region_section_addr(section,
-                                                                    addr));
+                ptr = qemu_get_ram_ptr(section->mr->ram_addr + addr1);
                 memcpy(buf, ptr, l);
             }
         }
@@ -1962,26 +1966,21 @@ void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
 void cpu_physical_memory_write_rom(hwaddr addr,
                                    const uint8_t *buf, int len)
 {
-    AddressSpaceDispatch *d = address_space_memory.dispatch;
-    int l;
+    hwaddr l;
     uint8_t *ptr;
-    hwaddr page;
+    hwaddr addr1;
     MemoryRegionSection *section;
 
     while (len > 0) {
-        page = addr & TARGET_PAGE_MASK;
-        l = (page + TARGET_PAGE_SIZE) - addr;
-        if (l > len)
-            l = len;
-        section = phys_page_find(d, page >> TARGET_PAGE_BITS);
+        l = len;
+        section = address_space_translate(&address_space_memory,
+                                          addr, &addr1, &l, true);
 
         if (!(memory_region_is_ram(section->mr) ||
               memory_region_is_romd(section->mr))) {
             /* do nothing */
         } else {
-            unsigned long addr1;
-            addr1 = memory_region_get_ram_addr(section->mr)
-                + memory_region_section_addr(section, addr);
+            addr1 += memory_region_get_ram_addr(section->mr);
             /* ROM/RAM case */
             ptr = qemu_get_ram_ptr(addr1);
             memcpy(ptr, buf, l);
@@ -2051,22 +2050,17 @@ void *address_space_map(AddressSpace *as,
                         hwaddr *plen,
                         bool is_write)
 {
-    AddressSpaceDispatch *d = as->dispatch;
     hwaddr len = *plen;
     hwaddr todo = 0;
-    int l;
-    hwaddr page;
+    hwaddr l, xlat;
     MemoryRegionSection *section;
     ram_addr_t raddr = RAM_ADDR_MAX;
     ram_addr_t rlen;
     void *ret;
 
     while (len > 0) {
-        page = addr & TARGET_PAGE_MASK;
-        l = (page + TARGET_PAGE_SIZE) - addr;
-        if (l > len)
-            l = len;
-        section = phys_page_find(d, page >> TARGET_PAGE_BITS);
+        l = len;
+        section = address_space_translate(as, addr, &xlat, &l, is_write);
 
         if (!(memory_region_is_ram(section->mr) && !section->readonly)) {
             if (todo || bounce.buffer) {
@@ -2083,8 +2077,11 @@ void *address_space_map(AddressSpace *as,
             return bounce.buffer;
         }
         if (!todo) {
-            raddr = memory_region_get_ram_addr(section->mr)
-                + memory_region_section_addr(section, addr);
+            raddr = memory_region_get_ram_addr(section->mr) + xlat;
+        } else {
+            if (memory_region_get_ram_addr(section->mr) + xlat != raddr + todo) {
+                break;
+            }
         }
 
         len -= l;
@@ -2150,14 +2147,16 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
     uint8_t *ptr;
     uint32_t val;
     MemoryRegionSection *section;
+    hwaddr l = 4;
+    hwaddr addr1;
 
-    section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
-    if (!(memory_region_is_ram(section->mr) ||
+    section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+                                      false);
+    if (l < 4 ||
+        !(memory_region_is_ram(section->mr) ||
           memory_region_is_romd(section->mr))) {
         /* I/O case */
-        addr = memory_region_section_addr(section, addr);
-        val = io_mem_read(section->mr, addr, 4);
+        val = io_mem_read(section->mr, addr1, 4);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap32(val);
@@ -2171,7 +2170,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
         /* RAM case */
         ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
                                 & TARGET_PAGE_MASK)
-                               + memory_region_section_addr(section, addr));
+                               + addr1);
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
             val = ldl_le_p(ptr);
@@ -2209,28 +2208,30 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
     uint8_t *ptr;
     uint64_t val;
     MemoryRegionSection *section;
+    hwaddr l = 8;
+    hwaddr addr1;
 
-    section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
-    if (!(memory_region_is_ram(section->mr) ||
+    section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+                                      false);
+    if (l < 8 ||
+        !(memory_region_is_ram(section->mr) ||
           memory_region_is_romd(section->mr))) {
         /* I/O case */
-        addr = memory_region_section_addr(section, addr);
 
         /* XXX This is broken when device endian != cpu endian.
                Fix and add "endian" variable check */
 #ifdef TARGET_WORDS_BIGENDIAN
-        val = io_mem_read(section->mr, addr, 4) << 32;
-        val |= io_mem_read(section->mr, addr + 4, 4);
+        val = io_mem_read(section->mr, addr1, 4) << 32;
+        val |= io_mem_read(section->mr, addr1 + 4, 4);
 #else
-        val = io_mem_read(section->mr, addr, 4);
-        val |= io_mem_read(section->mr, addr + 4, 4) << 32;
+        val = io_mem_read(section->mr, addr1, 4);
+        val |= io_mem_read(section->mr, addr1 + 4, 4) << 32;
 #endif
     } else {
         /* RAM case */
         ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
                                 & TARGET_PAGE_MASK)
-                               + memory_region_section_addr(section, addr));
+                               + addr1);
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
             val = ldq_le_p(ptr);
@@ -2276,14 +2277,16 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
     uint8_t *ptr;
     uint64_t val;
     MemoryRegionSection *section;
+    hwaddr l = 2;
+    hwaddr addr1;
 
-    section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
-    if (!(memory_region_is_ram(section->mr) ||
+    section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+                                      false);
+    if (l < 2 ||
+        !(memory_region_is_ram(section->mr) ||
           memory_region_is_romd(section->mr))) {
         /* I/O case */
-        addr = memory_region_section_addr(section, addr);
-        val = io_mem_read(section->mr, addr, 2);
+        val = io_mem_read(section->mr, addr1, 2);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap16(val);
@@ -2297,7 +2300,7 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
         /* RAM case */
         ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
                                 & TARGET_PAGE_MASK)
-                               + memory_region_section_addr(section, addr));
+                               + addr1);
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
             val = lduw_le_p(ptr);
@@ -2335,19 +2338,18 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
 {
     uint8_t *ptr;
     MemoryRegionSection *section;
+    hwaddr l = 4;
+    hwaddr addr1;
 
-    section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
-    if (!memory_region_is_ram(section->mr) || section->readonly) {
-        addr = memory_region_section_addr(section, addr);
+    section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+                                      true);
+    if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
         if (memory_region_is_ram(section->mr)) {
             section = &phys_sections[phys_section_rom];
         }
-        io_mem_write(section->mr, addr, val, 4);
+        io_mem_write(section->mr, addr1, val, 4);
     } else {
-        unsigned long addr1 = (memory_region_get_ram_addr(section->mr)
-                               & TARGET_PAGE_MASK)
-            + memory_region_section_addr(section, addr);
+        addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
         ptr = qemu_get_ram_ptr(addr1);
         stl_p(ptr, val);
 
@@ -2369,11 +2371,12 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
 {
     uint8_t *ptr;
     MemoryRegionSection *section;
+    hwaddr l = 4;
+    hwaddr addr1;
 
-    section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
-    if (!memory_region_is_ram(section->mr) || section->readonly) {
-        addr = memory_region_section_addr(section, addr);
+    section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+                                      true);
+    if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
         if (memory_region_is_ram(section->mr)) {
             section = &phys_sections[phys_section_rom];
         }
@@ -2386,12 +2389,10 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
             val = bswap32(val);
         }
 #endif
-        io_mem_write(section->mr, addr, val, 4);
+        io_mem_write(section->mr, addr1, val, 4);
     } else {
-        unsigned long addr1;
-        addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
-            + memory_region_section_addr(section, addr);
         /* RAM case */
+        addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
         ptr = qemu_get_ram_ptr(addr1);
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
@@ -2436,11 +2437,12 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
 {
     uint8_t *ptr;
     MemoryRegionSection *section;
+    hwaddr l = 2;
+    hwaddr addr1;
 
-    section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS);
-
-    if (!memory_region_is_ram(section->mr) || section->readonly) {
-        addr = memory_region_section_addr(section, addr);
+    section = address_space_translate(&address_space_memory, addr, &addr1, &l,
+                                      true);
+    if (l < 2 || !memory_region_is_ram(section->mr) || section->readonly) {
         if (memory_region_is_ram(section->mr)) {
             section = &phys_sections[phys_section_rom];
         }
@@ -2453,12 +2455,10 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
             val = bswap16(val);
         }
 #endif
-        io_mem_write(section->mr, addr, val, 2);
+        io_mem_write(section->mr, addr1, val, 2);
     } else {
-        unsigned long addr1;
-        addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
-            + memory_region_section_addr(section, addr);
         /* RAM case */
+        addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
         ptr = qemu_get_ram_ptr(addr1);
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
@@ -2561,9 +2561,10 @@ bool virtio_is_big_endian(void)
 bool cpu_physical_memory_is_io(hwaddr phys_addr)
 {
     MemoryRegionSection *section;
+    hwaddr l = 1;
 
-    section = phys_page_find(address_space_memory.dispatch,
-                             phys_addr >> TARGET_PAGE_BITS);
+    section = address_space_translate(&address_space_memory,
+                                      phys_addr, &phys_addr, &l, false);
 
     return !(memory_region_is_ram(section->mr) ||
              memory_region_is_romd(section->mr));
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index 733c885..e821660 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -26,8 +26,6 @@ void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t ram_addr,
                              target_ulong vaddr);
 void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start,
                            uintptr_t length);
-MemoryRegionSection *phys_page_find(struct AddressSpaceDispatch *d,
-                                    hwaddr index);
 void cpu_tlb_reset_dirty_all(ram_addr_t start1, ram_addr_t length);
 void tlb_set_dirty(CPUArchState *env, target_ulong vaddr);
 extern int tlb_flush_count;
@@ -35,11 +33,11 @@ extern int tlb_flush_count;
 /* exec.c */
 void tb_flush_jmp_cache(CPUArchState *env, target_ulong addr);
 hwaddr memory_region_section_get_iotlb(CPUArchState *env,
-                                                   MemoryRegionSection *section,
-                                                   target_ulong vaddr,
-                                                   hwaddr paddr,
-                                                   int prot,
-                                                   target_ulong *address);
+                                       MemoryRegionSection *section,
+                                       target_ulong vaddr,
+                                       hwaddr paddr, hwaddr xlat,
+                                       int prot,
+                                       target_ulong *address);
 bool memory_region_is_unassigned(MemoryRegion *mr);
 
 #endif
diff --git a/include/exec/memory.h b/include/exec/memory.h
index fdf55fe..688d3f0 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -752,23 +752,6 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
                                        hwaddr addr, uint64_t size);
 
 /**
- * memory_region_section_addr: get offset within MemoryRegionSection
- *
- * Returns offset within MemoryRegionSection
- *
- * @section: the memory region section being queried
- * @addr: address in address space
- */
-static inline hwaddr
-memory_region_section_addr(MemoryRegionSection *section,
-                           hwaddr addr)
-{
-    addr -= section->offset_within_address_space;
-    addr += section->offset_within_region;
-    return addr;
-}
-
-/**
  * address_space_sync_dirty_bitmap: synchronize the dirty log for all memory
  *
  * Synchronizes the dirty page log for an entire address space.
@@ -869,6 +852,20 @@ void address_space_write(AddressSpace *as, hwaddr addr,
  */
 void address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len);
 
+/* address_space_translate: translate an address range into an address space
+ * into a MemoryRegionSection and an address range into that section
+ *
+ * @as: #AddressSpace to be accessed
+ * @addr: address within that address space
+ * @xlat: pointer to address within the returned memory region section's
+ * #MemoryRegion.
+ * @len: pointer to length
+ * @is_write: indicates the transfer direction
+ */
+MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
+                                             hwaddr *xlat, hwaddr *len,
+                                             bool is_write);
+
 /* address_space_map: map a physical memory region into a host virtual address
  *
  * May map a subset of the requested range, given by and returned in @plen.
diff --git a/translate-all.c b/translate-all.c
index 0d84b0d..7a7d537 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1355,15 +1355,15 @@ void tb_invalidate_phys_addr(hwaddr addr)
 {
     ram_addr_t ram_addr;
     MemoryRegionSection *section;
+    hwaddr l = 1;
 
-    section = phys_page_find(address_space_memory.dispatch,
-                             addr >> TARGET_PAGE_BITS);
+    section = address_space_translate(&address_space_memory, addr, &addr, &l, false);
     if (!(memory_region_is_ram(section->mr)
           || memory_region_is_romd(section->mr))) {
         return;
     }
     ram_addr = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
-        + memory_region_section_addr(section, addr);
+        + addr;
     tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0);
 }
 #endif /* TARGET_HAS_ICE && !defined(CONFIG_USER_ONLY) */
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 08/22] memory: move unassigned_mem_ops to memory.c
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (6 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 07/22] memory: add address_space_translate Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 09/22] memory: assign MemoryRegionOps to all regions Paolo Bonzini
                   ` (14 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

reservation_ops is already doing the same thing.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                         | 12 ------------
 include/exec/memory-internal.h |  2 ++
 memory.c                       | 44 ++++++++++++++----------------------------
 3 files changed, 16 insertions(+), 42 deletions(-)

diff --git a/exec.c b/exec.c
index 5111327..613bbd7 100644
--- a/exec.c
+++ b/exec.c
@@ -1399,17 +1398,6 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
     return ram_addr;
 }
 
-static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
-                                   unsigned size, bool is_write)
-{
-    return false;
-}
-
-const MemoryRegionOps unassigned_mem_ops = {
-    .valid.accepts = unassigned_mem_accepts,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
 static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
                                uint64_t val, unsigned size)
 {
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 8d15f90..c18b36c 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -43,6 +43,8 @@ struct AddressSpaceDispatch {
 void address_space_init_dispatch(AddressSpace *as);
 void address_space_destroy_dispatch(AddressSpace *as);
 
+extern const MemoryRegionOps unassigned_mem_ops;
+
 ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
                                    MemoryRegion *mr);
 ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr);
diff --git a/memory.c b/memory.c
index 15da877..2e4f547 100644
--- a/memory.c
+++ b/memory.c
@@ -837,6 +839,17 @@ static void unassigned_mem_write(void *opaque, hwaddr addr,
 #endif
 }
 
+static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
+                                   unsigned size, bool is_write)
+{
+    return false;
+}
+
+const MemoryRegionOps unassigned_mem_ops = {
+    .valid.accepts = unassigned_mem_accepts,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
 static bool memory_region_access_valid(MemoryRegion *mr,
                                        hwaddr addr,
                                        unsigned size,
@@ -1001,40 +1014,11 @@ void memory_region_init_rom_device(MemoryRegion *mr,
     mr->ram_addr = qemu_ram_alloc(size, mr);
 }
 
-static uint64_t invalid_read(void *opaque, hwaddr addr,
-                             unsigned size)
-{
-    MemoryRegion *mr = opaque;
-
-    if (!mr->warning_printed) {
-        fprintf(stderr, "Invalid read from memory region %s\n", mr->name);
-        mr->warning_printed = true;
-    }
-    return -1U;
-}
-
-static void invalid_write(void *opaque, hwaddr addr, uint64_t data,
-                          unsigned size)
-{
-    MemoryRegion *mr = opaque;
-
-    if (!mr->warning_printed) {
-        fprintf(stderr, "Invalid write to memory region %s\n", mr->name);
-        mr->warning_printed = true;
-    }
-}
-
-static const MemoryRegionOps reservation_ops = {
-    .read = invalid_read,
-    .write = invalid_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
 void memory_region_init_reservation(MemoryRegion *mr,
                                     const char *name,
                                     uint64_t size)
 {
-    memory_region_init_io(mr, &reservation_ops, mr, name, size);
+    memory_region_init_io(mr, &unassigned_mem_ops, mr, name, size);
 }
 
 void memory_region_destroy(MemoryRegion *mr)
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 09/22] memory: assign MemoryRegionOps to all regions
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (7 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 08/22] memory: move unassigned_mem_ops to memory.c Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 10/22] exec: expect mr->ops to be initialized for ROM Paolo Bonzini
                   ` (13 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

This allows to remove the checks on section->readonly.  Write
accesses to ROM will not be considered "direct" even in exec.c,
and will go through mr->ops just like TCG does.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/memory.c b/memory.c
index 2e4f547..f2135d1 100644
--- a/memory.c
+++ b/memory.c
@@ -788,7 +788,8 @@ void memory_region_init(MemoryRegion *mr,
                         const char *name,
                         uint64_t size)
 {
-    mr->ops = NULL;
+    mr->ops = &unassigned_mem_ops;
+    mr->opaque = NULL;
     mr->parent = NULL;
     mr->size = int128_make64(size);
     if (size == UINT64_MAX) {
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 10/22] exec: expect mr->ops to be initialized for ROM
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (8 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 09/22] memory: assign MemoryRegionOps to all regions Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 11/22] exec: introduce memory_access_is_direct Paolo Bonzini
                   ` (12 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

There is no need anymore to use the special phys_section_rom section.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/exec.c b/exec.c
index 613bbd7..411a224 100644
--- a/exec.c
+++ b/exec.c
@@ -2332,9 +2332,6 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
     section = address_space_translate(&address_space_memory, addr, &addr1, &l,
                                       true);
     if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
-        if (memory_region_is_ram(section->mr)) {
-            section = &phys_sections[phys_section_rom];
-        }
         io_mem_write(section->mr, addr1, val, 4);
     } else {
         addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
@@ -2365,9 +2362,6 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
     section = address_space_translate(&address_space_memory, addr, &addr1, &l,
                                       true);
     if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
-        if (memory_region_is_ram(section->mr)) {
-            section = &phys_sections[phys_section_rom];
-        }
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap32(val);
@@ -2431,9 +2425,6 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
     section = address_space_translate(&address_space_memory, addr, &addr1, &l,
                                       true);
     if (l < 2 || !memory_region_is_ram(section->mr) || section->readonly) {
-        if (memory_region_is_ram(section->mr)) {
-            section = &phys_sections[phys_section_rom];
-        }
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap16(val);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 11/22] exec: introduce memory_access_is_direct
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (9 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 10/22] exec: expect mr->ops to be initialized for ROM Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 12/22] exec: introduce memory_access_size Paolo Bonzini
                   ` (11 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

After the previous patches, this is a common test for all read/write
functions.

address_space_rw to ROMs is now treated as "unassigned" instead of being
ignored.  This matches what TCG-generated code does.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/exec.c b/exec.c
index 411a224..7f6b5dd 100644
--- a/exec.c
+++ b/exec.c
@@ -1853,6 +1853,18 @@ static void invalidate_and_set_dirty(hwaddr addr,
     xen_modified_memory(addr, length);
 }
 
+static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
+{
+    if (memory_region_is_ram(mr)) {
+        return !(is_write && mr->readonly);
+    }
+    if (memory_region_is_romd(mr)) {
+        return !is_write;
+    }
+
+    return false;
+}
+
 void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                       int len, bool is_write)
 {
@@ -1867,7 +1879,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
         section = address_space_translate(as, addr, &addr1, &l, is_write);
 
         if (is_write) {
-            if (!memory_region_is_ram(section->mr)) {
+            if (!memory_access_is_direct(section->mr, is_write)) {
                 /* XXX: could force cpu_single_env to NULL to avoid
                    potential bugs */
                 if (l >= 4 && ((addr1 & 3) == 0)) {
@@ -1886,7 +1898,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                     io_mem_write(section->mr, addr1, val, 1);
                     l = 1;
                 }
-            } else if (!section->readonly) {
+            } else {
                 addr1 += memory_region_get_ram_addr(section->mr);
                 /* RAM case */
                 ptr = qemu_get_ram_ptr(addr1);
@@ -1894,8 +1906,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                 invalidate_and_set_dirty(addr1, l);
             }
         } else {
-            if (!(memory_region_is_ram(section->mr) ||
-                  memory_region_is_romd(section->mr))) {
+            if (!memory_access_is_direct(section->mr, is_write)) {
                 /* I/O case */
                 if (l >= 4 && ((addr1 & 3) == 0)) {
                     /* 32 bit read access */
@@ -2050,7 +2061,7 @@ void *address_space_map(AddressSpace *as,
         l = len;
         section = address_space_translate(as, addr, &xlat, &l, is_write);
 
-        if (!(memory_region_is_ram(section->mr) && !section->readonly)) {
+        if (!memory_access_is_direct(section->mr, is_write)) {
             if (todo || bounce.buffer) {
                 break;
             }
@@ -2140,9 +2151,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
 
     section = address_space_translate(&address_space_memory, addr, &addr1, &l,
                                       false);
-    if (l < 4 ||
-        !(memory_region_is_ram(section->mr) ||
-          memory_region_is_romd(section->mr))) {
+    if (l < 4 || !memory_access_is_direct(section->mr, false)) {
         /* I/O case */
         val = io_mem_read(section->mr, addr1, 4);
 #if defined(TARGET_WORDS_BIGENDIAN)
@@ -2201,9 +2210,7 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
 
     section = address_space_translate(&address_space_memory, addr, &addr1, &l,
                                       false);
-    if (l < 8 ||
-        !(memory_region_is_ram(section->mr) ||
-          memory_region_is_romd(section->mr))) {
+    if (l < 8 || !memory_access_is_direct(section->mr, false)) {
         /* I/O case */
 
         /* XXX This is broken when device endian != cpu endian.
@@ -2270,9 +2277,7 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
 
     section = address_space_translate(&address_space_memory, addr, &addr1, &l,
                                       false);
-    if (l < 2 ||
-        !(memory_region_is_ram(section->mr) ||
-          memory_region_is_romd(section->mr))) {
+    if (l < 2 || !memory_access_is_direct(section->mr, false)) {
         /* I/O case */
         val = io_mem_read(section->mr, addr1, 2);
 #if defined(TARGET_WORDS_BIGENDIAN)
@@ -2331,7 +2336,7 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
 
     section = address_space_translate(&address_space_memory, addr, &addr1, &l,
                                       true);
-    if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
+    if (l < 4 || !memory_access_is_direct(section->mr, true)) {
         io_mem_write(section->mr, addr1, val, 4);
     } else {
         addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK;
@@ -2361,7 +2366,7 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val,
 
     section = address_space_translate(&address_space_memory, addr, &addr1, &l,
                                       true);
-    if (l < 4 || !memory_region_is_ram(section->mr) || section->readonly) {
+    if (l < 4 || !memory_access_is_direct(section->mr, true)) {
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap32(val);
@@ -2424,7 +2429,7 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val,
 
     section = address_space_translate(&address_space_memory, addr, &addr1, &l,
                                       true);
-    if (l < 2 || !memory_region_is_ram(section->mr) || section->readonly) {
+    if (l < 2 || !memory_access_is_direct(section->mr, true)) {
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap16(val);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 12/22] exec: introduce memory_access_size
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (10 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 11/22] exec: introduce memory_access_is_direct Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 13/22] memory: export memory_region_access_valid to exec.c Paolo Bonzini
                   ` (10 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

This will be used by address_space_access_valid too.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/exec.c b/exec.c
index 7f6b5dd..519a82d 100644
--- a/exec.c
+++ b/exec.c
@@ -1865,6 +1865,17 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
     return false;
 }
 
+static inline int memory_access_size(int l, hwaddr addr)
+{
+    if (l >= 4 && ((addr & 3) == 0)) {
+        return 4;
+    }
+    if (l >= 2 && ((addr & 1) == 0)) {
+        return 2;
+    }
+    return 1;
+}
+
 void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                       int len, bool is_write)
 {
@@ -1880,23 +1891,21 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
 
         if (is_write) {
             if (!memory_access_is_direct(section->mr, is_write)) {
+                l = memory_access_size(l, addr1);
                 /* XXX: could force cpu_single_env to NULL to avoid
                    potential bugs */
-                if (l >= 4 && ((addr1 & 3) == 0)) {
+                if (l == 4) {
                     /* 32 bit write access */
                     val = ldl_p(buf);
                     io_mem_write(section->mr, addr1, val, 4);
-                    l = 4;
-                } else if (l >= 2 && ((addr1 & 1) == 0)) {
+                } else if (l == 2) {
                     /* 16 bit write access */
                     val = lduw_p(buf);
                     io_mem_write(section->mr, addr1, val, 2);
-                    l = 2;
                 } else {
                     /* 8 bit write access */
                     val = ldub_p(buf);
                     io_mem_write(section->mr, addr1, val, 1);
-                    l = 1;
                 }
             } else {
                 addr1 += memory_region_get_ram_addr(section->mr);
@@ -1908,21 +1917,19 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
         } else {
             if (!memory_access_is_direct(section->mr, is_write)) {
                 /* I/O case */
-                if (l >= 4 && ((addr1 & 3) == 0)) {
+                l = memory_access_size(l, addr1);
+                if (l == 4) {
                     /* 32 bit read access */
                     val = io_mem_read(section->mr, addr1, 4);
                     stl_p(buf, val);
-                    l = 4;
-                } else if (l >= 2 && ((addr1 & 1) == 0)) {
+                } else if (l == 2) {
                     /* 16 bit read access */
                     val = io_mem_read(section->mr, addr1, 2);
                     stw_p(buf, val);
-                    l = 2;
                 } else {
                     /* 8 bit read access */
                     val = io_mem_read(section->mr, addr1, 1);
                     stb_p(buf, val);
-                    l = 1;
                 }
             } else {
                 /* RAM case */
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 13/22] memory: export memory_region_access_valid to exec.c
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (11 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 12/22] exec: introduce memory_access_size Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 14/22] exec: implement .valid.accepts for subpages Paolo Bonzini
                   ` (9 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

We'll use it to implement address_space_access_valid.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/memory-internal.h | 3 +++
 memory.c                       | 8 ++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index c18b36c..799c02a 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -45,6 +45,9 @@ void address_space_destroy_dispatch(AddressSpace *as);
 
 extern const MemoryRegionOps unassigned_mem_ops;
 
+bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
+                                unsigned size, bool is_write);
+
 ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
                                    MemoryRegion *mr);
 ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr);
diff --git a/memory.c b/memory.c
index f2135d1..9e1c1a3 100644
--- a/memory.c
+++ b/memory.c
@@ -851,10 +851,10 @@ const MemoryRegionOps unassigned_mem_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static bool memory_region_access_valid(MemoryRegion *mr,
-                                       hwaddr addr,
-                                       unsigned size,
-                                       bool is_write)
+bool memory_region_access_valid(MemoryRegion *mr,
+                                hwaddr addr,
+                                unsigned size,
+                                bool is_write)
 {
     if (mr->ops->valid.accepts
         && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write)) {
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 14/22] exec: implement .valid.accepts for subpages
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (12 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 13/22] memory: export memory_region_access_valid to exec.c Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 15/22] memory: add address_space_access_valid Paolo Bonzini
                   ` (8 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/exec.c b/exec.c
index 519a82d..8107530 100644
--- a/exec.c
+++ b/exec.c
@@ -1555,9 +1555,29 @@ static void subpage_write(void *opaque, hwaddr addr,
     io_mem_write(section->mr, addr, value, len);
 }
 
+static bool subpage_accepts(void *opaque, hwaddr addr,
+                            unsigned size, bool is_write)
+{
+    subpage_t *mmio = opaque;
+    unsigned int idx = SUBPAGE_IDX(addr);
+    MemoryRegionSection *section;
+#if defined(DEBUG_SUBPAGE)
+    printf("%s: subpage %p %c len %d addr " TARGET_FMT_plx
+           " idx %d\n", __func__, mmio,
+           is_write ? 'w' : 'r', len, addr, idx);
+#endif
+
+    section = &phys_sections[mmio->sub_section[idx]];
+    addr += mmio->base;
+    addr -= section->offset_within_address_space;
+    addr += section->offset_within_region;
+    return memory_region_access_valid(section->mr, addr, size, is_write);
+}
+
 static const MemoryRegionOps subpage_ops = {
     .read = subpage_read,
     .write = subpage_write,
+    .valid.accepts = subpage_accepts,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 15/22] memory: add address_space_access_valid
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (13 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 14/22] exec: implement .valid.accepts for subpages Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 16/22] memory: accept mismatching sizes in memory_region_access_valid Paolo Bonzini
                   ` (7 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The old-style IOMMU lets you check whether an access is valid in a
given DMAContext.  There is no equivalent for AddressSpace in the
memory API, but we can implement it easily.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 dma-helpers.c         |  5 +++++
 exec.c                | 21 +++++++++++++++++++++
 include/exec/memory.h | 15 +++++++++++++++
 include/sysemu/dma.h  |  3 ++-
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/dma-helpers.c b/dma-helpers.c
index 272632f..2e298b6 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -298,6 +298,11 @@ bool iommu_dma_memory_valid(DMAContext *dma, dma_addr_t addr, dma_addr_t len,
             plen = len;
         }
 
+        if (!address_space_access_valid(dma->as, paddr, len,
+                                        dir == DMA_DIRECTION_FROM_DEVICE)) {
+            return false;
+        }
+
         len -= plen;
         addr += plen;
     }
diff --git a/exec.c b/exec.c
index 8107530..17a3292 100644
--- a/exec.c
+++ b/exec.c
@@ -2064,6 +2064,27 @@ static void cpu_notify_map_clients(void)
     }
 }
 
+bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_write)
+{
+    MemoryRegionSection *section;
+    hwaddr l, xlat;
+
+    while (len > 0) {
+        l = len;
+        section = address_space_translate(as, addr, &xlat, &l, is_write);
+        if (!memory_access_is_direct(section->mr, is_write)) {
+            l = memory_access_size(l, addr);
+            if (!memory_region_access_valid(section->mr, xlat, l, is_write)) {
+                return false;
+            }
+        }
+
+        len -= l;
+        addr += l;
+    }
+    return true;
+}
+
 /* Map a physical memory region into a host virtual address.
  * May map a subset of the requested range, given by and returned in *plen.
  * May return NULL if resources needed to perform the mapping are exhausted.
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 688d3f0..81e0e41 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -866,6 +866,21 @@ MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
                                              hwaddr *xlat, hwaddr *len,
                                              bool is_write);
 
+/* address_space_access_valid: check for validity of accessing an address
+ * space range
+ *
+ * Check whether memory is assigned to the given address space range.
+ *
+ * For now, addr and len should be aligned to a page size.  This limitation
+ * will be lifted in the future.
+ *
+ * @as: #AddressSpace to be accessed
+ * @addr: address within that address space
+ * @len: length of the area to be checked
+ * @is_write: indicates the transfer direction
+ */
+bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_write);
+
 /* address_space_map: map a physical memory region into a host virtual address
  *
  * May map a subset of the requested range, given by and returned in @plen.
diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index a52c93a..02e0dcd 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -113,7 +113,8 @@ static inline bool dma_memory_valid(DMAContext *dma,
                                     DMADirection dir)
 {
     if (!dma_has_iommu(dma)) {
-        return true;
+        return address_space_access_valid(dma->as, addr, len,
+                                          dir == DMA_DIRECTION_FROM_DEVICE);
     } else {
         return iommu_dma_memory_valid(dma, addr, len, dir);
     }
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 16/22] memory: accept mismatching sizes in memory_region_access_valid
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (14 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 15/22] memory: add address_space_access_valid Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 17/22] memory: add big endian support to access_with_adjusted_size Paolo Bonzini
                   ` (6 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The memory API is able to use smaller/wider accesses than requested,
match that in memory_region_access_valid.  Of course, the accepts
callback is still free to reject those accesses.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/memory.c b/memory.c
index 9e1c1a3..c72f56d 100644
--- a/memory.c
+++ b/memory.c
@@ -856,24 +856,35 @@ bool memory_region_access_valid(MemoryRegion *mr,
                                 unsigned size,
                                 bool is_write)
 {
-    if (mr->ops->valid.accepts
-        && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write)) {
-        return false;
-    }
+    int access_size_min, access_size_max;
+    int access_size, i;
 
     if (!mr->ops->valid.unaligned && (addr & (size - 1))) {
         return false;
     }
 
-    /* Treat zero as compatibility all valid */
-    if (!mr->ops->valid.max_access_size) {
+    if (!mr->ops->valid.accepts) {
         return true;
     }
 
-    if (size > mr->ops->valid.max_access_size
-        || size < mr->ops->valid.min_access_size) {
-        return false;
+    access_size_min = mr->ops->valid.min_access_size;
+    if (!mr->ops->valid.min_access_size) {
+        access_size_min = 1;
+    }
+
+    access_size_max = mr->ops->valid.max_access_size;
+    if (!mr->ops->valid.max_access_size) {
+        access_size_max = 4;
+    }
+
+    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)) {
+            return false;
+        }
     }
+
     return true;
 }
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 17/22] memory: add big endian support to access_with_adjusted_size
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (15 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 16/22] memory: accept mismatching sizes in memory_region_access_valid Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 18/22] memory: split accesses even when the old MMIO callbacks are used Paolo Bonzini
                   ` (5 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

This will be used to split 8-byte access down to two four-byte accesses.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/memory.c b/memory.c
index c72f56d..9085969 100644
--- a/memory.c
+++ b/memory.c
@@ -362,8 +362,12 @@ static void access_with_adjusted_size(hwaddr addr,
     access_size = MAX(MIN(size, access_size_max), access_size_min);
     access_mask = -1ULL >> (64 - access_size * 8);
     for (i = 0; i < size; i += access_size) {
-        /* FIXME: big-endian support */
+#if TARGET_WORDS_BIGENDIAN
+        access(opaque, addr + i, value, access_size,
+               (size - access_size - i) * 8, access_mask);
+#else
         access(opaque, addr + i, value, access_size, i * 8, access_mask);
+#endif
     }
 }
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 18/22] memory: split accesses even when the old MMIO callbacks are used
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (16 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 17/22] memory: add big endian support to access_with_adjusted_size Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 19/22] memory: correctly handle endian-swapped 64-bit accesses Paolo Bonzini
                   ` (4 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

This is useful for 64-bit memory accesses.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/memory.c b/memory.c
index 9085969..728a6c5 100644
--- a/memory.c
+++ b/memory.c
@@ -302,6 +302,20 @@ static void flatview_simplify(FlatView *view)
     }
 }
 
+static void memory_region_oldmmio_read_accessor(void *opaque,
+                                                hwaddr addr,
+                                                uint64_t *value,
+                                                unsigned size,
+                                                unsigned shift,
+                                                uint64_t mask)
+{
+    MemoryRegion *mr = opaque;
+    uint64_t tmp;
+
+    tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
+    *value |= (tmp & mask) << shift;
+}
+
 static void memory_region_read_accessor(void *opaque,
                                         hwaddr addr,
                                         uint64_t *value,
@@ -319,6 +333,20 @@ static void memory_region_read_accessor(void *opaque,
     *value |= (tmp & mask) << shift;
 }
 
+static void memory_region_oldmmio_write_accessor(void *opaque,
+                                                 hwaddr addr,
+                                                 uint64_t *value,
+                                                 unsigned size,
+                                                 unsigned shift,
+                                                 uint64_t mask)
+{
+    MemoryRegion *mr = opaque;
+    uint64_t tmp;
+
+    tmp = (*value >> shift) & mask;
+    mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
+}
+
 static void memory_region_write_accessor(void *opaque,
                                          hwaddr addr,
                                          uint64_t *value,
@@ -359,6 +387,8 @@ static void access_with_adjusted_size(hwaddr addr,
     if (!access_size_max) {
         access_size_max = 4;
     }
+
+    /* FIXME: support unaligned access? */
     access_size = MAX(MIN(size, access_size_max), access_size_min);
     access_mask = -1ULL >> (64 - access_size * 8);
     for (i = 0; i < size; i += access_size) {
@@ -902,16 +932,16 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
         return unassigned_mem_read(mr, addr, size);
     }
 
-    if (!mr->ops->read) {
-        return mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
+    if (mr->ops->read) {
+        access_with_adjusted_size(addr, &data, size,
+                                  mr->ops->impl.min_access_size,
+                                  mr->ops->impl.max_access_size,
+                                  memory_region_read_accessor, mr);
+    } else {
+        access_with_adjusted_size(addr, &data, size, 1, 4,
+                                  memory_region_oldmmio_read_accessor, mr);
     }
 
-    /* FIXME: support unaligned access */
-    access_with_adjusted_size(addr, &data, size,
-                              mr->ops->impl.min_access_size,
-                              mr->ops->impl.max_access_size,
-                              memory_region_read_accessor, mr);
-
     return data;
 }
 
@@ -956,16 +986,15 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
 
     adjust_endianness(mr, &data, size);
 
-    if (!mr->ops->write) {
-        mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, data);
-        return;
+    if (mr->ops->write) {
+        access_with_adjusted_size(addr, &data, size,
+                                  mr->ops->impl.min_access_size,
+                                  mr->ops->impl.max_access_size,
+                                  memory_region_write_accessor, mr);
+    } else {
+        access_with_adjusted_size(addr, &data, size, 1, 4,
+                                  memory_region_oldmmio_write_accessor, mr);
     }
-
-    /* FIXME: support unaligned access */
-    access_with_adjusted_size(addr, &data, size,
-                              mr->ops->impl.min_access_size,
-                              mr->ops->impl.max_access_size,
-                              memory_region_write_accessor, mr);
 }
 
 void memory_region_init_io(MemoryRegion *mr,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 19/22] memory: correctly handle endian-swapped 64-bit accesses
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (17 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 18/22] memory: split accesses even when the old MMIO callbacks are used Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 20/22] exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses Paolo Bonzini
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c   | 12 +++++++++---
 memory.c |  3 +++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index 17a3292..42f7636 100644
--- a/exec.c
+++ b/exec.c
@@ -2260,9 +2260,6 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
                                       false);
     if (l < 8 || !memory_access_is_direct(section->mr, false)) {
         /* I/O case */
-
-        /* XXX This is broken when device endian != cpu endian.
-               Fix and add "endian" variable check */
 #ifdef TARGET_WORDS_BIGENDIAN
         val = io_mem_read(section->mr, addr1, 4) << 32;
         val |= io_mem_read(section->mr, addr1 + 4, 4);
@@ -2270,6 +2267,15 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
         val = io_mem_read(section->mr, addr1, 4);
         val |= io_mem_read(section->mr, addr1 + 4, 4) << 32;
 #endif
+#if defined(TARGET_WORDS_BIGENDIAN)
+        if (endian == DEVICE_LITTLE_ENDIAN) {
+            val = bswap64(val);
+        }
+#else
+        if (endian == DEVICE_BIG_ENDIAN) {
+            val = bswap64(val);
+        }
+#endif
     } else {
         /* RAM case */
         ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
diff --git a/memory.c b/memory.c
index 728a6c5..c9833a3 100644
--- a/memory.c
+++ b/memory.c
@@ -957,6 +957,9 @@ static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
         case 4:
             *data = bswap32(*data);
             break;
+        case 8:
+            *data = bswap64(*data);
+            break;
         default:
             abort();
         }
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 20/22] exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (18 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 19/22] memory: correctly handle endian-swapped 64-bit accesses Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 21/22] memory: propagate errors on I/O dispatch Paolo Bonzini
                   ` (2 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The memory API is able to split it in two 4-byte accesses.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                          |  8 +-------
 include/exec/softmmu_template.h | 24 +-----------------------
 2 files changed, 2 insertions(+), 30 deletions(-)

diff --git a/exec.c b/exec.c
index 42f7636..3068077 100644
--- a/exec.c
+++ b/exec.c
@@ -2260,13 +2260,7 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
                                       false);
     if (l < 8 || !memory_access_is_direct(section->mr, false)) {
         /* I/O case */
-#ifdef TARGET_WORDS_BIGENDIAN
-        val = io_mem_read(section->mr, addr1, 4) << 32;
-        val |= io_mem_read(section->mr, addr1 + 4, 4);
-#else
-        val = io_mem_read(section->mr, addr1, 4);
-        val |= io_mem_read(section->mr, addr1 + 4, 4) << 32;
-#endif
+        val = io_mem_read(section->mr, addr1, 8);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap64(val);
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index ca91fd0..292ca02 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -63,7 +63,6 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
                                               target_ulong addr,
                                               uintptr_t retaddr)
 {
-    DATA_TYPE res;
     MemoryRegion *mr = iotlb_to_region(physaddr);
 
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
@@ -73,18 +72,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
     }
 
     env->mem_io_vaddr = addr;
-#if SHIFT <= 2
-    res = io_mem_read(mr, physaddr, 1 << SHIFT);
-#else
-#ifdef TARGET_WORDS_BIGENDIAN
-    res = io_mem_read(mr, physaddr, 4) << 32;
-    res |= io_mem_read(mr, physaddr + 4, 4);
-#else
-    res = io_mem_read(mr, physaddr, 4);
-    res |= io_mem_read(mr, physaddr + 4, 4) << 32;
-#endif
-#endif /* SHIFT > 2 */
-    return res;
+    return io_mem_read(mr, physaddr, 1 << SHIFT);
 }
 
 /* handle all cases except unaligned access which span two pages */
@@ -221,17 +209,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env,
 
     env->mem_io_vaddr = addr;
     env->mem_io_pc = retaddr;
-#if SHIFT <= 2
     io_mem_write(mr, physaddr, val, 1 << SHIFT);
-#else
-#ifdef TARGET_WORDS_BIGENDIAN
-    io_mem_write(mr, physaddr, (val >> 32), 4);
-    io_mem_write(mr, physaddr + 4, (uint32_t)val, 4);
-#else
-    io_mem_write(mr, physaddr, (uint32_t)val, 4);
-    io_mem_write(mr, physaddr + 4, val >> 32, 4);
-#endif
-#endif /* SHIFT > 2 */
 }
 
 void glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 21/22] memory: propagate errors on I/O dispatch
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (19 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 20/22] exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 22/22] memory: add return value to address_space_rw/read/write Paolo Bonzini
  2013-05-24 22:06 ` [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Richard Henderson
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                          | 21 ++++++++++++---------
 include/exec/exec-all.h         |  6 +++---
 include/exec/softmmu_template.h |  4 +++-
 memory.c                        | 35 ++++++++++++++++++-----------------
 4 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/exec.c b/exec.c
index 3068077..28dcee9 100644
--- a/exec.c
+++ b/exec.c
@@ -1523,6 +1523,8 @@ static uint64_t subpage_read(void *opaque, hwaddr addr,
 {
     subpage_t *mmio = opaque;
     unsigned int idx = SUBPAGE_IDX(addr);
+    uint64_t val;
+
     MemoryRegionSection *section;
 #if defined(DEBUG_SUBPAGE)
     printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d\n", __func__,
@@ -1533,7 +1535,8 @@ static uint64_t subpage_read(void *opaque, hwaddr addr,
     addr += mmio->base;
     addr -= section->offset_within_address_space;
     addr += section->offset_within_region;
-    return io_mem_read(section->mr, addr, len);
+    io_mem_read(section->mr, addr, &val, len);
+    return val;
 }
 
 static void subpage_write(void *opaque, hwaddr addr,
@@ -1901,7 +1904,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
 {
     hwaddr l;
     uint8_t *ptr;
-    uint32_t val;
+    uint64_t val;
     hwaddr addr1;
     MemoryRegionSection *section;
 
@@ -1940,15 +1943,15 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                 l = memory_access_size(l, addr1);
                 if (l == 4) {
                     /* 32 bit read access */
-                    val = io_mem_read(section->mr, addr1, 4);
+                    io_mem_read(section->mr, addr1, &val, 4);
                     stl_p(buf, val);
                 } else if (l == 2) {
                     /* 16 bit read access */
-                    val = io_mem_read(section->mr, addr1, 2);
+                    io_mem_read(section->mr, addr1, &val, 2);
                     stw_p(buf, val);
                 } else {
                     /* 8 bit read access */
-                    val = io_mem_read(section->mr, addr1, 1);
+                    io_mem_read(section->mr, addr1, &val, 1);
                     stb_p(buf, val);
                 }
             } else {
@@ -2192,7 +2195,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
                                          enum device_endian endian)
 {
     uint8_t *ptr;
-    uint32_t val;
+    uint64_t val;
     MemoryRegionSection *section;
     hwaddr l = 4;
     hwaddr addr1;
@@ -2201,7 +2204,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr,
                                       false);
     if (l < 4 || !memory_access_is_direct(section->mr, false)) {
         /* I/O case */
-        val = io_mem_read(section->mr, addr1, 4);
+        io_mem_read(section->mr, addr1, &val, 4);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap32(val);
@@ -2260,7 +2263,7 @@ static inline uint64_t ldq_phys_internal(hwaddr addr,
                                       false);
     if (l < 8 || !memory_access_is_direct(section->mr, false)) {
         /* I/O case */
-        val = io_mem_read(section->mr, addr1, 8);
+        io_mem_read(section->mr, addr1, &val, 8);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap64(val);
@@ -2327,7 +2330,7 @@ static inline uint32_t lduw_phys_internal(hwaddr addr,
                                       false);
     if (l < 2 || !memory_access_is_direct(section->mr, false)) {
         /* I/O case */
-        val = io_mem_read(section->mr, addr1, 2);
+        io_mem_read(section->mr, addr1, &val, 2);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap16(val);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 6362074..17fde25 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -367,9 +367,9 @@ bool is_tcg_gen_code(uintptr_t pc_ptr);
 #if !defined(CONFIG_USER_ONLY)
 
 struct MemoryRegion *iotlb_to_region(hwaddr index);
-uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
-                     unsigned size);
-void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
+bool io_mem_read(struct MemoryRegion *mr, hwaddr addr,
+                 uint64_t *pvalue, unsigned size);
+bool io_mem_write(struct MemoryRegion *mr, hwaddr addr,
                   uint64_t value, unsigned size);
 
 void tlb_fill(CPUArchState *env1, target_ulong addr, int is_write, int mmu_idx,
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index 292ca02..8584902 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -63,6 +63,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
                                               target_ulong addr,
                                               uintptr_t retaddr)
 {
+    uint64_t val;
     MemoryRegion *mr = iotlb_to_region(physaddr);
 
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
@@ -72,7 +73,8 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
     }
 
     env->mem_io_vaddr = addr;
-    return io_mem_read(mr, physaddr, 1 << SHIFT);
+    io_mem_read(mr, physaddr, &val, 1 << SHIFT);
+    return val;
 }
 
 /* handle all cases except unaligned access which span two pages */
diff --git a/memory.c b/memory.c
index c9833a3..5d3b7c3 100644
--- a/memory.c
+++ b/memory.c
@@ -928,10 +928,6 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
 {
     uint64_t data = 0;
 
-    if (!memory_region_access_valid(mr, addr, size, false)) {
-        return unassigned_mem_read(mr, addr, size);
-    }
-
     if (mr->ops->read) {
         access_with_adjusted_size(addr, &data, size,
                                   mr->ops->impl.min_access_size,
@@ -966,25 +962,29 @@ static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
     }
 }
 
-static uint64_t memory_region_dispatch_read(MemoryRegion *mr,
-                                            hwaddr addr,
-                                            unsigned size)
+static bool memory_region_dispatch_read(MemoryRegion *mr,
+                                        hwaddr addr,
+                                        uint64_t *pval,
+                                        unsigned size)
 {
-    uint64_t ret;
+    if (!memory_region_access_valid(mr, addr, size, false)) {
+        *pval = unassigned_mem_read(mr, addr, size);
+        return true;
+    }
 
-    ret = memory_region_dispatch_read1(mr, addr, size);
-    adjust_endianness(mr, &ret, size);
-    return ret;
+    *pval = memory_region_dispatch_read1(mr, addr, size);
+    adjust_endianness(mr, pval, size);
+    return false;
 }
 
-static void memory_region_dispatch_write(MemoryRegion *mr,
+static bool memory_region_dispatch_write(MemoryRegion *mr,
                                          hwaddr addr,
                                          uint64_t data,
                                          unsigned size)
 {
     if (!memory_region_access_valid(mr, addr, size, true)) {
         unassigned_mem_write(mr, addr, data, size);
-        return;
+        return true;
     }
 
     adjust_endianness(mr, &data, size);
@@ -998,6 +998,7 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
         access_with_adjusted_size(addr, &data, size, 1, 4,
                                   memory_region_oldmmio_write_accessor, mr);
     }
+    return false;
 }
 
 void memory_region_init_io(MemoryRegion *mr,
@@ -1650,15 +1651,15 @@ void address_space_destroy(AddressSpace *as)
     g_free(as->ioeventfds);
 }
 
-uint64_t io_mem_read(MemoryRegion *mr, hwaddr addr, unsigned size)
+bool io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, unsigned size)
 {
-    return memory_region_dispatch_read(mr, addr, size);
+    return memory_region_dispatch_read(mr, addr, pval, size);
 }
 
-void io_mem_write(MemoryRegion *mr, hwaddr addr,
+bool io_mem_write(MemoryRegion *mr, hwaddr addr,
                   uint64_t val, unsigned size)
 {
-    memory_region_dispatch_write(mr, addr, val, size);
+    return memory_region_dispatch_write(mr, addr, val, size);
 }
 
 typedef struct MemoryRegionList MemoryRegionList;
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH 22/22] memory: add return value to address_space_rw/read/write
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (20 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 21/22] memory: propagate errors on I/O dispatch Paolo Bonzini
@ 2013-05-24 17:05 ` Paolo Bonzini
  2013-05-24 22:06 ` [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Richard Henderson
  22 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2013-05-24 17:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                | 34 +++++++++++++++-------------------
 include/exec/memory.h | 12 +++++++++---
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/exec.c b/exec.c
index 28dcee9..07bc4f9 100644
--- a/exec.c
+++ b/exec.c
@@ -1899,7 +1899,7 @@ static inline int memory_access_size(int l, hwaddr addr)
     return 1;
 }
 
-void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
+bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                       int len, bool is_write)
 {
     hwaddr l;
@@ -1907,6 +1907,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
     uint64_t val;
     hwaddr addr1;
     MemoryRegionSection *section;
+    bool error = false;
 
     while (len > 0) {
         l = len;
@@ -1920,15 +1921,15 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                 if (l == 4) {
                     /* 32 bit write access */
                     val = ldl_p(buf);
-                    io_mem_write(section->mr, addr1, val, 4);
+                    error |= io_mem_write(section->mr, addr1, val, 4);
                 } else if (l == 2) {
                     /* 16 bit write access */
                     val = lduw_p(buf);
-                    io_mem_write(section->mr, addr1, val, 2);
+                    error |= io_mem_write(section->mr, addr1, val, 2);
                 } else {
                     /* 8 bit write access */
                     val = ldub_p(buf);
-                    io_mem_write(section->mr, addr1, val, 1);
+                    error |= io_mem_write(section->mr, addr1, val, 1);
                 }
             } else {
                 addr1 += memory_region_get_ram_addr(section->mr);
@@ -1943,15 +1944,15 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                 l = memory_access_size(l, addr1);
                 if (l == 4) {
                     /* 32 bit read access */
-                    io_mem_read(section->mr, addr1, &val, 4);
+                    error |= io_mem_read(section->mr, addr1, &val, 4);
                     stl_p(buf, val);
                 } else if (l == 2) {
                     /* 16 bit read access */
-                    io_mem_read(section->mr, addr1, &val, 2);
+                    error |= io_mem_read(section->mr, addr1, &val, 2);
                     stw_p(buf, val);
                 } else {
                     /* 8 bit read access */
-                    io_mem_read(section->mr, addr1, &val, 1);
+                    error |= io_mem_read(section->mr, addr1, &val, 1);
                     stb_p(buf, val);
                 }
             } else {
@@ -1964,31 +1965,26 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
         buf += l;
         addr += l;
     }
+
+    return error;
 }
 
-void address_space_write(AddressSpace *as, hwaddr addr,
+bool address_space_write(AddressSpace *as, hwaddr addr,
                          const uint8_t *buf, int len)
 {
-    address_space_rw(as, addr, (uint8_t *)buf, len, true);
+    return address_space_rw(as, addr, (uint8_t *)buf, len, true);
 }
 
-/**
- * address_space_read: read from an address space.
- *
- * @as: #AddressSpace to be accessed
- * @addr: address within that address space
- * @buf: buffer with the data transferred
- */
-void address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len)
+bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len)
 {
-    address_space_rw(as, addr, buf, len, false);
+    return address_space_rw(as, addr, buf, len, false);
 }
 
 
 void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
                             int len, int is_write)
 {
-    return address_space_rw(&address_space_memory, addr, buf, len, is_write);
+    address_space_rw(&address_space_memory, addr, buf, len, is_write);
 }
 
 /* used for ROM loading : can write in RAM and ROM */
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 81e0e41..d53a6a1 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -825,32 +825,38 @@ void address_space_destroy(AddressSpace *as);
 /**
  * address_space_rw: read from or write to an address space.
  *
+ * Return true if the operation hit any unassigned memory.
+ *
  * @as: #AddressSpace to be accessed
  * @addr: address within that address space
  * @buf: buffer with the data transferred
  * @is_write: indicates the transfer direction
  */
-void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
+bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
                       int len, bool is_write);
 
 /**
  * address_space_write: write to address space.
  *
+ * Return true if the operation hit any unassigned memory.
+ *
  * @as: #AddressSpace to be accessed
  * @addr: address within that address space
  * @buf: buffer with the data transferred
  */
-void address_space_write(AddressSpace *as, hwaddr addr,
+bool address_space_write(AddressSpace *as, hwaddr addr,
                          const uint8_t *buf, int len);
 
 /**
  * address_space_read: read from an address space.
  *
+ * Return true if the operation hit any unassigned memory.
+ *
  * @as: #AddressSpace to be accessed
  * @addr: address within that address space
  * @buf: buffer with the data transferred
  */
-void address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len);
+bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len);
 
 /* address_space_translate: translate an address range into an address space
  * into a MemoryRegionSection and an address range into that section
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection
  2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
                   ` (21 preceding siblings ...)
  2013-05-24 17:05 ` [Qemu-devel] [PATCH 22/22] memory: add return value to address_space_rw/read/write Paolo Bonzini
@ 2013-05-24 22:06 ` Richard Henderson
  22 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2013-05-24 22:06 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: peter.maydell, qemu-devel

On 2013-05-24 10:05, Paolo Bonzini wrote:
> Paolo Bonzini (22):
>    exec: eliminate io_mem_ram
>    exec: drop useless #if
>    cputlb: simplify tlb_set_page
>    exec: make io_mem_unassigned private
>    exec: do not use error_mem_read
>    memory: dispatch unassigned accesses based on .valid.accepts
>    memory: add address_space_translate
>    memory: move unassigned_mem_ops to memory.c
>    memory: assign MemoryRegionOps to all regions
>    exec: expect mr->ops to be initialized for ROM
>    exec: introduce memory_access_is_direct
>    exec: introduce memory_access_size
>    memory: export memory_region_access_valid to exec.c
>    exec: implement .valid.accepts for subpages
>    memory: add address_space_access_valid
>    memory: accept mismatching sizes in memory_region_access_valid
>    memory: add big endian support to access_with_adjusted_size
>    memory: split accesses even when the old MMIO callbacks are used
>    memory: correctly handle endian-swapped 64-bit accesses
>    exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses
>    memory: propagate errors on I/O dispatch
>    memory: add return value to address_space_rw/read/write

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2013-05-24 22:06 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-24 17:05 [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 01/22] exec: eliminate io_mem_ram Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 02/22] exec: drop useless #if Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 03/22] cputlb: simplify tlb_set_page Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 04/22] exec: make io_mem_unassigned private Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 05/22] exec: do not use error_mem_read Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 07/22] memory: add address_space_translate Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 08/22] memory: move unassigned_mem_ops to memory.c Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 09/22] memory: assign MemoryRegionOps to all regions Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 10/22] exec: expect mr->ops to be initialized for ROM Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 11/22] exec: introduce memory_access_is_direct Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 12/22] exec: introduce memory_access_size Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 13/22] memory: export memory_region_access_valid to exec.c Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 14/22] exec: implement .valid.accepts for subpages Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 15/22] memory: add address_space_access_valid Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 16/22] memory: accept mismatching sizes in memory_region_access_valid Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 17/22] memory: add big endian support to access_with_adjusted_size Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 18/22] memory: split accesses even when the old MMIO callbacks are used Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 19/22] memory: correctly handle endian-swapped 64-bit accesses Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 20/22] exec: just use io_mem_read/io_mem_write for 8-byte I/O accesses Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 21/22] memory: propagate errors on I/O dispatch Paolo Bonzini
2013-05-24 17:05 ` [Qemu-devel] [PATCH 22/22] memory: add return value to address_space_rw/read/write Paolo Bonzini
2013-05-24 22:06 ` [Qemu-devel] [PATCH 00/22] Memory/IOMMU patches, part 2: unassigned access detection Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.