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 10/16] cputlb: Partially inline memory_region_section_get_iotlb
Date: Wed, 25 Sep 2019 17:12:06 +0100 [thread overview]
Message-ID: <87r2449wyx.fsf@linaro.org> (raw)
In-Reply-To: <20190923230004.9231-11-richard.henderson@linaro.org>
Richard Henderson <richard.henderson@linaro.org> writes:
> There is only one caller, tlb_set_page_with_attrs. We cannot
> inline the entire function because the AddressSpaceDispatch
> structure is private to exec.c, and cannot easily be moved to
> include/exec/memory-internal.h.
>
> Compute is_ram and is_romd once within tlb_set_page_with_attrs.
> Fold the number of tests against these predicates. Compute
> cpu_physical_memory_is_clean outside of the tlb lock region.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> include/exec/exec-all.h | 6 +---
> accel/tcg/cputlb.c | 68 ++++++++++++++++++++++++++---------------
> exec.c | 22 ++-----------
> 3 files changed, 47 insertions(+), 49 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 81b02eb2fe..49db07ba0b 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -509,11 +509,7 @@ address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
> hwaddr *xlat, hwaddr *plen,
> MemTxAttrs attrs, int *prot);
> hwaddr memory_region_section_get_iotlb(CPUState *cpu,
> - MemoryRegionSection *section,
> - target_ulong vaddr,
> - hwaddr paddr, hwaddr xlat,
> - int prot,
> - target_ulong *address);
> + MemoryRegionSection *section);
> #endif
>
> /* vl.c */
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 05212ff244..05530a8b0c 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -704,13 +704,14 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
> MemoryRegionSection *section;
> unsigned int index;
> target_ulong address;
> - target_ulong code_address;
> + target_ulong write_address;
> uintptr_t addend;
> CPUTLBEntry *te, tn;
> hwaddr iotlb, xlat, sz, paddr_page;
> target_ulong vaddr_page;
> int asidx = cpu_asidx_from_attrs(cpu, attrs);
> int wp_flags;
> + bool is_ram, is_romd;
>
> assert_cpu_is_self(cpu);
>
> @@ -739,18 +740,46 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
> if (attrs.byte_swap) {
> address |= TLB_BSWAP;
> }
> - if (!memory_region_is_ram(section->mr) &&
> - !memory_region_is_romd(section->mr)) {
> - /* IO memory case */
> - address |= TLB_MMIO;
> - addend = 0;
> - } else {
> +
> + is_ram = memory_region_is_ram(section->mr);
> + is_romd = memory_region_is_romd(section->mr);
> +
> + if (is_ram || is_romd) {
> + /* RAM and ROMD both have associated host memory. */
> addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat;
> + } else {
> + /* I/O does not; force the host address to NULL. */
> + addend = 0;
> + }
> +
> + write_address = address;
> + if (is_ram) {
> + iotlb = memory_region_get_ram_addr(section->mr) + xlat;
> + /*
> + * Computing is_clean is expensive; avoid all that unless
> + * the page is actually writable.
> + */
> + if (prot & PAGE_WRITE) {
> + if (section->readonly) {
> + write_address |= TLB_ROM;
> + } else if (cpu_physical_memory_is_clean(iotlb)) {
> + write_address |= TLB_NOTDIRTY;
> + }
> + }
> + } else {
> + /* I/O or ROMD */
> + iotlb = memory_region_section_get_iotlb(cpu, section) + xlat;
> + /*
> + * Writes to romd devices must go through MMIO to enable write.
> + * Reads to romd devices go through the ram_ptr found above,
> + * but of course reads to I/O must go through MMIO.
> + */
> + write_address |= TLB_MMIO;
> + if (!is_romd) {
> + address = write_address;
> + }
> }
>
> - code_address = address;
> - iotlb = memory_region_section_get_iotlb(cpu, section, vaddr_page,
> - paddr_page, xlat, prot, &address);
> wp_flags = cpu_watchpoint_address_matches(cpu, vaddr_page,
> TARGET_PAGE_SIZE);
>
> @@ -790,8 +819,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
> /*
> * At this point iotlb contains a physical section number in the lower
> * TARGET_PAGE_BITS, and either
> - * + the ram_addr_t of the page base of the target RAM (if NOTDIRTY or ROM)
> - * + the offset within section->mr of the page base (otherwise)
> + * + the ram_addr_t of the page base of the target RAM (RAM)
> + * + the offset within section->mr of the page base (I/O, ROMD)
> * We subtract the vaddr_page (which is page aligned and thus won't
> * disturb the low bits) to give an offset which can be added to the
> * (non-page-aligned) vaddr of the eventual memory access to get
> @@ -814,25 +843,14 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
> }
>
> if (prot & PAGE_EXEC) {
> - tn.addr_code = code_address;
> + tn.addr_code = address;
> } else {
> tn.addr_code = -1;
> }
>
> tn.addr_write = -1;
> if (prot & PAGE_WRITE) {
> - 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;
> - }
> - }
> + tn.addr_write = write_address;
> if (prot & PAGE_WRITE_INV) {
> tn.addr_write |= TLB_INVALID_MASK;
> }
> diff --git a/exec.c b/exec.c
> index dc7001f115..961d7d6497 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1459,26 +1459,10 @@ bool cpu_physical_memory_snapshot_get_dirty(DirtyBitmapSnapshot *snap,
>
> /* Called from RCU critical section */
> hwaddr memory_region_section_get_iotlb(CPUState *cpu,
> - MemoryRegionSection *section,
> - target_ulong vaddr,
> - hwaddr paddr, hwaddr xlat,
> - int prot,
> - target_ulong *address)
> + MemoryRegionSection *section)
> {
> - hwaddr iotlb;
> -
> - if (memory_region_is_ram(section->mr)) {
> - /* Normal RAM. */
> - iotlb = memory_region_get_ram_addr(section->mr) + xlat;
> - } else {
> - AddressSpaceDispatch *d;
> -
> - d = flatview_to_dispatch(section->fv);
> - iotlb = section - d->map.sections;
> - iotlb += xlat;
> - }
> -
> - return iotlb;
> + AddressSpaceDispatch *d = flatview_to_dispatch(section->fv);
> + return section - d->map.sections;
> }
> #endif /* defined(CONFIG_USER_ONLY) */
--
Alex Bennée
next prev parent reply other threads:[~2019-09-25 16:16 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
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 [this message]
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=87r2449wyx.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).