linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions
@ 2020-05-07 13:29 Anshuman Khandual
  2020-05-13 15:04 ` Dave Martin
  0 siblings, 1 reply; 7+ messages in thread
From: Anshuman Khandual @ 2020-05-07 13:29 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mark Brown,
	Ard Biesheuvel, Mark Rutland, Suzuki K Poulose, linux-kernel

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.

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.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
This applies on 5.7-rc4

Changes in V2:

- Defined COMPAT_KERNEL_HWCAP[2] and updated the name arrays per Mark
- Updated the commit message as required

Changes in V1: (https://patchwork.kernel.org/patch/11532945/)

 arch/arm64/include/asm/hwcap.h | 101 +++++++++++++++++++++++++++++++++
 arch/arm64/kernel/cpuinfo.c    |  90 -----------------------------
 2 files changed, 101 insertions(+), 90 deletions(-)

diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index 0f00265248b5..589ac02e1ddd 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,98 @@
 #define KERNEL_HWCAP_DGH		__khwcap2_feature(DGH)
 #define KERNEL_HWCAP_RNG		__khwcap2_feature(RNG)
 
+static const char *const hwcap_str[] = {
+	[KERNEL_HWCAP_FP]		= "fp",
+	[KERNEL_HWCAP_ASIMD]		= "asimd",
+	[KERNEL_HWCAP_EVTSTRM]		= "evtstrm",
+	[KERNEL_HWCAP_AES]		= "aes",
+	[KERNEL_HWCAP_PMULL]		= "pmull",
+	[KERNEL_HWCAP_SHA1]		= "sha1",
+	[KERNEL_HWCAP_SHA2]		= "sha2",
+	[KERNEL_HWCAP_CRC32]		= "crc32",
+	[KERNEL_HWCAP_ATOMICS]		= "atomics",
+	[KERNEL_HWCAP_FPHP]		= "fphp",
+	[KERNEL_HWCAP_ASIMDHP]		= "asimdhp",
+	[KERNEL_HWCAP_CPUID]		= "cpuid",
+	[KERNEL_HWCAP_ASIMDRDM]		= "asimdrdm",
+	[KERNEL_HWCAP_JSCVT]		= "jscvt",
+	[KERNEL_HWCAP_FCMA]		= "fcma",
+	[KERNEL_HWCAP_LRCPC]		= "lrcpc",
+	[KERNEL_HWCAP_DCPOP]		= "dcpop",
+	[KERNEL_HWCAP_SHA3]		= "sha3",
+	[KERNEL_HWCAP_SM3]		= "sm3",
+	[KERNEL_HWCAP_SM4]		= "sm4",
+	[KERNEL_HWCAP_ASIMDDP]		= "asimddp",
+	[KERNEL_HWCAP_SHA512]		= "sha512",
+	[KERNEL_HWCAP_SVE]		= "sve",
+	[KERNEL_HWCAP_ASIMDFHM]		= "asimdfhm",
+	[KERNEL_HWCAP_DIT]		= "dit",
+	[KERNEL_HWCAP_USCAT]		= "uscat",
+	[KERNEL_HWCAP_ILRCPC]		= "ilrcpc",
+	[KERNEL_HWCAP_FLAGM]		= "flagm",
+	[KERNEL_HWCAP_SSBS]		= "ssbs",
+	[KERNEL_HWCAP_SB]		= "sb",
+	[KERNEL_HWCAP_PACA]		= "paca",
+	[KERNEL_HWCAP_PACG]		= "pacg",
+	[KERNEL_HWCAP_DCPODP]		= "dcpodp",
+	[KERNEL_HWCAP_SVE2]		= "sve2",
+	[KERNEL_HWCAP_SVEAES]		= "sveaes",
+	[KERNEL_HWCAP_SVEPMULL]		= "svepmull",
+	[KERNEL_HWCAP_SVEBITPERM]	= "svebitperm",
+	[KERNEL_HWCAP_SVESHA3]		= "svesha3",
+	[KERNEL_HWCAP_SVESM4]		= "svesm4",
+	[KERNEL_HWCAP_FLAGM2]		= "flagm2",
+	[KERNEL_HWCAP_FRINT]		= "frint",
+	[KERNEL_HWCAP_SVEI8MM]		= "svei8mm",
+	[KERNEL_HWCAP_SVEF32MM]		= "svef32mm",
+	[KERNEL_HWCAP_SVEF64MM]		= "svef64mm",
+	[KERNEL_HWCAP_SVEBF16]		= "svebf16",
+	[KERNEL_HWCAP_I8MM]		= "i8mm",
+	[KERNEL_HWCAP_BF16]		= "bf16",
+	[KERNEL_HWCAP_DGH]		= "dgh",
+	[KERNEL_HWCAP_RNG]		= "rng",
+	NULL
+};
+
+#ifdef CONFIG_COMPAT
+#define COMPAT_KERNEL_HWCAP(x)	const_ilog2(COMPAT_HWCAP_ ## x)
+static const char *const compat_hwcap_str[] = {
+	[COMPAT_KERNEL_HWCAP(SWP)]	= "swp",
+	[COMPAT_KERNEL_HWCAP(HALF)]	= "half",
+	[COMPAT_KERNEL_HWCAP(THUMB)]	= "thumb",
+	[COMPAT_KERNEL_HWCAP(26BIT)]	= "26bit",
+	[COMPAT_KERNEL_HWCAP(FAST_MULT)] = "fastmult",
+	[COMPAT_KERNEL_HWCAP(FPA)]	= "fpa",
+	[COMPAT_KERNEL_HWCAP(VFP)]	= "vfp",
+	[COMPAT_KERNEL_HWCAP(EDSP)]	= "edsp",
+	[COMPAT_KERNEL_HWCAP(JAVA)]	= "java",
+	[COMPAT_KERNEL_HWCAP(IWMMXT)]	= "iwmmxt",
+	[COMPAT_KERNEL_HWCAP(CRUNCH)]	= "crunch",
+	[COMPAT_KERNEL_HWCAP(THUMBEE)]	= "thumbee",
+	[COMPAT_KERNEL_HWCAP(NEON)]	= "neon",
+	[COMPAT_KERNEL_HWCAP(VFPv3)]	= "vfpv3",
+	[COMPAT_KERNEL_HWCAP(VFPV3D16)]	= "vfpv3d16",
+	[COMPAT_KERNEL_HWCAP(TLS)]	= "tls",
+	[COMPAT_KERNEL_HWCAP(VFPv4)]	= "vfpv4",
+	[COMPAT_KERNEL_HWCAP(IDIVA)]	= "idiva",
+	[COMPAT_KERNEL_HWCAP(IDIVT)]	= "idivt",
+	[COMPAT_KERNEL_HWCAP(VFPD32)]	= "vfpd32",
+	[COMPAT_KERNEL_HWCAP(LPAE)]	= "lpae",
+	[COMPAT_KERNEL_HWCAP(EVTSTRM)]	= "evtstrm",
+	NULL
+};
+
+#define COMPAT_KERNEL_HWCAP2(x)	const_ilog2(COMPAT_HWCAP2_ ## x)
+static const char *const compat_hwcap2_str[] = {
+	[COMPAT_KERNEL_HWCAP2(AES)]	= "aes",
+	[COMPAT_KERNEL_HWCAP2(PMULL)]	= "pmull",
+	[COMPAT_KERNEL_HWCAP2(SHA1)]	= "sha1",
+	[COMPAT_KERNEL_HWCAP2(SHA2)]	= "sha2",
+	[COMPAT_KERNEL_HWCAP2(CRC32)]	= "crc32",
+	NULL,
+};
+#endif /* CONFIG_COMPAT */
+
 /*
  * This yields a mask that user programs can use to figure out what
  * instruction set this cpu supports.
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 86136075ae41..f45f8614e8bd 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -42,96 +42,6 @@ static const char *icache_policy_str[] = {
 
 unsigned long __icache_flags;
 
-static const char *const hwcap_str[] = {
-	"fp",
-	"asimd",
-	"evtstrm",
-	"aes",
-	"pmull",
-	"sha1",
-	"sha2",
-	"crc32",
-	"atomics",
-	"fphp",
-	"asimdhp",
-	"cpuid",
-	"asimdrdm",
-	"jscvt",
-	"fcma",
-	"lrcpc",
-	"dcpop",
-	"sha3",
-	"sm3",
-	"sm4",
-	"asimddp",
-	"sha512",
-	"sve",
-	"asimdfhm",
-	"dit",
-	"uscat",
-	"ilrcpc",
-	"flagm",
-	"ssbs",
-	"sb",
-	"paca",
-	"pacg",
-	"dcpodp",
-	"sve2",
-	"sveaes",
-	"svepmull",
-	"svebitperm",
-	"svesha3",
-	"svesm4",
-	"flagm2",
-	"frint",
-	"svei8mm",
-	"svef32mm",
-	"svef64mm",
-	"svebf16",
-	"i8mm",
-	"bf16",
-	"dgh",
-	"rng",
-	NULL
-};
-
-#ifdef CONFIG_COMPAT
-static const char *const compat_hwcap_str[] = {
-	"swp",
-	"half",
-	"thumb",
-	"26bit",
-	"fastmult",
-	"fpa",
-	"vfp",
-	"edsp",
-	"java",
-	"iwmmxt",
-	"crunch",
-	"thumbee",
-	"neon",
-	"vfpv3",
-	"vfpv3d16",
-	"tls",
-	"vfpv4",
-	"idiva",
-	"idivt",
-	"vfpd32",
-	"lpae",
-	"evtstrm",
-	NULL
-};
-
-static const char *const compat_hwcap2_str[] = {
-	"aes",
-	"pmull",
-	"sha1",
-	"sha2",
-	"crc32",
-	NULL
-};
-#endif /* CONFIG_COMPAT */
-
 static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions
  2020-05-07 13:29 [PATCH V2] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions Anshuman Khandual
@ 2020-05-13 15:04 ` Dave Martin
  2020-05-14  1:44   ` Anshuman Khandual
  0 siblings, 1 reply; 7+ messages in thread
From: Dave Martin @ 2020-05-13 15:04 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, Mark Rutland, Catalin Marinas,
	Suzuki K Poulose, linux-kernel, Mark Brown, Will Deacon,
	Ard Biesheuvel

On Thu, May 07, 2020 at 06:59:10PM +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.
> 
> 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.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> ---
> This applies on 5.7-rc4
> 
> Changes in V2:
> 
> - Defined COMPAT_KERNEL_HWCAP[2] and updated the name arrays per Mark
> - Updated the commit message as required
> 
> Changes in V1: (https://patchwork.kernel.org/patch/11532945/)
> 
>  arch/arm64/include/asm/hwcap.h | 101 +++++++++++++++++++++++++++++++++
>  arch/arm64/kernel/cpuinfo.c    |  90 -----------------------------
>  2 files changed, 101 insertions(+), 90 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> index 0f00265248b5..589ac02e1ddd 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)

With the possible exception of SWP (does the swp emulation allow us to
report this as supported?), I think all these weren't mentioned because
they aren't included in ARMv8 and so can never be reported.

If we find ourselves reporting them, there's a bug somewhere.

So, can we just default all obsolete string entries to NULL?

When generating the cpuinfo strings we could WARN and just emit an empty
string for that hwcap.

Cheers
---Dave

[...]

> +#ifdef CONFIG_COMPAT
> +#define COMPAT_KERNEL_HWCAP(x)	const_ilog2(COMPAT_HWCAP_ ## x)
> +static const char *const compat_hwcap_str[] = {
> +	[COMPAT_KERNEL_HWCAP(SWP)]	= "swp",
> +	[COMPAT_KERNEL_HWCAP(HALF)]	= "half",
> +	[COMPAT_KERNEL_HWCAP(THUMB)]	= "thumb",
> +	[COMPAT_KERNEL_HWCAP(26BIT)]	= "26bit",
> +	[COMPAT_KERNEL_HWCAP(FAST_MULT)] = "fastmult",
> +	[COMPAT_KERNEL_HWCAP(FPA)]	= "fpa",
> +	[COMPAT_KERNEL_HWCAP(VFP)]	= "vfp",
> +	[COMPAT_KERNEL_HWCAP(EDSP)]	= "edsp",
> +	[COMPAT_KERNEL_HWCAP(JAVA)]	= "java",
> +	[COMPAT_KERNEL_HWCAP(IWMMXT)]	= "iwmmxt",
> +	[COMPAT_KERNEL_HWCAP(CRUNCH)]	= "crunch",
> +	[COMPAT_KERNEL_HWCAP(THUMBEE)]	= "thumbee",
> +	[COMPAT_KERNEL_HWCAP(NEON)]	= "neon",
> +	[COMPAT_KERNEL_HWCAP(VFPv3)]	= "vfpv3",
> +	[COMPAT_KERNEL_HWCAP(VFPV3D16)]	= "vfpv3d16",
> +	[COMPAT_KERNEL_HWCAP(TLS)]	= "tls",
> +	[COMPAT_KERNEL_HWCAP(VFPv4)]	= "vfpv4",
> +	[COMPAT_KERNEL_HWCAP(IDIVA)]	= "idiva",
> +	[COMPAT_KERNEL_HWCAP(IDIVT)]	= "idivt",
> +	[COMPAT_KERNEL_HWCAP(VFPD32)]	= "vfpd32",
> +	[COMPAT_KERNEL_HWCAP(LPAE)]	= "lpae",
> +	[COMPAT_KERNEL_HWCAP(EVTSTRM)]	= "evtstrm",
> +	NULL
> +};
> +
> +#define COMPAT_KERNEL_HWCAP2(x)	const_ilog2(COMPAT_HWCAP2_ ## x)
> +static const char *const compat_hwcap2_str[] = {
> +	[COMPAT_KERNEL_HWCAP2(AES)]	= "aes",
> +	[COMPAT_KERNEL_HWCAP2(PMULL)]	= "pmull",
> +	[COMPAT_KERNEL_HWCAP2(SHA1)]	= "sha1",
> +	[COMPAT_KERNEL_HWCAP2(SHA2)]	= "sha2",
> +	[COMPAT_KERNEL_HWCAP2(CRC32)]	= "crc32",
> +	NULL,
> +};
> +#endif /* CONFIG_COMPAT */
> +

[...]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions
  2020-05-13 15:04 ` Dave Martin
