From 1770b2f2d3d6fe8f1e2d61692692264cac44340d Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Thu, 23 Feb 2023 20:44:24 -0300 Subject: accel/tcg: Add 'size' param to probe_access_flags() probe_access_flags() as it is today uses probe_access_full(), which in turn uses probe_access_internal() with size = 0. probe_access_internal() then uses the size to call the tlb_fill() callback for the given CPU. This size param ('fault_size' as probe_access_internal() calls it) is ignored by most existing .tlb_fill callback implementations, e.g. arm_cpu_tlb_fill(), ppc_cpu_tlb_fill(), x86_cpu_tlb_fill() and mips_cpu_tlb_fill() to name a few. But RISC-V riscv_cpu_tlb_fill() actually uses it. The 'size' parameter is used to check for PMP (Physical Memory Protection) access. This is necessary because PMP does not make any guarantees about all the bytes of the same page having the same permissions, i.e. the same page can have different PMP properties, so we're forced to make sub-page range checks. To allow RISC-V emulation to do a probe_acess_flags() that covers PMP, we need to either add a 'size' param to the existing probe_acess_flags() or create a new interface (e.g. probe_access_range_flags). There are quite a few probe_* APIs already, so let's add a 'size' param to probe_access_flags() and re-use this API. This is done by open coding what probe_access_full() does inside probe_acess_flags() and passing the 'size' param to probe_acess_internal(). Existing probe_access_flags() callers use size = 0 to not change their current API usage. 'size' is asserted to enforce single page access like probe_access() already does. No behavioral changes intended. Signed-off-by: Daniel Henrique Barboza Message-Id: <20230223234427.521114-2-dbarboza@ventanamicro.com> Reviewed-by: Richard Henderson Signed-off-by: Richard Henderson --- include/exec/exec-all.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include/exec/exec-all.h') diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 0e36f4d063..165b050872 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -447,6 +447,7 @@ static inline void *probe_read(CPUArchState *env, target_ulong addr, int size, * probe_access_flags: * @env: CPUArchState * @addr: guest virtual address to look up + * @size: size of the access * @access_type: read, write or execute permission * @mmu_idx: MMU index to use for lookup * @nonfault: suppress the fault @@ -461,7 +462,7 @@ static inline void *probe_read(CPUArchState *env, target_ulong addr, int size, * Do handle clean pages, so exclude TLB_NOTDIRY from the returned flags. * For simplicity, all "mmio-like" flags are folded to TLB_MMIO. */ -int probe_access_flags(CPUArchState *env, target_ulong addr, +int probe_access_flags(CPUArchState *env, target_ulong addr, int size, MMUAccessType access_type, int mmu_idx, bool nonfault, void **phost, uintptr_t retaddr); -- cgit 1.4.1 From d507e6c565c8c44488eaf04ff4da3ba2183c6be5 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 23 Feb 2023 14:44:14 -1000 Subject: accel/tcg: Add 'size' param to probe_access_full MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change to match the recent change to probe_access_flags. All existing callers updated to supply 0, so no change in behaviour. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Daniel Henrique Barboza Signed-off-by: Richard Henderson --- accel/tcg/cputlb.c | 4 ++-- include/exec/exec-all.h | 2 +- target/arm/ptw.c | 2 +- target/arm/tcg/mte_helper.c | 4 ++-- target/arm/tcg/sve_helper.c | 2 +- target/arm/tcg/translate-a64.c | 2 +- target/i386/tcg/sysemu/excp_helper.c | 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-) (limited to 'include/exec/exec-all.h') diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index fc27e34457..008ae7a66d 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -1589,12 +1589,12 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr, return flags; } -int probe_access_full(CPUArchState *env, target_ulong addr, +int probe_access_full(CPUArchState *env, target_ulong addr, int size, MMUAccessType access_type, int mmu_idx, bool nonfault, void **phost, CPUTLBEntryFull **pfull, uintptr_t retaddr) { - int flags = probe_access_internal(env, addr, 0, access_type, mmu_idx, + int flags = probe_access_internal(env, addr, size, access_type, mmu_idx, nonfault, phost, pfull, retaddr); /* Handle clean RAM pages. */ diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 165b050872..b631832e17 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -475,7 +475,7 @@ int probe_access_flags(CPUArchState *env, target_ulong addr, int size, * and must be consumed or copied immediately, before any further * access or changes to TLB @mmu_idx. */ -int probe_access_full(CPUArchState *env, target_ulong addr, +int probe_access_full(CPUArchState *env, target_ulong addr, int size, MMUAccessType access_type, int mmu_idx, bool nonfault, void **phost, CPUTLBEntryFull **pfull, uintptr_t retaddr); diff --git a/target/arm/ptw.c b/target/arm/ptw.c index cf3f2fd703..8541ef56d6 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -259,7 +259,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, int flags; env->tlb_fi = fi; - flags = probe_access_full(env, addr, MMU_DATA_LOAD, + flags = probe_access_full(env, addr, 0, MMU_DATA_LOAD, arm_to_core_mmu_idx(s2_mmu_idx), true, &ptw->out_host, &full, 0); env->tlb_fi = NULL; diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c index 98bcf59c22..fee3c7eb96 100644 --- a/target/arm/tcg/mte_helper.c +++ b/target/arm/tcg/mte_helper.c @@ -118,7 +118,7 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx, * valid. Indicate to probe_access_flags no-fault, then assert that * we received a valid page. */ - flags = probe_access_full(env, ptr, ptr_access, ptr_mmu_idx, + flags = probe_access_full(env, ptr, 0, ptr_access, ptr_mmu_idx, ra == 0, &host, &full, ra); assert(!(flags & TLB_INVALID_MASK)); @@ -154,7 +154,7 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx, */ in_page = -(ptr | TARGET_PAGE_MASK); if (unlikely(ptr_size > in_page)) { - flags |= probe_access_full(env, ptr + in_page, ptr_access, + flags |= probe_access_full(env, ptr + in_page, 0, ptr_access, ptr_mmu_idx, ra == 0, &host, &full, ra); assert(!(flags & TLB_INVALID_MASK)); } diff --git a/target/arm/tcg/sve_helper.c b/target/arm/tcg/sve_helper.c index 51909c44ac..9a8951afa4 100644 --- a/target/arm/tcg/sve_helper.c +++ b/target/arm/tcg/sve_helper.c @@ -5356,7 +5356,7 @@ bool sve_probe_page(SVEHostPage *info, bool nofault, CPUARMState *env, &info->host, retaddr); #else CPUTLBEntryFull *full; - flags = probe_access_full(env, addr, access_type, mmu_idx, nofault, + flags = probe_access_full(env, addr, 0, access_type, mmu_idx, nofault, &info->host, &full, retaddr); #endif info->flags = flags; diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c index da9f877476..67e9c4ee79 100644 --- a/target/arm/tcg/translate-a64.c +++ b/target/arm/tcg/translate-a64.c @@ -14651,7 +14651,7 @@ static bool is_guarded_page(CPUARMState *env, DisasContext *s) * that the TLB entry must be present and valid, and thus this * access will never raise an exception. */ - flags = probe_access_full(env, addr, MMU_INST_FETCH, mmu_idx, + flags = probe_access_full(env, addr, 0, MMU_INST_FETCH, mmu_idx, false, &host, &full, 0); assert(!(flags & TLB_INVALID_MASK)); diff --git a/target/i386/tcg/sysemu/excp_helper.c b/target/i386/tcg/sysemu/excp_helper.c index 55bd1194d3..e87f90dbe3 100644 --- a/target/i386/tcg/sysemu/excp_helper.c +++ b/target/i386/tcg/sysemu/excp_helper.c @@ -64,7 +64,7 @@ static bool ptw_translate(PTETranslate *inout, hwaddr addr) int flags; inout->gaddr = addr; - flags = probe_access_full(inout->env, addr, MMU_DATA_STORE, + flags = probe_access_full(inout->env, addr, 0, MMU_DATA_STORE, inout->ptw_idx, true, &inout->haddr, &full, 0); if (unlikely(flags & TLB_INVALID_MASK)) { @@ -428,7 +428,7 @@ do_check_protect_pse36: CPUTLBEntryFull *full; int flags, nested_page_size; - flags = probe_access_full(env, paddr, access_type, + flags = probe_access_full(env, paddr, 0, access_type, MMU_NESTED_IDX, true, &pte_trans.haddr, &full, 0); if (unlikely(flags & TLB_INVALID_MASK)) { -- cgit 1.4.1 From 5b6dfc6c9b56372b674ceeabde26f1260c663645 Mon Sep 17 00:00:00 2001 From: Anton Johansson Date: Mon, 27 Feb 2023 14:51:36 +0100 Subject: include/exec: Introduce `CF_PCREL` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new field to TranslationBlock.cflags denoting whether or not the instructions of a given translation block are pc-relative. This field aims to replace the macro `TARGET_TB_PCREL`. Signed-off-by: Anton Johansson Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20230227135202.9710-2-anjo@rev.ng> Signed-off-by: Richard Henderson --- include/exec/exec-all.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/exec/exec-all.h') diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index b631832e17..1574eabac8 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -546,6 +546,7 @@ struct TranslationBlock { #define CF_INVALID 0x00040000 /* TB is stale. Set with @jmp_lock held */ #define CF_PARALLEL 0x00080000 /* Generate code for a parallel context */ #define CF_NOIRQ 0x00100000 /* Generate an uninterruptible TB */ +#define CF_PCREL 0x00200000 /* Opcodes in TB are PC-relative */ #define CF_CLUSTER_MASK 0xff000000 /* Top 8 bits are cluster ID */ #define CF_CLUSTER_SHIFT 24 -- cgit 1.4.1 From 1fad5fce1912a3ee94ae17efabcdb900442874d7 Mon Sep 17 00:00:00 2001 From: Anton Johansson Date: Mon, 27 Feb 2023 14:51:40 +0100 Subject: include/exec: Replace `TARGET_TB_PCREL` with `CF_PCREL` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Anton Johansson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20230227135202.9710-6-anjo@rev.ng> Signed-off-by: Richard Henderson --- include/exec/exec-all.h | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) (limited to 'include/exec/exec-all.h') diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 1574eabac8..6af001bfde 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -506,22 +506,20 @@ struct tb_tc { }; struct TranslationBlock { -#if !TARGET_TB_PCREL /* * Guest PC corresponding to this block. This must be the true * virtual address. Therefore e.g. x86 stores EIP + CS_BASE, and * targets like Arm, MIPS, HP-PA, which reuse low bits for ISA or * privilege, must store those bits elsewhere. * - * If TARGET_TB_PCREL, the opcodes for the TranslationBlock are - * written such that the TB is associated only with the physical - * page and may be run in any virtual address context. In this case, - * PC must always be taken from ENV in a target-specific manner. + * If CF_PCREL, the opcodes for the TranslationBlock are written + * such that the TB is associated only with the physical page and + * may be run in any virtual address context. In this case, PC + * must always be taken from ENV in a target-specific manner. * Unwind information is taken as offsets from the page, to be * deposited into the "current" PC. */ target_ulong pc; -#endif /* * Target-specific data associated with the TranslationBlock, e.g.: @@ -615,22 +613,19 @@ struct TranslationBlock { uintptr_t jmp_dest[2]; }; -/* Hide the read to avoid ifdefs for TARGET_TB_PCREL. */ -static inline target_ulong tb_pc(const TranslationBlock *tb) -{ -#if TARGET_TB_PCREL - qemu_build_not_reached(); -#else - return tb->pc; -#endif -} - /* Hide the qatomic_read to make code a little easier on the eyes */ static inline uint32_t tb_cflags(const TranslationBlock *tb) { return qatomic_read(&tb->cflags); } +/* Hide the read to avoid ifdefs for CF_PCREL. */ +static inline target_ulong tb_pc(const TranslationBlock *tb) +{ + assert(!(tb_cflags(tb) & CF_PCREL)); + return tb->pc; +} + static inline tb_page_addr_t tb_page_addr0(const TranslationBlock *tb) { #ifdef CONFIG_USER_ONLY -- cgit 1.4.1 From 640be074fa1cadf4b1447c122efaaa2987fa774a Mon Sep 17 00:00:00 2001 From: Anton Johansson Date: Mon, 27 Feb 2023 14:52:02 +0100 Subject: include/exec: Remove `tb_pc()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Anton Johansson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20230227135202.9710-28-anjo@rev.ng> Signed-off-by: Richard Henderson --- include/exec/exec-all.h | 7 ------- 1 file changed, 7 deletions(-) (limited to 'include/exec/exec-all.h') diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 6af001bfde..e09254333d 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -619,13 +619,6 @@ static inline uint32_t tb_cflags(const TranslationBlock *tb) return qatomic_read(&tb->cflags); } -/* Hide the read to avoid ifdefs for CF_PCREL. */ -static inline target_ulong tb_pc(const TranslationBlock *tb) -{ - assert(!(tb_cflags(tb) & CF_PCREL)); - return tb->pc; -} - static inline tb_page_addr_t tb_page_addr0(const TranslationBlock *tb) { #ifdef CONFIG_USER_ONLY -- cgit 1.4.1