[PATCH v3 0/4] selftest: x86: improve tests

[PATCH v3 0/4] selftest: x86: improve tests

[Muhammad Usama Anjum]

Changes since v1:
- Correct the description of patches with what improvements they are
  bringing and why they are required

Changes since v2:
- Update descriptions of patches and add output before/after

Muhammad Usama Anjum (4):
  selftests: x86: check_initial_reg_state: remove manual counting and
    increase maintainability
  selftests: x86: corrupt_xstate_header: remove manual counting and
    increase maintainability
  selftests: x86: fsgsbase_restore: remove manual counting and increase
    maintainability
  selftests: x86: entry_from_vm86: remove manual counting and increase
    maintainability

 .../selftests/x86/check_initial_reg_state.c   |  24 ++--
 .../selftests/x86/corrupt_xstate_header.c     |  30 +++--
 tools/testing/selftests/x86/entry_from_vm86.c | 109 ++++++++--------
 .../testing/selftests/x86/fsgsbase_restore.c  | 117 +++++++++---------
 4 files changed, 139 insertions(+), 141 deletions(-)

-- 
2.39.2

[PATCH v3 1/4] selftests: x86: check_initial_reg_state: remove manual counting and increase maintainability

[Muhammad Usama Anjum]

Removes manual counting of pass and fail tests. This increases readability
of tests, but also improves maintainability of the tests. Print logs in
standard format (without [RUN], [OK] tags)

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- correct description of the patch

Changes since v2:
- Update description of the patch and add before/after output

Before:
  # selftests: x86: check_initial_reg_state_32
  # [OK]	All GPRs except SP are 0
  # [OK]	FLAGS is 0x202
  ok 5 selftests: x86: check_initial_reg_state_32

After:
  # selftests: x86: check_initial_reg_state_32
  # TAP version 13
  # 1..2
  # ok 1 All GPRs except SP are 0
  # ok 2 FLAGS is 0x202
  # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
  ok 5 selftests: x86: check_initial_reg_state_32
---
 .../selftests/x86/check_initial_reg_state.c   | 24 +++++++++----------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/x86/check_initial_reg_state.c b/tools/testing/selftests/x86/check_initial_reg_state.c
index 3bc95f3ed5859..b91c2b06b9881 100644
--- a/tools/testing/selftests/x86/check_initial_reg_state.c
+++ b/tools/testing/selftests/x86/check_initial_reg_state.c
@@ -7,6 +7,7 @@
 #define _GNU_SOURCE
 
 #include <stdio.h>
+#include "../kselftest.h"
 
 unsigned long ax, bx, cx, dx, si, di, bp, sp, flags;
 unsigned long r8, r9, r10, r11, r12, r13, r14, r15;
@@ -53,20 +54,19 @@ asm (
 
 int main()
 {
-	int nerrs = 0;
+	ksft_print_header();
+	ksft_set_plan(2);
 
-	if (sp == 0) {
-		printf("[FAIL]\tTest was built incorrectly\n");
-		return 1;
-	}
+	if (sp == 0)
+		ksft_exit_fail_msg("Test was built incorrectly\n");
 
 	if (ax || bx || cx || dx || si || di || bp
 #ifdef __x86_64__
 	    || r8 || r9 || r10 || r11 || r12 || r13 || r14 || r15
 #endif
 		) {
-		printf("[FAIL]\tAll GPRs except SP should be 0\n");
-#define SHOW(x) printf("\t" #x " = 0x%lx\n", x);
+		ksft_test_result_fail("All GPRs except SP should be 0\n");
+#define SHOW(x) ksft_print_msg("\t" #x " = 0x%lx\n", x)
 		SHOW(ax);
 		SHOW(bx);
 		SHOW(cx);
@@ -85,17 +85,15 @@ int main()
 		SHOW(r14);
 		SHOW(r15);
 #endif
-		nerrs++;
 	} else {
-		printf("[OK]\tAll GPRs except SP are 0\n");
+		ksft_test_result_pass("All GPRs except SP are 0\n");
 	}
 
 	if (flags != 0x202) {
-		printf("[FAIL]\tFLAGS is 0x%lx, but it should be 0x202\n", flags);
-		nerrs++;
+		ksft_test_result_fail("FLAGS is 0x%lx, but it should be 0x202\n", flags);
 	} else {
-		printf("[OK]\tFLAGS is 0x202\n");
+		ksft_test_result_pass("FLAGS is 0x202\n");
 	}
 
-	return nerrs ? 1 : 0;
+	ksft_finished();
 }
-- 
2.39.2

Re: [PATCH v3 1/4] selftests: x86: check_initial_reg_state: remove manual counting and increase maintainability

[Shuah Khan]

On 7/18/24 05:32, Muhammad Usama Anjum wrote:
> Removes manual counting of pass and fail tests. This increases readability
> of tests, but also improves maintainability of the tests. Print logs in
> standard format (without [RUN], [OK] tags)
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - correct description of the patch
> 
> Changes since v2:
> - Update description of the patch and add before/after output
> 
> Before:
>    # selftests: x86: check_initial_reg_state_32
>    # [OK]	All GPRs except SP are 0
>    # [OK]	FLAGS is 0x202
>    ok 5 selftests: x86: check_initial_reg_state_32
> 
> After:
>    # selftests: x86: check_initial_reg_state_32
>    # TAP version 13
>    # 1..2
>    # ok 1 All GPRs except SP are 0
>    # ok 2 FLAGS is 0x202
>    # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
>    ok 5 selftests: x86: check_initial_reg_state_32

What's the output you see if you were run this as:

make ksefltest TARGETS=x86

How is this different from the output from the above command?

Please provide the same information for your other patches in this
series

thanks,
-- Shuah

Re: [PATCH v3 1/4] selftests: x86: check_initial_reg_state: remove manual counting and increase maintainability

[Muhammad Usama Anjum]

On 7/18/24 8:48 PM, Shuah Khan wrote:
> On 7/18/24 05:32, Muhammad Usama Anjum wrote:
>> Removes manual counting of pass and fail tests. This increases readability
>> of tests, but also improves maintainability of the tests. Print logs in
>> standard format (without [RUN], [OK] tags)
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Changes since v1:
>> - correct description of the patch
>>
>> Changes since v2:
>> - Update description of the patch and add before/after output
>>
>> Before:
>>    # selftests: x86: check_initial_reg_state_32
>>    # [OK]    All GPRs except SP are 0
>>    # [OK]    FLAGS is 0x202
>>    ok 5 selftests: x86: check_initial_reg_state_32
>>
>> After:
>>    # selftests: x86: check_initial_reg_state_32
>>    # TAP version 13
>>    # 1..2
>>    # ok 1 All GPRs except SP are 0
>>    # ok 2 FLAGS is 0x202
>>    # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
>>    ok 5 selftests: x86: check_initial_reg_state_32
> 
> What's the output you see if you were run this as:
> 
> make ksefltest TARGETS=x86
> 
> How is this different from the output from the above command?
The above before and after output has been taken by executing this above
command. I've copy/pasted the snippets for this patch only.

> 
> Please provide the same information for your other patches in this
> series
All other patches have this information already.


> 
> thanks,
> -- Shuah
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v3 1/4] selftests: x86: check_initial_reg_state: remove manual counting and increase maintainability

