All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/8] selftests: arm64: SVE ptrace test rework
@ 2021-09-13 12:54 ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:54 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

This series overhauls the selftests we have for the SVE ptrace interface
to make them much more comprehensive than they are currently, making the
coverage of the data read and written more complete.  The new coverage
for setting data on all vector lengths showed the issue with using the
wrong buffer size with ptrace reported and fixed by:

  https://lore.kernel.org/linux-arm-kernel/20210909165356.10675-1-broonie@kernel.org/

(arm64/sve: Use correct size when reinitialising SVE state).

Mark Brown (8):
  selftests: arm64: Use a define for the number of SVE ptrace tests to
    be run
  selftests: arm64: Don't log child creation as a test in SVE ptrace
    test
  selftests: arm64: Remove extraneous register setting code
  selftests: arm64: Document what the SVE ptrace test is doing
  selftests: arm64: Clarify output when verifying SVE register set
  selftests: arm64: Verify interoperation of SVE and FPSIMD register
    sets
  selftests: arm64: More comprehensively test the SVE ptrace interface
  selftests: arm64: Move FPSIMD in SVE ptrace test into a function

 tools/testing/selftests/arm64/fp/Makefile     |   2 +-
 tools/testing/selftests/arm64/fp/TODO         |   9 +-
 .../selftests/arm64/fp/sve-ptrace-asm.S       |  33 --
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 460 ++++++++++++------
 4 files changed, 321 insertions(+), 183 deletions(-)
 delete mode 100644 tools/testing/selftests/arm64/fp/sve-ptrace-asm.S


base-commit: 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH v1 0/8] selftests: arm64: SVE ptrace test rework
@ 2021-09-13 12:54 ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:54 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

This series overhauls the selftests we have for the SVE ptrace interface
to make them much more comprehensive than they are currently, making the
coverage of the data read and written more complete.  The new coverage
for setting data on all vector lengths showed the issue with using the
wrong buffer size with ptrace reported and fixed by:

  https://lore.kernel.org/linux-arm-kernel/20210909165356.10675-1-broonie@kernel.org/

(arm64/sve: Use correct size when reinitialising SVE state).

Mark Brown (8):
  selftests: arm64: Use a define for the number of SVE ptrace tests to
    be run
  selftests: arm64: Don't log child creation as a test in SVE ptrace
    test
  selftests: arm64: Remove extraneous register setting code
  selftests: arm64: Document what the SVE ptrace test is doing
  selftests: arm64: Clarify output when verifying SVE register set
  selftests: arm64: Verify interoperation of SVE and FPSIMD register
    sets
  selftests: arm64: More comprehensively test the SVE ptrace interface
  selftests: arm64: Move FPSIMD in SVE ptrace test into a function

 tools/testing/selftests/arm64/fp/Makefile     |   2 +-
 tools/testing/selftests/arm64/fp/TODO         |   9 +-
 .../selftests/arm64/fp/sve-ptrace-asm.S       |  33 --
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 460 ++++++++++++------
 4 files changed, 321 insertions(+), 183 deletions(-)
 delete mode 100644 tools/testing/selftests/arm64/fp/sve-ptrace-asm.S


base-commit: 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH v1 1/8] selftests: arm64: Use a define for the number of SVE ptrace tests to be run
  2021-09-13 12:54 ` Mark Brown
@ 2021-09-13 12:54   ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:54 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Partly in preparation for future refactoring move from hard coding the
number of tests in main() to putting #define at the top of the source
instead.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 612d3899614a..7f7ed1c96867 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -19,6 +19,8 @@
 
 #include "../../kselftest.h"
 
+#define EXPECTED_TESTS 20
+
 /* <linux/elf.h> and <sys/auxv.h> don't like each other, so: */
 #ifndef NT_ARM_SVE
 #define NT_ARM_SVE 0x405
@@ -313,7 +315,7 @@ int main(void)
 	pid_t child;
 
 	ksft_print_header();
-	ksft_set_plan(20);
+	ksft_set_plan(EXPECTED_TESTS);
 
 	if (!(getauxval(AT_HWCAP) & HWCAP_SVE))
 		ksft_exit_skip("SVE not available\n");
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 1/8] selftests: arm64: Use a define for the number of SVE ptrace tests to be run
@ 2021-09-13 12:54   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:54 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Partly in preparation for future refactoring move from hard coding the
number of tests in main() to putting #define at the top of the source
instead.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 612d3899614a..7f7ed1c96867 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -19,6 +19,8 @@
 
 #include "../../kselftest.h"
 
+#define EXPECTED_TESTS 20
+
 /* <linux/elf.h> and <sys/auxv.h> don't like each other, so: */
 #ifndef NT_ARM_SVE
 #define NT_ARM_SVE 0x405
@@ -313,7 +315,7 @@ int main(void)
 	pid_t child;
 
 	ksft_print_header();
-	ksft_set_plan(20);
+	ksft_set_plan(EXPECTED_TESTS);
 
 	if (!(getauxval(AT_HWCAP) & HWCAP_SVE))
 		ksft_exit_skip("SVE not available\n");
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 2/8] selftests: arm64: Don't log child creation as a test in SVE ptrace test
  2021-09-13 12:54 ` Mark Brown
@ 2021-09-13 12:54   ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:54 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Currently we log the creation of the child process as a test but it's not
really relevant to what we're trying to test and can make the output a
little confusing so don't do that.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 7f7ed1c96867..7035f01423b3 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -19,7 +19,7 @@
 
 #include "../../kselftest.h"
 
-#define EXPECTED_TESTS 20
+#define EXPECTED_TESTS 19
 
 /* <linux/elf.h> and <sys/auxv.h> don't like each other, so: */
 #ifndef NT_ARM_SVE
@@ -169,8 +169,6 @@ static int do_parent(pid_t child)
 		if (WIFEXITED(status) || WIFSIGNALED(status))
 			ksft_exit_fail_msg("Child died unexpectedly\n");
 
-		ksft_test_result(WIFSTOPPED(status), "WIFSTOPPED(%d)\n",
-				 status);
 		if (!WIFSTOPPED(status))
 			goto error;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 2/8] selftests: arm64: Don't log child creation as a test in SVE ptrace test
@ 2021-09-13 12:54   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:54 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Currently we log the creation of the child process as a test but it's not
really relevant to what we're trying to test and can make the output a
little confusing so don't do that.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 7f7ed1c96867..7035f01423b3 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -19,7 +19,7 @@
 
 #include "../../kselftest.h"
 
-#define EXPECTED_TESTS 20
+#define EXPECTED_TESTS 19
 
 /* <linux/elf.h> and <sys/auxv.h> don't like each other, so: */
 #ifndef NT_ARM_SVE
@@ -169,8 +169,6 @@ static int do_parent(pid_t child)
 		if (WIFEXITED(status) || WIFSIGNALED(status))
 			ksft_exit_fail_msg("Child died unexpectedly\n");
 
-		ksft_test_result(WIFSTOPPED(status), "WIFSTOPPED(%d)\n",
-				 status);
 		if (!WIFSTOPPED(status))
 			goto error;
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 3/8] selftests: arm64: Remove extraneous register setting code
  2021-09-13 12:54 ` Mark Brown
@ 2021-09-13 12:55   ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

For some reason the SVE ptrace test code starts off by setting values in
some of the SVE vector registers in the parent process which it then never
interacts with when verifying the ptrace interfaces. This is not especially
relevant to what's being tested and somewhat confusing when reading the
code so let's remove it.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/Makefile     |  2 +-
 .../selftests/arm64/fp/sve-ptrace-asm.S       | 33 -------------------
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 28 ----------------
 3 files changed, 1 insertion(+), 62 deletions(-)
 delete mode 100644 tools/testing/selftests/arm64/fp/sve-ptrace-asm.S

diff --git a/tools/testing/selftests/arm64/fp/Makefile b/tools/testing/selftests/arm64/fp/Makefile
index f2abdd6ba12e..4367125b7c27 100644
--- a/tools/testing/selftests/arm64/fp/Makefile
+++ b/tools/testing/selftests/arm64/fp/Makefile
@@ -12,7 +12,7 @@ all: $(TEST_GEN_PROGS) $(TEST_PROGS_EXTENDED)
 fpsimd-test: fpsimd-test.o
 	$(CC) -nostdlib $^ -o $@
 rdvl-sve: rdvl-sve.o rdvl.o
-sve-ptrace: sve-ptrace.o sve-ptrace-asm.o
+sve-ptrace: sve-ptrace.o
 sve-probe-vls: sve-probe-vls.o rdvl.o
 sve-test: sve-test.o
 	$(CC) -nostdlib $^ -o $@
diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace-asm.S b/tools/testing/selftests/arm64/fp/sve-ptrace-asm.S
deleted file mode 100644
index 3e81f9fab574..000000000000
--- a/tools/testing/selftests/arm64/fp/sve-ptrace-asm.S
+++ /dev/null
@@ -1,33 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-// Copyright (C) 2015-2019 ARM Limited.
-// Original author: Dave Martin <Dave.Martin@arm.com>
-#include <asm/unistd.h>
-
-.arch_extension sve
-
-.globl sve_store_patterns
-
-sve_store_patterns:
-	mov	x1, x0
-
-	index	z0.b, #0, #1
-	str	q0, [x1]
-
-	mov	w8, #__NR_getpid
-	svc	#0
-	str	q0, [x1, #0x10]
-
-	mov	z1.d, z0.d
-	str	q0, [x1, #0x20]
-
-	mov	w8, #__NR_getpid
-	svc	#0
-	str	q0, [x1, #0x30]
-
-	mov	z1.d, z0.d
-	str	q0, [x1, #0x40]
-
-	ret
-
-.size	sve_store_patterns, . - sve_store_patterns
-.type	sve_store_patterns, @function
diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 7035f01423b3..d2ec48f649f9 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -26,11 +26,6 @@
 #define NT_ARM_SVE 0x405
 #endif
 
-/* Number of registers filled in by sve_store_patterns */
-#define NR_VREGS 5
-
-void sve_store_patterns(__uint128_t v[NR_VREGS]);
-
 static void dump(const void *buf, size_t size)
 {
 	size_t i;
@@ -40,23 +35,6 @@ static void dump(const void *buf, size_t size)
 		printf(" %.2x", *p++);
 }
 
-static int check_vregs(const __uint128_t vregs[NR_VREGS])
-{
-	int i;
-	int ok = 1;
-
-	for (i = 0; i < NR_VREGS; ++i) {
-		printf("# v[%d]:", i);
-		dump(&vregs[i], sizeof vregs[i]);
-		putchar('\n');
-
-		if (vregs[i] != vregs[0])
-			ok = 0;
-	}
-
-	return ok;
-}
-
 static int do_child(void)
 {
 	if (ptrace(PTRACE_TRACEME, -1, NULL, NULL))
@@ -309,7 +287,6 @@ static int do_parent(pid_t child)
 int main(void)
 {
 	int ret = EXIT_SUCCESS;
-	__uint128_t v[NR_VREGS];
 	pid_t child;
 
 	ksft_print_header();
@@ -318,11 +295,6 @@ int main(void)
 	if (!(getauxval(AT_HWCAP) & HWCAP_SVE))
 		ksft_exit_skip("SVE not available\n");
 
-	sve_store_patterns(v);
-
-	if (!check_vregs(v))
-		ksft_exit_fail_msg("Initial check_vregs() failed\n");
-
 	child = fork();
 	if (!child)
 		return do_child();
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 3/8] selftests: arm64: Remove extraneous register setting code
@ 2021-09-13 12:55   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

For some reason the SVE ptrace test code starts off by setting values in
some of the SVE vector registers in the parent process which it then never
interacts with when verifying the ptrace interfaces. This is not especially
relevant to what's being tested and somewhat confusing when reading the
code so let's remove it.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/Makefile     |  2 +-
 .../selftests/arm64/fp/sve-ptrace-asm.S       | 33 -------------------
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 28 ----------------
 3 files changed, 1 insertion(+), 62 deletions(-)
 delete mode 100644 tools/testing/selftests/arm64/fp/sve-ptrace-asm.S

diff --git a/tools/testing/selftests/arm64/fp/Makefile b/tools/testing/selftests/arm64/fp/Makefile
index f2abdd6ba12e..4367125b7c27 100644
--- a/tools/testing/selftests/arm64/fp/Makefile
+++ b/tools/testing/selftests/arm64/fp/Makefile
@@ -12,7 +12,7 @@ all: $(TEST_GEN_PROGS) $(TEST_PROGS_EXTENDED)
 fpsimd-test: fpsimd-test.o
 	$(CC) -nostdlib $^ -o $@
 rdvl-sve: rdvl-sve.o rdvl.o
-sve-ptrace: sve-ptrace.o sve-ptrace-asm.o
+sve-ptrace: sve-ptrace.o
 sve-probe-vls: sve-probe-vls.o rdvl.o
 sve-test: sve-test.o
 	$(CC) -nostdlib $^ -o $@
diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace-asm.S b/tools/testing/selftests/arm64/fp/sve-ptrace-asm.S
deleted file mode 100644
index 3e81f9fab574..000000000000
--- a/tools/testing/selftests/arm64/fp/sve-ptrace-asm.S
+++ /dev/null
@@ -1,33 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-// Copyright (C) 2015-2019 ARM Limited.
-// Original author: Dave Martin <Dave.Martin@arm.com>
-#include <asm/unistd.h>
-
-.arch_extension sve
-
-.globl sve_store_patterns
-
-sve_store_patterns:
-	mov	x1, x0
-
-	index	z0.b, #0, #1
-	str	q0, [x1]
-
-	mov	w8, #__NR_getpid
-	svc	#0
-	str	q0, [x1, #0x10]
-
-	mov	z1.d, z0.d
-	str	q0, [x1, #0x20]
-
-	mov	w8, #__NR_getpid
-	svc	#0
-	str	q0, [x1, #0x30]
-
-	mov	z1.d, z0.d
-	str	q0, [x1, #0x40]
-
-	ret
-
-.size	sve_store_patterns, . - sve_store_patterns
-.type	sve_store_patterns, @function
diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 7035f01423b3..d2ec48f649f9 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -26,11 +26,6 @@
 #define NT_ARM_SVE 0x405
 #endif
 
-/* Number of registers filled in by sve_store_patterns */
-#define NR_VREGS 5
-
-void sve_store_patterns(__uint128_t v[NR_VREGS]);
-
 static void dump(const void *buf, size_t size)
 {
 	size_t i;
@@ -40,23 +35,6 @@ static void dump(const void *buf, size_t size)
 		printf(" %.2x", *p++);
 }
 
-static int check_vregs(const __uint128_t vregs[NR_VREGS])
-{
-	int i;
-	int ok = 1;
-
-	for (i = 0; i < NR_VREGS; ++i) {
-		printf("# v[%d]:", i);
-		dump(&vregs[i], sizeof vregs[i]);
-		putchar('\n');
-
-		if (vregs[i] != vregs[0])
-			ok = 0;
-	}
-
-	return ok;
-}
-
 static int do_child(void)
 {
 	if (ptrace(PTRACE_TRACEME, -1, NULL, NULL))
@@ -309,7 +287,6 @@ static int do_parent(pid_t child)
 int main(void)
 {
 	int ret = EXIT_SUCCESS;
-	__uint128_t v[NR_VREGS];
 	pid_t child;
 
 	ksft_print_header();
@@ -318,11 +295,6 @@ int main(void)
 	if (!(getauxval(AT_HWCAP) & HWCAP_SVE))
 		ksft_exit_skip("SVE not available\n");
 
-	sve_store_patterns(v);
-
-	if (!check_vregs(v))
-		ksft_exit_fail_msg("Initial check_vregs() failed\n");
-
 	child = fork();
 	if (!child)
 		return do_child();
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 4/8] selftests: arm64: Document what the SVE ptrace test is doing
  2021-09-13 12:54 ` Mark Brown
@ 2021-09-13 12:55   ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Before we go modifying it further let's add some comments and output
clarifications explaining what this test is actually doing.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index d2ec48f649f9..fc4a672825eb 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -181,6 +181,7 @@ static int do_parent(pid_t child)
 		}
 	}
 
+	/* New process should start with FPSIMD registers only */
 	sve = get_sve(pid, &svebuf, &svebufsz);
 	if (!sve) {
 		int e = errno;
@@ -191,14 +192,15 @@ static int do_parent(pid_t child)
 
 		goto error;
 	} else {
-		ksft_test_result_pass("get_sve\n");
+		ksft_test_result_pass("get_sve(FPSIMD)\n");
 	}
 
 	ksft_test_result((sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_FPSIMD,
-			 "FPSIMD registers\n");
+			 "Set FPSIMD registers\n");
 	if ((sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_FPSIMD)
 		goto error;
 
+	/* Try to set a known FPSIMD state via PT_REGS_SVE */
 	fpsimd = (struct user_fpsimd_state *)((char *)sve +
 					      SVE_PT_FPSIMD_OFFSET);
 	for (i = 0; i < 32; ++i) {
@@ -219,6 +221,7 @@ static int do_parent(pid_t child)
 		goto error;
 	}
 
+	/* Zero the first SVE Z register */
 	vq = sve_vq_from_vl(sve->vl);
 
 	newsvebufsz = SVE_PT_SVE_ZREG_OFFSET(vq, 1);
@@ -245,6 +248,7 @@ static int do_parent(pid_t child)
 		goto error;
 	}
 
+	/* Try to read back the value we just set */
 	new_sve = get_sve(pid, &newsvebuf, &newsvebufsz);
 	if (!new_sve) {
 		int e = errno;
@@ -257,12 +261,13 @@ static int do_parent(pid_t child)
 	}
 
 	ksft_test_result((new_sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_SVE,
-			 "SVE registers\n");
+			 "Get SVE registers\n");
 	if ((new_sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_SVE)
 		goto error;
 
 	dump_sve_regs(new_sve, 3, sizeof fpsimd->vregs[0]);
 
+	/* Verify that the register we set has the value we expected */
 	p = (unsigned char *)new_sve + SVE_PT_SVE_ZREG_OFFSET(vq, 1);
 	for (i = 0; i < sizeof fpsimd->vregs[0]; ++i) {
 		unsigned char expected = i;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 4/8] selftests: arm64: Document what the SVE ptrace test is doing
@ 2021-09-13 12:55   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Before we go modifying it further let's add some comments and output
clarifications explaining what this test is actually doing.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index d2ec48f649f9..fc4a672825eb 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -181,6 +181,7 @@ static int do_parent(pid_t child)
 		}
 	}
 
+	/* New process should start with FPSIMD registers only */
 	sve = get_sve(pid, &svebuf, &svebufsz);
 	if (!sve) {
 		int e = errno;
@@ -191,14 +192,15 @@ static int do_parent(pid_t child)
 
 		goto error;
 	} else {
-		ksft_test_result_pass("get_sve\n");
+		ksft_test_result_pass("get_sve(FPSIMD)\n");
 	}
 
 	ksft_test_result((sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_FPSIMD,
-			 "FPSIMD registers\n");
+			 "Set FPSIMD registers\n");
 	if ((sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_FPSIMD)
 		goto error;
 
+	/* Try to set a known FPSIMD state via PT_REGS_SVE */
 	fpsimd = (struct user_fpsimd_state *)((char *)sve +
 					      SVE_PT_FPSIMD_OFFSET);
 	for (i = 0; i < 32; ++i) {
@@ -219,6 +221,7 @@ static int do_parent(pid_t child)
 		goto error;
 	}
 
+	/* Zero the first SVE Z register */
 	vq = sve_vq_from_vl(sve->vl);
 
 	newsvebufsz = SVE_PT_SVE_ZREG_OFFSET(vq, 1);
@@ -245,6 +248,7 @@ static int do_parent(pid_t child)
 		goto error;
 	}
 
+	/* Try to read back the value we just set */
 	new_sve = get_sve(pid, &newsvebuf, &newsvebufsz);
 	if (!new_sve) {
 		int e = errno;
@@ -257,12 +261,13 @@ static int do_parent(pid_t child)
 	}
 
 	ksft_test_result((new_sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_SVE,
-			 "SVE registers\n");
+			 "Get SVE registers\n");
 	if ((new_sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_SVE)
 		goto error;
 
 	dump_sve_regs(new_sve, 3, sizeof fpsimd->vregs[0]);
 
+	/* Verify that the register we set has the value we expected */
 	p = (unsigned char *)new_sve + SVE_PT_SVE_ZREG_OFFSET(vq, 1);
 	for (i = 0; i < sizeof fpsimd->vregs[0]; ++i) {
 		unsigned char expected = i;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 5/8] selftests: arm64: Clarify output when verifying SVE register set
  2021-09-13 12:54 ` Mark Brown
@ 2021-09-13 12:55   ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

When verifying setting a Z register via ptrace we check each byte by hand,
iterating over the buffer using a pointer called p and treating each
register value written as a test. This creates output referring to "p[X]"
which is confusing since SVE also has predicate registers Pn. Tweak the
output to avoid confusion here.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index fc4a672825eb..2d130fedc019 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -275,7 +275,7 @@ static int do_parent(pid_t child)
 		if (__BYTE_ORDER == __BIG_ENDIAN)
 			expected = sizeof fpsimd->vregs[0] - 1 - expected;
 
-		ksft_test_result(p[i] == expected, "p[%d] == expected\n", i);
+		ksft_test_result(p[i] == expected, "buf[%d] == expected\n", i);
 		if (p[i] != expected)
 			goto error;
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 5/8] selftests: arm64: Clarify output when verifying SVE register set
@ 2021-09-13 12:55   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

When verifying setting a Z register via ptrace we check each byte by hand,
iterating over the buffer using a pointer called p and treating each
register value written as a test. This creates output referring to "p[X]"
which is confusing since SVE also has predicate registers Pn. Tweak the
output to avoid confusion here.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index fc4a672825eb..2d130fedc019 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -275,7 +275,7 @@ static int do_parent(pid_t child)
 		if (__BYTE_ORDER == __BIG_ENDIAN)
 			expected = sizeof fpsimd->vregs[0] - 1 - expected;
 
-		ksft_test_result(p[i] == expected, "p[%d] == expected\n", i);
+		ksft_test_result(p[i] == expected, "buf[%d] == expected\n", i);
 		if (p[i] != expected)
 			goto error;
 	}
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 6/8] selftests: arm64: Verify interoperation of SVE and FPSIMD register sets
  2021-09-13 12:54 ` Mark Brown
@ 2021-09-13 12:55   ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

After setting the FPSIMD registers via the SVE register set read them back
via the FPSIMD register set, validating that the two register sets are
interoperating and that the values we thought we set made it into the
child process.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 28 +++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 2d130fedc019..31a2c2fc529d 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -46,6 +46,15 @@ static int do_child(void)
 	return EXIT_SUCCESS;
 }
 
+static int get_fpsimd(pid_t pid, struct user_fpsimd_state *fpsimd)
+{
+	struct iovec iov;
+
+	iov.iov_base = fpsimd;
+	iov.iov_len = sizeof(*fpsimd);
+	return ptrace(PTRACE_GETREGSET, pid, NT_PRFPREG, &iov);
+}
+
 static struct user_sve_header *get_sve(pid_t pid, void **buf, size_t *size)
 {
 	struct user_sve_header *sve;
@@ -122,7 +131,7 @@ static int do_parent(pid_t child)
 	void *svebuf = NULL, *newsvebuf;
 	size_t svebufsz = 0, newsvebufsz;
 	struct user_sve_header *sve, *new_sve;
-	struct user_fpsimd_state *fpsimd;
+	struct user_fpsimd_state *fpsimd, new_fpsimd;
 	unsigned int i, j;
 	unsigned char *p;
 	unsigned int vq;
@@ -221,7 +230,22 @@ static int do_parent(pid_t child)
 		goto error;
 	}
 
-	/* Zero the first SVE Z register */
+	/* Verify via the FPSIMD regset */
+	if (get_fpsimd(pid, &new_fpsimd)) {
+		int e = errno;
+
+		ksft_test_result_fail("get_fpsimd(): %s\n",
+				      strerror(errno));
+		if (e == ESRCH)
+			goto disappeared;
+
+		goto error;
+	}
+	if (memcmp(fpsimd, &new_fpsimd, sizeof(*fpsimd)) == 0)
+		ksft_test_result_pass("get_fpsimd() gave same state\n");
+	else
+		ksft_test_result_fail("get_fpsimd() gave different state\n");
+
 	vq = sve_vq_from_vl(sve->vl);
 
 	newsvebufsz = SVE_PT_SVE_ZREG_OFFSET(vq, 1);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 6/8] selftests: arm64: Verify interoperation of SVE and FPSIMD register sets
@ 2021-09-13 12:55   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

After setting the FPSIMD registers via the SVE register set read them back
via the FPSIMD register set, validating that the two register sets are
interoperating and that the values we thought we set made it into the
child process.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 28 +++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 2d130fedc019..31a2c2fc529d 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -46,6 +46,15 @@ static int do_child(void)
 	return EXIT_SUCCESS;
 }
 
+static int get_fpsimd(pid_t pid, struct user_fpsimd_state *fpsimd)
+{
+	struct iovec iov;
+
+	iov.iov_base = fpsimd;
+	iov.iov_len = sizeof(*fpsimd);
+	return ptrace(PTRACE_GETREGSET, pid, NT_PRFPREG, &iov);
+}
+
 static struct user_sve_header *get_sve(pid_t pid, void **buf, size_t *size)
 {
 	struct user_sve_header *sve;
@@ -122,7 +131,7 @@ static int do_parent(pid_t child)
 	void *svebuf = NULL, *newsvebuf;
 	size_t svebufsz = 0, newsvebufsz;
 	struct user_sve_header *sve, *new_sve;
-	struct user_fpsimd_state *fpsimd;
+	struct user_fpsimd_state *fpsimd, new_fpsimd;
 	unsigned int i, j;
 	unsigned char *p;
 	unsigned int vq;
@@ -221,7 +230,22 @@ static int do_parent(pid_t child)
 		goto error;
 	}
 
-	/* Zero the first SVE Z register */
+	/* Verify via the FPSIMD regset */
+	if (get_fpsimd(pid, &new_fpsimd)) {
+		int e = errno;
+
+		ksft_test_result_fail("get_fpsimd(): %s\n",
+				      strerror(errno));
+		if (e == ESRCH)
+			goto disappeared;
+
+		goto error;
+	}
+	if (memcmp(fpsimd, &new_fpsimd, sizeof(*fpsimd)) == 0)
+		ksft_test_result_pass("get_fpsimd() gave same state\n");
+	else
+		ksft_test_result_fail("get_fpsimd() gave different state\n");
+
 	vq = sve_vq_from_vl(sve->vl);
 
 	newsvebufsz = SVE_PT_SVE_ZREG_OFFSET(vq, 1);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 7/8] selftests: arm64: More comprehensively test the SVE ptrace interface
  2021-09-13 12:54 ` Mark Brown
@ 2021-09-13 12:55   ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Currently the selftest for the SVE register set is not quite as thorough
as is desirable - it only validates that the value of a single Z register
is not modified by a partial write to a lower numbered Z register after
having previously been set through the FPSIMD regset.

Make this more thorough:
 - Test the ability to set vector lengths and enumerate those supported in
   the system.
 - Validate data in all Z and P registers, plus FPSR and FPCR.
 - Test reads via the FPSIMD regset after set via the SVE regset.

There's still some oversights, the main one being that due to the need to
generate a pattern in FFR and the fact that this rewrite is primarily
motivated by SME's streaming SVE which doesn't have FFR we don't currently
test FFR. Update the TODO to reflect those that occurred to me (and fix an
adjacent typo in there).

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/TODO         |   9 +-
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 327 +++++++++++++-----
 2 files changed, 254 insertions(+), 82 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/TODO b/tools/testing/selftests/arm64/fp/TODO
index b6b7ebfcf362..44004e53da33 100644
--- a/tools/testing/selftests/arm64/fp/TODO
+++ b/tools/testing/selftests/arm64/fp/TODO
@@ -1,4 +1,7 @@
 - Test unsupported values in the ABIs.
-- More coverage for ptrace (eg, vector length conversions).
-- Coverage for signals.
-- Test PR_SVE_VL_INHERITY after a double fork.
+- More coverage for ptrace:
+ - Get/set of FFR.
+ - Ensure ptraced processes actually see the register state visible through
+   the ptrace interface.
+ - Big endian.
+- Test PR_SVE_VL_INHERIT after a double fork.
diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 31a2c2fc529d..199710ba65c7 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -1,15 +1,17 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (C) 2015-2020 ARM Limited.
+ * Copyright (C) 2015-2021 ARM Limited.
  * Original author: Dave Martin <Dave.Martin@arm.com>
  */
 #include <errno.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <sys/auxv.h>
+#include <sys/prctl.h>
 #include <sys/ptrace.h>
 #include <sys/types.h>
 #include <sys/uio.h>
@@ -19,20 +21,22 @@
 
 #include "../../kselftest.h"
 
-#define EXPECTED_TESTS 19
+#define VL_TESTS (((SVE_VQ_MAX - SVE_VQ_MIN) + 1) * 3)
+#define FPSIMD_TESTS 3
+
+#define EXPECTED_TESTS (VL_TESTS + FPSIMD_TESTS)
 
 /* <linux/elf.h> and <sys/auxv.h> don't like each other, so: */
 #ifndef NT_ARM_SVE
 #define NT_ARM_SVE 0x405
 #endif
 
-static void dump(const void *buf, size_t size)
+static void fill_buf(char *buf, size_t size)
 {
-	size_t i;
-	const unsigned char *p = buf;
+	int i;
 
-	for (i = 0; i < size; ++i)
-		printf(" %.2x", *p++);
+	for (i = 0; i < size; i++)
+		buf[i] = random();
 }
 
 static int do_child(void)
@@ -101,25 +105,228 @@ static int set_sve(pid_t pid, const struct user_sve_header *sve)
 	return ptrace(PTRACE_SETREGSET, pid, NT_ARM_SVE, &iov);
 }
 
-static void dump_sve_regs(const struct user_sve_header *sve, unsigned int num,
-			  unsigned int vlmax)
+/* Validate attempting to set the specfied VL via ptrace */
+static void ptrace_set_get_vl(pid_t child, unsigned int vl, bool *supported)
+{
+	struct user_sve_header sve;
+	struct user_sve_header *new_sve = NULL;
+	size_t new_sve_size = 0;
+	int ret, prctl_vl;
+
+	*supported = false;
+
+	/* Check if the VL is supported in this process */
+	prctl_vl = prctl(PR_SVE_SET_VL, vl);
+	if (prctl_vl == -1)
+		ksft_exit_fail_msg("prctl(PR_SVE_SET_VL) failed: %s (%d)\n",
+				   strerror(errno), errno);
+
+	/* If the VL is not supported then a supported VL will be returned */
+	*supported = (prctl_vl == vl);
+
+	/* Set the VL by doing a set with no register payload */
+	memset(&sve, 0, sizeof(sve));
+	sve.size = sizeof(sve);
+	sve.vl = vl;
+	ret = set_sve(child, &sve);
+	if (ret != 0) {
+		ksft_test_result_fail("Failed to set VL %u\n", vl);
+		return;
+	}
+
+	/*
+	 * Read back the new register state and verify that we have the
+	 * same VL that we got from prctl() on ourselves.
+	 */
+	if (!get_sve(child, (void **)&new_sve, &new_sve_size)) {
+		ksft_test_result_fail("Failed to read VL %u\n", vl);
+		return;
+	}
+
+	ksft_test_result(new_sve->vl = prctl_vl, "Set VL %u\n", vl);
+
+	free(new_sve);
+}
+
+static void check_u32(unsigned int vl, const char *reg,
+		      uint32_t *in, uint32_t *out, int *errors)
+{
+	if (*in != *out) {
+		printf("# VL %d %s wrote %x read %x\n",
+		       vl, reg, *in, *out);
+		(*errors)++;
+	}
+}
+
+/* Validate attempting to set SVE data and read SVE data */
+static void ptrace_set_sve_get_sve_data(pid_t child, unsigned int vl)
+{
+	void *write_buf;
+	void *read_buf = NULL;
+	struct user_sve_header *write_sve;
+	struct user_sve_header *read_sve;
+	size_t read_sve_size = 0;
+	unsigned int vq = sve_vq_from_vl(vl);
+	int ret, i;
+	size_t data_size;
+	int errors = 0;
+
+	data_size = SVE_PT_SVE_OFFSET + SVE_PT_SVE_SIZE(vq, SVE_PT_REGS_SVE);
+	write_buf = malloc(data_size);
+	if (!write_buf) {
+		ksft_test_result_fail("Error allocating %d byte buffer for VL %u\n",
+				      data_size, vl);
+		return;
+	}
+	write_sve = write_buf;
+
+	/* Set up some data and write it out */
+	memset(write_sve, 0, data_size);
+	write_sve->size = data_size;
+	write_sve->vl = vl;
+	write_sve->flags = SVE_PT_REGS_SVE;
+
+	for (i = 0; i < __SVE_NUM_ZREGS; i++)
+		fill_buf(write_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+			 SVE_PT_SVE_ZREG_SIZE(vq));
+
+	for (i = 0; i < __SVE_NUM_PREGS; i++)
+		fill_buf(write_buf + SVE_PT_SVE_PREG_OFFSET(vq, i),
+			 SVE_PT_SVE_PREG_SIZE(vq));
+
+	fill_buf(write_buf + SVE_PT_SVE_FPSR_OFFSET(vq), SVE_PT_SVE_FPSR_SIZE);
+	fill_buf(write_buf + SVE_PT_SVE_FPCR_OFFSET(vq), SVE_PT_SVE_FPCR_SIZE);
+
+	/* TODO: Generate a valid FFR pattern */
+
+	ret = set_sve(child, write_sve);
+	if (ret != 0) {
+		ksft_test_result_fail("Failed to set VL %u data\n", vl);
+		goto out;
+	}
+
+	/* Read the data back */
+	if (!get_sve(child, (void **)&read_buf, &read_sve_size)) {
+		ksft_test_result_fail("Failed to read VL %u data\n", vl);
+		goto out;
+	}
+	read_sve = read_buf;
+
+	/* We might read more data if there's extensions we don't know */
+	if (read_sve->size < write_sve->size) {
+		ksft_test_result_fail("Wrote %d bytes, only read %d\n",
+				      write_sve->size, read_sve->size);
+		goto out_read;
+	}
+
+	for (i = 0; i < __SVE_NUM_ZREGS; i++) {
+		if (memcmp(write_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+			   read_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+			   SVE_PT_SVE_ZREG_SIZE(vq)) != 0) {
+			printf("# Mismatch in %u Z%d\n", vl, i);
+			errors++;
+		}
+	}
+
+	for (i = 0; i < __SVE_NUM_PREGS; i++) {
+		if (memcmp(write_buf + SVE_PT_SVE_PREG_OFFSET(vq, i),
+			   read_buf + SVE_PT_SVE_PREG_OFFSET(vq, i),
+			   SVE_PT_SVE_PREG_SIZE(vq)) != 0) {
+			printf("# Mismatch in %u P%d\n", vl, i);
+			errors++;
+		}
+	}
+
+	check_u32(vl, "FPSR", write_buf + SVE_PT_SVE_FPSR_OFFSET(vq),
+		  read_buf + SVE_PT_SVE_FPSR_OFFSET(vq), &errors);
+	check_u32(vl, "FPCR", write_buf + SVE_PT_SVE_FPCR_OFFSET(vq),
+		  read_buf + SVE_PT_SVE_FPCR_OFFSET(vq), &errors);
+
+	ksft_test_result(errors == 0, "Set and get SVE data for VL %u\n", vl);
+
+out_read:
+	free(read_buf);
+out:
+	free(write_buf);
+}
+
+/* Validate attempting to set SVE data and read SVE data */
+static void ptrace_set_sve_get_fpsimd_data(pid_t child, unsigned int vl)
 {
-	unsigned int vq;
-	unsigned int i;
+	void *write_buf;
+	struct user_sve_header *write_sve;
+	unsigned int vq = sve_vq_from_vl(vl);
+	struct user_fpsimd_state fpsimd_state;
+	int ret, i;
+	size_t data_size;
+	int errors = 0;
+
+	if (__BYTE_ORDER == __BIG_ENDIAN) {
+		ksft_test_result_skip("Big endian not supported\n");
+		return;
+	}
+
+	data_size = SVE_PT_SVE_OFFSET + SVE_PT_SVE_SIZE(vq, SVE_PT_REGS_SVE);
+	write_buf = malloc(data_size);
+	if (!write_buf) {
+		ksft_test_result_fail("Error allocating %d byte buffer for VL %u\n",
+				      data_size, vl);
+		return;
+	}
+	write_sve = write_buf;
 
-	if ((sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_SVE)
-		ksft_exit_fail_msg("Dumping non-SVE register\n");
+	/* Set up some data and write it out */
+	memset(write_sve, 0, data_size);
+	write_sve->size = data_size;
+	write_sve->vl = vl;
+	write_sve->flags = SVE_PT_REGS_SVE;
 
-	if (vlmax > sve->vl)
-		vlmax = sve->vl;
+	for (i = 0; i < __SVE_NUM_ZREGS; i++)
+		fill_buf(write_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+			 SVE_PT_SVE_ZREG_SIZE(vq));
 
-	vq = sve_vq_from_vl(sve->vl);
-	for (i = 0; i < num; ++i) {
-		printf("# z%u:", i);
-		dump((const char *)sve + SVE_PT_SVE_ZREG_OFFSET(vq, i),
-		     vlmax);
-		printf("%s\n", vlmax == sve->vl ? "" : " ...");
+	fill_buf(write_buf + SVE_PT_SVE_FPSR_OFFSET(vq), SVE_PT_SVE_FPSR_SIZE);
+	fill_buf(write_buf + SVE_PT_SVE_FPCR_OFFSET(vq), SVE_PT_SVE_FPCR_SIZE);
+
+	ret = set_sve(child, write_sve);
+	if (ret != 0) {
+		ksft_test_result_fail("Failed to set VL %u data\n", vl);
+		goto out;
+	}
+
+	/* Read the data back */
+	if (get_fpsimd(child, &fpsimd_state)) {
+		ksft_test_result_fail("Failed to read VL %u FPSIMD data\n",
+				      vl);
+		goto out;
+	}
+
+	for (i = 0; i < __SVE_NUM_ZREGS; i++) {
+		__uint128_t tmp = 0;
+
+		/*
+		 * Z regs are stored endianness invariant, this won't
+		 * work for big endian
+		 */
+		memcpy(&tmp, write_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+		       sizeof(tmp));
+
+		if (tmp != fpsimd_state.vregs[i]) {
+			printf("# Mismatch in FPSIMD for VL %u Z%d\n", vl, i);
+			errors++;
+		}
 	}
+
+	check_u32(vl, "FPSR", write_buf + SVE_PT_SVE_FPSR_OFFSET(vq),
+		  &fpsimd_state.fpsr, &errors);
+	check_u32(vl, "FPCR", write_buf + SVE_PT_SVE_FPCR_OFFSET(vq),
+		  &fpsimd_state.fpcr, &errors);
+
+	ksft_test_result(errors == 0, "Set and get FPSIMD data for VL %u\n",
+			 vl);
+
+out:
+	free(write_buf);
 }
 
 static int do_parent(pid_t child)
@@ -128,13 +335,14 @@ static int do_parent(pid_t child)
 	pid_t pid;
 	int status;
 	siginfo_t si;
-	void *svebuf = NULL, *newsvebuf;
-	size_t svebufsz = 0, newsvebufsz;
-	struct user_sve_header *sve, *new_sve;
+	void *svebuf = NULL;
+	size_t svebufsz = 0;
+	struct user_sve_header *sve;
 	struct user_fpsimd_state *fpsimd, new_fpsimd;
 	unsigned int i, j;
 	unsigned char *p;
-	unsigned int vq;
+	unsigned int vq, vl;
+	bool vl_supported;
 
 	/* Attach to the child */
 	while (1) {
@@ -246,62 +454,21 @@ static int do_parent(pid_t child)
 	else
 		ksft_test_result_fail("get_fpsimd() gave different state\n");
 
-	vq = sve_vq_from_vl(sve->vl);
-
-	newsvebufsz = SVE_PT_SVE_ZREG_OFFSET(vq, 1);
-	new_sve = newsvebuf = malloc(newsvebufsz);
-	if (!new_sve) {
-		errno = ENOMEM;
-		perror(NULL);
-		goto error;
-	}
-
-	*new_sve = *sve;
-	new_sve->flags &= ~SVE_PT_REGS_MASK;
-	new_sve->flags |= SVE_PT_REGS_SVE;
-	memset((char *)new_sve + SVE_PT_SVE_ZREG_OFFSET(vq, 0),
-	       0, SVE_PT_SVE_ZREG_SIZE(vq));
-	new_sve->size = SVE_PT_SVE_ZREG_OFFSET(vq, 1);
-	if (set_sve(pid, new_sve)) {
-		int e = errno;
-
-		ksft_test_result_fail("set_sve(ZREG): %s\n", strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
-
-		goto error;
-	}
-
-	/* Try to read back the value we just set */
-	new_sve = get_sve(pid, &newsvebuf, &newsvebufsz);
-	if (!new_sve) {
-		int e = errno;
-
-		ksft_test_result_fail("get_sve(ZREG): %s\n", strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
+	/* Step through every possible VQ */
+	for (vq = SVE_VQ_MIN; vq <= SVE_VQ_MAX; vq++) {
+		vl = sve_vl_from_vq(vq);
 
-		goto error;
-	}
-
-	ksft_test_result((new_sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_SVE,
-			 "Get SVE registers\n");
-	if ((new_sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_SVE)
-		goto error;
-
-	dump_sve_regs(new_sve, 3, sizeof fpsimd->vregs[0]);
+		/* First, try to set this vector length */
+		ptrace_set_get_vl(child, vl, &vl_supported);
 
-	/* Verify that the register we set has the value we expected */
-	p = (unsigned char *)new_sve + SVE_PT_SVE_ZREG_OFFSET(vq, 1);
-	for (i = 0; i < sizeof fpsimd->vregs[0]; ++i) {
-		unsigned char expected = i;
-
-		if (__BYTE_ORDER == __BIG_ENDIAN)
-			expected = sizeof fpsimd->vregs[0] - 1 - expected;
-
-		ksft_test_result(p[i] == expected, "buf[%d] == expected\n", i);
-		if (p[i] != expected)
-			goto error;
+		/* If the VL is supported validate data set/get */
+		if (vl_supported) {
+			ptrace_set_sve_get_sve_data(child, vl);
+			ptrace_set_sve_get_fpsimd_data(child, vl);
+		} else {
+			ksft_test_result_skip("set SVE get SVE for VL %d\n", vl);
+			ksft_test_result_skip("set SVE get FPSIMD for VL %d\n", vl);
+		}
 	}
 
 	ret = EXIT_SUCCESS;
@@ -318,6 +485,8 @@ int main(void)
 	int ret = EXIT_SUCCESS;
 	pid_t child;
 
+	srandom(getpid());
+
 	ksft_print_header();
 	ksft_set_plan(EXPECTED_TESTS);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 7/8] selftests: arm64: More comprehensively test the SVE ptrace interface
@ 2021-09-13 12:55   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Currently the selftest for the SVE register set is not quite as thorough
as is desirable - it only validates that the value of a single Z register
is not modified by a partial write to a lower numbered Z register after
having previously been set through the FPSIMD regset.

Make this more thorough:
 - Test the ability to set vector lengths and enumerate those supported in
   the system.
 - Validate data in all Z and P registers, plus FPSR and FPCR.
 - Test reads via the FPSIMD regset after set via the SVE regset.

There's still some oversights, the main one being that due to the need to
generate a pattern in FFR and the fact that this rewrite is primarily
motivated by SME's streaming SVE which doesn't have FFR we don't currently
test FFR. Update the TODO to reflect those that occurred to me (and fix an
adjacent typo in there).

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/TODO         |   9 +-
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 327 +++++++++++++-----
 2 files changed, 254 insertions(+), 82 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/TODO b/tools/testing/selftests/arm64/fp/TODO
index b6b7ebfcf362..44004e53da33 100644
--- a/tools/testing/selftests/arm64/fp/TODO
+++ b/tools/testing/selftests/arm64/fp/TODO
@@ -1,4 +1,7 @@
 - Test unsupported values in the ABIs.
-- More coverage for ptrace (eg, vector length conversions).
-- Coverage for signals.
-- Test PR_SVE_VL_INHERITY after a double fork.
+- More coverage for ptrace:
+ - Get/set of FFR.
+ - Ensure ptraced processes actually see the register state visible through
+   the ptrace interface.
+ - Big endian.
+- Test PR_SVE_VL_INHERIT after a double fork.
diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 31a2c2fc529d..199710ba65c7 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -1,15 +1,17 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (C) 2015-2020 ARM Limited.
+ * Copyright (C) 2015-2021 ARM Limited.
  * Original author: Dave Martin <Dave.Martin@arm.com>
  */
 #include <errno.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <sys/auxv.h>
+#include <sys/prctl.h>
 #include <sys/ptrace.h>
 #include <sys/types.h>
 #include <sys/uio.h>
@@ -19,20 +21,22 @@
 
 #include "../../kselftest.h"
 
-#define EXPECTED_TESTS 19
+#define VL_TESTS (((SVE_VQ_MAX - SVE_VQ_MIN) + 1) * 3)
+#define FPSIMD_TESTS 3
+
+#define EXPECTED_TESTS (VL_TESTS + FPSIMD_TESTS)
 
 /* <linux/elf.h> and <sys/auxv.h> don't like each other, so: */
 #ifndef NT_ARM_SVE
 #define NT_ARM_SVE 0x405
 #endif
 
-static void dump(const void *buf, size_t size)
+static void fill_buf(char *buf, size_t size)
 {
-	size_t i;
-	const unsigned char *p = buf;
+	int i;
 
-	for (i = 0; i < size; ++i)
-		printf(" %.2x", *p++);
+	for (i = 0; i < size; i++)
+		buf[i] = random();
 }
 
 static int do_child(void)
@@ -101,25 +105,228 @@ static int set_sve(pid_t pid, const struct user_sve_header *sve)
 	return ptrace(PTRACE_SETREGSET, pid, NT_ARM_SVE, &iov);
 }
 
-static void dump_sve_regs(const struct user_sve_header *sve, unsigned int num,
-			  unsigned int vlmax)
+/* Validate attempting to set the specfied VL via ptrace */
+static void ptrace_set_get_vl(pid_t child, unsigned int vl, bool *supported)
+{
+	struct user_sve_header sve;
+	struct user_sve_header *new_sve = NULL;
+	size_t new_sve_size = 0;
+	int ret, prctl_vl;
+
+	*supported = false;
+
+	/* Check if the VL is supported in this process */
+	prctl_vl = prctl(PR_SVE_SET_VL, vl);
+	if (prctl_vl == -1)
+		ksft_exit_fail_msg("prctl(PR_SVE_SET_VL) failed: %s (%d)\n",
+				   strerror(errno), errno);
+
+	/* If the VL is not supported then a supported VL will be returned */
+	*supported = (prctl_vl == vl);
+
+	/* Set the VL by doing a set with no register payload */
+	memset(&sve, 0, sizeof(sve));
+	sve.size = sizeof(sve);
+	sve.vl = vl;
+	ret = set_sve(child, &sve);
+	if (ret != 0) {
+		ksft_test_result_fail("Failed to set VL %u\n", vl);
+		return;
+	}
+
+	/*
+	 * Read back the new register state and verify that we have the
+	 * same VL that we got from prctl() on ourselves.
+	 */
+	if (!get_sve(child, (void **)&new_sve, &new_sve_size)) {
+		ksft_test_result_fail("Failed to read VL %u\n", vl);
+		return;
+	}
+
+	ksft_test_result(new_sve->vl = prctl_vl, "Set VL %u\n", vl);
+
+	free(new_sve);
+}
+
+static void check_u32(unsigned int vl, const char *reg,
+		      uint32_t *in, uint32_t *out, int *errors)
+{
+	if (*in != *out) {
+		printf("# VL %d %s wrote %x read %x\n",
+		       vl, reg, *in, *out);
+		(*errors)++;
+	}
+}
+
+/* Validate attempting to set SVE data and read SVE data */
+static void ptrace_set_sve_get_sve_data(pid_t child, unsigned int vl)
+{
+	void *write_buf;
+	void *read_buf = NULL;
+	struct user_sve_header *write_sve;
+	struct user_sve_header *read_sve;
+	size_t read_sve_size = 0;
+	unsigned int vq = sve_vq_from_vl(vl);
+	int ret, i;
+	size_t data_size;
+	int errors = 0;
+
+	data_size = SVE_PT_SVE_OFFSET + SVE_PT_SVE_SIZE(vq, SVE_PT_REGS_SVE);
+	write_buf = malloc(data_size);
+	if (!write_buf) {
+		ksft_test_result_fail("Error allocating %d byte buffer for VL %u\n",
+				      data_size, vl);
+		return;
+	}
+	write_sve = write_buf;
+
+	/* Set up some data and write it out */
+	memset(write_sve, 0, data_size);
+	write_sve->size = data_size;
+	write_sve->vl = vl;
+	write_sve->flags = SVE_PT_REGS_SVE;
+
+	for (i = 0; i < __SVE_NUM_ZREGS; i++)
+		fill_buf(write_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+			 SVE_PT_SVE_ZREG_SIZE(vq));
+
+	for (i = 0; i < __SVE_NUM_PREGS; i++)
+		fill_buf(write_buf + SVE_PT_SVE_PREG_OFFSET(vq, i),
+			 SVE_PT_SVE_PREG_SIZE(vq));
+
+	fill_buf(write_buf + SVE_PT_SVE_FPSR_OFFSET(vq), SVE_PT_SVE_FPSR_SIZE);
+	fill_buf(write_buf + SVE_PT_SVE_FPCR_OFFSET(vq), SVE_PT_SVE_FPCR_SIZE);
+
+	/* TODO: Generate a valid FFR pattern */
+
+	ret = set_sve(child, write_sve);
+	if (ret != 0) {
+		ksft_test_result_fail("Failed to set VL %u data\n", vl);
+		goto out;
+	}
+
+	/* Read the data back */
+	if (!get_sve(child, (void **)&read_buf, &read_sve_size)) {
+		ksft_test_result_fail("Failed to read VL %u data\n", vl);
+		goto out;
+	}
+	read_sve = read_buf;
+
+	/* We might read more data if there's extensions we don't know */
+	if (read_sve->size < write_sve->size) {
+		ksft_test_result_fail("Wrote %d bytes, only read %d\n",
+				      write_sve->size, read_sve->size);
+		goto out_read;
+	}
+
+	for (i = 0; i < __SVE_NUM_ZREGS; i++) {
+		if (memcmp(write_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+			   read_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+			   SVE_PT_SVE_ZREG_SIZE(vq)) != 0) {
+			printf("# Mismatch in %u Z%d\n", vl, i);
+			errors++;
+		}
+	}
+
+	for (i = 0; i < __SVE_NUM_PREGS; i++) {
+		if (memcmp(write_buf + SVE_PT_SVE_PREG_OFFSET(vq, i),
+			   read_buf + SVE_PT_SVE_PREG_OFFSET(vq, i),
+			   SVE_PT_SVE_PREG_SIZE(vq)) != 0) {
+			printf("# Mismatch in %u P%d\n", vl, i);
+			errors++;
+		}
+	}
+
+	check_u32(vl, "FPSR", write_buf + SVE_PT_SVE_FPSR_OFFSET(vq),
+		  read_buf + SVE_PT_SVE_FPSR_OFFSET(vq), &errors);
+	check_u32(vl, "FPCR", write_buf + SVE_PT_SVE_FPCR_OFFSET(vq),
+		  read_buf + SVE_PT_SVE_FPCR_OFFSET(vq), &errors);
+
+	ksft_test_result(errors == 0, "Set and get SVE data for VL %u\n", vl);
+
+out_read:
+	free(read_buf);
+out:
+	free(write_buf);
+}
+
+/* Validate attempting to set SVE data and read SVE data */
+static void ptrace_set_sve_get_fpsimd_data(pid_t child, unsigned int vl)
 {
-	unsigned int vq;
-	unsigned int i;
+	void *write_buf;
+	struct user_sve_header *write_sve;
+	unsigned int vq = sve_vq_from_vl(vl);
+	struct user_fpsimd_state fpsimd_state;
+	int ret, i;
+	size_t data_size;
+	int errors = 0;
+
+	if (__BYTE_ORDER == __BIG_ENDIAN) {
+		ksft_test_result_skip("Big endian not supported\n");
+		return;
+	}
+
+	data_size = SVE_PT_SVE_OFFSET + SVE_PT_SVE_SIZE(vq, SVE_PT_REGS_SVE);
+	write_buf = malloc(data_size);
+	if (!write_buf) {
+		ksft_test_result_fail("Error allocating %d byte buffer for VL %u\n",
+				      data_size, vl);
+		return;
+	}
+	write_sve = write_buf;
 
-	if ((sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_SVE)
-		ksft_exit_fail_msg("Dumping non-SVE register\n");
+	/* Set up some data and write it out */
+	memset(write_sve, 0, data_size);
+	write_sve->size = data_size;
+	write_sve->vl = vl;
+	write_sve->flags = SVE_PT_REGS_SVE;
 
-	if (vlmax > sve->vl)
-		vlmax = sve->vl;
+	for (i = 0; i < __SVE_NUM_ZREGS; i++)
+		fill_buf(write_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+			 SVE_PT_SVE_ZREG_SIZE(vq));
 
-	vq = sve_vq_from_vl(sve->vl);
-	for (i = 0; i < num; ++i) {
-		printf("# z%u:", i);
-		dump((const char *)sve + SVE_PT_SVE_ZREG_OFFSET(vq, i),
-		     vlmax);
-		printf("%s\n", vlmax == sve->vl ? "" : " ...");
+	fill_buf(write_buf + SVE_PT_SVE_FPSR_OFFSET(vq), SVE_PT_SVE_FPSR_SIZE);
+	fill_buf(write_buf + SVE_PT_SVE_FPCR_OFFSET(vq), SVE_PT_SVE_FPCR_SIZE);
+
+	ret = set_sve(child, write_sve);
+	if (ret != 0) {
+		ksft_test_result_fail("Failed to set VL %u data\n", vl);
+		goto out;
+	}
+
+	/* Read the data back */
+	if (get_fpsimd(child, &fpsimd_state)) {
+		ksft_test_result_fail("Failed to read VL %u FPSIMD data\n",
+				      vl);
+		goto out;
+	}
+
+	for (i = 0; i < __SVE_NUM_ZREGS; i++) {
+		__uint128_t tmp = 0;
+
+		/*
+		 * Z regs are stored endianness invariant, this won't
+		 * work for big endian
+		 */
+		memcpy(&tmp, write_buf + SVE_PT_SVE_ZREG_OFFSET(vq, i),
+		       sizeof(tmp));
+
+		if (tmp != fpsimd_state.vregs[i]) {
+			printf("# Mismatch in FPSIMD for VL %u Z%d\n", vl, i);
+			errors++;
+		}
 	}
+
+	check_u32(vl, "FPSR", write_buf + SVE_PT_SVE_FPSR_OFFSET(vq),
+		  &fpsimd_state.fpsr, &errors);
+	check_u32(vl, "FPCR", write_buf + SVE_PT_SVE_FPCR_OFFSET(vq),
+		  &fpsimd_state.fpcr, &errors);
+
+	ksft_test_result(errors == 0, "Set and get FPSIMD data for VL %u\n",
+			 vl);
+
+out:
+	free(write_buf);
 }
 
 static int do_parent(pid_t child)
@@ -128,13 +335,14 @@ static int do_parent(pid_t child)
 	pid_t pid;
 	int status;
 	siginfo_t si;
-	void *svebuf = NULL, *newsvebuf;
-	size_t svebufsz = 0, newsvebufsz;
-	struct user_sve_header *sve, *new_sve;
+	void *svebuf = NULL;
+	size_t svebufsz = 0;
+	struct user_sve_header *sve;
 	struct user_fpsimd_state *fpsimd, new_fpsimd;
 	unsigned int i, j;
 	unsigned char *p;
-	unsigned int vq;
+	unsigned int vq, vl;
+	bool vl_supported;
 
 	/* Attach to the child */
 	while (1) {
@@ -246,62 +454,21 @@ static int do_parent(pid_t child)
 	else
 		ksft_test_result_fail("get_fpsimd() gave different state\n");
 
-	vq = sve_vq_from_vl(sve->vl);
-
-	newsvebufsz = SVE_PT_SVE_ZREG_OFFSET(vq, 1);
-	new_sve = newsvebuf = malloc(newsvebufsz);
-	if (!new_sve) {
-		errno = ENOMEM;
-		perror(NULL);
-		goto error;
-	}
-
-	*new_sve = *sve;
-	new_sve->flags &= ~SVE_PT_REGS_MASK;
-	new_sve->flags |= SVE_PT_REGS_SVE;
-	memset((char *)new_sve + SVE_PT_SVE_ZREG_OFFSET(vq, 0),
-	       0, SVE_PT_SVE_ZREG_SIZE(vq));
-	new_sve->size = SVE_PT_SVE_ZREG_OFFSET(vq, 1);
-	if (set_sve(pid, new_sve)) {
-		int e = errno;
-
-		ksft_test_result_fail("set_sve(ZREG): %s\n", strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
-
-		goto error;
-	}
-
-	/* Try to read back the value we just set */
-	new_sve = get_sve(pid, &newsvebuf, &newsvebufsz);
-	if (!new_sve) {
-		int e = errno;
-
-		ksft_test_result_fail("get_sve(ZREG): %s\n", strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
+	/* Step through every possible VQ */
+	for (vq = SVE_VQ_MIN; vq <= SVE_VQ_MAX; vq++) {
+		vl = sve_vl_from_vq(vq);
 
-		goto error;
-	}
-
-	ksft_test_result((new_sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_SVE,
-			 "Get SVE registers\n");
-	if ((new_sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_SVE)
-		goto error;
-
-	dump_sve_regs(new_sve, 3, sizeof fpsimd->vregs[0]);
+		/* First, try to set this vector length */
+		ptrace_set_get_vl(child, vl, &vl_supported);
 
-	/* Verify that the register we set has the value we expected */
-	p = (unsigned char *)new_sve + SVE_PT_SVE_ZREG_OFFSET(vq, 1);
-	for (i = 0; i < sizeof fpsimd->vregs[0]; ++i) {
-		unsigned char expected = i;
-
-		if (__BYTE_ORDER == __BIG_ENDIAN)
-			expected = sizeof fpsimd->vregs[0] - 1 - expected;
-
-		ksft_test_result(p[i] == expected, "buf[%d] == expected\n", i);
-		if (p[i] != expected)
-			goto error;
+		/* If the VL is supported validate data set/get */
+		if (vl_supported) {
+			ptrace_set_sve_get_sve_data(child, vl);
+			ptrace_set_sve_get_fpsimd_data(child, vl);
+		} else {
+			ksft_test_result_skip("set SVE get SVE for VL %d\n", vl);
+			ksft_test_result_skip("set SVE get FPSIMD for VL %d\n", vl);
+		}
 	}
 
 	ret = EXIT_SUCCESS;
@@ -318,6 +485,8 @@ int main(void)
 	int ret = EXIT_SUCCESS;
 	pid_t child;
 
+	srandom(getpid());
+
 	ksft_print_header();
 	ksft_set_plan(EXPECTED_TESTS);
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 8/8] selftests: arm64: Move FPSIMD in SVE ptrace test into a function
  2021-09-13 12:54 ` Mark Brown
@ 2021-09-13 12:55   ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Now that all the other tests are in functions rather than inline in the
main parent process function also move the test for accessing the FPSIMD
registers via the SVE regset out into their own function.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 120 +++++++++---------
 1 file changed, 59 insertions(+), 61 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 199710ba65c7..ac0629f05365 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -158,6 +158,63 @@ static void check_u32(unsigned int vl, const char *reg,
 	}
 }
 
+/* Access the FPSIMD registers via the SVE regset */
+static void ptrace_sve_fpsimd(pid_t child)
+{
+	void *svebuf = NULL;
+	size_t svebufsz = 0;
+	struct user_sve_header *sve;
+	struct user_fpsimd_state *fpsimd, new_fpsimd;
+	unsigned int i, j;
+	unsigned char *p;
+
+	/* New process should start with FPSIMD registers only */
+	sve = get_sve(child, &svebuf, &svebufsz);
+	if (!sve) {
+		ksft_test_result_fail("get_sve: %s\n", strerror(errno));
+
+		return;
+	} else {
+		ksft_test_result_pass("get_sve(FPSIMD)\n");
+	}
+
+	ksft_test_result((sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_FPSIMD,
+			 "Set FPSIMD registers\n");
+	if ((sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_FPSIMD)
+		goto out;
+
+	/* Try to set a known FPSIMD state via PT_REGS_SVE */
+	fpsimd = (struct user_fpsimd_state *)((char *)sve +
+					      SVE_PT_FPSIMD_OFFSET);
+	for (i = 0; i < 32; ++i) {
+		p = (unsigned char *)&fpsimd->vregs[i];
+
+		for (j = 0; j < sizeof(fpsimd->vregs[i]); ++j)
+			p[j] = j;
+	}
+
+	if (set_sve(child, sve)) {
+		ksft_test_result_fail("set_sve(FPSIMD): %s\n",
+				      strerror(errno));
+
+		goto out;
+	}
+
+	/* Verify via the FPSIMD regset */
+	if (get_fpsimd(child, &new_fpsimd)) {
+		ksft_test_result_fail("get_fpsimd(): %s\n",
+				      strerror(errno));
+		goto out;
+	}
+	if (memcmp(fpsimd, &new_fpsimd, sizeof(*fpsimd)) == 0)
+		ksft_test_result_pass("get_fpsimd() gave same state\n");
+	else
+		ksft_test_result_fail("get_fpsimd() gave different state\n");
+
+out:
+	free(svebuf);
+}
+
 /* Validate attempting to set SVE data and read SVE data */
 static void ptrace_set_sve_get_sve_data(pid_t child, unsigned int vl)
 {
@@ -335,12 +392,6 @@ static int do_parent(pid_t child)
 	pid_t pid;
 	int status;
 	siginfo_t si;
-	void *svebuf = NULL;
-	size_t svebufsz = 0;
-	struct user_sve_header *sve;
-	struct user_fpsimd_state *fpsimd, new_fpsimd;
-	unsigned int i, j;
-	unsigned char *p;
 	unsigned int vq, vl;
 	bool vl_supported;
 
@@ -398,61 +449,8 @@ static int do_parent(pid_t child)
 		}
 	}
 
-	/* New process should start with FPSIMD registers only */
-	sve = get_sve(pid, &svebuf, &svebufsz);
-	if (!sve) {
-		int e = errno;
-
-		ksft_test_result_fail("get_sve: %s\n", strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
-
-		goto error;
-	} else {
-		ksft_test_result_pass("get_sve(FPSIMD)\n");
-	}
-
-	ksft_test_result((sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_FPSIMD,
-			 "Set FPSIMD registers\n");
-	if ((sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_FPSIMD)
-		goto error;
-
-	/* Try to set a known FPSIMD state via PT_REGS_SVE */
-	fpsimd = (struct user_fpsimd_state *)((char *)sve +
-					      SVE_PT_FPSIMD_OFFSET);
-	for (i = 0; i < 32; ++i) {
-		p = (unsigned char *)&fpsimd->vregs[i];
-
-		for (j = 0; j < sizeof fpsimd->vregs[i]; ++j)
-			p[j] = j;
-	}
-
-	if (set_sve(pid, sve)) {
-		int e = errno;
-
-		ksft_test_result_fail("set_sve(FPSIMD): %s\n",
-				      strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
-
-		goto error;
-	}
-
-	/* Verify via the FPSIMD regset */
-	if (get_fpsimd(pid, &new_fpsimd)) {
-		int e = errno;
-
-		ksft_test_result_fail("get_fpsimd(): %s\n",
-				      strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
-
-		goto error;
-	}
-	if (memcmp(fpsimd, &new_fpsimd, sizeof(*fpsimd)) == 0)
-		ksft_test_result_pass("get_fpsimd() gave same state\n");
-	else
-		ksft_test_result_fail("get_fpsimd() gave different state\n");
+	/* FPSIMD via SVE regset */
+	ptrace_sve_fpsimd(child);
 
 	/* Step through every possible VQ */
 	for (vq = SVE_VQ_MIN; vq <= SVE_VQ_MAX; vq++) {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v1 8/8] selftests: arm64: Move FPSIMD in SVE ptrace test into a function
@ 2021-09-13 12:55   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-09-13 12:55 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan, Shuah Khan
  Cc: linux-arm-kernel, linux-kselftest, Mark Brown

Now that all the other tests are in functions rather than inline in the
main parent process function also move the test for accessing the FPSIMD
registers via the SVE regset out into their own function.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/fp/sve-ptrace.c | 120 +++++++++---------
 1 file changed, 59 insertions(+), 61 deletions(-)

diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
index 199710ba65c7..ac0629f05365 100644
--- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
+++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
@@ -158,6 +158,63 @@ static void check_u32(unsigned int vl, const char *reg,
 	}
 }
 
+/* Access the FPSIMD registers via the SVE regset */
+static void ptrace_sve_fpsimd(pid_t child)
+{
+	void *svebuf = NULL;
+	size_t svebufsz = 0;
+	struct user_sve_header *sve;
+	struct user_fpsimd_state *fpsimd, new_fpsimd;
+	unsigned int i, j;
+	unsigned char *p;
+
+	/* New process should start with FPSIMD registers only */
+	sve = get_sve(child, &svebuf, &svebufsz);
+	if (!sve) {
+		ksft_test_result_fail("get_sve: %s\n", strerror(errno));
+
+		return;
+	} else {
+		ksft_test_result_pass("get_sve(FPSIMD)\n");
+	}
+
+	ksft_test_result((sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_FPSIMD,
+			 "Set FPSIMD registers\n");
+	if ((sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_FPSIMD)
+		goto out;
+
+	/* Try to set a known FPSIMD state via PT_REGS_SVE */
+	fpsimd = (struct user_fpsimd_state *)((char *)sve +
+					      SVE_PT_FPSIMD_OFFSET);
+	for (i = 0; i < 32; ++i) {
+		p = (unsigned char *)&fpsimd->vregs[i];
+
+		for (j = 0; j < sizeof(fpsimd->vregs[i]); ++j)
+			p[j] = j;
+	}
+
+	if (set_sve(child, sve)) {
+		ksft_test_result_fail("set_sve(FPSIMD): %s\n",
+				      strerror(errno));
+
+		goto out;
+	}
+
+	/* Verify via the FPSIMD regset */
+	if (get_fpsimd(child, &new_fpsimd)) {
+		ksft_test_result_fail("get_fpsimd(): %s\n",
+				      strerror(errno));
+		goto out;
+	}
+	if (memcmp(fpsimd, &new_fpsimd, sizeof(*fpsimd)) == 0)
+		ksft_test_result_pass("get_fpsimd() gave same state\n");
+	else
+		ksft_test_result_fail("get_fpsimd() gave different state\n");
+
+out:
+	free(svebuf);
+}
+
 /* Validate attempting to set SVE data and read SVE data */
 static void ptrace_set_sve_get_sve_data(pid_t child, unsigned int vl)
 {
@@ -335,12 +392,6 @@ static int do_parent(pid_t child)
 	pid_t pid;
 	int status;
 	siginfo_t si;
-	void *svebuf = NULL;
-	size_t svebufsz = 0;
-	struct user_sve_header *sve;
-	struct user_fpsimd_state *fpsimd, new_fpsimd;
-	unsigned int i, j;
-	unsigned char *p;
 	unsigned int vq, vl;
 	bool vl_supported;
 
@@ -398,61 +449,8 @@ static int do_parent(pid_t child)
 		}
 	}
 
-	/* New process should start with FPSIMD registers only */
-	sve = get_sve(pid, &svebuf, &svebufsz);
-	if (!sve) {
-		int e = errno;
-
-		ksft_test_result_fail("get_sve: %s\n", strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
-
-		goto error;
-	} else {
-		ksft_test_result_pass("get_sve(FPSIMD)\n");
-	}
-
-	ksft_test_result((sve->flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_FPSIMD,
-			 "Set FPSIMD registers\n");
-	if ((sve->flags & SVE_PT_REGS_MASK) != SVE_PT_REGS_FPSIMD)
-		goto error;
-
-	/* Try to set a known FPSIMD state via PT_REGS_SVE */
-	fpsimd = (struct user_fpsimd_state *)((char *)sve +
-					      SVE_PT_FPSIMD_OFFSET);
-	for (i = 0; i < 32; ++i) {
-		p = (unsigned char *)&fpsimd->vregs[i];
-
-		for (j = 0; j < sizeof fpsimd->vregs[i]; ++j)
-			p[j] = j;
-	}
-
-	if (set_sve(pid, sve)) {
-		int e = errno;
-
-		ksft_test_result_fail("set_sve(FPSIMD): %s\n",
-				      strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
-
-		goto error;
-	}
-
-	/* Verify via the FPSIMD regset */
-	if (get_fpsimd(pid, &new_fpsimd)) {
-		int e = errno;
-
-		ksft_test_result_fail("get_fpsimd(): %s\n",
-				      strerror(errno));
-		if (e == ESRCH)
-			goto disappeared;
-
-		goto error;
-	}
-	if (memcmp(fpsimd, &new_fpsimd, sizeof(*fpsimd)) == 0)
-		ksft_test_result_pass("get_fpsimd() gave same state\n");
-	else
-		ksft_test_result_fail("get_fpsimd() gave different state\n");
+	/* FPSIMD via SVE regset */
+	ptrace_sve_fpsimd(child);
 
 	/* Step through every possible VQ */
 	for (vq = SVE_VQ_MIN; vq <= SVE_VQ_MAX; vq++) {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH v1 0/8] selftests: arm64: SVE ptrace test rework
  2021-09-13 12:54 ` Mark Brown
@ 2021-09-29 17:48   ` Will Deacon
  -1 siblings, 0 replies; 20+ messages in thread
From: Will Deacon @ 2021-09-29 17:48 UTC (permalink / raw)
  To: Catalin Marinas, Mark Brown, Shuah Khan, Shuah Khan
  Cc: kernel-team, Will Deacon, linux-arm-kernel, linux-kselftest

On Mon, 13 Sep 2021 13:54:57 +0100, Mark Brown wrote:
> This series overhauls the selftests we have for the SVE ptrace interface
> to make them much more comprehensive than they are currently, making the
> coverage of the data read and written more complete.  The new coverage
> for setting data on all vector lengths showed the issue with using the
> wrong buffer size with ptrace reported and fixed by:
> 
>   https://lore.kernel.org/linux-arm-kernel/20210909165356.10675-1-broonie@kernel.org/
> 
> [...]

Applied to arm64 (for-next/kselftest), thanks!

[1/8] selftests: arm64: Use a define for the number of SVE ptrace tests to be run
      https://git.kernel.org/arm64/c/78d2d816c45a
[2/8] selftests: arm64: Don't log child creation as a test in SVE ptrace test
      https://git.kernel.org/arm64/c/09121ad7186e
[3/8] selftests: arm64: Remove extraneous register setting code
      https://git.kernel.org/arm64/c/eab281e3afa6
[4/8] selftests: arm64: Document what the SVE ptrace test is doing
      https://git.kernel.org/arm64/c/736e6d5a5451
[5/8] selftests: arm64: Clarify output when verifying SVE register set
      https://git.kernel.org/arm64/c/8c9eece0bfbf
[6/8] selftests: arm64: Verify interoperation of SVE and FPSIMD register sets
      https://git.kernel.org/arm64/c/9f7d03a2c5a1
[7/8] selftests: arm64: More comprehensively test the SVE ptrace interface
      https://git.kernel.org/arm64/c/a1d7111257cd
[8/8] selftests: arm64: Move FPSIMD in SVE ptrace test into a function
      https://git.kernel.org/arm64/c/34785030dc06

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v1 0/8] selftests: arm64: SVE ptrace test rework
@ 2021-09-29 17:48   ` Will Deacon
  0 siblings, 0 replies; 20+ messages in thread
From: Will Deacon @ 2021-09-29 17:48 UTC (permalink / raw)
  To: Catalin Marinas, Mark Brown, Shuah Khan, Shuah Khan
  Cc: kernel-team, Will Deacon, linux-arm-kernel, linux-kselftest

On Mon, 13 Sep 2021 13:54:57 +0100, Mark Brown wrote:
> This series overhauls the selftests we have for the SVE ptrace interface
> to make them much more comprehensive than they are currently, making the
> coverage of the data read and written more complete.  The new coverage
> for setting data on all vector lengths showed the issue with using the
> wrong buffer size with ptrace reported and fixed by:
> 
>   https://lore.kernel.org/linux-arm-kernel/20210909165356.10675-1-broonie@kernel.org/
> 
> [...]

Applied to arm64 (for-next/kselftest), thanks!

[1/8] selftests: arm64: Use a define for the number of SVE ptrace tests to be run
      https://git.kernel.org/arm64/c/78d2d816c45a
[2/8] selftests: arm64: Don't log child creation as a test in SVE ptrace test
      https://git.kernel.org/arm64/c/09121ad7186e
[3/8] selftests: arm64: Remove extraneous register setting code
      https://git.kernel.org/arm64/c/eab281e3afa6
[4/8] selftests: arm64: Document what the SVE ptrace test is doing
      https://git.kernel.org/arm64/c/736e6d5a5451
[5/8] selftests: arm64: Clarify output when verifying SVE register set
      https://git.kernel.org/arm64/c/8c9eece0bfbf
[6/8] selftests: arm64: Verify interoperation of SVE and FPSIMD register sets
      https://git.kernel.org/arm64/c/9f7d03a2c5a1
[7/8] selftests: arm64: More comprehensively test the SVE ptrace interface
      https://git.kernel.org/arm64/c/a1d7111257cd
[8/8] selftests: arm64: Move FPSIMD in SVE ptrace test into a function
      https://git.kernel.org/arm64/c/34785030dc06

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-09-29 17:51 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 12:54 [PATCH v1 0/8] selftests: arm64: SVE ptrace test rework Mark Brown
2021-09-13 12:54 ` Mark Brown
2021-09-13 12:54 ` [PATCH v1 1/8] selftests: arm64: Use a define for the number of SVE ptrace tests to be run Mark Brown
2021-09-13 12:54   ` Mark Brown
2021-09-13 12:54 ` [PATCH v1 2/8] selftests: arm64: Don't log child creation as a test in SVE ptrace test Mark Brown
2021-09-13 12:54   ` Mark Brown
2021-09-13 12:55 ` [PATCH v1 3/8] selftests: arm64: Remove extraneous register setting code Mark Brown
2021-09-13 12:55   ` Mark Brown
2021-09-13 12:55 ` [PATCH v1 4/8] selftests: arm64: Document what the SVE ptrace test is doing Mark Brown
2021-09-13 12:55   ` Mark Brown
2021-09-13 12:55 ` [PATCH v1 5/8] selftests: arm64: Clarify output when verifying SVE register set Mark Brown
2021-09-13 12:55   ` Mark Brown
2021-09-13 12:55 ` [PATCH v1 6/8] selftests: arm64: Verify interoperation of SVE and FPSIMD register sets Mark Brown
2021-09-13 12:55   ` Mark Brown
2021-09-13 12:55 ` [PATCH v1 7/8] selftests: arm64: More comprehensively test the SVE ptrace interface Mark Brown
2021-09-13 12:55   ` Mark Brown
2021-09-13 12:55 ` [PATCH v1 8/8] selftests: arm64: Move FPSIMD in SVE ptrace test into a function Mark Brown
2021-09-13 12:55   ` Mark Brown
2021-09-29 17:48 ` [PATCH v1 0/8] selftests: arm64: SVE ptrace test rework Will Deacon
2021-09-29 17:48   ` Will Deacon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.