[PATCH 0/5] switchtest: test different migration types

[PATCH 0/5] switchtest: test different migration types

[T. Schaffner]

From: Tobias Schaffner <tobias.schaffner@siemens.com>

Switchtest only tested intended switches between primary and secondary mode in
userland.

Let the rtuo thread also test fault based migrations and migrations that occur
while in kernel mode.

This patch series will also test the bug that was fixed in
829d4d33cb7aa3ee485ad56da6ac0c3b0ce5dbe5

Tobias Schaffner (5):
  switchtest: fix rtuo not respectiong _ufps
  switchtest: split the set_mode method
  switchtest: test multiple secondary switch cases
  switchtest: Remove assertion after fault migration
  switchtest: test mode switche while in kernel mode

 include/rtdm/uapi/testing.h         |   4 +
 kernel/drivers/testing/switchtest.c |   4 +
 testsuite/switchtest/switchtest.c   | 110 ++++++++++++++++++++++------
 3 files changed, 96 insertions(+), 22 deletions(-)

-- 
2.34.1

[PATCH 1/5] switchtest: fix rtuo not respectiong _ufps

[T. Schaffner]

From: Tobias Schaffner <tobias.schaffner@siemens.com>

The mode variable in the rtuo thread switches between 1 and 2.

A wrong check on the mode variable leaded to never using the FPU in secondary
mode.

Evaluate the mode variable correctly.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 testsuite/switchtest/switchtest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
index f2a5c1601..e4339b0e2 100644
--- a/testsuite/switchtest/switchtest.c
+++ b/testsuite/switchtest/switchtest.c
@@ -668,7 +668,7 @@ static void *rtuo(void *cookie)
 		}
 
 		expected = rtsw.from + i * 1000;
-		if ((mode && param->fp & UFPP) || (!mode && param->fp & UFPS))
+		if ((mode == 2 && param->fp & UFPP) || (mode == 1 && param->fp & UFPS))
 			fp_regs_set(fp_features, expected);
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
 		while (err == -1 && errno == EINTR)
@@ -682,7 +682,7 @@ static void *rtuo(void *cookie)
 		case -1:
 			clean_exit(EXIT_FAILURE);
 		}
