diff mbox series

[v4,09/16] cputlb: Move NOTDIRTY handling from I/O path to TLB path

Message ID 20190923230004.9231-10-richard.henderson@linaro.org
State Superseded
Headers show
Series Move rom and notdirty handling to cputlb | expand

Commit Message

Richard Henderson Sept. 23, 2019, 10:59 p.m. UTC
Pages that we want to track for NOTDIRTY are RAM.  We do not
really need to go through the I/O path to handle them.

Acked-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 include/exec/cpu-common.h |  2 --
 accel/tcg/cputlb.c        | 26 +++++++++++++++++---
 exec.c                    | 50 ---------------------------------------
 memory.c                  | 16 -------------
 4 files changed, 23 insertions(+), 71 deletions(-)

-- 
2.17.1

Comments

Alex Bennée Sept. 25, 2019, 4:06 p.m. UTC | #1
Richard Henderson <richard.henderson@linaro.org> writes:

> Pages that we want to track for NOTDIRTY are RAM.  We do not

> really need to go through the I/O path to handle them.

>

> Acked-by: David Hildenbrand <david@redhat.com>

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  include/exec/cpu-common.h |  2 --

>  accel/tcg/cputlb.c        | 26 +++++++++++++++++---

>  exec.c                    | 50 ---------------------------------------

>  memory.c                  | 16 -------------

>  4 files changed, 23 insertions(+), 71 deletions(-)


Nice clean-up ;)

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>



>

> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h

> index 1c0e03ddc2..81753bbb34 100644

> --- a/include/exec/cpu-common.h

> +++ b/include/exec/cpu-common.h

> @@ -100,8 +100,6 @@ void qemu_flush_coalesced_mmio_buffer(void);

>

>  void cpu_flush_icache_range(hwaddr start, hwaddr len);

>

> -extern struct MemoryRegion io_mem_notdirty;

> -

>  typedef int (RAMBlockIterFunc)(RAMBlock *rb, void *opaque);

>

>  int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque);

> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c

> index af9a44a847..05212ff244 100644

> --- a/accel/tcg/cputlb.c

> +++ b/accel/tcg/cputlb.c

> @@ -904,7 +904,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>      mr = section->mr;

>      mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;

>      cpu->mem_io_pc = retaddr;

> -    if (mr != &io_mem_notdirty && !cpu->can_do_io) {

> +    if (!cpu->can_do_io) {

>          cpu_io_recompile(cpu, retaddr);

>      }

>

> @@ -945,7 +945,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>      section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs);

>      mr = section->mr;

>      mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;

