Re: [PATCH 4/6] target/arm: Implement FGWTE3 traps

On Fri, 24 Jul 2026 at 04:07, Richard Henderson
<[email protected]> wrote:
>
> Signed-off-by: Richard Henderson <[email protected]>
> ---
>  target/arm/tcg/op_helper.c     | 36 ++++++++++++++++++++++------------
>  target/arm/tcg/translate-a64.c | 13 +++++++++++-
>  2 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
> index c4433be2ed..f040a3fc87 100644
> --- a/target/arm/tcg/op_helper.c
> +++ b/target/arm/tcg/op_helper.c
> @@ -1067,12 +1067,10 @@ const void *HELPER(access_check_cp_reg)(CPUARMState *env, uint32_t key,
>       * higher priority than trap-to-EL3, and we don't care about priority
>       * order with other EL2 traps because the syndrome value is the same.
>       */

We should extend this comment, e.g something like:
   FGWTE3 traps are exclusively traps to EL3 on EL3-accessible-only
   registers, so there's no possibility of a trap to EL2, and we
   can check these here also.

> -    if (arm_fgt_active(env, arm_current_el(env))) {
> +    if (ri->fgt) {
>          uint64_t trapword = 0;
>          unsigned int idx = FIELD_EX32(ri->fgt, FGT, IDX);
>          unsigned int bitpos = FIELD_EX32(ri->fgt, FGT, BITPOS);
> -        bool rev = FIELD_EX32(ri->fgt, FGT, REV);
> -        bool nxs = FIELD_EX32(ri->fgt, FGT, NXS);
>          bool trapbit;
>
>          if (ri->fgt & FGT_EXEC) {
> @@ -1085,19 +1083,31 @@ const void *HELPER(access_check_cp_reg)(CPUARMState *env, uint32_t key,
>              assert(idx < ARRAY_SIZE(env->cp15.fgt_write));
>              trapword = env->cp15.fgt_write[idx];
>          }
> +        trapbit = extract64(trapword, bitpos, 1);
>
> -        if (nxs && (arm_hcrx_el2_eff(env) & HCRX_FGTNXS)) {
> +        if ((ri->access & ~PL3_RW) == 0) {
>              /*
> -             * If HCRX_EL2.FGTnXS is 1 then the fine-grained trap for
> -             * TLBI maintenance insns does *not* apply to the nXS variant.
> +             * EL3 cpreg -- must be FGWTE3, and FGWTE3_EL3 can only be
> +             * set from AArch64, and if the feature is enabled.
>               */
> -            trapbit = 0;
> -        } else {
> -            trapbit = extract64(trapword, bitpos, 1);
> -        }
> -        if (trapbit != rev) {
> -            res = CP_ACCESS_TRAP_EL2;
> -            goto fail;
> +            if (trapbit) {
> +                res = CP_ACCESS_TRAP_EL3;
> +                goto fail;
> +            }
> +        } else if (arm_fgt_active(env, arm_current_el(env))) {
> +            bool nxs = FIELD_EX32(ri->fgt, FGT, NXS);
> +            bool rev = FIELD_EX32(ri->fgt, FGT, REV);
> +            if (nxs && (arm_hcrx_el2_eff(env) & HCRX_FGTNXS)) {
> +                /*
> +                 * If HCRX_EL2.FGTnXS is 1 then the fine-grained trap for
> +                 * TLBI maintenance insns does *not* apply to the nXS variant.
> +                 */
> +                trapbit = 0;
> +            }
> +            if (trapbit != rev) {
> +                res = CP_ACCESS_TRAP_EL2;
> +                goto fail;
> +            }
>          }
>      }
>
> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
> index 1780490065..ff880eab8d 100644
> --- a/target/arm/tcg/translate-a64.c
> +++ b/target/arm/tcg/translate-a64.c
> @@ -2886,6 +2886,7 @@ static void handle_sys(DisasContext *s, bool isread,
>  {
>      uint32_t key = ENCODE_AA64_CP_REG(op0, op1, crn, crm, op2);
>      const ARMCPRegInfo *ri = get_arm_cp_reginfo(s->cp_regs, key);
> +    bool need_helper = false;
>      bool need_exit_tb = false;
>      bool nv_trap_to_el2 = false;
>      bool nv_redirect_reg = false;
> @@ -2999,7 +3000,17 @@ static void handle_sys(DisasContext *s, bool isread,
>          ri = redirect_cpreg(s, key, isread);
>      }
>
> -    if (ri->accessfn || (ri->fgt && s->fgt_active)) {
> +    if (ri->accessfn) {
> +        need_helper = true;
> +    } else if (ri->fgt) {

I might add a comment here:
   /*
    * EL3-only access means this must be an FGWTE3 trap (which are always
    * active); otherwise it's an FGT trap to EL2.
    */

> +        if ((ri->access & ~PL3_RW) == 0) {
> +            need_helper = (arm_dc_feature(s, ARM_FEATURE_EL3) &&
> +                           dc_isar_feature(aa64_fgwte3, s));
> +        } else {
> +            need_helper = s->fgt_active;
> +        }
> +    }
> +    if (need_helper) {
>          /* Emit code to perform further access permissions checks at
>           * runtime; this may result in an exception.
>           */

Otherwise
Reviewed-by: Peter Maydell <[email protected]>

thanks
-- PMM