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].
-- 
2.34.1

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; [0]

where pref is mte_tcf_preferred (CPU, the value set in sys /mte_tcf_preferred)
and mte_ctr comes from the process, i.e. is the value specified by the user in
the flags -- hence the default on Linux if both flags are specified is ASYNC,
not SYNC.

(just some notes for the records).

Thanks.


[0] https://github.com/torvalds/linux/blob/master/arch/arm64/kernel/mte.c#L180-L186

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