[PATCH 1/2] selftests/powerpc: Add missing clobbered register to to ptrace TM tests

[PATCH 1/2] selftests/powerpc: Add missing clobbered register to to ptrace TM tests

[Jordan Niethe]

ISA v3.1 removes TM but includes a synthetic implementation for
backwards compatibility.  With this implementation,  the tests
ptrace-tm-spd-gpr and ptrace-tm-gpr should never be able to make any
forward progress and eventually should be killed by the timeout.
Instead on a P10 running in P9 mode, ptrace_tm_gpr fails like so:

test: ptrace_tm_gpr
tags: git_version:unknown
Starting the child
...
...
GPR[27]: 1 Expected: 2
GPR[28]: 1 Expected: 2
GPR[29]: 1 Expected: 2
GPR[30]: 1 Expected: 2
GPR[31]: 1 Expected: 2
[FAIL] Test FAILED on line 98
failure: ptrace_tm_gpr
selftests:  ptrace-tm-gpr [FAIL]

The problem is in the inline assembly of the child. r0 is loaded with a
value in the child's transaction abort handler but this register is not
included in the clobbers list.  This means it is possible that this
statement:
	cptr[1] = 0;
which is meant to signal the parent to wait may actually use the value
placed into r0 by the inline assembly incorrectly signal the parent to
continue.

By inspection the same problem is present in ptrace-tm-spd-gpr.

Adding r0 to the clobbbers list makes the test fail correctly via a
timeout on a P10 running in P8/P9 compatibility mode.

Suggested-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c     | 2 +-
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
index 82f7bdc2e5e6..7df7100a29be 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
@@ -57,7 +57,7 @@ void tm_gpr(void)
 		: [gpr_1]"i"(GPR_1), [gpr_2]"i"(GPR_2),
 		[sprn_texasr] "i" (SPRN_TEXASR), [flt_1] "b" (&a),
 		[flt_2] "b" (&b), [cptr1] "b" (&cptr[1])
-		: "memory", "r7", "r8", "r9", "r10",
+		: "memory", "r0", "r7", "r8", "r9", "r10",
 		"r11", "r12", "r13", "r14", "r15", "r16",
 		"r17", "r18", "r19", "r20", "r21", "r22",
 		"r23", "r24", "r25", "r26", "r27", "r28",
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c
index ad65be6e8e85..8706bea5d015 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c
@@ -65,7 +65,7 @@ void tm_spd_gpr(void)
 		: [gpr_1]"i"(GPR_1), [gpr_2]"i"(GPR_2), [gpr_4]"i"(GPR_4),
 		[sprn_texasr] "i" (SPRN_TEXASR), [flt_1] "b" (&a),
 		[flt_4] "b" (&d)
-		: "memory", "r5", "r6", "r7",
+		: "memory", "r0", "r5", "r6", "r7",
 		"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
 		"r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
 		"r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31"
-- 
2.25.1

[PATCH 2/2] selftests: Skip TM tests on synthetic TM implementations

[Jordan Niethe]

Transactional Memory was removed from the architecture in ISA v3.1. For
threads running in P8/P9 compatibility mode on P10 a synthetic TM
implementation is provided. In this implementation, tbegin. always sets
cr0 eq meaning the abort handler is always called. This is not an issue
as users of TM are expected to have a fallback non transactional way to
make forward progress in the abort handler.

As the TM self tests exist only to test TM, no alternative path forward
is provided, leading to them timing out and failing on the synthetic TM
implementation.

The TEXASR indicates if a transaction failure is due to a synthetic
implementation. Check for a synthetic implementation and skip the TM
tests if so.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 .../selftests/powerpc/ptrace/ptrace-tm-gpr.c  |  1 +
 .../powerpc/ptrace/ptrace-tm-spd-gpr.c        |  1 +
 .../powerpc/ptrace/ptrace-tm-spd-tar.c        |  1 +
 .../selftests/powerpc/ptrace/ptrace-tm-tar.c  |  1 +
 .../selftests/powerpc/tm/tm-resched-dscr.c    |  1 +
 .../selftests/powerpc/tm/tm-signal-stack.c    |  1 +
 .../testing/selftests/powerpc/tm/tm-syscall.c |  2 +-
 tools/testing/selftests/powerpc/tm/tm.h       | 36 +++++++++++++++++++
 8 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
