Re: [PATCH v2] arm64: Implement prctl(PR_{G,S}ET_TSC)

[Will Deacon <will@kernel.org>]

On Fri, May 17, 2024 at 02:25:51PM -0700, Peter Collingbourne wrote:
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 4ae31b7af6c31..1a2ae7830c179 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -43,6 +43,7 @@
>  #include <linux/stacktrace.h>
>  
>  #include <asm/alternative.h>
> +#include <asm/arch_timer.h>
>  #include <asm/compat.h>
>  #include <asm/cpufeature.h>
>  #include <asm/cacheflush.h>
> @@ -472,27 +473,49 @@ static void entry_task_switch(struct task_struct *next)
>  }
>  
>  /*
> - * ARM erratum 1418040 handling, affecting the 32bit view of CNTVCT.
> - * Ensure access is disabled when switching to a 32bit task, ensure
> - * access is enabled when switching to a 64bit task.
> + * Handle sysreg updates for ARM erratum 1418040 which affects the 32bit view of
> + * CNTVCT, various other errata which require trapping all CNTVCT{,_EL0}
> + * accesses and prctl(PR_SET_TSC). Ensure access is disabled iff a workaround is
> + * required or PR_TSC_SIGSEGV is set.
>   */
> -static void erratum_1418040_thread_switch(struct task_struct *next)
> +static void update_cntkctl_el1(struct task_struct *next)
>  {
> -	if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_1418040) ||
> -	    !this_cpu_has_cap(ARM64_WORKAROUND_1418040))
> -		return;
> +	struct thread_info *ti = task_thread_info(next);
>  
> -	if (is_compat_thread(task_thread_info(next)))
> +	if (test_ti_thread_flag(ti, TIF_TSC_SIGSEGV) ||
> +	    has_erratum_handler(read_cntvct_el0) ||
> +	    (IS_ENABLED(CONFIG_ARM64_ERRATUM_1418040) &&
> +	     this_cpu_has_cap(ARM64_WORKAROUND_1418040) &&
> +	     is_compat_thread(ti)))
>  		sysreg_clear_set(cntkctl_el1, ARCH_TIMER_USR_VCT_ACCESS_EN, 0);
>  	else
>  		sysreg_clear_set(cntkctl_el1, 0, ARCH_TIMER_USR_VCT_ACCESS_EN);
>  }
>  
> -static void erratum_1418040_new_exec(void)
> +static void cntkctl_thread_switch(struct task_struct *prev,
> +				  struct task_struct *next)
> +{
> +	if ((read_ti_thread_flags(task_thread_info(prev)) &
> +	     (_TIF_32BIT | _TIF_TSC_SIGSEGV)) !=
> +	    (read_ti_thread_flags(task_thread_info(next)) &
> +	     (_TIF_32BIT | _TIF_TSC_SIGSEGV)))
> +		update_cntkctl_el1(next);
> +}
> +
> +static int do_set_tsc_mode(unsigned int val)
>  {
> +	if (val == PR_TSC_SIGSEGV)
> +		set_thread_flag(TIF_TSC_SIGSEGV);
> +	else if (val == PR_TSC_ENABLE)
> +		clear_thread_flag(TIF_TSC_SIGSEGV);
> +	else
> +		return -EINVAL;

Aren't we in trouble if we get preempted at this point? The flag is
out-of-sync with the register, so I worry that we could fail to set the
controls correctly for the next task.

Will