@ 2020-05-14  1:44   ` Anshuman Khandual
  2020-05-14  7:36     ` Will Deacon
  0 siblings, 1 reply; 7+ messages in thread
From: Anshuman Khandual @ 2020-05-14  1:44 UTC (permalink / raw)
  To: Dave Martin
  Cc: linux-arm-kernel, Mark Rutland, Catalin Marinas,
	Suzuki K Poulose, linux-kernel, Mark Brown, Will Deacon,
	Ard Biesheuvel



On 05/13/2020 08:34 PM, Dave Martin wrote:
> On Thu, May 07, 2020 at 06:59:10PM +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.
>>
>> 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.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: Ard Biesheuvel <ardb@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> Acked-by: Mark Rutland <mark.rutland@arm.com>
>> ---
>> This applies on 5.7-rc4
>>
>> Changes in V2:
>>
>> - Defined COMPAT_KERNEL_HWCAP[2] and updated the name arrays per Mark
>> - Updated the commit message as required
>>
>> Changes in V1: (https://patchwork.kernel.org/patch/11532945/)
>>
>>  arch/arm64/include/asm/hwcap.h | 101 +++++++++++++++++++++++++++++++++
>>  arch/arm64/kernel/cpuinfo.c    |  90 -----------------------------
>>  2 files changed, 101 insertions(+), 90 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
>> index 0f00265248b5..589ac02e1ddd 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)
> 
> With the possible exception of SWP (does the swp emulation allow us to
> report this as supported?), I think all these weren't mentioned because
> they aren't included in ARMv8 and so can never be reported.
> 
> If we find ourselves reporting them, there's a bug somewhere.
> 
> So, can we just default all obsolete string entries to NULL?
> 
> When generating the cpuinfo strings we could WARN and just emit an empty
> string for that hwcap.

All these above will be a change in the existing user visible behavior on
the system and this patch never intended to create one. Hence, I will just
defer this to maintainers on whether we should change existing /proc/cpuinfo
output (including non-practically-possible ones on ARMv8) or even treat swap
emulation as SWP.

But IMHO, in case we decide to go for a /proc/cpuinfo change (which I would
be happy implement), it should be applied over and above this patch which
just preserved existing semantics and behavior.

- Anshuman

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions
  2020-05-14  1:44   ` Anshuman Khandual
@ 2020-05-14  7:36     ` Will Deacon
  2020-05-15  3:28       ` Anshuman Khandual
  0 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2020-05-14  7:36 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Dave Martin, linux-arm-kernel, Mark Rutland, Catalin Marinas,
	Suzuki K Poulose, linux-kernel, Mark Brown, Ard Biesheuvel

On Thu, May 14, 2020 at 07:14:58AM +0530, Anshuman Khandual wrote:
> On 05/13/2020 08:34 PM, Dave Martin wrote:
> > On Thu, May 07, 2020 at 06:59:10PM +0530, Anshuman Khandual wrote:
> >> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> >> index 0f00265248b5..589ac02e1ddd 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)
> > 
> > With the possible exception of SWP (does the swp emulation allow us to
> > report this as supported?), I think all these weren't mentioned because
> > they aren't included in ARMv8 and so can never be reported.
> > 
> > If we find ourselves reporting them, there's a bug somewhere.
> > 
> > So, can we just default all obsolete string entries to NULL?
> > 
> > When generating the cpuinfo strings we could WARN and just emit an empty
> > string for that hwcap.
> 
> All these above will be a change in the existing user visible behavior on
> the system and this patch never intended to create one.

Why is it a change? We've never reported e.g. "java" on an arm64 kernel, so
I agree with Dave that we shouldn't be adding this string. If it /ever/ ends
up in userspace it's because something has gone horribly wrong. NULL would
be much better. Couldn't you achieve that by simply omitting these entries?
e.g. deleting things like:

	[COMPAT_KERNEL_HWCAP(JAVA)]     = "java",

completely (including the COMPAT_HWCAP_JAVA definition)?

> Hence, I will just defer this to maintainers on whether we should change
> existing /proc/cpuinfo output (including non-practically-possible ones on
> ARMv8) or even treat swap emulation as SWP.

SWP is fine because we emulate it and so userspace can use it. Removing that
*would* be a change in behaviour. I don't think the compat ABI is broken
here, so please don't change it without good reason.

Will

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions
  2020-05-14  7:36     ` Will Deacon