> -    if (mr != &io_mem_notdirty && !cpu->can_do_io) {

> +    if (!cpu->can_do_io) {

>          cpu_io_recompile(cpu, retaddr);

>      }

>      cpu->mem_io_vaddr = addr;

> @@ -1607,7 +1607,7 @@ store_helper(CPUArchState *env, target_ulong addr, uint64_t val,

>          need_swap = size > 1 && (tlb_addr & TLB_BSWAP);

>

>          /* Handle I/O access.  */

> -        if (likely(tlb_addr & (TLB_MMIO | TLB_NOTDIRTY))) {

> +        if (tlb_addr & TLB_MMIO) {

>              io_writex(env, iotlbentry, mmu_idx, val, addr, retaddr,

>                        op ^ (need_swap * MO_BSWAP));

>              return;

> @@ -1620,6 +1620,26 @@ store_helper(CPUArchState *env, target_ulong addr, uint64_t val,

>

>          haddr = (void *)((uintptr_t)addr + entry->addend);

>

> +        /* Handle clean RAM pages.  */

> +        if (tlb_addr & TLB_NOTDIRTY) {

> +            NotDirtyInfo ndi;

> +

> +            /* We require mem_io_pc in tb_invalidate_phys_page_range.  */

> +            env_cpu(env)->mem_io_pc = retaddr;

> +

> +            memory_notdirty_write_prepare(&ndi, env_cpu(env), addr,

> +                                          addr + iotlbentry->addr, size);

> +

> +            if (unlikely(need_swap)) {

> +                store_memop(haddr, val, op ^ MO_BSWAP);

> +            } else {

> +                store_memop(haddr, val, op);

> +            }

> +

> +            memory_notdirty_write_complete(&ndi);

> +            return;

> +        }

> +

>          if (unlikely(need_swap)) {

>              store_memop(haddr, val, op ^ MO_BSWAP);

>          } else {

> diff --git a/exec.c b/exec.c

> index ea8c0b18ac..dc7001f115 100644

> --- a/exec.c

> +++ b/exec.c

> @@ -88,7 +88,6 @@ static MemoryRegion *system_io;

>  AddressSpace address_space_io;

>  AddressSpace address_space_memory;

>

> -MemoryRegion io_mem_notdirty;

>  static MemoryRegion io_mem_unassigned;

>  #endif

>

> @@ -191,7 +190,6 @@ typedef struct subpage_t {

>  } subpage_t;

>

>  #define PHYS_SECTION_UNASSIGNED 0

> -#define PHYS_SECTION_NOTDIRTY 1

>

>  static void io_mem_init(void);

>  static void memory_map_init(void);

> @@ -1472,9 +1470,6 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,

>      if (memory_region_is_ram(section->mr)) {

>          /* Normal RAM.  */

>          iotlb = memory_region_get_ram_addr(section->mr) + xlat;

> -        if (!section->readonly) {

> -            iotlb |= PHYS_SECTION_NOTDIRTY;

> -        }

>      } else {

>          AddressSpaceDispatch *d;

>

> @@ -2783,42 +2778,6 @@ void memory_notdirty_write_complete(NotDirtyInfo *ndi)

>      }

>  }

>

> -/* Called within RCU critical section.  */

> -static void notdirty_mem_write(void *opaque, hwaddr ram_addr,

> -                               uint64_t val, unsigned size)

> -{

> -    NotDirtyInfo ndi;

> -

> -    memory_notdirty_write_prepare(&ndi, current_cpu, current_cpu->mem_io_vaddr,

> -                         ram_addr, size);

> -

> -    stn_p(qemu_map_ram_ptr(NULL, ram_addr), size, val);

> -    memory_notdirty_write_complete(&ndi);

> -}

> -

> -static bool notdirty_mem_accepts(void *opaque, hwaddr addr,

> -                                 unsigned size, bool is_write,

> -                                 MemTxAttrs attrs)

> -{

> -    return is_write;

> -}

> -

> -static const MemoryRegionOps notdirty_mem_ops = {

> -    .write = notdirty_mem_write,

> -    .valid.accepts = notdirty_mem_accepts,

> -    .endianness = DEVICE_NATIVE_ENDIAN,

> -    .valid = {

> -        .min_access_size = 1,

> -        .max_access_size = 8,

> -        .unaligned = false,

> -    },

> -    .impl = {

> -        .min_access_size = 1,

> -        .max_access_size = 8,

> -        .unaligned = false,

> -    },

> -};

> -

>  /* Generate a debug exception if a watchpoint has been hit.  */

>  void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,

>                            MemTxAttrs attrs, int flags, uintptr_t ra)

> @@ -3014,13 +2973,6 @@ static void io_mem_init(void)

>  {

>      memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,

>                            NULL, UINT64_MAX);

> -

> -    /* io_mem_notdirty calls tb_invalidate_phys_page_fast,

> -     * which can be called without the iothread mutex.

> -     */

> -    memory_region_init_io(&io_mem_notdirty, NULL, &notdirty_mem_ops, NULL,

> -                          NULL, UINT64_MAX);

> -    memory_region_clear_global_locking(&io_mem_notdirty);

>  }

>

>  AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv)

> @@ -3030,8 +2982,6 @@ AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv)

>

>      n = dummy_section(&d->map, fv, &io_mem_unassigned);

>      assert(n == PHYS_SECTION_UNASSIGNED);

> -    n = dummy_section(&d->map, fv, &io_mem_notdirty);

> -    assert(n == PHYS_SECTION_NOTDIRTY);

>

>      d->phys_map  = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };

>

> diff --git a/memory.c b/memory.c

> index 57c44c97db..a99b8c0767 100644

> --- a/memory.c

> +++ b/memory.c

> @@ -434,10 +434,6 @@ static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,

>      tmp = mr->ops->read(mr->opaque, addr, size);

>      if (mr->subpage) {

>          trace_memory_region_subpage_read(get_cpu_index(), mr, addr, tmp, size);

> -    } else if (mr == &io_mem_notdirty) {

> -        /* Accesses to code which has previously been translated into a TB show

> -         * up in the MMIO path, as accesses to the io_mem_notdirty

> -         * MemoryRegion. */

>      } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {

>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);

>          trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);

> @@ -460,10 +456,6 @@ static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,

>      r = mr->ops->read_with_attrs(mr->opaque, addr, &tmp, size, attrs);

