[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 23/42] target/arm: Use probe_access_full for BTI
From: |
Peter Maydell |
Subject: |
Re: [PATCH v3 23/42] target/arm: Use probe_access_full for BTI |
Date: |
Thu, 6 Oct 2022 15:57:13 +0100 |
On Sat, 1 Oct 2022 at 17:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Add a field to TARGET_PAGE_ENTRY_EXTRA to hold the guarded bit.
> In is_guarded_page, use probe_access_full instead of just guessing
> that the tlb entry is still present. Also handles the FIXME about
> executing from device memory.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/cpu-param.h | 8 ++++----
> target/arm/cpu.h | 13 -------------
> target/arm/internals.h | 1 +
> target/arm/ptw.c | 7 ++++---
> target/arm/translate-a64.c | 22 ++++++++--------------
> 5 files changed, 17 insertions(+), 34 deletions(-)
>
> diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
> index 118ca0e5c0..689a9645dc 100644
> --- a/target/arm/cpu-param.h
> +++ b/target/arm/cpu-param.h
> @@ -32,12 +32,12 @@
> # define TARGET_PAGE_BITS_MIN 10
>
> /*
> - * Cache the attrs and sharability fields from the page table entry.
> + * Cache the attrs, sharability, and gp fields from the page table entry.
> */
> # define TARGET_PAGE_ENTRY_EXTRA \
> - uint8_t pte_attrs; \
> - uint8_t shareability;
> -
> + uint8_t pte_attrs; \
> + uint8_t shareability; \
> + bool guarded;
I notice this now brings this very close to just having an ARMCacheAttrs
struct in it (in fact it's going to be one byte bigger than the ARMCachettrs).
But it's probably better to keep them separate since we care a lot more
about keeping the TLB entry small I suppose.
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 5b67375f4e..22802d1d2f 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -14601,22 +14601,16 @@ static bool is_guarded_page(CPUARMState *env,
> DisasContext *s)
> #ifdef CONFIG_USER_ONLY
> return page_get_flags(addr) & PAGE_BTI;
> #else
> + CPUTLBEntryFull *full;
> + void *host;
> int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx);
> - unsigned int index = tlb_index(env, mmu_idx, addr);
> - CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> + int flags;
>
> - /*
> - * We test this immediately after reading an insn, which means
> - * that any normal page must be in the TLB. The only exception
> - * would be for executing from flash or device memory, which
> - * does not retain the TLB entry.
> - *
> - * FIXME: Assume false for those, for now. We could use
> - * arm_cpu_get_phys_page_attrs_debug to re-read the page
> - * table entry even for that case.
> - */
I think we should keep at least some of this comment: the part
about the reason we can assert that probe_access_full() doesn't
return TLB_INVALID being that we tested immediately after the
insn read is still true, right?
> - return (tlb_hit(entry->addr_code, addr) &&
> - arm_tlb_bti_gp(&env_tlb(env)->d[mmu_idx].fulltlb[index].attrs));
> + flags = probe_access_full(env, addr, MMU_INST_FETCH, mmu_idx,
> + false, &host, &full, 0);
> + assert(!(flags & TLB_INVALID_MASK));
> +
> + return full->guarded;
> #endif
> }
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
- [PATCH v3 19/42] target/arm: Fix cacheattr in get_phys_addr_disabled, (continued)
- [PATCH v3 19/42] target/arm: Fix cacheattr in get_phys_addr_disabled, Richard Henderson, 2022/10/01
- [PATCH v3 20/42] target/arm: Use tlb_set_page_full, Richard Henderson, 2022/10/01
- [PATCH v3 22/42] target/arm: Use probe_access_full for MTE, Richard Henderson, 2022/10/01
- [PATCH v3 09/42] target/arm: Add is_secure parameter to do_ats_write, Richard Henderson, 2022/10/01
- [PATCH v3 17/42] target/arm: Fix ATS12NSO* from S PL1, Richard Henderson, 2022/10/01
- [PATCH v3 18/42] target/arm: Split out get_phys_addr_disabled, Richard Henderson, 2022/10/01
- [PATCH v3 23/42] target/arm: Use probe_access_full for BTI, Richard Henderson, 2022/10/01
- Re: [PATCH v3 23/42] target/arm: Use probe_access_full for BTI,
Peter Maydell <=
- [PATCH v3 24/42] target/arm: Add ARMMMUIdx_Phys_{S,NS}, Richard Henderson, 2022/10/01
- [PATCH v3 21/42] target/arm: Enable TARGET_PAGE_ENTRY_EXTRA, Richard Henderson, 2022/10/01
- [PATCH v3 25/42] target/arm: Move ARMMMUIdx_Stage2 to a real tlb mmu_idx, Richard Henderson, 2022/10/01
- [PATCH v3 26/42] target/arm: Plumb debug into S1_ptw_translate, Richard Henderson, 2022/10/01