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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).