[PATCH v1 0/4] arm64/mte: Asymmetric MTE support in userspace

[PATCH v1 0/4] arm64/mte: Asymmetric MTE support in userspace

[Mark Brown]

MTE3 adds a new mode which is synchronous for writes but asynchronous for
reads. Currently we make use of this within the kernel but do not make
it available for userspace, this series extends the existing userspace
ABI to allow that to happen.

There's no updates to the kselftests for MTE here, I have some work in
progress there but there's some overlap with Joey's work extending the
tests to cover some additional cases and there's some fairly extensive
assumptions that need to be unpicked about only being able to generate
one type of fault at once. I'll clean that up and post it later.

Mark Brown (4):
  arm64/mte: Document ABI for asymmetric mode
  arm64/mte: Add a little bit of documentation for
    mte_update_sctlr_user()
  arm64/mte: Add hwcap for asymmetric mode
  arm64/mte: Add userspace interface for enabling asymmetric mode

 Documentation/arm64/elf_hwcaps.rst            |  5 ++++
 .../arm64/memory-tagging-extension.rst        | 12 +++++----
 arch/arm64/include/asm/hwcap.h                |  1 +
 arch/arm64/include/asm/processor.h            |  1 +
 arch/arm64/include/uapi/asm/hwcap.h           |  1 +
 arch/arm64/kernel/cpufeature.c                |  1 +
 arch/arm64/kernel/cpuinfo.c                   |  1 +
 arch/arm64/kernel/mte.c                       | 26 ++++++++++++++++++-
 arch/arm64/kernel/process.c                   |  5 +++-
 include/uapi/linux/prctl.h                    |  4 ++-
 10 files changed, 49 insertions(+), 8 deletions(-)


base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 1/4] arm64/mte: Document ABI for asymmetric mode

[Mark Brown]

MTE3 adds a new mode which is synchronous for writes but asynchronous for
reads. Document the userspace ABI for this feature, we call the new mode
ASYMM and add a new prctl flag and mte_tcf_preferred value for it.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/arm64/memory-tagging-extension.rst | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/arm64/memory-tagging-extension.rst b/Documentation/arm64/memory-tagging-extension.rst
index 7b99c8f428eb..5218b838d062 100644
--- a/Documentation/arm64/memory-tagging-extension.rst
+++ b/Documentation/arm64/memory-tagging-extension.rst
@@ -76,6 +76,9 @@ configurable behaviours:
   with ``.si_code = SEGV_MTEAERR`` and ``.si_addr = 0`` (the faulting
   address is unknown).
 
+- *Asymmetric* - Reads are handled as for synchronous mode while writes
+  are handled as for asynchronous mode.
+
 The user can select the above modes, per thread, using the
 ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call where ``flags``
 contains any number of the following values in the ``PR_MTE_TCF_MASK``
@@ -85,6 +88,7 @@ bit-field:
                          (ignored if combined with other options)
 - ``PR_MTE_TCF_SYNC``  - *Synchronous* tag check fault mode
 - ``PR_MTE_TCF_ASYNC`` - *Asynchronous* tag check fault mode
+- ``PR_MTE_TCF_ASYMM`` - *Asymmetric* tag check fault mode
 
 If no modes are specified, tag check faults are ignored. If a single
 mode is specified, the program will run in that mode. If multiple
@@ -139,16 +143,14 @@ tag checking mode as the CPU's preferred tag checking mode.
 
 The preferred tag checking mode for each CPU is controlled by
 ``/sys/devices/system/cpu/cpu<N>/mte_tcf_preferred``, to which a
-privileged user may write the value ``async`` or ``sync``.  The default
-preferred mode for each CPU is ``async``.
+privileged user may write the value ``async``, ``sync`` or ``asymm``.  The
+default preferred mode for each CPU is ``async``.
 
 To allow a program to potentially run in the CPU's preferred tag
 checking mode, the user program may set multiple tag check fault mode
 bits in the ``flags`` argument to the ``prctl(PR_SET_TAGGED_ADDR_CTRL,
 flags, 0, 0, 0)`` system call. If the CPU's preferred tag checking
-mode is in the task's set of provided tag checking modes (this will
-always be the case at present because the kernel only supports two
-tag checking modes, but future kernels may support more modes), that
+mode is in the task's set of provided tag checking modes, that
 mode will be selected. Otherwise, one of the modes in the task's mode
 set will be selected in a currently unspecified manner.
 
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 2/4] arm64/mte: Add a little bit of documentation for mte_update_sctlr_user()

[Mark Brown]

The code isn't that obscure but it probably won't hurt to have a little
bit more documentation for anyone trying to find out where everything
actually takes effect.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/mte.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index f418ebc65f95..fa4001fee12a 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -186,6 +186,11 @@ void mte_check_tfsr_el1(void)
 }
 #endif
 
+/*
+ * This is where we actually resolve the system and process MTE mode
+ * configuration into an actual value in SCTLR_EL1 that affects
+ * userspace.
+ */
 static void mte_update_sctlr_user(struct task_struct *task)
 {
 	/*
@@ -199,8 +204,17 @@ static void mte_update_sctlr_user(struct task_struct *task)
 	unsigned long pref, resolved_mte_tcf;
 
 	pref = __this_cpu_read(mte_tcf_preferred);
+	/*
+	 * If there is no overlap between the system preferred and
+	 * program requested values go with what was requested.
+	 */
 	resolved_mte_tcf = (mte_ctrl & pref) ? pref : mte_ctrl;
 	sctlr &= ~SCTLR_EL1_TCF0_MASK;
+	/*
+	 * Pick an actual setting. The order in which we check for
+	 * set bits and map into register values determines our
+	 * default order.
+	 */
 	if (resolved_mte_tcf & MTE_CTRL_TCF_ASYNC)
 		sctlr |= SCTLR_EL1_TCF0_ASYNC;
 	else if (resolved_mte_tcf & MTE_CTRL_TCF_SYNC)
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 3/4] arm64/mte: Add hwcap for asymmetric mode

[Mark Brown]

Allow userspace to detect support for asymmetric mode by providing a hwcap
for it, using the official feature name FEAT_MTE3.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/arm64/elf_hwcaps.rst  | 5 +++++
 arch/arm64/include/asm/hwcap.h      | 1 +
 arch/arm64/include/uapi/asm/hwcap.h | 1 +
 arch/arm64/kernel/cpufeature.c      | 1 +
 arch/arm64/kernel/cpuinfo.c         | 1 +
 5 files changed, 9 insertions(+)

diff --git a/Documentation/arm64/elf_hwcaps.rst b/Documentation/arm64/elf_hwcaps.rst
index b72ff17d600a..a8f30963e550 100644
--- a/Documentation/arm64/elf_hwcaps.rst
+++ b/Documentation/arm64/elf_hwcaps.rst
@@ -259,6 +259,11 @@ HWCAP2_RPRES
 
     Functionality implied by ID_AA64ISAR2_EL1.RPRES == 0b0001.
 
+HWCAP2_MTE3
+
+    Functionality implied by ID_AA64PFR1_EL1.MTE == 0b0011, as described
+    by Documentation/arm64/memory-tagging-extension.rst.
+
 4. Unused AT_HWCAP bits
 -----------------------
 
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index f68fbb207473..8db5ec0089db 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -108,6 +108,7 @@
 #define KERNEL_HWCAP_ECV		__khwcap2_feature(ECV)
 #define KERNEL_HWCAP_AFP		__khwcap2_feature(AFP)
 #define KERNEL_HWCAP_RPRES		__khwcap2_feature(RPRES)
+#define KERNEL_HWCAP_MTE3		__khwcap2_feature(MTE3)
 
 /*
  * This yields a mask that user programs can use to figure out what
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index f03731847d9d..99cb5d383048 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -78,5 +78,6 @@
 #define HWCAP2_ECV		(1 << 19)
 #define HWCAP2_AFP		(1 << 20)
 #define HWCAP2_RPRES		(1 << 21)
+#define HWCAP2_MTE3		(1 << 22)
 
 #endif /* _UAPI__ASM_HWCAP_H */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index a46ab3b1c4d5..57de3f3e0518 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2485,6 +2485,7 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
 #endif
 #ifdef CONFIG_ARM64_MTE
 	HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_MTE_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_MTE, CAP_HWCAP, KERNEL_HWCAP_MTE),
+	HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_MTE_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_MTE_ASYMM, CAP_HWCAP, KERNEL_HWCAP_MTE3),
 #endif /* CONFIG_ARM64_MTE */
 	HWCAP_CAP(SYS_ID_AA64MMFR0_EL1, ID_AA64MMFR0_ECV_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ECV),
 	HWCAP_CAP(SYS_ID_AA64MMFR1_EL1, ID_AA64MMFR1_AFP_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_AFP),
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 591c18a889a5..330b92ea863a 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -97,6 +97,7 @@ static const char *const hwcap_str[] = {
 	[KERNEL_HWCAP_ECV]		= "ecv",
 	[KERNEL_HWCAP_AFP]		= "afp",
 	[KERNEL_HWCAP_RPRES]		= "rpres",
+	[KERNEL_HWCAP_MTE3]		= "mte3",
 };
 
 #ifdef CONFIG_COMPAT
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Mark Brown]

The architecture provides an asymmetric mode for MTE where tag mismatches
are checked asynchronously for reads but synchronously for loads. Allow
userspace processes to select this and make it available as a default mode
via the existing per-CPU sysfs interface.

Since there PR_MTE_TCF_ values are a bitmask (allowing the kernel to choose
between the multiple modes) and there are no free bits adjacent to the
existing PR_MTE_TCF_ bits the set of bits used to specify the mode becomes
disjoint. Programs using the new interface should be aware of this and
programs that do not use it will not see any change in behaviour.

When userspace requests two possible modes but the system default for the
CPU is the third mode (eg, default is synchronous but userspace requests
either asynchronous or asymmetric) the preference order is:

   ASYMM > ASYNC > SYNC

This situation is not currently possible since there are only two modes and
it is mandatory to have a system default so there could be no ambiguity and
there is no ABI change. The chosen order is basically arbitrary as we do not
have a clear metric for what is better here.

If userspace requests specifically asymmetric mode via the prctl() and the
system does not support it then we will return an error, this mirrors
how we handle the case where userspace enables MTE on a system that does
not support MTE at all and the behaviour that will be seen if running on
an older kernel that does not support userspace use of asymmetric mode.

Attempts to set asymmetric mode as the default mode will result in an error
if the system does not support it.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/processor.h |  1 +
 arch/arm64/kernel/mte.c            | 12 +++++++++++-
 arch/arm64/kernel/process.c        |  5 ++++-
 include/uapi/linux/prctl.h         |  4 +++-
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 6f41b65f9962..73e38d9a540c 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -21,6 +21,7 @@
 
 #define MTE_CTRL_TCF_SYNC		(1UL << 16)
 #define MTE_CTRL_TCF_ASYNC		(1UL << 17)
+#define MTE_CTRL_TCF_ASYMM		(1UL << 18)
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index fa4001fee12a..fb777d8fea32 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -215,7 +215,9 @@ static void mte_update_sctlr_user(struct task_struct *task)
 	 * set bits and map into register values determines our
 	 * default order.
 	 */
-	if (resolved_mte_tcf & MTE_CTRL_TCF_ASYNC)
+	if (resolved_mte_tcf & MTE_CTRL_TCF_ASYMM)
+		sctlr |= SCTLR_EL1_TCF0_ASYMM;
+	else if (resolved_mte_tcf & MTE_CTRL_TCF_ASYNC)
 		sctlr |= SCTLR_EL1_TCF0_ASYNC;
 	else if (resolved_mte_tcf & MTE_CTRL_TCF_SYNC)
 		sctlr |= SCTLR_EL1_TCF0_SYNC;
@@ -306,6 +308,8 @@ long set_mte_ctrl(struct task_struct *task, unsigned long arg)
 		mte_ctrl |= MTE_CTRL_TCF_ASYNC;
 	if (arg & PR_MTE_TCF_SYNC)
 		mte_ctrl |= MTE_CTRL_TCF_SYNC;
+	if (arg & PR_MTE_TCF_ASYMM)
+		mte_ctrl |= MTE_CTRL_TCF_ASYMM;
 
 	task->thread.mte_ctrl = mte_ctrl;
 	if (task == current) {
@@ -334,6 +338,8 @@ long get_mte_ctrl(struct task_struct *task)
 		ret |= PR_MTE_TCF_ASYNC;
 	if (mte_ctrl & MTE_CTRL_TCF_SYNC)
 		ret |= PR_MTE_TCF_SYNC;
+	if (mte_ctrl & MTE_CTRL_TCF_ASYMM)
+		ret |= PR_MTE_TCF_ASYMM;
 
 	return ret;
 }
@@ -481,6 +487,8 @@ static ssize_t mte_tcf_preferred_show(struct device *dev,
 		return sysfs_emit(buf, "async\n");
 	case MTE_CTRL_TCF_SYNC:
 		return sysfs_emit(buf, "sync\n");
+	case MTE_CTRL_TCF_ASYMM:
+		return sysfs_emit(buf, "asymm\n");
 	default:
 		return sysfs_emit(buf, "???\n");
 	}
@@ -496,6 +504,8 @@ static ssize_t mte_tcf_preferred_store(struct device *dev,
 		tcf = MTE_CTRL_TCF_ASYNC;
 	else if (sysfs_streq(buf, "sync"))
 		tcf = MTE_CTRL_TCF_SYNC;
+	else if (cpus_have_cap(ARM64_MTE_ASYMM) && sysfs_streq(buf, "asymm"))
+		tcf = MTE_CTRL_TCF_ASYMM;
 	else
 		return -EINVAL;
 
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 5369e649fa79..941cfa7117b9 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -635,7 +635,10 @@ long set_tagged_addr_ctrl(struct task_struct *task, unsigned long arg)
 		return -EINVAL;
 
 	if (system_supports_mte())
-		valid_mask |= PR_MTE_TCF_MASK | PR_MTE_TAG_MASK;
+		valid_mask |= PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC \
+			| PR_MTE_TAG_MASK;
+	if (cpus_have_cap(ARM64_MTE_ASYMM))
+		valid_mask |= PR_MTE_TCF_ASYMM;
 
 	if (arg & ~valid_mask)
 		return -EINVAL;
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index e998764f0262..4ae2b21e4066 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -238,7 +238,9 @@ struct prctl_mm_map {
 # define PR_MTE_TCF_NONE		0UL
 # define PR_MTE_TCF_SYNC		(1UL << 1)
 # define PR_MTE_TCF_ASYNC		(1UL << 2)
-# define PR_MTE_TCF_MASK		(PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC)
+# define PR_MTE_TCF_ASYMM		(1UL << 19)
+# define PR_MTE_TCF_MASK		(PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC | \
+					 PR_MTE_TCF_ASYMM)
 /* MTE tag inclusion mask */
 # define PR_MTE_TAG_SHIFT		3
 # define PR_MTE_TAG_MASK		(0xffffUL << PR_MTE_TAG_SHIFT)
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/4] arm64/mte: Document ABI for asymmetric mode

[Catalin Marinas]

On Thu, Jan 27, 2022 at 07:57:09PM +0000, Mark Brown wrote:
> MTE3 adds a new mode which is synchronous for writes but asynchronous for
> reads.

It's the other way around: synchronous for reads, async for writes. You
got it correctly in the doc.

> Document the userspace ABI for this feature, we call the new mode
> ASYMM and add a new prctl flag and mte_tcf_preferred value for it.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  Documentation/arm64/memory-tagging-extension.rst | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/arm64/memory-tagging-extension.rst b/Documentation/arm64/memory-tagging-extension.rst
> index 7b99c8f428eb..5218b838d062 100644
> --- a/Documentation/arm64/memory-tagging-extension.rst
> +++ b/Documentation/arm64/memory-tagging-extension.rst
> @@ -76,6 +76,9 @@ configurable behaviours:
>    with ``.si_code = SEGV_MTEAERR`` and ``.si_addr = 0`` (the faulting
>    address is unknown).
>  
> +- *Asymmetric* - Reads are handled as for synchronous mode while writes
> +  are handled as for asynchronous mode.

That's correct.

> +
>  The user can select the above modes, per thread, using the
>  ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call where ``flags``
>  contains any number of the following values in the ``PR_MTE_TCF_MASK``
> @@ -85,6 +88,7 @@ bit-field:
>                           (ignored if combined with other options)
>  - ``PR_MTE_TCF_SYNC``  - *Synchronous* tag check fault mode
>  - ``PR_MTE_TCF_ASYNC`` - *Asynchronous* tag check fault mode
> +- ``PR_MTE_TCF_ASYMM`` - *Asymmetric* tag check fault mode
>  
>  If no modes are specified, tag check faults are ignored. If a single
>  mode is specified, the program will run in that mode. If multiple
> @@ -139,16 +143,14 @@ tag checking mode as the CPU's preferred tag checking mode.
>  
>  The preferred tag checking mode for each CPU is controlled by
>  ``/sys/devices/system/cpu/cpu<N>/mte_tcf_preferred``, to which a
> -privileged user may write the value ``async`` or ``sync``.  The default
> -preferred mode for each CPU is ``async``.
> +privileged user may write the value ``async``, ``sync`` or ``asymm``.  The
> +default preferred mode for each CPU is ``async``.

That's fine by me. It can be configured at boot for each CPU.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 2/4] arm64/mte: Add a little bit of documentation for mte_update_sctlr_user()

[Catalin Marinas]

On Thu, Jan 27, 2022 at 07:57:10PM +0000, Mark Brown wrote:
> The code isn't that obscure but it probably won't hurt to have a little
> bit more documentation for anyone trying to find out where everything
> actually takes effect.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 3/4] arm64/mte: Add hwcap for asymmetric mode

[Catalin Marinas]

On Thu, Jan 27, 2022 at 07:57:11PM +0000, Mark Brown wrote:
> diff --git a/Documentation/arm64/elf_hwcaps.rst b/Documentation/arm64/elf_hwcaps.rst
> index b72ff17d600a..a8f30963e550 100644
> --- a/Documentation/arm64/elf_hwcaps.rst
> +++ b/Documentation/arm64/elf_hwcaps.rst
> @@ -259,6 +259,11 @@ HWCAP2_RPRES
>  
>      Functionality implied by ID_AA64ISAR2_EL1.RPRES == 0b0001.
>  
> +HWCAP2_MTE3
> +
> +    Functionality implied by ID_AA64PFR1_EL1.MTE == 0b0011, as described
> +    by Documentation/arm64/memory-tagging-extension.rst.

In hindsight, we should have called the other HWCAP2_MTE2 but since we
decided not to expose MTE == 0b0001 to user, we removed the '2'. Anyway,
I'm fine with HWCAP2_MTE3.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/4] arm64/mte: Document ABI for asymmetric mode

[Vincenzo Frascino]

Hi Mark,

On 1/27/22 7:57 PM, Mark Brown wrote:
> MTE3 adds a new mode which is synchronous for writes but asynchronous for
> reads.

MTE3 (Asymmetric Mode) is synchronous for reads and asynchronous for writes.

> Document the userspace ABI for this feature, we call the new mode
> ASYMM and add a new prctl flag and mte_tcf_preferred value for it.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

A part that:

Reviewed-by: Vincenzo Frascino <Vincenzo.Frascino@arm.com>

> ---
>  Documentation/arm64/memory-tagging-extension.rst | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/arm64/memory-tagging-extension.rst b/Documentation/arm64/memory-tagging-extension.rst
> index 7b99c8f428eb..5218b838d062 100644
> --- a/Documentation/arm64/memory-tagging-extension.rst
> +++ b/Documentation/arm64/memory-tagging-extension.rst
> @@ -76,6 +76,9 @@ configurable behaviours:
>    with ``.si_code = SEGV_MTEAERR`` and ``.si_addr = 0`` (the faulting
>    address is unknown).
>  
> +- *Asymmetric* - Reads are handled as for synchronous mode while writes
> +  are handled as for asynchronous mode.
> +
>  The user can select the above modes, per thread, using the
>  ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call where ``flags``
>  contains any number of the following values in the ``PR_MTE_TCF_MASK``
> @@ -85,6 +88,7 @@ bit-field:
>                           (ignored if combined with other options)
>  - ``PR_MTE_TCF_SYNC``  - *Synchronous* tag check fault mode
>  - ``PR_MTE_TCF_ASYNC`` - *Asynchronous* tag check fault mode
> +- ``PR_MTE_TCF_ASYMM`` - *Asymmetric* tag check fault mode
>  
>  If no modes are specified, tag check faults are ignored. If a single
>  mode is specified, the program will run in that mode. If multiple
> @@ -139,16 +143,14 @@ tag checking mode as the CPU's preferred tag checking mode.
>  
>  The preferred tag checking mode for each CPU is controlled by
>  ``/sys/devices/system/cpu/cpu<N>/mte_tcf_preferred``, to which a
> -privileged user may write the value ``async`` or ``sync``.  The default
> -preferred mode for each CPU is ``async``.
> +privileged user may write the value ``async``, ``sync`` or ``asymm``.  The
> +default preferred mode for each CPU is ``async``.
>  
>  To allow a program to potentially run in the CPU's preferred tag
>  checking mode, the user program may set multiple tag check fault mode
>  bits in the ``flags`` argument to the ``prctl(PR_SET_TAGGED_ADDR_CTRL,
>  flags, 0, 0, 0)`` system call. If the CPU's preferred tag checking
> -mode is in the task's set of provided tag checking modes (this will
> -always be the case at present because the kernel only supports two
> -tag checking modes, but future kernels may support more modes), that
> +mode is in the task's set of provided tag checking modes, that
>  mode will be selected. Otherwise, one of the modes in the task's mode
>  set will be selected in a currently unspecified manner.
>  
> 

-- 
Regards,
Vincenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 2/4] arm64/mte: Add a little bit of documentation for mte_update_sctlr_user()

[Vincenzo Frascino]

On 1/27/22 7:57 PM, Mark Brown wrote:
> The code isn't that obscure but it probably won't hurt to have a little
> bit more documentation for anyone trying to find out where everything
> actually takes effect.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Vincenzo Frascino <Vincenzo.Frascino@arm.com>

> ---
>  arch/arm64/kernel/mte.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index f418ebc65f95..fa4001fee12a 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -186,6 +186,11 @@ void mte_check_tfsr_el1(void)
>  }
>  #endif
>  
> +/*
> + * This is where we actually resolve the system and process MTE mode
> + * configuration into an actual value in SCTLR_EL1 that affects
> + * userspace.
> + */
>  static void mte_update_sctlr_user(struct task_struct *task)
>  {
>  	/*
> @@ -199,8 +204,17 @@ static void mte_update_sctlr_user(struct task_struct *task)
>  	unsigned long pref, resolved_mte_tcf;
>  
>  	pref = __this_cpu_read(mte_tcf_preferred);
> +	/*
> +	 * If there is no overlap between the system preferred and
> +	 * program requested values go with what was requested.
> +	 */
>  	resolved_mte_tcf = (mte_ctrl & pref) ? pref : mte_ctrl;
>  	sctlr &= ~SCTLR_EL1_TCF0_MASK;
> +	/*
> +	 * Pick an actual setting. The order in which we check for
> +	 * set bits and map into register values determines our
> +	 * default order.
> +	 */
>  	if (resolved_mte_tcf & MTE_CTRL_TCF_ASYNC)
>  		sctlr |= SCTLR_EL1_TCF0_ASYNC;
>  	else if (resolved_mte_tcf & MTE_CTRL_TCF_SYNC)
> 

-- 
Regards,
Vincenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 3/4] arm64/mte: Add hwcap for asymmetric mode

[Vincenzo Frascino]

On 1/27/22 7:57 PM, Mark Brown wrote:
> Allow userspace to detect support for asymmetric mode by providing a hwcap
> for it, using the official feature name FEAT_MTE3.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Vincenzo Frascino <Vincenzo.Frascino@arm.com>

> ---
>  Documentation/arm64/elf_hwcaps.rst  | 5 +++++
>  arch/arm64/include/asm/hwcap.h      | 1 +
>  arch/arm64/include/uapi/asm/hwcap.h | 1 +
>  arch/arm64/kernel/cpufeature.c      | 1 +
>  arch/arm64/kernel/cpuinfo.c         | 1 +
>  5 files changed, 9 insertions(+)
> 
> diff --git a/Documentation/arm64/elf_hwcaps.rst b/Documentation/arm64/elf_hwcaps.rst
> index b72ff17d600a..a8f30963e550 100644
> --- a/Documentation/arm64/elf_hwcaps.rst
> +++ b/Documentation/arm64/elf_hwcaps.rst
> @@ -259,6 +259,11 @@ HWCAP2_RPRES
>  
>      Functionality implied by ID_AA64ISAR2_EL1.RPRES == 0b0001.
>  
> +HWCAP2_MTE3
> +
> +    Functionality implied by ID_AA64PFR1_EL1.MTE == 0b0011, as described
> +    by Documentation/arm64/memory-tagging-extension.rst.
> +
>  4. Unused AT_HWCAP bits
>  -----------------------
>  
> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> index f68fbb207473..8db5ec0089db 100644
> --- a/arch/arm64/include/asm/hwcap.h
> +++ b/arch/arm64/include/asm/hwcap.h
> @@ -108,6 +108,7 @@
>  #define KERNEL_HWCAP_ECV		__khwcap2_feature(ECV)
>  #define KERNEL_HWCAP_AFP		__khwcap2_feature(AFP)
>  #define KERNEL_HWCAP_RPRES		__khwcap2_feature(RPRES)
> +#define KERNEL_HWCAP_MTE3		__khwcap2_feature(MTE3)
>  
>  /*
>   * This yields a mask that user programs can use to figure out what
> diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
> index f03731847d9d..99cb5d383048 100644
> --- a/arch/arm64/include/uapi/asm/hwcap.h
> +++ b/arch/arm64/include/uapi/asm/hwcap.h
> @@ -78,5 +78,6 @@
>  #define HWCAP2_ECV		(1 << 19)
>  #define HWCAP2_AFP		(1 << 20)
>  #define HWCAP2_RPRES		(1 << 21)
> +#define HWCAP2_MTE3		(1 << 22)
>  
>  #endif /* _UAPI__ASM_HWCAP_H */
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index a46ab3b1c4d5..57de3f3e0518 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2485,6 +2485,7 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
>  #endif
>  #ifdef CONFIG_ARM64_MTE
>  	HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_MTE_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_MTE, CAP_HWCAP, KERNEL_HWCAP_MTE),
> +	HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_MTE_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_MTE_ASYMM, CAP_HWCAP, KERNEL_HWCAP_MTE3),
>  #endif /* CONFIG_ARM64_MTE */
>  	HWCAP_CAP(SYS_ID_AA64MMFR0_EL1, ID_AA64MMFR0_ECV_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ECV),
>  	HWCAP_CAP(SYS_ID_AA64MMFR1_EL1, ID_AA64MMFR1_AFP_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_AFP),
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index 591c18a889a5..330b92ea863a 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -97,6 +97,7 @@ static const char *const hwcap_str[] = {
>  	[KERNEL_HWCAP_ECV]		= "ecv",
>  	[KERNEL_HWCAP_AFP]		= "afp",
>  	[KERNEL_HWCAP_RPRES]		= "rpres",
> +	[KERNEL_HWCAP_MTE3]		= "mte3",
>  };
>  
>  #ifdef CONFIG_COMPAT
> 

-- 
Regards,
Vincenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Vincenzo Frascino]

On 1/27/22 7:57 PM, Mark Brown wrote:
> The architecture provides an asymmetric mode for MTE where tag mismatches
> are checked asynchronously for reads but synchronously for loads. 

MTE3 checks synchronously the reads and asynchronously the writes.

Nit: Please use load/store or read/write.


> Allow userspace processes to select this and make it available as a default mode
> via the existing per-CPU sysfs interface.
> 
> Since there PR_MTE_TCF_ values are a bitmask (allowing the kernel to choose
> between the multiple modes) and there are no free bits adjacent to the
> existing PR_MTE_TCF_ bits the set of bits used to specify the mode becomes
> disjoint. Programs using the new interface should be aware of this and
> programs that do not use it will not see any change in behaviour.
> > When userspace requests two possible modes but the system default for the
> CPU is the third mode (eg, default is synchronous but userspace requests
> either asynchronous or asymmetric) the preference order is:
> 
>    ASYMM > ASYNC > SYNC
> 
> This situation is not currently possible since there are only two modes and
> it is mandatory to have a system default so there could be no ambiguity and
> there is no ABI change. The chosen order is basically arbitrary as we do not
> have a clear metric for what is better here.
> 
> If userspace requests specifically asymmetric mode via the prctl() and the
> system does not support it then we will return an error, this mirrors
> how we handle the case where userspace enables MTE on a system that does
> not support MTE at all and the behaviour that will be seen if running on
> an older kernel that does not support userspace use of asymmetric mode.
> 
> Attempts to set asymmetric mode as the default mode will result in an error
> if the system does not support it.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Otherwise:

Reviewed-by: Vincenzo Frascino <Vincenzo.Frascino@arm.com>

> ---
>  arch/arm64/include/asm/processor.h |  1 +
>  arch/arm64/kernel/mte.c            | 12 +++++++++++-
>  arch/arm64/kernel/process.c        |  5 ++++-
>  include/uapi/linux/prctl.h         |  4 +++-
>  4 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index 6f41b65f9962..73e38d9a540c 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -21,6 +21,7 @@
>  
>  #define MTE_CTRL_TCF_SYNC		(1UL << 16)
>  #define MTE_CTRL_TCF_ASYNC		(1UL << 17)
> +#define MTE_CTRL_TCF_ASYMM		(1UL << 18)
>  
>  #ifndef __ASSEMBLY__
>  
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index fa4001fee12a..fb777d8fea32 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -215,7 +215,9 @@ static void mte_update_sctlr_user(struct task_struct *task)
>  	 * set bits and map into register values determines our
>  	 * default order.
>  	 */
> -	if (resolved_mte_tcf & MTE_CTRL_TCF_ASYNC)
> +	if (resolved_mte_tcf & MTE_CTRL_TCF_ASYMM)
> +		sctlr |= SCTLR_EL1_TCF0_ASYMM;
> +	else if (resolved_mte_tcf & MTE_CTRL_TCF_ASYNC)
>  		sctlr |= SCTLR_EL1_TCF0_ASYNC;
>  	else if (resolved_mte_tcf & MTE_CTRL_TCF_SYNC)
>  		sctlr |= SCTLR_EL1_TCF0_SYNC;
> @@ -306,6 +308,8 @@ long set_mte_ctrl(struct task_struct *task, unsigned long arg)
>  		mte_ctrl |= MTE_CTRL_TCF_ASYNC;
>  	if (arg & PR_MTE_TCF_SYNC)
>  		mte_ctrl |= MTE_CTRL_TCF_SYNC;
> +	if (arg & PR_MTE_TCF_ASYMM)
> +		mte_ctrl |= MTE_CTRL_TCF_ASYMM;
>  
>  	task->thread.mte_ctrl = mte_ctrl;
>  	if (task == current) {
> @@ -334,6 +338,8 @@ long get_mte_ctrl(struct task_struct *task)
>  		ret |= PR_MTE_TCF_ASYNC;
>  	if (mte_ctrl & MTE_CTRL_TCF_SYNC)
>  		ret |= PR_MTE_TCF_SYNC;
> +	if (mte_ctrl & MTE_CTRL_TCF_ASYMM)
> +		ret |= PR_MTE_TCF_ASYMM;
>  
>  	return ret;
>  }
> @@ -481,6 +487,8 @@ static ssize_t mte_tcf_preferred_show(struct device *dev,
>  		return sysfs_emit(buf, "async\n");
>  	case MTE_CTRL_TCF_SYNC:
>  		return sysfs_emit(buf, "sync\n");
> +	case MTE_CTRL_TCF_ASYMM:
> +		return sysfs_emit(buf, "asymm\n");
>  	default:
>  		return sysfs_emit(buf, "???\n");
>  	}
> @@ -496,6 +504,8 @@ static ssize_t mte_tcf_preferred_store(struct device *dev,
>  		tcf = MTE_CTRL_TCF_ASYNC;
>  	else if (sysfs_streq(buf, "sync"))
>  		tcf = MTE_CTRL_TCF_SYNC;
> +	else if (cpus_have_cap(ARM64_MTE_ASYMM) && sysfs_streq(buf, "asymm"))
> +		tcf = MTE_CTRL_TCF_ASYMM;
>  	else
>  		return -EINVAL;
>  
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 5369e649fa79..941cfa7117b9 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -635,7 +635,10 @@ long set_tagged_addr_ctrl(struct task_struct *task, unsigned long arg)
>  		return -EINVAL;
>  
>  	if (system_supports_mte())
> -		valid_mask |= PR_MTE_TCF_MASK | PR_MTE_TAG_MASK;
> +		valid_mask |= PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC \
> +			| PR_MTE_TAG_MASK;
> +	if (cpus_have_cap(ARM64_MTE_ASYMM))
> +		valid_mask |= PR_MTE_TCF_ASYMM;
>  
>  	if (arg & ~valid_mask)
>  		return -EINVAL;
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index e998764f0262..4ae2b21e4066 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -238,7 +238,9 @@ struct prctl_mm_map {
>  # define PR_MTE_TCF_NONE		0UL
>  # define PR_MTE_TCF_SYNC		(1UL << 1)
>  # define PR_MTE_TCF_ASYNC		(1UL << 2)
> -# define PR_MTE_TCF_MASK		(PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC)
> +# define PR_MTE_TCF_ASYMM		(1UL << 19)
> +# define PR_MTE_TCF_MASK		(PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC | \
> +					 PR_MTE_TCF_ASYMM)
>  /* MTE tag inclusion mask */
>  # define PR_MTE_TAG_SHIFT		3
>  # define PR_MTE_TAG_MASK		(0xffffUL << PR_MTE_TAG_SHIFT)
> 

-- 
Regards,
Vincenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Catalin Marinas]

On Thu, Jan 27, 2022 at 07:57:12PM +0000, Mark Brown wrote:
> The architecture provides an asymmetric mode for MTE where tag mismatches
> are checked asynchronously for reads but synchronously for loads. Allow
> userspace processes to select this and make it available as a default mode
> via the existing per-CPU sysfs interface.
> 
> Since there PR_MTE_TCF_ values are a bitmask (allowing the kernel to choose
> between the multiple modes) and there are no free bits adjacent to the
> existing PR_MTE_TCF_ bits the set of bits used to specify the mode becomes
> disjoint. Programs using the new interface should be aware of this and
> programs that do not use it will not see any change in behaviour.
> 
> When userspace requests two possible modes but the system default for the
> CPU is the third mode (eg, default is synchronous but userspace requests
> either asynchronous or asymmetric) the preference order is:
> 
>    ASYMM > ASYNC > SYNC
> 
> This situation is not currently possible since there are only two modes and
> it is mandatory to have a system default so there could be no ambiguity and
> there is no ABI change. The chosen order is basically arbitrary as we do not
> have a clear metric for what is better here.
> 
> If userspace requests specifically asymmetric mode via the prctl() and the
> system does not support it then we will return an error, this mirrors
> how we handle the case where userspace enables MTE on a system that does
> not support MTE at all and the behaviour that will be seen if running on
> an older kernel that does not support userspace use of asymmetric mode.
> 
> Attempts to set asymmetric mode as the default mode will result in an error
> if the system does not support it.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/4] arm64/mte: Document ABI for asymmetric mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 981 bytes --]

On Fri, Jan 28, 2022 at 04:43:21PM +0000, Catalin Marinas wrote:
> On Thu, Jan 27, 2022 at 07:57:09PM +0000, Mark Brown wrote:

> > MTE3 adds a new mode which is synchronous for writes but asynchronous for
> > reads.

> It's the other way around: synchronous for reads, async for writes. You
> got it correctly in the doc.

Doh.  See, I know that the way to get fast review is to make some
obvious error which someone can point out :P

> >  The preferred tag checking mode for each CPU is controlled by
> >  ``/sys/devices/system/cpu/cpu<N>/mte_tcf_preferred``, to which a
> > -privileged user may write the value ``async`` or ``sync``.  The default
> > -preferred mode for each CPU is ``async``.
> > +privileged user may write the value ``async``, ``sync`` or ``asymm``.  The
> > +default preferred mode for each CPU is ``async``.
> 
> That's fine by me. It can be configured at boot for each CPU.
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 0/4] arm64/mte: Asymmetric MTE support in userspace

