* [PATCH 0/3] arm64: Open code .arch_extension
@ 2020-03-25 11:41 Mark Brown
2020-03-25 11:41 ` [PATCH 1/3] arm64: asm: Provide macro to control enabling architecture extensions Mark Brown
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Mark Brown @ 2020-03-25 11:41 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Eric Biggers, Ard Biesheuvel
Cc: linux-arm-kernel, linux-crypto, Mark Brown
Currently several assembler files override the default architecture to
enable extensions in order to allow them to implement optimised routines
for systems with those extensions. Since inserting BTI landing pads into
assembler functions will require us to change the default architecture we
need a way to enable extensions without hard coding the architecture.
The assembler has the .arch_extension feature but this was introduced
for arm64 in gas 2.26 which is too modern for us to rely on it.
We could just update the base architecture used by these assembler files
but this would mean the assembler would no longer catch attempts to use
newer instructions so instead introduce a macro which sets the default
architecture centrally. Doing this will also make our use of .arch and
.cpu to select the base architecture more consistent.
Mark Brown (3):
arm64: asm: Provide macro to control enabling architecture extensions
arm64: lib: Use ARM64_EXTENSIONS()
arm64: crypto: Use ARM64_EXTENSIONS()
arch/arm64/crypto/aes-ce-ccm-core.S | 3 ++-
arch/arm64/crypto/aes-ce-core.S | 2 +-
arch/arm64/crypto/aes-ce.S | 2 +-
arch/arm64/crypto/crct10dif-ce-core.S | 3 ++-
arch/arm64/crypto/ghash-ce-core.S | 3 ++-
arch/arm64/crypto/sha1-ce-core.S | 3 ++-
arch/arm64/crypto/sha2-ce-core.S | 3 ++-
arch/arm64/include/asm/linkage.h | 6 ++++++
arch/arm64/lib/crc32.S | 2 +-
9 files changed, 19 insertions(+), 8 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] arm64: asm: Provide macro to control enabling architecture extensions 2020-03-25 11:41 [PATCH 0/3] arm64: Open code .arch_extension Mark Brown @ 2020-03-25 11:41 ` Mark Brown 2020-03-25 11:41 ` [PATCH 2/3] arm64: lib: Use ARM64_EXTENSIONS() Mark Brown ` (3 subsequent siblings) 4 siblings, 0 replies; 18+ messages in thread From: Mark Brown @ 2020-03-25 11:41 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Eric Biggers, Ard Biesheuvel Cc: linux-arm-kernel, linux-crypto, Mark Brown Currently several assembler files override the default architecture to enable extensions in order to allow them to implement optimised routines for systems with those extensions. Since inserting BTI landing pads into assembler functions will require us to change the default architecture we need a way to enable extensions without hard coding the architecture. The assembler has the .arch_extension feature but this was introduced for arm64 in gas 2.26 which is too modern for us to rely on it. We could just update the base architecture used by these assembler files but this would mean the assembler would no longer catch attempts to use newer instructions so instead introduce a macro which sets the default architecture centrally. Doing this will also make our use of .arch and .cpu to select the base architecture more consistent. Signed-off-by: Mark Brown <broonie@kernel.org> --- arch/arm64/include/asm/linkage.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/arm64/include/asm/linkage.h b/arch/arm64/include/asm/linkage.h index ebee3113a62f..e5856c75720b 100644 --- a/arch/arm64/include/asm/linkage.h +++ b/arch/arm64/include/asm/linkage.h @@ -20,4 +20,10 @@ SYM_FUNC_END(x); \ SYM_FUNC_END_ALIAS(__pi_##x) +/* + * Enable additional architecture extensions (eg, for optimized asm + * routines). + */ +#define ARM64_EXTENSIONS(x) .arch armv8-a+x + #endif -- 2.20.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] arm64: lib: Use ARM64_EXTENSIONS() 2020-03-25 11:41 [PATCH 0/3] arm64: Open code .arch_extension Mark Brown 2020-03-25 11:41 ` [PATCH 1/3] arm64: asm: Provide macro to control enabling architecture extensions Mark Brown @ 2020-03-25 11:41 ` Mark Brown 2020-03-25 11:41 ` [PATCH 3/3] arm64: crypto: " Mark Brown ` (2 subsequent siblings) 4 siblings, 0 replies; 18+ messages in thread From: Mark Brown @ 2020-03-25 11:41 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Eric Biggers, Ard Biesheuvel Cc: linux-arm-kernel, linux-crypto, Mark Brown Use the newly introduced ARM64_EXTENSIONS() macro to enable the CRC instructions. Signed-off-by: Mark Brown <broonie@kernel.org> --- arch/arm64/lib/crc32.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/lib/crc32.S b/arch/arm64/lib/crc32.S index 243e107e9896..0fd9ef030ab1 100644 --- a/arch/arm64/lib/crc32.S +++ b/arch/arm64/lib/crc32.S @@ -9,7 +9,7 @@ #include <asm/alternative.h> #include <asm/assembler.h> - .cpu generic+crc +ARM64_EXTENSIONS(crc) .macro __crc32, c cmp x2, #16 -- 2.20.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] arm64: crypto: Use ARM64_EXTENSIONS() 2020-03-25 11:41 [PATCH 0/3] arm64: Open code .arch_extension Mark Brown 2020-03-25 11:41 ` [PATCH 1/3] arm64: asm: Provide macro to control enabling architecture extensions Mark Brown 2020-03-25 11:41 ` [PATCH 2/3] arm64: lib: Use ARM64_EXTENSIONS() Mark Brown @ 2020-03-25 11:41 ` Mark Brown 2020-03-25 11:45 ` [PATCH 0/3] arm64: Open code .arch_extension Ard Biesheuvel 2020-03-25 12:31 ` Mark Rutland 4 siblings, 0 replies; 18+ messages in thread From: Mark Brown @ 2020-03-25 11:41 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Eric Biggers, Ard Biesheuvel Cc: linux-arm-kernel, linux-crypto, Mark Brown Use the newly introduced ARM64_EXTENSIONS() macro to enable the crypto extensions. Signed-off-by: Mark Brown <broonie@kernel.org> --- arch/arm64/crypto/aes-ce-ccm-core.S | 3 ++- arch/arm64/crypto/aes-ce-core.S | 2 +- arch/arm64/crypto/aes-ce.S | 2 +- arch/arm64/crypto/crct10dif-ce-core.S | 3 ++- arch/arm64/crypto/ghash-ce-core.S | 3 ++- arch/arm64/crypto/sha1-ce-core.S | 3 ++- arch/arm64/crypto/sha2-ce-core.S | 3 ++- 7 files changed, 12 insertions(+), 7 deletions(-) diff --git a/arch/arm64/crypto/aes-ce-ccm-core.S b/arch/arm64/crypto/aes-ce-ccm-core.S index 99a028e298ed..bb6d85c2a260 100644 --- a/arch/arm64/crypto/aes-ce-ccm-core.S +++ b/arch/arm64/crypto/aes-ce-ccm-core.S @@ -9,7 +9,8 @@ #include <asm/assembler.h> .text - .arch armv8-a+crypto + +ARM64_EXTENSIONS(crypto) /* * void ce_aes_ccm_auth_data(u8 mac[], u8 const in[], u32 abytes, diff --git a/arch/arm64/crypto/aes-ce-core.S b/arch/arm64/crypto/aes-ce-core.S index e52e13eb8fdb..a8291111f68d 100644 --- a/arch/arm64/crypto/aes-ce-core.S +++ b/arch/arm64/crypto/aes-ce-core.S @@ -6,7 +6,7 @@ #include <linux/linkage.h> #include <asm/assembler.h> - .arch armv8-a+crypto +ARM64_EXTENSIONS(crypto) SYM_FUNC_START(__aes_ce_encrypt) sub w3, w3, #2 diff --git a/arch/arm64/crypto/aes-ce.S b/arch/arm64/crypto/aes-ce.S index 1dc5bbbfeed2..6493a8e8d8d0 100644 --- a/arch/arm64/crypto/aes-ce.S +++ b/arch/arm64/crypto/aes-ce.S @@ -12,7 +12,7 @@ #define AES_FUNC_START(func) SYM_FUNC_START(ce_ ## func) #define AES_FUNC_END(func) SYM_FUNC_END(ce_ ## func) - .arch armv8-a+crypto +ARM64_EXTENSIONS(crypto) xtsmask .req v16 cbciv .req v16 diff --git a/arch/arm64/crypto/crct10dif-ce-core.S b/arch/arm64/crypto/crct10dif-ce-core.S index 5a95c2628fbf..bb6f3a14e9e8 100644 --- a/arch/arm64/crypto/crct10dif-ce-core.S +++ b/arch/arm64/crypto/crct10dif-ce-core.S @@ -66,7 +66,8 @@ #include <asm/assembler.h> .text - .cpu generic+crypto + +ARM64_EXTENSIONS(crypto) init_crc .req w19 buf .req x20 diff --git a/arch/arm64/crypto/ghash-ce-core.S b/arch/arm64/crypto/ghash-ce-core.S index 6b958dcdf136..85839b701c83 100644 --- a/arch/arm64/crypto/ghash-ce-core.S +++ b/arch/arm64/crypto/ghash-ce-core.S @@ -57,7 +57,8 @@ HH34 .req v19 .text - .arch armv8-a+crypto + +ARM64_EXTENSIONS(crypto) .macro __pmull_p64, rd, rn, rm pmull \rd\().1q, \rn\().1d, \rm\().1d diff --git a/arch/arm64/crypto/sha1-ce-core.S b/arch/arm64/crypto/sha1-ce-core.S index 92d0d2753e81..8fa2d920be36 100644 --- a/arch/arm64/crypto/sha1-ce-core.S +++ b/arch/arm64/crypto/sha1-ce-core.S @@ -9,7 +9,8 @@ #include <asm/assembler.h> .text - .arch armv8-a+crypto + +ARM64_EXTENSIONS(crypto) k0 .req v0 k1 .req v1 diff --git a/arch/arm64/crypto/sha2-ce-core.S b/arch/arm64/crypto/sha2-ce-core.S index 3f9d0f326987..d8680b43a3fd 100644 --- a/arch/arm64/crypto/sha2-ce-core.S +++ b/arch/arm64/crypto/sha2-ce-core.S @@ -9,7 +9,8 @@ #include <asm/assembler.h> .text - .arch armv8-a+crypto + +ARM64_EXTENSIONS(crypto) dga .req q20 dgav .req v20 -- 2.20.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 11:41 [PATCH 0/3] arm64: Open code .arch_extension Mark Brown ` (2 preceding siblings ...) 2020-03-25 11:41 ` [PATCH 3/3] arm64: crypto: " Mark Brown @ 2020-03-25 11:45 ` Ard Biesheuvel 2020-03-25 11:50 ` Mark Brown 2020-03-25 12:31 ` Mark Rutland 4 siblings, 1 reply; 18+ messages in thread From: Ard Biesheuvel @ 2020-03-25 11:45 UTC (permalink / raw) To: Mark Brown Cc: Catalin Marinas, Will Deacon, Eric Biggers, linux-arm-kernel, linux-crypto On Wed, 25 Mar 2020 at 12:41, Mark Brown <broonie@kernel.org> wrote: > > Currently several assembler files override the default architecture to > enable extensions in order to allow them to implement optimised routines > for systems with those extensions. Since inserting BTI landing pads into > assembler functions will require us to change the default architecture we > need a way to enable extensions without hard coding the architecture. > The assembler has the .arch_extension feature but this was introduced > for arm64 in gas 2.26 which is too modern for us to rely on it. > > We could just update the base architecture used by these assembler files > but this would mean the assembler would no longer catch attempts to use > newer instructions so instead introduce a macro which sets the default > architecture centrally. Doing this will also make our use of .arch and > .cpu to select the base architecture more consistent. > > Mark Brown (3): > arm64: asm: Provide macro to control enabling architecture extensions > arm64: lib: Use ARM64_EXTENSIONS() > arm64: crypto: Use ARM64_EXTENSIONS() > Hi Mark, I don't think this is the right fix. What is wrong with keeping these .cpu and .arch directives in the .S files, and simply make SYM_FUNC_START() expand to something that includes .arch_extension pac or .arch_extension bti when needed? That way, we only use .arch_extension when we know the assembler supports it (given that .arch_extension support itself should predate BTI or PAC support in GAS or Clang) > arch/arm64/crypto/aes-ce-ccm-core.S | 3 ++- > arch/arm64/crypto/aes-ce-core.S | 2 +- > arch/arm64/crypto/aes-ce.S | 2 +- > arch/arm64/crypto/crct10dif-ce-core.S | 3 ++- > arch/arm64/crypto/ghash-ce-core.S | 3 ++- > arch/arm64/crypto/sha1-ce-core.S | 3 ++- > arch/arm64/crypto/sha2-ce-core.S | 3 ++- > arch/arm64/include/asm/linkage.h | 6 ++++++ > arch/arm64/lib/crc32.S | 2 +- > 9 files changed, 19 insertions(+), 8 deletions(-) > > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 11:45 ` [PATCH 0/3] arm64: Open code .arch_extension Ard Biesheuvel @ 2020-03-25 11:50 ` Mark Brown 2020-03-25 11:54 ` Ard Biesheuvel 2020-04-22 18:00 ` Catalin Marinas 0 siblings, 2 replies; 18+ messages in thread From: Mark Brown @ 2020-03-25 11:50 UTC (permalink / raw) To: Ard Biesheuvel Cc: Catalin Marinas, Will Deacon, Eric Biggers, linux-arm-kernel, linux-crypto [-- Attachment #1: Type: text/plain, Size: 707 bytes --] On Wed, Mar 25, 2020 at 12:45:11PM +0100, Ard Biesheuvel wrote: > I don't think this is the right fix. What is wrong with keeping these > .cpu and .arch directives in the .S files, and simply make > SYM_FUNC_START() expand to something that includes .arch_extension pac > or .arch_extension bti when needed? That way, we only use > .arch_extension when we know the assembler supports it (given that > .arch_extension support itself should predate BTI or PAC support in > GAS or Clang) Since BTI is a mandatory feature of v8.5 there is no BTI arch_extension, you can only enable it by moving the base architecture to v8.5. You'd need to use .arch and that feels likely to find us sharp edges to run into. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 11:50 ` Mark Brown @ 2020-03-25 11:54 ` Ard Biesheuvel 2020-03-25 12:01 ` Mark Brown 2020-03-25 12:03 ` Mark Rutland 2020-04-22 18:00 ` Catalin Marinas 1 sibling, 2 replies; 18+ messages in thread From: Ard Biesheuvel @ 2020-03-25 11:54 UTC (permalink / raw) To: Mark Brown Cc: Catalin Marinas, Will Deacon, Eric Biggers, linux-arm-kernel, linux-crypto On Wed, 25 Mar 2020 at 12:50, Mark Brown <broonie@kernel.org> wrote: > > On Wed, Mar 25, 2020 at 12:45:11PM +0100, Ard Biesheuvel wrote: > > > I don't think this is the right fix. What is wrong with keeping these > > .cpu and .arch directives in the .S files, and simply make > > SYM_FUNC_START() expand to something that includes .arch_extension pac > > or .arch_extension bti when needed? That way, we only use > > .arch_extension when we know the assembler supports it (given that > > .arch_extension support itself should predate BTI or PAC support in > > GAS or Clang) > > Since BTI is a mandatory feature of v8.5 there is no BTI arch_extension, > you can only enable it by moving the base architecture to v8.5. You'd > need to use .arch and that feels likely to find us sharp edges to run > into. I think we should talk to the toolchain folks about this. Even if .arch_extension today does not support the 'bti' argument, it *is* most definitely an architecture extension, even it it is mandatory in v8.5 (given that v8.5 is itself an architecture extension). ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 11:54 ` Ard Biesheuvel @ 2020-03-25 12:01 ` Mark Brown 2020-03-25 12:03 ` Mark Rutland 1 sibling, 0 replies; 18+ messages in thread From: Mark Brown @ 2020-03-25 12:01 UTC (permalink / raw) To: Ard Biesheuvel Cc: Catalin Marinas, Will Deacon, Eric Biggers, linux-arm-kernel, linux-crypto [-- Attachment #1: Type: text/plain, Size: 907 bytes --] On Wed, Mar 25, 2020 at 12:54:10PM +0100, Ard Biesheuvel wrote: > On Wed, 25 Mar 2020 at 12:50, Mark Brown <broonie@kernel.org> wrote: > > Since BTI is a mandatory feature of v8.5 there is no BTI arch_extension, > > you can only enable it by moving the base architecture to v8.5. You'd > > need to use .arch and that feels likely to find us sharp edges to run > > into. > I think we should talk to the toolchain folks about this. Even if > .arch_extension today does not support the 'bti' argument, it *is* > most definitely an architecture extension, even it it is mandatory in > v8.5 (given that v8.5 is itself an architecture extension). I agree entirely, the current behaviour is surprising and doesn't really map onto how the architecture is described - my first thought was similar to yours. It won't help us right now but it would help for future architecture extensions and for other projects. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 11:54 ` Ard Biesheuvel 2020-03-25 12:01 ` Mark Brown @ 2020-03-25 12:03 ` Mark Rutland 2020-03-25 12:24 ` Robin Murphy 1 sibling, 1 reply; 18+ messages in thread From: Mark Rutland @ 2020-03-25 12:03 UTC (permalink / raw) To: Ard Biesheuvel Cc: Mark Brown, Catalin Marinas, Will Deacon, linux-crypto, linux-arm-kernel, Eric Biggers On Wed, Mar 25, 2020 at 12:54:10PM +0100, Ard Biesheuvel wrote: > On Wed, 25 Mar 2020 at 12:50, Mark Brown <broonie@kernel.org> wrote: > > Since BTI is a mandatory feature of v8.5 there is no BTI arch_extension, > > you can only enable it by moving the base architecture to v8.5. You'd > > need to use .arch and that feels likely to find us sharp edges to run > > into. > > I think we should talk to the toolchain folks about this. Even if > .arch_extension today does not support the 'bti' argument, it *is* > most definitely an architecture extension, even it it is mandatory in > v8.5 (given that v8.5 is itself an architecture extension). It certianly seems unfortunate, as it goes against the premise of having HINT space instructions. Most software will want to enable HINT space instructions from ARMv8.x but nothing else to ensure binary compatibility with existing hardware. I see the same is true for pointer authentication judging by: https://sourceware.org/binutils/docs/as/AArch64-Extensions.html#AArch64-Extensions ... so worth raising with toolchain folk as a general principle even if we have to bodge around it for now. Thanks, Mark. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 12:03 ` Mark Rutland @ 2020-03-25 12:24 ` Robin Murphy 0 siblings, 0 replies; 18+ messages in thread From: Robin Murphy @ 2020-03-25 12:24 UTC (permalink / raw) To: Mark Rutland, Ard Biesheuvel Cc: Eric Biggers, Catalin Marinas, Mark Brown, linux-crypto, Will Deacon, linux-arm-kernel On 2020-03-25 12:03 pm, Mark Rutland wrote: > On Wed, Mar 25, 2020 at 12:54:10PM +0100, Ard Biesheuvel wrote: >> On Wed, 25 Mar 2020 at 12:50, Mark Brown <broonie@kernel.org> wrote: >>> Since BTI is a mandatory feature of v8.5 there is no BTI arch_extension, >>> you can only enable it by moving the base architecture to v8.5. You'd >>> need to use .arch and that feels likely to find us sharp edges to run >>> into. >> >> I think we should talk to the toolchain folks about this. Even if >> .arch_extension today does not support the 'bti' argument, it *is* >> most definitely an architecture extension, even it it is mandatory in >> v8.5 (given that v8.5 is itself an architecture extension). > > It certianly seems unfortunate, as it goes against the premise of having > HINT space instructions. Most software will want to enable HINT space > instructions from ARMv8.x but nothing else to ensure binary > compatibility with existing hardware. > > I see the same is true for pointer authentication judging by: > > https://sourceware.org/binutils/docs/as/AArch64-Extensions.html#AArch64-Extensions > > ... so worth raising with toolchain folk as a general principle even if > we have to bodge around it for now. Indeed, in general, just because a feature is mandatory in v8.n doesn't necessarily mean it can't be included in a v8.(n-1) implementation which may not have the *other* mandatory parts of v8.n which ".arch armv8.<n>-a" would let through. Robin. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 11:50 ` Mark Brown 2020-03-25 11:54 ` Ard Biesheuvel @ 2020-04-22 18:00 ` Catalin Marinas 2020-04-23 11:18 ` Mark Brown 1 sibling, 1 reply; 18+ messages in thread From: Catalin Marinas @ 2020-04-22 18:00 UTC (permalink / raw) To: Mark Brown Cc: Ard Biesheuvel, Will Deacon, linux-crypto, linux-arm-kernel, Eric Biggers On Wed, Mar 25, 2020 at 11:50:38AM +0000, Mark Brown wrote: > On Wed, Mar 25, 2020 at 12:45:11PM +0100, Ard Biesheuvel wrote: > > I don't think this is the right fix. What is wrong with keeping these > > .cpu and .arch directives in the .S files, and simply make > > SYM_FUNC_START() expand to something that includes .arch_extension pac > > or .arch_extension bti when needed? That way, we only use > > .arch_extension when we know the assembler supports it (given that > > .arch_extension support itself should predate BTI or PAC support in > > GAS or Clang) > > Since BTI is a mandatory feature of v8.5 there is no BTI arch_extension, > you can only enable it by moving the base architecture to v8.5. You'd > need to use .arch and that feels likely to find us sharp edges to run > into. For MTE, .arch armv8-a+memtag won't work since this is only available with armv8.5-a. My preference would be to have the highest arch version supported by the kernel in the assembler.h file, i.e. ".arch armv8.5-a" followed by .arch_extension in each .S file, as needed. Forcing .S files to armv8.5 would not cause any problems with the base armv8.0 that the kernel image support since it shouldn't change the opcodes gas generates. The .S files would use alternatives anyway (or simply have code not called). The inline asm is slightly more problematic, especially with the clang builtin assembler which goes in a single pass. But we could do something similar to what we did with the LSE atomics and raising the base of the inline asm to armv8.5 (or 8.6 etc., whatever we need in the future). -- Catalin ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-04-22 18:00 ` Catalin Marinas @ 2020-04-23 11:18 ` Mark Brown 2020-04-23 11:59 ` Catalin Marinas 0 siblings, 1 reply; 18+ messages in thread From: Mark Brown @ 2020-04-23 11:18 UTC (permalink / raw) To: Catalin Marinas Cc: Ard Biesheuvel, Will Deacon, linux-crypto, linux-arm-kernel, Eric Biggers [-- Attachment #1: Type: text/plain, Size: 1587 bytes --] On Wed, Apr 22, 2020 at 07:00:28PM +0100, Catalin Marinas wrote: > On Wed, Mar 25, 2020 at 11:50:38AM +0000, Mark Brown wrote: > > Since BTI is a mandatory feature of v8.5 there is no BTI arch_extension, > > you can only enable it by moving the base architecture to v8.5. You'd > > need to use .arch and that feels likely to find us sharp edges to run > > into. > For MTE, .arch armv8-a+memtag won't work since this is only available > with armv8.5-a. My preference would be to have the highest arch version > supported by the kernel in the assembler.h file, i.e. ".arch armv8.5-a" > followed by .arch_extension in each .S file, as needed. I think we decided that .arch_extension was too new to be used for things like the crypto stuff where we still support older toolchains? > Forcing .S files to armv8.5 would not cause any problems with > the base armv8.0 that the kernel image support since it shouldn't change > the opcodes gas generates. The .S files would use alternatives anyway > (or simply have code not called). We do loose the checking that the assembler does that nobody used a newer feature by mistake but yeah, shouldn't affect the output. > The inline asm is slightly more problematic, especially with the clang > builtin assembler which goes in a single pass. But we could do something > similar to what we did with the LSE atomics and raising the base of the > inline asm to armv8.5 (or 8.6 etc., whatever we need in the future). FWIW I did something different to this for BTI so I wasn't using the instructions directly so I was going to abandon this series. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-04-23 11:18 ` Mark Brown @ 2020-04-23 11:59 ` Catalin Marinas 2020-04-23 13:40 ` Catalin Marinas 0 siblings, 1 reply; 18+ messages in thread From: Catalin Marinas @ 2020-04-23 11:59 UTC (permalink / raw) To: Mark Brown Cc: Ard Biesheuvel, Will Deacon, linux-crypto, linux-arm-kernel, Eric Biggers On Thu, Apr 23, 2020 at 12:18:03PM +0100, Mark Brown wrote: > On Wed, Apr 22, 2020 at 07:00:28PM +0100, Catalin Marinas wrote: > > On Wed, Mar 25, 2020 at 11:50:38AM +0000, Mark Brown wrote: > > > Since BTI is a mandatory feature of v8.5 there is no BTI arch_extension, > > > you can only enable it by moving the base architecture to v8.5. You'd > > > need to use .arch and that feels likely to find us sharp edges to run > > > into. > > > For MTE, .arch armv8-a+memtag won't work since this is only available > > with armv8.5-a. My preference would be to have the highest arch version > > supported by the kernel in the assembler.h file, i.e. ".arch armv8.5-a" > > followed by .arch_extension in each .S file, as needed. > > I think we decided that .arch_extension was too new to be used for > things like the crypto stuff where we still support older toolchains? .arch_extension would be issued conditionally only for features like CONFIG_ARM64_MTE which already have a dependency on a newer toolchain. However, '.arch_extension memtag' is not sufficient for MTE, it needs a prior '.arch armv8.5-a'. > > Forcing .S files to armv8.5 would not cause any problems with > > the base armv8.0 that the kernel image support since it shouldn't change > > the opcodes gas generates. The .S files would use alternatives anyway > > (or simply have code not called). > > We do loose the checking that the assembler does that nobody used a > newer feature by mistake but yeah, shouldn't affect the output. We may need some push/pop_arch macros to contain the supported features. The gas documentation says that .arch_extension may be used multiple times to add or remove extensions. However, I couldn't find a way to remove memtag after adding it (tried -memtag, !memtag, empty string). So I may go with a '.arch armv8.0-a' as a base, followed by temporary setting of '.arch armv8.5-a+memtag' (and hope we don't need combinations of such extensions). > > The inline asm is slightly more problematic, especially with the clang > > builtin assembler which goes in a single pass. But we could do something > > similar to what we did with the LSE atomics and raising the base of the > > inline asm to armv8.5 (or 8.6 etc., whatever we need in the future). > > FWIW I did something different to this for BTI so I wasn't using the > instructions directly so I was going to abandon this series. I can't work around this easily for MTE, there are more instructions with register encoding. I'll see if the push/pop idea works or just leave it to whoever does the next feature, figure out how it interacts with MTE ;). -- Catalin ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-04-23 11:59 ` Catalin Marinas @ 2020-04-23 13:40 ` Catalin Marinas 0 siblings, 0 replies; 18+ messages in thread From: Catalin Marinas @ 2020-04-23 13:40 UTC (permalink / raw) To: Mark Brown Cc: Eric Biggers, Will Deacon, Ard Biesheuvel, linux-arm-kernel, linux-crypto On Thu, Apr 23, 2020 at 12:59:05PM +0100, Catalin Marinas wrote: > On Thu, Apr 23, 2020 at 12:18:03PM +0100, Mark Brown wrote: > > On Wed, Apr 22, 2020 at 07:00:28PM +0100, Catalin Marinas wrote: > > > Forcing .S files to armv8.5 would not cause any problems with > > > the base armv8.0 that the kernel image support since it shouldn't change > > > the opcodes gas generates. The .S files would use alternatives anyway > > > (or simply have code not called). > > > > We do loose the checking that the assembler does that nobody used a > > newer feature by mistake but yeah, shouldn't affect the output. > > We may need some push/pop_arch macros to contain the supported features. > > The gas documentation says that .arch_extension may be used multiple > times to add or remove extensions. However, I couldn't find a way to > remove memtag after adding it (tried -memtag, !memtag, empty string). So > I may go with a '.arch armv8.0-a' as a base, followed by temporary > setting of '.arch armv8.5-a+memtag' (and hope we don't need combinations > of such extensions). Quick attempt at this on top of the MTE patches: diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index e7338e129dfd..6180ac605406 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -24,10 +24,18 @@ #include <asm/sysreg.h> #include <asm/thread_info.h> -#ifdef CONFIG_ARM64_MTE - .arch armv8.5-a - .arch_extension memtag -#endif + /* Base architecture version for the .S files */ + .arch armv8-a + + .macro arm64_set_arch, arch, enable = 1 + .if \enable + .arch \arch + .endif + .endm + + .macro arm64_reset_arch + .arch armv8-a + .endm .macro save_and_disable_daif, flags mrs \flags, daif diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index e4ab82e543cf..df2037fc431b 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -148,6 +148,7 @@ alternative_cb_end /* Check for MTE asynchronous tag check faults */ .macro check_mte_async_tcf, flgs, tmp #ifdef CONFIG_ARM64_MTE + arm64_set_arch armv8.5-a+memtag alternative_if_not ARM64_MTE b 1f alternative_else_nop_endif @@ -158,16 +159,19 @@ alternative_else_nop_endif str \flgs, [tsk, #TSK_TI_FLAGS] msr_s SYS_TFSRE0_EL1, xzr 1: + arm64_reset_arch #endif .endm /* Clear the MTE asynchronous tag check faults */ .macro clear_mte_async_tcf #ifdef CONFIG_ARM64_MTE + arm64_set_arch armv8.5-a+memtag alternative_if ARM64_MTE dsb ish msr_s SYS_TFSRE0_EL1, xzr alternative_else_nop_endif + arm64_reset_arch #endif .endm diff --git a/arch/arm64/lib/clear_page.S b/arch/arm64/lib/clear_page.S index 9f85a4cf9568..a8e26f232502 100644 --- a/arch/arm64/lib/clear_page.S +++ b/arch/arm64/lib/clear_page.S @@ -22,8 +22,10 @@ SYM_FUNC_START(clear_page) mov x2, #4 lsl x1, x2, x1 1: + arm64_set_arch armv8.5-a+memtag, IS_ENABLED(CONFIG_ARM64_MTE) alternative_insn "dc zva, x0", "stzgm xzr, [x0]", \ ARM64_MTE, IS_ENABLED(CONFIG_ARM64_MTE), 1 + arm64_reset_arch add x0, x0, x1 tst x0, #(PAGE_SIZE - 1) b.ne 1b diff --git a/arch/arm64/lib/copy_page.S b/arch/arm64/lib/copy_page.S index c3234175efe0..8322043e75e6 100644 --- a/arch/arm64/lib/copy_page.S +++ b/arch/arm64/lib/copy_page.S @@ -26,6 +26,7 @@ alternative_if ARM64_HAS_NO_HW_PREFETCH alternative_else_nop_endif #ifdef CONFIG_ARM64_MTE + arm64_set_arch armv8.5-a+memtag alternative_if_not ARM64_MTE b 2f alternative_else_nop_endif @@ -46,6 +47,7 @@ alternative_else_nop_endif tst x2, #(PAGE_SIZE - 1) b.ne 1b 2: + arm64_reset_arch #endif ldp x2, x3, [x1] diff --git a/arch/arm64/lib/mte.S b/arch/arm64/lib/mte.S index 45be04a8c73c..8f824fc62ad4 100644 --- a/arch/arm64/lib/mte.S +++ b/arch/arm64/lib/mte.S @@ -7,6 +7,8 @@ #include <asm/assembler.h> #include <asm/mte.h> + arm64_set_arch armv8.5-a+memtag + /* * Compare tags of two pages * x0 - page1 address -- Catalin ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 11:41 [PATCH 0/3] arm64: Open code .arch_extension Mark Brown ` (3 preceding siblings ...) 2020-03-25 11:45 ` [PATCH 0/3] arm64: Open code .arch_extension Ard Biesheuvel @ 2020-03-25 12:31 ` Mark Rutland 2020-03-25 13:26 ` Ard Biesheuvel 2020-03-25 13:27 ` Mark Brown 4 siblings, 2 replies; 18+ messages in thread From: Mark Rutland @ 2020-03-25 12:31 UTC (permalink / raw) To: Mark Brown Cc: Catalin Marinas, Will Deacon, Eric Biggers, Ard Biesheuvel, linux-crypto, linux-arm-kernel On Wed, Mar 25, 2020 at 11:41:07AM +0000, Mark Brown wrote: > Since inserting BTI landing pads into assembler functions will require > us to change the default architecture we need a way to enable > extensions without hard coding the architecture. Assuming we'll poke the toolchain folk, let's consider alternative ways around this in the mean time. Is there anything akin to push/pop versions of .arch directitves that we can use around the BTI instructions specifically? ... or could we encode the BTI instructions with a .inst, and wrap those in macros so that GAS won't complain (like we do for mrs_s and friends)? ... does asking GCC to use BTI for C code make the default arch v8.5 for inline asm, or does it do something special to allow BTI instructions in specific locations? Thanks, Mark. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 12:31 ` Mark Rutland @ 2020-03-25 13:26 ` Ard Biesheuvel 2020-03-25 13:30 ` Mark Brown 2020-03-25 13:27 ` Mark Brown 1 sibling, 1 reply; 18+ messages in thread From: Ard Biesheuvel @ 2020-03-25 13:26 UTC (permalink / raw) To: Mark Rutland Cc: Mark Brown, Catalin Marinas, Will Deacon, Eric Biggers, linux-crypto, linux-arm-kernel , On Wed, 25 Mar 2020 at 13:32, Mark Rutland <mark.rutland@arm.com> wrote: > > On Wed, Mar 25, 2020 at 11:41:07AM +0000, Mark Brown wrote: > > Since inserting BTI landing pads into assembler functions will require > > us to change the default architecture we need a way to enable > > extensions without hard coding the architecture. > > Assuming we'll poke the toolchain folk, let's consider alternative ways > around this in the mean time. > > Is there anything akin to push/pop versions of .arch directitves that we > can use around the BTI instructions specifically? > > ... or could we encode the BTI instructions with a .inst, and wrap those > in macros so that GAS won't complain (like we do for mrs_s and friends)? > > ... does asking GCC to use BTI for C code make the default arch v8.5 for > inline asm, or does it do something special to allow BTI instructions in > specific locations? > I think using macros wrapping .inst directives is the most hassle free way to achieve this, assuming there is no need to encode registers or immediates (which makes it slightly messy - refer to arch/arm64/crypto/sm3-ce-core.S for an example) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 13:26 ` Ard Biesheuvel @ 2020-03-25 13:30 ` Mark Brown 0 siblings, 0 replies; 18+ messages in thread From: Mark Brown @ 2020-03-25 13:30 UTC (permalink / raw) To: Ard Biesheuvel Cc: Mark Rutland, Catalin Marinas, Will Deacon, Eric Biggers, linux-crypto, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 405 bytes --] On Wed, Mar 25, 2020 at 02:26:49PM +0100, Ard Biesheuvel wrote: > I think using macros wrapping .inst directives is the most hassle free > way to achieve this, assuming there is no need to encode registers or > immediates (which makes it slightly messy - refer to > arch/arm64/crypto/sm3-ce-core.S for an example) There isn't - you just have to encode the four target classes, of which we only use one. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] arm64: Open code .arch_extension 2020-03-25 12:31 ` Mark Rutland 2020-03-25 13:26 ` Ard Biesheuvel @ 2020-03-25 13:27 ` Mark Brown 1 sibling, 0 replies; 18+ messages in thread From: Mark Brown @ 2020-03-25 13:27 UTC (permalink / raw) To: Mark Rutland Cc: Catalin Marinas, Will Deacon, Eric Biggers, Ard Biesheuvel, linux-crypto, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 939 bytes --] On Wed, Mar 25, 2020 at 12:31:59PM +0000, Mark Rutland wrote: > Is there anything akin to push/pop versions of .arch directitves that we > can use around the BTI instructions specifically? Not that I can see. > ... or could we encode the BTI instructions with a .inst, and wrap those > in macros so that GAS won't complain (like we do for mrs_s and friends)? That should work, I think it's a taste thing which is better. For me it was a combination of there being a small number of files that were affected, the fact that even within that small set there was divergence in how they were doing this and the fact that neither solution is a thing of great beauty. > ... does asking GCC to use BTI for C code make the default arch v8.5 for > inline asm, or does it do something special to allow BTI instructions in > specific locations? This is only an issue for freestanding assembly files as far as I can see. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-04-23 13:40 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-25 11:41 [PATCH 0/3] arm64: Open code .arch_extension Mark Brown 2020-03-25 11:41 ` [PATCH 1/3] arm64: asm: Provide macro to control enabling architecture extensions Mark Brown 2020-03-25 11:41 ` [PATCH 2/3] arm64: lib: Use ARM64_EXTENSIONS() Mark Brown 2020-03-25 11:41 ` [PATCH 3/3] arm64: crypto: " Mark Brown 2020-03-25 11:45 ` [PATCH 0/3] arm64: Open code .arch_extension Ard Biesheuvel 2020-03-25 11:50 ` Mark Brown 2020-03-25 11:54 ` Ard Biesheuvel 2020-03-25 12:01 ` Mark Brown 2020-03-25 12:03 ` Mark Rutland 2020-03-25 12:24 ` Robin Murphy 2020-04-22 18:00 ` Catalin Marinas 2020-04-23 11:18 ` Mark Brown 2020-04-23 11:59 ` Catalin Marinas 2020-04-23 13:40 ` Catalin Marinas 2020-03-25 12:31 ` Mark Rutland 2020-03-25 13:26 ` Ard Biesheuvel 2020-03-25 13:30 ` Mark Brown 2020-03-25 13:27 ` Mark Brown
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).