[PATCH 1/5] selftests/powerpc: Fix error handling in FPU/VMX preemption tests

[PATCH 1/5] selftests/powerpc: Fix error handling in FPU/VMX preemption tests

[Michael Ellerman]

The FPU & VMX preemption tests do not check for errors returned by the
low-level asm routines, preempt_fpu() / preempt_vsx() respectively.
That means any register corruption detected by the asm routines does not
result in a test failure.

Fix it by returning the return value of the asm routines from the
pthread child routines.

Fixes: e5ab8be68e44 ("selftests/powerpc: Test preservation of FPU and VMX regs across preemption")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/powerpc/math/fpu_preempt.c |  9 +++++----
 tools/testing/selftests/powerpc/math/vmx_preempt.c | 10 ++++++----
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/powerpc/math/fpu_preempt.c b/tools/testing/selftests/powerpc/math/fpu_preempt.c
index 5235bdc8c0b1..3e5b5663d244 100644
--- a/tools/testing/selftests/powerpc/math/fpu_preempt.c
+++ b/tools/testing/selftests/powerpc/math/fpu_preempt.c
@@ -37,19 +37,20 @@ __thread double darray[] = {0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
 int threads_starting;
 int running;
 
-extern void preempt_fpu(double *darray, int *threads_starting, int *running);
+extern int preempt_fpu(double *darray, int *threads_starting, int *running);
 
 void *preempt_fpu_c(void *p)
 {
+	long rc;
 	int i;
+
 	srand(pthread_self());
 	for (i = 0; i < 21; i++)
 		darray[i] = rand();
 
-	/* Test failed if it ever returns */
-	preempt_fpu(darray, &threads_starting, &running);
+	rc = preempt_fpu(darray, &threads_starting, &running);
 
-	return p;
+	return (void *)rc;
 }
 
 int test_preempt_fpu(void)
diff --git a/tools/testing/selftests/powerpc/math/vmx_preempt.c b/tools/testing/selftests/powerpc/math/vmx_preempt.c
index 6761d6ce30ec..6f7cf400c687 100644
--- a/tools/testing/selftests/powerpc/math/vmx_preempt.c
+++ b/tools/testing/selftests/powerpc/math/vmx_preempt.c
@@ -37,19 +37,21 @@ __thread vector int varray[] = {{1, 2, 3, 4}, {5, 6, 7, 8}, {9, 10,11,12},
 int threads_starting;
 int running;
 
-extern void preempt_vmx(vector int *varray, int *threads_starting, int *running);
+extern int preempt_vmx(vector int *varray, int *threads_starting, int *running);
 
 void *preempt_vmx_c(void *p)
 {
 	int i, j;
+	long rc;
+
 	srand(pthread_self());
 	for (i = 0; i < 12; i++)
 		for (j = 0; j < 4; j++)
 			varray[i][j] = rand();
 
-	/* Test fails if it ever returns */
-	preempt_vmx(varray, &threads_starting, &running);
-	return p;
+	rc = preempt_vmx(varray, &threads_starting, &running);
+
+	return (void *)rc;
 }
 
 int test_preempt_vmx(void)
-- 
2.41.0

[PATCH 2/5] selftests/powerpc: Check all FPRs in fpu_preempt

[Michael Ellerman]

There's a selftest that checks FPRs aren't corrupted by preemption, or
just process scheduling. However it only checks the non-volatile FPRs,
meaning corruption of the volatile FPRs could go undetected.

The check_fpu function it calls is used by several other tests, so for
now add a new routine to check all the FPRs. Increase the size of the
array of FPRs to 32, and initialise them all with random values.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 .../testing/selftests/powerpc/math/fpu_asm.S  | 41 +++++++++++++++++--
 .../selftests/powerpc/math/fpu_preempt.c      | 15 +++----
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/powerpc/math/fpu_asm.S b/tools/testing/selftests/powerpc/math/fpu_asm.S
index 9dc0c158f871..051392ad3ce7 100644
--- a/tools/testing/selftests/powerpc/math/fpu_asm.S
+++ b/tools/testing/selftests/powerpc/math/fpu_asm.S
@@ -66,6 +66,40 @@ FUNC_START(check_fpu)
 	li	r3,0 # Success!!!
 1:	blr
 
+
+// int check_all_fprs(double darray[32])
+FUNC_START(check_all_fprs)
+	PUSH_BASIC_STACK(8)
+	mr	r4, r3	// r4 = darray
+	li	r3, 1	// prepare for failure
+
+	stfd	f31, STACK_FRAME_LOCAL(0, 0)(sp) // backup f31
+
+	// Check regs f0-f30, using f31 as scratch
+	.set i, 0
+	.rept 31
+	lfd	f31, (8 * i)(r4)	// load expected value
+	fcmpu	cr0, i, f31		// compare
+	bne	cr0, 1f			// bail if mismatch
+	.set i, i + 1
+	.endr
+
+	lfd	f31, STACK_FRAME_LOCAL(0, 0)(sp) // reload f31
+	stfd	f30, STACK_FRAME_LOCAL(0, 0)(sp) // backup f30
+
+	lfd	f30, (8 * 31)(r4)	// load expected value of f31
+	fcmpu	cr0, f30, f31		// compare
+	bne	cr0, 1f			// bail if mismatch
+
+	lfd	f30, STACK_FRAME_LOCAL(0, 0)(sp) // reload f30
+
+	// Success
+	li	r3, 0
+
+1:	POP_BASIC_STACK(8)
+	blr
+FUNC_END(check_all_fprs)
+
 FUNC_START(test_fpu)
 	# r3 holds pointer to where to put the result of fork
 	# r4 holds pointer to the pid
@@ -104,8 +138,8 @@ FUNC_START(preempt_fpu)
 	std r4,STACK_FRAME_PARAM(1)(sp) # int *threads_starting
 	std r5,STACK_FRAME_PARAM(2)(sp) # int *running
 
-	bl load_fpu
-	nop
+	// Load FPRs with expected values
+	OP_REGS lfd, 8, 0, 31, r3
 
 	sync
 	# Atomic DEC
@@ -116,8 +150,7 @@ FUNC_START(preempt_fpu)
 	bne- 1b
 
 2:	ld r3,STACK_FRAME_PARAM(0)(sp)
-	bl check_fpu
-	nop
+	bl check_all_fprs
 	cmpdi r3,0
 	bne 3f
 	ld r4,STACK_FRAME_PARAM(2)(sp)
diff --git a/tools/testing/selftests/powerpc/math/fpu_preempt.c b/tools/testing/selftests/powerpc/math/fpu_preempt.c
index 3e5b5663d244..24b5abacccdc 100644
--- a/tools/testing/selftests/powerpc/math/fpu_preempt.c
+++ b/tools/testing/selftests/powerpc/math/fpu_preempt.c
@@ -1,13 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright 2015, Cyril Bur, IBM Corp.
+ * Copyright 2023, Michael Ellerman, IBM Corp.
  *
  * This test attempts to see if the FPU registers change across preemption.
- * Two things should be noted here a) The check_fpu function in asm only checks
- * the non volatile registers as it is reused from the syscall test b) There is
- * no way to be sure preemption happened so this test just uses many threads
- * and a long wait. As such, a successful test doesn't mean much but a failure
- * is bad.
+ * There is no way to be sure preemption happened so this test just uses many
+ * threads and a long wait. As such, a successful test doesn't mean much but
+ * a failure is bad.
  */
 
 #include <stdio.h>
