From: David Hildenbrand <david@redhat.com>
To: Richard Henderson <richard.henderson@linaro.org>, qemu-devel@nongnu.org
Cc: pbonzini@redhat.com, alex.bennee@linaro.org, stefanha@redhat.com
Subject: Re: [PATCH v3 12/20] cputlb: Move ROM handling from I/O path to TLB path
Date: Mon, 23 Sep 2019 10:39:44 +0200 [thread overview]
Message-ID: <ef7b34cd-32ba-28b5-329a-3edd914a7fb4@redhat.com> (raw)
In-Reply-To: <20190922035458.14879-13-richard.henderson@linaro.org>
On 22.09.19 05:54, Richard Henderson wrote:
> It does not require going through the whole I/O path
> in order to discard a write.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/cpu-all.h | 5 ++++-
> include/exec/cpu-common.h | 1 -
> accel/tcg/cputlb.c | 35 +++++++++++++++++++--------------
> exec.c | 41 +--------------------------------------
> 4 files changed, 25 insertions(+), 57 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 1ebd1b59ab..9f0b17802e 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -348,12 +348,15 @@ CPUArchState *cpu_copy(CPUArchState *env);
> #define TLB_WATCHPOINT (1 << (TARGET_PAGE_BITS_MIN - 4))
> /* Set if TLB entry requires byte swap. */
> #define TLB_BSWAP (1 << (TARGET_PAGE_BITS_MIN - 5))
> +/* Set if TLB entry writes ignored. */
> +#define TLB_ROM (1 << (TARGET_PAGE_BITS_MIN - 6))
I was wondering if TLB_DISCARD_WRITE/TLB_IGNORE_WRITE/TLB_READONLY would
make it clearer what's actually happening here.
E.g., it isn't used for memory_region_is_romd(section->mr) but only for
memory_region_is_ram(section->mr) && section->readonly.
But anyhow, changes look fine to me
Reviewed-by: David Hildenbrand <david@redhat.com>
>
> /* Use this mask to check interception with an alignment mask
> * in a TCG backend.
> */
> #define TLB_FLAGS_MASK \
> - (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_MMIO | TLB_WATCHPOINT | TLB_BSWAP)
> + (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_MMIO \
> + | TLB_WATCHPOINT | TLB_BSWAP | TLB_ROM)
>
> /**
> * tlb_hit_page: return true if page aligned @addr is a hit against the
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index f7dbe75fbc..1c0e03ddc2 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -100,7 +100,6 @@ void qemu_flush_coalesced_mmio_buffer(void);
>
> void cpu_flush_icache_range(hwaddr start, hwaddr len);
>
> -extern struct MemoryRegion io_mem_rom;
> extern struct MemoryRegion io_mem_notdirty;
>
> typedef int (RAMBlockIterFunc)(RAMBlock *rb, void *opaque);
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index cb603917a2..7ab523d7ec 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -577,7 +577,7 @@ static void tlb_reset_dirty_range_locked(CPUTLBEntry *tlb_entry,
> {
> uintptr_t addr = tlb_entry->addr_write;
>
> - if ((addr & (TLB_INVALID_MASK | TLB_MMIO | TLB_NOTDIRTY)) == 0) {
> + if ((addr & (TLB_INVALID_MASK | TLB_MMIO | TLB_ROM | TLB_NOTDIRTY)) == 0) {
> addr &= TARGET_PAGE_MASK;
> addr += tlb_entry->addend;
> if ((addr - start) < length) {
> @@ -745,7 +745,6 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
> address |= TLB_MMIO;
> addend = 0;
> } else {
> - /* TLB_MMIO for rom/romd handled below */
> addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat;
> }
>
> @@ -822,16 +821,17 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
>
> tn.addr_write = -1;
> if (prot & PAGE_WRITE) {
> - if ((memory_region_is_ram(section->mr) && section->readonly)
> - || memory_region_is_romd(section->mr)) {
> - /* Write access calls the I/O callback. */
> - tn.addr_write = address | TLB_MMIO;
> - } else if (memory_region_is_ram(section->mr)
> - && cpu_physical_memory_is_clean(
> - memory_region_get_ram_addr(section->mr) + xlat)) {
> - tn.addr_write = address | TLB_NOTDIRTY;
> - } else {
> - tn.addr_write = address;
> + tn.addr_write = address;
> + if (memory_region_is_romd(section->mr)) {
> + /* Use the MMIO path so that the device can switch states. */
> + tn.addr_write |= TLB_MMIO;
> + } else if (memory_region_is_ram(section->mr)) {
> + if (section->readonly) {
> + tn.addr_write |= TLB_ROM;
> + } else if (cpu_physical_memory_is_clean(
> + memory_region_get_ram_addr(section->mr) + xlat)) {
> + tn.addr_write |= TLB_NOTDIRTY;
> + }
> }
> if (prot & PAGE_WRITE_INV) {
> tn.addr_write |= TLB_INVALID_MASK;
> @@ -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_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {
> + if (mr != &io_mem_notdirty && !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_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {
> + if (mr != &io_mem_notdirty && !cpu->can_do_io) {
> cpu_io_recompile(cpu, retaddr);
> }
> cpu->mem_io_vaddr = addr;
> @@ -1125,7 +1125,7 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
> }
>
> /* Reject I/O access, or other required slow-path. */
> - if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO | TLB_BSWAP)) {
> + if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO | TLB_BSWAP | TLB_ROM)) {
> return NULL;
> }
>
> @@ -1612,6 +1612,11 @@ store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
> return;
> }
>
> + /* Ignore writes to ROM. */
> + if (unlikely(tlb_addr & TLB_ROM)) {
> + return;
> + }
> +
> haddr = (void *)((uintptr_t)addr + entry->addend);
>
> if (unlikely(tlb_addr & TLB_BSWAP)) {
> diff --git a/exec.c b/exec.c
> index 7ce0515635..e21e068535 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -88,7 +88,7 @@ static MemoryRegion *system_io;
> AddressSpace address_space_io;
> AddressSpace address_space_memory;
>
> -MemoryRegion io_mem_rom, io_mem_notdirty;
> +MemoryRegion io_mem_notdirty;
> static MemoryRegion io_mem_unassigned;
> #endif
>
> @@ -158,7 +158,6 @@ typedef struct subpage_t {
>
> #define PHYS_SECTION_UNASSIGNED 0
> #define PHYS_SECTION_NOTDIRTY 1
> -#define PHYS_SECTION_ROM 2
>
> static void io_mem_init(void);
> static void memory_map_init(void);
> @@ -1441,8 +1440,6 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
> iotlb = memory_region_get_ram_addr(section->mr) + xlat;
> if (!section->readonly) {
> iotlb |= PHYS_SECTION_NOTDIRTY;
> - } else {
> - iotlb |= PHYS_SECTION_ROM;
> }
> } else {
> AddressSpaceDispatch *d;
> @@ -2968,38 +2965,6 @@ static uint16_t dummy_section(PhysPageMap *map, FlatView *fv, MemoryRegion *mr)
> return phys_section_add(map, §ion);
> }
>
> -static void readonly_mem_write(void *opaque, hwaddr addr,
> - uint64_t val, unsigned size)
> -{
> - /* Ignore any write to ROM. */
> -}
> -
> -static bool readonly_mem_accepts(void *opaque, hwaddr addr,
> - unsigned size, bool is_write,
> - MemTxAttrs attrs)
> -{
> - return is_write;
> -}
> -
> -/* This will only be used for writes, because reads are special cased
> - * to directly access the underlying host ram.
> - */
> -static const MemoryRegionOps readonly_mem_ops = {
> - .write = readonly_mem_write,
> - .valid.accepts = readonly_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,
> - },
> -};
> -
> MemoryRegionSection *iotlb_to_section(CPUState *cpu,
> hwaddr index, MemTxAttrs attrs)
> {
> @@ -3013,8 +2978,6 @@ MemoryRegionSection *iotlb_to_section(CPUState *cpu,
>
> static void io_mem_init(void)
> {
> - memory_region_init_io(&io_mem_rom, NULL, &readonly_mem_ops,
> - NULL, NULL, UINT64_MAX);
> memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,
> NULL, UINT64_MAX);
>
> @@ -3035,8 +2998,6 @@ AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv)
> assert(n == PHYS_SECTION_UNASSIGNED);
> n = dummy_section(&d->map, fv, &io_mem_notdirty);
> assert(n == PHYS_SECTION_NOTDIRTY);
> - n = dummy_section(&d->map, fv, &io_mem_rom);
> - assert(n == PHYS_SECTION_ROM);
>
> d->phys_map = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
>
>
--
Thanks,
David / dhildenb
next prev parent reply other threads:[~2019-09-23 9:06 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-22 3:54 [PATCH v3 00/20] Move rom and notdirty handling to cputlb Richard Henderson
2019-09-22 3:54 ` [PATCH v3 01/20] exec: Use TARGET_PAGE_BITS_MIN for TLB flags Richard Henderson
2019-09-23 8:24 ` David Hildenbrand
2019-09-22 3:54 ` [PATCH v3 02/20] exec: Split out variable page size support to exec-vary.c Richard Henderson
2019-09-23 8:26 ` David Hildenbrand
2019-09-23 16:27 ` Richard Henderson
2019-09-22 3:54 ` [PATCH v3 03/20] exec: Use const alias for TARGET_PAGE_BITS_VARY Richard Henderson
2019-09-22 3:54 ` [PATCH v3 04/20] exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG Richard Henderson
2019-09-22 3:54 ` [PATCH v3 05/20] exec: Promote TARGET_PAGE_MASK to target_long Richard Henderson
2019-09-23 8:30 ` David Hildenbrand
2019-09-22 3:54 ` [PATCH v3 06/20] exec: Tidy TARGET_PAGE_ALIGN Richard Henderson
2019-09-23 8:30 ` David Hildenbrand
2019-09-22 3:54 ` [PATCH v3 07/20] exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY Richard Henderson
2019-09-23 8:31 ` David Hildenbrand
2019-09-22 3:54 ` [PATCH v3 08/20] cputlb: Disable __always_inline__ without optimization Richard Henderson
2019-09-23 9:18 ` Philippe Mathieu-Daudé
2019-09-23 9:45 ` Paolo Bonzini
2019-09-23 16:00 ` Richard Henderson
2019-09-23 16:49 ` Paolo Bonzini
2019-09-23 18:09 ` Richard Henderson
2019-09-22 3:54 ` [PATCH v3 09/20] cputlb: Replace switches in load/store_helper with callback Richard Henderson
2019-09-23 8:32 ` David Hildenbrand
2019-09-23 9:27 ` Philippe Mathieu-Daudé
2019-09-23 9:51 ` Paolo Bonzini
2019-09-23 9:54 ` David Hildenbrand
2019-09-23 10:02 ` Paolo Bonzini
2019-09-23 15:52 ` Richard Henderson
2019-09-23 18:18 ` Richard Henderson
2019-09-22 3:54 ` [PATCH v3 10/20] cputlb: Introduce TLB_BSWAP Richard Henderson
2019-09-23 8:33 ` David Hildenbrand
2019-09-22 3:54 ` [PATCH v3 11/20] exec: Adjust notdirty tracing Richard Henderson
2019-09-23 9:17 ` Philippe Mathieu-Daudé
2019-09-22 3:54 ` [PATCH v3 12/20] cputlb: Move ROM handling from I/O path to TLB path Richard Henderson
2019-09-23 8:39 ` David Hildenbrand [this message]
2019-09-22 3:54 ` [PATCH v3 13/20] cputlb: Move NOTDIRTY " Richard Henderson
2019-09-23 8:41 ` David Hildenbrand
2019-09-23 9:30 ` Philippe Mathieu-Daudé
2019-09-22 3:54 ` [PATCH v3 14/20] cputlb: Partially inline memory_region_section_get_iotlb Richard Henderson
2019-09-22 3:54 ` [PATCH v3 15/20] cputlb: Merge and move memory_notdirty_write_{prepare, complete} Richard Henderson
2019-09-22 3:54 ` [PATCH v3 16/20] cputlb: Handle TLB_NOTDIRTY in probe_access Richard Henderson
2019-09-22 3:54 ` [PATCH v3 17/20] cputlb: Remove cpu->mem_io_vaddr Richard Henderson
2019-09-23 8:50 ` David Hildenbrand
2019-09-22 3:54 ` [PATCH v3 18/20] cputlb: Remove tb_invalidate_phys_page_range is_cpu_write_access Richard Henderson
2019-09-23 8:52 ` David Hildenbrand
2019-09-23 16:05 ` Richard Henderson
2019-09-23 16:50 ` Paolo Bonzini
2019-09-22 3:54 ` [PATCH v3 19/20] cputlb: Pass retaddr to tb_invalidate_phys_page_fast Richard Henderson
2019-09-23 8:53 ` David Hildenbrand
2019-09-22 3:54 ` [PATCH v3 20/20] cputlb: Pass retaddr to tb_check_watchpoint Richard Henderson
2019-09-23 8:54 ` David Hildenbrand
2019-09-22 4:02 ` [PATCH v3 00/20] Move rom and notdirty handling to cputlb Richard Henderson
2019-09-22 6:46 ` no-reply
2019-09-23 8:23 ` David Hildenbrand
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=ef7b34cd-32ba-28b5-329a-3edd914a7fb4@redhat.com \
--to=david@redhat.com \
--cc=alex.bennee@linaro.org \
--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).