public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress
@ 2024-10-23 20:38 Mark Brown
  2024-10-23 20:38 ` [PATCH 1/6] kselftest/arm64: Correct misleading comments on fp-stress irritators Mark Brown
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Mark Brown @ 2024-10-23 20:38 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Mark Rutland, linux-arm-kernel, linux-kselftest, linux-kernel,
	Mark Brown

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>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/6] kselftest/arm64: Correct misleading comments on fp-stress irritators
  2024-10-23 20:38 [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress Mark Brown
@ 2024-10-23 20:38 ` Mark Brown
  2024-11-06 11:29   ` Mark Rutland
  2024-10-23 20:38 ` [PATCH 2/6] kselftest/arm64: Remove unused ADRs from irritator handlers Mark Brown
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-23 20:38 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Mark Rutland, linux-arm-kernel, linux-kselftest, linux-kernel,
	Mark Brown

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]

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/6] kselftest/arm64: Remove unused ADRs from irritator handlers
  2024-10-23 20:38 [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress Mark Brown
  2024-10-23 20:38 ` [PATCH 1/6] kselftest/arm64: Correct misleading comments on fp-stress irritators Mark Brown
@ 2024-10-23 20:38 ` Mark Brown
  2024-11-06 11:29   ` Mark Rutland
  2024-10-23 20:38 ` [PATCH 3/6] kselftest/arm64: Corrupt P15 in the irritator when testing SSVE Mark Brown
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-23 20:38 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Mark Rutland, linux-arm-kernel, linux-kselftest, linux-kernel,
	Mark Brown

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

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/6] kselftest/arm64: Corrupt P15 in the irritator when testing SSVE
  2024-10-23 20:38 [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress Mark Brown
  2024-10-23 20:38 ` [PATCH 1/6] kselftest/arm64: Correct misleading comments on fp-stress irritators Mark Brown
  2024-10-23 20:38 ` [PATCH 2/6] kselftest/arm64: Remove unused ADRs from irritator handlers Mark Brown
@ 2024-10-23 20:38 ` Mark Brown
  2024-11-06 11:27   ` Mark Rutland
  2024-10-23 20:38 ` [PATCH 4/6] kselftest/arm64: Implement irritators for ZA and ZT Mark Brown
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-23 20:38 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Mark Rutland, linux-arm-kernel, linux-kselftest, linux-kernel,
	Mark Brown

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

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/6] kselftest/arm64: Implement irritators for ZA and ZT
  2024-10-23 20:38 [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress Mark Brown
                   ` (2 preceding siblings ...)
  2024-10-23 20:38 ` [PATCH 3/6] kselftest/arm64: Corrupt P15 in the irritator when testing SSVE Mark Brown
@ 2024-10-23 20:38 ` Mark Brown
  2024-11-06 11:31   ` Mark Rutland
  2024-10-23 20:38 ` [PATCH 5/6] kselftest/arm64: Provide a SIGUSR1 handler in the kernel mode FP stress test Mark Brown
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-23 20:38 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Mark Rutland, linux-arm-kernel, linux-kselftest, linux-kernel,
	Mark Brown

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

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/6] kselftest/arm64: Provide a SIGUSR1 handler in the kernel mode FP stress test
  2024-10-23 20:38 [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress Mark Brown
                   ` (3 preceding siblings ...)
  2024-10-23 20:38 ` [PATCH 4/6] kselftest/arm64: Implement irritators for ZA and ZT Mark Brown
@ 2024-10-23 20:38 ` Mark Brown
  2024-11-06 11:32   ` Mark Rutland
  2024-10-23 20:38 ` [PATCH 6/6] kselftest/arm64: Test signal handler state modification in fp-stress Mark Brown
  2024-10-28 14:26 ` [PATCH 0/6] kselftest/arm64: Test floating point signal context restore " Mark Rutland
  6 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-23 20:38 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Mark Rutland, linux-arm-kernel, linux-kselftest, linux-kernel,
	Mark Brown

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",

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/6] kselftest/arm64: Test signal handler state modification in fp-stress
  2024-10-23 20:38 [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress Mark Brown
                   ` (4 preceding siblings ...)
  2024-10-23 20:38 ` [PATCH 5/6] kselftest/arm64: Provide a SIGUSR1 handler in the kernel mode FP stress test Mark Brown
@ 2024-10-23 20:38 ` Mark Brown
  2024-11-06 11:33   ` Mark Rutland
  2024-10-28 14:26 ` [PATCH 0/6] kselftest/arm64: Test floating point signal context restore " Mark Rutland
  6 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-23 20:38 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Mark Rutland, linux-arm-kernel, linux-kselftest, linux-kernel,
	Mark Brown

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)

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress
  2024-10-23 20:38 [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress Mark Brown
                   ` (5 preceding siblings ...)
  2024-10-23 20:38 ` [PATCH 6/6] kselftest/arm64: Test signal handler state modification in fp-stress Mark Brown
@ 2024-10-28 14:26 ` Mark Rutland
  2024-10-28 15:38   ` Mark Brown
  6 siblings, 1 reply; 16+ messages in thread
From: Mark Rutland @ 2024-10-28 14:26 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Shuah Khan, linux-arm-kernel,
	linux-kselftest, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress
  2024-10-28 14:26 ` [PATCH 0/6] kselftest/arm64: Test floating point signal context restore " Mark Rutland
@ 2024-10-28 15:38   ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2024-10-28 15:38 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Catalin Marinas, Will Deacon, Shuah Khan, linux-arm-kernel,
	linux-kselftest, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1616 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/6] kselftest/arm64: Corrupt P15 in the irritator when testing SSVE
  2024-10-23 20:38 ` [PATCH 3/6] kselftest/arm64: Corrupt P15 in the irritator when testing SSVE Mark Brown
@ 2024-11-06 11:27   ` Mark Rutland
  2024-11-06 12:46     ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Rutland @ 2024-11-06 11:27 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Shuah Khan, linux-arm-kernel,
	linux-kselftest, linux-kernel

On Wed, Oct 23, 2024 at 09:38:31PM +0100, Mark Brown wrote:
> 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.

I think you mean P0 rather than P15 here? 

	rdffr   p0.b

... reads from the FFR and writes to P0, modifying P0.

	wrffr   p15.b

... reads from P15 and writes to the FRR, leaving P15 unchanged.

> 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

Both RDFFR and WRFFR are illegal in streaming mode unless FEAT_FA64 is
implemented and enabled, so we cannot use DRFFR in the SSVE case.

Is there a different instruction we can use?

Mark.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/6] kselftest/arm64: Correct misleading comments on fp-stress irritators
  2024-10-23 20:38 ` [PATCH 1/6] kselftest/arm64: Correct misleading comments on fp-stress irritators Mark Brown
@ 2024-11-06 11:29   ` Mark Rutland
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2024-11-06 11:29 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Shuah Khan, linux-arm-kernel,
	linux-kselftest, linux-kernel

On Wed, Oct 23, 2024 at 09:38:29PM +0100, Mark Brown wrote:
> 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>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/6] kselftest/arm64: Remove unused ADRs from irritator handlers
  2024-10-23 20:38 ` [PATCH 2/6] kselftest/arm64: Remove unused ADRs from irritator handlers Mark Brown
@ 2024-11-06 11:29   ` Mark Rutland
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2024-11-06 11:29 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Shuah Khan, linux-arm-kernel,
	linux-kselftest, linux-kernel

On Wed, Oct 23, 2024 at 09:38:30PM +0100, Mark Brown wrote:
> 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>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] kselftest/arm64: Implement irritators for ZA and ZT
  2024-10-23 20:38 ` [PATCH 4/6] kselftest/arm64: Implement irritators for ZA and ZT Mark Brown
@ 2024-11-06 11:31   ` Mark Rutland
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2024-11-06 11:31 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Shuah Khan, linux-arm-kernel,
	linux-kselftest, linux-kernel

On Wed, Oct 23, 2024 at 09:38:32PM +0100, Mark Brown wrote:
> 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>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/6] kselftest/arm64: Provide a SIGUSR1 handler in the kernel mode FP stress test
  2024-10-23 20:38 ` [PATCH 5/6] kselftest/arm64: Provide a SIGUSR1 handler in the kernel mode FP stress test Mark Brown
@ 2024-11-06 11:32   ` Mark Rutland
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2024-11-06 11:32 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Shuah Khan, linux-arm-kernel,
	linux-kselftest, linux-kernel

On Wed, Oct 23, 2024 at 09:38:33PM +0100, Mark Brown wrote:
> 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>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  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",
> 
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] kselftest/arm64: Test signal handler state modification in fp-stress
  2024-10-23 20:38 ` [PATCH 6/6] kselftest/arm64: Test signal handler state modification in fp-stress Mark Brown
@ 2024-11-06 11:33   ` Mark Rutland
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2024-11-06 11:33 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Shuah Khan, linux-arm-kernel,
	linux-kselftest, linux-kernel

On Wed, Oct 23, 2024 at 09:38:34PM +0100, Mark Brown wrote:
> 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>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/6] kselftest/arm64: Corrupt P15 in the irritator when testing SSVE
  2024-11-06 11:27   ` Mark Rutland
@ 2024-11-06 12:46     ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2024-11-06 12:46 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Catalin Marinas, Will Deacon, Shuah Khan, linux-arm-kernel,
	linux-kselftest, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Wed, Nov 06, 2024 at 11:27:24AM +0000, Mark Rutland wrote:

> > +#ifndef SSVE
> >  	// And FFR
> >  	wrffr	p15.b
> >  #endif

> Both RDFFR and WRFFR are illegal in streaming mode unless FEAT_FA64 is
> implemented and enabled, so we cannot use DRFFR in the SSVE case.

Indeed, I'm surprised that managed to pass my testing...

> Is there a different instruction we can use?

We could just trash p0 with something like a load or add.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-11-06 12:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-23 20:38 [PATCH 0/6] kselftest/arm64: Test floating point signal context restore in fp-stress Mark Brown
2024-10-23 20:38 ` [PATCH 1/6] kselftest/arm64: Correct misleading comments on fp-stress irritators Mark Brown
2024-11-06 11:29   ` Mark Rutland
2024-10-23 20:38 ` [PATCH 2/6] kselftest/arm64: Remove unused ADRs from irritator handlers Mark Brown
2024-11-06 11:29   ` Mark Rutland
2024-10-23 20:38 ` [PATCH 3/6] kselftest/arm64: Corrupt P15 in the irritator when testing SSVE Mark Brown
2024-11-06 11:27   ` Mark Rutland
2024-11-06 12:46     ` Mark Brown
2024-10-23 20:38 ` [PATCH 4/6] kselftest/arm64: Implement irritators for ZA and ZT Mark Brown
2024-11-06 11:31   ` Mark Rutland
2024-10-23 20:38 ` [PATCH 5/6] kselftest/arm64: Provide a SIGUSR1 handler in the kernel mode FP stress test Mark Brown
2024-11-06 11:32   ` Mark Rutland
2024-10-23 20:38 ` [PATCH 6/6] kselftest/arm64: Test signal handler state modification in fp-stress Mark Brown
2024-11-06 11:33   ` Mark Rutland
2024-10-28 14:26 ` [PATCH 0/6] kselftest/arm64: Test floating point signal context restore " Mark Rutland
2024-10-28 15:38   ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox