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,
-- 
Mark Brown <broonie@kernel.org>

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:

> 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?

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.

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