Re: [RFC PATCH] target/arm: don't process FCSE translations on physical TLBs

[Richard Henderson]

On 10/22/22 03:23, Alex Bennée wrote:
> A recent change to the page table walking code missed checking if we
> are at the processing the physical translation and bombs attempting to
> derive the current EL. Fix this by introducing a new
> helper (regime_is_phys) and extending the check around the FCSE
> processing.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Fixes: f3639a64f6 (target/arm: Use softmmu tlbs for page table walking)
> Cc: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/internals.h | 12 ++++++++++++
>   target/arm/ptw.c       |  2 +-
>   2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index c3c3920ded..0e753203b5 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -673,6 +673,18 @@ static inline bool regime_is_pan(CPUARMState *env, 
> ARMMMUIdx mmu_idx)
>       }
>   }
>   
> +static inline bool regime_is_phys(ARMMMUIdx mmu_idx)
> +{
> +    switch (mmu_idx) {
> +    case ARMMMUIdx_Phys_NS:
> +    case ARMMMUIdx_Phys_S:
> +        return true;
> +    default:
> +        return false;
> +    }
> +}
> +
> +
>   /* Return the exception level which controls this address translation regime 
> */
>   static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
>   {
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 6c5ed56a10..1456a2f1de 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -2557,7 +2557,7 @@ static bool get_phys_addr_with_struct(CPUARMState *env, 
> S1Translate *ptw,
>        * Fast Context Switch Extension. This doesn't exist at all in v8.
>        * In v7 and earlier it affects all stage 1 translations.
>        */
> -    if (address < 0x02000000 && mmu_idx != ARMMMUIdx_Stage2
> +    if (address < 0x02000000 && !(mmu_idx == ARMMMUIdx_Stage2 || 
> regime_is_phys(mmu_idx))
>           && !arm_feature(env, ARM_FEATURE_V8)) {
>           if (regime_el(env, mmu_idx) == 3) {
>               address += env->cp15.fcseidr_s;

Arg!  This is fixed in patch 2 of the FEAT_HAFDBS patches:

20221020223548.2310496-3-richard.henderson@linaro.org/">https://lore.kernel.org/qemu-devel/20221020223548.2310496-3-richard.henderson@linaro.org/

    switch (mmu_idx) {
    case ARMMMUIdx_Phys_S:
    case ARMMMUIdx_Phys_NS:
        /* Checking Phys early avoids special casing later vs regime_el. */
        return get_phys_addr_disabled(env, address, access_type, mmu_idx,
                                      is_secure, result, fi);


r~