[Branislav Rankov]

Tested-by: Branislav Rankov <branislav.rankov@arm.com>

On 1/27/22 19:57, Mark Brown wrote:
> MTE3 adds a new mode which is synchronous for writes but asynchronous for
> reads. Currently we make use of this within the kernel but do not make
> it available for userspace, this series extends the existing userspace
> ABI to allow that to happen.
> 
> There's no updates to the kselftests for MTE here, I have some work in
> progress there but there's some overlap with Joey's work extending the
> tests to cover some additional cases and there's some fairly extensive
> assumptions that need to be unpicked about only being able to generate
> one type of fault at once. I'll clean that up and post it later.
> 
> Mark Brown (4):
>   arm64/mte: Document ABI for asymmetric mode
>   arm64/mte: Add a little bit of documentation for
>     mte_update_sctlr_user()
>   arm64/mte: Add hwcap for asymmetric mode
>   arm64/mte: Add userspace interface for enabling asymmetric mode
> 
>  Documentation/arm64/elf_hwcaps.rst            |  5 ++++
>  .../arm64/memory-tagging-extension.rst        | 12 +++++----
>  arch/arm64/include/asm/hwcap.h                |  1 +
>  arch/arm64/include/asm/processor.h            |  1 +
>  arch/arm64/include/uapi/asm/hwcap.h           |  1 +
>  arch/arm64/kernel/cpufeature.c                |  1 +
>  arch/arm64/kernel/cpuinfo.c                   |  1 +
>  arch/arm64/kernel/mte.c                       | 26 ++++++++++++++++++-
>  arch/arm64/kernel/process.c                   |  5 +++-
>  include/uapi/linux/prctl.h                    |  4 ++-
>  10 files changed, 49 insertions(+), 8 deletions(-)
> 
> 
> base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/4] arm64/mte: Document ABI for asymmetric mode

[Will Deacon]

On Thu, Jan 27, 2022 at 07:57:09PM +0000, Mark Brown wrote:
> MTE3 adds a new mode which is synchronous for writes but asynchronous for
> reads. Document the userspace ABI for this feature, we call the new mode
> ASYMM and add a new prctl flag and mte_tcf_preferred value for it.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  Documentation/arm64/memory-tagging-extension.rst | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/arm64/memory-tagging-extension.rst b/Documentation/arm64/memory-tagging-extension.rst
> index 7b99c8f428eb..5218b838d062 100644
> --- a/Documentation/arm64/memory-tagging-extension.rst
> +++ b/Documentation/arm64/memory-tagging-extension.rst
> @@ -76,6 +76,9 @@ configurable behaviours:
>    with ``.si_code = SEGV_MTEAERR`` and ``.si_addr = 0`` (the faulting
>    address is unknown).
>  
> +- *Asymmetric* - Reads are handled as for synchronous mode while writes
> +  are handled as for asynchronous mode.
> +
>  The user can select the above modes, per thread, using the
>  ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call where ``flags``
>  contains any number of the following values in the ``PR_MTE_TCF_MASK``
> @@ -85,6 +88,7 @@ bit-field:
>                           (ignored if combined with other options)
>  - ``PR_MTE_TCF_SYNC``  - *Synchronous* tag check fault mode
>  - ``PR_MTE_TCF_ASYNC`` - *Asynchronous* tag check fault mode
> +- ``PR_MTE_TCF_ASYMM`` - *Asymmetric* tag check fault mode
>  
>  If no modes are specified, tag check faults are ignored. If a single
>  mode is specified, the program will run in that mode. If multiple
> @@ -139,16 +143,14 @@ tag checking mode as the CPU's preferred tag checking mode.
>  
>  The preferred tag checking mode for each CPU is controlled by
>  ``/sys/devices/system/cpu/cpu<N>/mte_tcf_preferred``, to which a
> -privileged user may write the value ``async`` or ``sync``.  The default
> -preferred mode for each CPU is ``async``.
> +privileged user may write the value ``async``, ``sync`` or ``asymm``.  The
> +default preferred mode for each CPU is ``async``.
>  
>  To allow a program to potentially run in the CPU's preferred tag
>  checking mode, the user program may set multiple tag check fault mode
>  bits in the ``flags`` argument to the ``prctl(PR_SET_TAGGED_ADDR_CTRL,
>  flags, 0, 0, 0)`` system call. If the CPU's preferred tag checking
> -mode is in the task's set of provided tag checking modes (this will
> -always be the case at present because the kernel only supports two
> -tag checking modes, but future kernels may support more modes), that
> +mode is in the task's set of provided tag checking modes, that
>  mode will be selected. Otherwise, one of the modes in the task's mode
>  set will be selected in a currently unspecified manner.

I think it's time we specified the "unspecified manner" now that we have
three options.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/4] arm64/mte: Document ABI for asymmetric mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 467 bytes --]

On Tue, Feb 15, 2022 at 06:14:23PM +0000, Will Deacon wrote:
> On Thu, Jan 27, 2022 at 07:57:09PM +0000, Mark Brown wrote:

> >  mode will be selected. Otherwise, one of the modes in the task's mode
> >  set will be selected in a currently unspecified manner.

> I think it's time we specified the "unspecified manner" now that we have
> three options.

Are you happy with the manner that is implemented in the patch set or is
this purely a request for documenation?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/4] arm64/mte: Document ABI for asymmetric mode

[Will Deacon]

On Wed, Feb 16, 2022 at 04:36:22PM +0000, Mark Brown wrote:
> On Tue, Feb 15, 2022 at 06:14:23PM +0000, Will Deacon wrote:
> > On Thu, Jan 27, 2022 at 07:57:09PM +0000, Mark Brown wrote:
> 
> > >  mode will be selected. Otherwise, one of the modes in the task's mode
> > >  set will be selected in a currently unspecified manner.
> 
> > I think it's time we specified the "unspecified manner" now that we have
> > three options.
> 
> Are you happy with the manner that is implemented in the patch set or is
> this purely a request for documenation?

Sorry, yes, I'm just saying let's document what the code does, as that is
what people will end up relying on.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Evgenii Stepanov]

On Thu, Jan 27, 2022 at 07:57:12PM +0000, Mark Brown wrote:
> Since there PR_MTE_TCF_ values are a bitmask (allowing the kernel to choose
> between the multiple modes) and there are no free bits adjacent to the
> existing PR_MTE_TCF_ bits the set of bits used to specify the mode becomes
> disjoint. Programs using the new interface should be aware of this and
> programs that do not use it will not see any change in behaviour.

Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
against an old version of the header this would fail to remove the ASYMM
bit.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Catalin Marinas]

On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:
> On Thu, Jan 27, 2022 at 07:57:12PM +0000, Mark Brown wrote:
> > Since there PR_MTE_TCF_ values are a bitmask (allowing the kernel to choose
> > between the multiple modes) and there are no free bits adjacent to the
> > existing PR_MTE_TCF_ bits the set of bits used to specify the mode becomes
> > disjoint. Programs using the new interface should be aware of this and
> > programs that do not use it will not see any change in behaviour.
> 
> Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> against an old version of the header this would fail to remove the ASYMM
> bit.

But if the app is compiled against an old version, it wouldn't set
MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 721 bytes --]

On Wed, Mar 02, 2022 at 11:44:53AM +0000, Catalin Marinas wrote:
> On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:

> > Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> > code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> > against an old version of the header this would fail to remove the ASYMM
> > bit.

> But if the app is compiled against an old version, it wouldn't set
> MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.

Indeed - and any attempt to set MTE_CTRL_TCF_ASYMM (or any other bit
without a specified meaning) on a system without this support results in
an error so it's hard to see how older code could have mistakenly
enabled it.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Evgenii Stepanov]

On Wed, Mar 2, 2022 at 5:10 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Mar 02, 2022 at 11:44:53AM +0000, Catalin Marinas wrote:
> > On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:
>
> > > Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> > > code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> > > against an old version of the header this would fail to remove the ASYMM
> > > bit.
>
> > But if the app is compiled against an old version, it wouldn't set
> > MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.

Libraries within a single process can be built against different
header versions. In our case, this is libc vs the app: we expect to
set all 3 mode bits when an app asks for "async" to enable the
mte_tcf_preferred logic. Even if the app is built against an older NDK
and unaware of the Asymm mode existence!

This is more of a theoretical concern at this point. Android provides
a better interface to disable MTE (mallopt) which applies to all
threads at once, and also disables tagging logic in the heap
allocator, but we can not prevent an app from shooting itself in the
foot with a raw prctl.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1505 bytes --]

On Wed, Mar 02, 2022 at 10:44:31AM -0800, Evgenii Stepanov wrote:
> On Wed, Mar 2, 2022 at 5:10 AM Mark Brown <broonie@kernel.org> wrote:
> > On Wed, Mar 02, 2022 at 11:44:53AM +0000, Catalin Marinas wrote:
> > > On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:

> > > > Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> > > > code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> > > > against an old version of the header this would fail to remove the ASYMM
> > > > bit.

> > > But if the app is compiled against an old version, it wouldn't set
> > > MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.