[Shuah Khan]

On 7/19/24 01:28, Muhammad Usama Anjum wrote:
> On 7/18/24 8:48 PM, Shuah Khan wrote:
>> On 7/18/24 05:32, Muhammad Usama Anjum wrote:
>>> Removes manual counting of pass and fail tests. This increases readability
>>> of tests, but also improves maintainability of the tests. Print logs in
>>> standard format (without [RUN], [OK] tags)
>>>
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>>> Changes since v1:
>>> - correct description of the patch
>>>
>>> Changes since v2:
>>> - Update description of the patch and add before/after output
>>>
>>> Before:
>>>     # selftests: x86: check_initial_reg_state_32
>>>     # [OK]    All GPRs except SP are 0
>>>     # [OK]    FLAGS is 0x202
>>>     ok 5 selftests: x86: check_initial_reg_state_32
>>>
>>> After:
>>>     # selftests: x86: check_initial_reg_state_32
>>>     # TAP version 13
>>>     # 1..2
>>>     # ok 1 All GPRs except SP are 0
>>>     # ok 2 FLAGS is 0x202
>>>     # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
>>>     ok 5 selftests: x86: check_initial_reg_state_32
>>
>> What's the output you see if you were run this as:
>>
>> make ksefltest TARGETS=x86
>>

That is what is confusing to me. As mentioned in response to your
vDSO patch, this change to add ksft_header to individual tests.

When you run the test from the wrapper
if you want the header printed for q

>> How is this different from the output from the above command?
> The above before and after output has been taken by executing this above
> command. I've copy/pasted the snippets for this patch only.
> 

Yes. That is the problem. Youa re giving me snippets as opposed to
the header. When I run it I see TAP header at the top of the test
suite. The idea is that the TAP header should not be printed for
each test in the test suite.

It is printed once for test suite. What is the point in printing TAP
header for each of the tests in test suite if there are 100 tests
like in the case of break_points test.

Even if it is desired that each individual should have TAP header
(There has to be a good reason why - not because we want to see it),
this change should be added to
   
tools/testing/selftests/kselftest/runner.sh
This makes it uniform and maintainable.

Sorry. I am not going accept patches that add ksft header to individual
tests and test cases.

thanks,
-- Shuah

[PATCH v3 2/4] selftests: x86: corrupt_xstate_header: remove manual counting and increase maintainability

[Muhammad Usama Anjum]

Correct the skip, pass and fail return codes. Otherwise the test always
return 0. After the chaanges, correct number of stats about passed or
failed tests is being returned. Also print logs in standard format.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- correct description of the patch

Changes since v2:
- Update description of the patch and add before/after output

Before:
  # selftests: x86: corrupt_xstate_header_64
  # [RUN]	Send ourselves a signal
  # 	Wreck XSTATE header
  # 	Got SIGSEGV
  # [OK]	Back from the signal.  Now schedule.
  # [OK]	Back in the main thread.
  ok 41 selftests: x86: corrupt_xstate_header_64

After:
  # selftests: x86: corrupt_xstate_header_64
  # TAP version 13
  # 1..2
  # # Send ourselves a signal
  # # Wreck XSTATE header
  # # Got SIGSEGV
  # ok 1 Back from the signal. Now schedule.
  # ok 2 Back in the main thread.
  # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
  ok 41 selftests: x86: corrupt_xstate_header_64
---
 .../selftests/x86/corrupt_xstate_header.c     | 30 +++++++++++--------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/x86/corrupt_xstate_header.c b/tools/testing/selftests/x86/corrupt_xstate_header.c
index cf9ce8fbb656c..119ff74763c6d 100644
--- a/tools/testing/selftests/x86/corrupt_xstate_header.c
+++ b/tools/testing/selftests/x86/corrupt_xstate_header.c
@@ -39,7 +39,7 @@ static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
 	sa.sa_flags = SA_SIGINFO | flags;
 	sigemptyset(&sa.sa_mask);
 	if (sigaction(sig, &sa, 0))
-		err(1, "sigaction");
+		ksft_exit_fail_perror("sigaction");
 }
 
 static void sigusr1(int sig, siginfo_t *info, void *uc_void)
@@ -48,26 +48,29 @@ static void sigusr1(int sig, siginfo_t *info, void *uc_void)
 	uint8_t *fpstate = (uint8_t *)uc->uc_mcontext.fpregs;
 	uint64_t *xfeatures = (uint64_t *)(fpstate + 512);
 
-	printf("\tWreck XSTATE header\n");
+	ksft_print_msg("Wreck XSTATE header\n");
 	/* Wreck the first reserved bytes in the header */
 	*(xfeatures + 2) = 0xfffffff;
 }
 
 static void sigsegv(int sig, siginfo_t *info, void *uc_void)
 {
-	printf("\tGot SIGSEGV\n");
+	ksft_print_msg("Got SIGSEGV\n");
 }
 
 int main(void)
 {
 	cpu_set_t set;
 
+	ksft_print_header();
+	ksft_set_plan(2);
+
 	sethandler(SIGUSR1, sigusr1, 0);
 	sethandler(SIGSEGV, sigsegv, 0);
 
 	if (!xsave_enabled()) {
-		printf("[SKIP] CR4.OSXSAVE disabled.\n");
-		return 0;
+		ksft_print_msg("CR4.OSXSAVE disabled.\n");
+		return KSFT_SKIP;
 	}
 
 	CPU_ZERO(&set);
@@ -79,18 +82,21 @@ int main(void)
 	 */
 	sched_setaffinity(getpid(), sizeof(set), &set);
 
-	printf("[RUN]\tSend ourselves a signal\n");
+	ksft_print_msg("Send ourselves a signal\n");
 	raise(SIGUSR1);
 
-	printf("[OK]\tBack from the signal.  Now schedule.\n");
+	ksft_test_result_pass("Back from the signal. Now schedule.\n");
+
 	pid_t child = fork();
-	if (child < 0)
-		err(1, "fork");
 	if (child == 0)
 		return 0;
-	if (child)
+
+	if (child < 0) {
+		ksft_test_result_fail("fork: %s\n", strerror(errno));
+	} else if (child) {
 		waitpid(child, NULL, 0);
-	printf("[OK]\tBack in the main thread.\n");
+		ksft_test_result_pass("Back in the main thread.\n");
+	}
 
 	/*
 	 * We could try to confirm that extended state is still preserved
@@ -98,5 +104,5 @@ int main(void)
 	 * a warning in the kernel logs.
 	 */
 
-	return 0;
+	ksft_finished();
 }
-- 
2.39.2

[PATCH v3 3/4] selftests: x86: fsgsbase_restore: remove manual counting and increase maintainability

[Muhammad Usama Anjum]

Add the test skip support instead of returning success at skip time for
the kselftest runner script to understand correct exit status. Add logs
to print details about errno if error occurs. Needless to say, all of
this improves readability and maintainability.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- correct description of the patch

Changes since v2:
- Update description of the patch and add before/after output

Before:
  # selftests: x86: fsgsbase_restore_32
  # ERROR: ld.so: object '/usr/libexec/coreutils/libstdbuf.so' from LD_PRELOAD cannot be preloaded (wrong ELF class: ELFCLASS64): ignored.
  # 	Setting up a segment
  # 	segment base address = 0xf7f38000
  # 	using LDT slot 0
  # [OK]	The segment points to the right place.
  # 	Tracee will take a nap until signaled
  # 	Tracee: in tracee_zap_segment()
  # 	Tracee is going back to sleep
  # 	Tracee was resumed.  Will re-check segment.
  # [OK]	The segment points to the right place.
  # 	Setting up a segment
  # 	segment base address = 0xf7f38000
  # 	using LDT slot 0
  # [OK]	The segment points to the right place.
  # 	Child FS=0x7
  # 	Tracer: redirecting tracee to tracee_zap_segment()
  # 	Tracer: restoring tracee state
  # [OK]	All is well.
  ok 12 selftests: x86: fsgsbase_restore_32

After:
  # selftests: x86: fsgsbase_restore_32
  # ERROR: ld.so: object '/usr/libexec/coreutils/libstdbuf.so' from LD_PRELOAD cannot be preloaded (wrong ELF class: ELFCLASS64): ignored.
  # TAP version 13
  # 1..2
  # # Setting up a segment
  # # segment base address = 0xf7f76000
  # # using LDT slot 0
  # ok 1 The segment points to the right place.
  # # Tracee will take a nap until signaled
  # # Child FS=0x7
  # # Tracer: redirecting tracee to tracee_zap_segment()
  # # Tracee: in tracee_zap_segment()
  # # Tracee is going back to sleep
  # # Tracer: restoring tracee state
  # # Tracee was resumed. Will re-check segment.
  # # The segment points to the right place.
  # ok 2 Tracee exited correctly
  # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
  ok 12 selftests: x86: fsgsbase_restore_32
---
 .../testing/selftests/x86/fsgsbase_restore.c  | 117 +++++++++---------
 1 file changed, 57 insertions(+), 60 deletions(-)

diff --git a/tools/testing/selftests/x86/fsgsbase_restore.c b/tools/testing/selftests/x86/fsgsbase_restore.c
index 224058c1e4b2e..ef326c08d3508 100644
--- a/tools/testing/selftests/x86/fsgsbase_restore.c
+++ b/tools/testing/selftests/x86/fsgsbase_restore.c
@@ -30,6 +30,7 @@
 #include <sys/ptrace.h>
 #include <sys/wait.h>
 #include <stdint.h>
+#include "../kselftest.h"
 
 #define EXPECTED_VALUE 0x1337f00d
 
@@ -45,18 +46,18 @@
  */
 unsigned int dereference_seg_base(void);
 
-static void init_seg(void)
+static int init_seg(void)
 {
 	unsigned int *target = mmap(
 		NULL, sizeof(unsigned int),
 		PROT_READ | PROT_WRITE,
 		MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT, -1, 0);
 	if (target == MAP_FAILED)
-		err(1, "mmap");
+		ksft_exit_fail_perror("mmap");
 
 	*target = EXPECTED_VALUE;
 
-	printf("\tsegment base address = 0x%lx\n", (unsigned long)target);
+	ksft_print_msg("segment base address = 0x%lx\n", (unsigned long)target);
 
 	struct user_desc desc = {
 		.entry_number    = 0,
@@ -70,7 +71,7 @@ static void init_seg(void)
 		.useable         = 0
 	};
 	if (syscall(SYS_modify_ldt, 1, &desc, sizeof(desc)) == 0) {
-		printf("\tusing LDT slot 0\n");
+		ksft_print_msg("using LDT slot 0\n");
 		asm volatile ("mov %0, %" SEG :: "rm" ((unsigned short)0x7));
 	} else {
 		/* No modify_ldt for us (configured out, perhaps) */
@@ -96,14 +97,16 @@ static void init_seg(void)
 		munmap(low_desc, sizeof(desc));
 
 		if (ret != 0) {
-			printf("[NOTE]\tcould not create a segment -- can't test anything\n");
-			exit(0);
+			ksft_print_msg("could not create a segment -- can't test anything\n");
+			return KSFT_SKIP;
 		}
-		printf("\tusing GDT slot %d\n", desc.entry_number);
+		ksft_print_msg("using GDT slot %d\n", desc.entry_number);
 
 		unsigned short sel = (unsigned short)((desc.entry_number << 3) | 0x3);
 		asm volatile ("mov %0, %" SEG :: "rm" (sel));
 	}
+
+	return 0;
 }
 
 static void tracee_zap_segment(void)
@@ -114,7 +117,7 @@ static void tracee_zap_segment(void)
 	 * we modify a segment register in order to make sure that ptrace
 	 * can correctly restore segment registers.
 	 */
-	printf("\tTracee: in tracee_zap_segment()\n");
+	ksft_print_msg("Tracee: in tracee_zap_segment()\n");
 
 	/*
 	 * Write a nonzero selector with base zero to the segment register.
@@ -129,70 +132,72 @@ static void tracee_zap_segment(void)
 
 	pid_t pid = getpid(), tid = syscall(SYS_gettid);
 
-	printf("\tTracee is going back to sleep\n");
+	ksft_print_msg("Tracee is going back to sleep\n");
 	syscall(SYS_tgkill, pid, tid, SIGSTOP);
 
 	/* Should not get here. */
-	while (true) {
-		printf("[FAIL]\tTracee hit unreachable code\n");
-		pause();
-	}
+	ksft_exit_fail_msg("Tracee hit unreachable code\n");
 }
 
 int main()
 {
-	printf("\tSetting up a segment\n");
-	init_seg();
+	int ret;
+
+	ksft_print_header();
+	ksft_set_plan(2);
+
+	ksft_print_msg("Setting up a segment\n");
+
+	ret = init_seg();
+	if (ret)
+		return ret;
 
 	unsigned int val = dereference_seg_base();
-	if (val != EXPECTED_VALUE) {
-		printf("[FAIL]\tseg[0] == %x; should be %x\n", val, EXPECTED_VALUE);
-		return 1;
-	}
-	printf("[OK]\tThe segment points to the right place.\n");
+	ksft_test_result(val == EXPECTED_VALUE, "The segment points to the right place.\n");
 
 	pid_t chld = fork();
 	if (chld < 0)
-		err(1, "fork");
+		ksft_exit_fail_perror("fork");
 
 	if (chld == 0) {
 		prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0, 0);
 
 		if (ptrace(PTRACE_TRACEME, 0, 0, 0) != 0)
-			err(1, "PTRACE_TRACEME");
+			ksft_exit_fail_perror("PTRACE_TRACEME");
 
 		pid_t pid = getpid(), tid = syscall(SYS_gettid);
 
-		printf("\tTracee will take a nap until signaled\n");
+		ksft_print_msg("Tracee will take a nap until signaled\n");
 		syscall(SYS_tgkill, pid, tid, SIGSTOP);
 
-		printf("\tTracee was resumed.  Will re-check segment.\n");
+		ksft_print_msg("Tracee was resumed. Will re-check segment.\n");
 
 		val = dereference_seg_base();
-		if (val != EXPECTED_VALUE) {
-			printf("[FAIL]\tseg[0] == %x; should be %x\n", val, EXPECTED_VALUE);
-			exit(1);
+
+		if (val == EXPECTED_VALUE) {
+			ksft_print_msg("The segment points to the right place.\n");
+			return EXIT_SUCCESS;
 		}
 
-		printf("[OK]\tThe segment points to the right place.\n");
-		exit(0);
+		ksft_print_msg("seg[0] == %x; should be %x\n", val, EXPECTED_VALUE);
+		return EXIT_FAILURE;
 	}
 
 	int status;
 
 	/* Wait for SIGSTOP. */
 	if (waitpid(chld, &status, 0) != chld || !WIFSTOPPED(status))
-		err(1, "waitpid");
+		ksft_exit_fail_perror("waitpid");
 
 	struct user_regs_struct regs;
 
 	if (ptrace(PTRACE_GETREGS, chld, NULL, &regs) != 0)
-		err(1, "PTRACE_GETREGS");
+		ksft_exit_fail_perror("PTRACE_GETREGS");
 
 #ifdef __x86_64__
-	printf("\tChild GS=0x%lx, GSBASE=0x%lx\n", (unsigned long)regs.gs, (unsigned long)regs.gs_base);
+	ksft_print_msg("Child GS=0x%lx, GSBASE=0x%lx\n", (unsigned long)regs.gs, (unsigned long)regs.gs_base);
 #else
-	printf("\tChild FS=0x%lx\n", (unsigned long)regs.xfs);
+	ksft_print_msg("Child FS=0x%lx\n", (unsigned long)regs.xfs);
 #endif
 
 	struct user_regs_struct regs2 = regs;
@@ -203,42 +208,34 @@ int main()
 	regs2.eip = (unsigned long)tracee_zap_segment;
 #endif
 
-	printf("\tTracer: redirecting tracee to tracee_zap_segment()\n");
+	ksft_print_msg("Tracer: redirecting tracee to tracee_zap_segment()\n");
 	if (ptrace(PTRACE_SETREGS, chld, NULL, &regs2) != 0)
-		err(1, "PTRACE_GETREGS");
+		ksft_exit_fail_perror("PTRACE_GETREGS");
 	if (ptrace(PTRACE_CONT, chld, NULL, NULL) != 0)
-		err(1, "PTRACE_GETREGS");
+		ksft_exit_fail_perror("PTRACE_GETREGS");
 
 	/* Wait for SIGSTOP. */
 	if (waitpid(chld, &status, 0) != chld || !WIFSTOPPED(status))
-		err(1, "waitpid");
+		ksft_exit_fail_perror("waitpid");
 
-	printf("\tTracer: restoring tracee state\n");
+	ksft_print_msg("Tracer: restoring tracee state\n");
 	if (ptrace(PTRACE_SETREGS, chld, NULL, &regs) != 0)
-		err(1, "PTRACE_GETREGS");
+		ksft_exit_fail_perror("PTRACE_GETREGS");
 	if (ptrace(PTRACE_DETACH, chld, NULL, NULL) != 0)
-		err(1, "PTRACE_GETREGS");
+		ksft_exit_fail_perror("PTRACE_GETREGS");
 
 	/* Wait for SIGSTOP. */
 	if (waitpid(chld, &status, 0) != chld)
-		err(1, "waitpid");
-
-	if (WIFSIGNALED(status)) {
-		printf("[FAIL]\tTracee crashed\n");
-		return 1;
-	}
-
-	if (!WIFEXITED(status)) {
-		printf("[FAIL]\tTracee stopped for an unexpected reason: %d\n", status);
-		return 1;
-	}
-
-	int exitcode = WEXITSTATUS(status);
-	if (exitcode != 0) {
-		printf("[FAIL]\tTracee reported failure\n");
-		return 1;
-	}
-
-	printf("[OK]\tAll is well.\n");
-	return 0;
+		ksft_exit_fail_perror("waitpid");
+
+	if (WIFSIGNALED(status))
+		ksft_test_result_fail("Tracee crashed\n");
+	else if (!WIFEXITED(status))
+		ksft_test_result_fail("Tracee stopped for an unexpected reason: %d\n", status);
+	else if (WEXITSTATUS(status) != 0)
+		ksft_test_result_fail("Tracee reported failure\n");
+	else
+		ksft_test_result_pass("Tracee exited correctly\n");
+
+	ksft_finished();
 }
-- 
2.39.2

[PATCH v3 4/4] selftests: x86: entry_from_vm86: remove manual counting and increase maintainability

[Muhammad Usama Anjum]

Remove the manual pass/fail tests counting and use kselftests helpers
which improves readability as well. Print details about errno if error
occurs and also print logs in standard format.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- correct description of the patch

Changes since v2:
- Update description of the patch and add before/after output

Before:
  # selftests: x86: entry_from_vm86_32
  # ERROR: ld.so: object '/usr/libexec/coreutils/libstdbuf.so' from LD_PRELOAD cannot be preloaded (wrong ELF class: ELFCLASS64): ignored.
  # [RUN]	#BR from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	SYSENTER from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	SYSCALL from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	STI with VIP set from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	POPF with VIP set and IF clear from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	POPF with VIP and IF set from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	POPF with VIP clear and IF set from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	INT3 from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	int80 from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	UMIP tests from vm86 mode
  # [SKIP]	vm86 not supported
  # [INFO]	Result from SMSW:[0x0000]
  # [INFO]	Result from SIDT: limit[0x0000]base[0x00000000]
  # [INFO]	Result from SGDT: limit[0x0000]base[0x00000000]
  # [PASS]	All the results from SMSW are identical.
  # [PASS]	All the results from SGDT are identical.
  # [PASS]	All the results from SIDT are identical.
  # [RUN]	STR instruction from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	SLDT instruction from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	Execute null pointer from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	#BR from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	SYSENTER from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	SYSCALL from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	STI with VIP set from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	POPF with VIP set and IF clear from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	POPF with VIP and IF set from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	POPF with VIP clear and IF set from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	INT3 from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	int80 from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	UMIP tests from vm86 mode
  # [SKIP]	vm86 not supported
  # [INFO]	Result from SMSW:[0x0000]
  # [INFO]	Result from SIDT: limit[0x0000]base[0x00000000]
  # [INFO]	Result from SGDT: limit[0x0000]base[0x00000000]
  # [PASS]	All the results from SMSW are identical.
  # [PASS]	All the results from SGDT are identical.
  # [PASS]	All the results from SIDT are identical.
  # [RUN]	STR instruction from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	SLDT instruction from vm86 mode
  # [SKIP]	vm86 not supported
  # [RUN]	Execute null pointer from vm86 mode
  # [SKIP]	vm86 not supported
  ok 15 selftests: x86: entry_from_vm86_32