>      if (mr->subpage) {

>          trace_memory_region_subpage_read(get_cpu_index(), mr, addr, tmp, size);

> -    } else if (mr == &io_mem_notdirty) {

> -        /* Accesses to code which has previously been translated into a TB show

> -         * up in the MMIO path, as accesses to the io_mem_notdirty

> -         * MemoryRegion. */

>      } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {

>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);

>          trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);

> @@ -484,10 +476,6 @@ static MemTxResult memory_region_write_accessor(MemoryRegion *mr,

>

>      if (mr->subpage) {

>          trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);

> -    } else if (mr == &io_mem_notdirty) {

> -        /* Accesses to code which has previously been translated into a TB show

> -         * up in the MMIO path, as accesses to the io_mem_notdirty

> -         * MemoryRegion. */

>      } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {

>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);

>          trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);

> @@ -508,10 +496,6 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,

>

>      if (mr->subpage) {

>          trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);

> -    } else if (mr == &io_mem_notdirty) {

> -        /* Accesses to code which has previously been translated into a TB show

> -         * up in the MMIO path, as accesses to the io_mem_notdirty

> -         * MemoryRegion. */

>      } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {

>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);

>          trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);



--
Alex Bennée
diff mbox series

Patch

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 1c0e03ddc2..81753bbb34 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -100,8 +100,6 @@  void qemu_flush_coalesced_mmio_buffer(void);
 
 void cpu_flush_icache_range(hwaddr start, hwaddr len);
 
-extern struct MemoryRegion io_mem_notdirty;
-
 typedef int (RAMBlockIterFunc)(RAMBlock *rb, void *opaque);
 
 int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque);
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index af9a44a847..05212ff244 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -904,7 +904,7 @@  static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
     mr = section->mr;
     mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;
     cpu->mem_io_pc = retaddr;
-    if (mr != &io_mem_notdirty && !cpu->can_do_io) {
+    if (!cpu->can_do_io) {
         cpu_io_recompile(cpu, retaddr);
     }
 
@@ -945,7 +945,7 @@  static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
     section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs);
     mr = section->mr;
     mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;
-    if (mr != &io_mem_notdirty && !cpu->can_do_io) {
+    if (!cpu->can_do_io) {
         cpu_io_recompile(cpu, retaddr);
     }
     cpu->mem_io_vaddr = addr;
@@ -1607,7 +1607,7 @@  store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
         need_swap = size > 1 && (tlb_addr & TLB_BSWAP);
 
         /* Handle I/O access.  */
-        if (likely(tlb_addr & (TLB_MMIO | TLB_NOTDIRTY))) {
+        if (tlb_addr & TLB_MMIO) {
             io_writex(env, iotlbentry, mmu_idx, val, addr, retaddr,
                       op ^ (need_swap * MO_BSWAP));
             return;
@@ -1620,6 +1620,26 @@  store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
 
         haddr = (void *)((uintptr_t)addr + entry->addend);
 
+        /* Handle clean RAM pages.  */
+        if (tlb_addr & TLB_NOTDIRTY) {
+            NotDirtyInfo ndi;
+
+            /* We require mem_io_pc in tb_invalidate_phys_page_range.  */
+            env_cpu(env)->mem_io_pc = retaddr;
+
+            memory_notdirty_write_prepare(&ndi, env_cpu(env), addr,
+                                          addr + iotlbentry->addr, size);
+
+            if (unlikely(need_swap)) {
+                store_memop(haddr, val, op ^ MO_BSWAP);
+            } else {
+                store_memop(haddr, val, op);
+            }
+
+            memory_notdirty_write_complete(&ndi);
+            return;
+        }
+
         if (unlikely(need_swap)) {
             store_memop(haddr, val, op ^ MO_BSWAP);
         } else {
diff --git a/exec.c b/exec.c
index ea8c0b18ac..dc7001f115 100644
--- a/exec.c
+++ b/exec.c
@@ -88,7 +88,6 @@  static MemoryRegion *system_io;
 AddressSpace address_space_io;
 AddressSpace address_space_memory;
 
-MemoryRegion io_mem_notdirty;
 static MemoryRegion io_mem_unassigned;
 #endif
 
@@ -191,7 +190,6 @@  typedef struct subpage_t {
 } subpage_t;
 
 #define PHYS_SECTION_UNASSIGNED 0
-#define PHYS_SECTION_NOTDIRTY 1
 
 static void io_mem_init(void);
 static void memory_map_init(void);
@@ -1472,9 +1470,6 @@  hwaddr memory_region_section_get_iotlb(CPUState *cpu,
     if (memory_region_is_ram(section->mr)) {
         /* Normal RAM.  */
         iotlb = memory_region_get_ram_addr(section->mr) + xlat;
-        if (!section->readonly) {
-            iotlb |= PHYS_SECTION_NOTDIRTY;
-        }
     } else {
         AddressSpaceDispatch *d;
 
@@ -2783,42 +2778,6 @@  void memory_notdirty_write_complete(NotDirtyInfo *ndi)
     }
 }
 
-/* Called within RCU critical section.  */
-static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
-                               uint64_t val, unsigned size)
-{
-    NotDirtyInfo ndi;
-
-    memory_notdirty_write_prepare(&ndi, current_cpu, current_cpu->mem_io_vaddr,
-                         ram_addr, size);
-
-    stn_p(qemu_map_ram_ptr(NULL, ram_addr), size, val);
-    memory_notdirty_write_complete(&ndi);
-}
-
-static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
-                                 unsigned size, bool is_write,
-                                 MemTxAttrs attrs)
-{
-    return is_write;
-}
-
-static const MemoryRegionOps notdirty_mem_ops = {
-    .write = notdirty_mem_write,
-    .valid.accepts = notdirty_mem_accepts,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-    .valid = {
-        .min_access_size = 1,
-        .max_access_size = 8,
-        .unaligned = false,
-    },
-    .impl = {
-        .min_access_size = 1,
-        .max_access_size = 8,
-        .unaligned = false,
-    },
-};
-
 /* Generate a debug exception if a watchpoint has been hit.  */
 void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
                           MemTxAttrs attrs, int flags, uintptr_t ra)
@@ -3014,13 +2973,6 @@  static void io_mem_init(void)
 {
     memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,
                           NULL, UINT64_MAX);
-
-    /* io_mem_notdirty calls tb_invalidate_phys_page_fast,
-     * which can be called without the iothread mutex.
-     */
-    memory_region_init_io(&io_mem_notdirty, NULL, &notdirty_mem_ops, NULL,
-                          NULL, UINT64_MAX);
-    memory_region_clear_global_locking(&io_mem_notdirty);
 }
 
 AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv)
