When SME support was merged support for TPIDR2 in signal frames was omitted, meaning that it was not possible for signal handers to inspect or modify it. This will present an issue for programs using signals to implement lightweight threads so let's provide access to TPIDR2 in signal handlers.
Implement a new record type for TPIDR2 using the same format as we use for ESR and add coverage to make sure that this appears in the signal context as expected. Due to TPIDR2 being reserved for libc we only validate that the value is unchanged, meaning we're likely to just be validating the default value of 0 on current systems. I have tested with a modified version that sets an explicit value.
v3: - Rebase onto v6.2-rc1. v2: - Rebase onto v6.1-rc3. - Change the signal frame magic to 0x54504902 (TPI).
To: Catalin Marinas catalin.marinas@arm.com To: Will Deacon will@kernel.org To: Shuah Khan shuah@kernel.org Cc: Szabolcs Nagy szabolcs.nagy@arm.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kselftest@vger.kernel.org Signed-off-by: Mark Brown broonie@kernel.org
--- Mark Brown (4): arm64/sme: Document ABI for TPIDR2 signal information arm64/signal: Include TPIDR2 in the signal context kselftest/arm64: Add TPIDR2 to the set of known signal context records kselftest/arm64: Add test case for TPIDR2 signal frame records
Documentation/arm64/sme.rst | 3 + arch/arm64/include/uapi/asm/sigcontext.h | 8 ++ arch/arm64/kernel/signal.c | 59 ++++++++++++++ tools/testing/selftests/arm64/signal/.gitignore | 1 + .../selftests/arm64/signal/testcases/testcases.c | 4 + .../arm64/signal/testcases/tpidr2_siginfo.c | 90 ++++++++++++++++++++++ 6 files changed, 165 insertions(+) --- base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2 change-id: 20221208-arm64-tpidr2-sig-8fbb93725d8e
Best regards,
In order to allow access to TPIDR2 from signal handlers we need to add it to the signal context, document that we are doing so.
Signed-off-by: Mark Brown broonie@kernel.org --- Documentation/arm64/sme.rst | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/Documentation/arm64/sme.rst b/Documentation/arm64/sme.rst index 16d2db4c2e2e..36b08a105dd1 100644 --- a/Documentation/arm64/sme.rst +++ b/Documentation/arm64/sme.rst @@ -111,6 +111,9 @@ be zeroed.
* Signal handlers are invoked with streaming mode and ZA disabled.
+* A new signal frame record TPIDR2_MAGIC is added formatted as a struct + tpidr2_context to allow access to TPIDR2_EL0 from signal handlers. + * A new signal frame record za_context encodes the ZA register contents on signal delivery. [1]
Add a new signal frame record for TPIDR2 using the same format as we already use for ESR with different magic, a header with the value from the register appended as the only data. If SME is supported then this record is always included.
Signed-off-by: Mark Brown broonie@kernel.org Reviewed-by: Szabolcs Nagy szabolcs.nagy@arm.com --- arch/arm64/include/uapi/asm/sigcontext.h | 8 +++++ arch/arm64/kernel/signal.c | 59 ++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+)
diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h index 9525041e4a14..4b4398d7fb2d 100644 --- a/arch/arm64/include/uapi/asm/sigcontext.h +++ b/arch/arm64/include/uapi/asm/sigcontext.h @@ -144,6 +144,14 @@ struct sve_context {
#define SVE_SIG_FLAG_SM 0x1 /* Context describes streaming mode */
+/* TPIDR2_EL0 context */ +#define TPIDR2_MAGIC 0x54504902 + +struct tpidr2_context { + struct _aarch64_ctx head; + __u64 tpidr2; +}; + #define ZA_MAGIC 0x54366345
struct za_context { diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c index e0d09bf5b01b..5fe45c7c5e4f 100644 --- a/arch/arm64/kernel/signal.c +++ b/arch/arm64/kernel/signal.c @@ -56,6 +56,7 @@ struct rt_sigframe_user_layout { unsigned long fpsimd_offset; unsigned long esr_offset; unsigned long sve_offset; + unsigned long tpidr2_offset; unsigned long za_offset; unsigned long extra_offset; unsigned long end_offset; @@ -220,6 +221,7 @@ static int restore_fpsimd_context(struct fpsimd_context __user *ctx) struct user_ctxs { struct fpsimd_context __user *fpsimd; struct sve_context __user *sve; + struct tpidr2_context __user *tpidr2; struct za_context __user *za; };
@@ -361,6 +363,32 @@ extern int preserve_sve_context(void __user *ctx);
#ifdef CONFIG_ARM64_SME
+static int preserve_tpidr2_context(struct tpidr2_context __user *ctx) +{ + int err = 0; + + current->thread.tpidr2_el0 = read_sysreg_s(SYS_TPIDR2_EL0); + + __put_user_error(TPIDR2_MAGIC, &ctx->head.magic, err); + __put_user_error(sizeof(*ctx), &ctx->head.size, err); + __put_user_error(current->thread.tpidr2_el0, &ctx->tpidr2, err); + + return err; +} + +static int restore_tpidr2_context(struct user_ctxs *user) +{ + u64 tpidr2_el0; + int err = 0; + + /* Magic and size were validated deciding to call this function */ + __get_user_error(tpidr2_el0, &user->tpidr2->tpidr2, err); + if (!err) + current->thread.tpidr2_el0 = tpidr2_el0; + + return err; +} + static int preserve_za_context(struct za_context __user *ctx) { int err = 0; @@ -450,6 +478,8 @@ static int restore_za_context(struct user_ctxs *user) #else /* ! CONFIG_ARM64_SME */
/* Turn any non-optimised out attempts to use these into a link error: */ +extern int preserve_tpidr2_context(void __user *ctx); +extern int restore_tpidr2_context(struct user_ctxs *user); extern int preserve_za_context(void __user *ctx); extern int restore_za_context(struct user_ctxs *user);
@@ -468,6 +498,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
user->fpsimd = NULL; user->sve = NULL; + user->tpidr2 = NULL; user->za = NULL;
if (!IS_ALIGNED((unsigned long)base, 16)) @@ -534,6 +565,19 @@ static int parse_user_sigframe(struct user_ctxs *user, user->sve = (struct sve_context __user *)head; break;
+ case TPIDR2_MAGIC: + if (!system_supports_sme()) + goto invalid; + + if (user->tpidr2) + goto invalid; + + if (size != sizeof(*user->tpidr2)) + goto invalid; + + user->tpidr2 = (struct tpidr2_context __user *)head; + break; + case ZA_MAGIC: if (!system_supports_sme()) goto invalid; @@ -666,6 +710,9 @@ static int restore_sigframe(struct pt_regs *regs, err = restore_fpsimd_context(user.fpsimd); }
+ if (err == 0 && system_supports_sme() && user.tpidr2) + err = restore_tpidr2_context(&user); + if (err == 0 && system_supports_sme() && user.za) err = restore_za_context(&user);
@@ -760,6 +807,11 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user, else vl = task_get_sme_vl(current);
+ err = sigframe_alloc(user, &user->tpidr2_offset, + sizeof(struct tpidr2_context)); + if (err) + return err; + if (thread_za_enabled(¤t->thread)) vq = sve_vq_from_vl(vl);
@@ -817,6 +869,13 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user, err |= preserve_sve_context(sve_ctx); }
+ /* TPIDR2 if supported */ + if (system_supports_sme() && err == 0) { + struct tpidr2_context __user *tpidr2_ctx = + apply_user_offset(user, user->tpidr2_offset); + err |= preserve_tpidr2_context(tpidr2_ctx); + } + /* ZA state if present */ if (system_supports_sme() && err == 0 && user->za_offset) { struct za_context __user *za_ctx =
When validating the set of signal context records check that any TPIDR2 record has the correct size, also suppressing warnings due to seeing an unknown record type.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/signal/testcases/testcases.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.c b/tools/testing/selftests/arm64/signal/testcases/testcases.c index d2eda7b5de26..23487c458240 100644 --- a/tools/testing/selftests/arm64/signal/testcases/testcases.c +++ b/tools/testing/selftests/arm64/signal/testcases/testcases.c @@ -163,6 +163,10 @@ bool validate_reserved(ucontext_t *uc, size_t resv_sz, char **err) if (head->size != sizeof(struct esr_context)) *err = "Bad size for esr_context"; break; + case TPIDR2_MAGIC: + if (head->size != sizeof(struct tpidr2_context)) + *err = "Bad size for tpidr2_context"; + break; case SVE_MAGIC: if (flags & SVE_CTX) *err = "Multiple SVE_MAGIC";
Ensure that we get signal context for TPIDR2 if and only if SME is present on the system. Since TPIDR2 is owned by libc we merely validate that the value is whatever it was set to, this isn't ideal since it's likely to just be the default of 0 with current systems but it avoids future false positives.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/signal/.gitignore | 1 + .../arm64/signal/testcases/tpidr2_siginfo.c | 90 ++++++++++++++++++++++ 2 files changed, 91 insertions(+)
diff --git a/tools/testing/selftests/arm64/signal/.gitignore b/tools/testing/selftests/arm64/signal/.gitignore index e8d2b57f73ec..e1b6c4d961b5 100644 --- a/tools/testing/selftests/arm64/signal/.gitignore +++ b/tools/testing/selftests/arm64/signal/.gitignore @@ -4,5 +4,6 @@ fake_sigreturn_* sme_* ssve_* sve_* +tpidr2_siginfo za_* !*.[ch] diff --git a/tools/testing/selftests/arm64/signal/testcases/tpidr2_siginfo.c b/tools/testing/selftests/arm64/signal/testcases/tpidr2_siginfo.c new file mode 100644 index 000000000000..6a2c82bf7ead --- /dev/null +++ b/tools/testing/selftests/arm64/signal/testcases/tpidr2_siginfo.c @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2022 ARM Limited + * + * Verify that the TPIDR2 register context in signal frames is set up as + * expected. + */ + +#include <signal.h> +#include <ucontext.h> +#include <sys/auxv.h> +#include <sys/prctl.h> +#include <unistd.h> +#include <asm/sigcontext.h> + +#include "test_signals_utils.h" +#include "testcases.h" + +static union { + ucontext_t uc; + char buf[1024 * 128]; +} context; + +#define SYS_TPIDR2 "S3_3_C13_C0_5" + +static uint64_t get_tpidr2(void) +{ + uint64_t val; + + asm volatile ( + "mrs %0, " SYS_TPIDR2 "\n" + : "=r"(val) + : + : "cc"); + + return val; +} + +int tpidr2_present(struct tdescr *td, siginfo_t *si, ucontext_t *uc) +{ + struct _aarch64_ctx *head = GET_BUF_RESV_HEAD(context); + struct tpidr2_context *tpidr2_ctx; + size_t offset; + bool in_sigframe; + bool have_sme; + __u64 orig_tpidr2; + + have_sme = getauxval(AT_HWCAP2) & HWCAP2_SME; + if (have_sme) + orig_tpidr2 = get_tpidr2(); + + if (!get_current_context(td, &context.uc, sizeof(context))) + return 1; + + tpidr2_ctx = (struct tpidr2_context *) + get_header(head, TPIDR2_MAGIC, td->live_sz, &offset); + + in_sigframe = tpidr2_ctx != NULL; + + fprintf(stderr, "TPIDR2 sigframe %s on system %s SME\n", + in_sigframe ? "present" : "absent", + have_sme ? "with" : "without"); + + td->pass = (in_sigframe == have_sme); + + /* + * Check that the value we read back was the one present at + * the time that the signal was triggered. TPIDR2 is owned by + * libc so we can't safely choose the value and it is possible + * that we may need to revisit this in future if something + * starts deciding to set a new TPIDR2 between us reading and + * the signal. + */ + if (have_sme && tpidr2_ctx) { + if (tpidr2_ctx->tpidr2 != orig_tpidr2) { + fprintf(stderr, "TPIDR2 in frame is %llx, was %llx\n", + tpidr2_ctx->tpidr2, orig_tpidr2); + td->pass = false; + } + } + + return 0; +} + +struct tdescr tde = { + .name = "TPIDR2", + .descr = "Validate that TPIDR2 is present as expected", + .timeout = 3, + .run = tpidr2_present, +};
On Tue, 27 Dec 2022 14:20:40 +0000, Mark Brown wrote:
When SME support was merged support for TPIDR2 in signal frames was omitted, meaning that it was not possible for signal handers to inspect or modify it. This will present an issue for programs using signals to implement lightweight threads so let's provide access to TPIDR2 in signal handlers.
Implement a new record type for TPIDR2 using the same format as we use for ESR and add coverage to make sure that this appears in the signal context as expected. Due to TPIDR2 being reserved for libc we only validate that the value is unchanged, meaning we're likely to just be validating the default value of 0 on current systems. I have tested with a modified version that sets an explicit value.
[...]
Applied to arm64 (for-next/tpidr2), thanks!
[1/4] arm64/sme: Document ABI for TPIDR2 signal information https://git.kernel.org/arm64/c/17d0c4a27b2a [2/4] arm64/signal: Include TPIDR2 in the signal context https://git.kernel.org/arm64/c/39e54499280f [3/4] kselftest/arm64: Add TPIDR2 to the set of known signal context records https://git.kernel.org/arm64/c/bae393dabf35 [4/4] kselftest/arm64: Add test case for TPIDR2 signal frame records https://git.kernel.org/arm64/c/8ced92801935
I'll look at the signal handling clean-up patches as well but they probably conflicts with this series and may need to be rebased on top.
linux-kselftest-mirror@lists.linaro.org