@ 2020-05-15  3:28       ` Anshuman Khandual
  2020-05-18 22:35         ` Will Deacon
  0 siblings, 1 reply; 7+ messages in thread
From: Anshuman Khandual @ 2020-05-15  3:28 UTC (permalink / raw)
  To: Will Deacon
  Cc: Dave Martin, linux-arm-kernel, Mark Rutland, Catalin Marinas,
	Suzuki K Poulose, linux-kernel, Mark Brown, Ard Biesheuvel



On 05/14/2020 01:06 PM, Will Deacon wrote:
> On Thu, May 14, 2020 at 07:14:58AM +0530, Anshuman Khandual wrote:
>> On 05/13/2020 08:34 PM, Dave Martin wrote:
>>> On Thu, May 07, 2020 at 06:59:10PM +0530, Anshuman Khandual wrote:
>>>> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
>>>> index 0f00265248b5..589ac02e1ddd 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)
>>>
>>> With the possible exception of SWP (does the swp emulation allow us to
>>> report this as supported?), I think all these weren't mentioned because
>>> they aren't included in ARMv8 and so can never be reported.
>>>
>>> If we find ourselves reporting them, there's a bug somewhere.
>>>
>>> So, can we just default all obsolete string entries to NULL?
>>>
>>> When generating the cpuinfo strings we could WARN and just emit an empty
>>> string for that hwcap.
>>
>> All these above will be a change in the existing user visible behavior on
>> the system and this patch never intended to create one.
> 
> Why is it a change? We've never reported e.g. "java" on an arm64 kernel, so

