Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH kselftest 0/3] getrandom & chacha cleanups
       [not found] <Zv20olVBlnxL9UnS@zx2c4.com>
@ 2024-10-03  3:13 ` Jason A. Donenfeld
  2024-10-03  3:13   ` [PATCH kselftest 1/3] selftests: vDSO: condition chacha build on chacha implementation Jason A. Donenfeld
                     ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-03  3:13 UTC (permalink / raw)
  To: skhan, greg, linux-kselftest; +Cc: Jason A. Donenfeld

Hi Shuah,

I've now read your email several times trying to figure out what you
meant and what your objections are. This series is my best attempt at
trying to satisfy that. But my understanding still has a lot of question
marks, so I may have missed your point here. Nonetheless, maybe this
moves things forward a bit.

Jason

Cc: Shuah Khan <skhan@linuxfoundation.org>
Cc: Greg KH <greg@kroah.com>

Jason A. Donenfeld (3):
  selftests: vDSO: condition chacha build on chacha implementation
  selftests: vDSO: unconditionally build getrandom test
  selftests: vDSO: improve getrandom and chacha error messages

 tools/testing/selftests/vDSO/Makefile         |  4 +-
 .../testing/selftests/vDSO/vdso_test_chacha.c | 27 ++++---
 .../selftests/vDSO/vdso_test_getrandom.c      | 75 ++++++++-----------
 3 files changed, 52 insertions(+), 54 deletions(-)

-- 
2.46.0


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

* [PATCH kselftest 1/3] selftests: vDSO: condition chacha build on chacha implementation
  2024-10-03  3:13 ` [PATCH kselftest 0/3] getrandom & chacha cleanups Jason A. Donenfeld
@ 2024-10-03  3:13   ` Jason A. Donenfeld
  2024-10-03  3:13   ` [PATCH kselftest 2/3] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-03  3:13 UTC (permalink / raw)
  To: skhan, greg, linux-kselftest; +Cc: Jason A. Donenfeld

The chacha test can build anywhere that the vgetrandom-chacha.S file is
available, so condition it on the existence of that file, rather than a
specific arch.

There is one wrinkle, which is that x86 and x86_64 are the same ARCH,
but the code is only functional for x86_64. So filter out the test for
CONFIG_X86_32 in the same block that the other 32-bit special case
lives.

We have to define top_srcdir ourselves, because even though lib.mk
defines it later, that has to be included after TEST_GEN_PROGS is
populated. Fortunately the definition is fairly trivial.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/testing/selftests/vDSO/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index af9cedbf5357..2c38c9c6d056 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 include ../../../scripts/Makefile.arch
+top_srcdir = $(realpath ../../../..)
 
 TEST_GEN_PROGS := vdso_test_gettimeofday
 TEST_GEN_PROGS += vdso_test_getcpu
@@ -11,6 +12,8 @@ endif
 TEST_GEN_PROGS += vdso_test_correctness
 ifeq ($(ARCH)$(CONFIG_X86_32),$(filter $(ARCH)$(CONFIG_X86_32),x86 x86_64 loongarch arm64 powerpc s390))
 TEST_GEN_PROGS += vdso_test_getrandom
+endif
+ifneq ($(wildcard $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S),)
 TEST_GEN_PROGS += vdso_test_chacha
 endif
 
@@ -18,6 +21,7 @@ CFLAGS := -std=gnu99 -O2
 
 ifeq ($(CONFIG_X86_32),y)
 LDLIBS += -lgcc_s
+TEST_GEN_PROGS := $(filter-out vdso_test_chacha,$(TEST_GEN_PROGS))
 endif
 
 include ../lib.mk
-- 
2.46.0


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

* [PATCH kselftest 2/3] selftests: vDSO: unconditionally build getrandom test
  2024-10-03  3:13 ` [PATCH kselftest 0/3] getrandom & chacha cleanups Jason A. Donenfeld
  2024-10-03  3:13   ` [PATCH kselftest 1/3] selftests: vDSO: condition chacha build on chacha implementation Jason A. Donenfeld
@ 2024-10-03  3:13   ` Jason A. Donenfeld
  2024-10-03  3:13   ` [PATCH kselftest 3/3] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-03  3:13 UTC (permalink / raw)
  To: skhan, greg, linux-kselftest; +Cc: Jason A. Donenfeld

Rather than building on supported archs, build on all archs, and then
use the presence of the symbol in the vDSO to either skip the test or
move forward with it.

Note that this means that this test no longer checks whether the symbol
was correctly added to the kernel. But hopefully this will be clear
enough to developers and we'll cross our fingers that symbols aren't
removed by accident and not caught after this change.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/testing/selftests/vDSO/Makefile              | 2 --
 tools/testing/selftests/vDSO/vdso_test_getrandom.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 2c38c9c6d056..96b4f2a766e2 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -10,9 +10,7 @@ ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
 TEST_GEN_PROGS += vdso_standalone_test_x86
 endif
 TEST_GEN_PROGS += vdso_test_correctness
-ifeq ($(ARCH)$(CONFIG_X86_32),$(filter $(ARCH)$(CONFIG_X86_32),x86 x86_64 loongarch arm64 powerpc s390))
 TEST_GEN_PROGS += vdso_test_getrandom
-endif
 ifneq ($(wildcard $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S),)
 TEST_GEN_PROGS += vdso_test_chacha
 endif
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
index 72a1d9b43a84..5489a2f07434 100644
--- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
+++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
@@ -119,7 +119,7 @@ static void vgetrandom_init(void)
 	vgrnd.fn = (__typeof__(vgrnd.fn))vdso_sym(version, name);
 	if (!vgrnd.fn) {
 		printf("%s is missing!\n", name);
-		exit(KSFT_FAIL);
+		exit(KSFT_SKIP);
 	}
 	ret = VDSO_CALL(vgrnd.fn, 5, NULL, 0, 0, &vgrnd.params, ~0UL);
 	if (ret == -ENOSYS) {
-- 
2.46.0


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

* [PATCH kselftest 3/3] selftests: vDSO: improve getrandom and chacha error messages
  2024-10-03  3:13 ` [PATCH kselftest 0/3] getrandom & chacha cleanups Jason A. Donenfeld
  2024-10-03  3:13   ` [PATCH kselftest 1/3] selftests: vDSO: condition chacha build on chacha implementation Jason A. Donenfeld
  2024-10-03  3:13   ` [PATCH kselftest 2/3] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
@ 2024-10-03  3:13   ` Jason A. Donenfeld
  2024-10-06  3:35     ` kernel test robot
  2024-10-03 17:18   ` [PATCH kselftest 4/3] " Jason A. Donenfeld
  2024-10-03 22:42   ` [PATCH kselftest 0/3] getrandom & chacha cleanups Shuah Khan
  4 siblings, 1 reply; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-03  3:13 UTC (permalink / raw)
  To: skhan, greg, linux-kselftest; +Cc: Jason A. Donenfeld

Improve the error and skip condition messages to let the developer know
precisely where a test has failed. Also make better use of the ksft api
for this.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 .../testing/selftests/vDSO/vdso_test_chacha.c | 27 ++++---
 .../selftests/vDSO/vdso_test_getrandom.c      | 75 ++++++++-----------
 2 files changed, 49 insertions(+), 53 deletions(-)

diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c
index b1ea532c5996..60c5ea5cb925 100644
--- a/tools/testing/selftests/vDSO/vdso_test_chacha.c
+++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
@@ -90,10 +90,8 @@ int main(int argc, char *argv[])
 	ksft_set_plan(1);
 
 	for (unsigned int trial = 0; trial < TRIALS; ++trial) {
-		if (getrandom(key, sizeof(key), 0) != sizeof(key)) {
-			printf("getrandom() failed!\n");
-			return KSFT_SKIP;
-		}
+		if (getrandom(key, sizeof(key), 0) != sizeof(key))
+			ksft_exit_skip("getrandom() failed unexpectedly\n");
 		memset(counter1, 0, sizeof(counter1));
 		reference_chacha20_blocks(output1, key, counter1, BLOCKS);
 		for (unsigned int split = 0; split < BLOCKS; ++split) {
@@ -102,8 +100,10 @@ int main(int argc, char *argv[])
 			if (split)
 				__arch_chacha20_blocks_nostack(output2, key, counter2, split);
 			__arch_chacha20_blocks_nostack(output2 + split * BLOCK_SIZE, key, counter2, BLOCKS - split);
-			if (memcmp(output1, output2, sizeof(output1)) || memcmp(counter1, counter2, sizeof(counter1)))
-				return KSFT_FAIL;
+			if (memcmp(output1, output2, sizeof(output1)))
+				ksft_exit_fail_msg("Main loop outputs do not match on trial %u, split %u\n", trial, split);
+			if (memcmp(counter1, counter2, sizeof(counter1)))
+				ksft_exit_fail_msg("Main loop counters do not match on trial %u, split %u\n", trial, split);
 		}
 	}
 	memset(counter1, 0, sizeof(counter1));
@@ -113,14 +113,19 @@ int main(int argc, char *argv[])
 
 	reference_chacha20_blocks(output1, key, counter1, BLOCKS);
 	__arch_chacha20_blocks_nostack(output2, key, counter2, BLOCKS);
-	if (memcmp(output1, output2, sizeof(output1)) || memcmp(counter1, counter2, sizeof(counter1)))
-		return KSFT_FAIL;
+	if (memcmp(output1, output2, sizeof(output1)))
+		ksft_exit_fail_msg("Block limit outputs do not match after first round\n");
+	if (memcmp(counter1, counter2, sizeof(counter1)))
+		ksft_exit_fail_msg("Block limit counters do not match after first round\n");
 
 	reference_chacha20_blocks(output1, key, counter1, BLOCKS);
 	__arch_chacha20_blocks_nostack(output2, key, counter2, BLOCKS);
-	if (memcmp(output1, output2, sizeof(output1)) || memcmp(counter1, counter2, sizeof(counter1)))
-		return KSFT_FAIL;
+	if (memcmp(output1, output2, sizeof(output1)))
+		ksft_exit_fail_msg("Block limit outputs do not match after second round\n");
+	if (memcmp(counter1, counter2, sizeof(counter1)))
+		ksft_exit_fail_msg("Block limit counters do not match after second round\n");
 
 	ksft_test_result_pass("chacha: PASS\n");
-	return KSFT_PASS;
+	ksft_exit_pass();
+	return 0;
 }
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
index 5489a2f07434..f6f7054900ab 100644
--- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
+++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
@@ -40,6 +40,9 @@
 	} while (0)
 #endif
 
+#define ksft_assert(condition) \
+	if (!(condition)) ksft_exit_fail_msg("Assertion failed: %s\n", #condition)
+
 static struct {
 	pthread_mutex_t lock;
 	void **states;
@@ -109,26 +112,19 @@ static void vgetrandom_init(void)
 	const char *version = versions[VDSO_VERSION];
 	const char *name = names[VDSO_NAMES][6];
 	unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
-	size_t ret;
+	ssize_t ret;
 
-	if (!sysinfo_ehdr) {
-		printf("AT_SYSINFO_EHDR is not present!\n");
-		exit(KSFT_SKIP);
-	}
+	if (!sysinfo_ehdr)
+		ksft_exit_skip("AT_SYSINFO_EHDR is not present\n");
 	vdso_init_from_sysinfo_ehdr(sysinfo_ehdr);
 	vgrnd.fn = (__typeof__(vgrnd.fn))vdso_sym(version, name);
-	if (!vgrnd.fn) {
-		printf("%s is missing!\n", name);
-		exit(KSFT_SKIP);
-	}
+	if (!vgrnd.fn)
+		ksft_exit_skip("%s@%s symbol is missing from vDSO\n", name, version);
 	ret = VDSO_CALL(vgrnd.fn, 5, NULL, 0, 0, &vgrnd.params, ~0UL);
-	if (ret == -ENOSYS) {
-		printf("unsupported architecture\n");
-		exit(KSFT_SKIP);
-	} else if (ret) {
-		printf("failed to fetch vgetrandom params!\n");
-		exit(KSFT_FAIL);
-	}
+	if (ret == -ENOSYS)
+		ksft_exit_skip("CPU does not have runtime support\n");
+	else if (ret)
+		ksft_exit_fail_msg("Failed to fetch vgetrandom params: %zd\n", ret);
 }
 
 static ssize_t vgetrandom(void *buf, size_t len, unsigned long flags)
@@ -137,10 +133,7 @@ static ssize_t vgetrandom(void *buf, size_t len, unsigned long flags)
 
 	if (!state) {
 		state = vgetrandom_get_state();
-		if (!state) {
-			printf("vgetrandom_get_state failed!\n");
-			exit(KSFT_FAIL);
-		}
+		ksft_assert(state);
 	}
 	return VDSO_CALL(vgrnd.fn, 5, buf, len, flags, state, vgrnd.params.size_of_opaque_state);
 }
@@ -152,7 +145,7 @@ static void *test_vdso_getrandom(void *ctx)
 	for (size_t i = 0; i < TRIALS; ++i) {
 		unsigned int val;
 		ssize_t ret = vgetrandom(&val, sizeof(val), 0);
-		assert(ret == sizeof(val));
+		ksft_assert(ret == sizeof(val));
 	}
 	return NULL;
 }
@@ -162,7 +155,7 @@ static void *test_libc_getrandom(void *ctx)
 	for (size_t i = 0; i < TRIALS; ++i) {
 		unsigned int val;
 		ssize_t ret = getrandom(&val, sizeof(val), 0);
-		assert(ret == sizeof(val));
+		ksft_assert(ret == sizeof(val));
 	}
 	return NULL;
 }
@@ -172,7 +165,7 @@ static void *test_syscall_getrandom(void *ctx)
 	for (size_t i = 0; i < TRIALS; ++i) {
 		unsigned int val;
 		ssize_t ret = syscall(__NR_getrandom, &val, sizeof(val), 0);
-		assert(ret == sizeof(val));
+		ksft_assert(ret == sizeof(val));
 	}
 	return NULL;
 }
@@ -207,7 +200,7 @@ static void bench_multi(void)
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	for (size_t i = 0; i < THREADS; ++i)
-		assert(pthread_create(&threads[i], NULL, test_vdso_getrandom, NULL) == 0);
+		ksft_assert(pthread_create(&threads[i], NULL, test_vdso_getrandom, NULL) == 0);
 	for (size_t i = 0; i < THREADS; ++i)
 		pthread_join(threads[i], NULL);
 	clock_gettime(CLOCK_MONOTONIC, &end);
@@ -216,7 +209,7 @@ static void bench_multi(void)
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	for (size_t i = 0; i < THREADS; ++i)
-		assert(pthread_create(&threads[i], NULL, test_libc_getrandom, NULL) == 0);
+		ksft_assert(pthread_create(&threads[i], NULL, test_libc_getrandom, NULL) == 0);
 	for (size_t i = 0; i < THREADS; ++i)
 		pthread_join(threads[i], NULL);
 	clock_gettime(CLOCK_MONOTONIC, &end);
@@ -225,7 +218,7 @@ static void bench_multi(void)
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	for (size_t i = 0; i < THREADS; ++i)
-		assert(pthread_create(&threads[i], NULL, test_syscall_getrandom, NULL) == 0);
+		ksft_assert(pthread_create(&threads[i], NULL, test_syscall_getrandom, NULL) == 0);
 	for (size_t i = 0; i < THREADS; ++i)
 		pthread_join(threads[i], NULL);
 	clock_gettime(CLOCK_MONOTONIC, &end);
@@ -250,48 +243,46 @@ static void kselftest(void)
 
 	for (size_t i = 0; i < 1000; ++i) {
 		ssize_t ret = vgetrandom(weird_size, sizeof(weird_size), 0);
-		if (ret != sizeof(weird_size))
-			exit(KSFT_FAIL);
+		ksft_assert(ret == sizeof(weird_size));
 	}
 
 	ksft_test_result_pass("getrandom: PASS\n");
 
 	unshare(CLONE_NEWUSER);
-	assert(unshare(CLONE_NEWTIME) == 0);
+	ksft_assert(unshare(CLONE_NEWTIME) == 0);
 	child = fork();
-	assert(child >= 0);
+	ksft_assert(child >= 0);
 	if (!child) {
 		vgetrandom_init();
 		child = getpid();
-		assert(ptrace(PTRACE_TRACEME, 0, NULL, NULL) == 0);
-		assert(kill(child, SIGSTOP) == 0);
-		assert(vgetrandom(weird_size, sizeof(weird_size), 0) == sizeof(weird_size));
+		ksft_assert(ptrace(PTRACE_TRACEME, 0, NULL, NULL) == 0);
+		ksft_assert(kill(child, SIGSTOP) == 0);
+		ksft_assert(vgetrandom(weird_size, sizeof(weird_size), 0) == sizeof(weird_size));
 		_exit(0);
 	}
 	for (;;) {
 		struct ptrace_syscall_info info = { 0 };
 		int status, ret;
-		assert(waitpid(child, &status, 0) >= 0);
+		ksft_assert(waitpid(child, &status, 0) >= 0);
 		if (WIFEXITED(status)) {
-			if (WEXITSTATUS(status) != 0)
-				exit(KSFT_FAIL);
+			ksft_assert(WEXITSTATUS(status) == 0);
 			break;
 		}
-		assert(WIFSTOPPED(status));
+		ksft_assert(WIFSTOPPED(status));
 		if (WSTOPSIG(status) == SIGSTOP)
-			assert(ptrace(PTRACE_SETOPTIONS, child, 0, PTRACE_O_TRACESYSGOOD) == 0);
+			ksft_assert(ptrace(PTRACE_SETOPTIONS, child, 0, PTRACE_O_TRACESYSGOOD) == 0);
 		else if (WSTOPSIG(status) == (SIGTRAP | 0x80)) {
-			assert(ptrace(PTRACE_GET_SYSCALL_INFO, child, sizeof(info), &info) > 0);
+			ksft_assert(ptrace(PTRACE_GET_SYSCALL_INFO, child, sizeof(info), &info) > 0);
 			if (info.op == PTRACE_SYSCALL_INFO_ENTRY && info.entry.nr == __NR_getrandom &&
 			    info.entry.args[0] == (uintptr_t)weird_size && info.entry.args[1] == sizeof(weird_size))
-				exit(KSFT_FAIL);
+				ksft_exit_fail_msg("vgetrandom passed buffer to syscall getrandom unexpectedly\n");
 		}
-		assert(ptrace(PTRACE_SYSCALL, child, 0, 0) == 0);
+		ksft_assert(ptrace(PTRACE_SYSCALL, child, 0, 0) == 0);
 	}
 
 	ksft_test_result_pass("getrandom timens: PASS\n");
 
-	exit(KSFT_PASS);
+	ksft_exit_pass();
 }
 
 static void usage(const char *argv0)
-- 
2.46.0


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