After:
  # selftests: x86: entry_from_vm86_32
  # ERROR: ld.so: object '/usr/libexec/coreutils/libstdbuf.so' from LD_PRELOAD cannot be preloaded (wrong ELF class: ELFCLASS64): ignored.
  # TAP version 13
  # 1..18
  # # #BR from vm86 mode
  # ok 1 # SKIP vm86 not supported
  # # SYSENTER from vm86 mode
  # ok 2 # SKIP vm86 not supported
  # # SYSCALL from vm86 mode
  # ok 3 # SKIP vm86 not supported
  # # STI with VIP set from vm86 mode
  # ok 4 # SKIP vm86 not supported
  # # POPF with VIP set and IF clear from vm86 mode
  # ok 5 # SKIP vm86 not supported
  # # POPF with VIP and IF set from vm86 mode
  # ok 6 # SKIP vm86 not supported
  # # POPF with VIP clear and IF set from vm86 mode
  # ok 7 # SKIP vm86 not supported
  # # INT3 from vm86 mode
  # ok 8 # SKIP vm86 not supported
  # # int80 from vm86 mode
  # ok 9 # SKIP vm86 not supported
  # # UMIP tests from vm86 mode
  # ok 10 # SKIP vm86 not supported
  # # Result from SMSW:[0x0000]
  # # Result from SIDT: limit[0x0000]base[0x00000000]
  # # Result from SGDT: limit[0x0000]base[0x00000000]
  # ok 11 All the results from SMSW are identical.
  # ok 12 All the results from SGDT are identical.
  # ok 13 All the results from SIDT are identical.
  # # STR instruction from vm86 mode
  # ok 14 # SKIP vm86 not supported
  # # SLDT instruction from vm86 mode
  # ok 15 # SKIP vm86 not supported
  # # Execute null pointer from vm86 mode
  # ok 16 # SKIP vm86 not supported
  # ok 17 # SKIP Received SIGSEGV
  # ok 18 fork succeeded
  # # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:14 error:0
  ok 15 selftests: x86: entry_from_vm86_32
---
 tools/testing/selftests/x86/entry_from_vm86.c | 109 +++++++++---------
 1 file changed, 53 insertions(+), 56 deletions(-)

diff --git a/tools/testing/selftests/x86/entry_from_vm86.c b/tools/testing/selftests/x86/entry_from_vm86.c
index d1e919b0c1dc8..a4efa4588e6f8 100644
--- a/tools/testing/selftests/x86/entry_from_vm86.c
+++ b/tools/testing/selftests/x86/entry_from_vm86.c
@@ -23,9 +23,9 @@
 #include <stdbool.h>
 #include <errno.h>
 #include <sys/vm86.h>
+#include "../kselftest.h"
 
 static unsigned long load_addr = 0x10000;
-static int nerrs = 0;
 
 static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
 		       int flags)
@@ -36,7 +36,7 @@ static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
 	sa.sa_flags = SA_SIGINFO | flags;
 	sigemptyset(&sa.sa_mask);
 	if (sigaction(sig, &sa, 0))
-		err(1, "sigaction");
+		ksft_exit_fail_perror("sigaction");
 }
 
 static void clearhandler(int sig)
@@ -46,7 +46,7 @@ static void clearhandler(int sig)
 	sa.sa_handler = SIG_DFL;
 	sigemptyset(&sa.sa_mask);
 	if (sigaction(sig, &sa, 0))
-		err(1, "sigaction");
+		ksft_exit_fail_perror("sigaction");
 }
 
 static sig_atomic_t got_signal;
@@ -56,10 +56,8 @@ static void sighandler(int sig, siginfo_t *info, void *ctx_void)
 	ucontext_t *ctx = (ucontext_t*)ctx_void;
 
 	if (ctx->uc_mcontext.gregs[REG_EFL] & X86_EFLAGS_VM ||
-	    (ctx->uc_mcontext.gregs[REG_CS] & 3) != 3) {
-		printf("[FAIL]\tSignal frame should not reflect vm86 mode\n");
-		nerrs++;
-	}
+	    (ctx->uc_mcontext.gregs[REG_CS] & 3) != 3)
+		ksft_test_result_fail("Signal frame should not reflect vm86 mode\n");
 
 	const char *signame;
 	if (sig == SIGSEGV)
@@ -69,9 +67,9 @@ static void sighandler(int sig, siginfo_t *info, void *ctx_void)
 	else
 		signame = "unexpected signal";
 
-	printf("[INFO]\t%s: FLAGS = 0x%lx, CS = 0x%hx\n", signame,
-	       (unsigned long)ctx->uc_mcontext.gregs[REG_EFL],
-	       (unsigned short)ctx->uc_mcontext.gregs[REG_CS]);
+	ksft_test_result_pass("%s: FLAGS = 0x%lx, CS = 0x%hx\n", signame,
+			      (unsigned long)ctx->uc_mcontext.gregs[REG_EFL],
+			      (unsigned short)ctx->uc_mcontext.gregs[REG_CS]);
 
 	got_signal = 1;
 }