> Libraries within a single process can be built against different
> header versions. In our case, this is libc vs the app: we expect to
> set all 3 mode bits when an app asks for "async" to enable the
> mte_tcf_preferred logic. Even if the app is built against an older NDK
> and unaware of the Asymm mode existence!

I can't see how we can resolve that case in the kernel except by adding
a specific call to disable all MTE modes which would obviously only be
useful for future proofing given that no existing applications would
support it.  We would have been fine if we'd kept the original ABI that
only allowed applications to request one mode at once but that ship
sailed.  libc could arrange to know if the application was built against
a new enough NDK to have the new flag and not enable asymmetric mode if
it wasn't though.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Evgenii Stepanov]

On Wed, Mar 2, 2022 at 11:33 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Mar 02, 2022 at 10:44:31AM -0800, Evgenii Stepanov wrote:
> > On Wed, Mar 2, 2022 at 5:10 AM Mark Brown <broonie@kernel.org> wrote:
> > > On Wed, Mar 02, 2022 at 11:44:53AM +0000, Catalin Marinas wrote:
> > > > On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:
>
> > > > > Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> > > > > code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> > > > > against an old version of the header this would fail to remove the ASYMM
> > > > > bit.
>
> > > > But if the app is compiled against an old version, it wouldn't set
> > > > MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.
>
> > Libraries within a single process can be built against different
> > header versions. In our case, this is libc vs the app: we expect to
> > set all 3 mode bits when an app asks for "async" to enable the
> > mte_tcf_preferred logic. Even if the app is built against an older NDK
> > and unaware of the Asymm mode existence!
>
> I can't see how we can resolve that case in the kernel except by adding
> a specific call to disable all MTE modes which would obviously only be
> useful for future proofing given that no existing applications would
> support it.

One option would be to introduce a new, future-proofed prctl with a
wider mask, and throw in a few extra reserved bits just in case. Then
have the legacy prctl always clear MTE_CTRL_TCF_ASYMM.

> We would have been fine if we'd kept the original ABI that
> only allowed applications to request one mode at once but that ship
> sailed.  libc could arrange to know if the application was built against
> a new enough NDK to have the new flag and not enable asymmetric mode if
> it wasn't though.

Yes, that's a valid option, as well. But now that I think about it,
this is not the only pitfall with using the raw prctl in an Android
app. I'm inclined to just declare this case unsupported and have the
apps go through mallopt.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Catalin Marinas]

On Wed, Mar 02, 2022 at 12:58:48PM -0800, Evgenii Stepanov wrote:
> On Wed, Mar 2, 2022 at 11:33 AM Mark Brown <broonie@kernel.org> wrote:
> > On Wed, Mar 02, 2022 at 10:44:31AM -0800, Evgenii Stepanov wrote:
> > > On Wed, Mar 2, 2022 at 5:10 AM Mark Brown <broonie@kernel.org> wrote:
> > > > On Wed, Mar 02, 2022 at 11:44:53AM +0000, Catalin Marinas wrote:
> > > > > On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:
> >
> > > > > > Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> > > > > > code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> > > > > > against an old version of the header this would fail to remove the ASYMM
> > > > > > bit.
> >
> > > > > But if the app is compiled against an old version, it wouldn't set
> > > > > MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.
> >
> > > Libraries within a single process can be built against different
> > > header versions. In our case, this is libc vs the app: we expect to
> > > set all 3 mode bits when an app asks for "async" to enable the
> > > mte_tcf_preferred logic. Even if the app is built against an older NDK
> > > and unaware of the Asymm mode existence!
> >
> > I can't see how we can resolve that case in the kernel except by adding
> > a specific call to disable all MTE modes which would obviously only be
> > useful for future proofing given that no existing applications would
> > support it.
> 
> One option would be to introduce a new, future-proofed prctl with a
> wider mask, and throw in a few extra reserved bits just in case. Then
> have the legacy prctl always clear MTE_CTRL_TCF_ASYMM.

If this problem is real, we can easily tweak the current proposal so the
that ASYMM can only be set *if* both ASYNC and SYNC are set. IOW, it's
not a specific mode an app requests on its own but rather something the
kernel may choose via the preferred mode if the app opted in. We still
introduce a new bit for ASYMM but not change the mask.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1051 bytes --]

On Thu, Mar 03, 2022 at 10:34:34AM +0000, Catalin Marinas wrote:
> On Wed, Mar 02, 2022 at 12:58:48PM -0800, Evgenii Stepanov wrote:

> > One option would be to introduce a new, future-proofed prctl with a
> > wider mask, and throw in a few extra reserved bits just in case. Then
> > have the legacy prctl always clear MTE_CTRL_TCF_ASYMM.

> If this problem is real, we can easily tweak the current proposal so the
> that ASYMM can only be set *if* both ASYNC and SYNC are set. IOW, it's
> not a specific mode an app requests on its own but rather something the
> kernel may choose via the preferred mode if the app opted in. We still
> introduce a new bit for ASYMM but not change the mask.

That would work for the normal usage case but it would mean that the
only way to guarantee you actually end up with asymmetric mode would be
to force it on as the system default which might be annoying if you're
trying to test that userspace code works with all the modes.  It's not
the end of the world I think but I can see testsuites running into
issues.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Evgenii Stepanov]

On Thu, Mar 3, 2022 at 7:44 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Mar 03, 2022 at 10:34:34AM +0000, Catalin Marinas wrote:
> > On Wed, Mar 02, 2022 at 12:58:48PM -0800, Evgenii Stepanov wrote:
>
> > > One option would be to introduce a new, future-proofed prctl with a
> > > wider mask, and throw in a few extra reserved bits just in case. Then
> > > have the legacy prctl always clear MTE_CTRL_TCF_ASYMM.
>
> > If this problem is real, we can easily tweak the current proposal so the
> > that ASYMM can only be set *if* both ASYNC and SYNC are set. IOW, it's
> > not a specific mode an app requests on its own but rather something the
> > kernel may choose via the preferred mode if the app opted in. We still
> > introduce a new bit for ASYMM but not change the mask.
>
> That would work for the normal usage case but it would mean that the
> only way to guarantee you actually end up with asymmetric mode would be
> to force it on as the system default which might be annoying if you're
> trying to test that userspace code works with all the modes.  It's not
> the end of the world I think but I can see testsuites running into
> issues.

Right, I think it is important to keep the flexibility to select any
specific mode in the prctl interface.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Peter Collingbourne]

On Thu, Mar 3, 2022 at 2:41 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Wed, Mar 02, 2022 at 12:58:48PM -0800, Evgenii Stepanov wrote:
> > On Wed, Mar 2, 2022 at 11:33 AM Mark Brown <broonie@kernel.org> wrote:
> > > On Wed, Mar 02, 2022 at 10:44:31AM -0800, Evgenii Stepanov wrote:
> > > > On Wed, Mar 2, 2022 at 5:10 AM Mark Brown <broonie@kernel.org> wrote:
> > > > > On Wed, Mar 02, 2022 at 11:44:53AM +0000, Catalin Marinas wrote:
> > > > > > On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:
> > >
> > > > > > > Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> > > > > > > code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> > > > > > > against an old version of the header this would fail to remove the ASYMM
> > > > > > > bit.
> > >
> > > > > > But if the app is compiled against an old version, it wouldn't set
> > > > > > MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.
> > >
> > > > Libraries within a single process can be built against different
> > > > header versions. In our case, this is libc vs the app: we expect to
> > > > set all 3 mode bits when an app asks for "async" to enable the
> > > > mte_tcf_preferred logic. Even if the app is built against an older NDK
> > > > and unaware of the Asymm mode existence!
> > >
> > > I can't see how we can resolve that case in the kernel except by adding
> > > a specific call to disable all MTE modes which would obviously only be
> > > useful for future proofing given that no existing applications would
> > > support it.
> >
> > One option would be to introduce a new, future-proofed prctl with a
> > wider mask, and throw in a few extra reserved bits just in case. Then
> > have the legacy prctl always clear MTE_CTRL_TCF_ASYMM.
>
> If this problem is real, we can easily tweak the current proposal so the
> that ASYMM can only be set *if* both ASYNC and SYNC are set. IOW, it's
> not a specific mode an app requests on its own but rather something the
> kernel may choose via the preferred mode if the app opted in. We still
> introduce a new bit for ASYMM but not change the mask.

As discussed out-of-band, I've never really liked this API style of
trying to cram everything into a single prctl(), not least because of
these compatibility concerns. Evgenii proposed a prctl() with a wider
mask, but I think this sort of approach just kicks the can down the
road (well, maybe for *this* feature we can't expect there to be too
many new modes added, but can we say the same for everything else in
tagged_addr_ctrl?). And an attempt to guess the user intent from which
bits are set seems prone to failure and unnecessarily restrictive. It
seems far more preferable to have a separate prctl() control for the
TCF bits and start working towards splitting out the other bits to
their own prctl()s. Then any user code that manipulates these bits
will "naturally" work. For compatibility the TCF bits would still be
accessible via the existing prctl(), but an attempt to set TCF via
this prctl() would clear the ASYMM bit.

