Re: [PATCH v3 1/6] linux-user/aarch64: Choose SYNC as the preferred MTE mode

[Peter Maydell]

On Wed, 7 Feb 2024 at 20:04, Gustavo Romero <gustavo.romero@linaro.org> wrote:
>
>
> On 2/6/24 11:52 PM, Richard Henderson wrote:
> > The API does not generate an error for setting ASYNC | SYNC; that merely
> > constrains the selection vs the per-cpu default.  For qemu linux-user,
> > choose SYNC as the default.
> >
> > Cc: qemu-stable@nongnu.org
> > Reported-by: Gustavo Romero <gustavo.romero@linaro.org>
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   linux-user/aarch64/target_prctl.h | 29 +++++++++++++++++------------
> >   1 file changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/linux-user/aarch64/target_prctl.h 
> > b/linux-user/aarch64/target_prctl.h
> > index 5067e7d731..aa8e203c15 100644
> > --- a/linux-user/aarch64/target_prctl.h
> > +++ b/linux-user/aarch64/target_prctl.h
> > @@ -173,21 +173,26 @@ static abi_long 
> > do_prctl_set_tagged_addr_ctrl(CPUArchState *env, abi_long arg2)
> >       env->tagged_addr_enable = arg2 & PR_TAGGED_ADDR_ENABLE;
> >
> >       if (cpu_isar_feature(aa64_mte, cpu)) {
> > -        switch (arg2 & PR_MTE_TCF_MASK) {
> > -        case PR_MTE_TCF_NONE:
> > -        case PR_MTE_TCF_SYNC:
> > -        case PR_MTE_TCF_ASYNC:
> > -            break;
> > -        default:
> > -            return -EINVAL;
> > -        }
> > -
> >           /*
> >            * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
> > -         * Note that the syscall values are consistent with hw.
> > +         *
> > +         * The kernel has a per-cpu configuration for the sysadmin,
> > +         * /sys/devices/system/cpu/cpu<N>/mte_tcf_preferred,
> > +         * which qemu does not implement.
> > +         *
> > +         * Because there is no performance difference between the modes, 
> > and
> > +         * because SYNC is most useful for debugging MTE errors, choose 
> > SYNC
> > +         * as the preferred mode.  With this preference, and the way the 
> > API
> > +         * uses only two bits, there is no way for the program to select
> > +         * ASYMM mode.
> >            */
> > -        env->cp15.sctlr_el[1] =
> > -            deposit64(env->cp15.sctlr_el[1], 38, 2, arg2 >> 
> > PR_MTE_TCF_SHIFT);
> > +        unsigned tcf = 0;
> > +        if (arg2 & PR_MTE_TCF_SYNC) {
> > +            tcf = 1;
> > +        } else if (arg2 & PR_MTE_TCF_ASYNC) {
> > +            tcf = 2;
> > +        }
> > +        env->cp15.sctlr_el[1] = deposit64(env->cp15.sctlr_el[1], 38, 2, 
> > tcf);
> >
> >           /*
> >            * Write PR_MTE_TAG to GCR_EL1[Exclude].
> >
>
> ok, so no ASYMM in QEMU user-mode, plus if both SYNC and ASYNC flags are
> specified by the user SYNC is selected. Contrary to what happens by default
> on Linux, because of the mte_tcf_preferred value, which is ASYNC, and the
> final value selected is define by:
>
> resolved_mte_tcf = (mte_ctrl & pref) ? pref : mte_ctrl;

Yes; the kernel default mte_tcf_preferred is 'async', but as the comment
notes we do the equivalent of the sysadmin having set mte_tcf_preferred
to 'sync'. (The idea of the tunable is that you can set it to whatever
is a good balance between performance and debugging precision depending
on what your CPU implementation happens to be; for QEMU since there is
no performance gain from 'async' or 'asymm' we might as well act like
the sysadmin set the tunable to 'sync'.)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM