[PATCH 0/3] arm64: Open code .arch_extension

[PATCH 0/3] arm64: Open code .arch_extension

[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

[PATCH 1/3] arm64: asm: Provide macro to control enabling architecture extensions

[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

[PATCH 2/3] arm64: lib: Use ARM64_EXTENSIONS()

[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

[PATCH 3/3] arm64: crypto: Use ARM64_EXTENSIONS()

[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

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Ard Biesheuvel]

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
>

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Mark Brown]

[-- 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 --]

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Ard Biesheuvel]

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).

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Mark Brown]

[-- 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 --]

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Mark Rutland]

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.

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Robin Murphy]

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.

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Mark Rutland]

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.

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Ard Biesheuvel]

,

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)

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Mark Brown]

[-- 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 --]

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Mark Brown]

[-- 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 --]

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Catalin Marinas]

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

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Mark Brown]

[-- 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 --]

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Catalin Marinas]

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

Re: [PATCH 0/3] arm64: Open code .arch_extension

[Catalin Marinas]

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