The only downside that I've seen mentioned is that we will end up
issuing more syscalls on process startup which could harm performance.
But is this really significant? In the past I've measured the overhead
of a syscall to be on the order of a few hundred nanoseconds on recent
arm64 platforms. Starting a new process is expected to be a relatively
expensive operation anyway so I wouldn't expect this to make much
difference. It also seems like prioritizing performance here is the
wrong tradeoff for a kernel with a stable userspace ABI. If it is
really a concern I think I would prefer that we add a way to issue
multiple syscalls with a single kernel entry instead (this is
something that I've wanted for other use cases as well).

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Catalin Marinas]

On Fri, Mar 04, 2022 at 01:09:00PM -0800, Peter Collingbourne wrote:
> On Thu, Mar 3, 2022 at 2:41 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Wed, Mar 02, 2022 at 12:58:48PM -0800, Evgenii Stepanov wrote:
> > > On Wed, Mar 2, 2022 at 11:33 AM Mark Brown <broonie@kernel.org> wrote:
> > > > On Wed, Mar 02, 2022 at 10:44:31AM -0800, Evgenii Stepanov wrote:
> > > > > On Wed, Mar 2, 2022 at 5:10 AM Mark Brown <broonie@kernel.org> wrote:
> > > > > > On Wed, Mar 02, 2022 at 11:44:53AM +0000, Catalin Marinas wrote:
> > > > > > > On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:
> > > >
> > > > > > > > Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> > > > > > > > code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> > > > > > > > against an old version of the header this would fail to remove the ASYMM
> > > > > > > > bit.
> > > >
> > > > > > > But if the app is compiled against an old version, it wouldn't set
> > > > > > > MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.
> > > >
> > > > > Libraries within a single process can be built against different
> > > > > header versions. In our case, this is libc vs the app: we expect to
> > > > > set all 3 mode bits when an app asks for "async" to enable the
> > > > > mte_tcf_preferred logic. Even if the app is built against an older NDK
> > > > > and unaware of the Asymm mode existence!
> > > >
> > > > I can't see how we can resolve that case in the kernel except by adding
> > > > a specific call to disable all MTE modes which would obviously only be
> > > > useful for future proofing given that no existing applications would
> > > > support it.
> > >
> > > One option would be to introduce a new, future-proofed prctl with a
> > > wider mask, and throw in a few extra reserved bits just in case. Then
> > > have the legacy prctl always clear MTE_CTRL_TCF_ASYMM.
> >
> > If this problem is real, we can easily tweak the current proposal so the
> > that ASYMM can only be set *if* both ASYNC and SYNC are set. IOW, it's
> > not a specific mode an app requests on its own but rather something the
> > kernel may choose via the preferred mode if the app opted in. We still
> > introduce a new bit for ASYMM but not change the mask.
> 
> As discussed out-of-band, I've never really liked this API style of
> trying to cram everything into a single prctl(), not least because of
> these compatibility concerns.

I'd say the main problem is not necessarily the single prctl() which, at
least initially, followed the hw control closely. The two main issues I
see are: (1) unclear user-space ownership of such control and (2) the
ABI change from single mode request to a mask of supported application
modes which we did not foresee the need for when first adding MTE, nor
did we see the potential problems with the asymmetric mode later.

In Evgenii's use-case, it seems that it's not the libc owning the MTE
controls but any other piece of code that may not be recompiled to the
latest libc headers. We could argue that it's a user problem to solve
and not that different from apps that use a non-zero tag or some pointer
arithmetics and get confused by an MTE-aware allocator.

Can any libc versioning help with making sure old apps are not linked
against newer libc with such change? I guess this only works for symbols
but here it's a macro.

> Evgenii proposed a prctl() with a wider
> mask, but I think this sort of approach just kicks the can down the
> road (well, maybe for *this* feature we can't expect there to be too
> many new modes added, but can we say the same for everything else in
> tagged_addr_ctrl?).

Extending the mask and adding a few more bits doesn't solve the original
problem raised by Evgenii: applications compiled against current headers
but (dynamically) linked against future libc could be confused if they
manage the prctl() themselves.

> And an attempt to guess the user intent from which
> bits are set seems prone to failure and unnecessarily restrictive.

Only allowing asymmetric mode if both symmetric and async are supported
is not that confusing. The only downside is that one cannot ask for
asymmetric mode only but is this such a big problem? For testing one can
always set the sysfs preferred mode to asymm. It will be some time
before we see asymm in production.

> It
> seems far more preferable to have a separate prctl() control for the
> TCF bits and start working towards splitting out the other bits to
> their own prctl()s. Then any user code that manipulates these bits
> will "naturally" work. For compatibility the TCF bits would still be
> accessible via the existing prctl(), but an attempt to set TCF via
> this prctl() would clear the ASYMM bit.

In hindsight, this would have been better. Right now we have to make
sure we don't break the ABI and it's not only about setting the TCF but
also getting the controls (prctl(PR_GET_TAGGED_ADDR_CTRL), ptrace()).

Assuming we have a different prctl() for setting TCF, what would we
report in the current PR_GET_ for existing apps when only the asymm mode
was set? By the same logic as the TCF mask breaking current apps, this
doesn't work if somehow an app uses the information and, for example,
retry a faulting operation indefinitely.

I don't see MTE gaining more TCF modes in the future but it's not
excluded to get other controls.

> The only downside that I've seen mentioned is that we will end up
> issuing more syscalls on process startup which could harm performance.

I don't think that's the only downside. It's an ABI change as well
w.r.t. getting the current TCF setting and asymm being invisible to an
unaware app. The only way out I see is to make asymm a kernel choice
when both sync and async are selected (potentially with another bit to
opt it but without changing the TCF mask).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2332 bytes --]

On Mon, Mar 07, 2022 at 03:36:52PM +0000, Catalin Marinas wrote:
> On Fri, Mar 04, 2022 at 01:09:00PM -0800, Peter Collingbourne wrote:

> > And an attempt to guess the user intent from which
> > bits are set seems prone to failure and unnecessarily restrictive.
> 
> Only allowing asymmetric mode if both symmetric and async are supported
> is not that confusing. The only downside is that one cannot ask for
> asymmetric mode only but is this such a big problem? For testing one can
> always set the sysfs preferred mode to asymm. It will be some time
> before we see asymm in production.

People like to do things like run their testsuites in containers and
generally not as root which is a problem for any interface that relies
on fiddling with the system settings.  It's something people can work
around but it's not great.

Another option here would be to just not expose asymmetric mode directly
via the pcrtl() at all and just allow it if it's supported and both
sync and async are enabled.  Userspace ought to be able to tolerate
this since the preferred mode may vary depending on which CPU the task
gets scheduled on so the application may get a mix of modes if it's
requested both and it won't be able to worry about requesting async mode
since the option simply isn't there meaning it's less likely to get
specific coverage in application testsuites.

> > The only downside that I've seen mentioned is that we will end up
> > issuing more syscalls on process startup which could harm performance.

> I don't think that's the only downside. It's an ABI change as well
> w.r.t. getting the current TCF setting and asymm being invisible to an
> unaware app. The only way out I see is to make asymm a kernel choice
> when both sync and async are selected (potentially with another bit to
> opt it but without changing the TCF mask).

It's not the most lovely API but given that we're faced with choices of
what to break or bodge another kludge would be to require that a second
bit be set when enabling asymmetric mode with that bit reading as zero.
That would break old code doing a save/restore flow though so while it
helps the original problem I don't know that it's actually doing much
overall.  I think this just comes down to a question of where we end up
causing problems.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Peter Collingbourne]

On Mon, Mar 7, 2022 at 7:36 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Fri, Mar 04, 2022 at 01:09:00PM -0800, Peter Collingbourne wrote:
> > On Thu, Mar 3, 2022 at 2:41 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > On Wed, Mar 02, 2022 at 12:58:48PM -0800, Evgenii Stepanov wrote:
> > > > On Wed, Mar 2, 2022 at 11:33 AM Mark Brown <broonie@kernel.org> wrote:
> > > > > On Wed, Mar 02, 2022 at 10:44:31AM -0800, Evgenii Stepanov wrote:
> > > > > > On Wed, Mar 2, 2022 at 5:10 AM Mark Brown <broonie@kernel.org> wrote:
> > > > > > > On Wed, Mar 02, 2022 at 11:44:53AM +0000, Catalin Marinas wrote:
> > > > > > > > On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:
> > > > >
> > > > > > > > > Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> > > > > > > > > code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> > > > > > > > > against an old version of the header this would fail to remove the ASYMM
> > > > > > > > > bit.
> > > > >
> > > > > > > > But if the app is compiled against an old version, it wouldn't set
> > > > > > > > MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.
> > > > >
> > > > > > Libraries within a single process can be built against different
> > > > > > header versions. In our case, this is libc vs the app: we expect to
> > > > > > set all 3 mode bits when an app asks for "async" to enable the
> > > > > > mte_tcf_preferred logic. Even if the app is built against an older NDK
> > > > > > and unaware of the Asymm mode existence!
> > > > >
> > > > > I can't see how we can resolve that case in the kernel except by adding
> > > > > a specific call to disable all MTE modes which would obviously only be
> > > > > useful for future proofing given that no existing applications would
> > > > > support it.
> > > >
> > > > One option would be to introduce a new, future-proofed prctl with a
> > > > wider mask, and throw in a few extra reserved bits just in case. Then
> > > > have the legacy prctl always clear MTE_CTRL_TCF_ASYMM.
> > >
> > > If this problem is real, we can easily tweak the current proposal so the
> > > that ASYMM can only be set *if* both ASYNC and SYNC are set. IOW, it's
> > > not a specific mode an app requests on its own but rather something the
> > > kernel may choose via the preferred mode if the app opted in. We still
> > > introduce a new bit for ASYMM but not change the mask.
> >
> > As discussed out-of-band, I've never really liked this API style of
> > trying to cram everything into a single prctl(), not least because of
> > these compatibility concerns.
>
> I'd say the main problem is not necessarily the single prctl() which, at
> least initially, followed the hw control closely. The two main issues I
> see are: (1) unclear user-space ownership of such control and (2) the
> ABI change from single mode request to a mask of supported application
> modes which we did not foresee the need for when first adding MTE, nor
> did we see the potential problems with the asymmetric mode later.
>
> In Evgenii's use-case, it seems that it's not the libc owning the MTE
> controls but any other piece of code that may not be recompiled to the
> latest libc headers. We could argue that it's a user problem to solve
> and not that different from apps that use a non-zero tag or some pointer
> arithmetics and get confused by an MTE-aware allocator.

