Re: [PATCH v2 2/2] selftests/powerpc: Add scv versions of the basic TM syscall tests

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 01/09/2021 à 18:54, Nicholas Piggin a écrit :
> The basic TM vs syscall test code hard codes an sc instruction for the
> system call, which fails to cover scv even when the userspace libc has
> support for it.
> 
> Duplicate the tests with hard coded scv variants so both are tested
> when possible.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   .../selftests/powerpc/tm/tm-syscall-asm.S     | 46 +++++++++++++++++++
>   .../testing/selftests/powerpc/tm/tm-syscall.c | 36 ++++++++++++---
>   2 files changed, 75 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/powerpc/tm/tm-syscall-asm.S b/tools/testing/selftests/powerpc/tm/tm-syscall-asm.S
> index bd1ca25febe4..849316831e6a 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-syscall-asm.S
> +++ b/tools/testing/selftests/powerpc/tm/tm-syscall-asm.S
> @@ -2,6 +2,10 @@
>   #include <ppc-asm.h>
>   #include <asm/unistd.h>
>   
> +/* ppc-asm.h does not define r0 or r1 */
> +#define r0 0
> +#define r1 1
> +

See https://github.com/gcc-mirror/gcc/blob/master/gcc/config/rs6000/ppc-asm.h

It doesn't not define r1 but it defines r0.

And it defines 'sp' as register 1.

>   	.text
>   FUNC_START(getppid_tm_active)
>   	tbegin.
> @@ -26,3 +30,45 @@ FUNC_START(getppid_tm_suspended)
>   1:
>   	li	r3, -1
>   	blr
> +
> +FUNC_START(getppid_scv_tm_active)
> +	mflr	r0
> +	std	r0,16(r1)
> +	stdu	r1,-32(r1)
> +	tbegin.
> +	beq 1f
> +	li	r0, __NR_getppid
> +	scv	0
> +	tend.
> +	addi	r1,r1,32
> +	ld	r0,16(r1)
> +	mtlr	r0
> +	blr
> +1:
> +	li	r3, -1
> +	addi	r1,r1,32
> +	ld	r0,16(r1)
> +	mtlr	r0
> +	blr
> +
> +FUNC_START(getppid_scv_tm_suspended)
> +	mflr	r0
> +	std	r0,16(r1)
> +	stdu	r1,-32(r1)
> +	tbegin.
> +	beq 1f
> +	li	r0, __NR_getppid
> +	tsuspend.
> +	scv	0
> +	tresume.
> +	tend.
> +	addi	r1,r1,32
> +	ld	r0,16(r1)
> +	mtlr	r0
> +	blr
> +1:
> +	li	r3, -1
> +	addi	r1,r1,32
> +	ld	r0,16(r1)
> +	mtlr	r0
> +	blr
> diff --git a/tools/testing/selftests/powerpc/tm/tm-syscall.c b/tools/testing/selftests/powerpc/tm/tm-syscall.c
> index becb8207b432..9a822208680e 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-syscall.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-syscall.c
> @@ -19,24 +19,37 @@
>   #include "utils.h"
>   #include "tm.h"
>   
> +#ifndef PPC_FEATURE2_SCV
> +#define PPC_FEATURE2_SCV               0x00100000 /* scv syscall */
> +#endif
> +
>   extern int getppid_tm_active(void);
>   extern int getppid_tm_suspended(void);
> +extern int getppid_scv_tm_active(void);
> +extern int getppid_scv_tm_suspended(void);
>   
>   unsigned retries = 0;
>   
>   #define TEST_DURATION 10 /* seconds */
>   #define TM_RETRIES 100
>   
> -pid_t getppid_tm(bool suspend)
> +pid_t getppid_tm(bool scv, bool suspend)
>   {
>   	int i;
>   	pid_t pid;
>   
>   	for (i = 0; i < TM_RETRIES; i++) {
> -		if (suspend)
> -			pid = getppid_tm_suspended();
> -		else
> -			pid = getppid_tm_active();
> +		if (suspend) {
> +			if (scv)
> +				pid = getppid_scv_tm_suspended();
> +			else
> +				pid = getppid_tm_suspended();
> +		} else {
> +			if (scv)
> +				pid = getppid_scv_tm_active();
> +			else
> +				pid = getppid_tm_active();
> +		}
>   
>   		if (pid >= 0)
>   			return pid;
> @@ -82,15 +95,24 @@ int tm_syscall(void)
>   		 * Test a syscall within a suspended transaction and verify
>   		 * that it succeeds.
>   		 */
> -		FAIL_IF(getppid_tm(true) == -1); /* Should succeed. */
> +		FAIL_IF(getppid_tm(false, true) == -1); /* Should succeed. */
>   
>   		/*
>   		 * Test a syscall within an active transaction and verify that
>   		 * it fails with the correct failure code.
>   		 */
> -		FAIL_IF(getppid_tm(false) != -1);  /* Should fail... */
> +		FAIL_IF(getppid_tm(false, false) != -1);  /* Should fail... */
>   		FAIL_IF(!failure_is_persistent()); /* ...persistently... */
>   		FAIL_IF(!failure_is_syscall());    /* ...with code syscall. */
> +
> +		/* Now do it all again with scv if it is available. */
> +		if (have_hwcap2(PPC_FEATURE2_SCV)) {
> +			FAIL_IF(getppid_tm(true, true) == -1); /* Should succeed. */
> +			FAIL_IF(getppid_tm(true, false) != -1);  /* Should fail... */
> +			FAIL_IF(!failure_is_persistent()); /* ...persistently... */
> +			FAIL_IF(!failure_is_syscall());    /* ...with code syscall. */
> +		}
> +
>   		gettimeofday(&now, 0);
>   	}
>   
>