@@ -3030,8 +2982,6 @@  AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv)
 
     n = dummy_section(&d->map, fv, &io_mem_unassigned);
     assert(n == PHYS_SECTION_UNASSIGNED);
-    n = dummy_section(&d->map, fv, &io_mem_notdirty);
-    assert(n == PHYS_SECTION_NOTDIRTY);
 
     d->phys_map  = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
 
diff --git a/memory.c b/memory.c
index 57c44c97db..a99b8c0767 100644
--- a/memory.c
+++ b/memory.c
@@ -434,10 +434,6 @@  static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,
     tmp = mr->ops->read(mr->opaque, addr, size);
     if (mr->subpage) {
         trace_memory_region_subpage_read(get_cpu_index(), mr, addr, tmp, size);
-    } else if (mr == &io_mem_notdirty) {
-        /* Accesses to code which has previously been translated into a TB show
-         * up in the MMIO path, as accesses to the io_mem_notdirty
-         * MemoryRegion. */
     } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
@@ -460,10 +456,6 @@  static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,
     r = mr->ops->read_with_attrs(mr->opaque, addr, &tmp, size, attrs);
     if (mr->subpage) {
         trace_memory_region_subpage_read(get_cpu_index(), mr, addr, tmp, size);
-    } else if (mr == &io_mem_notdirty) {
-        /* Accesses to code which has previously been translated into a TB show
-         * up in the MMIO path, as accesses to the io_mem_notdirty
-         * MemoryRegion. */
     } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
@@ -484,10 +476,6 @@  static MemTxResult memory_region_write_accessor(MemoryRegion *mr,
 
     if (mr->subpage) {
         trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
-    } else if (mr == &io_mem_notdirty) {
-        /* Accesses to code which has previously been translated into a TB show
-         * up in the MMIO path, as accesses to the io_mem_notdirty
-         * MemoryRegion. */
     } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);
@@ -508,10 +496,6 @@  static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
 
     if (mr->subpage) {
         trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
-    } else if (mr == &io_mem_notdirty) {
-        /* Accesses to code which has previously been translated into a TB show
-         * up in the MMIO path, as accesses to the io_mem_notdirty
-         * MemoryRegion. */
     } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);