-		if ((mode && param->fp & UFPP) || (!mode && param->fp & UFPS)) {
+		if ((mode == 2 && param->fp & UFPP) || (mode == 1 && param->fp & UFPS)) {
 			fp_val = check_fp_result(expected);
 			if (fp_val != expected)
 				handle_bad_fpreg(param->cpu, fp_val);
-- 
2.34.1

[PATCH 2/5] switchtest: split the set_mode method

[T. Schaffner]

From: Tobias Schaffner <tobias.schaffner@siemens.com>

Split the set_mode method in separate methods for switching to primary and
secondary mode for improved readability.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 testsuite/switchtest/switchtest.c | 48 ++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
index e4339b0e2..3a6126a81 100644
--- a/testsuite/switchtest/switchtest.c
+++ b/testsuite/switchtest/switchtest.c
@@ -440,15 +440,21 @@ static void *fpu_stress(void *cookie)
 	return NULL;
 }
 
-static void set_mode(const char *prefix, int fd, unsigned mode)
+static void switch_to_primary_mode(void)
 {
-	switch (mode) {
-	case 1:
-		cobalt_thread_harden();
-		return;
+	cobalt_thread_harden();
+	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
+		perror("Switch to primary mode failed.");
+		clean_exit(EXIT_FAILURE);
+	}
+}
 
-	case 2:
-		cobalt_thread_relax();
+static void switch_to_secondary_mode(void)
+{
+	cobalt_thread_relax();
+	if (!(cobalt_thread_mode() & (XNRELAX))) {
+		perror("Switch to secondary mode failed.");
+		clean_exit(EXIT_FAILURE);
 	}
 }
 
@@ -475,7 +481,7 @@ static void *rtup(void *cookie)
 	   allowed when suspended in ioctl. */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
-	set_mode("rtup", fd, 1);
+	switch_to_primary_mode();
 
 	do {
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
@@ -556,7 +562,7 @@ static void *rtus(void *cookie)
 	   allowed when suspended in ioctl. */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
-	set_mode("rtus", fd, 2);
+	switch_to_secondary_mode();
 
 	do {
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
@@ -637,8 +643,9 @@ static void *rtuo(void *cookie)
 	   allowed when suspended in ioctl. */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
-	mode = 1;
-	set_mode("rtuo", fd, mode);
+	switch_to_primary_mode();
+	mode = COBALT_PRIMARY;
+
 	do {
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
 	} while (err == -1 && errno == EINTR);
@@ -668,8 +675,10 @@ static void *rtuo(void *cookie)
 		}
 
 		expected = rtsw.from + i * 1000;
-		if ((mode == 2 && param->fp & UFPP) || (mode == 1 && param->fp & UFPS))
+		if ((mode == COBALT_PRIMARY && param->fp & UFPP) ||
+		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
 			fp_regs_set(fp_features, expected);
+		}
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
 		while (err == -1 && errno == EINTR)
 			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
@@ -682,7 +691,9 @@ static void *rtuo(void *cookie)
 		case -1:
 			clean_exit(EXIT_FAILURE);
 		}
-		if ((mode == 2 && param->fp & UFPP) || (mode == 1 && param->fp & UFPS)) {
+
+		if ((mode == COBALT_PRIMARY && param->fp & UFPP) ||
+		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
 			fp_val = check_fp_result(expected);
 			if (fp_val != expected)
 				handle_bad_fpreg(param->cpu, fp_val);
@@ -690,11 +701,16 @@ static void *rtuo(void *cookie)
 
 		/* Switch mode. */
 		if (i % 3 == 2) {
-			mode = 3 - mode;
-			set_mode("rtuo", fd, mode);
+			if (mode == COBALT_PRIMARY) {
+				switch_to_secondary_mode();
+				mode = COBALT_SECONDARY;
+			} else {
+				switch_to_primary_mode();
+				mode = COBALT_PRIMARY;
+			}
 		}
 
-		if(++i == 4000000)
+		if (++i == 4000000)
 			i = 0;
 	}
 
-- 
2.34.1

[PATCH 3/5] switchtest: test multiple secondary switch cases

[T. Schaffner]

From: Tobias Schaffner <tobias.schaffner@siemens.com>

Switches to secondary mode do not only occure when a relax is explicitly called
via the cobalt api but also e.g. when a linux syscall is called in primary mode.

Also test fault related cases in the oscillating switchtest thread.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 testsuite/switchtest/switchtest.c | 84 +++++++++++++++++++++----------
 1 file changed, 58 insertions(+), 26 deletions(-)

diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
index 3a6126a81..f3737d193 100644
--- a/testsuite/switchtest/switchtest.c
+++ b/testsuite/switchtest/switchtest.c
@@ -34,7 +34,9 @@
 #include <semaphore.h>
 #include <setjmp.h>
 #include <getopt.h>
+#include <asm/unistd.h>
 #include <asm/xenomai/features.h>
+#include <asm/xenomai/syscall.h>
 #include <asm/xenomai/uapi/fptest.h>
 #include <cobalt/trace.h>
 #include <rtdm/testing.h>
@@ -297,6 +299,59 @@ static int printout(const char *fmt, ...)
 #define check_fp_result(__expected)	\
 	fp_regs_check(fp_features, __expected, printout)
 
+static void _assert_primary_mode(char const * calling_func) {
+	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
+		fprintf(stderr, "Switch to primary mode failed in %s.", calling_func);
+		clean_exit(EXIT_FAILURE);
+	}
+}
+
+#define assert_primary_mode() _assert_primary_mode(__func__)
+
+static void _assert_secondary_mode(char const * calling_func) {
+	if (!(cobalt_thread_mode() & (XNRELAX))) {
+		fprintf(stderr, "Switch to secondary mode failed in %s.", calling_func);
+		clean_exit(EXIT_FAILURE);
+	}
+}
+
+#define assert_secondary_mode() _assert_secondary_mode(__func__)
+
+static void switch_to_primary_mode(void)
+{
+	cobalt_thread_harden();
+	assert_primary_mode();
+}
+
+static void switch_to_secondary_mode(void)
+{
+	cobalt_thread_relax();
+	assert_secondary_mode();
+}
+
+static void switch_to_secondary_mode_by_using_linux_syscall(void)
+{
+	syscall(__NR_gettid);
+	assert_secondary_mode();
+}
+
+static void switch_to_secondary_mode_by_fault_in_xenomai_syscall(void)
+{
+	XENOMAI_SYSCALL2(sc_cobalt_sem_timedwait64, NULL, NULL);
+	assert_secondary_mode();
+}
+
+#define SWITCH_FUNC_COUNT 6
+
+static void (*switch_funcs[SWITCH_FUNC_COUNT]) (void) = {
+	switch_to_secondary_mode,
+	switch_to_primary_mode,
+	switch_to_secondary_mode_by_using_linux_syscall,
+	switch_to_primary_mode,
+	switch_to_secondary_mode_by_fault_in_xenomai_syscall,
+	switch_to_primary_mode,
+};
+
 static void *sleeper_switcher(void *cookie)
 {
 	struct task_params *param = (struct task_params *) cookie;
@@ -440,24 +495,6 @@ static void *fpu_stress(void *cookie)
 	return NULL;
 }
 
-static void switch_to_primary_mode(void)
-{
-	cobalt_thread_harden();
-	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
-		perror("Switch to primary mode failed.");
-		clean_exit(EXIT_FAILURE);
-	}
-}
-
-static void switch_to_secondary_mode(void)
-{
-	cobalt_thread_relax();
-	if (!(cobalt_thread_mode() & (XNRELAX))) {
-		perror("Switch to secondary mode failed.");
-		clean_exit(EXIT_FAILURE);
-	}
-}
-
 static void *rtup(void *cookie)
 {
 	struct task_params *param = (struct task_params *) cookie;
@@ -699,15 +736,10 @@ static void *rtuo(void *cookie)
 				handle_bad_fpreg(param->cpu, fp_val);
 		}
 
-		/* Switch mode. */
+		/* Switch between primary and secondary mode */
 		if (i % 3 == 2) {
-			if (mode == COBALT_PRIMARY) {
-				switch_to_secondary_mode();
-				mode = COBALT_SECONDARY;
-			} else {
-				switch_to_primary_mode();
-				mode = COBALT_PRIMARY;
-			}
+			switch_funcs[i / 3 % SWITCH_FUNC_COUNT]();
+			mode = !mode;
 		}
 
 		if (++i == 4000000)
-- 
2.34.1

[PATCH 4/5] switchtest: Remove assertion after fault migration

[T. Schaffner]

From: Tobias Schaffner <tobias.schaffner@siemens.com>

Faults in a xenomai syscall lead to a migration to secondary mode most but not
all of the time. Do not assert that the migration worked but expect it to work
on next iteration of the test.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 testsuite/switchtest/switchtest.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
index f3737d193..970790b2e 100644
--- a/testsuite/switchtest/switchtest.c
+++ b/testsuite/switchtest/switchtest.c
@@ -309,7 +309,7 @@ static void _assert_primary_mode(char const * calling_func) {
 #define assert_primary_mode() _assert_primary_mode(__func__)
 
 static void _assert_secondary_mode(char const * calling_func) {
-	if (!(cobalt_thread_mode() & (XNRELAX))) {
+	if (! (cobalt_thread_mode() & (XNRELAX))) {
 		fprintf(stderr, "Switch to secondary mode failed in %s.", calling_func);
 		clean_exit(EXIT_FAILURE);
 	}
@@ -338,7 +338,12 @@ static void switch_to_secondary_mode_by_using_linux_syscall(void)
 static void switch_to_secondary_mode_by_fault_in_xenomai_syscall(void)
 {
 	XENOMAI_SYSCALL2(sc_cobalt_sem_timedwait64, NULL, NULL);
-	assert_secondary_mode();
+
+	/* A fault in a xenomai syscall will lead to a migration most but not all
+	   of the time. 
+
+	   assert_secondary_mode();
+	*/
 }
 
 #define SWITCH_FUNC_COUNT 6
-- 
2.34.1

[PATCH 5/5] switchtest: test mode switche while in kernel mode

[T. Schaffner]

From: Tobias Schaffner <tobias.schaffner@siemens.com>

Make the rtuo thread switch between primary and secondary mode while in the
RTTST_RTIOC_SWTEST_SWITCH_TO syscall.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 include/rtdm/uapi/testing.h         |  4 ++++
 kernel/drivers/testing/switchtest.c |  4 ++++
 testsuite/switchtest/switchtest.c   | 15 ++++++++++++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/include/rtdm/uapi/testing.h b/include/rtdm/uapi/testing.h
index f8207b8c7..059a1e0b6 100644
--- a/include/rtdm/uapi/testing.h
+++ b/include/rtdm/uapi/testing.h
@@ -24,6 +24,7 @@
 #define _RTDM_UAPI_TESTING_H
 
 #include <linux/types.h>
+#include <stdbool.h>
 
 #define RTTST_PROFILE_VER		2
 
@@ -74,6 +75,9 @@ struct rttst_swtest_task {
 struct rttst_swtest_dir {
 	unsigned int from;
 	unsigned int to;
+
+        /* Switch mode of from thread on RTTST_RTIOC_SWTEST_SWITCH_TO call. */
+	bool switch_mode;
 };
 
 struct rttst_swtest_error {
diff --git a/kernel/drivers/testing/switchtest.c b/kernel/drivers/testing/switchtest.c
index 312b4d870..05938a261 100644
--- a/kernel/drivers/testing/switchtest.c
+++ b/kernel/drivers/testing/switchtest.c
@@ -647,6 +647,8 @@ static int rtswitch_ioctl_nrt(struct rtdm_fd *fd,
 				    arg,
 				    sizeof(fromto));
 
+		if (fromto.switch_mode)
+		    xnthread_harden();
 		return rtswitch_to_nrt(ctx, fromto.from, fromto.to);
 
 	case RTTST_RTIOC_SWTEST_GET_SWITCHES_COUNT:
@@ -701,6 +703,8 @@ static int rtswitch_ioctl_rt(struct rtdm_fd *fd,
 				    arg,
 				    sizeof(fromto));
 
+		if (fromto.switch_mode)
+		    xnthread_relax(0, 0);
 		return rtswitch_to_rt(ctx, fromto.from, fromto.to);
 
 	case RTTST_RTIOC_SWTEST_GET_LAST_ERROR:
diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
index 970790b2e..e7441f9d4 100644
--- a/testsuite/switchtest/switchtest.c
+++ b/testsuite/switchtest/switchtest.c
@@ -376,6 +376,7 @@ static void *sleeper_switcher(void *cookie)
 		clean_exit(EXIT_FAILURE);
 	}
 
+	rtsw.switch_mode = false;
 	rtsw.from = param->swt.index;
 	to = param->swt.index;
 
@@ -516,6 +517,7 @@ static void *rtup(void *cookie)
 		clean_exit(EXIT_FAILURE);
 	}
 
+	rtsw.switch_mode = false;
 	rtsw.from = param->swt.index;
 	to = param->swt.index;
 
@@ -597,6 +599,7 @@ static void *rtus(void *cookie)
 		clean_exit(EXIT_FAILURE);
 	}
 
+	rtsw.switch_mode = false;
 	rtsw.from = param->swt.index;
 	to = param->swt.index;
 
@@ -678,6 +681,7 @@ static void *rtuo(void *cookie)
 		clean_exit(EXIT_FAILURE);
 	}
 
+	rtsw.switch_mode = false;
 	rtsw.from = param->swt.index;
 	to = param->swt.index;
 
@@ -721,10 +725,14 @@ static void *rtuo(void *cookie)
 		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
 			fp_regs_set(fp_features, expected);
 		}