* [PATCH kselftest 4/3] selftests: vDSO: unconditionally build chacha test
  2024-10-03  3:13 ` [PATCH kselftest 0/3] getrandom & chacha cleanups Jason A. Donenfeld
                     ` (2 preceding siblings ...)
  2024-10-03  3:13   ` [PATCH kselftest 3/3] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
@ 2024-10-03 17:18   ` Jason A. Donenfeld
  2024-10-03 22:42   ` [PATCH kselftest 0/3] getrandom & chacha cleanups Shuah Khan
  4 siblings, 0 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-03 17:18 UTC (permalink / raw)
  To: skhan, greg, linux-kselftest; +Cc: Jason A. Donenfeld

Rather than using symlinks to find the vgetrandom-chacha.S file for each
arch, store this in a file that uses the compiler to determine
architecture, and then make use of weak symbols to skip the test on
architectures that don't provide the code.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/arch/arm64/vdso                            |  1 -
 tools/arch/loongarch/vdso                        |  1 -
 tools/arch/powerpc/vdso                          |  1 -
 tools/arch/s390/vdso                             |  1 -
 tools/arch/x86/vdso                              |  1 -
 tools/testing/selftests/vDSO/Makefile            |  6 +-----
 tools/testing/selftests/vDSO/vdso_test_chacha.c  |  6 ++++++
 .../selftests/vDSO/vgetrandom-chacha-finder.S    | 16 ++++++++++++++++
 8 files changed, 23 insertions(+), 10 deletions(-)
 delete mode 120000 tools/arch/arm64/vdso
 delete mode 120000 tools/arch/loongarch/vdso
 delete mode 120000 tools/arch/powerpc/vdso
 delete mode 120000 tools/arch/s390/vdso
 delete mode 120000 tools/arch/x86/vdso
 create mode 100644 tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S

diff --git a/tools/arch/arm64/vdso b/tools/arch/arm64/vdso
deleted file mode 120000
index 233c7a26f6e5..000000000000
--- a/tools/arch/arm64/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/arm64/kernel/vdso
\ No newline at end of file
diff --git a/tools/arch/loongarch/vdso b/tools/arch/loongarch/vdso
deleted file mode 120000
index ebda43a82db7..000000000000
--- a/tools/arch/loongarch/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/loongarch/vdso
\ No newline at end of file
diff --git a/tools/arch/powerpc/vdso b/tools/arch/powerpc/vdso
deleted file mode 120000
index 4e676d1d1cb4..000000000000
--- a/tools/arch/powerpc/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/powerpc/kernel/vdso
\ No newline at end of file
diff --git a/tools/arch/s390/vdso b/tools/arch/s390/vdso
deleted file mode 120000
index 6cf4c1cebdcd..000000000000
--- a/tools/arch/s390/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/s390/kernel/vdso64
\ No newline at end of file
diff --git a/tools/arch/x86/vdso b/tools/arch/x86/vdso
deleted file mode 120000
index 7eb962fd3454..000000000000
--- a/tools/arch/x86/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/x86/entry/vdso/
\ No newline at end of file
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 96b4f2a766e2..e453c178d97f 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 include ../../../scripts/Makefile.arch
-top_srcdir = $(realpath ../../../..)
 
 TEST_GEN_PROGS := vdso_test_gettimeofday
 TEST_GEN_PROGS += vdso_test_getcpu
@@ -11,15 +10,12 @@ TEST_GEN_PROGS += vdso_standalone_test_x86
 endif
 TEST_GEN_PROGS += vdso_test_correctness
 TEST_GEN_PROGS += vdso_test_getrandom
-ifneq ($(wildcard $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S),)
 TEST_GEN_PROGS += vdso_test_chacha
-endif
 
 CFLAGS := -std=gnu99 -O2
 
 ifeq ($(CONFIG_X86_32),y)
 LDLIBS += -lgcc_s
-TEST_GEN_PROGS := $(filter-out vdso_test_chacha,$(TEST_GEN_PROGS))
 endif
 
 include ../lib.mk
@@ -39,7 +35,7 @@ $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
                                          $(KHDR_INCLUDES) \
                                          -isystem $(top_srcdir)/include/uapi
 
-$(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S
+$(OUTPUT)/vdso_test_chacha: vgetrandom-chacha-finder.S
 $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \
                                       -idirafter $(top_srcdir)/tools/include/generated \
                                       -idirafter $(top_srcdir)/arch/$(SRCARCH)/include \
diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c
index 60c5ea5cb925..e68f1cd4ebcf 100644
--- a/tools/testing/selftests/vDSO/vdso_test_chacha.c
+++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
@@ -78,6 +78,12 @@ typedef uint32_t u32;
 typedef uint64_t u64;
 #include <vdso/getrandom.h>
 
+__attribute__((weak))
+void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks)
+{
+	ksft_exit_skip("Not implemented on architecture\n");
+}
+
 int main(int argc, char *argv[])
 {
 	enum { TRIALS = 1000, BLOCKS = 128, BLOCK_SIZE = 64 };
diff --git a/tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S b/tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S
new file mode 100644
index 000000000000..1ca240ddaab7
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#if defined(__aarch64__)
+#include "../../../../arch/arm64/kernel/vdso/vgetrandom-chacha.S"
+#elif defined(__loongarch__)
+#include "../../../../arch/loongarch/vdso/vgetrandom-chacha.S"
+#elif defined(__powerpc__) || defined(__powerpc64__)
+#include "../../../../arch/powerpc/kernel/vdso/vgetrandom-chacha.S"
+#elif defined(__s390x__)
+#include "../../../../arch/s390/kernel/vdso64/vgetrandom-chacha.S"
+#elif defined(__x86_64__)
+#include "../../../../arch/x86/entry/vdso/vgetrandom-chacha.S"
+#endif
-- 
2.46.0


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

* Re: [PATCH kselftest 0/3] getrandom & chacha cleanups
  2024-10-03  3:13 ` [PATCH kselftest 0/3] getrandom & chacha cleanups Jason A. Donenfeld
                     ` (3 preceding siblings ...)
  2024-10-03 17:18   ` [PATCH kselftest 4/3] " Jason A. Donenfeld
@ 2024-10-03 22:42   ` Shuah Khan
  4 siblings, 0 replies; 17+ messages in thread
From: Shuah Khan @ 2024-10-03 22:42 UTC (permalink / raw)
  To: Jason A. Donenfeld, greg, linux-kselftest; +Cc: Shuah Khan

On 10/2/24 21:13, Jason A. Donenfeld wrote:
> Hi Shuah,
> 
> I've now read your email several times trying to figure out what you
> meant and what your objections are. This series is my best attempt at
> trying to satisfy that. But my understanding still has a lot of question
> marks, so I may have missed your point here. Nonetheless, maybe this
> moves things forward a bit.
> 
> Jason
> 
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: Greg KH <greg@kroah.com>
> 
> Jason A. Donenfeld (3):
>    selftests: vDSO: condition chacha build on chacha implementation
>    selftests: vDSO: unconditionally build getrandom test
>    selftests: vDSO: improve getrandom and chacha error messages
> 
>   tools/testing/selftests/vDSO/Makefile         |  4 +-
>   .../testing/selftests/vDSO/vdso_test_chacha.c | 27 ++++---
>   .../selftests/vDSO/vdso_test_getrandom.c      | 75 ++++++++-----------
>   3 files changed, 52 insertions(+), 54 deletions(-)
> 

Thank you Jason for fixing the problem. The patches look good
to me. I will run some tests and apply all 4 patches for next rc.

I did review the 4th patch as well.
https://patchwork.kernel.org/project/linux-kselftest/patch/20241003171852.2386463-1-Jason@zx2c4.com/

than

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

* Re: [PATCH kselftest 3/3] selftests: vDSO: improve getrandom and chacha error messages
  2024-10-03  3:13   ` [PATCH kselftest 3/3] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
@ 2024-10-06  3:35     ` kernel test robot
  2024-10-06  4:35       ` Jason A. Donenfeld
  0 siblings, 1 reply; 17+ messages in thread
From: kernel test robot @ 2024-10-06  3:35 UTC (permalink / raw)
  To: Jason A. Donenfeld, skhan, greg, linux-kselftest
  Cc: oe-kbuild-all, Jason A. Donenfeld

Hi Jason,

kernel test robot noticed the following build warnings:

