Re: [PATCH v6 01/18] arm64: cpufeature: Fix meta-capability cpufeature check

[Vincenzo Frascino <vincenzo.frascino@arm.com>]

Hi Amit,

On 3/6/20 6:35 AM, Amit Daniel Kachhap wrote:
> Some existing/future meta cpucaps match need the presence of individual
> cpucaps. Currently the individual cpucaps checks it via an array based
> flag and this introduces dependency on the array entry order.
> This limitation exists only for system scope cpufeature.
> 
> This patch introduces an internal helper function (__system_matches_cap)
> to invoke the matching handler for system scope. This helper has to be
> used during a narrow window when,
> - The system wide safe registers are set with all the SMP CPUs and,
> - The SYSTEM_FEATURE cpu_hwcaps may not have been set.
> 
> Normal users should use the existing cpus_have_{const_}cap() global
> function.
> 
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
> ---
>  arch/arm64/kernel/cpufeature.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 0b67156..3818685 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -116,6 +116,8 @@ cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
>  
>  static void cpu_enable_cnp(struct arm64_cpu_capabilities const *cap);
>  
> +static bool __system_matches_cap(unsigned int n);
> +
>  /*
>   * NOTE: Any changes to the visibility of features should be kept in
>   * sync with the documentation of the CPU feature register ABI.
> @@ -2146,6 +2148,17 @@ bool this_cpu_has_cap(unsigned int n)
>  	return false;
>  }
>  

Nit: you might want to add a comment on top of __system_matches_cap() that
explains why we introduce this function and when should be used.

Otherwise looks good to me.

Reviewed-by: Vincenzo Frascino <Vincenzo.Frascino@arm.com>

> +static bool __system_matches_cap(unsigned int n)
> +{
> +	if (n < ARM64_NCAPS) {
> +		const struct arm64_cpu_capabilities *cap = cpu_hwcaps_ptrs[n];
> +
> +		if (cap)
> +			return cap->matches(cap, SCOPE_SYSTEM);
> +	}
> +	return false;
> +}
> +
>  void cpu_set_feature(unsigned int num)
>  {
>  	WARN_ON(num >= MAX_CPU_FEATURES);
> @@ -2218,7 +2231,7 @@ void __init setup_cpu_features(void)
>  static bool __maybe_unused
>  cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
>  {
> -	return (cpus_have_const_cap(ARM64_HAS_PAN) && !cpus_have_const_cap(ARM64_HAS_UAO));
> +	return (__system_matches_cap(ARM64_HAS_PAN) && !__system_matches_cap(ARM64_HAS_UAO));
>  }
>  
>  static void __maybe_unused cpu_enable_cnp(struct arm64_cpu_capabilities const *cap)
> 

-- 
Regards,
Vincenzo

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