[PATCH v3] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis

[PATCH v3] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis

[Peter Collingbourne]

On some CPUs the performance of MTE in synchronous mode is the same
as that of asynchronous mode. This makes it worthwhile to enable
synchronous mode on those CPUs when asynchronous mode is requested,
in order to gain the error detection benefits of synchronous mode
without the performance downsides. Therefore, make it possible for
user programs to opt into upgrading to synchronous mode on those CPUs
via a new prctl flag. The flag is orthogonal to the existing TCF modes
in order to accommodate upgrading from other TCF modes in the future.

The feature is controlled on a per-CPU basis via sysfs.

Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/Id6f95b71fde6e701dd30b5e108126af7286147e8
---
v3:
- drop the device tree support
- add documentation
- add static_assert to ensure no overlap with real HW bits
- move per-CPU variable initialization to mte.c
- use smp_call_function_single instead of stop_machine

v2:
- make it an opt-in behavior
- change the format of the device tree node
- also allow controlling the feature via sysfs

 .../arm64/memory-tagging-extension.rst        | 20 ++++++
 arch/arm64/include/asm/processor.h            |  8 +++
 arch/arm64/kernel/mte.c                       | 71 ++++++++++++++++++-
 arch/arm64/kernel/process.c                   | 15 +++-
 include/uapi/linux/prctl.h                    |  2 +
 5 files changed, 113 insertions(+), 3 deletions(-)

diff --git a/Documentation/arm64/memory-tagging-extension.rst b/Documentation/arm64/memory-tagging-extension.rst
index b540178a93f8..2fc145de6530 100644
--- a/Documentation/arm64/memory-tagging-extension.rst
+++ b/Documentation/arm64/memory-tagging-extension.rst
@@ -120,6 +120,25 @@ in the ``PR_MTE_TAG_MASK`` bit-field.
 interface provides an include mask. An include mask of ``0`` (exclusion
 mask ``0xffff``) results in the CPU always generating tag ``0``.
 
+Upgrading to stricter tag checking modes
+----------------------------------------
+
+On some CPUs the performance of MTE in stricter tag checking modes
+is the same as that of less strict tag checking modes. This makes it
+worthwhile to enable stricter checks on those CPUs when a less strict
+checking mode is requested, in order to gain the error detection
+benefits of the stricter checks without the performance downsides. To
+opt into upgrading to a stricter checking mode on those CPUs, the user
+can set the ``PR_MTE_DYNAMIC_TCF`` flag bit in the ``flags`` argument
+to the ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call.
+
+This feature is currently only supported for upgrading from
+asynchronous mode. To configure a CPU to upgrade from asynchronous mode
+to synchronous mode, a privileged user may write the value ``1`` to
+``/sys/devices/system/cpu/cpu<N>/mte_upgrade_async``, and to disable
+upgrading they may write the value ``2``. By default the feature is
+disabled on all CPUs.
+
 Initial process state
 ---------------------
 
@@ -128,6 +147,7 @@ On ``execve()``, the new process has the following configuration:
 - ``PR_TAGGED_ADDR_ENABLE`` set to 0 (disabled)
 - Tag checking mode set to ``PR_MTE_TCF_NONE``
 - ``PR_MTE_TAG_MASK`` set to 0 (all tags excluded)
+- ``PR_MTE_DYNAMIC_TCF`` set to 0 (disabled)
 - ``PSTATE.TCO`` set to 0
 - ``PROT_MTE`` not set on any of the initial memory maps
 
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 9df3feeee890..531f580e859e 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -159,6 +159,13 @@ struct thread_struct {
 #define SCTLR_USER_MASK                                                        \
 	(SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | SCTLR_ELx_ENDA | SCTLR_ELx_ENDB |   \
 	 SCTLR_EL1_TCF0_MASK)
+#define SCTLR_USER_DYNAMIC_TCF (1ULL << 63)
+
+/*
+ * Make sure that SCTLR_USER_DYNAMIC_TCF doesn't overlap with any of the real
+ * per-task hardware bits.
+ */
+static_assert((SCTLR_USER_MASK & SCTLR_USER_DYNAMIC_TCF) == 0);
 
 static inline void arch_thread_struct_whitelist(unsigned long *offset,
 						unsigned long *size)
@@ -251,6 +258,7 @@ extern void release_thread(struct task_struct *);
 
 unsigned long get_wchan(struct task_struct *p);
 
+void update_sctlr_el1(u64 sctlr);
 void set_task_sctlr_el1(u64 sctlr);
 
 /* Thread switching */
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 125a10e413e9..d40bb03dfacc 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/cpu.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/prctl.h>
@@ -26,6 +27,9 @@ u64 gcr_kernel_excl __ro_after_init;
 
 static bool report_fault_once = true;
 
+DEFINE_PER_CPU_READ_MOSTLY(u64, mte_upgrade_async);
+EXPORT_PER_CPU_SYMBOL(mte_upgrade_async);
+
 #ifdef CONFIG_KASAN_HW_TAGS
 /* Whether the MTE asynchronous mode is enabled. */
 DEFINE_STATIC_KEY_FALSE(mte_async_mode);
@@ -262,7 +266,8 @@ void mte_suspend_exit(void)
 
 long set_mte_ctrl(struct task_struct *task, unsigned long arg)
 {
-	u64 sctlr = task->thread.sctlr_user & ~SCTLR_EL1_TCF0_MASK;
+	u64 sctlr = task->thread.sctlr_user &
+		    ~(SCTLR_EL1_TCF0_MASK | SCTLR_USER_DYNAMIC_TCF);
 	u64 gcr_excl = ~((arg & PR_MTE_TAG_MASK) >> PR_MTE_TAG_SHIFT) &
 		       SYS_GCR_EL1_EXCL_MASK;
 
@@ -283,6 +288,9 @@ long set_mte_ctrl(struct task_struct *task, unsigned long arg)
 		return -EINVAL;
 	}
 
+	if (arg & PR_MTE_DYNAMIC_TCF)
+		sctlr |= SCTLR_USER_DYNAMIC_TCF;
+
 	if (task != current) {
 		task->thread.sctlr_user = sctlr;
 		task->thread.gcr_user_excl = gcr_excl;
@@ -316,6 +324,9 @@ long get_mte_ctrl(struct task_struct *task)
 		break;
 	}
 
+	if (task->thread.sctlr_user & SCTLR_USER_DYNAMIC_TCF)
+		ret |= PR_MTE_DYNAMIC_TCF;
+
 	return ret;
 }
 
@@ -453,3 +464,61 @@ int mte_ptrace_copy_tags(struct task_struct *child, long request,
 
 	return ret;
 }
+
+static ssize_t mte_upgrade_async_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	u32 val = per_cpu(mte_upgrade_async, dev->id) >> SCTLR_EL1_TCF0_SHIFT;
+
+	return sysfs_emit(buf, "%u\n", val);
+}
+
+static void sync_sctlr(void *arg)
+{
+	update_sctlr_el1(current->thread.sctlr_user);
+}
+
+static ssize_t mte_upgrade_async_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	ssize_t ret;
+	u32 val;
+	u64 tcf;
+
+	ret = kstrtou32(buf, 0, &val);
+	if (ret < 0)
+		return ret;
+
+	tcf = ((u64)val) << SCTLR_EL1_TCF0_SHIFT;
+	if (tcf != SCTLR_EL1_TCF0_NONE && tcf != SCTLR_EL1_TCF0_SYNC &&
+	    tcf != SCTLR_EL1_TCF0_ASYNC)
+		return -EINVAL;
+
+	device_lock(dev);
+	per_cpu(mte_upgrade_async, dev->id) = tcf;
+
+	if (cpu_online(dev->id))
+		ret = smp_call_function_single(dev->id, sync_sctlr, NULL, 0);
+	if (ret == 0)
+		ret = count;
+	device_unlock(dev);
+
+	return ret;
+}
+static DEVICE_ATTR_RW(mte_upgrade_async);
+
+static void register_mte_upgrade_async_sysctl(void)
+{
+	unsigned int cpu;
+
+	if (!system_supports_mte())
+		return;
+
+	for_each_possible_cpu(cpu) {
+		per_cpu(mte_upgrade_async, cpu) = SCTLR_EL1_TCF0_ASYNC;
+		device_create_file(get_cpu_device(cpu),
+				   &dev_attr_mte_upgrade_async);
+	}
+}
+subsys_initcall(register_mte_upgrade_async_sysctl);
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index b4bb67f17a2c..27a12c53529d 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -527,8 +527,19 @@ static void erratum_1418040_thread_switch(struct task_struct *prev,
 	write_sysreg(val, cntkctl_el1);
 }
 