@@ -137,13 +135,13 @@ static bool do_test(struct vm86plus_struct *v86, unsigned long eip,
 {
 	long ret;
 
-	printf("[RUN]\t%s from vm86 mode\n", text);
+	ksft_print_msg("%s from vm86 mode\n", text);
 	v86->regs.eip = eip;
 	ret = vm86(VM86_ENTER, v86);
 
 	if (ret == -1 && (errno == ENOSYS || errno == EPERM)) {
-		printf("[SKIP]\tvm86 %s\n",
-		       errno == ENOSYS ? "not supported" : "not allowed");
+		ksft_test_result_skip("vm86 %s\n",
+				      errno == ENOSYS ? "not supported" : "not allowed");
 		return false;
 	}
 
@@ -159,29 +157,27 @@ static bool do_test(struct vm86plus_struct *v86, unsigned long eip,
 		else
 			sprintf(trapname, "%d", trapno);
 
-		printf("[INFO]\tExited vm86 mode due to #%s\n", trapname);
+		ksft_print_msg("Exited vm86 mode due to #%s\n", trapname);
 	} else if (VM86_TYPE(ret) == VM86_UNKNOWN) {
-		printf("[INFO]\tExited vm86 mode due to unhandled GP fault\n");
+		ksft_print_msg("Exited vm86 mode due to unhandled GP fault\n");
 	} else if (VM86_TYPE(ret) == VM86_TRAP) {
-		printf("[INFO]\tExited vm86 mode due to a trap (arg=%ld)\n",
-		       VM86_ARG(ret));
+		ksft_print_msg("Exited vm86 mode due to a trap (arg=%ld)\n",
+			       VM86_ARG(ret));
 	} else if (VM86_TYPE(ret) == VM86_SIGNAL) {
-		printf("[INFO]\tExited vm86 mode due to a signal\n");
+		ksft_print_msg("Exited vm86 mode due to a signal\n");
 	} else if (VM86_TYPE(ret) == VM86_STI) {
-		printf("[INFO]\tExited vm86 mode due to STI\n");
+		ksft_print_msg("Exited vm86 mode due to STI\n");
 	} else {
-		printf("[INFO]\tExited vm86 mode due to type %ld, arg %ld\n",
-		       VM86_TYPE(ret), VM86_ARG(ret));
+		ksft_print_msg("Exited vm86 mode due to type %ld, arg %ld\n",
+			       VM86_TYPE(ret), VM86_ARG(ret));
 	}
 
 	if (rettype == -1 ||
-	    (VM86_TYPE(ret) == rettype && VM86_ARG(ret) == retarg)) {
-		printf("[OK]\tReturned correctly\n");
-	} else {
-		printf("[FAIL]\tIncorrect return reason (started at eip = 0x%lx, ended at eip = 0x%lx)\n", eip, v86->regs.eip);
-		nerrs++;
-	}
-
+	    (VM86_TYPE(ret) == rettype && VM86_ARG(ret) == retarg))
+		ksft_test_result_pass("Returned correctly\n");
+	else
+		ksft_test_result_fail("Incorrect return reason (started at eip = 0x%lx, ended at eip = 0x%lx)\n",
+				      eip, v86->regs.eip);
 	return true;
 }
 
@@ -215,26 +211,20 @@ void do_umip_tests(struct vm86plus_struct *vm86, unsigned char *test_mem)
 	/* Results when using register operands */
 	msw3 = *(unsigned short *)(test_mem + 2080);
 
-	printf("[INFO]\tResult from SMSW:[0x%04x]\n", msw1);
-	printf("[INFO]\tResult from SIDT: limit[0x%04x]base[0x%08lx]\n",
-	       idt1.limit, idt1.base);
-	printf("[INFO]\tResult from SGDT: limit[0x%04x]base[0x%08lx]\n",
-	       gdt1.limit, gdt1.base);
+	ksft_print_msg("Result from SMSW:[0x%04x]\n", msw1);
+	ksft_print_msg("Result from SIDT: limit[0x%04x]base[0x%08lx]\n",
+		       idt1.limit, idt1.base);
+	ksft_print_msg("Result from SGDT: limit[0x%04x]base[0x%08lx]\n",
+		       gdt1.limit, gdt1.base);
 
-	if (msw1 != msw2 || msw1 != msw3)
-		printf("[FAIL]\tAll the results of SMSW should be the same.\n");
-	else
-		printf("[PASS]\tAll the results from SMSW are identical.\n");
+	ksft_test_result((msw1 == msw2 && msw1 == msw3),
+			 "All the results from SMSW are identical.\n");
 
-	if (memcmp(&gdt1, &gdt2, sizeof(gdt1)))
-		printf("[FAIL]\tAll the results of SGDT should be the same.\n");
-	else
-		printf("[PASS]\tAll the results from SGDT are identical.\n");
+	ksft_test_result(!memcmp(&gdt1, &gdt2, sizeof(gdt1)),
+			 "All the results from SGDT are identical.\n");
 