+
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
 		while (err == -1 && errno == EINTR)
 			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
 
+		// Return to default: do not switch in syscall
+		rtsw.switch_mode = false;
+
 		switch (err) {
 		case 0:
 			break;
@@ -743,7 +751,12 @@ static void *rtuo(void *cookie)
 
 		/* Switch between primary and secondary mode */
 		if (i % 3 == 2) {
-			switch_funcs[i / 3 % SWITCH_FUNC_COUNT]();
+			if (i / 3 % (SWITCH_FUNC_COUNT + 2 ) < SWITCH_FUNC_COUNT) {
+				switch_funcs[i / 3 % (SWITCH_FUNC_COUNT + 2 )]();
+			} else {
+				// Switch mode on next RTTST_RTIOC_SWTEST_SWITCH_TO syscall
+				rtsw.switch_mode = true;
+			}
 			mode = !mode;
 		}
 
-- 
2.34.1

Re: [PATCH 1/5] switchtest: fix rtuo not respectiong _ufps

[Florian Bezdeka]

Hi Tobias,

nice to have one more active contributor. Welcome!

On Wed, 2022-11-30 at 15:11 +0100, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> The mode variable in the rtuo thread switches between 1 and 2.
> 
> A wrong check on the mode variable leaded to never using the FPU in secondary
> mode.
> 
> Evaluate the mode variable correctly.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  testsuite/switchtest/switchtest.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
> index f2a5c1601..e4339b0e2 100644
> --- a/testsuite/switchtest/switchtest.c
> +++ b/testsuite/switchtest/switchtest.c
> @@ -668,7 +668,7 @@ static void *rtuo(void *cookie)
>  		}
>  
>  		expected = rtsw.from + i * 1000;
> -		if ((mode && param->fp & UFPP) || (!mode && param->fp & UFPS))
> +		if ((mode == 2 && param->fp & UFPP) || (mode == 1 && param->fp & UFPS))

1 and 2 still look like magic values to me. Don't we have some #defines
for that? Looks like we have when scanning your following patches.
Can't we use them here already?

>  			fp_regs_set(fp_features, expected);
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
>  		while (err == -1 && errno == EINTR)
> @@ -682,7 +682,7 @@ static void *rtuo(void *cookie)
>  		case -1:
>  			clean_exit(EXIT_FAILURE);
>  		}
> -		if ((mode && param->fp & UFPP) || (!mode && param->fp & UFPS)) {
> +		if ((mode == 2 && param->fp & UFPP) || (mode == 1 && param->fp & UFPS)) {
>  			fp_val = check_fp_result(expected);
>  			if (fp_val != expected)
>  				handle_bad_fpreg(param->cpu, fp_val);

Re: [PATCH 2/5] switchtest: split the set_mode method

[Florian Bezdeka]

On Wed, 2022-11-30 at 15:11 +0100, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Split the set_mode method in separate methods for switching to primary and
> secondary mode for improved readability.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  testsuite/switchtest/switchtest.c | 48 ++++++++++++++++++++-----------
>  1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
> index e4339b0e2..3a6126a81 100644
> --- a/testsuite/switchtest/switchtest.c
> +++ b/testsuite/switchtest/switchtest.c
> @@ -440,15 +440,21 @@ static void *fpu_stress(void *cookie)
>  	return NULL;
>  }
>  
> -static void set_mode(const char *prefix, int fd, unsigned mode)
> +static void switch_to_primary_mode(void)
>  {
> -	switch (mode) {
> -	case 1:
> -		cobalt_thread_harden();
> -		return;
> +	cobalt_thread_harden();
> +	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
> +		perror("Switch to primary mode failed.");
> +		clean_exit(EXIT_FAILURE);
> +	}
> +}
>  
> -	case 2:
> -		cobalt_thread_relax();
> +static void switch_to_secondary_mode(void)
> +{
> +	cobalt_thread_relax();
> +	if (!(cobalt_thread_mode() & (XNRELAX))) {
> +		perror("Switch to secondary mode failed.");
> +		clean_exit(EXIT_FAILURE);
>  	}
>  }
>  
> @@ -475,7 +481,7 @@ static void *rtup(void *cookie)
>  	   allowed when suspended in ioctl. */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
>  
> -	set_mode("rtup", fd, 1);
> +	switch_to_primary_mode();
>  
>  	do {
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
> @@ -556,7 +562,7 @@ static void *rtus(void *cookie)
>  	   allowed when suspended in ioctl. */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
>  
> -	set_mode("rtus", fd, 2);
> +	switch_to_secondary_mode();
>  
>  	do {
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
> @@ -637,8 +643,9 @@ static void *rtuo(void *cookie)
>  	   allowed when suspended in ioctl. */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
>  
> -	mode = 1;
> -	set_mode("rtuo", fd, mode);
> +	switch_to_primary_mode();
> +	mode = COBALT_PRIMARY;
> +
>  	do {
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
>  	} while (err == -1 && errno == EINTR);
> @@ -668,8 +675,10 @@ static void *rtuo(void *cookie)
>  		}
>  
>  		expected = rtsw.from + i * 1000;
> -		if ((mode == 2 && param->fp & UFPP) || (mode == 1 && param->fp & UFPS))
> +		if ((mode == COBALT_PRIMARY && param->fp & UFPP) ||
> +		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
>  			fp_regs_set(fp_features, expected);
> +		}

I guess that answers my previous questions.

You should avoid touching the same code multiple times within one
series.

>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
>  		while (err == -1 && errno == EINTR)
>  			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
> @@ -682,7 +691,9 @@ static void *rtuo(void *cookie)
>  		case -1:
>  			clean_exit(EXIT_FAILURE);
>  		}
> -		if ((mode == 2 && param->fp & UFPP) || (mode == 1 && param->fp & UFPS)) {
> +
> +		if ((mode == COBALT_PRIMARY && param->fp & UFPP) ||
> +		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
>  			fp_val = check_fp_result(expected);
>  			if (fp_val != expected)
>  				handle_bad_fpreg(param->cpu, fp_val);
> @@ -690,11 +701,16 @@ static void *rtuo(void *cookie)
>  
>  		/* Switch mode. */
>  		if (i % 3 == 2) {
> -			mode = 3 - mode;
> -			set_mode("rtuo", fd, mode);
> +			if (mode == COBALT_PRIMARY) {
> +				switch_to_secondary_mode();
> +				mode = COBALT_SECONDARY;
> +			} else {
> +				switch_to_primary_mode();
> +				mode = COBALT_PRIMARY;
> +			}
>  		}
>  
> -		if(++i == 4000000)
> +		if (++i == 4000000)
>  			i = 0;
>  	}
>  

Re: [PATCH 3/5] switchtest: test multiple secondary switch cases

[Florian Bezdeka]

On Wed, 2022-11-30 at 15:11 +0100, T. Schaffner wrote:
> +static void switch_to_secondary_mode_by_fault_in_xenomai_syscall(void)
> +{
> +	XENOMAI_SYSCALL2(sc_cobalt_sem_timedwait64, NULL, NULL);
> +	assert_secondary_mode();
> +}

Let me revisit this one here. I guess we should not fault at all. Seems
you re-detected a bug. I had some problems with that while working on
the first round y2038 stuff and decided to keep it as it always was.

Will have to check where the fault actually happens. Maybe it's
acceptable to trigger the fault with NULL. (At least it seems to be
handled correctly)

Open question: If we would not trigger a fault, how to handle that one
of your test requirements goes away...

Re: [PATCH 4/5] switchtest: Remove assertion after fault migration

[Florian Bezdeka]

On Wed, 2022-11-30 at 15:11 +0100, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Faults in a xenomai syscall lead to a migration to secondary mode most but not
> all of the time. Do not assert that the migration worked but expect it to work
> on next iteration of the test.

Seems this patch should be folded / squashed into the patch that
introduced this code.

> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  testsuite/switchtest/switchtest.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
> index f3737d193..970790b2e 100644
> --- a/testsuite/switchtest/switchtest.c
> +++ b/testsuite/switchtest/switchtest.c
> @@ -309,7 +309,7 @@ static void _assert_primary_mode(char const * calling_func) {
>  #define assert_primary_mode() _assert_primary_mode(__func__)
>  
>  static void _assert_secondary_mode(char const * calling_func) {
> -	if (!(cobalt_thread_mode() & (XNRELAX))) {
> +	if (! (cobalt_thread_mode() & (XNRELAX))) {
>  		fprintf(stderr, "Switch to secondary mode failed in %s.", calling_func);
>  		clean_exit(EXIT_FAILURE);
>  	}

We normally tend to ignore cosmetic changes.

> @@ -338,7 +338,12 @@ static void switch_to_secondary_mode_by_using_linux_syscall(void)
>  static void switch_to_secondary_mode_by_fault_in_xenomai_syscall(void)
>  {
>  	XENOMAI_SYSCALL2(sc_cobalt_sem_timedwait64, NULL, NULL);
> -	assert_secondary_mode();
> +
> +	/* A fault in a xenomai syscall will lead to a migration most but not all
> +	   of the time. 

I would be very interested in the reason why this doesn't happen all
the time. Please enlighten me ;-)

> +
> +	   assert_secondary_mode();
> +	*/
>  }
>  
>  #define SWITCH_FUNC_COUNT 6

Re: [PATCH 5/5] switchtest: test mode switche while in kernel mode

[Florian Bezdeka]

On 30.11.22 15:11, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Make the rtuo thread switch between primary and secondary mode while in the
> RTTST_RTIOC_SWTEST_SWITCH_TO syscall.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  include/rtdm/uapi/testing.h         |  4 ++++
>  kernel/drivers/testing/switchtest.c |  4 ++++
>  testsuite/switchtest/switchtest.c   | 15 ++++++++++++++-
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/include/rtdm/uapi/testing.h b/include/rtdm/uapi/testing.h
> index f8207b8c7..059a1e0b6 100644
> --- a/include/rtdm/uapi/testing.h
> +++ b/include/rtdm/uapi/testing.h
> @@ -24,6 +24,7 @@
>  #define _RTDM_UAPI_TESTING_H
>  
>  #include <linux/types.h>
> +#include <stdbool.h>
>  
>  #define RTTST_PROFILE_VER		2
>  
> @@ -74,6 +75,9 @@ struct rttst_swtest_task {
>  struct rttst_swtest_dir {
>  	unsigned int from;
>  	unsigned int to;
> +
> +        /* Switch mode of from thread on RTTST_RTIOC_SWTEST_SWITCH_TO call. */
> +	bool switch_mode;

I'm not sure here. Maybe we should use something like doxygen "doc
format", so /** instead of /* here or even move the documentation
infront of struct rttst_swtest_task.

While at it: Maybe it's time to document the other members as well?

>  };
>  
>  struct rttst_swtest_error {
> diff --git a/kernel/drivers/testing/switchtest.c b/kernel/drivers/testing/switchtest.c
> index 312b4d870..05938a261 100644
> --- a/kernel/drivers/testing/switchtest.c
> +++ b/kernel/drivers/testing/switchtest.c
> @@ -647,6 +647,8 @@ static int rtswitch_ioctl_nrt(struct rtdm_fd *fd,
>  				    arg,
>  				    sizeof(fromto));
>  
> +		if (fromto.switch_mode)
> +		    xnthread_harden();

Code style: whitespaces vs tabs / tabs mixed with ws.

Should normally be detected by checkpatch (part of the kernel sources).

>  		return rtswitch_to_nrt(ctx, fromto.from, fromto.to);
>  
>  	case RTTST_RTIOC_SWTEST_GET_SWITCHES_COUNT:
> @@ -701,6 +703,8 @@ static int rtswitch_ioctl_rt(struct rtdm_fd *fd,
>  				    arg,
>  				    sizeof(fromto));
>  
> +		if (fromto.switch_mode)
> +		    xnthread_relax(0, 0);
>  		return rtswitch_to_rt(ctx, fromto.from, fromto.to);
>  
>  	case RTTST_RTIOC_SWTEST_GET_LAST_ERROR:
> diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
> index 970790b2e..e7441f9d4 100644
> --- a/testsuite/switchtest/switchtest.c
> +++ b/testsuite/switchtest/switchtest.c
> @@ -376,6 +376,7 @@ static void *sleeper_switcher(void *cookie)
>  		clean_exit(EXIT_FAILURE);
>  	}
>  
> +	rtsw.switch_mode = false;
>  	rtsw.from = param->swt.index;
>  	to = param->swt.index;
>  
> @@ -516,6 +517,7 @@ static void *rtup(void *cookie)
>  		clean_exit(EXIT_FAILURE);
>  	}
>  
> +	rtsw.switch_mode = false;
>  	rtsw.from = param->swt.index;
>  	to = param->swt.index;
>  
> @@ -597,6 +599,7 @@ static void *rtus(void *cookie)
>  		clean_exit(EXIT_FAILURE);
>  	}
>  
> +	rtsw.switch_mode = false;
>  	rtsw.from = param->swt.index;
>  	to = param->swt.index;
>  
> @@ -678,6 +681,7 @@ static void *rtuo(void *cookie)
>  		clean_exit(EXIT_FAILURE);
>  	}
>  
> +	rtsw.switch_mode = false;
>  	rtsw.from = param->swt.index;
>  	to = param->swt.index;
>  
> @@ -721,10 +725,14 @@ static void *rtuo(void *cookie)
>  		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
>  			fp_regs_set(fp_features, expected);
>  		}
> +
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
>  		while (err == -1 && errno == EINTR)
>  			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
>  
> +		// Return to default: do not switch in syscall
> +		rtsw.switch_mode = false;
> +
>  		switch (err) {
>  		case 0:
>  			break;
> @@ -743,7 +751,12 @@ static void *rtuo(void *cookie)
>  
>  		/* Switch between primary and secondary mode */
>  		if (i % 3 == 2) {
> -			switch_funcs[i / 3 % SWITCH_FUNC_COUNT]();
> +			if (i / 3 % (SWITCH_FUNC_COUNT + 2 ) < SWITCH_FUNC_COUNT) {
> +				switch_funcs[i / 3 % (SWITCH_FUNC_COUNT + 2 )]();

Magic numbers: 2 and 3

> +			} else {
> +				// Switch mode on next RTTST_RTIOC_SWTEST_SWITCH_TO syscall
> +				rtsw.switch_mode = true;
> +			}
>  			mode = !mode;
>  		}
>  

Re: [PATCH 3/5] switchtest: test multiple secondary switch cases

[Florian Bezdeka]

On 30.11.22 15:11, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Switches to secondary mode do not only occure when a relax is explicitly called
> via the cobalt api but also e.g. when a linux syscall is called in primary mode.
> 
> Also test fault related cases in the oscillating switchtest thread.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  testsuite/switchtest/switchtest.c | 84 +++++++++++++++++++++----------
>  1 file changed, 58 insertions(+), 26 deletions(-)
> 
> diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
> index 3a6126a81..f3737d193 100644
> --- a/testsuite/switchtest/switchtest.c
> +++ b/testsuite/switchtest/switchtest.c
> @@ -34,7 +34,9 @@
>  #include <semaphore.h>
>  #include <setjmp.h>
>  #include <getopt.h>
> +#include <asm/unistd.h>
>  #include <asm/xenomai/features.h>
> +#include <asm/xenomai/syscall.h>
>  #include <asm/xenomai/uapi/fptest.h>
>  #include <cobalt/trace.h>
>  #include <rtdm/testing.h>
> @@ -297,6 +299,59 @@ static int printout(const char *fmt, ...)
>  #define check_fp_result(__expected)	\
>  	fp_regs_check(fp_features, __expected, printout)
>  
> +static void _assert_primary_mode(char const * calling_func) {
> +	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
> +		fprintf(stderr, "Switch to primary mode failed in %s.", calling_func);
> +		clean_exit(EXIT_FAILURE);
> +	}
> +}

The same applies here and to some more functions below:

We could reduce the code indention level when we inverse the condition like:

{
	if (nothing_to_do)
		return; // all right, bail out

	do_error_reporting;
]

This is normally easier to parse/read.

> +
> +#define assert_primary_mode() _assert_primary_mode(__func__)
> +
> +static void _assert_secondary_mode(char const * calling_func) {
> +	if (!(cobalt_thread_mode() & (XNRELAX))) {
> +		fprintf(stderr, "Switch to secondary mode failed in %s.", calling_func);
> +		clean_exit(EXIT_FAILURE);
> +	}
> +> +
> +#define assert_secondary_mode() _assert_secondary_mode(__func__)
> +
> +static void switch_to_primary_mode(void)
> +{
> +	cobalt_thread_harden();
> +	assert_primary_mode();
> +}
> +
> +static void switch_to_secondary_mode(void)
> +{
> +	cobalt_thread_relax();
> +	assert_secondary_mode();
> +}
> +
> +static void switch_to_secondary_mode_by_using_linux_syscall(void)
> +{
> +	syscall(__NR_gettid);
> +	assert_secondary_mode();
> +}
> +
> +static void switch_to_secondary_mode_by_fault_in_xenomai_syscall(void)
> +{
> +	XENOMAI_SYSCALL2(sc_cobalt_sem_timedwait64, NULL, NULL);
> +	assert_secondary_mode();
> +}
> +
> +#define SWITCH_FUNC_COUNT 6
> +
> +static void (*switch_funcs[SWITCH_FUNC_COUNT]) (void) = {
> +	switch_to_secondary_mode,
> +	switch_to_primary_mode,
> +	switch_to_secondary_mode_by_using_linux_syscall,
> +	switch_to_primary_mode,
> +	switch_to_secondary_mode_by_fault_in_xenomai_syscall,
> +	switch_to_primary_mode,
> +};
> +
>  static void *sleeper_switcher(void *cookie)
>  {
>  	struct task_params *param = (struct task_params *) cookie;
> @@ -440,24 +495,6 @@ static void *fpu_stress(void *cookie)
>  	return NULL;
>  }
>  
> -static void switch_to_primary_mode(void)
> -{
> -	cobalt_thread_harden();
> -	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
> -		perror("Switch to primary mode failed.");
> -		clean_exit(EXIT_FAILURE);
> -	}
> -}
> -
> -static void switch_to_secondary_mode(void)
> -{
> -	cobalt_thread_relax();
> -	if (!(cobalt_thread_mode() & (XNRELAX))) {
> -		perror("Switch to secondary mode failed.");
> -		clean_exit(EXIT_FAILURE);
> -	}
> -}
> -
>  static void *rtup(void *cookie)
>  {
>  	struct task_params *param = (struct task_params *) cookie;
> @@ -699,15 +736,10 @@ static void *rtuo(void *cookie)
>  				handle_bad_fpreg(param->cpu, fp_val);
>  		}
>  
> -		/* Switch mode. */
> +		/* Switch between primary and secondary mode */
>  		if (i % 3 == 2) {
> -			if (mode == COBALT_PRIMARY) {
> -				switch_to_secondary_mode();
> -				mode = COBALT_SECONDARY;
> -			} else {
> -				switch_to_primary_mode();
> -				mode = COBALT_PRIMARY;
> -			}
> +			switch_funcs[i / 3 % SWITCH_FUNC_COUNT]();
> +			mode = !mode;
>  		}
>  
>  		if (++i == 4000000)

Re: [PATCH 3/5] switchtest: test multiple secondary switch cases

[Florian Bezdeka]

On Wed, 2022-11-30 at 15:37 +0100, Florian Bezdeka wrote:
> On Wed, 2022-11-30 at 15:11 +0100, T. Schaffner wrote:
> > +static void switch_to_secondary_mode_by_fault_in_xenomai_syscall(void)
> > +{
> > +	XENOMAI_SYSCALL2(sc_cobalt_sem_timedwait64, NULL, NULL);
> > +	assert_secondary_mode();
> > +}
> 
> Let me revisit this one here. I guess we should not fault at all. Seems
> you re-detected a bug. I had some problems with that while working on
> the first round y2038 stuff and decided to keep it as it always was.
> 
> Will have to check where the fault actually happens. Maybe it's
> acceptable to trigger the fault with NULL. (At least it seems to be
> handled correctly)
> 
> Open question: If we would not trigger a fault, how to handle that one
> of your test requirements goes away...
> 

Done today. You should not expect the sc_cobalt_sem_timedwait64 to
fault when the first parameter is NULL. That's more a "bug". It's still
correctly handled but a primary => secondary transition by involving
the page fault handler is a overkill here (IMHO).

Can't we extend the switchtest uapi like you did with the new
switch_mode flag that you introduced? Something like a new
trigger_fault flag should be doable, no?

I guess that would do it:
	int tmp;
	if (trigger_fault)
		cobalt_copy_from_user(&tmp, 0xdeadbeefUL,
sizeof(tmp));

That would keep the switchtest utility decoupled from stuff like y2038
syscalls. I would highly recommend decoupling.

Best regards,
Florian