-static void update_sctlr_el1(u64 sctlr)
+DECLARE_PER_CPU_READ_MOSTLY(u64, mte_upgrade_async);
+
+void update_sctlr_el1(u64 sctlr)
 {
+	if (sctlr & SCTLR_USER_DYNAMIC_TCF) {
+		sctlr &= ~SCTLR_USER_DYNAMIC_TCF;
+
+		if ((sctlr & SCTLR_EL1_TCF0_MASK) == SCTLR_EL1_TCF0_ASYNC) {
+			sctlr &= ~SCTLR_EL1_TCF0_MASK;
+			sctlr |= __this_cpu_read(mte_upgrade_async);
+		}
+	}
+
 	/*
 	 * EnIA must not be cleared while in the kernel as this is necessary for
 	 * in-kernel PAC. It will be cleared on kernel exit if needed.
@@ -659,7 +670,7 @@ 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_MASK | PR_MTE_TAG_MASK | PR_MTE_DYNAMIC_TCF;
 
 	if (arg & ~valid_mask)
 		return -EINVAL;
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 18a9f59dc067..4dab44732814 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -242,6 +242,8 @@ struct prctl_mm_map {
 /* MTE tag inclusion mask */
 # define PR_MTE_TAG_SHIFT		3
 # define PR_MTE_TAG_MASK		(0xffffUL << PR_MTE_TAG_SHIFT)
+/* Enable dynamic upgrading of MTE tag check fault mode */
+# define PR_MTE_DYNAMIC_TCF		(1UL << 19)
 
 /* Control reclaim behavior when allocating memory */
 #define PR_SET_IO_FLUSHER		57
-- 
2.32.0.272.g935e593368-goog


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

Re: [PATCH v3] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis

[Catalin Marinas]

On Fri, Jun 11, 2021 at 02:51:01PM -0700, Peter Collingbourne wrote:
> diff --git a/Documentation/arm64/memory-tagging-extension.rst b/Documentation/arm64/memory-tagging-extension.rst
> index b540178a93f8..2fc145de6530 100644
> --- a/Documentation/arm64/memory-tagging-extension.rst
> +++ b/Documentation/arm64/memory-tagging-extension.rst
> @@ -120,6 +120,25 @@ in the ``PR_MTE_TAG_MASK`` bit-field.
>  interface provides an include mask. An include mask of ``0`` (exclusion
>  mask ``0xffff``) results in the CPU always generating tag ``0``.
>  
> +Upgrading to stricter tag checking modes
> +----------------------------------------
> +
> +On some CPUs the performance of MTE in stricter tag checking modes
> +is the same as that of less strict tag checking modes. This makes it
> +worthwhile to enable stricter checks on those CPUs when a less strict
> +checking mode is requested, in order to gain the error detection
> +benefits of the stricter checks without the performance downsides. To
> +opt into upgrading to a stricter checking mode on those CPUs, the user
> +can set the ``PR_MTE_DYNAMIC_TCF`` flag bit in the ``flags`` argument
> +to the ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call.
> +
> +This feature is currently only supported for upgrading from
> +asynchronous mode. To configure a CPU to upgrade from asynchronous mode
> +to synchronous mode, a privileged user may write the value ``1`` to
> +``/sys/devices/system/cpu/cpu<N>/mte_upgrade_async``, and to disable
> +upgrading they may write the value ``2``. By default the feature is
> +disabled on all CPUs.

Why not 0 to disable? This should be the default. I'd keep 2 for
upgrading to the new asymmetric mode in v8.7 (reads sync, writes async).

> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index b4bb67f17a2c..27a12c53529d 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -527,8 +527,19 @@ static void erratum_1418040_thread_switch(struct task_struct *prev,
>  	write_sysreg(val, cntkctl_el1);
>  }
>  
> -static void update_sctlr_el1(u64 sctlr)
> +DECLARE_PER_CPU_READ_MOSTLY(u64, mte_upgrade_async);
> +
> +void update_sctlr_el1(u64 sctlr)
>  {
> +	if (sctlr & SCTLR_USER_DYNAMIC_TCF) {
> +		sctlr &= ~SCTLR_USER_DYNAMIC_TCF;
> +
> +		if ((sctlr & SCTLR_EL1_TCF0_MASK) == SCTLR_EL1_TCF0_ASYNC) {
> +			sctlr &= ~SCTLR_EL1_TCF0_MASK;
> +			sctlr |= __this_cpu_read(mte_upgrade_async);
> +		}
> +	}

I replied to your v2 already. I'd prefer this to be handled in mte.c.

-- 
Catalin

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

Re: [PATCH v3] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis

[Catalin Marinas]

On Mon, Jun 14, 2021 at 06:56:09PM +0100, Catalin Marinas wrote:
> On Fri, Jun 11, 2021 at 02:51:01PM -0700, Peter Collingbourne wrote:
> > diff --git a/Documentation/arm64/memory-tagging-extension.rst b/Documentation/arm64/memory-tagging-extension.rst
> > index b540178a93f8..2fc145de6530 100644
> > --- a/Documentation/arm64/memory-tagging-extension.rst
> > +++ b/Documentation/arm64/memory-tagging-extension.rst
> > @@ -120,6 +120,25 @@ in the ``PR_MTE_TAG_MASK`` bit-field.
> >  interface provides an include mask. An include mask of ``0`` (exclusion
> >  mask ``0xffff``) results in the CPU always generating tag ``0``.
> >  
> > +Upgrading to stricter tag checking modes
> > +----------------------------------------
> > +
> > +On some CPUs the performance of MTE in stricter tag checking modes
> > +is the same as that of less strict tag checking modes. This makes it
> > +worthwhile to enable stricter checks on those CPUs when a less strict
> > +checking mode is requested, in order to gain the error detection
> > +benefits of the stricter checks without the performance downsides. To
> > +opt into upgrading to a stricter checking mode on those CPUs, the user
> > +can set the ``PR_MTE_DYNAMIC_TCF`` flag bit in the ``flags`` argument
> > +to the ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call.
> > +
> > +This feature is currently only supported for upgrading from
> > +asynchronous mode. To configure a CPU to upgrade from asynchronous mode
> > +to synchronous mode, a privileged user may write the value ``1`` to
> > +``/sys/devices/system/cpu/cpu<N>/mte_upgrade_async``, and to disable
> > +upgrading they may write the value ``2``. By default the feature is
> > +disabled on all CPUs.
> 
> Why not 0 to disable? This should be the default. I'd keep 2 for
> upgrading to the new asymmetric mode in v8.7 (reads sync, writes async).

Ah, I think I get it, you wanted to use the TCF values. Fine by me but
I'd actually still keep 0 for disable (which is the default value I
guess) with the 1 (or 3 later) for the upgrade. We may also add an
mte_upgrade_asym file at some point.

-- 
Catalin

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

Re: [PATCH v3] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis

[Catalin Marinas]

On Fri, Jun 11, 2021 at 02:51:01PM -0700, Peter Collingbourne wrote:
> +static ssize_t mte_upgrade_async_store(struct device *dev,
> +				       struct device_attribute *attr,
> +				       const char *buf, size_t count)
> +{
> +	ssize_t ret;
> +	u32 val;
> +	u64 tcf;
> +
> +	ret = kstrtou32(buf, 0, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	tcf = ((u64)val) << SCTLR_EL1_TCF0_SHIFT;
> +	if (tcf != SCTLR_EL1_TCF0_NONE && tcf != SCTLR_EL1_TCF0_SYNC &&
> +	    tcf != SCTLR_EL1_TCF0_ASYNC)
> +		return -EINVAL;
> +
> +	device_lock(dev);
> +	per_cpu(mte_upgrade_async, dev->id) = tcf;
> +
> +	if (cpu_online(dev->id))
> +		ret = smp_call_function_single(dev->id, sync_sctlr, NULL, 0);

Forgot about this. I get it now, you only need to kick the CPU that is
getting changed. This looks fine.

-- 
Catalin

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

Re: [PATCH v3] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis

[Peter Collingbourne]

On Mon, Jun 14, 2021 at 11:02 AM Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> On Mon, Jun 14, 2021 at 06:56:09PM +0100, Catalin Marinas wrote:
> > On Fri, Jun 11, 2021 at 02:51:01PM -0700, Peter Collingbourne wrote:
> > > diff --git a/Documentation/arm64/memory-tagging-extension.rst b/Documentation/arm64/memory-tagging-extension.rst
> > > index b540178a93f8..2fc145de6530 100644
> > > --- a/Documentation/arm64/memory-tagging-extension.rst
> > > +++ b/Documentation/arm64/memory-tagging-extension.rst
> > > @@ -120,6 +120,25 @@ in the ``PR_MTE_TAG_MASK`` bit-field.
> > >  interface provides an include mask. An include mask of ``0`` (exclusion
> > >  mask ``0xffff``) results in the CPU always generating tag ``0``.
> > >
> > > +Upgrading to stricter tag checking modes
> > > +----------------------------------------
> > > +
> > > +On some CPUs the performance of MTE in stricter tag checking modes
> > > +is the same as that of less strict tag checking modes. This makes it
> > > +worthwhile to enable stricter checks on those CPUs when a less strict
> > > +checking mode is requested, in order to gain the error detection
> > > +benefits of the stricter checks without the performance downsides. To
> > > +opt into upgrading to a stricter checking mode on those CPUs, the user
> > > +can set the ``PR_MTE_DYNAMIC_TCF`` flag bit in the ``flags`` argument
> > > +to the ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call.
> > > +
> > > +This feature is currently only supported for upgrading from
> > > +asynchronous mode. To configure a CPU to upgrade from asynchronous mode
> > > +to synchronous mode, a privileged user may write the value ``1`` to
> > > +``/sys/devices/system/cpu/cpu<N>/mte_upgrade_async``, and to disable
> > > +upgrading they may write the value ``2``. By default the feature is
> > > +disabled on all CPUs.
> >
> > Why not 0 to disable? This should be the default. I'd keep 2 for
> > upgrading to the new asymmetric mode in v8.7 (reads sync, writes async).
>
> Ah, I think I get it, you wanted to use the TCF values. Fine by me but
> I'd actually still keep 0 for disable (which is the default value I
> guess) with the 1 (or 3 later) for the upgrade. We may also add an
> mte_upgrade_asym file at some point.

So you want the settings to be:

0 - disable upgrading
1 - upgrade to sync
2 - upgrade to async
3 - upgrade to asym

Where 2 is disallowed? I guess that makes sense. So for
mte_upgrade_asym we would only allow writing 0 or 1 and for
mte_upgrade_none we would allow writing any value.

Peter

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