| Message ID | 20240207025210.8837-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: assorted mte fixes | expand |
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
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].
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(-)