Will noticed that with newer toolchains memcpy() ends up being implemented with SVE instructions, breaking the signals tests when in streaming mode. We fixed this by using an open coded version of OPTIMZER_HIDE_VAR(), but in the process it was noticed that some of the selftests are using the tools/include headers and it might be nice to share things there. We also have a custom compiler.h in the BTI tests.
Update the tools/include headers to have what we need, pull them into the arm64 selftests build and make use of them in the signals and BTI tests. Since the resulting changes are a bit invasive for a fix we keep an initial patch using the open coding, updating and replacing that later.
Signed-off-by: Mark Brown broonie@kernel.org --- Changes in v4: - Roll in a refactoring to include and use the tools/include headers. - Link to v3: https://lore.kernel.org/r/20230720-arm64-signal-memcpy-fix-v3-1-08aed2385d68...
Changes in v3: - Open code OPTIMISER_HIDE_VAR() instead of the memory clobber. - Link to v2: https://lore.kernel.org/r/20230712-arm64-signal-memcpy-fix-v2-1-494f7025caf6...
Changes in v2: - Rebase onto v6.5-rc1. - Link to v1: https://lore.kernel.org/r/20230628-arm64-signal-memcpy-fix-v1-1-db3e0300829e...
--- Mark Brown (6): kselftest/arm64: Exit streaming mode after collecting signal context tools compiler.h: Add OPTIMIZER_HIDE_VAR() tools include: Add some common function attributes kselftest/arm64: Make the tools/include headers available kselftest/arm64: Use shared OPTIMZER_HIDE_VAR() definiton kselftest/arm64: Use the tools/include compiler.h rather than our own
tools/include/linux/compiler.h | 18 +++++++++++++++ tools/testing/selftests/arm64/Makefile | 2 ++ tools/testing/selftests/arm64/bti/compiler.h | 21 ----------------- tools/testing/selftests/arm64/bti/system.c | 4 +--- tools/testing/selftests/arm64/bti/system.h | 4 ++-- tools/testing/selftests/arm64/bti/test.c | 1 - .../selftests/arm64/signal/test_signals_utils.h | 27 +++++++++++++++++++++- 7 files changed, 49 insertions(+), 28 deletions(-) --- base-commit: 6eaae198076080886b9e7d57f4ae06fa782f90ef change-id: 20230628-arm64-signal-memcpy-fix-7de3b3c8fa10
Best regards,
When we collect a signal context with one of the SME modes enabled we will have enabled that mode behind the compiler and libc's back so they may issue some instructions not valid in streaming mode, causing spurious failures.
For the code prior to issuing the BRK to trigger signal handling we need to stay in streaming mode if we were already there since that's a part of the signal context the caller is trying to collect. Unfortunately this code includes a memset() which is likely to be heavily optimised and is likely to use FP instructions incompatible with streaming mode. We can avoid this happening by open coding the memset(), inserting a volatile assembly statement to avoid the compiler recognising what's being done and doing something in optimisation. This code is not performance critical so the inefficiency should not be an issue.
After collecting the context we can simply exit streaming mode, avoiding these issues. Use a full SMSTOP for safety to prevent any issues appearing with ZA.
Reported-by: Will Deacon will@kernel.org Signed-off-by: Mark Brown broonie@kernel.org --- .../selftests/arm64/signal/test_signals_utils.h | 25 +++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h index 222093f51b67..c7f5627171dd 100644 --- a/tools/testing/selftests/arm64/signal/test_signals_utils.h +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h @@ -60,13 +60,25 @@ static __always_inline bool get_current_context(struct tdescr *td, size_t dest_sz) { static volatile bool seen_already; + int i; + char *uc = (char *)dest_uc;
assert(td && dest_uc); /* it's a genuine invocation..reinit */ seen_already = 0; td->live_uc_valid = 0; td->live_sz = dest_sz; - memset(dest_uc, 0x00, td->live_sz); + + /* + * This is a memset() but we don't want the compiler to + * optimise it into either instructions or a library call + * which might be incompatible with streaming mode. + */ + for (i = 0; i < td->live_sz; i++) { + uc[i] = 0; + __asm__ ("" : "=r" (uc[i]) : "0" (uc[i])); + } + td->live_uc = dest_uc; /* * Grab ucontext_t triggering a SIGTRAP. @@ -103,6 +115,17 @@ static __always_inline bool get_current_context(struct tdescr *td, : : "memory");
+ /* + * If we were grabbing a streaming mode context then we may + * have entered streaming mode behind the system's back and + * libc or compiler generated code might decide to do + * something invalid in streaming mode, or potentially even + * the state of ZA. Issue a SMSTOP to exit both now we have + * grabbed the state. + */ + if (td->feats_supported & FEAT_SME) + asm volatile("msr S0_3_C4_C6_3, xzr"); + /* * If we get here with seen_already==1 it implies the td->live_uc * context has been used to get back here....this probably means
Port over the definition of OPTIMIZER_HIDE_VAR() so we can use it in kselftests.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/include/linux/compiler.h | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/tools/include/linux/compiler.h b/tools/include/linux/compiler.h index 9d36c8ce1fe7..f75cced41d59 100644 --- a/tools/include/linux/compiler.h +++ b/tools/include/linux/compiler.h @@ -190,4 +190,10 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s #define ___PASTE(a, b) a##b #define __PASTE(a, b) ___PASTE(a, b)
+#ifndef OPTIMIZER_HIDE_VAR +/* Make the optimizer believe the variable can be manipulated arbitrarily. */ +#define OPTIMIZER_HIDE_VAR(var) \ + __asm__ ("" : "=r" (var) : "0" (var)) +#endif + #endif /* _TOOLS_LINUX_COMPILER_H */
We don't have definitions of __always_unused or __noreturn in the tools version of compiler.h, add them so we can use them in kselftests.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/include/linux/compiler.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tools/include/linux/compiler.h b/tools/include/linux/compiler.h index f75cced41d59..1684216e826a 100644 --- a/tools/include/linux/compiler.h +++ b/tools/include/linux/compiler.h @@ -42,6 +42,18 @@ # define __always_inline inline __attribute__((always_inline)) #endif
+#ifndef __always_unused +#define __always_unused __attribute__((__unused__)) +#endif + +#ifndef __noreturn +#define __noreturn __attribute__((__noreturn__)) +#endif + +#ifndef unreachable +#define unreachable() __builtin_unreachable() +#endif + #ifndef noinline #define noinline #endif
Make the generic tools/include headers available to the arm64 selftests so we can reduce some duplication.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/Makefile | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile index ace8b67fb22d..28b93cab8c0d 100644 --- a/tools/testing/selftests/arm64/Makefile +++ b/tools/testing/selftests/arm64/Makefile @@ -19,6 +19,8 @@ CFLAGS += -I$(top_srcdir)/tools/testing/selftests/
CFLAGS += $(KHDR_INCLUDES)
+CFLAGS += -I$(top_srcdir)/tools/include + export CFLAGS export top_srcdir
We had open coded the definition of OPTIMIZER_HIDE_VAR() as a fix but now that we have the generic tools/include available and that has had a definition of OPTIMIZER_HIDE_VAR() we can switch to the define.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/signal/test_signals_utils.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h index c7f5627171dd..762c8fe9c54a 100644 --- a/tools/testing/selftests/arm64/signal/test_signals_utils.h +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h @@ -8,6 +8,8 @@ #include <stdio.h> #include <string.h>
+#include <linux/compiler.h> + #include "test_signals.h"
int test_init(struct tdescr *td); @@ -76,7 +78,7 @@ static __always_inline bool get_current_context(struct tdescr *td, */ for (i = 0; i < td->live_sz; i++) { uc[i] = 0; - __asm__ ("" : "=r" (uc[i]) : "0" (uc[i])); + OPTIMIZER_HIDE_VAR(uc[0]); }
td->live_uc = dest_uc;
The BTI test program started life as standalone programs outside the kselftest suite so provided it's own compiler.h. Now that we have updated the tools/include compiler.h to have all the definitions that we are using and the arm64 selftsets pull in tools/includes let's drop our custom version.
__unreachable() is named unreachable() there requiring an update in the code.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/bti/compiler.h | 21 --------------------- tools/testing/selftests/arm64/bti/system.c | 4 +--- tools/testing/selftests/arm64/bti/system.h | 4 ++-- tools/testing/selftests/arm64/bti/test.c | 1 - 4 files changed, 3 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/arm64/bti/compiler.h b/tools/testing/selftests/arm64/bti/compiler.h deleted file mode 100644 index ebb6204f447a..000000000000 --- a/tools/testing/selftests/arm64/bti/compiler.h +++ /dev/null @@ -1,21 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/* - * Copyright (C) 2019 Arm Limited - * Original author: Dave Martin Dave.Martin@arm.com - */ - -#ifndef COMPILER_H -#define COMPILER_H - -#define __always_unused __attribute__((__unused__)) -#define __noreturn __attribute__((__noreturn__)) -#define __unreachable() __builtin_unreachable() - -/* curse(e) has value e, but the compiler cannot assume so */ -#define curse(e) ({ \ - __typeof__(e) __curse_e = (e); \ - asm ("" : "+r" (__curse_e)); \ - __curse_e; \ -}) - -#endif /* ! COMPILER_H */ diff --git a/tools/testing/selftests/arm64/bti/system.c b/tools/testing/selftests/arm64/bti/system.c index 6385d8d4973b..93d772b00bfe 100644 --- a/tools/testing/selftests/arm64/bti/system.c +++ b/tools/testing/selftests/arm64/bti/system.c @@ -8,12 +8,10 @@
#include <asm/unistd.h>
-#include "compiler.h" - void __noreturn exit(int n) { syscall(__NR_exit, n); - __unreachable(); + unreachable(); }
ssize_t write(int fd, const void *buf, size_t size) diff --git a/tools/testing/selftests/arm64/bti/system.h b/tools/testing/selftests/arm64/bti/system.h index aca118589705..2e9ee1284a0c 100644 --- a/tools/testing/selftests/arm64/bti/system.h +++ b/tools/testing/selftests/arm64/bti/system.h @@ -14,12 +14,12 @@ typedef __kernel_size_t size_t; typedef __kernel_ssize_t ssize_t;
#include <linux/errno.h> +#include <linux/compiler.h> + #include <asm/hwcap.h> #include <asm/ptrace.h> #include <asm/unistd.h>
-#include "compiler.h" - long syscall(int nr, ...);
void __noreturn exit(int n); diff --git a/tools/testing/selftests/arm64/bti/test.c b/tools/testing/selftests/arm64/bti/test.c index 2cd8dcee5aec..28a8e8a28a84 100644 --- a/tools/testing/selftests/arm64/bti/test.c +++ b/tools/testing/selftests/arm64/bti/test.c @@ -17,7 +17,6 @@ typedef struct ucontext ucontext_t;
#include "btitest.h" -#include "compiler.h" #include "signal.h"
#define EXPECTED_TESTS 18
On Fri, 28 Jul 2023 00:26:11 +0100, Mark Brown wrote:
Will noticed that with newer toolchains memcpy() ends up being implemented with SVE instructions, breaking the signals tests when in streaming mode. We fixed this by using an open coded version of OPTIMZER_HIDE_VAR(), but in the process it was noticed that some of the selftests are using the tools/include headers and it might be nice to share things there. We also have a custom compiler.h in the BTI tests.
[...]
Applied to arm64 (for-next/selftests), thanks!
[1/6] kselftest/arm64: Exit streaming mode after collecting signal context https://git.kernel.org/arm64/c/d6da04b6fbab [2/6] tools compiler.h: Add OPTIMIZER_HIDE_VAR() https://git.kernel.org/arm64/c/e5d51a665021 [3/6] tools include: Add some common function attributes https://git.kernel.org/arm64/c/51e6ac1fa451 [4/6] kselftest/arm64: Make the tools/include headers available https://git.kernel.org/arm64/c/35d7bc983a74 [5/6] kselftest/arm64: Use shared OPTIMZER_HIDE_VAR() definiton https://git.kernel.org/arm64/c/db7a89f706d6 [6/6] kselftest/arm64: Use the tools/include compiler.h rather than our own https://git.kernel.org/arm64/c/672dbf97f612
Cheers,
linux-kselftest-mirror@lists.linaro.org