From: "Alex Bennée" <alex.bennee@linaro.org>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com,
david@redhat.com
Subject: Re: [PATCH v4 09/16] cputlb: Move NOTDIRTY handling from I/O path to TLB path
Date: Wed, 25 Sep 2019 17:06:08 +0100 [thread overview]
Message-ID: <87sgok9x8v.fsf@linaro.org> (raw)
In-Reply-To: <20190923230004.9231-10-richard.henderson@linaro.org>
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, ¬dirty_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
next prev parent reply other threads:[~2019-09-25 16:10 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-23 22:59 [PATCH v4 00/16] Move rom and notdirty handling to cputlb Richard Henderson
2019-09-23 22:59 ` [PATCH v4 01/16] exec: Use TARGET_PAGE_BITS_MIN for TLB flags Richard Henderson
2019-09-24 13:53 ` Alex Bennée
2019-09-23 22:59 ` [PATCH v4 02/16] cputlb: Disable __always_inline__ without optimization Richard Henderson
2019-09-24 13:56 ` Alex Bennée
2019-09-23 22:59 ` [PATCH v4 03/16] qemu/compiler.h: Add optimize_away Richard Henderson
2019-09-24 7:47 ` David Hildenbrand
2019-09-24 17:27 ` Richard Henderson
2019-09-24 17:29 ` David Hildenbrand
2019-09-24 15:47 ` Alex Bennée
2019-09-23 22:59 ` [PATCH v4 04/16] cputlb: Use optimize_away in load/store_helpers Richard Henderson
2019-09-24 7:47 ` David Hildenbrand
2019-09-24 15:47 ` Alex Bennée
2019-09-23 22:59 ` [PATCH v4 05/16] cputlb: Split out load/store_memop Richard Henderson
2019-09-24 7:48 ` David Hildenbrand
2019-09-24 15:51 ` Alex Bennée
2019-09-23 22:59 ` [PATCH v4 06/16] cputlb: Introduce TLB_BSWAP Richard Henderson
2019-09-24 18:25 ` Alex Bennée
2019-09-25 17:36 ` Richard Henderson
2019-09-23 22:59 ` [PATCH v4 07/16] exec: Adjust notdirty tracing Richard Henderson
2019-09-24 21:53 ` Alex Bennée
2019-09-23 22:59 ` [PATCH v4 08/16] cputlb: Move ROM handling from I/O path to TLB path Richard Henderson
2019-09-25 0:16 ` Alex Bennée
2019-09-25 6:59 ` David Hildenbrand
2019-09-25 16:01 ` Alex Bennée
2019-09-25 17:01 ` Richard Henderson
2019-09-23 22:59 ` [PATCH v4 09/16] cputlb: Move NOTDIRTY " Richard Henderson
2019-09-25 16:06 ` Alex Bennée [this message]
2019-09-23 22:59 ` [PATCH v4 10/16] cputlb: Partially inline memory_region_section_get_iotlb Richard Henderson
2019-09-24 7:59 ` David Hildenbrand
2019-09-25 17:55 ` Richard Henderson
2019-09-25 19:40 ` David Hildenbrand
2019-09-25 16:12 ` Alex Bennée
2019-09-23 22:59 ` [PATCH v4 11/16] cputlb: Merge and move memory_notdirty_write_{prepare, complete} Richard Henderson
2019-09-24 8:04 ` [PATCH v4 11/16] cputlb: Merge and move memory_notdirty_write_{prepare,complete} David Hildenbrand
2019-09-25 16:15 ` Alex Bennée
2019-09-23 23:00 ` [PATCH v4 12/16] cputlb: Handle TLB_NOTDIRTY in probe_access Richard Henderson
2019-09-24 8:05 ` David Hildenbrand
2019-09-25 16:21 ` Alex Bennée
2019-09-23 23:00 ` [PATCH v4 13/16] cputlb: Remove cpu->mem_io_vaddr Richard Henderson
2019-09-25 16:22 ` Alex Bennée
2019-09-23 23:00 ` [PATCH v4 14/16] cputlb: Remove tb_invalidate_phys_page_range is_cpu_write_access Richard Henderson
2019-09-25 16:23 ` Alex Bennée
2019-09-23 23:00 ` [PATCH v4 15/16] cputlb: Pass retaddr to tb_invalidate_phys_page_fast Richard Henderson
2019-09-25 16:28 ` Alex Bennée
2019-09-23 23:00 ` [PATCH v4 16/16] cputlb: Pass retaddr to tb_check_watchpoint Richard Henderson
2019-09-25 16:30 ` Alex Bennée
2019-09-25 18:52 ` [PATCH v4 00/16] Move rom and notdirty handling to cputlb Mark Cave-Ayland
2019-09-25 18:54 ` Mark Cave-Ayland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87sgok9x8v.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=david@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.