index 7df7100a29be..67ca297c5cca 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
@@ -113,6 +113,7 @@ int ptrace_tm_gpr(void)
 	int ret, status;
 
 	SKIP_IF(!have_htm());
+	SKIP_IF(htm_is_synthetic());
 	shm_id = shmget(IPC_PRIVATE, sizeof(int) * 2, 0777|IPC_CREAT);
 	pid = fork();
 	if (pid < 0) {
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c
index 8706bea5d015..6f2bce1b6c5d 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c
@@ -119,6 +119,7 @@ int ptrace_tm_spd_gpr(void)
 	int ret, status;
 
 	SKIP_IF(!have_htm());
+	SKIP_IF(htm_is_synthetic());
 	shm_id = shmget(IPC_PRIVATE, sizeof(int) * 3, 0777|IPC_CREAT);
 	pid = fork();
 	if (pid < 0) {
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
index 2ecfa1158e2b..e112a34fbe59 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
@@ -129,6 +129,7 @@ int ptrace_tm_spd_tar(void)
 	int ret, status;
 
 	SKIP_IF(!have_htm());
+	SKIP_IF(htm_is_synthetic());
 	shm_id = shmget(IPC_PRIVATE, sizeof(int) * 3, 0777|IPC_CREAT);
 	pid = fork();
 	if (pid == 0)
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
index 46ef378a15ec..d0db6df0f0ea 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
@@ -117,6 +117,7 @@ int ptrace_tm_tar(void)
 	int ret, status;
 
 	SKIP_IF(!have_htm());
+	SKIP_IF(htm_is_synthetic());
 	shm_id = shmget(IPC_PRIVATE, sizeof(int) * 2, 0777|IPC_CREAT);
 	pid = fork();
 	if (pid == 0)
diff --git a/tools/testing/selftests/powerpc/tm/tm-resched-dscr.c b/tools/testing/selftests/powerpc/tm/tm-resched-dscr.c
index 4cdb83964bb3..85c940ae6ff8 100644
--- a/tools/testing/selftests/powerpc/tm/tm-resched-dscr.c
+++ b/tools/testing/selftests/powerpc/tm/tm-resched-dscr.c
@@ -40,6 +40,7 @@ int test_body(void)
 	uint64_t rv, dscr1 = 1, dscr2, texasr;
 
 	SKIP_IF(!have_htm());
+	SKIP_IF(htm_is_synthetic());
 
 	printf("Check DSCR TM context switch: ");
 	fflush(stdout);
diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-stack.c b/tools/testing/selftests/powerpc/tm/tm-signal-stack.c
index cdcf8c5bbbc7..68807aac8dd3 100644
--- a/tools/testing/selftests/powerpc/tm/tm-signal-stack.c
+++ b/tools/testing/selftests/powerpc/tm/tm-signal-stack.c
@@ -35,6 +35,7 @@ int tm_signal_stack()
 	int pid;
 
 	SKIP_IF(!have_htm());
+	SKIP_IF(htm_is_synthetic());
 
 	pid = fork();
 	if (pid < 0)
diff --git a/tools/testing/selftests/powerpc/tm/tm-syscall.c b/tools/testing/selftests/powerpc/tm/tm-syscall.c
index becb8207b432..467a6b3134b2 100644
--- a/tools/testing/selftests/powerpc/tm/tm-syscall.c
+++ b/tools/testing/selftests/powerpc/tm/tm-syscall.c
@@ -25,7 +25,6 @@ extern int getppid_tm_suspended(void);
 unsigned retries = 0;
 
 #define TEST_DURATION 10 /* seconds */
-#define TM_RETRIES 100
 
 pid_t getppid_tm(bool suspend)
 {
@@ -67,6 +66,7 @@ int tm_syscall(void)
 	struct timeval end, now;
 
 	SKIP_IF(!have_htm_nosc());
+	SKIP_IF(htm_is_synthetic());
 
 	setbuf(stdout, NULL);
 
diff --git a/tools/testing/selftests/powerpc/tm/tm.h b/tools/testing/selftests/powerpc/tm/tm.h
index c5a1e5c163fc..c03c6e778876 100644
--- a/tools/testing/selftests/powerpc/tm/tm.h
+++ b/tools/testing/selftests/powerpc/tm/tm.h
@@ -10,6 +10,9 @@
 #include <asm/tm.h>
 
 #include "utils.h"
+#include "reg.h"
+
+#define TM_RETRIES 100
 
 static inline bool have_htm(void)
 {
@@ -31,6 +34,39 @@ static inline bool have_htm_nosc(void)
 #endif
 }
 
+/*
+ * Transactional Memory was removed in ISA 3.1. A synthetic TM implementation
+ * is provided on P10 for threads running in P8/P9 compatibility  mode. The
+ * synthetic implementation immediately fails after tbegin. This failure sets
+ * Bit 7 (Failure Persistent) and Bit 15 (Implementation-specific).
+ */
+static inline bool htm_is_synthetic(void)
+{
+	int i;
+
+	/*
+	 * Per the ISA, the Failure Persistent bit may be incorrect. Try a few
+	 * times in case we got an Implementation-specific failure on a non ISA
+	 * v3.1 system. On these systems the Implementation-specific failure
+	 * should not be persistent.
+	 */
+	for (i = 0; i < TM_RETRIES; i++) {
+		asm volatile(
+		"tbegin.;"
+		"beq 1f;"
+		"tend.;"
+		"1:"
+		:
+		:
+		: "memory");
+
+		if ((__builtin_get_texasr() & (TEXASR_FP | TEXASR_IC)) !=
+		    (TEXASR_FP | TEXASR_IC))
+			break;
+	}
+	return i == TM_RETRIES;
+}
+
 static inline long failure_code(void)
 {
 	return __builtin_get_texasru() >> 24;
-- 
2.25.1

Re: [PATCH 2/2] selftests: Skip TM tests on synthetic TM implementations

[Jordan Niethe]

On Tue, Jun 8, 2021 at 9:37 AM Jordan Niethe <jniethe5@gmail.com> wrote:
>
> Transactional Memory was removed from the architecture in ISA v3.1. For
> threads running in P8/P9 compatibility mode on P10 a synthetic TM
> implementation is provided. In this implementation, tbegin. always sets
> cr0 eq meaning the abort handler is always called. This is not an issue
> as users of TM are expected to have a fallback non transactional way to
> make forward progress in the abort handler.
>
> As the TM self tests exist only to test TM, no alternative path forward
> is provided, leading to them timing out and failing on the synthetic TM
> implementation.
>
> The TEXASR indicates if a transaction failure is due to a synthetic
> implementation. Check for a synthetic implementation and skip the TM
> tests if so.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
>  .../selftests/powerpc/ptrace/ptrace-tm-gpr.c  |  1 +
>  .../powerpc/ptrace/ptrace-tm-spd-gpr.c        |  1 +
>  .../powerpc/ptrace/ptrace-tm-spd-tar.c        |  1 +
>  .../selftests/powerpc/ptrace/ptrace-tm-tar.c  |  1 +
>  .../selftests/powerpc/tm/tm-resched-dscr.c    |  1 +
>  .../selftests/powerpc/tm/tm-signal-stack.c    |  1 +
>  .../testing/selftests/powerpc/tm/tm-syscall.c |  2 +-
>  tools/testing/selftests/powerpc/tm/tm.h       | 36 +++++++++++++++++++
>  8 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
> index 7df7100a29be..67ca297c5cca 100644
> --- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
> +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
> @@ -113,6 +113,7 @@ int ptrace_tm_gpr(void)
>         int ret, status;
>
>         SKIP_IF(!have_htm());
> +       SKIP_IF(htm_is_synthetic());
>         shm_id = shmget(IPC_PRIVATE, sizeof(int) * 2, 0777|IPC_CREAT);
>         pid = fork();
>         if (pid < 0) {
> diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c
> index 8706bea5d015..6f2bce1b6c5d 100644
> --- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c
> +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-gpr.c
> @@ -119,6 +119,7 @@ int ptrace_tm_spd_gpr(void)
>         int ret, status;
>
>         SKIP_IF(!have_htm());
> +       SKIP_IF(htm_is_synthetic());
>         shm_id = shmget(IPC_PRIVATE, sizeof(int) * 3, 0777|IPC_CREAT);
>         pid = fork();
>         if (pid < 0) {
> diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
> index 2ecfa1158e2b..e112a34fbe59 100644
> --- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
> +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
> @@ -129,6 +129,7 @@ int ptrace_tm_spd_tar(void)
>         int ret, status;
>
>         SKIP_IF(!have_htm());
> +       SKIP_IF(htm_is_synthetic());
>         shm_id = shmget(IPC_PRIVATE, sizeof(int) * 3, 0777|IPC_CREAT);
>         pid = fork();
>         if (pid == 0)
> diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
> index 46ef378a15ec..d0db6df0f0ea 100644
> --- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
> +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
> @@ -117,6 +117,7 @@ int ptrace_tm_tar(void)
>         int ret, status;
>
>         SKIP_IF(!have_htm());
> +       SKIP_IF(htm_is_synthetic());
>         shm_id = shmget(IPC_PRIVATE, sizeof(int) * 2, 0777|IPC_CREAT);
>         pid = fork();
>         if (pid == 0)
> diff --git a/tools/testing/selftests/powerpc/tm/tm-resched-dscr.c b/tools/testing/selftests/powerpc/tm/tm-resched-dscr.c
> index 4cdb83964bb3..85c940ae6ff8 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-resched-dscr.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-resched-dscr.c
> @@ -40,6 +40,7 @@ int test_body(void)
>         uint64_t rv, dscr1 = 1, dscr2, texasr;
>
>         SKIP_IF(!have_htm());
> +       SKIP_IF(htm_is_synthetic());
>
>         printf("Check DSCR TM context switch: ");
>         fflush(stdout);
> diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-stack.c b/tools/testing/selftests/powerpc/tm/tm-signal-stack.c
> index cdcf8c5bbbc7..68807aac8dd3 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-signal-stack.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-signal-stack.c
> @@ -35,6 +35,7 @@ int tm_signal_stack()
>         int pid;
>
>         SKIP_IF(!have_htm());
> +       SKIP_IF(htm_is_synthetic());
>
>         pid = fork();
>         if (pid < 0)
> diff --git a/tools/testing/selftests/powerpc/tm/tm-syscall.c b/tools/testing/selftests/powerpc/tm/tm-syscall.c
> index becb8207b432..467a6b3134b2 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-syscall.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-syscall.c
> @@ -25,7 +25,6 @@ extern int getppid_tm_suspended(void);
>  unsigned retries = 0;
>
>  #define TEST_DURATION 10 /* seconds */
> -#define TM_RETRIES 100
>
>  pid_t getppid_tm(bool suspend)
>  {
> @@ -67,6 +66,7 @@ int tm_syscall(void)
>         struct timeval end, now;
>
>         SKIP_IF(!have_htm_nosc());
> +       SKIP_IF(htm_is_synthetic());
>
>         setbuf(stdout, NULL);
>
> diff --git a/tools/testing/selftests/powerpc/tm/tm.h b/tools/testing/selftests/powerpc/tm/tm.h
> index c5a1e5c163fc..c03c6e778876 100644
> --- a/tools/testing/selftests/powerpc/tm/tm.h
> +++ b/tools/testing/selftests/powerpc/tm/tm.h
> @@ -10,6 +10,9 @@
>  #include <asm/tm.h>
>
>  #include "utils.h"
> +#include "reg.h"
> +
> +#define TM_RETRIES 100
>
>  static inline bool have_htm(void)
>  {
> @@ -31,6 +34,39 @@ static inline bool have_htm_nosc(void)
>  #endif
>  }
>
> +/*
> + * Transactional Memory was removed in ISA 3.1. A synthetic TM implementation
> + * is provided on P10 for threads running in P8/P9 compatibility  mode. The
> + * synthetic implementation immediately fails after tbegin. This failure sets
> + * Bit 7 (Failure Persistent) and Bit 15 (Implementation-specific).
> + */
> +static inline bool htm_is_synthetic(void)
> +{
> +       int i;
> +
> +       /*
> +        * Per the ISA, the Failure Persistent bit may be incorrect. Try a few
> +        * times in case we got an Implementation-specific failure on a non ISA
> +        * v3.1 system. On these systems the Implementation-specific failure
> +        * should not be persistent.
> +        */
> +       for (i = 0; i < TM_RETRIES; i++) {
> +               asm volatile(
> +               "tbegin.;"
> +               "beq 1f;"
> +               "tend.;"
> +               "1:"
> +               :
> +               :
> +               : "memory");
> +
> +               if ((__builtin_get_texasr() & (TEXASR_FP | TEXASR_IC)) !=
> +                   (TEXASR_FP | TEXASR_IC))
> +                       break;
> +       }
> +       return i == TM_RETRIES;
> +}
> +
>  static inline long failure_code(void)
>  {
>         return __builtin_get_texasru() >> 24;
> --
> 2.25.1
>
There's a couple more tests that need the same treatment, Will send a
new version.