[PATCH 1/3] arm64/cpufeature.h: Add macros for a cpu features testing

[PATCH 1/3] arm64/cpufeature.h: Add macros for a cpu features testing

[Alexander Kuleshov]

This patch provides a couple of macros for the testing of processor
features (crypto and FP/SIMD) like support of SHA1, AES instructions,
support for FPU and etc. There is already a couple of places in the
arch/arm64/kernel where these processor features are tested and these
macros are facilitate this.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/arm64/include/asm/cpufeature.h | 44 +++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index c104421..2919455 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -28,7 +28,50 @@
 
 #define ARM64_NCAPS				4
 
+/*
+ * ID_AA64ISAR0_EL1 AES, bits [7:4]
+ */
+#define ID_AA64ISAR0_EL1_AES_MASK	4
+#define ID_AA64ISAR0_EL1_AES(feature)	\
+	(((feature >> ID_AA64ISAR0_EL1_AES_MASK) & 0xf) & 1UL)
+#define ID_AA64ISAR0_EL1_PMULL(feature)	\
+	(((feature >> ID_AA64ISAR0_EL1_AES_MASK) & 0xf) & 2UL)
+
+/*
+ * ID_AA64ISAR0_EL1 SHA1, bits [11:8]
+ */
+#define ID_AA64ISAR0_EL1_SHA1_MASK	8
+#define ID_AA64ISAR0_EL1_SHA1(feature)	\
+	(((feature >> ID_AA64ISAR0_EL1_SHA1_MASK) & 0xf) & 1UL)
+
+/*
+ * ID_AA64ISAR0_EL1 SHA2, bits [15:12]
+ */
+#define ID_AA64ISAR0_EL1_SHA2_MASK	12
+#define ID_AA64ISAR0_EL1_SHA2(feature)	\
+	(((feature >> ID_AA64ISAR0_EL1_SHA2_MASK) & 0xf) & 1UL)
+
+/*
+ * ID_AA64ISAR0_EL1 CRC32, bits [19:16]
+ */
+#define ID_AA64ISAR0_EL1_CRC32_MASK	16
+#define ID_AA64ISAR0_EL1_CRC32(feature)	\
+	(((feature >> ID_AA64ISAR0_EL1_CRC32_MASK) & 0xf) & 1UL)
+
+/*
+ * ID_AA64PFR0_EL1 FP, bits [19:16]
+ */
+#define ID_AA64PFR0_EL1_FP_MASK		16
+#define ID_AA64PFR0_EL1_FP(ptr)		\
+	(ptr & (0xf << ID_AA64PFR0_EL1_FP_MASK))
+
+/*
+ * ID_AA64PFR0_EL1 AdvSIMD, bits [23:20]
+ */
+#define ID_AA64PFR0_EL1_ADV_SIMD_MASK	20
+#define ID_AA64PFR0_EL1_ADV_SIMD(ptr)	\
+	(ptr & (0xf << ID_AA64PFR0_EL1_ADV_SIMD_MASK))
+
 #ifndef __ASSEMBLY__
 
 struct arm64_cpu_capabilities {
-- 
2.5.0

[PATCH 2/3] arm64/setup: Use ID_AA64ISAR0_EL1_.* macros

[Alexander Kuleshov]

The 26d75e67c commit (arm64/cpufeature.h: Add macros for a cpu features
testing) provides set of macros for the testing processor's crypto features.
Let's use these macros instead of direct calculation.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/arm64/kernel/setup.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 926ae8d..a3faf4f 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -250,33 +250,22 @@ static void __init setup_processor(void)
 
 	/*
 	 * ID_AA64ISAR0_EL1 contains 4-bit wide signed feature blocks.
-	 * The blocks we test below represent incremental functionality
-	 * for non-negative values. Negative values are reserved.
 	 */
 	features = read_cpuid(ID_AA64ISAR0_EL1);
-	block = (features >> 4) & 0xf;
-	if (!(block & 0x8)) {
-		switch (block) {
-		default:
-		case 2:
-			elf_hwcap |= HWCAP_PMULL;
-		case 1:
-			elf_hwcap |= HWCAP_AES;
-		case 0:
-			break;
-		}
-	}
 
-	block = (features >> 8) & 0xf;
-	if (block && !(block & 0x8))
+	if (ID_AA64ISAR0_EL1_AES(features))
+		elf_hwcap |= HWCAP_AES;
+
+	if (ID_AA64ISAR0_EL1_PMULL(features))
+		elf_hwcap |= HWCAP_PMULL;
+
+	if (ID_AA64ISAR0_EL1_SHA1(features))
 		elf_hwcap |= HWCAP_SHA1;
 
-	block = (features >> 12) & 0xf;
-	if (block && !(block & 0x8))
+	if (ID_AA64ISAR0_EL1_SHA2(features))
 		elf_hwcap |= HWCAP_SHA2;
 
-	block = (features >> 16) & 0xf;
-	if (block && !(block & 0x8))
+	if (ID_AA64ISAR0_EL1_CRC32(features))
 		elf_hwcap |= HWCAP_CRC32;
 
 #ifdef CONFIG_COMPAT
-- 
2.5.0

Re: [PATCH 2/3] arm64/setup: Use ID_AA64ISAR0_EL1_.* macros

[Catalin Marinas]

On Fri, Sep 04, 2015 at 12:12:52AM +0600, Alexander Kuleshov wrote:
> The 26d75e67c commit (arm64/cpufeature.h: Add macros for a cpu features

Please drop the commit number here, that's specific to your tree and not
something in mainline.

-- 
Catalin

Re: [PATCH 2/3] arm64/setup: Use ID_AA64ISAR0_EL1_.* macros

[Suzuki K Poulose]

On 03/09/15 19:12, Alexander Kuleshov wrote:
> The 26d75e67c commit (arm64/cpufeature.h: Add macros for a cpu features
> testing) provides set of macros for the testing processor's crypto features.
> Let's use these macros instead of direct calculation.
>



> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  arch/arm64/kernel/setup.c | 29 +++++++++--------------------
>  1 file changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 926ae8d..a3faf4f 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c

This patch doesn't apply on the current mainline tree. Where does this patch apply ?
The elf_hwcap calculation has been moved to a separate function setup_elf_hwcaps()
in arch/arm64/kernel/cpufeature.c,  which makes uses of a table of arm64_cpu_capabilities.

Suzuki

> @@ -250,33 +250,22 @@ static void __init setup_processor(void)
>
>  	/*
>  	 * ID_AA64ISAR0_EL1 contains 4-bit wide signed feature blocks.
> -	 * The blocks we test below represent incremental functionality
> -	 * for non-negative values. Negative values are reserved.
>  	 */
>  	features = read_cpuid(ID_AA64ISAR0_EL1);
> -	block = (features >> 4) & 0xf;
> -	if (!(block & 0x8)) {
> -		switch (block) {
> -		default:
> -		case 2:
> -			elf_hwcap |= HWCAP_PMULL;
> -		case 1:
> -			elf_hwcap |= HWCAP_AES;
> -		case 0:
> -			break;
> -		}
> -	}
>
> -	block = (features >> 8) & 0xf;
> -	if (block && !(block & 0x8))
> +	if (ID_AA64ISAR0_EL1_AES(features))
> +		elf_hwcap |= HWCAP_AES;
> +
> +	if (ID_AA64ISAR0_EL1_PMULL(features))
> +		elf_hwcap |= HWCAP_PMULL;
> +
> +	if (ID_AA64ISAR0_EL1_SHA1(features))
>  		elf_hwcap |= HWCAP_SHA1;
>
> -	block = (features >> 12) & 0xf;
> -	if (block && !(block & 0x8))
> +	if (ID_AA64ISAR0_EL1_SHA2(features))
>  		elf_hwcap |= HWCAP_SHA2;
>
> -	block = (features >> 16) & 0xf;
> -	if (block && !(block & 0x8))
> +	if (ID_AA64ISAR0_EL1_CRC32(features))
>  		elf_hwcap |= HWCAP_CRC32;

[PATCH 3/3] arm64/fpsimd: Use ID_AA64PFR0_EL1_.* macros

[Alexander Kuleshov]

The 26d75e67c commit (arm64/cpufeature.h: Add macros for a cpu features
testing) provides set of macros for the testing processor's FP and advanced
SIMD features.

Let's use these macros instead of direct calculation.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/arm64/kernel/fpsimd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 44d6f75..12943a5 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -27,6 +27,7 @@
 
 #include <asm/fpsimd.h>
 #include <asm/cputype.h>
+#include <asm/cpufeature.h>
 
 #define FPEXC_IOF	(1 << 0)
 #define FPEXC_DZF	(1 << 1)
@@ -333,13 +334,13 @@ static int __init fpsimd_init(void)
 {
 	u64 pfr = read_cpuid(ID_AA64PFR0_EL1);
 
-	if (pfr & (0xf << 16)) {
+	if (ID_AA64PFR0_EL1_FP(pfr)) {
 		pr_notice("Floating-point is not implemented\n");
 		return 0;
 	}
 	elf_hwcap |= HWCAP_FP;
 
-	if (pfr & (0xf << 20))
+	if (ID_AA64PFR0_EL1_ADV_SIMD(pfr))
 		pr_notice("Advanced SIMD is not implemented\n");
 	else
 		elf_hwcap |= HWCAP_ASIMD;
-- 
2.5.0

Re: [PATCH 3/3] arm64/fpsimd: Use ID_AA64PFR0_EL1_.* macros

[Suzuki K Poulose]

On 03/09/15 19:12, Alexander Kuleshov wrote:
> The 26d75e67c commit (arm64/cpufeature.h: Add macros for a cpu features
> testing) provides set of macros for the testing processor's FP and advanced
> SIMD features.
>
> Let's use these macros instead of direct calculation.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  arch/arm64/kernel/fpsimd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 44d6f75..12943a5 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -27,6 +27,7 @@
>
>  #include <asm/fpsimd.h>
>  #include <asm/cputype.h>
> +#include <asm/cpufeature.h>
>
>  #define FPEXC_IOF	(1 << 0)
>  #define FPEXC_DZF	(1 << 1)
> @@ -333,13 +334,13 @@ static int __init fpsimd_init(void)
>  {
>  	u64 pfr = read_cpuid(ID_AA64PFR0_EL1);
>
> -	if (pfr & (0xf << 16)) {
> +	if (ID_AA64PFR0_EL1_FP(pfr)) {
>  		pr_notice("Floating-point is not implemented\n");
>  		return 0;
>  	}
>  	elf_hwcap |= HWCAP_FP;
>
> -	if (pfr & (0xf << 20))
> +	if (ID_AA64PFR0_EL1_ADV_SIMD(pfr))
>  		pr_notice("Advanced SIMD is not implemented\n");
>  	else
>  		elf_hwcap |= HWCAP_ASIMD;
>

Similar to the previous one, this won't apply anymore.

Suzuki

Re: [PATCH 1/3] arm64/cpufeature.h: Add macros for a cpu features testing

[Catalin Marinas]

On Fri, Sep 04, 2015 at 12:11:51AM +0600, Alexander Kuleshov wrote:
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -28,7 +28,50 @@
>  
>  #define ARM64_NCAPS				4
>  
> +/*
> + * ID_AA64ISAR0_EL1 AES, bits [7:4]
> + */
> +#define ID_AA64ISAR0_EL1_AES_MASK	4
> +#define ID_AA64ISAR0_EL1_AES(feature)	\
> +	(((feature >> ID_AA64ISAR0_EL1_AES_MASK) & 0xf) & 1UL)

This looks more like a shift than a mask. I don't think it's worth
defining another macro for the shift.

> +#define ID_AA64ISAR0_EL1_PMULL(feature)	\
> +	(((feature >> ID_AA64ISAR0_EL1_AES_MASK) & 0xf) & 2UL)

I'm not against some clean-up here but I think you break the original
logic. AES and PMULL are not exclusive, the latter implies the former
but the way you check here is just individual bits. These id fields are
meant to be treated as 4-bit signed values, so if AES means >= 1, PMULL
means >= 2. We have a cpuid_feature_extract_field() (in linux-next and
about to go in 4.3-rc1), so use this one for extracting the signed 4-bit
field.

-- 
Catalin

Re: [PATCH 1/3] arm64/cpufeature.h: Add macros for a cpu features testing

[Suzuki K. Poulose]

On 03/09/15 19:11, Alexander Kuleshov wrote:
> This patch provides a couple of macros for the testing of processor
> features (crypto and FP/SIMD) like support of SHA1, AES instructions,
> support for FPU and etc. There is already a couple of places in the
> arch/arm64/kernel where these processor features are tested and these
> macros are facilitate this.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>   arch/arm64/include/asm/cpufeature.h | 44 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 44 insertions(+)
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index c104421..2919455 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -28,7 +28,50 @@
>
>   #define ARM64_NCAPS				4
>
> +/*
> + * ID_AA64ISAR0_EL1 AES, bits [7:4]
> + */
> +#define ID_AA64ISAR0_EL1_AES_MASK	4
> +#define ID_AA64ISAR0_EL1_AES(feature)	\
> +	(((feature >> ID_AA64ISAR0_EL1_AES_MASK) & 0xf) & 1UL)
> +#define ID_AA64ISAR0_EL1_PMULL(feature)	\
> +	(((feature >> ID_AA64ISAR0_EL1_AES_MASK) & 0xf) & 2UL)
> +


There is generic CPUID feature helper queued for 4.3, which can extract
the feature bits

  cpuid_feature_extract_field(feature, shift)

You might want to use it instead. Btw, I have a patch series(waiting for
the merge window, before I post), which changes the way we initialise the
HWCAP bits.

Thanks
Suzuki

Re: [PATCH 1/3] arm64/cpufeature.h: Add macros for a cpu features testing

[Alexander Kuleshov]

2015-09-04 18:00 GMT+06:00 Suzuki K. Poulose <Suzuki.Poulose@arm.com>:
> There is generic CPUID feature helper queued for 4.3, which can extract
> the feature bits
>
>  cpuid_feature_extract_field(feature, shift)
>
> You might want to use it instead. Btw, I have a patch series(waiting for
> the merge window, before I post), which changes the way we initialise the
> HWCAP bits.

Hello,

Thanks for feedback first of all. So, I'll wait while your changes will be
in the mainline kernel and will reconsider the changes.

Thank you.

Re: [PATCH 1/3] arm64/cpufeature.h: Add macros for a cpu features testing

[Suzuki K. Poulose]

On 04/09/15 13:19, Alexander Kuleshov wrote:
> 2015-09-04 18:00 GMT+06:00 Suzuki K. Poulose <Suzuki.Poulose@arm.com>:
>> There is generic CPUID feature helper queued for 4.3, which can extract
>> the feature bits
>>
>>   cpuid_feature_extract_field(feature, shift)
>>
>> You might want to use it instead. Btw, I have a patch series(waiting for
>> the merge window, before I post), which changes the way we initialise the
>> HWCAP bits.
>
> Hello,
>
> Thanks for feedback first of all. So, I'll wait while your changes will be
> in the mainline kernel and will reconsider the changes.
>

Please find the series here :

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/370386.html

Thanks
Suzuki