We already have "java" defined in existing compat_hwcap_str[] array even
though it might never get set in compat_elf_hwcap. AFAICS, compat_elf_hwcap
will have the following capabilities set (at the most).

Via COMPAT_ELF_HWCAP_DEFAULT

01.  COMPAT_HWCAP_HALF
02.  COMPAT_HWCAP_THUMB
03.  COMPAT_HWCAP_FAST_MULT
04.  COMPAT_HWCAP_EDSP
05.  COMPAT_HWCAP_TLS
06.  COMPAT_HWCAP_IDIV
07.  COMPAT_HWCAP_LPAE

Via setup_elf_hwcaps(compat_elf_hwcaps) <-- setup_cpu_features()

8.  COMPAT_HWCAP_NEON
9.  COMPAT_HWCAP_VFPv4
10. COMPAT_HWCAP_VFP
11. COMPAT_HWCAP_VFPv3

Via arch_timer_set_evtstrm_feature()

12. COMPAT_HWCAP_EVTSTRM

The code exists for "java" string to be displayed with /proc/cpuinfo but it
may never get triggered as compat_elf_hwcap will never have JAVA capability
unless there is a bug as you had rightly mentioned.

The current patch preserves status quo without fixing the underlying problem
("java" should have been dropped from possible /proc/cpuinfo display strings
as it can never happen). It tries to formalize what is already being done.

