Re: [PATCH v3 3/7] arm64: HWCAP: encapsulate elf_hwcap

[Dave Martin <Dave.Martin@arm.com>]

On Mon, Apr 01, 2019 at 11:45:11AM +0100, Andrew Murray wrote:
> The introduction of AT_HWCAP2 introduced accessors which ensure that
> hwcap features are set and tested appropriately.
> 
> Let's now mandate access to elf_hwcap via these accessors by making
> elf_hwcap static within cpufeature.c.

Looks reasonable except for a couple of minor nits below.

I had wondered whether putting these accessors out of line would affect
any hot paths, but I can't see these used from anything that looks like
a hot path.  So we're probably fine.

cpus_have_const_cap() is preferred for places where this matters,
anyway.

[...]

> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 986ceeacd19f..84ca52fa75e5 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -35,8 +35,7 @@
>  #include <asm/traps.h>
>  #include <asm/virt.h>
>  
> -unsigned long elf_hwcap __read_mostly;
> -EXPORT_SYMBOL_GPL(elf_hwcap);
> +static unsigned long elf_hwcap __read_mostly;

Now that this doesn't correspond directly to ELF_HWCAP any more and we
hide it, can we rename it to avoid confusion?

Maybe "kernel_hwcap"?

>  #ifdef CONFIG_COMPAT
>  #define COMPAT_ELF_HWCAP_DEFAULT	\
> @@ -1947,6 +1946,35 @@ bool this_cpu_has_cap(unsigned int n)
>  	return false;
>  }
>  
> +void cpu_set_feature(unsigned int num)
> +{
> +	WARN_ON(num >= MAX_CPU_FEATURES);
> +	elf_hwcap |= BIT(num);
> +}
> +EXPORT_SYMBOL_GPL(cpu_set_feature);
> +
> +bool cpu_have_feature(unsigned int num)
> +{
> +	WARN_ON(num >= MAX_CPU_FEATURES);
> +	return elf_hwcap & BIT(num);
> +}
> +EXPORT_SYMBOL_GPL(cpu_have_feature);
> +
> +unsigned long cpu_get_elf_hwcap(void)
> +{
> +	/*
> +	 * We currently only populate the first 32 bits of AT_HWCAP. Please
> +	 * note that for userspace compatibility we guarantee that bit 62
> +	 * will always be returned as 0.
> +	 */

Presumably also bit 63?

It is reasonable to say this here, but I think there should also be a
note in Documentation/arm64/elf_hwcaps.txt.

[...]

Cheers
---Dave