Re: [PATCH 1/2] arm64: mte: Unify kernel MTE policy and manipulation of TCO

[Will Deacon <will@kernel.org>]

On Thu, Oct 30, 2025 at 08:49:31PM -0700, Carl Worth wrote:
> From: Taehyun Noh <taehyun@utexas.edu>
> 
> The kernel's primary knob for controlling MTE tag checking is the
> PSTATE.TCO bit (tag check override). TCO is enabled (which,
> confusingly _disables_ tag checking) by the hardware at the time of an
> exception. Then, at various times, when the kernel needs to enable
> tag-checking it clears TCO, (which in turn allows TCF0 or TCF to
> control whether tag-checking occurs).
> 
> Some of the TCO manipulation code has redundancy and confusing naming.
> 
> Fix the redundancy by introducing a new function user_uses_tagcheck
> which captures the existing repeated condition. The new function
> includes significant new comments to help explain the logic.
> 
> Fix the naming by renaming mte_disable_tco_entry() to
> set_kernel_mte_policy(). This function does not necessarily disable
> TCO, but does so only conditionally in the case of KASAN HW TAGS. The
> new name accurately describes the purpose of the function.
> 
> This commit should have no behavioral change.
> 
> Signed-off-by: Taehyun Noh <taehyun@utexas.edu>
> Co-developed-by: Carl Worth <carl@os.amperecomputing.com>
> Signed-off-by: Carl Worth <carl@os.amperecomputing.com>
> ---
>  arch/arm64/include/asm/mte.h     | 40 +++++++++++++++++++++++++++++++++-------
>  arch/arm64/kernel/entry-common.c |  4 ++--
>  arch/arm64/kernel/mte.c          |  2 +-
>  3 files changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index 3b5069f4683d..70dabc884616 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -224,7 +224,35 @@ static inline bool folio_try_hugetlb_mte_tagging(struct folio *folio)
>  }
>  #endif
>  
> -static inline void mte_disable_tco_entry(struct task_struct *task)
> +static inline bool user_uses_tagcheck(void)
> +{
> +	/*
> +	 * To decide whether userspace wants tag checking we only look
> +	 * at TCF0 (SCTLR_EL1.TCF0 bit 0 is set for both synchronous
> +	 * or asymmetric mode).
> +	 *
> +	 * There's an argument that could be made that the kernel
> +	 * should also consider the state of TCO (tag check override)
> +	 * since userspace does have the ability to set that as well,
> +	 * and that could suggest a desire to disable tag checking in
> +	 * spite of the state of TCF0. However, the Linux kernel has
> +	 * never historically considered the userspace state of TCO,
> +	 * (so changing this would be an ABI break), and the hardware
> +	 * unconditionally sets TCO when an exception occurs
> +	 * anyway.
> +	 *
> +	 * So, again, here we look only at TCF0 and do not consider
> +	 * TCO.
> +	 */
> +	return (current->thread.sctlr_user & (1UL << SCTLR_EL1_TCF0_SHIFT));
> +}
> +
> +/*
> + * Set the kernel's desired policy for MTE tag checking.
> + *
> + * This function should be used right after the kernel entry.
> + */
> +static inline void set_kernel_mte_policy(struct task_struct *task)
>  {
>  	if (!system_supports_mte())
>  		return;
> @@ -232,15 +260,13 @@ static inline void mte_disable_tco_entry(struct task_struct *task)
>  	/*
>  	 * Re-enable tag checking (TCO set on exception entry). This is only
>  	 * necessary if MTE is enabled in either the kernel or the userspace
> -	 * task in synchronous or asymmetric mode (SCTLR_EL1.TCF0 bit 0 is set
> -	 * for both). With MTE disabled in the kernel and disabled or
> -	 * asynchronous in userspace, tag check faults (including in uaccesses)
> -	 * are not reported, therefore there is no need to re-enable checking.
> +	 * task. With MTE disabled in the kernel and disabled or asynchronous
> +	 * in userspace, tag check faults (including in uaccesses) are not
> +	 * reported, therefore there is no need to re-enable checking.
>  	 * This is beneficial on microarchitectures where re-enabling TCO is
>  	 * expensive.

The comment implies that toggling TCO can be expensive, so it's not clear
to me that moving it to the uaccess routines in the next patch is
necessarily a good idea in general. I understand that you see improvements
with memcached, but have you tried exercising workloads that are heavy on
user accesses?

>  	 */
> -	if (kasan_hw_tags_enabled() ||
> -	    (task->thread.sctlr_user & (1UL << SCTLR_EL1_TCF0_SHIFT)))
> +	if (kasan_hw_tags_enabled() || user_uses_tagcheck())
>  		asm volatile(SET_PSTATE_TCO(0));
>  }
>  
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index f546a914f041..466562d1d966 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -49,7 +49,7 @@ static noinstr irqentry_state_t enter_from_kernel_mode(struct pt_regs *regs)
>  
>  	state = __enter_from_kernel_mode(regs);
>  	mte_check_tfsr_entry();
> -	mte_disable_tco_entry(current);
> +	set_kernel_mte_policy(current);
>  
>  	return state;
>  }
> @@ -83,7 +83,7 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs,
>  static __always_inline void __enter_from_user_mode(struct pt_regs *regs)
>  {
>  	enter_from_user_mode(regs);
> -	mte_disable_tco_entry(current);
> +	set_kernel_mte_policy(current);
>  }
>  
>  static __always_inline void arm64_enter_from_user_mode(struct pt_regs *regs)
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index 43f7a2f39403..0cc698714328 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -289,7 +289,7 @@ void mte_thread_switch(struct task_struct *next)
>  	mte_update_gcr_excl(next);
>  
>  	/* TCO may not have been disabled on exception entry for the current task. */
> -	mte_disable_tco_entry(next);
> +	set_kernel_mte_policy(next);

So this passes 'next' as the task, but user_uses_tagcheck() looks at
current. Shouldn't you propagate the task through?

Will