Re: [PATCH v3 56/60] target/arm: Enable FEAT_CSV2_2 for -cpu max

[Damien Hedde]

On 4/17/22 19:44, Richard Henderson wrote:
> There is no branch prediction in TCG, therefore there is no
> need to actually include the context number into the predictor.
> Therefore all we need to do is add the state for SCXTNUM_ELx. >
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Update emulation.rst; clear CSV2_FRAC; use decimal; tidy access_scxtnum.
> v3: Rely on EL3-no-EL2 squashing during registration.
> ---
>   docs/system/arm/emulation.rst |  3 ++
>   target/arm/cpu.h              | 16 +++++++++
>   target/arm/cpu64.c            |  3 +-
>   target/arm/helper.c           | 66 ++++++++++++++++++++++++++++++++++-
>   4 files changed, 86 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> @@ -7233,7 +7243,57 @@ static const ARMCPRegInfo mte_el0_cacheop_reginfo[] = {
>       },
>   };

Hi Richard,

I tried to compare with the pseudocode from arm doc and I've a few 
interrogations. It seems to me there are missing cases in the access 
checks, but I lack the background to know if these are not checked 
somewhere else or guaranteed to never happen.

>   
> -#endif > +static CPAccessResult access_scxtnum(CPUARMState *env, const 
ARMCPRegInfo *ri,
> +                                     bool isread)
> +{
The following checks are missing:
   + for HFG[W/R]TR_EL2.SCXTNUM_EL0/1
   + HCR_EL2.<NV2,NV1,NV> when accessing SCXTNUM_EL1, but maybe these 
are always guaranteed to fail because we don't support the features ?
   + HCR_EL2.NV when accessing SCXTNUM_EL2
> +    int el = arm_current_el(env);
> +
> +    if (el == 0) {
> +        uint64_t hcr = arm_hcr_el2_eff(env);
> +        if ((hcr & (HCR_TGE | HCR_E2H)) != (HCR_TGE | HCR_E2H)) {
> +            if (env->cp15.sctlr_el[1] & SCTLR_TSCXT) {
> +                if (hcr & HCR_TGE) {
> +                    return CP_ACCESS_TRAP_EL2;
> +                }
> +                return CP_ACCESS_TRAP;
> +            }
> +            if (arm_is_el2_enabled(env) && !(hcr & HCR_ENSCXT)) {
This case is also present when accessing SCXTNUM_EL0 from el1 (but 
without "(hcr & (HCR_TGE | HCR_E2H)) != (HCR_TGE | HCR_E2H)" precondition)
> +                return CP_ACCESS_TRAP_EL2;
> +            }
> +            goto no_sctlr_el2;
> +        }
> +    }
> +    if (el < 2 && (env->cp15.sctlr_el[2] & SCTLR_TSCXT)) {
> +        return CP_ACCESS_TRAP_EL2;
> +    }
> + no_sctlr_el2:
> +    if (el < 3
> +        && arm_feature(env, ARM_FEATURE_EL3)
> +        && !(env->cp15.scr_el3 & SCR_ENSCXT)) {
> +        return CP_ACCESS_TRAP_EL3;
> +    }
> +    return CP_ACCESS_OK;
> +}

Regards,
--
Damien