Re: [PATCH] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions

[Mark Rutland]

On Thu, May 07, 2020 at 03:26:30PM +0530, Anshuman Khandual wrote:
> All HWCAP name arrays (i.e hwcap_str, compat_hwcap_str, compat_hwcap2_str)
> that are scanned for /proc/cpuinfo output are detached from their bit fild
> definitions making it difficult to corelate. This is also bit problematic
> because during /proc/cpuinfo dump these arrays get traversed sequentially
> assuming that they reflect and match HWCAP bit sequence, to test various
> features for a given CPU.

Making this relationship explicit is certianly a good idea!

> This moves all HWCAP name arrays near their bit definitions. But first it
> defines all missing COMPAT_HWCAP_XXX that are present in the name string.
> Also defines all COMPAT_KERNEL_HWCAP macros to be used for the HWCAP name
> string indexing.

I think we don't need all the indirection here, and can simplify things
a bit. Comments on that below.

> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
> Cc: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> ---
> In longer term we might want to generate these HWCAP macros with scripting,
> and this is a step in that direction. This applies on 5.7-rc4.

Having attempted to build scripting for this in the past, it turns out
to be a bigger job than it seems initially, and I agree it'd make sense
to take a change like this without scripting as a first step.

>  arch/arm64/include/asm/hwcap.h | 132 +++++++++++++++++++++++++++++++++
>  arch/arm64/kernel/cpuinfo.c    |  90 ----------------------
>  2 files changed, 132 insertions(+), 90 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> index 0f00265248b5..cb99b5f75e5d 100644
> --- a/arch/arm64/include/asm/hwcap.h
> +++ b/arch/arm64/include/asm/hwcap.h
> @@ -8,18 +8,27 @@
>  #include <uapi/asm/hwcap.h>
>  #include <asm/cpufeature.h>
>  
> +#define COMPAT_HWCAP_SWP	(1 << 0)
>  #define COMPAT_HWCAP_HALF	(1 << 1)
>  #define COMPAT_HWCAP_THUMB	(1 << 2)
> +#define COMPAT_HWCAP_26BIT	(1 << 3)
>  #define COMPAT_HWCAP_FAST_MULT	(1 << 4)
> +#define COMPAT_HWCAP_FPA	(1 << 5)
>  #define COMPAT_HWCAP_VFP	(1 << 6)
>  #define COMPAT_HWCAP_EDSP	(1 << 7)
> +#define COMPAT_HWCAP_JAVA	(1 << 8)
> +#define COMPAT_HWCAP_IWMMXT	(1 << 9)
> +#define COMPAT_HWCAP_CRUNCH	(1 << 10)
> +#define COMPAT_HWCAP_THUMBEE	(1 << 11)
>  #define COMPAT_HWCAP_NEON	(1 << 12)
>  #define COMPAT_HWCAP_VFPv3	(1 << 13)
> +#define COMPAT_HWCAP_VFPV3D16	(1 << 14)
>  #define COMPAT_HWCAP_TLS	(1 << 15)
>  #define COMPAT_HWCAP_VFPv4	(1 << 16)
>  #define COMPAT_HWCAP_IDIVA	(1 << 17)
>  #define COMPAT_HWCAP_IDIVT	(1 << 18)
>  #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
> +#define COMPAT_HWCAP_VFPD32	(1 << 19)
>  #define COMPAT_HWCAP_LPAE	(1 << 20)
>  #define COMPAT_HWCAP_EVTSTRM	(1 << 21)
>  
> @@ -95,6 +104,129 @@
>  #define KERNEL_HWCAP_DGH		__khwcap2_feature(DGH)
>  #define KERNEL_HWCAP_RNG		__khwcap2_feature(RNG)
>  
> +#ifdef CONFIG_COMPAT
> +#define __compat_khwcap_feature(x)	const_ilog2(COMPAT_HWCAP_ ## x)

For now I reckon this can be:

#define COMPAT_KERNEL_HWCAP(x)		const_ilog2(COMPAT_HWCAP_##x)

> +#define COMPAT_KERNEL_HWCAP_SWP		__compat_khwcap_feature(SWP)
> +#define COMPAT_KERNEL_HWCAP_HALF	__compat_khwcap_feature(HALF)
> +#define COMPAT_KERNEL_HWCAP_THUMB	__compat_khwcap_feature(THUMB)
> +#define COMPAT_KERNEL_HWCAP_26BIT	__compat_khwcap_feature(26BIT)
> +#define COMPAT_KERNEL_HWCAP_FAST_MULT	__compat_khwcap_feature(FAST_MULT)
> +#define COMPAT_KERNEL_HWCAP_FPA		__compat_khwcap_feature(FPA)
> +#define COMPAT_KERNEL_HWCAP_VFP		__compat_khwcap_feature(VFP)
> +#define COMPAT_KERNEL_HWCAP_EDSP	__compat_khwcap_feature(EDSP)
> +#define COMPAT_KERNEL_HWCAP_JAVA	__compat_khwcap_feature(JAVA)
> +#define COMPAT_KERNEL_HWCAP_IWMMXT	__compat_khwcap_feature(IWMMXT)
> +#define COMPAT_KERNEL_HWCAP_CRUNCH	__compat_khwcap_feature(CRUNCH)
> +#define COMPAT_KERNEL_HWCAP_THUMBEE	__compat_khwcap_feature(THUMBEE)
> +#define COMPAT_KERNEL_HWCAP_NEON	__compat_khwcap_feature(NEON)
> +#define COMPAT_KERNEL_HWCAP_VFPv3	__compat_khwcap_feature(VFPv3)
> +#define COMPAT_KERNEL_HWCAP_VFPV3D16	__compat_khwcap_feature(VFPV3D16)
> +#define COMPAT_KERNEL_HWCAP_TLS		__compat_khwcap_feature(TLS)
> +#define COMPAT_KERNEL_HWCAP_VFPv4	__compat_khwcap_feature(VFPv4)
> +#define COMPAT_KERNEL_HWCAP_IDIVA	__compat_khwcap_feature(IDIVA)
> +#define COMPAT_KERNEL_HWCAP_IDIVT	__compat_khwcap_feature(IDIVT)
> +#define COMPAT_KERNEL_HWCAP_VFPD32	__compat_khwcap_feature(VFPD32)
> +#define COMPAT_KERNEL_HWCAP_LPAE	__compat_khwcap_feature(LPAE)
> +#define COMPAT_KERNEL_HWCAP_EVTSTRM	__compat_khwcap_feature(EVTSTRM)

... so we don't need any of the above, as e.g. we can just do:

	[COMPAT_KERNEL_HWCAP(SWP)] = "swp",

... at the points of usage.

> +#define __compat_khwcap2_feature(x)	const_ilog2(COMPAT_HWCAP2_ ## x)
> +#define COMPAT_KERNEL_HWCAP_AES		__compat_khwcap2_feature(AES)
> +#define COMPAT_KERNEL_HWCAP_PMULL	__compat_khwcap2_feature(PMULL)
> +#define COMPAT_KERNEL_HWCAP_SHA1	__compat_khwcap2_feature(SHA1)
> +#define COMPAT_KERNEL_HWCAP_SHA2	__compat_khwcap2_feature(SHA2)
> +#define COMPAT_KERNEL_HWCAP_CRC32	__compat_khwcap2_feature(CRC32)

Likewise here with:

#define COMPAT_KERNEL_HWCAP2(x)		const_ilog2(COMPAT_HWCAP2_##x)

With that, and the usage in the arrays changed to match:

Acked-by: Mark Rutland <mark.rutland@xxxxxxx>

Thanks,
Mark.