arch/arm64/kernel/cpuinfo.c

static int c_show(struct seq_file *m, void *v)
{
	....
#ifdef CONFIG_COMPAT
	for (j = 0; compat_hwcap_str[j]; j++)
		if (compat_elf_hwcap & (1 << j))
		seq_printf(m, " %s", compat_hwcap_str[j]);
	....
#endif
	....
}

When compat_hwcap_str[j] == "java" just define COMPAT_HWCAP_JAVA as (1 << j).

> I agree with Dave that we shouldn't be adding this string. If it /ever/ ends
> up in userspace it's because something has gone horribly wrong. NULL would
> be much better. Couldn't you achieve that by simply omitting these entries?
> e.g. deleting things like:
> 
> 	[COMPAT_KERNEL_HWCAP(JAVA)]     = "java",
> 
> completely (including the COMPAT_HWCAP_JAVA definition)?

Right, dropping these above will fix the existing underlying problem. But the
only point here is should that be done in this patch (which keeps everything
as is) or with a separate patch fixing an already existing problem.

> 
>> Hence, I will just defer this to maintainers on whether we should change
>> existing /proc/cpuinfo output (including non-practically-possible ones on
>> ARMv8) or even treat swap emulation as SWP.
> 
> SWP is fine because we emulate it and so userspace can use it. Removing that
> *would* be a change in behaviour. I don't think the compat ABI is broken
> here, so please don't change it without good reason.

Understood, this patch was not trying to change the compat ABI. SWP just gets
carried over as is like before.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions
  2020-05-15  3:28       ` Anshuman Khandual
@ 2020-05-18 22:35         ` Will Deacon
  2020-05-20  0:27           ` Anshuman Khandual
  0 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2020-05-18 22:35 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Dave Martin, linux-arm-kernel, Mark Rutland, Catalin Marinas,
	Suzuki K Poulose, linux-kernel, Mark Brown, Ard Biesheuvel

On Fri, May 15, 2020 at 08:58:05AM +0530, Anshuman Khandual wrote:
> On 05/14/2020 01:06 PM, Will Deacon wrote:
> > Why is it a change? We've never reported e.g. "java" on an arm64 kernel, so
> 
> We already have "java" defined in existing compat_hwcap_str[] array even
> though it might never get set in compat_elf_hwcap. AFAICS, compat_elf_hwcap
> will have the following capabilities set (at the most).
> 
> Via COMPAT_ELF_HWCAP_DEFAULT
> 
> 01.  COMPAT_HWCAP_HALF
> 02.  COMPAT_HWCAP_THUMB
> 03.  COMPAT_HWCAP_FAST_MULT
> 04.  COMPAT_HWCAP_EDSP
> 05.  COMPAT_HWCAP_TLS
> 06.  COMPAT_HWCAP_IDIV
> 07.  COMPAT_HWCAP_LPAE
> 
> Via setup_elf_hwcaps(compat_elf_hwcaps) <-- setup_cpu_features()
> 
> 8.  COMPAT_HWCAP_NEON
> 9.  COMPAT_HWCAP_VFPv4
> 10. COMPAT_HWCAP_VFP
> 11. COMPAT_HWCAP_VFPv3
> 
> Via arch_timer_set_evtstrm_feature()
> 
> 12. COMPAT_HWCAP_EVTSTRM
> 
> The code exists for "java" string to be displayed with /proc/cpuinfo but it
> may never get triggered as compat_elf_hwcap will never have JAVA capability
> unless there is a bug as you had rightly mentioned.

Fair enough, but applying this patch causes a *tonne* of warnings from
aiaiai:

+In file included from arch/arm64/include/asm/cpufeature.h:11,
+                 from arch/arm64/include/asm/ptrace.h:11,
+                 from arch/arm64/include/asm/irqflags.h:10,
+                 from include/linux/irqflags.h:16,
+                 from include/linux/spinlock.h:54,
+                 from include/linux/seqlock.h:36,
+                 from include/linux/time.h:6,
+                 from arch/arm64/include/asm/stat.h:12,
+                 from include/linux/stat.h:6,
+                 from include/linux/module.h:13,
+                 from drivers/media/rc/keymaps/rc-imon-mce.mod.c:1:
+arch/arm64/include/asm/hwcap.h:189:26: warning: ‘compat_hwcap2_str’ defined but not used [-Wunused-const-variable=]
+  189 | static const char *const compat_hwcap2_str[] = {
+      |                          ^~~~~~~~~~~~~~~~~

so I'm dropping this for now.

Will

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions
  2020-05-18 22:35         ` Will Deacon
@ 2020-05-20  0:27           ` Anshuman Khandual
  0 siblings, 0 replies; 7+ messages in thread
From: Anshuman Khandual @ 2020-05-20  0:27 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Suzuki K Poulose, Catalin Marinas, linux-kernel,
	Ard Biesheuvel, Mark Brown, Dave Martin, linux-arm-kernel



On 05/19/2020 04:05 AM, Will Deacon wrote:
> On Fri, May 15, 2020 at 08:58:05AM +0530, Anshuman Khandual wrote:
>> On 05/14/2020 01:06 PM, Will Deacon wrote:
>>> Why is it a change? We've never reported e.g. "java" on an arm64 kernel, so
>>
>> We already have "java" defined in existing compat_hwcap_str[] array even
>> though it might never get set in compat_elf_hwcap. AFAICS, compat_elf_hwcap
>> will have the following capabilities set (at the most).
>>
>> Via COMPAT_ELF_HWCAP_DEFAULT
>>
>> 01.  COMPAT_HWCAP_HALF
>> 02.  COMPAT_HWCAP_THUMB
>> 03.  COMPAT_HWCAP_FAST_MULT
>> 04.  COMPAT_HWCAP_EDSP
>> 05.  COMPAT_HWCAP_TLS
>> 06.  COMPAT_HWCAP_IDIV
>> 07.  COMPAT_HWCAP_LPAE
>>
>> Via setup_elf_hwcaps(compat_elf_hwcaps) <-- setup_cpu_features()
>>
>> 8.  COMPAT_HWCAP_NEON
>> 9.  COMPAT_HWCAP_VFPv4
>> 10. COMPAT_HWCAP_VFP
>> 11. COMPAT_HWCAP_VFPv3
>>
>> Via arch_timer_set_evtstrm_feature()
>>
>> 12. COMPAT_HWCAP_EVTSTRM
>>
>> The code exists for "java" string to be displayed with /proc/cpuinfo but it
>> may never get triggered as compat_elf_hwcap will never have JAVA capability
>> unless there is a bug as you had rightly mentioned.
> 
> Fair enough, but applying this patch causes a *tonne* of warnings from
> aiaiai:
> 
> +In file included from arch/arm64/include/asm/cpufeature.h:11,
> +                 from arch/arm64/include/asm/ptrace.h:11,
> +                 from arch/arm64/include/asm/irqflags.h:10,
> +                 from include/linux/irqflags.h:16,
> +                 from include/linux/spinlock.h:54,
> +                 from include/linux/seqlock.h:36,
> +                 from include/linux/time.h:6,
> +                 from arch/arm64/include/asm/stat.h:12,
> +                 from include/linux/stat.h:6,
> +                 from include/linux/module.h:13,
> +                 from drivers/media/rc/keymaps/rc-imon-mce.mod.c:1:
> +arch/arm64/include/asm/hwcap.h:189:26: warning: ‘compat_hwcap2_str’ defined but not used [-Wunused-const-variable=]
> +  189 | static const char *const compat_hwcap2_str[] = {
> +      |                          ^~~~~~~~~~~~~~~~~

Native default build (defconfig) on arm64 does not throw this warning.
I guess this came up with 'make W=n' or something. Anyways if this is
indeed an issue, we will have to add in helpers for each HWCAP arrays
which will fetch a feature string on index value to be used in c_show
(arch/arm64/kernel/cpuinfo.c). I wanted to avoid this to reduce code. 

> 
> so I'm dropping this for now.

Sure, will respin this later probably after 5.8-rc1.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-05-20  0:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 13:29 [PATCH V2] arm64/cpuinfo: Move HWCAP name arrays alongside their bit definitions Anshuman Khandual
2020-05-13 15:04 ` Dave Martin
2020-05-14  1:44   ` Anshuman Khandual
2020-05-14  7:36     ` Will Deacon
2020-05-15  3:28       ` Anshuman Khandual
2020-05-18 22:35         ` Will Deacon
2020-05-20  0:27           ` Anshuman Khandual

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).