ARM: advertise availability of v8 Crypto instructions
PATCH v2 1/1 ARM: advertise availability of v8 Crypto instructions
When running the 32-bit ARM kernel on ARMv8 capable bare metal (e.g., 32-bit Android userland and kernel on a Cortex-A53), or as a KVM guest on a 64-bit host, we should advertise the availability of the Crypto instructions, so that userland libraries such as OpenSSL may use them. (Support for the v8 Crypto instructions in the 32-bit build was added to OpenSSL more than six months ago)
This adds the ID feature bit detection, and sets elf_hwcap2 accordingly.
---
v2:
- drop redundant architecture check -> accessing ID_ISAR5 should be safe even on v7-M, and even if we don't expect to find crypto features there
- add comment regarding pmull->aes
arch/arm/kernel/setup.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
arch/arm/kernel/setup.c +15 -1
@@ -375,7 +375,7 @@ void __init early_print(const char *str, ...)static void __init cpuid_init_hwcaps (void ) { -unsigned int divide_instrs, vmsa; +unsigned int divide_instrs, vmsa, isar5;if (cpu_architecture () < CPU_ARCH_ARMv7)return ;
@@ -393,6 +393,20 @@ static void __init cpuid_init_hwcaps(void) vmsa = (read_cpuid_ext (CPUID_EXT_MMFR0 ) &0xf ) >>0 ;if (vmsa >=5 ) elf_hwcap |=HWCAP_LPAE ; + +/* check for supported v8 Crypto instructions */ + isar5 =read_cpuid_ext (CPUID_EXT_ISAR5 ); + +switch ((isar5 >>4 ) &0xf ) { +case 2 : elf_hwcap2 |=HWCAP2_PMULL ;/* pmull implies aes */ +case 1 : elf_hwcap2 |=HWCAP2_AES ; + } +if (((isar5 >>8 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_SHA1 ; +if (((isar5 >>12 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_SHA2 ; +if (((isar5 >>16 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_CRC32 ; }static void __init elf_hwcap_fixup (void )
Signature · 1 line
1.8.3.2
PATCH v2 1/1 ARM: advertise availability of v8 Crypto instructions
Quoted 8 lines · Ard Biesheuvel
arch/arm/kernel/setup.c +4
-0@@ -393,6 +393,20 @@ static void __init cpuid_init_hwcaps(void) vmsa = (read_cpuid_ext (CPUID_EXT_MMFR0 ) &0xf ) >>0 ;if (vmsa >=5 ) elf_hwcap |=HWCAP_LPAE ; + +/* check for supported v8 Crypto instructions */ + isar5 =read_cpuid_ext (CPUID_EXT_ISAR5 ); +
arch/arm/kernel/setup.c static void __init cpuid_init_hwcaps(void) · line 409
Quoted 10 lines
arch/arm/kernel/setup.c +10
-0+switch ((isar5 >>4 ) &0xf ) { +case 2 : elf_hwcap2 |=HWCAP2_PMULL ;/* pmull implies aes */ +case 1 : elf_hwcap2 |=HWCAP2_AES ; + } +if (((isar5 >>8 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_SHA1 ; +if (((isar5 >>12 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_SHA2 ; +if (((isar5 >>16 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_CRC32 ;
Reading through the ARMv7 ARM, the ISAR registers seem to work in a way that "feature >= N" is sufficient to test for something (in other words, the feature revision bits build on previous instructions added by older revisions of that feature.)
Hence why PMULL implies AES - that follows the same pattern. So, I wonder if those == should all be >=, and there should be a "default:" before "case 2:" with the zero case handled separately.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH v2 1/1 ARM: advertise availability of v8 Crypto instructions
Quoted 16 lines · Russell King - ARM Linux
Quoted 15 lines · Ard Biesheuvel
arch/arm/kernel/setup.c +11
-0@@ -393,6 +393,20 @@ static void __init cpuid_init_hwcaps(void) vmsa = (read_cpuid_ext (CPUID_EXT_MMFR0 ) &0xf ) >>0 ;if (vmsa >=5 ) elf_hwcap |=HWCAP_LPAE ; + +/* check for supported v8 Crypto instructions */ + isar5 =read_cpuid_ext (CPUID_EXT_ISAR5 ); + +switch ((isar5 >>4 ) &0xf ) { +case 2 : elf_hwcap2 |=HWCAP2_PMULL ;/* pmull implies aes */ +case 1 : elf_hwcap2 |=HWCAP2_AES ; + } +if (((isar5 >>8 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_SHA1 ; +if (((isar5 >>12 ) &0xf ) ==1 )
Quoted 9 lines
arch/arm/kernel/setup.c static void __init cpuid_init_hwcaps(void) · line 409
Quoted 3 lines
arch/arm/kernel/setup.c +3
-0+ elf_hwcap2 |=HWCAP2_SHA2 ; +if (((isar5 >>16 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_CRC32 ;Reading through the ARMv7 ARM, the ISAR registers seem to work in a way that "feature >= N" is sufficient to test for something (in other words, the feature revision bits build on previous instructions added by older revisions of that feature.)
There was quite some discussion about that when we implemented this for arm64. Perhaps Steve wants to elaborate on the details, because I don't remember them exactly.
Quoted 4 lines
Hence why PMULL implies AES - that follows the same pattern. So, I wonder if those == should all be >=, and there should be a "default:" before "case 2:" with the zero case handled separately.
Quoting from my version of the ARM ARM (DDI0487A_d):
AES, bits [7:4]
Indicates whether AES instructions are implemented in AArch32. 0000 No AES instructions implemented.
0001 AESE, AESD, AESMC, and AESIMC implemented.
0010 As for 0001 , plus PMULL/PMULL2 instructions operating on 64-bit data quantities.
All other values are reserved.
Even though they are obviously not bitfields, 'reserved' to me suggests that it is still incorrect to assume both PMULL and AES capability for values not listed yet > 0b0010.
The same applies to the other blocks: all other values are reserved.
Signature · 1 line
Ard.
PATCH v2 1/1 ARM: advertise availability of v8 Crypto instructions
Quoted 47 lines · Ard Biesheuvel
Quoted 16 lines · Russell King - ARM Linux
Quoted 15 lines · Ard Biesheuvel
arch/arm/kernel/setup.c +11
-0@@ -393,6 +393,20 @@ static void __init cpuid_init_hwcaps(void) vmsa = (read_cpuid_ext (CPUID_EXT_MMFR0 ) &0xf ) >>0 ;if (vmsa >=5 ) elf_hwcap |=HWCAP_LPAE ; + +/* check for supported v8 Crypto instructions */ + isar5 =read_cpuid_ext (CPUID_EXT_ISAR5 ); + +switch ((isar5 >>4 ) &0xf ) { +case 2 : elf_hwcap2 |=HWCAP2_PMULL ;/* pmull implies aes */ +case 1 : elf_hwcap2 |=HWCAP2_AES ; + } +if (((isar5 >>8 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_SHA1 ; +if (((isar5 >>12 ) &0xf ) ==1 )Quoted 9 lines
arch/arm/kernel/setup.c static void __init cpuid_init_hwcaps(void) · line 409
Quoted 3 lines
arch/arm/kernel/setup.c +3
-0+ elf_hwcap2 |=HWCAP2_SHA2 ; +if (((isar5 >>16 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_CRC32 ;Reading through the ARMv7 ARM, the ISAR registers seem to work in a way that "feature >= N" is sufficient to test for something (in other words, the feature revision bits build on previous instructions added by older revisions of that feature.)
There was quite some discussion about that when we implemented this for arm64. Perhaps Steve wants to elaborate on the details, because I don't remember them exactly.
Quoted 4 lines
Hence why PMULL implies AES - that follows the same pattern. So, I wonder if those == should all be >=, and there should be a "default:" before "case 2:" with the zero case handled separately.
Quoting from my version of the ARM ARM (
DDI0487A_d):AES, bits [7:4]
Indicates whether AES instructions are implemented in AArch32. 0000 No AES instructions implemented.
0001 AESE, AESD, AESMC, and AESIMC implemented.
0010 As for 0001 , plus PMULL/PMULL2 instructions operating on 64-bit data quantities.
All other values are reserved.
Quoted 5 lines
Even though they are obviously not bitfields, 'reserved' to me suggests that it is still incorrect to assume both PMULL and AES capability for values not listed yet > 0b0010.
The same applies to the other blocks: all other values are reserved.
Replying to self: looking at the arm64 code, it seems the 4-bit block are signed quantities, and the negative valiues are reserved. This means only signed higher values should be considered incremental, and values with bit 4 set should be ignored.
However, I can't find anything about it in the ARMv8 ARM.
If you prefer, I can rework the patch to duplicate the compat code in
arm64: at least they will be the same then.
------------------------>8--------------------------- features = read_cpuid(ID_ISAR5_EL1);
block = (features >> 4) & 0xf;
if (!(block & 0x8)) {
switch (block) {
default:
case 2:compat_elf_hwcap2 |= COMPAT_HWCAP2_PMULL; case 1:compat_elf_hwcap2 |= COMPAT_HWCAP2_AES;
case 0:
break;
}
}
block = (features >> 8) & 0xf;
if (block && !(block & 0x8))compat_elf_hwcap2 |= COMPAT_HWCAP2_SHA1;
block = (features >> 12) & 0xf;
if (block && !(block & 0x8))compat_elf_hwcap2 |= COMPAT_HWCAP2_SHA2;
block = (features >> 16) & 0xf;
if (block && !(block & 0x8)) compat_elf_hwcap2 |= COMPAT_HWCAP2_CRC32; ------------------------>8---------------------------
PATCH v2 1/1 ARM: advertise availability of v8 Crypto instructions
Quoted 4 lines · Ard Biesheuvel
Replying to self: looking at the arm64 code, it seems the 4-bit block are signed quantities, and the negative valiues are reserved. This means only signed higher values should be considered incremental, and values with bit 4 set should be ignored.
ITYM bit 3.
On 32-bit, we've assumed that they're unsigned:
/* LPAE implies atomic ldrd/strd instructions */
vmsa = (read_cpuid_ext(CPUID_EXT_MMFR0) & 0xf) >> 0;
if (vmsa >= 5)
elf_hwcap |= HWCAP_LPAE;
sync_prim = ((read_cpuid_ext(CPUID_EXT_ISAR3) >> 8) & 0xf0) |
((read_cpuid_ext(CPUID_EXT_ISAR4) >> 20) & 0x0f);
if (sync_prim >= 0x13)
elf_hwcap &= ~HWCAP_SWP;Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH v2 1/1 ARM: advertise availability of v8 Crypto instructions
Quoted 8 lines · Russell King - ARM Linux
Quoted 4 lines · Ard Biesheuvel
Replying to self: looking at the arm64 code, it seems the 4-bit block are signed quantities, and the negative valiues are reserved. This means only signed higher values should be considered incremental, and values with bit 4 set should be ignored.
ITYM bit 3.
Sorry, yes, the top bit.
Quoted 12 lines
On 32-bit, we've assumed that they're unsigned:
/* LPAE implies atomic ldrd/strd instructions */ vmsa = (read_cpuid_ext(CPUID_EXT_MMFR0) & 0xf) >> 0; if (vmsa >= 5) elf_hwcap |= HWCAP_LPAE; sync_prim = ((read_cpuid_ext(CPUID_EXT_ISAR3) >> 8) & 0xf0) | ((read_cpuid_ext(CPUID_EXT_ISAR4) >> 20) & 0x0f); if (sync_prim >= 0x13) elf_hwcap &= ~HWCAP_SWP;
PATCH v2 1/1 ARM: advertise availability of v8 Crypto instructions
In reply to Ard BiesheuvelQuoted 47 lines · Ard Biesheuvel
Quoted 16 lines · Russell King - ARM Linux
Quoted 15 lines · Ard Biesheuvel
arch/arm/kernel/setup.c +11
-0@@ -393,6 +393,20 @@ static void __init cpuid_init_hwcaps(void) vmsa = (read_cpuid_ext (CPUID_EXT_MMFR0 ) &0xf ) >>0 ;if (vmsa >=5 ) elf_hwcap |=HWCAP_LPAE ; + +/* check for supported v8 Crypto instructions */ + isar5 =read_cpuid_ext (CPUID_EXT_ISAR5 ); + +switch ((isar5 >>4 ) &0xf ) { +case 2 : elf_hwcap2 |=HWCAP2_PMULL ;/* pmull implies aes */ +case 1 : elf_hwcap2 |=HWCAP2_AES ; + } +if (((isar5 >>8 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_SHA1 ; +if (((isar5 >>12 ) &0xf ) ==1 )Quoted 9 lines
arch/arm/kernel/setup.c static void __init cpuid_init_hwcaps(void) · line 409
Quoted 3 lines
arch/arm/kernel/setup.c +3
-0+ elf_hwcap2 |=HWCAP2_SHA2 ; +if (((isar5 >>16 ) &0xf ) ==1 ) + elf_hwcap2 |=HWCAP2_CRC32 ;Reading through the ARMv7 ARM, the ISAR registers seem to work in a way that "feature >= N" is sufficient to test for something (in other words, the feature revision bits build on previous instructions added by older revisions of that feature.)
There was quite some discussion about that when we implemented this for arm64. Perhaps Steve wants to elaborate on the details, because I don't remember them exactly.
Quoted 4 lines
Hence why PMULL implies AES - that follows the same pattern. So, I wonder if those == should all be >=, and there should be a "default:" before "case 2:" with the zero case handled separately.
Quoting from my version of the ARM ARM (
DDI0487A_d):AES, bits [7:4]
Indicates whether AES instructions are implemented in AArch32. 0000 No AES instructions implemented.
0001 AESE, AESD, AESMC, and AESIMC implemented.
0010 As for 0001 , plus PMULL/PMULL2 instructions operating on 64-bit data quantities.
All other values are reserved.
Quoted 4 lines
Even though they are obviously not bitfields, 'reserved' to me suggests that it is still incorrect to assume both PMULL and AES capability for values not listed yet > 0b0010.
The same applies to the other blocks: all other values are reserved.
The use of "reserved" in this manner isn't actually detailed in the glossary, which makes it a little hard to work out exactly what is meant here (that's a documentation bug).
Is it "ARM Ltd reserves the right to add further values which augment the previous values" or is it "ARM Ltd reserve the right to add further values with completely different behaviours."
Really, the ARM ARM needs to be improved in this area. It needs to contain a clear statement about how the bitfields are intended to work. The purpose of the CPUID stuff is to allow software to adapt to the features of the CPU, but without a clearly defined behaviour for the "unknown values", moving a piece of software to a newer CPU which implements a superset of the features of a previous CPU will lead it to fail (because the CPUID fields could now be in the reserved ranges) if strict tests were applied for exact field values.
I'm not talking about moving a piece of binary software: I'm talking there more about the maintanence burden of having to go through the entire program checking for any references to CPUID fields, and checking and updating every single one.
The current CPUID situation is which encourages exact precise tests is, IMHO, not well thought out.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.