[PATCH] arm64: errata: Fix handling of 1418040 with late CPU onlining

[Suzuki K Poulose]

Hi Will,

On 11/6/20 11:49 AM, Will Deacon wrote:
> In a surprising turn of events, it transpires that CPU capabilities
> configured as ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE are never set as the
> result of late-onlining. Therefore our handling of erratum 1418040 does
> not get activated if it is not required by any of the boot CPUs, even
> though we allow late-onlining of an affected CPU.

The capability state is not altered after the SMP boot for all types
of caps. The weak caps are there to allow a late CPU to turn online
without getting "banned". This may be something we could relax with
a new flag in the scope.

> 
> In order to get things working again, replace the cpus_have_const_cap()
> invocation with an explicit check for the current CPU using
> this_cpu_has_cap().
> 
> Cc: Marc Zyngier <maz at kernel.org>
> Cc: Sai Prakash Ranjan <saiprakash.ranjan at codeaurora.org>
> Cc: Stephen Boyd <swboyd at chromium.org>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: Mark Rutland <mark.rutland at arm.com>
> Cc: Suzuki Poulose <suzuki.poulose at arm.com>
> Signed-off-by: Will Deacon <will at kernel.org>
> ---
> 
> Found by code inspection and compile-tested only, so I would really
> appreciate a second look.
> 
>   arch/arm64/include/asm/cpufeature.h | 2 ++
>   arch/arm64/kernel/process.c         | 5 ++---
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index f7e7144af174..c59c16a6ea8b 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -268,6 +268,8 @@ extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;
>   /*
>    * CPU feature detected at boot time based on feature of one or more CPUs.
>    * All possible conflicts for a late CPU are ignored.
> + * NOTE: this means that a late CPU with the feature will *not* cause the
> + * capability to be advertised by cpus_have_*cap()!

This comment applies to all the types, so it may be confusing. And the
comment already mentions that the feature is detected at boot time.

>    */
>   #define ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE		\
>   	(ARM64_CPUCAP_SCOPE_LOCAL_CPU		|	\
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 4784011cecac..a47a40ec6ad9 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -522,14 +522,13 @@ static void erratum_1418040_thread_switch(struct task_struct *prev,
>   	bool prev32, next32;
>   	u64 val;
>   
> -	if (!(IS_ENABLED(CONFIG_ARM64_ERRATUM_1418040) &&
> -	      cpus_have_const_cap(ARM64_WORKAROUND_1418040)))
> +	if (!IS_ENABLED(CONFIG_ARM64_ERRATUM_1418040))
>   		return;
>   
>   	prev32 = is_compat_thread(task_thread_info(prev));
>   	next32 = is_compat_thread(task_thread_info(next));
>   
> -	if (prev32 == next32)
> +	if (prev32 == next32 || !this_cpu_has_cap(ARM64_WORKAROUND_1418040))
>   		return;
>   
>   	val = read_sysreg(cntkctl_el1);
> 


This change as such looks good to me.

Reviewed-by: Suzuki K Poulose <suzuki.poulose at arm.com>