@@ -30,9 +29,7 @@
 #define THREAD_FACTOR 8
 
 
-__thread double darray[] = {0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
-		     1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 2.0,
-		     2.1};
+__thread double darray[32];
 
 int threads_starting;
 int running;
@@ -45,7 +42,7 @@ void *preempt_fpu_c(void *p)
 	int i;
 
 	srand(pthread_self());
-	for (i = 0; i < 21; i++)
+	for (i = 0; i < ARRAY_SIZE(darray); i++)
 		darray[i] = rand();
 
 	rc = preempt_fpu(darray, &threads_starting, &running);
-- 
2.41.0

[PATCH 3/5] selftests/powerpc: Generate better bit patterns for FPU tests

[Michael Ellerman]

The fpu_preempt test randomly initialises an array of doubles to try and
detect FPU register corruption.

However the values it generates do not occupy the full range of values
possible in the 64-bit double, meaning some partial register corruption
could go undetected.

Without getting too carried away, add some better initialisation to
generate values that occupy more bits.

Sample values before:

  f0             902677510               (raw 0x41cae6e203000000)
  f1             325217596               (raw 0x41b3626d3c000000)
  f2             1856578300              (raw 0x41dbaa48bf000000)
  f3             1247189984              (raw 0x41d295a6f8000000)

And after:

  f0             1.1078153481413311e-09  (raw 0x3e13083932805cc2)
  f1             1.0576648474801922e+17  (raw 0x43777c20eb88c261)
  f2             -6.6245033413594075e-10 (raw 0xbe06c2f989facae9)
  f3             3.0085988827156291e+18  (raw 0x43c4e0585f2df37b)

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/powerpc/math/fpu.h    | 25 +++++++++++++++++++
 .../selftests/powerpc/math/fpu_preempt.c      |  6 ++---
 2 files changed, 27 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/powerpc/math/fpu.h

diff --git a/tools/testing/selftests/powerpc/math/fpu.h b/tools/testing/selftests/powerpc/math/fpu.h
new file mode 100644
index 000000000000..a8ad0d42604e
--- /dev/null
+++ b/tools/testing/selftests/powerpc/math/fpu.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2023, Michael Ellerman, IBM Corporation.
+ */
+
+#ifndef _SELFTESTS_POWERPC_FPU_H
+#define _SELFTESTS_POWERPC_FPU_H
+
+static inline void randomise_darray(double *darray, int num)
+{
+	long val;
+
+	for (int i = 0; i < num; i++) {
+		val = random();
+		if (val & 1)
+			val *= -1;
+
+		if (val & 2)
+			darray[i] = 1.0 / val;
+		else
+			darray[i] = val * val;
+	}
+}
+
+#endif /* _SELFTESTS_POWERPC_FPU_H */
diff --git a/tools/testing/selftests/powerpc/math/fpu_preempt.c b/tools/testing/selftests/powerpc/math/fpu_preempt.c
index 24b5abacccdc..97dff3690136 100644
--- a/tools/testing/selftests/powerpc/math/fpu_preempt.c
+++ b/tools/testing/selftests/powerpc/math/fpu_preempt.c
@@ -19,6 +19,7 @@
 #include <pthread.h>
 
 #include "utils.h"
+#include "fpu.h"
 
 /* Time to wait for workers to get preempted (seconds) */
 #define PREEMPT_TIME 20
@@ -39,12 +40,9 @@ extern int preempt_fpu(double *darray, int *threads_starting, int *running);
 void *preempt_fpu_c(void *p)
 {
 	long rc;
-	int i;
 
 	srand(pthread_self());
-	for (i = 0; i < ARRAY_SIZE(darray); i++)
-		darray[i] = rand();
-
+	randomise_darray(darray, ARRAY_SIZE(darray));
 	rc = preempt_fpu(darray, &threads_starting, &running);
 
 	return (void *)rc;
-- 
2.41.0

[PATCH 4/5] selftests/powerpc: Run fpu_preempt test for 60 seconds

[Michael Ellerman]

The FPU preempt test only runs for 20 seconds, which is not particularly
long. Run it for 60 seconds to increase the chance of detecting
corruption.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/powerpc/math/fpu_preempt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/powerpc/math/fpu_preempt.c b/tools/testing/selftests/powerpc/math/fpu_preempt.c
index 97dff3690136..9ddede0770ed 100644
--- a/tools/testing/selftests/powerpc/math/fpu_preempt.c
+++ b/tools/testing/selftests/powerpc/math/fpu_preempt.c
@@ -22,7 +22,7 @@
 #include "fpu.h"
 
 /* Time to wait for workers to get preempted (seconds) */
-#define PREEMPT_TIME 20
+#define PREEMPT_TIME 60
 /*
  * Factor by which to multiply number of online CPUs for total number of
  * worker threads
-- 
2.41.0

[PATCH 5/5] selftests/powerpc: Check all FPRs in fpu_syscall test

[Michael Ellerman]

There is a selftest that checks if FPRs are corrupted across a fork, aka
clone. It was added as part of the series that optimised the clone path
to save the parent's FP state without "giving up" (turning off FP).

See commit 8792468da5e1 ("powerpc: Add the ability to save FPU without
giving it up").

The test encodes the assumption that FPRs 0-13 are volatile across the
syscall, by only checking the volatile FPRs are not changed by the fork.
There was also a comment in the fpu_preempt test alluding to that:

  The check_fpu function in asm only checks the non volatile registers
  as it is reused from the syscall test

It is true that the function call ABI treats f0-f13 as volatile,
however the syscall ABI has since been documented as *not* treating those
registers as volatile. See commit 7b8845a2a2ec ("powerpc/64: Document
the syscall ABI").

So change the test to check all FPRs are not corrupted by the syscall.
Note that this currently fails, because save_fpu() etc. do not restore
f0/vsr0.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/powerpc/math/fpu_asm.S     | 7 ++++---
 tools/testing/selftests/powerpc/math/fpu_syscall.c | 8 +++++---
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/powerpc/math/fpu_asm.S b/tools/testing/selftests/powerpc/math/fpu_asm.S
index 051392ad3ce7..efe1e1be4695 100644
--- a/tools/testing/selftests/powerpc/math/fpu_asm.S
+++ b/tools/testing/selftests/powerpc/math/fpu_asm.S
@@ -109,8 +109,9 @@ FUNC_START(test_fpu)
 	std	r3,STACK_FRAME_PARAM(0)(sp) # Address of darray
 	std r4,STACK_FRAME_PARAM(1)(sp) # Address of pid
 
-	bl load_fpu
-	nop
+	// Load FPRs with expected values
+	OP_REGS lfd, 8, 0, 31, r3
+
 	li	r0,__NR_fork
 	sc
 
@@ -119,7 +120,7 @@ FUNC_START(test_fpu)
 	std	r3,0(r9)
 
 	ld r3,STACK_FRAME_PARAM(0)(sp)
-	bl check_fpu
+	bl check_all_fprs
 	nop
 
 	POP_FPU(256)
diff --git a/tools/testing/selftests/powerpc/math/fpu_syscall.c b/tools/testing/selftests/powerpc/math/fpu_syscall.c
index 694f225c7e45..751d46b133fc 100644
--- a/tools/testing/selftests/powerpc/math/fpu_syscall.c
+++ b/tools/testing/selftests/powerpc/math/fpu_syscall.c
@@ -14,12 +14,11 @@
 #include <stdlib.h>
 
 #include "utils.h"
+#include "fpu.h"
 
 extern int test_fpu(double *darray, pid_t *pid);
 
-double darray[] = {0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
-		     1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 2.0,
-		     2.1};
+double darray[32];
 
 int syscall_fpu(void)
 {
@@ -27,6 +26,9 @@ int syscall_fpu(void)
 	int i;
 	int ret;
 	int child_ret;
+
+	randomise_darray(darray, ARRAY_SIZE(darray));
+
 	for (i = 0; i < 1000; i++) {
 		/* test_fpu will fork() */
 		ret = test_fpu(darray, &fork_pid);
-- 
2.41.0

Re: [PATCH 1/5] selftests/powerpc: Fix error handling in FPU/VMX preemption tests

[Michael Ellerman]

On Wed, 29 Nov 2023 00:27:44 +1100, Michael Ellerman wrote:
> The FPU & VMX preemption tests do not check for errors returned by the
> low-level asm routines, preempt_fpu() / preempt_vsx() respectively.
> That means any register corruption detected by the asm routines does not
> result in a test failure.
> 
> Fix it by returning the return value of the asm routines from the
> pthread child routines.
> 
> [...]

Applied to powerpc/next.

[1/5] selftests/powerpc: Fix error handling in FPU/VMX preemption tests
      https://git.kernel.org/powerpc/c/9dbd5927408c4a0707de73ae9dd9306b184e8fee
[2/5] selftests/powerpc: Check all FPRs in fpu_preempt
      https://git.kernel.org/powerpc/c/e5d00aaac651a69b8016d355123438387bfd37be
[3/5] selftests/powerpc: Generate better bit patterns for FPU tests
      https://git.kernel.org/powerpc/c/2ba107f6795d780bb54b85040a9b2c6a60fccb15
[4/5] selftests/powerpc: Run fpu_preempt test for 60 seconds
      https://git.kernel.org/powerpc/c/60d2c3af9a0c04dc2d34a62e9f5e7033b40dfed8
[5/5] selftests/powerpc: Check all FPRs in fpu_syscall test
      https://git.kernel.org/powerpc/c/1bdf22580b794a498b17617bcd7ae417d6b78444

cheers