[auto build test WARNING on shuah-kselftest/next]
[also build test WARNING on shuah-kselftest/fixes linus/master v6.12-rc1 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-A-Donenfeld/selftests-vDSO-condition-chacha-build-on-chacha-implementation/20241003-111508
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
patch link:    https://lore.kernel.org/r/20241003031307.2236454-4-Jason%40zx2c4.com
patch subject: [PATCH kselftest 3/3] selftests: vDSO: improve getrandom and chacha error messages
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241005/202410051432.uAzAV7yV-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202410051432.uAzAV7yV-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> vdso_test_getrandom.c:274:3: warning: add explicit braces to avoid dangling else [-Wdangling-else]
     274 |                 else if (WSTOPSIG(status) == (SIGTRAP | 0x80)) {
         |                 ^
   1 warning generated.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


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

* Re: [PATCH kselftest 3/3] selftests: vDSO: improve getrandom and chacha error messages
  2024-10-06  3:35     ` kernel test robot
@ 2024-10-06  4:35       ` Jason A. Donenfeld
  2024-10-06  4:40         ` [PATCH kselftest v2 1/4] selftests: vDSO: condition chacha build on chacha implementation Jason A. Donenfeld
  0 siblings, 1 reply; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-06  4:35 UTC (permalink / raw)
  To: kernel test robot; +Cc: skhan, greg, linux-kselftest, oe-kbuild-all

I'll send a v2.

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

* [PATCH kselftest v2 1/4] selftests: vDSO: condition chacha build on chacha implementation
  2024-10-06  4:35       ` Jason A. Donenfeld
@ 2024-10-06  4:40         ` Jason A. Donenfeld
  2024-10-06  4:40           ` [PATCH kselftest v2 2/4] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
                             ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-06  4:40 UTC (permalink / raw)
  To: linux-kselftest, greg, skhan; +Cc: Jason A. Donenfeld

The chacha test can build anywhere that the vgetrandom-chacha.S file is
available, so condition it on the existence of that file, rather than a
specific arch.

There is one wrinkle, which is that x86 and x86_64 are the same ARCH,
but the code is only functional for x86_64. So filter out the test for
CONFIG_X86_32 in the same block that the other 32-bit special case
lives.

We have to define top_srcdir ourselves, because even though lib.mk
defines it later, that has to be included after TEST_GEN_PROGS is
populated. Fortunately the definition is fairly trivial.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/testing/selftests/vDSO/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index af9cedbf5357..2c38c9c6d056 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 include ../../../scripts/Makefile.arch
+top_srcdir = $(realpath ../../../..)
 
 TEST_GEN_PROGS := vdso_test_gettimeofday
 TEST_GEN_PROGS += vdso_test_getcpu
@@ -11,6 +12,8 @@ endif
 TEST_GEN_PROGS += vdso_test_correctness
 ifeq ($(ARCH)$(CONFIG_X86_32),$(filter $(ARCH)$(CONFIG_X86_32),x86 x86_64 loongarch arm64 powerpc s390))
 TEST_GEN_PROGS += vdso_test_getrandom
+endif
+ifneq ($(wildcard $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S),)
 TEST_GEN_PROGS += vdso_test_chacha
 endif
 
@@ -18,6 +21,7 @@ CFLAGS := -std=gnu99 -O2
 
 ifeq ($(CONFIG_X86_32),y)
 LDLIBS += -lgcc_s
+TEST_GEN_PROGS := $(filter-out vdso_test_chacha,$(TEST_GEN_PROGS))
 endif
 
 include ../lib.mk
-- 
2.46.0


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

* [PATCH kselftest v2 2/4] selftests: vDSO: unconditionally build getrandom test
  2024-10-06  4:40         ` [PATCH kselftest v2 1/4] selftests: vDSO: condition chacha build on chacha implementation Jason A. Donenfeld
@ 2024-10-06  4:40           ` Jason A. Donenfeld
  2024-10-06  4:40           ` [PATCH kselftest v2 3/4] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
  2024-10-06  4:40           ` [PATCH kselftest v2 4/4] selftests: vDSO: unconditionally build chacha test Jason A. Donenfeld
  2 siblings, 0 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-06  4:40 UTC (permalink / raw)
  To: linux-kselftest, greg, skhan; +Cc: Jason A. Donenfeld

Rather than building on supported archs, build on all archs, and then
use the presence of the symbol in the vDSO to either skip the test or
move forward with it.

Note that this means that this test no longer checks whether the symbol
was correctly added to the kernel. But hopefully this will be clear
enough to developers and we'll cross our fingers that symbols aren't
removed by accident and not caught after this change.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/testing/selftests/vDSO/Makefile              | 2 --
 tools/testing/selftests/vDSO/vdso_test_getrandom.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 2c38c9c6d056..96b4f2a766e2 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -10,9 +10,7 @@ ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
 TEST_GEN_PROGS += vdso_standalone_test_x86
 endif
 TEST_GEN_PROGS += vdso_test_correctness
-ifeq ($(ARCH)$(CONFIG_X86_32),$(filter $(ARCH)$(CONFIG_X86_32),x86 x86_64 loongarch arm64 powerpc s390))
 TEST_GEN_PROGS += vdso_test_getrandom
-endif
 ifneq ($(wildcard $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S),)
 TEST_GEN_PROGS += vdso_test_chacha
 endif
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
index 72a1d9b43a84..5489a2f07434 100644
--- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
+++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
@@ -119,7 +119,7 @@ static void vgetrandom_init(void)
 	vgrnd.fn = (__typeof__(vgrnd.fn))vdso_sym(version, name);
 	if (!vgrnd.fn) {
 		printf("%s is missing!\n", name);
-		exit(KSFT_FAIL);
+		exit(KSFT_SKIP);
 	}
 	ret = VDSO_CALL(vgrnd.fn, 5, NULL, 0, 0, &vgrnd.params, ~0UL);
 	if (ret == -ENOSYS) {
-- 
2.46.0


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

* [PATCH kselftest v2 3/4] selftests: vDSO: improve getrandom and chacha error messages
  2024-10-06  4:40         ` [PATCH kselftest v2 1/4] selftests: vDSO: condition chacha build on chacha implementation Jason A. Donenfeld
  2024-10-06  4:40           ` [PATCH kselftest v2 2/4] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
@ 2024-10-06  4:40           ` Jason A. Donenfeld
  2024-10-06  4:40           ` [PATCH kselftest v2 4/4] selftests: vDSO: unconditionally build chacha test Jason A. Donenfeld
  2 siblings, 0 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-06  4:40 UTC (permalink / raw)
  To: linux-kselftest, greg, skhan; +Cc: Jason A. Donenfeld

Improve the error and skip condition messages to let the developer know
precisely where a test has failed. Also make better use of the ksft api
for this.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 .../testing/selftests/vDSO/vdso_test_chacha.c | 27 ++++---
 .../selftests/vDSO/vdso_test_getrandom.c      | 75 ++++++++-----------
 2 files changed, 49 insertions(+), 53 deletions(-)

diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c
index b1ea532c5996..60c5ea5cb925 100644
--- a/tools/testing/selftests/vDSO/vdso_test_chacha.c
+++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
@@ -90,10 +90,8 @@ int main(int argc, char *argv[])
 	ksft_set_plan(1);
 
 	for (unsigned int trial = 0; trial < TRIALS; ++trial) {
-		if (getrandom(key, sizeof(key), 0) != sizeof(key)) {
-			printf("getrandom() failed!\n");
-			return KSFT_SKIP;
-		}
+		if (getrandom(key, sizeof(key), 0) != sizeof(key))
+			ksft_exit_skip("getrandom() failed unexpectedly\n");
 		memset(counter1, 0, sizeof(counter1));
 		reference_chacha20_blocks(output1, key, counter1, BLOCKS);
 		for (unsigned int split = 0; split < BLOCKS; ++split) {
@@ -102,8 +100,10 @@ int main(int argc, char *argv[])
 			if (split)
 				__arch_chacha20_blocks_nostack(output2, key, counter2, split);
 			__arch_chacha20_blocks_nostack(output2 + split * BLOCK_SIZE, key, counter2, BLOCKS - split);
-			if (memcmp(output1, output2, sizeof(output1)) || memcmp(counter1, counter2, sizeof(counter1)))
-				return KSFT_FAIL;
+			if (memcmp(output1, output2, sizeof(output1)))
+				ksft_exit_fail_msg("Main loop outputs do not match on trial %u, split %u\n", trial, split);
+			if (memcmp(counter1, counter2, sizeof(counter1)))
+				ksft_exit_fail_msg("Main loop counters do not match on trial %u, split %u\n", trial, split);
 		}
 	}
 	memset(counter1, 0, sizeof(counter1));
@@ -113,14 +113,19 @@ int main(int argc, char *argv[])
 
 	reference_chacha20_blocks(output1, key, counter1, BLOCKS);
 	__arch_chacha20_blocks_nostack(output2, key, counter2, BLOCKS);
-	if (memcmp(output1, output2, sizeof(output1)) || memcmp(counter1, counter2, sizeof(counter1)))
-		return KSFT_FAIL;
+	if (memcmp(output1, output2, sizeof(output1)))
+		ksft_exit_fail_msg("Block limit outputs do not match after first round\n");
+	if (memcmp(counter1, counter2, sizeof(counter1)))
+		ksft_exit_fail_msg("Block limit counters do not match after first round\n");
 
 	reference_chacha20_blocks(output1, key, counter1, BLOCKS);
 	__arch_chacha20_blocks_nostack(output2, key, counter2, BLOCKS);
-	if (memcmp(output1, output2, sizeof(output1)) || memcmp(counter1, counter2, sizeof(counter1)))
-		return KSFT_FAIL;
+	if (memcmp(output1, output2, sizeof(output1)))
+		ksft_exit_fail_msg("Block limit outputs do not match after second round\n");
+	if (memcmp(counter1, counter2, sizeof(counter1)))
+		ksft_exit_fail_msg("Block limit counters do not match after second round\n");
 
 	ksft_test_result_pass("chacha: PASS\n");
-	return KSFT_PASS;
+	ksft_exit_pass();
+	return 0;
 }
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
index 5489a2f07434..50ec9ad4c7ec 100644
--- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
+++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
@@ -40,6 +40,9 @@
 	} while (0)
 #endif
 
+#define ksft_assert(condition) \
+	do { if (!(condition)) ksft_exit_fail_msg("Assertion failed: %s\n", #condition); } while (0)
+
 static struct {
 	pthread_mutex_t lock;
 	void **states;
@@ -109,26 +112,19 @@ static void vgetrandom_init(void)
 	const char *version = versions[VDSO_VERSION];
 	const char *name = names[VDSO_NAMES][6];
 	unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
-	size_t ret;
+	ssize_t ret;
 
-	if (!sysinfo_ehdr) {
-		printf("AT_SYSINFO_EHDR is not present!\n");
-		exit(KSFT_SKIP);
-	}
+	if (!sysinfo_ehdr)
+		ksft_exit_skip("AT_SYSINFO_EHDR is not present\n");
 	vdso_init_from_sysinfo_ehdr(sysinfo_ehdr);
 	vgrnd.fn = (__typeof__(vgrnd.fn))vdso_sym(version, name);
-	if (!vgrnd.fn) {
-		printf("%s is missing!\n", name);
-		exit(KSFT_SKIP);
-	}
+	if (!vgrnd.fn)
+		ksft_exit_skip("%s@%s symbol is missing from vDSO\n", name, version);
 	ret = VDSO_CALL(vgrnd.fn, 5, NULL, 0, 0, &vgrnd.params, ~0UL);
-	if (ret == -ENOSYS) {
-		printf("unsupported architecture\n");
-		exit(KSFT_SKIP);
-	} else if (ret) {
-		printf("failed to fetch vgetrandom params!\n");
-		exit(KSFT_FAIL);
-	}
+	if (ret == -ENOSYS)
+		ksft_exit_skip("CPU does not have runtime support\n");
+	else if (ret)
+		ksft_exit_fail_msg("Failed to fetch vgetrandom params: %zd\n", ret);
 }
 
 static ssize_t vgetrandom(void *buf, size_t len, unsigned long flags)
@@ -137,10 +133,7 @@ static ssize_t vgetrandom(void *buf, size_t len, unsigned long flags)
 
 	if (!state) {
 		state = vgetrandom_get_state();
-		if (!state) {
-			printf("vgetrandom_get_state failed!\n");
-			exit(KSFT_FAIL);
-		}
+		ksft_assert(state);
 	}
 	return VDSO_CALL(vgrnd.fn, 5, buf, len, flags, state, vgrnd.params.size_of_opaque_state);
 }
@@ -152,7 +145,7 @@ static void *test_vdso_getrandom(void *ctx)
 	for (size_t i = 0; i < TRIALS; ++i) {
 		unsigned int val;
 		ssize_t ret = vgetrandom(&val, sizeof(val), 0);
-		assert(ret == sizeof(val));
+		ksft_assert(ret == sizeof(val));
 	}
 	return NULL;
 }
@@ -162,7 +155,7 @@ static void *test_libc_getrandom(void *ctx)
 	for (size_t i = 0; i < TRIALS; ++i) {
 		unsigned int val;
 		ssize_t ret = getrandom(&val, sizeof(val), 0);
-		assert(ret == sizeof(val));
+		ksft_assert(ret == sizeof(val));
 	}
 	return NULL;
 }
@@ -172,7 +165,7 @@ static void *test_syscall_getrandom(void *ctx)
 	for (size_t i = 0; i < TRIALS; ++i) {
 		unsigned int val;
 		ssize_t ret = syscall(__NR_getrandom, &val, sizeof(val), 0);
-		assert(ret == sizeof(val));
+		ksft_assert(ret == sizeof(val));
 	}
 	return NULL;
 }
@@ -207,7 +200,7 @@ static void bench_multi(void)
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	for (size_t i = 0; i < THREADS; ++i)
-		assert(pthread_create(&threads[i], NULL, test_vdso_getrandom, NULL) == 0);
+		ksft_assert(pthread_create(&threads[i], NULL, test_vdso_getrandom, NULL) == 0);
 	for (size_t i = 0; i < THREADS; ++i)
 		pthread_join(threads[i], NULL);
 	clock_gettime(CLOCK_MONOTONIC, &end);
@@ -216,7 +209,7 @@ static void bench_multi(void)
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	for (size_t i = 0; i < THREADS; ++i)
-		assert(pthread_create(&threads[i], NULL, test_libc_getrandom, NULL) == 0);
+		ksft_assert(pthread_create(&threads[i], NULL, test_libc_getrandom, NULL) == 0);
 	for (size_t i = 0; i < THREADS; ++i)
 		pthread_join(threads[i], NULL);
 	clock_gettime(CLOCK_MONOTONIC, &end);
@@ -225,7 +218,7 @@ static void bench_multi(void)
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	for (size_t i = 0; i < THREADS; ++i)
-		assert(pthread_create(&threads[i], NULL, test_syscall_getrandom, NULL) == 0);
+		ksft_assert(pthread_create(&threads[i], NULL, test_syscall_getrandom, NULL) == 0);
 	for (size_t i = 0; i < THREADS; ++i)
 		pthread_join(threads[i], NULL);
 	clock_gettime(CLOCK_MONOTONIC, &end);
@@ -250,48 +243,46 @@ static void kselftest(void)
 
 	for (size_t i = 0; i < 1000; ++i) {
 		ssize_t ret = vgetrandom(weird_size, sizeof(weird_size), 0);
-		if (ret != sizeof(weird_size))
-			exit(KSFT_FAIL);
+		ksft_assert(ret == sizeof(weird_size));
 	}
 
 	ksft_test_result_pass("getrandom: PASS\n");
 
 	unshare(CLONE_NEWUSER);
-	assert(unshare(CLONE_NEWTIME) == 0);
+	ksft_assert(unshare(CLONE_NEWTIME) == 0);
 	child = fork();
-	assert(child >= 0);
+	ksft_assert(child >= 0);
 	if (!child) {
 		vgetrandom_init();
 		child = getpid();
-		assert(ptrace(PTRACE_TRACEME, 0, NULL, NULL) == 0);
-		assert(kill(child, SIGSTOP) == 0);
-		assert(vgetrandom(weird_size, sizeof(weird_size), 0) == sizeof(weird_size));
+		ksft_assert(ptrace(PTRACE_TRACEME, 0, NULL, NULL) == 0);
+		ksft_assert(kill(child, SIGSTOP) == 0);
+		ksft_assert(vgetrandom(weird_size, sizeof(weird_size), 0) == sizeof(weird_size));
 		_exit(0);
 	}
 	for (;;) {
 		struct ptrace_syscall_info info = { 0 };
 		int status, ret;
-		assert(waitpid(child, &status, 0) >= 0);
+		ksft_assert(waitpid(child, &status, 0) >= 0);
 		if (WIFEXITED(status)) {
-			if (WEXITSTATUS(status) != 0)
-				exit(KSFT_FAIL);
+			ksft_assert(WEXITSTATUS(status) == 0);
 			break;
 		}
-		assert(WIFSTOPPED(status));
+		ksft_assert(WIFSTOPPED(status));
 		if (WSTOPSIG(status) == SIGSTOP)
-			assert(ptrace(PTRACE_SETOPTIONS, child, 0, PTRACE_O_TRACESYSGOOD) == 0);
+			ksft_assert(ptrace(PTRACE_SETOPTIONS, child, 0, PTRACE_O_TRACESYSGOOD) == 0);
 		else if (WSTOPSIG(status) == (SIGTRAP | 0x80)) {
-			assert(ptrace(PTRACE_GET_SYSCALL_INFO, child, sizeof(info), &info) > 0);
+			ksft_assert(ptrace(PTRACE_GET_SYSCALL_INFO, child, sizeof(info), &info) > 0);
 			if (info.op == PTRACE_SYSCALL_INFO_ENTRY && info.entry.nr == __NR_getrandom &&
 			    info.entry.args[0] == (uintptr_t)weird_size && info.entry.args[1] == sizeof(weird_size))
-				exit(KSFT_FAIL);
+				ksft_exit_fail_msg("vgetrandom passed buffer to syscall getrandom unexpectedly\n");
 		}
-		assert(ptrace(PTRACE_SYSCALL, child, 0, 0) == 0);
+		ksft_assert(ptrace(PTRACE_SYSCALL, child, 0, 0) == 0);
 	}
 
 	ksft_test_result_pass("getrandom timens: PASS\n");
 
-	exit(KSFT_PASS);
+	ksft_exit_pass();
 }
 
 static void usage(const char *argv0)
-- 
2.46.0


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

* [PATCH kselftest v2 4/4] selftests: vDSO: unconditionally build chacha test
  2024-10-06  4:40         ` [PATCH kselftest v2 1/4] selftests: vDSO: condition chacha build on chacha implementation Jason A. Donenfeld
  2024-10-06  4:40           ` [PATCH kselftest v2 2/4] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
  2024-10-06  4:40           ` [PATCH kselftest v2 3/4] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
@ 2024-10-06  4:40           ` Jason A. Donenfeld
  2024-10-07 20:53             ` Shuah Khan
  2 siblings, 1 reply; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-06  4:40 UTC (permalink / raw)
  To: linux-kselftest, greg, skhan; +Cc: Jason A. Donenfeld

Rather than using symlinks to find the vgetrandom-chacha.S file for each
arch, store this in a file that uses the compiler to determine
architecture, and then make use of weak symbols to skip the test on
architectures that don't provide the code.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/arch/arm64/vdso                            |  1 -
 tools/arch/loongarch/vdso                        |  1 -
 tools/arch/powerpc/vdso                          |  1 -
 tools/arch/s390/vdso                             |  1 -
 tools/arch/x86/vdso                              |  1 -
 tools/testing/selftests/vDSO/Makefile            |  6 +-----
 tools/testing/selftests/vDSO/vdso_test_chacha.c  |  6 ++++++
 .../selftests/vDSO/vgetrandom-chacha-finder.S    | 16 ++++++++++++++++
 8 files changed, 23 insertions(+), 10 deletions(-)
 delete mode 120000 tools/arch/arm64/vdso
 delete mode 120000 tools/arch/loongarch/vdso
 delete mode 120000 tools/arch/powerpc/vdso
 delete mode 120000 tools/arch/s390/vdso
 delete mode 120000 tools/arch/x86/vdso
 create mode 100644 tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S

diff --git a/tools/arch/arm64/vdso b/tools/arch/arm64/vdso
deleted file mode 120000
index 233c7a26f6e5..000000000000
--- a/tools/arch/arm64/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/arm64/kernel/vdso
\ No newline at end of file
diff --git a/tools/arch/loongarch/vdso b/tools/arch/loongarch/vdso
deleted file mode 120000
index ebda43a82db7..000000000000
--- a/tools/arch/loongarch/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/loongarch/vdso
\ No newline at end of file
diff --git a/tools/arch/powerpc/vdso b/tools/arch/powerpc/vdso
deleted file mode 120000
index 4e676d1d1cb4..000000000000
--- a/tools/arch/powerpc/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/powerpc/kernel/vdso
\ No newline at end of file
diff --git a/tools/arch/s390/vdso b/tools/arch/s390/vdso
deleted file mode 120000
index 6cf4c1cebdcd..000000000000
--- a/tools/arch/s390/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/s390/kernel/vdso64
\ No newline at end of file
diff --git a/tools/arch/x86/vdso b/tools/arch/x86/vdso
deleted file mode 120000
index 7eb962fd3454..000000000000
--- a/tools/arch/x86/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/x86/entry/vdso/
\ No newline at end of file
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 96b4f2a766e2..e453c178d97f 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 include ../../../scripts/Makefile.arch
-top_srcdir = $(realpath ../../../..)
 
 TEST_GEN_PROGS := vdso_test_gettimeofday
 TEST_GEN_PROGS += vdso_test_getcpu
@@ -11,15 +10,12 @@ TEST_GEN_PROGS += vdso_standalone_test_x86
 endif
 TEST_GEN_PROGS += vdso_test_correctness
 TEST_GEN_PROGS += vdso_test_getrandom
-ifneq ($(wildcard $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S),)
 TEST_GEN_PROGS += vdso_test_chacha
-endif
 
 CFLAGS := -std=gnu99 -O2
 
 ifeq ($(CONFIG_X86_32),y)
 LDLIBS += -lgcc_s
-TEST_GEN_PROGS := $(filter-out vdso_test_chacha,$(TEST_GEN_PROGS))
 endif
 
 include ../lib.mk
@@ -39,7 +35,7 @@ $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
                                          $(KHDR_INCLUDES) \
                                          -isystem $(top_srcdir)/include/uapi
 
-$(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S
+$(OUTPUT)/vdso_test_chacha: vgetrandom-chacha-finder.S
 $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \
                                       -idirafter $(top_srcdir)/tools/include/generated \
                                       -idirafter $(top_srcdir)/arch/$(SRCARCH)/include \
diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c
index 60c5ea5cb925..e68f1cd4ebcf 100644
--- a/tools/testing/selftests/vDSO/vdso_test_chacha.c
+++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
@@ -78,6 +78,12 @@ typedef uint32_t u32;
 typedef uint64_t u64;
 #include <vdso/getrandom.h>
 
+__attribute__((weak))
+void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks)
+{
+	ksft_exit_skip("Not implemented on architecture\n");
+}
+
 int main(int argc, char *argv[])
 {
 	enum { TRIALS = 1000, BLOCKS = 128, BLOCK_SIZE = 64 };
diff --git a/tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S b/tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S
new file mode 100644
index 000000000000..1ca240ddaab7
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#if defined(__aarch64__)
+#include "../../../../arch/arm64/kernel/vdso/vgetrandom-chacha.S"
+#elif defined(__loongarch__)
+#include "../../../../arch/loongarch/vdso/vgetrandom-chacha.S"
+#elif defined(__powerpc__) || defined(__powerpc64__)
+#include "../../../../arch/powerpc/kernel/vdso/vgetrandom-chacha.S"
+#elif defined(__s390x__)
+#include "../../../../arch/s390/kernel/vdso64/vgetrandom-chacha.S"
+#elif defined(__x86_64__)
+#include "../../../../arch/x86/entry/vdso/vgetrandom-chacha.S"
+#endif
-- 
2.46.0


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

* Re: [PATCH kselftest v2 4/4] selftests: vDSO: unconditionally build chacha test
  2024-10-06  4:40           ` [PATCH kselftest v2 4/4] selftests: vDSO: unconditionally build chacha test Jason A. Donenfeld
@ 2024-10-07 20:53             ` Shuah Khan
  2024-10-07 21:45               ` [PATCH kselftest v3 1/3] " Jason A. Donenfeld
  0 siblings, 1 reply; 17+ messages in thread
From: Shuah Khan @ 2024-10-07 20:53 UTC (permalink / raw)
  To: Jason A. Donenfeld, linux-kselftest; +Cc: Shuah Khan, greg

On 10/5/24 22:40, Jason A. Donenfeld wrote:
> Rather than using symlinks to find the vgetrandom-chacha.S file for each
> arch, store this in a file that uses the compiler to determine
> architecture, and then make use of weak symbols to skip the test on
> architectures that don't provide the code.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>   tools/arch/arm64/vdso                            |  1 -
>   tools/arch/loongarch/vdso                        |  1 -
>   tools/arch/powerpc/vdso                          |  1 -
>   tools/arch/s390/vdso                             |  1 -
>   tools/arch/x86/vdso                              |  1 -
>   tools/testing/selftests/vDSO/Makefile            |  6 +-----
>   tools/testing/selftests/vDSO/vdso_test_chacha.c  |  6 ++++++
>   .../selftests/vDSO/vgetrandom-chacha-finder.S    | 16 ++++++++++++++++
>   8 files changed, 23 insertions(+), 10 deletions(-)
>   delete mode 120000 tools/arch/arm64/vdso
>   delete mode 120000 tools/arch/loongarch/vdso
>   delete mode 120000 tools/arch/powerpc/vdso
>   delete mode 120000 tools/arch/s390/vdso
>   delete mode 120000 tools/arch/x86/vdso
>   create mode 100644 tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S

Jason,

I am seeing a few checkpatch warnings on this patch. They are
worth looking at.

WARNING: Prefer __weak over __attribute__((weak))
#176: FILE: tools/testing/selftests/vDSO/vdso_test_chacha.c:81:
+__attribute__((weak))

WARNING: Improper SPDX comment style for 'tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S', please use '/*' instead
#191: FILE: tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S:1:
+// SPDX-License-Identifier: GPL-2.0

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#191: FILE: tools/testing/selftests/vDSO/vgetrandom-chacha-finder.S:1:
+// SPDX-License-Identifier: GPL-2.0


thanks,
-- Shuah

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

* [PATCH kselftest v3 1/3] selftests: vDSO: unconditionally build chacha test
  2024-10-07 20:53             ` Shuah Khan
@ 2024-10-07 21:45               ` Jason A. Donenfeld
  2024-10-07 21:45                 ` [PATCH kselftest v3 2/3] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
                                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-07 21:45 UTC (permalink / raw)
  To: linux-kselftest, greg, skhan; +Cc: Jason A. Donenfeld

Rather than using symlinks to find the vgetrandom-chacha.S file for each
arch, store this in a file that uses the compiler to determine
architecture, and then make use of weak symbols to skip the test on
architectures that don't provide the code.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/arch/arm64/vdso                          |  1 -
 tools/arch/loongarch/vdso                      |  1 -
 tools/arch/powerpc/vdso                        |  1 -
 tools/arch/s390/vdso                           |  1 -
 tools/arch/x86/vdso                            |  1 -
 tools/testing/selftests/vDSO/Makefile          |  6 +++---
 .../testing/selftests/vDSO/vdso_test_chacha.c  |  9 +++++----
 .../testing/selftests/vDSO/vgetrandom-chacha.S | 18 ++++++++++++++++++
 8 files changed, 26 insertions(+), 12 deletions(-)
 delete mode 120000 tools/arch/arm64/vdso
 delete mode 120000 tools/arch/loongarch/vdso
 delete mode 120000 tools/arch/powerpc/vdso
 delete mode 120000 tools/arch/s390/vdso
 delete mode 120000 tools/arch/x86/vdso
 create mode 100644 tools/testing/selftests/vDSO/vgetrandom-chacha.S

diff --git a/tools/arch/arm64/vdso b/tools/arch/arm64/vdso
deleted file mode 120000
index 233c7a26f6e5..000000000000
--- a/tools/arch/arm64/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/arm64/kernel/vdso
\ No newline at end of file
diff --git a/tools/arch/loongarch/vdso b/tools/arch/loongarch/vdso
deleted file mode 120000
index ebda43a82db7..000000000000
--- a/tools/arch/loongarch/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/loongarch/vdso
\ No newline at end of file
diff --git a/tools/arch/powerpc/vdso b/tools/arch/powerpc/vdso
deleted file mode 120000
index 4e676d1d1cb4..000000000000
--- a/tools/arch/powerpc/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/powerpc/kernel/vdso
\ No newline at end of file
diff --git a/tools/arch/s390/vdso b/tools/arch/s390/vdso
deleted file mode 120000
index 6cf4c1cebdcd..000000000000
--- a/tools/arch/s390/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/s390/kernel/vdso64
\ No newline at end of file
diff --git a/tools/arch/x86/vdso b/tools/arch/x86/vdso
deleted file mode 120000
index 7eb962fd3454..000000000000
--- a/tools/arch/x86/vdso
+++ /dev/null
@@ -1 +0,0 @@
-../../../arch/x86/entry/vdso/
\ No newline at end of file
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index af9cedbf5357..45641386c662 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -11,8 +11,8 @@ endif
 TEST_GEN_PROGS += vdso_test_correctness
 ifeq ($(ARCH)$(CONFIG_X86_32),$(filter $(ARCH)$(CONFIG_X86_32),x86 x86_64 loongarch arm64 powerpc s390))
 TEST_GEN_PROGS += vdso_test_getrandom
-TEST_GEN_PROGS += vdso_test_chacha
 endif
+TEST_GEN_PROGS += vdso_test_chacha
 
 CFLAGS := -std=gnu99 -O2
 
@@ -37,9 +37,9 @@ $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
                                          $(KHDR_INCLUDES) \
                                          -isystem $(top_srcdir)/include/uapi
 
-$(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S
+$(OUTPUT)/vdso_test_chacha: vgetrandom-chacha.S
 $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \
                                       -idirafter $(top_srcdir)/tools/include/generated \
                                       -idirafter $(top_srcdir)/arch/$(SRCARCH)/include \
                                       -idirafter $(top_srcdir)/include \
-                                      -D__ASSEMBLY__ -Wa,--noexecstack
+                                      -Wa,--noexecstack
diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c
index b1ea532c5996..c66eb9df89bd 100644
--- a/tools/testing/selftests/vDSO/vdso_test_chacha.c
+++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2022-2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
  */
 
+#include <linux/compiler.h>
 #include <tools/le_byteshift.h>
 #include <sys/random.h>
 #include <sys/auxv.h>
@@ -73,10 +74,10 @@ static void reference_chacha20_blocks(uint8_t *dst_bytes, const uint32_t *key, u
 	counter[1] = s[13];
 }
 
-typedef uint8_t u8;
-typedef uint32_t u32;
-typedef uint64_t u64;
-#include <vdso/getrandom.h>
+void __weak __arch_chacha20_blocks_nostack(uint8_t *dst_bytes, const uint32_t *key, uint32_t *counter, size_t nblocks)
+{
+	ksft_exit_skip("Not implemented on architecture\n");
+}
 
 int main(int argc, char *argv[])
 {
diff --git a/tools/testing/selftests/vDSO/vgetrandom-chacha.S b/tools/testing/selftests/vDSO/vgetrandom-chacha.S
new file mode 100644
index 000000000000..d6e09af7c0a9
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vgetrandom-chacha.S
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#define __ASSEMBLY__
+
+#if defined(__aarch64__)
+#include "../../../../arch/arm64/kernel/vdso/vgetrandom-chacha.S"
+#elif defined(__loongarch__)
+#include "../../../../arch/loongarch/vdso/vgetrandom-chacha.S"
+#elif defined(__powerpc__) || defined(__powerpc64__)
+#include "../../../../arch/powerpc/kernel/vdso/vgetrandom-chacha.S"
+#elif defined(__s390x__)
+#include "../../../../arch/s390/kernel/vdso64/vgetrandom-chacha.S"
+#elif defined(__x86_64__)
+#include "../../../../arch/x86/entry/vdso/vgetrandom-chacha.S"
+#endif
-- 
2.46.0


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

* [PATCH kselftest v3 2/3] selftests: vDSO: unconditionally build getrandom test
  2024-10-07 21:45               ` [PATCH kselftest v3 1/3] " Jason A. Donenfeld
@ 2024-10-07 21:45                 ` Jason A. Donenfeld
  2024-10-07 21:45                 ` [PATCH kselftest v3 3/3] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
  2024-10-08 21:36                 ` [PATCH kselftest v3 1/3] selftests: vDSO: unconditionally build chacha test Shuah Khan
  2 siblings, 0 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-07 21:45 UTC (permalink / raw)
  To: linux-kselftest, greg, skhan; +Cc: Jason A. Donenfeld

Rather than building on supported archs, build on all archs, and then
use the presence of the symbol in the vDSO to either skip the test or
move forward with it.

Note that this means that this test no longer checks whether the symbol
was correctly added to the kernel. But hopefully this will be clear
enough to developers and we'll cross our fingers that symbols aren't
removed by accident and not caught after this change.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/testing/selftests/vDSO/Makefile              | 2 --
 tools/testing/selftests/vDSO/vdso_test_getrandom.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 45641386c662..1cf14a8da438 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -9,9 +9,7 @@ ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
 TEST_GEN_PROGS += vdso_standalone_test_x86
 endif
 TEST_GEN_PROGS += vdso_test_correctness
-ifeq ($(ARCH)$(CONFIG_X86_32),$(filter $(ARCH)$(CONFIG_X86_32),x86 x86_64 loongarch arm64 powerpc s390))
 TEST_GEN_PROGS += vdso_test_getrandom
-endif
 TEST_GEN_PROGS += vdso_test_chacha
 
 CFLAGS := -std=gnu99 -O2
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
index 72a1d9b43a84..5489a2f07434 100644
--- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
+++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
@@ -119,7 +119,7 @@ static void vgetrandom_init(void)
 	vgrnd.fn = (__typeof__(vgrnd.fn))vdso_sym(version, name);
 	if (!vgrnd.fn) {
 		printf("%s is missing!\n", name);
-		exit(KSFT_FAIL);
+		exit(KSFT_SKIP);
 	}
 	ret = VDSO_CALL(vgrnd.fn, 5, NULL, 0, 0, &vgrnd.params, ~0UL);
 	if (ret == -ENOSYS) {
-- 
2.46.0


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

* [PATCH kselftest v3 3/3] selftests: vDSO: improve getrandom and chacha error messages
  2024-10-07 21:45               ` [PATCH kselftest v3 1/3] " Jason A. Donenfeld
  2024-10-07 21:45                 ` [PATCH kselftest v3 2/3] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
@ 2024-10-07 21:45                 ` Jason A. Donenfeld
  2024-10-08 21:36                 ` [PATCH kselftest v3 1/3] selftests: vDSO: unconditionally build chacha test Shuah Khan
  2 siblings, 0 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2024-10-07 21:45 UTC (permalink / raw)
  To: linux-kselftest, greg, skhan; +Cc: Jason A. Donenfeld

Improve the error and skip condition messages to let the developer know
precisely where a test has failed. Also make better use of the ksft api
for this.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 .../testing/selftests/vDSO/vdso_test_chacha.c | 27 ++++---
 .../selftests/vDSO/vdso_test_getrandom.c      | 75 ++++++++-----------
 2 files changed, 49 insertions(+), 53 deletions(-)

diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c
index c66eb9df89bd..8757f738b0b1 100644
--- a/tools/testing/selftests/vDSO/vdso_test_chacha.c
+++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
@@ -91,10 +91,8 @@ int main(int argc, char *argv[])
 	ksft_set_plan(1);
 
 	for (unsigned int trial = 0; trial < TRIALS; ++trial) {
-		if (getrandom(key, sizeof(key), 0) != sizeof(key)) {
-			printf("getrandom() failed!\n");
-			return KSFT_SKIP;
-		}
+		if (getrandom(key, sizeof(key), 0) != sizeof(key))
+			ksft_exit_skip("getrandom() failed unexpectedly\n");
 		memset(counter1, 0, sizeof(counter1));
 		reference_chacha20_blocks(output1, key, counter1, BLOCKS);
 		for (unsigned int split = 0; split < BLOCKS; ++split) {
@@ -103,8 +101,10 @@ int main(int argc, char *argv[])
 			if (split)
 				__arch_chacha20_blocks_nostack(output2, key, counter2, split);
 			__arch_chacha20_blocks_nostack(output2 + split * BLOCK_SIZE, key, counter2, BLOCKS - split);
-			if (memcmp(output1, output2, sizeof(output1)) || memcmp(counter1, counter2, sizeof(counter1)))
-				return KSFT_FAIL;
+			if (memcmp(output1, output2, sizeof(output1)))
+				ksft_exit_fail_msg("Main loop outputs do not match on trial %u, split %u\n", trial, split);
+			if (memcmp(counter1, counter2, sizeof(counter1)))
+				ksft_exit_fail_msg("Main loop counters do not match on trial %u, split %u\n", trial, split);
 		}
 	}
 	memset(counter1, 0, sizeof(counter1));
@@ -114,14 +114,19 @@ int main(int argc, char *argv[])
 
 	reference_chacha20_blocks(output1, key, counter1, BLOCKS);
 	__arch_chacha20_blocks_nostack(output2, key, counter2, BLOCKS);
-	if (memcmp(output1, output2, sizeof(output1)) || memcmp(counter1, counter2, sizeof(counter1)))
-		return KSFT_FAIL;
+	if (memcmp(output1, output2, sizeof(output1)))
+		ksft_exit_fail_msg("Block limit outputs do not match after first round\n");
+	if (memcmp(counter1, counter2, sizeof(counter1)))
+		ksft_exit_fail_msg("Block limit counters do not match after first round\n");
 
 	reference_chacha20_blocks(output1, key, counter1, BLOCKS);
 	__arch_chacha20_blocks_nostack(output2, key, counter2, BLOCKS);
-	if (memcmp(output1, output2, sizeof(output1)) || memcmp(counter1, counter2, sizeof(counter1)))
-		return KSFT_FAIL;
+	if (memcmp(output1, output2, sizeof(output1)))
+		ksft_exit_fail_msg("Block limit outputs do not match after second round\n");
+	if (memcmp(counter1, counter2, sizeof(counter1)))
+		ksft_exit_fail_msg("Block limit counters do not match after second round\n");
 
 	ksft_test_result_pass("chacha: PASS\n");
-	return KSFT_PASS;
+	ksft_exit_pass();
+	return 0;
 }
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
index 5489a2f07434..50ec9ad4c7ec 100644
--- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
+++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
@@ -40,6 +40,9 @@
 	} while (0)
 #endif
 
+#define ksft_assert(condition) \
+	do { if (!(condition)) ksft_exit_fail_msg("Assertion failed: %s\n", #condition); } while (0)
+
 static struct {
 	pthread_mutex_t lock;
 	void **states;
@@ -109,26 +112,19 @@ static void vgetrandom_init(void)
 	const char *version = versions[VDSO_VERSION];
 	const char *name = names[VDSO_NAMES][6];
 	unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
-	size_t ret;
+	ssize_t ret;
 
-	if (!sysinfo_ehdr) {
-		printf("AT_SYSINFO_EHDR is not present!\n");
-		exit(KSFT_SKIP);
-	}
+	if (!sysinfo_ehdr)
+		ksft_exit_skip("AT_SYSINFO_EHDR is not present\n");
 	vdso_init_from_sysinfo_ehdr(sysinfo_ehdr);
 	vgrnd.fn = (__typeof__(vgrnd.fn))vdso_sym(version, name);
-	if (!vgrnd.fn) {
-		printf("%s is missing!\n", name);
-		exit(KSFT_SKIP);
-	}
+	if (!vgrnd.fn)
+		ksft_exit_skip("%s@%s symbol is missing from vDSO\n", name, version);
 	ret = VDSO_CALL(vgrnd.fn, 5, NULL, 0, 0, &vgrnd.params, ~0UL);
-	if (ret == -ENOSYS) {
-		printf("unsupported architecture\n");
-		exit(KSFT_SKIP);
-	} else if (ret) {
-		printf("failed to fetch vgetrandom params!\n");
-		exit(KSFT_FAIL);
-	}
+	if (ret == -ENOSYS)
+		ksft_exit_skip("CPU does not have runtime support\n");
+	else if (ret)
+		ksft_exit_fail_msg("Failed to fetch vgetrandom params: %zd\n", ret);
 }
 
 static ssize_t vgetrandom(void *buf, size_t len, unsigned long flags)
@@ -137,10 +133,7 @@ static ssize_t vgetrandom(void *buf, size_t len, unsigned long flags)
 
 	if (!state) {
 		state = vgetrandom_get_state();
-		if (!state) {
-			printf("vgetrandom_get_state failed!\n");
-			exit(KSFT_FAIL);
-		}
+		ksft_assert(state);
 	}
 	return VDSO_CALL(vgrnd.fn, 5, buf, len, flags, state, vgrnd.params.size_of_opaque_state);
 }
@@ -152,7 +145,7 @@ static void *test_vdso_getrandom(void *ctx)
 	for (size_t i = 0; i < TRIALS; ++i) {
 		unsigned int val;
 		ssize_t ret = vgetrandom(&val, sizeof(val), 0);
-		assert(ret == sizeof(val));
+		ksft_assert(ret == sizeof(val));
 	}
 	return NULL;
 }
@@ -162,7 +155,7 @@ static void *test_libc_getrandom(void *ctx)
 	for (size_t i = 0; i < TRIALS; ++i) {
 		unsigned int val;
 		ssize_t ret = getrandom(&val, sizeof(val), 0);
-		assert(ret == sizeof(val));
+		ksft_assert(ret == sizeof(val));
 	}
 	return NULL;
 }
@@ -172,7 +165,7 @@ static void *test_syscall_getrandom(void *ctx)
 	for (size_t i = 0; i < TRIALS; ++i) {
 		unsigned int val;
 		ssize_t ret = syscall(__NR_getrandom, &val, sizeof(val), 0);
-		assert(ret == sizeof(val));
+		ksft_assert(ret == sizeof(val));
 	}
 	return NULL;
 }
@@ -207,7 +200,7 @@ static void bench_multi(void)
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	for (size_t i = 0; i < THREADS; ++i)
-		assert(pthread_create(&threads[i], NULL, test_vdso_getrandom, NULL) == 0);
+		ksft_assert(pthread_create(&threads[i], NULL, test_vdso_getrandom, NULL) == 0);
 	for (size_t i = 0; i < THREADS; ++i)
 		pthread_join(threads[i], NULL);
 	clock_gettime(CLOCK_MONOTONIC, &end);
@@ -216,7 +209,7 @@ static void bench_multi(void)
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	for (size_t i = 0; i < THREADS; ++i)
-		assert(pthread_create(&threads[i], NULL, test_libc_getrandom, NULL) == 0);
+		ksft_assert(pthread_create(&threads[i], NULL, test_libc_getrandom, NULL) == 0);
 	for (size_t i = 0; i < THREADS; ++i)
 		pthread_join(threads[i], NULL);
 	clock_gettime(CLOCK_MONOTONIC, &end);
@@ -225,7 +218,7 @@ static void bench_multi(void)
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	for (size_t i = 0; i < THREADS; ++i)
-		assert(pthread_create(&threads[i], NULL, test_syscall_getrandom, NULL) == 0);
+		ksft_assert(pthread_create(&threads[i], NULL, test_syscall_getrandom, NULL) == 0);
 	for (size_t i = 0; i < THREADS; ++i)
 		pthread_join(threads[i], NULL);
 	clock_gettime(CLOCK_MONOTONIC, &end);
@@ -250,48 +243,46 @@ static void kselftest(void)
 
 	for (size_t i = 0; i < 1000; ++i) {
 		ssize_t ret = vgetrandom(weird_size, sizeof(weird_size), 0);
-		if (ret != sizeof(weird_size))
-			exit(KSFT_FAIL);
+		ksft_assert(ret == sizeof(weird_size));
 	}
 
 	ksft_test_result_pass("getrandom: PASS\n");
 
 	unshare(CLONE_NEWUSER);
-	assert(unshare(CLONE_NEWTIME) == 0);
+	ksft_assert(unshare(CLONE_NEWTIME) == 0);
 	child = fork();
-	assert(child >= 0);
+	ksft_assert(child >= 0);
 	if (!child) {
 		vgetrandom_init();
 		child = getpid();
-		assert(ptrace(PTRACE_TRACEME, 0, NULL, NULL) == 0);
-		assert(kill(child, SIGSTOP) == 0);
-		assert(vgetrandom(weird_size, sizeof(weird_size), 0) == sizeof(weird_size));
+		ksft_assert(ptrace(PTRACE_TRACEME, 0, NULL, NULL) == 0);
+		ksft_assert(kill(child, SIGSTOP) == 0);
+		ksft_assert(vgetrandom(weird_size, sizeof(weird_size), 0) == sizeof(weird_size));
 		_exit(0);
 	}
 	for (;;) {
 		struct ptrace_syscall_info info = { 0 };
 		int status, ret;
-		assert(waitpid(child, &status, 0) >= 0);
+		ksft_assert(waitpid(child, &status, 0) >= 0);
 		if (WIFEXITED(status)) {
-			if (WEXITSTATUS(status) != 0)
-				exit(KSFT_FAIL);
+			ksft_assert(WEXITSTATUS(status) == 0);
 			break;
 		}
-		assert(WIFSTOPPED(status));
+		ksft_assert(WIFSTOPPED(status));
 		if (WSTOPSIG(status) == SIGSTOP)
-			assert(ptrace(PTRACE_SETOPTIONS, child, 0, PTRACE_O_TRACESYSGOOD) == 0);
+			ksft_assert(ptrace(PTRACE_SETOPTIONS, child, 0, PTRACE_O_TRACESYSGOOD) == 0);
 		else if (WSTOPSIG(status) == (SIGTRAP | 0x80)) {
-			assert(ptrace(PTRACE_GET_SYSCALL_INFO, child, sizeof(info), &info) > 0);
+			ksft_assert(ptrace(PTRACE_GET_SYSCALL_INFO, child, sizeof(info), &info) > 0);
 			if (info.op == PTRACE_SYSCALL_INFO_ENTRY && info.entry.nr == __NR_getrandom &&
 			    info.entry.args[0] == (uintptr_t)weird_size && info.entry.args[1] == sizeof(weird_size))
-				exit(KSFT_FAIL);
+				ksft_exit_fail_msg("vgetrandom passed buffer to syscall getrandom unexpectedly\n");
 		}
-		assert(ptrace(PTRACE_SYSCALL, child, 0, 0) == 0);
+		ksft_assert(ptrace(PTRACE_SYSCALL, child, 0, 0) == 0);
 	}
 
 	ksft_test_result_pass("getrandom timens: PASS\n");
 
-	exit(KSFT_PASS);
+	ksft_exit_pass();
 }
 
 static void usage(const char *argv0)
-- 
2.46.0


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

* Re: [PATCH kselftest v3 1/3] selftests: vDSO: unconditionally build chacha test
  2024-10-07 21:45               ` [PATCH kselftest v3 1/3] " Jason A. Donenfeld
  2024-10-07 21:45                 ` [PATCH kselftest v3 2/3] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
  2024-10-07 21:45                 ` [PATCH kselftest v3 3/3] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
@ 2024-10-08 21:36                 ` Shuah Khan
  2 siblings, 0 replies; 17+ messages in thread
From: Shuah Khan @ 2024-10-08 21:36 UTC (permalink / raw)
  To: Jason A. Donenfeld, linux-kselftest, greg; +Cc: Shuah Khan

On 10/7/24 15:45, Jason A. Donenfeld wrote:
> Rather than using symlinks to find the vgetrandom-chacha.S file for each
> arch, store this in a file that uses the compiler to determine
> architecture, and then make use of weak symbols to skip the test on
> architectures that don't provide the code.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---

Thank you.

Applied all three patches to linux-kselftest fixes and will
include them in my next pull request for rc3.

thanks,
-- Shuah


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

end of thread, other threads:[~2024-10-08 21:37 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <Zv20olVBlnxL9UnS@zx2c4.com>
2024-10-03  3:13 ` [PATCH kselftest 0/3] getrandom & chacha cleanups Jason A. Donenfeld
2024-10-03  3:13   ` [PATCH kselftest 1/3] selftests: vDSO: condition chacha build on chacha implementation Jason A. Donenfeld
2024-10-03  3:13   ` [PATCH kselftest 2/3] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
2024-10-03  3:13   ` [PATCH kselftest 3/3] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
2024-10-06  3:35     ` kernel test robot
2024-10-06  4:35       ` Jason A. Donenfeld
2024-10-06  4:40         ` [PATCH kselftest v2 1/4] selftests: vDSO: condition chacha build on chacha implementation Jason A. Donenfeld
2024-10-06  4:40           ` [PATCH kselftest v2 2/4] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
2024-10-06  4:40           ` [PATCH kselftest v2 3/4] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
2024-10-06  4:40           ` [PATCH kselftest v2 4/4] selftests: vDSO: unconditionally build chacha test Jason A. Donenfeld
2024-10-07 20:53             ` Shuah Khan
2024-10-07 21:45               ` [PATCH kselftest v3 1/3] " Jason A. Donenfeld
2024-10-07 21:45                 ` [PATCH kselftest v3 2/3] selftests: vDSO: unconditionally build getrandom test Jason A. Donenfeld
2024-10-07 21:45                 ` [PATCH kselftest v3 3/3] selftests: vDSO: improve getrandom and chacha error messages Jason A. Donenfeld
2024-10-08 21:36                 ` [PATCH kselftest v3 1/3] selftests: vDSO: unconditionally build chacha test Shuah Khan
2024-10-03 17:18   ` [PATCH kselftest 4/3] " Jason A. Donenfeld
2024-10-03 22:42   ` [PATCH kselftest 0/3] getrandom & chacha cleanups Shuah Khan

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