Factor out the code used for setting the MTE TCF0 field from the prctl
code into a convenient function. Other subsystems, like gdbstub, need to
set this field as well, so keep it as a separate function to avoid
duplication and ensure consistency in how this field is set across the
board.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
 linux-user/aarch64/target_prctl.h | 22 ++---------------
 target/arm/tcg/mte_user_helper.h  | 40 +++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 20 deletions(-)
 create mode 100644 target/arm/tcg/mte_user_helper.h

diff --git a/linux-user/aarch64/target_prctl.h b/linux-user/aarch64/target_prctl.h
index aa8e203c15..cfc8567eac 100644
--- a/linux-user/aarch64/target_prctl.h
+++ b/linux-user/aarch64/target_prctl.h
@@ -7,6 +7,7 @@
 #define AARCH64_TARGET_PRCTL_H
 
 #include "target/arm/cpu-features.h"
+#include "target/arm/tcg/mte_user_helper.h"
 
 static abi_long do_prctl_sve_get_vl(CPUArchState *env)
 {
@@ -173,26 +174,7 @@ 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)) {
-        /*
-         * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
-         *
-         * 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.
-         */
-        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);
+        arm_set_mte_tcf0(env, arg2);
 
         /*
          * Write PR_MTE_TAG to GCR_EL1[Exclude].
diff --git a/target/arm/tcg/mte_user_helper.h b/target/arm/tcg/mte_user_helper.h
new file mode 100644
index 0000000000..dee74d0923
--- /dev/null
+++ b/target/arm/tcg/mte_user_helper.h
@@ -0,0 +1,40 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef MTE_H
+#define MTE_H
+
+#ifdef CONFIG_USER_ONLY
+#include "sys/prctl.h"
+
+static inline void arm_set_mte_tcf0(CPUArchState *env, abi_long value)
+{
+    /*
+     * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
+     *
+     * 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.
+     */
+    unsigned tcf = 0;
+    if (value & PR_MTE_TCF_SYNC) {
+        tcf = 1;
+    } else if (value & PR_MTE_TCF_ASYNC) {
+        tcf = 2;
+    }
+    env->cp15.sctlr_el[1] = deposit64(env->cp15.sctlr_el[1], 38, 2, tcf);
+}
+#endif /* CONFIG_USER_ONLY */
+
+#endif /* MTE_H */
-- 
2.34.1

On 6/16/24 23:28, Gustavo Romero wrote:
> Factor out the code used for setting the MTE TCF0 field from the prctl
> code into a convenient function. Other subsystems, like gdbstub, need to
> set this field as well, so keep it as a separate function to avoid
> duplication and ensure consistency in how this field is set across the
> board.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
>   linux-user/aarch64/target_prctl.h | 22 ++---------------
>   target/arm/tcg/mte_user_helper.h  | 40 +++++++++++++++++++++++++++++++

I'm not keen on this placement, because this is specifically linux syscall semantics.

I'm not sure what the right thing to do here, because it's not like there are any other OS 
that support MTE at the moment, and gdb is inheriting linux's ptrace interface.

I think it would be less bad if we put the header in linux-user/aarch64/ and have 
target/arm/gdbstub.c include that if CONFIG_USER_ONLY & CONFIG_LINUX.


r~

Hi Richard,

On 6/21/24 1:35 AM, Richard Henderson wrote:
> On 6/16/24 23:28, Gustavo Romero wrote:
>> Factor out the code used for setting the MTE TCF0 field from the prctl
>> code into a convenient function. Other subsystems, like gdbstub, need to
>> set this field as well, so keep it as a separate function to avoid
>> duplication and ensure consistency in how this field is set across the
>> board.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> ---
>>   linux-user/aarch64/target_prctl.h | 22 ++---------------
>>   target/arm/tcg/mte_user_helper.h  | 40 +++++++++++++++++++++++++++++++
> 
> I'm not keen on this placement, because this is specifically linux syscall semantics.
> 
> I'm not sure what the right thing to do here, because it's not like there are any other OS that support MTE at the moment, and gdb is inheriting linux's ptrace interface.
> 
> I think it would be less bad if we put the header in linux-user/aarch64/ and have target/arm/gdbstub.c include that if CONFIG_USER_ONLY & CONFIG_LINUX.

I think that makes more sense indeed. Done in v4. Thanks.


Cheers,
Gustavo