Re: [PATCH] target/arm: Fix BTI versus CF_PCREL

[Peter Maydell]

On Tue, 30 Jul 2024 at 03:07, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> With pcrel, we cannot check the guarded page bit at translation
> time, as different mappings of the same physical page may or may
> not have the GP bit set.
>
> Instead, add a couple of helpers to check the page at runtime,
> after all other filters that might obviate the need for the check.
>
> The set_btype_for_br call must be moved after the gen_a64_set_pc
> call to ensure the current pc can still be computed.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/tcg/helper-a64.h    |  3 ++
>  target/arm/tcg/translate.h     |  2 --
>  target/arm/tcg/helper-a64.c    | 38 ++++++++++++++++++++
>  target/arm/tcg/translate-a64.c | 64 ++++++++--------------------------
>  4 files changed, 55 insertions(+), 52 deletions(-)
>
> diff --git a/target/arm/tcg/helper-a64.h b/target/arm/tcg/helper-a64.h
> index 371388f61b..481007bf39 100644
> --- a/target/arm/tcg/helper-a64.h
> +++ b/target/arm/tcg/helper-a64.h
> @@ -133,6 +133,9 @@ DEF_HELPER_4(cpyfp, void, env, i32, i32, i32)
>  DEF_HELPER_4(cpyfm, void, env, i32, i32, i32)
>  DEF_HELPER_4(cpyfe, void, env, i32, i32, i32)
>
> +DEF_HELPER_FLAGS_1(guarded_page_check, TCG_CALL_NO_WG, void, env)
> +DEF_HELPER_FLAGS_2(guarded_page_br, TCG_CALL_NO_RWG, void, env, tl)
> +
>  DEF_HELPER_FLAGS_5(gvec_fdiv_h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(gvec_fdiv_s, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
>  DEF_HELPER_FLAGS_5(gvec_fdiv_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, 
> i32)
> diff --git a/target/arm/tcg/translate.h b/target/arm/tcg/translate.h
> index a8672c857c..01c217f4a4 100644
> --- a/target/arm/tcg/translate.h
> +++ b/target/arm/tcg/translate.h
> @@ -163,8 +163,6 @@ typedef struct DisasContext {
>      uint8_t dcz_blocksize;
>      /* A copy of cpu->gm_blocksize. */
>      uint8_t gm_blocksize;
> -    /* True if this page is guarded.  */
> -    bool guarded_page;
>      /* True if the current insn_start has been updated. */
>      bool insn_start_updated;
>      /* Bottom two bits of XScale c15_cpar coprocessor access control reg */
> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
> index c60d2a7ec9..424fe927b4 100644
> --- a/target/arm/tcg/helper-a64.c
> +++ b/target/arm/tcg/helper-a64.c
> @@ -1877,3 +1877,41 @@ void HELPER(cpyfe)(CPUARMState *env, uint32_t 
> syndrome, uint32_t wdesc,
>  {
>      do_cpye(env, syndrome, wdesc, rdesc, false, GETPC());
>  }
> +
> +static bool is_guarded_page(CPUARMState *env, target_ulong addr)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    return page_get_flags(addr) & PAGE_BTI;
> +#else
> +    CPUTLBEntryFull *full;
> +    void *host;
> +    int mmu_idx = cpu_mmu_index(env_cpu(env), true);
> +    int flags = probe_access_full(env, addr, 0, MMU_INST_FETCH, mmu_idx,
> +                                  false, &host, &full, 0);
> +
> +    assert(!(flags & TLB_INVALID_MASK));

Is there a race condition here where some other vCPU
knocks this entry out of the TLB between the point when
we started executing the TB and when we made this helper
function call ?

> +    return full->extra.arm.guarded;
> +#endif
> +}

thanks
-- PMM