lib/crc: arm64: use .arch_extension to enable CRC and crypto
Subsystems (2)
PATCH 1/1 lib/crc: arm64: use .arch_extension to enable CRC and crypto
Instead of using ".cpu generic+crc+crypto", use ".arch_extension crc" and ".arch_extension crypto" to enable the required instructions.
The ".cpu generic+crc+crypto" directive replaces the assembler's current feature set with generic+crc+crypto, dropping any other features that were previously enabled, for example by -march or by a toolchain configured with --with-arch=armv9-a. In contrast, ".arch_extension" enables the requested extensions while preserving the current feature set.
This is not a functional change: crc32-core.S only uses base A64 instructions plus CRC32 and PMULL, which remain available, and BTI landing pads are emitted as HINT instructions. However, using .arch_extension avoids the pitfall of silently disabling features. See commit 9142c2f8f84b ("octeontx2: use arch_extension to enable LSE").
--- lib/crc/arm64/crc32-core.S | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
lib/crc/arm64/crc32-core.S +2 -1
@@ -11,7 +11,8 @@ #include <linux/linkage.h> #include <asm/assembler.h> - .cpu generic+crc+crypto + .arch_extension crc + .arch_extension crypto .macro bitle, reg .endm
Signature · 1 line
2.55.0
Eric Biggers
maintainerQuoted 9 lines · Emanuele Rocca
Instead of using ".cpu generic+crc+crypto", use ".arch_extension crc" and ".arch_extension crypto" to enable the required instructions.
The ".cpu generic+crc+crypto" directive replaces the assembler's current feature set with generic+crc+crypto, dropping any other features that were previously enabled, for example by -march or by a toolchain configured with --with-arch=armv9-a. In contrast, ".arch_extension" enables the requested extensions while preserving the current feature set.
Quoted 7 lines
This is not a functional change:
crc32-core.Sonly uses base A64 instructions plus CRC32 and PMULL, which remain available, and BTI landing pads are emitted as HINT instructions. However, using .arch_extension avoids the pitfall of silently disabling features. See commit9142c2f8f84b("octeontx2: usearch_extensionto enable LSE").Signed-off-by Emanuele Rocca <[email protected]>
Thanks. Just to confirm, all supported versions of binutils and clang support this? (As per Documentation/process/changes.rst, the kernel currently supports binutils 2.30 and later, and clang 17.0.1 and later.)
A lot of the arm64 crypto code uses ".arch armv8-a+crypto". Should
those be replaced with ".arch_extension crypto" for the same reason?
- Eric
Hi Eric,
Quoted 3 lines · Eric Biggers
Thanks. Just to confirm, all supported versions of binutils and clang support this? (As per
Documentation/process/changes.rst, the kernel currently supports binutils 2.30 and later, and clang 17.0.1 and later.)
Yes, binutils supports .arch_extension since 2.26:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=ae527cd876fe35de72d876d3484e194ccc66232f
$ git tag --contains ae527cd876fe35de72d876d3484e194ccc66232f | head -1 binutils-2_26
clang added support in 8.0.0:
https://releases.llvm.org/8.0.0/docs/ReleaseNotes.html#changes-to-the-aarch64-target
Quoted 2 lines
A lot of the arm64 crypto code uses ".arch armv8-a+crypto". Should
those be replaced with ".arch_extension crypto" for the same reason?
I think so, yes. Let me look more closely, I'll send another series for those if they need changing as well.
Thanks!
Emanuele
Ard Biesheuvel
reviewerHello Emanuele,
Quoted 9 lines · Emanuele Rocca
Instead of using ".cpu generic+crc+crypto", use ".arch_extension crc" and ".arch_extension crypto" to enable the required instructions.
The ".cpu generic+crc+crypto" directive replaces the assembler's current feature set with generic+crc+crypto, dropping any other features that were previously enabled, for example by -march or by a toolchain configured with --with-arch=armv9-a. In contrast, ".arch_extension" enables the requested extensions while preserving the current feature set.
Quoted 6 lines
This is not a functional change:
crc32-core.Sonly uses base A64 instructions plus CRC32 and PMULL, which remain available, and BTI landing pads are emitted as HINT instructions. However, using .arch_extension avoids the pitfall of silently disabling features. See commit9142c2f8f84b("octeontx2: usearch_extensionto enable LSE").
I don't mind the patch, but I will point out that issues such as these can never occur in .S sources. The emitted instructions are always the same, and the only reason we need the .arch is so that the assembler permits the use of the special ones.
Commit 9142c2f8f84b is different because it uses .arch in inline asm, where it switches the assembler into a different mode behind the back of the compiler, leading it to reject instructions generated by the compiler.
Eric Biggers
maintainerQuoted 19 lines · Ard Biesheuvel
Hello Emanuele,
Quoted 9 lines · Emanuele Rocca
Instead of using ".cpu generic+crc+crypto", use ".arch_extension crc" and ".arch_extension crypto" to enable the required instructions.
The ".cpu generic+crc+crypto" directive replaces the assembler's current feature set with generic+crc+crypto, dropping any other features that were previously enabled, for example by -march or by a toolchain configured with --with-arch=armv9-a. In contrast, ".arch_extension" enables the requested extensions while preserving the current feature set.
Quoted 6 lines
This is not a functional change:
crc32-core.Sonly uses base A64 instructions plus CRC32 and PMULL, which remain available, and BTI landing pads are emitted as HINT instructions. However, using .arch_extension avoids the pitfall of silently disabling features. See commit9142c2f8f84b("octeontx2: usearch_extensionto enable LSE").
Quoted 9 lines
I don't mind the patch, but I will point out that issues such as these can never occur in .S sources. The emitted instructions are always the same, and the only reason we need the .arch is so that the assembler permits the use of the special ones.
Commit
9142c2f8f84bis different because it uses .arch in inline asm, where it switches the assembler into a different mode behind the back of the compiler, leading it to reject instructions generated by the compiler.
Thanks. Emanuele, could you maybe note that in the commit message?
Also, this patch is not complete even for lib/crc/arm64/, since lib/crc/arm64/crc-t10dif-core.S still uses "armv8-a+crypto". If we're bothering with this, I would suggest just taking care of both crc32-core.S and crc-t10dif-core.S in one patch. (The crypto files I mentioned before should be a different patch.)
- Eric