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

[Catalin Marinas <catalin.marinas@arm.com>]

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