Yeah, one option we're considering is "do nothing" if none of the
possible solutions seem palatable, as so far this is only a
hypothetical problem and we could declare direct prctl() manipulation
from applications to be unsupported.

> Can any libc versioning help with making sure old apps are not linked
> against newer libc with such change? I guess this only works for symbols
> but here it's a macro.

Native binaries on Android have an ELF note indicating the version of
the headers that they were compiled against. So one option on Android
would be to use these ELF notes for controlling on a per-app basis
whether to set ASYMM.

> > Evgenii proposed a prctl() with a wider
> > mask, but I think this sort of approach just kicks the can down the
> > road (well, maybe for *this* feature we can't expect there to be too
> > many new modes added, but can we say the same for everything else in
> > tagged_addr_ctrl?).
>
> Extending the mask and adding a few more bits doesn't solve the original
> problem raised by Evgenii: applications compiled against current headers
> but (dynamically) linked against future libc could be confused if they
> manage the prctl() themselves.
>
> > And an attempt to guess the user intent from which
> > bits are set seems prone to failure and unnecessarily restrictive.
>
> Only allowing asymmetric mode if both symmetric and async are supported
> is not that confusing. The only downside is that one cannot ask for
> asymmetric mode only but is this such a big problem? For testing one can
> always set the sysfs preferred mode to asymm. It will be some time
> before we see asymm in production.

It could be a problem on Android because one of the modes that we
intend to expose to application developers is "asymm-only" (as opposed
to "upgradable asymm"). This would be used by developers who need a
finer grained control over the MTE mode used in their application.
Although to be honest I'm not 100% certain that we would need this
mode so perhaps we could live with just the "upgradable asymm" mode.

> > It
> > seems far more preferable to have a separate prctl() control for the
> > TCF bits and start working towards splitting out the other bits to
> > their own prctl()s. Then any user code that manipulates these bits
> > will "naturally" work. For compatibility the TCF bits would still be
> > accessible via the existing prctl(), but an attempt to set TCF via
> > this prctl() would clear the ASYMM bit.
>
> In hindsight, this would have been better. Right now we have to make
> sure we don't break the ABI and it's not only about setting the TCF but
> also getting the controls (prctl(PR_GET_TAGGED_ADDR_CTRL), ptrace()).
>
> Assuming we have a different prctl() for setting TCF, what would we
> report in the current PR_GET_ for existing apps when only the asymm mode
> was set? By the same logic as the TCF mask breaking current apps, this
> doesn't work if somehow an app uses the information and, for example,
> retry a faulting operation indefinitely.

One option may be to reject the GET with EINVAL if ASYMM is set. This
should hopefully result in the application acting as if MTE is not
enabled/supported. Then ASYMM-aware applications can do something like
pass arg2=1 to get the other bits regardless. We wouldn't do anything
like this for ptrace() (i.e. it would always return the exposed bits
of tagged_addr_ctrl regardless of ASYMM setting), but ptrace() seems
less of a concern since it's only used by debuggers and such.

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Peter Collingbourne]

On Mon, Mar 7, 2022 at 11:10 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Mar 07, 2022 at 03:36:52PM +0000, Catalin Marinas wrote:
> > On Fri, Mar 04, 2022 at 01:09:00PM -0800, Peter Collingbourne wrote:
>
> > > And an attempt to guess the user intent from which
> > > bits are set seems prone to failure and unnecessarily restrictive.
> >
> > Only allowing asymmetric mode if both symmetric and async are supported
> > is not that confusing. The only downside is that one cannot ask for
> > asymmetric mode only but is this such a big problem? For testing one can
> > always set the sysfs preferred mode to asymm. It will be some time
> > before we see asymm in production.
>
> People like to do things like run their testsuites in containers and
> generally not as root which is a problem for any interface that relies
> on fiddling with the system settings.  It's something people can work
> around but it's not great.
>
> Another option here would be to just not expose asymmetric mode directly
> via the pcrtl() at all and just allow it if it's supported and both
> sync and async are enabled.  Userspace ought to be able to tolerate
> this since the preferred mode may vary depending on which CPU the task
> gets scheduled on so the application may get a mix of modes if it's
> requested both and it won't be able to worry about requesting async mode
> since the option simply isn't there meaning it's less likely to get
> specific coverage in application testsuites.
>
> > > The only downside that I've seen mentioned is that we will end up
> > > issuing more syscalls on process startup which could harm performance.
>
> > I don't think that's the only downside. It's an ABI change as well
> > w.r.t. getting the current TCF setting and asymm being invisible to an
> > unaware app. The only way out I see is to make asymm a kernel choice
> > when both sync and async are selected (potentially with another bit to
> > opt it but without changing the TCF mask).
>
> It's not the most lovely API but given that we're faced with choices of
> what to break or bodge another kludge would be to require that a second
> bit be set when enabling asymmetric mode with that bit reading as zero.
> That would break old code doing a save/restore flow though so while it
> helps the original problem I don't know that it's actually doing much
> overall.  I think this just comes down to a question of where we end up
> causing problems.

I agree with Mark that there are going to be (to some extent
hypothetical) compatibility problems no matter what we do. I think a
good way to think about it is: if we need to do the same thing again
in the future (in this case, adding another mode), which design would
allow us to avoid the same potential compatibility problems? To me the
PR_GET_TCF/PR_SET_TCF control seems like the best way of avoiding
that.

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 857 bytes --]

On Mon, Mar 07, 2022 at 12:55:31PM -0800, Peter Collingbourne wrote:

> I agree with Mark that there are going to be (to some extent
> hypothetical) compatibility problems no matter what we do. I think a
> good way to think about it is: if we need to do the same thing again
> in the future (in this case, adding another mode), which design would
> allow us to avoid the same potential compatibility problems? To me the
> PR_GET_TCF/PR_SET_TCF control seems like the best way of avoiding
> that.

Providing a decomposed version of the prctl does seem like the most
robust thing for future changes, I think it's mostly a question of
working out if it's worth the effort of deciding how to handle the
interaction with the current ABI.

I do question how sensible it is to have multiple enteties in userspace
managing MTE without coordinating with each other.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Catalin Marinas]

On Mon, Mar 07, 2022 at 12:55:28PM -0800, Peter Collingbourne wrote:
> On Mon, Mar 7, 2022 at 7:36 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > Only allowing asymmetric mode if both symmetric and async are supported
> > is not that confusing. The only downside is that one cannot ask for
> > asymmetric mode only but is this such a big problem? For testing one can
> > always set the sysfs preferred mode to asymm. It will be some time
> > before we see asymm in production.
> 
> It could be a problem on Android because one of the modes that we
> intend to expose to application developers is "asymm-only" (as opposed
> to "upgradable asymm"). This would be used by developers who need a
> finer grained control over the MTE mode used in their application.
> Although to be honest I'm not 100% certain that we would need this
> mode so perhaps we could live with just the "upgradable asymm" mode.

With the introduction of the preferred mode, we kind of agreed that an
application doesn't necessarily know what it wants and we should leave
this decision to the kernel (via sysfs) as it knows the performance
aspects better. To avoid confusing applications, we changed the TCF
field in prctl() from an exact mode to a supported mode mask so that an
app is not take by surprise. Now, the asymmetric mode is just a
combination of sync and async, and an app just states that it supports
both. It's not any different from the kernel changing the preferred mode
at run-time.

We can later add a new bit, "force asymm" or something if needed while
still requiring that both async and sync bits are set. So I'm tempted to
go with Mark's latest patch, once I looked at it.

> > Assuming we have a different prctl() for setting TCF, what would we
> > report in the current PR_GET_ for existing apps when only the asymm mode
> > was set? By the same logic as the TCF mask breaking current apps, this
> > doesn't work if somehow an app uses the information and, for example,
> > retry a faulting operation indefinitely.
> 
> One option may be to reject the GET with EINVAL if ASYMM is set. This
> should hopefully result in the application acting as if MTE is not
> enabled/supported.

Well, the app still gets TCF faults while it may have concluded that MTE
is disabled. We can't always guess what the user wants (and the history
around this ABI shows this).

Anyway, I'm open to improving the ABI later but I don't think we should
delay the asym MTE patches, especially if we merge Mark's asym removal
from prctl().

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel