On 13/6/24 20:15, Gustavo Romero wrote:
Hi Phil,

On 6/13/24 2:35 PM, Philippe Mathieu-Daudé wrote:
On 13/6/24 19:21, 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.rom...@linaro.org>
---
  linux-user/aarch64/target_prctl.h | 22 ++-----------
  target/arm/mte.h                  | 53 +++++++++++++++++++++++++++++++
  2 files changed, 55 insertions(+), 20 deletions(-)
  create mode 100644 target/arm/mte.h


diff --git a/target/arm/mte.h b/target/arm/mte.h
new file mode 100644
index 0000000000..89712aad70
--- /dev/null
+++ b/target/arm/mte.h
@@ -0,0 +1,53 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * Copyright (c) 2024 Linaro, Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef MTE_H
+#define MTE_H
+
+#ifdef CONFIG_TCG
+#ifdef CONFIG_USER_ONLY
+#include "sys/prctl.h"
+
+static void set_mte_tcf0(CPUArchState *env, abi_long value)

Either declare it inlined (otherwise we'll get multiple symbols
declared if this header is included multiple times), or
preferably only expose the prototype.

Also I'd use the 'arm_' prefix.

Thanks, I forgot to add the inline hint and was really wondering if
I should add the arm_ prefix.

If you expose the prototype, it can be used elsewhere. Here it
will be used by linux-user code. Althought it will be used by ARM
specific code, from this other subsystem PoV it will be clearer
that this method is ARM specific if the prefix is used. But I'm
being picky and it isn't a requirement.

However the question about why do we want this method inlined remains
(usually all inlined functions need a justification).

+{
+    /*
+     * 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 /* CONFIG_TCG */
+
+#endif /* MTE_H */



Reply via email to