-	if (memcmp(&idt1, &idt2, sizeof(idt1)))
-		printf("[FAIL]\tAll the results of SIDT should be the same.\n");
-	else
-		printf("[PASS]\tAll the results from SIDT are identical.\n");
+	ksft_test_result(!memcmp(&idt1, &idt2, sizeof(idt1)),
+			 "All the results from SIDT are identical.\n");
 
 	sethandler(SIGILL, sighandler, 0);
 	do_test(vm86, vmcode_umip_str - vmcode, VM86_SIGNAL, 0,
@@ -250,11 +240,15 @@ void do_umip_tests(struct vm86plus_struct *vm86, unsigned char *test_mem)
 int main(void)
 {
 	struct vm86plus_struct v86;
-	unsigned char *addr = mmap((void *)load_addr, 4096,
-				   PROT_READ | PROT_WRITE | PROT_EXEC,
-				   MAP_ANONYMOUS | MAP_PRIVATE, -1,0);
+	unsigned char *addr;
+
+	ksft_print_header();
+	ksft_set_plan(18);
+
+	addr = mmap((void *)load_addr, 4096, PROT_READ | PROT_WRITE | PROT_EXEC,
+		    MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
 	if (addr != (unsigned char *)load_addr)
-		err(1, "mmap");
+		ksft_exit_fail_perror("mmap");
 
 	memcpy(addr, vmcode, end_vmcode - vmcode);
 	addr[2048] = 2;
@@ -270,7 +264,8 @@ int main(void)
 	/* Use the end of the page as our stack. */
 	v86.regs.esp = 4096;
 
-	assert((v86.regs.cs & 3) == 0);	/* Looks like RPL = 0 */
+	if (v86.regs.cs & 3)
+		ksft_exit_fail_msg("Looks like RPL = 0\n");
 
 	/* #BR -- should deliver SIG??? */
 	do_test(&v86, vmcode_bound - vmcode, VM86_INTx, 5, "#BR");
@@ -333,16 +328,18 @@ int main(void)
 	v86.regs.ss = 0;
 	sethandler(SIGSEGV, sighandler, 0);
 	got_signal = 0;
-	if (do_test(&v86, 0, VM86_SIGNAL, 0, "Execute null pointer") &&
-	    !got_signal) {
-		printf("[FAIL]\tDid not receive SIGSEGV\n");
-		nerrs++;
-	}
+	if (do_test(&v86, 0, VM86_SIGNAL, 0, "Execute null pointer"))
+		ksft_test_result(got_signal, "Received SIGSEGV\n");
+	else
+		ksft_test_result_skip("Received SIGSEGV\n");
+
 	clearhandler(SIGSEGV);
 
 	/* Make sure nothing explodes if we fork. */
 	if (fork() == 0)
 		return 0;
 
-	return (nerrs == 0 ? 0 : 1);
+	ksft_test_result_pass("fork succeeded\n");
+
+	ksft_finished();
 }
-- 
2.39.2

Re: [PATCH v3 0/4] selftest: x86: improve tests

[Shuah Khan]

On 7/18/24 05:32, Muhammad Usama Anjum wrote:
> Changes since v1:
> - Correct the description of patches with what improvements they are
>    bringing and why they are required
> 
> Changes since v2:
> - Update descriptions of patches and add output before/after
> 
> Muhammad Usama Anjum (4):
>    selftests: x86: check_initial_reg_state: remove manual counting and
>      increase maintainability
>    selftests: x86: corrupt_xstate_header: remove manual counting and
>      increase maintainability
>    selftests: x86: fsgsbase_restore: remove manual counting and increase
>      maintainability
>    selftests: x86: entry_from_vm86: remove manual counting and increase
>      maintainability
> 
>   .../selftests/x86/check_initial_reg_state.c   |  24 ++--
>   .../selftests/x86/corrupt_xstate_header.c     |  30 +++--
>   tools/testing/selftests/x86/entry_from_vm86.c | 109 ++++++++--------
>   .../testing/selftests/x86/fsgsbase_restore.c  | 117 +++++++++---------
>   4 files changed, 139 insertions(+), 141 deletions(-)
> 

I am still not convinced what how these changes improve reporting
over running "make kselftest TARGETS=x86"

thanks,
-- Shuah

[PATCH v3 0/4] selftest: x86: remove manual counting and increase maintainability

[Muhammad Usama Anjum]

In this series, 4 tests are being conformed to TAP.

Changes since v1:
- Correct the description of patches with what improvements they are
  bringing and why they are required

Changes since v2:
- Correct the subject of series

Muhammad Usama Anjum (4):
  selftests: x86: check_initial_reg_state: remove manual counting and
    increase maintainability
  selftests: x86: corrupt_xstate_header: remove manual counting and
    increase maintainability
  selftests: x86: fsgsbase_restore: remove manual counting and increase
    maintainability
  selftests: x86: entry_from_vm86: remove manual counting and increase
    maintainability

 .../selftests/x86/check_initial_reg_state.c   |  24 ++--
 .../selftests/x86/corrupt_xstate_header.c     |  30 +++--
 tools/testing/selftests/x86/entry_from_vm86.c | 109 ++++++++--------
 .../testing/selftests/x86/fsgsbase_restore.c  | 117 +++++++++---------
 4 files changed, 139 insertions(+), 141 deletions(-)

-- 
2.39.2

[PATCH v3 1/4] selftests: x86: check_initial_reg_state: remove manual counting and increase maintainability

[Muhammad Usama Anjum]

- Removes manual counting of pass and fail tests
- Increase readability and maintainability of tests
- Print logs in standard format (without [RUN], [OK] tags)

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 .../selftests/x86/check_initial_reg_state.c   | 24 +++++++++----------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/x86/check_initial_reg_state.c b/tools/testing/selftests/x86/check_initial_reg_state.c
index 3bc95f3ed5859..b91c2b06b9881 100644
--- a/tools/testing/selftests/x86/check_initial_reg_state.c
+++ b/tools/testing/selftests/x86/check_initial_reg_state.c
@@ -7,6 +7,7 @@
 #define _GNU_SOURCE
 
 #include <stdio.h>
+#include "../kselftest.h"
 
 unsigned long ax, bx, cx, dx, si, di, bp, sp, flags;
 unsigned long r8, r9, r10, r11, r12, r13, r14, r15;
@@ -53,20 +54,19 @@ asm (
 
 int main()
 {
-	int nerrs = 0;
+	ksft_print_header();
+	ksft_set_plan(2);
 
-	if (sp == 0) {
-		printf("[FAIL]\tTest was built incorrectly\n");
-		return 1;
-	}
+	if (sp == 0)
+		ksft_exit_fail_msg("Test was built incorrectly\n");
 
 	if (ax || bx || cx || dx || si || di || bp
 #ifdef __x86_64__
 	    || r8 || r9 || r10 || r11 || r12 || r13 || r14 || r15
 #endif
 		) {
-		printf("[FAIL]\tAll GPRs except SP should be 0\n");
-#define SHOW(x) printf("\t" #x " = 0x%lx\n", x);
+		ksft_test_result_fail("All GPRs except SP should be 0\n");
+#define SHOW(x) ksft_print_msg("\t" #x " = 0x%lx\n", x)
 		SHOW(ax);
 		SHOW(bx);
 		SHOW(cx);
@@ -85,17 +85,15 @@ int main()
 		SHOW(r14);
 		SHOW(r15);
 #endif
-		nerrs++;
 	} else {
-		printf("[OK]\tAll GPRs except SP are 0\n");
+		ksft_test_result_pass("All GPRs except SP are 0\n");
 	}
 
 	if (flags != 0x202) {
-		printf("[FAIL]\tFLAGS is 0x%lx, but it should be 0x202\n", flags);
-		nerrs++;
+		ksft_test_result_fail("FLAGS is 0x%lx, but it should be 0x202\n", flags);
 	} else {
-		printf("[OK]\tFLAGS is 0x202\n");
+		ksft_test_result_pass("FLAGS is 0x202\n");
 	}
 
-	return nerrs ? 1 : 0;
+	ksft_finished();
 }
-- 
2.39.2