Currently we test signal delivery to the programs run by fp-stress but our signal handlers simply count the number of signals seen and don't do anything with the floating point state. The original fpsimd-test and sve-test programs had signal handlers called irritators which modify the live register state, verifying that we restore the signal context on return, but a combination of misleading comments and code resulted in them never being used and the equivalent handlers in the other tests being stubbed or omitted.
Clarify the code, implement effective irritator handlers for the test programs that can have them and then switch the signals generated by the fp-stress program over to use the irritators, ensuring that we validate that we restore the saved signal context properly.
Signed-off-by: Mark Brown broonie@kernel.org --- Mark Brown (6): kselftest/arm64: Correct misleading comments on fp-stress irritators kselftest/arm64: Remove unused ADRs from irritator handlers kselftest/arm64: Corrupt P15 in the irritator when testing SSVE kselftest/arm64: Implement irritators for ZA and ZT kselftest/arm64: Provide a SIGUSR1 handler in the kernel mode FP stress test kselftest/arm64: Test signal handler state modification in fp-stress
tools/testing/selftests/arm64/fp/fp-stress.c | 2 +- tools/testing/selftests/arm64/fp/fpsimd-test.S | 4 +--- tools/testing/selftests/arm64/fp/kernel-test.c | 4 ++++ tools/testing/selftests/arm64/fp/sve-test.S | 6 ++---- tools/testing/selftests/arm64/fp/za-test.S | 13 ++++--------- tools/testing/selftests/arm64/fp/zt-test.S | 13 ++++--------- 6 files changed, 16 insertions(+), 26 deletions(-) --- base-commit: 8e929cb546ee42c9a61d24fae60605e9e3192354 change-id: 20241023-arm64-fp-stress-irritator-5415fe92adef
Best regards,
The comments in the handlers for the irritator signal in the test threads for fp-stress suggest that the irritator will corrupt the register state observed by the main thread but this is not the case, instead the FPSIMD and SVE irritators (which are the only ones that are implemented) modify the current register state which is expected to be overwritten on return from the handler by the saved register state. Update the comment to reflect what the handler is actually doing.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/fp/fpsimd-test.S | 3 +-- tools/testing/selftests/arm64/fp/sve-test.S | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/arm64/fp/fpsimd-test.S b/tools/testing/selftests/arm64/fp/fpsimd-test.S index 8b960d01ed2e0ef516893b68794078ddf8c01e1f..bdfb7cf2e4ec175fda62c1c2f38c6ebb1a1c48bf 100644 --- a/tools/testing/selftests/arm64/fp/fpsimd-test.S +++ b/tools/testing/selftests/arm64/fp/fpsimd-test.S @@ -134,8 +134,7 @@ function check_vreg b memcmp endfunction
-// Any SVE register modified here can cause corruption in the main -// thread -- but *only* the registers modified here. +// Modify live register state, the signal return will undo our changes function irritator_handler // Increment the irritation signal count (x23): ldr x0, [x2, #ucontext_regs + 8 * 23] diff --git a/tools/testing/selftests/arm64/fp/sve-test.S b/tools/testing/selftests/arm64/fp/sve-test.S index fff60e2a25addfd4850ef71aa3cf6535ac880ffd..e3c0d585684df29723a49265f3df6d23817498c7 100644 --- a/tools/testing/selftests/arm64/fp/sve-test.S +++ b/tools/testing/selftests/arm64/fp/sve-test.S @@ -291,8 +291,7 @@ function check_ffr #endif endfunction
-// Any SVE register modified here can cause corruption in the main -// thread -- but *only* the registers modified here. +// Modify live register state, the signal return will undo our changes function irritator_handler // Increment the irritation signal count (x23): ldr x0, [x2, #ucontext_regs + 8 * 23]
The irritator handlers for the fp-stress test programs all use ADR to load an address into x0 which is then not referenced. Remove these ADRs as they just cause confusion.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/fp/fpsimd-test.S | 1 - tools/testing/selftests/arm64/fp/sve-test.S | 1 - tools/testing/selftests/arm64/fp/za-test.S | 1 - tools/testing/selftests/arm64/fp/zt-test.S | 1 - 4 files changed, 4 deletions(-)
diff --git a/tools/testing/selftests/arm64/fp/fpsimd-test.S b/tools/testing/selftests/arm64/fp/fpsimd-test.S index bdfb7cf2e4ec175fda62c1c2f38c6ebb1a1c48bf..9977ffdd758a51a7af67cd607d019a6c54d3a6c6 100644 --- a/tools/testing/selftests/arm64/fp/fpsimd-test.S +++ b/tools/testing/selftests/arm64/fp/fpsimd-test.S @@ -142,7 +142,6 @@ function irritator_handler str x0, [x2, #ucontext_regs + 8 * 23]
// Corrupt some random V-regs - adr x0, .text + (irritator_handler - .text) / 16 * 16 movi v0.8b, #7 movi v9.16b, #9 movi v31.8b, #31 diff --git a/tools/testing/selftests/arm64/fp/sve-test.S b/tools/testing/selftests/arm64/fp/sve-test.S index e3c0d585684df29723a49265f3df6d23817498c7..f1fb9745c681786f686f1fafcb7e1154f3c8e1a3 100644 --- a/tools/testing/selftests/arm64/fp/sve-test.S +++ b/tools/testing/selftests/arm64/fp/sve-test.S @@ -299,7 +299,6 @@ function irritator_handler str x0, [x2, #ucontext_regs + 8 * 23]
// Corrupt some random Z-regs - adr x0, .text + (irritator_handler - .text) / 16 * 16 movi v0.8b, #1 movi v9.16b, #2 movi v31.8b, #3 diff --git a/tools/testing/selftests/arm64/fp/za-test.S b/tools/testing/selftests/arm64/fp/za-test.S index 095b45531640966e685408057c08ada67e68998b..1ee0ec36766d2bef92aff50a002813e76e22963c 100644 --- a/tools/testing/selftests/arm64/fp/za-test.S +++ b/tools/testing/selftests/arm64/fp/za-test.S @@ -158,7 +158,6 @@ function irritator_handler
// Corrupt some random ZA data #if 0 - adr x0, .text + (irritator_handler - .text) / 16 * 16 movi v0.8b, #1 movi v9.16b, #2 movi v31.8b, #3 diff --git a/tools/testing/selftests/arm64/fp/zt-test.S b/tools/testing/selftests/arm64/fp/zt-test.S index b5c81e81a37946c1bffe810568855939e9ceb08e..ade9c98abcdafc2755ef4796670566d99e919e5c 100644 --- a/tools/testing/selftests/arm64/fp/zt-test.S +++ b/tools/testing/selftests/arm64/fp/zt-test.S @@ -127,7 +127,6 @@ function irritator_handler
// Corrupt some random ZT data #if 0 - adr x0, .text + (irritator_handler - .text) / 16 * 16 movi v0.8b, #1 movi v9.16b, #2 movi v31.8b, #3
When building for streaming SVE the irritator for SVE skips updates of both P15 and FFR. While FFR is skipped since it might not be present there is no reason to skip corrupting P15 so move the ifdef appropriately.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/fp/sve-test.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/fp/sve-test.S b/tools/testing/selftests/arm64/fp/sve-test.S index f1fb9745c681786f686f1fafcb7e1154f3c8e1a3..3c88dfe9c8cad29f44217314aeaffa984bac05e5 100644 --- a/tools/testing/selftests/arm64/fp/sve-test.S +++ b/tools/testing/selftests/arm64/fp/sve-test.S @@ -302,9 +302,9 @@ function irritator_handler movi v0.8b, #1 movi v9.16b, #2 movi v31.8b, #3 -#ifndef SSVE // And P0 rdffr p0.b +#ifndef SSVE // And FFR wrffr p15.b #endif
Currently we don't use the irritator signal in our floating point stress tests so when we added ZA and ZT stress tests we didn't actually bother implementing any actual action in the handlers, we just counted the signal deliveries. In preparation for using the irritators let's implement them, just trivially SMSTOP and SMSTART to reset all bits in the register to 0.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/fp/za-test.S | 12 ++++-------- tools/testing/selftests/arm64/fp/zt-test.S | 12 ++++-------- 2 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/arm64/fp/za-test.S b/tools/testing/selftests/arm64/fp/za-test.S index 1ee0ec36766d2bef92aff50a002813e76e22963c..f902e6ef9077bfa34fa7f85ce572ce3df4346789 100644 --- a/tools/testing/selftests/arm64/fp/za-test.S +++ b/tools/testing/selftests/arm64/fp/za-test.S @@ -148,20 +148,16 @@ function check_za b memcmp endfunction
-// Any SME register modified here can cause corruption in the main -// thread -- but *only* the locations modified here. +// Modify the live SME register state, signal return will undo our changes function irritator_handler // Increment the irritation signal count (x23): ldr x0, [x2, #ucontext_regs + 8 * 23] add x0, x0, #1 str x0, [x2, #ucontext_regs + 8 * 23]
- // Corrupt some random ZA data -#if 0 - movi v0.8b, #1 - movi v9.16b, #2 - movi v31.8b, #3 -#endif + // This will reset ZA to all bits 0 + smstop + smstart
ret endfunction diff --git a/tools/testing/selftests/arm64/fp/zt-test.S b/tools/testing/selftests/arm64/fp/zt-test.S index ade9c98abcdafc2755ef4796670566d99e919e5c..c96cb7c2ad4b406c54099fe3f73917259bd947e4 100644 --- a/tools/testing/selftests/arm64/fp/zt-test.S +++ b/tools/testing/selftests/arm64/fp/zt-test.S @@ -117,20 +117,16 @@ function check_zt b memcmp endfunction
-// Any SME register modified here can cause corruption in the main -// thread -- but *only* the locations modified here. +// Modify the live SME register state, signal return will undo our changes function irritator_handler // Increment the irritation signal count (x23): ldr x0, [x2, #ucontext_regs + 8 * 23] add x0, x0, #1 str x0, [x2, #ucontext_regs + 8 * 23]
- // Corrupt some random ZT data -#if 0 - movi v0.8b, #1 - movi v9.16b, #2 - movi v31.8b, #3 -#endif + // This will reset ZT to all bits 0 + smstop + smstart
ret endfunction
The other stress test programs provide a SIGUSR1 handler which modifies the live register state in order to validate that signal context is being restored during signal return. While we can't usefully do this when testing kernel mode FP usage provide a handler for SIGUSR1 which just counts the number of signals like we do for SIGUSR2, allowing fp-stress to treat all the test programs uniformly.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/fp/kernel-test.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/arm64/fp/kernel-test.c b/tools/testing/selftests/arm64/fp/kernel-test.c index e8da3b4cbd23202c6504ffd8043f8ef351d739f6..859345379044fc287458644309d66cf5f3d8bdf5 100644 --- a/tools/testing/selftests/arm64/fp/kernel-test.c +++ b/tools/testing/selftests/arm64/fp/kernel-test.c @@ -267,6 +267,10 @@ int main(void) strerror(errno), errno);
sa.sa_sigaction = handle_kick_signal; + ret = sigaction(SIGUSR1, &sa, NULL); + if (ret < 0) + printf("Failed to install SIGUSR1 handler: %s (%d)\n", + strerror(errno), errno); ret = sigaction(SIGUSR2, &sa, NULL); if (ret < 0) printf("Failed to install SIGUSR2 handler: %s (%d)\n",
Currently in fp-stress we test signal delivery to the test threads by sending SIGUSR2 which simply counts how many signals are delivered. The test programs now also all have a SIGUSR1 handler which for the threads doing userspace testing additionally modifies the floating point register state in the signal handler, verifying that when we return the saved register state is restored from the signal context as expected. Switch over to triggering that to validate that we are restoring as expected.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/fp/fp-stress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/fp/fp-stress.c b/tools/testing/selftests/arm64/fp/fp-stress.c index faac24bdefeb9436e2daf20b7250d0ae25ca23a7..3d477249dee0632b662b48582433d39323d18e18 100644 --- a/tools/testing/selftests/arm64/fp/fp-stress.c +++ b/tools/testing/selftests/arm64/fp/fp-stress.c @@ -221,7 +221,7 @@ static void child_output(struct child_data *child, uint32_t events, static void child_tickle(struct child_data *child) { if (child->output_seen && !child->exited) - kill(child->pid, SIGUSR2); + kill(child->pid, SIGUSR1); }
static void child_stop(struct child_data *child)
On Wed, Oct 23, 2024 at 09:38:28PM +0100, Mark Brown wrote:
Currently we test signal delivery to the programs run by fp-stress but our signal handlers simply count the number of signals seen and don't do anything with the floating point state. The original fpsimd-test and sve-test programs had signal handlers called irritators which modify the live register state, verifying that we restore the signal context on return, but a combination of misleading comments and code resulted in them never being used and the equivalent handlers in the other tests being stubbed or omitted.
Clarify the code, implement effective irritator handlers for the test programs that can have them and then switch the signals generated by the fp-stress program over to use the irritators, ensuring that we validate that we restore the saved signal context properly.
Superficially these look good, but two thing stand out:
1) We only singal the tasks once a second. Dave's original shell test script hammered this constantly, and it makes a substantial impact actually triggering a bug.
Without these patches, I hacked the fp-stress.c main loop to trigger a signal every ~1ms (by reducing the epoll_wait() timeout to 1 and scaling the overall timeout to 10000 accordingly), and those changes make the tests reliably trigger the "Bad SVCR" splats within a few seconds after a few hundred signals, even if only using the SIGUSR2 tickle handlers.
Can we change the fp-stress.c main loop to signal threads more often?
I had some minor changes to only log every ~1000 iterations or so, to avoid spamming the log.
2) The SIGUSR2 tickle handlers are left behind.
Given they're unused, it'd be nice to clean them up.
Mark.
Signed-off-by: Mark Brown broonie@kernel.org
Mark Brown (6): kselftest/arm64: Correct misleading comments on fp-stress irritators kselftest/arm64: Remove unused ADRs from irritator handlers kselftest/arm64: Corrupt P15 in the irritator when testing SSVE kselftest/arm64: Implement irritators for ZA and ZT kselftest/arm64: Provide a SIGUSR1 handler in the kernel mode FP stress test kselftest/arm64: Test signal handler state modification in fp-stress
tools/testing/selftests/arm64/fp/fp-stress.c | 2 +- tools/testing/selftests/arm64/fp/fpsimd-test.S | 4 +--- tools/testing/selftests/arm64/fp/kernel-test.c | 4 ++++ tools/testing/selftests/arm64/fp/sve-test.S | 6 ++---- tools/testing/selftests/arm64/fp/za-test.S | 13 ++++--------- tools/testing/selftests/arm64/fp/zt-test.S | 13 ++++--------- 6 files changed, 16 insertions(+), 26 deletions(-)
base-commit: 8e929cb546ee42c9a61d24fae60605e9e3192354 change-id: 20241023-arm64-fp-stress-irritator-5415fe92adef
Best regards,
Mark Brown broonie@kernel.org
On Mon, Oct 28, 2024 at 02:26:44PM +0000, Mark Rutland wrote:
- We only singal the tasks once a second. Dave's original shell test script hammered this constantly, and it makes a substantial impact actually triggering a bug.
Without these patches, I hacked the fp-stress.c main loop to trigger a signal every ~1ms (by reducing the epoll_wait() timeout to 1 and scaling the overall timeout to 10000 accordingly), and those changes make the tests reliably trigger the "Bad SVCR" splats within a few seconds after a few hundred signals, even if only using the SIGUSR2 tickle handlers.
Can we change the fp-stress.c main loop to signal threads more often?
Yeah, the once a second number was kind of pulled out of thin air (IIRC I originally picked that for UI purposes and then added the signalling later without specific purpose, I wasn't particularly referencing the shell scripts here since I never used them much). I don't see any reason not to raise the rate, we do need it to be low enough to allow the main loops of the test programs to make reasonable progress so miliseconds feels like it might be a bit aggressive for a fully loaded FVP configuration. That'd be a separate patch anyway.
- The SIGUSR2 tickle handlers are left behind.
Given they're unused, it'd be nice to clean them up.
I don't see an urgent need to remove them, like the SIGUSR1 handlers previously they're not doing any harm sitting there and could come in handy when debugging things - the programs get a reasonable amount of use standalone.
linux-kselftest-mirror@lists.linaro.org
-
Mark Brown -
Mark Rutland