| Message ID | 20210901165418.1412891-2-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] powerpc/64s: system call scv tabort fix for corrupt irq soft-mask state | expand |
| Related | show |
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); > } > >
Excerpts from Christophe Leroy's message of September 2, 2021 3:15 am: > > > 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. Oops, I'll fix that. > And it defines 'sp' as register 1. Does userspace code typically use that? Kernel code AFAIKS does not. Thanks, Nick
Nicholas Piggin <npiggin@gmail.com> writes: > Excerpts from Christophe Leroy's message of September 2, 2021 3:15 am: >> 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. > > Oops, I'll fix that. > >> And it defines 'sp' as register 1. > > Does userspace code typically use that? Kernel code AFAIKS does not. Some does, but it's not used consistently IME. I'd prefer you just use %r1. cheers
Nicholas Piggin <npiggin@gmail.com> writes: > 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 > + > .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 There's some macros in tools/testing/selftests/powerpc/include/basic_asm.h that can do some of this boiler plate stack setup/teardown. Incremental diff below to use them, only build tested. cheers diff --git a/tools/testing/selftests/powerpc/tm/tm-syscall-asm.S b/tools/testing/selftests/powerpc/tm/tm-syscall-asm.S index 849316831e6a..a73694daca71 100644 --- a/tools/testing/selftests/powerpc/tm/tm-syscall-asm.S +++ b/tools/testing/selftests/powerpc/tm/tm-syscall-asm.S @@ -1,10 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0 */ -#include <ppc-asm.h> -#include <asm/unistd.h> - -/* ppc-asm.h does not define r0 or r1 */ -#define r0 0 -#define r1 1 +#include <basic_asm.h> .text FUNC_START(getppid_tm_active) @@ -32,29 +27,21 @@ FUNC_START(getppid_tm_suspended) blr FUNC_START(getppid_scv_tm_active) - mflr r0 - std r0,16(r1) - stdu r1,-32(r1) + PUSH_BASIC_STACK(0) tbegin. beq 1f li r0, __NR_getppid scv 0 tend. - addi r1,r1,32 - ld r0,16(r1) - mtlr r0 + POP_BASIC_STACK(0) blr 1: li r3, -1 - addi r1,r1,32 - ld r0,16(r1) - mtlr r0 + POP_BASIC_STACK(0) blr FUNC_START(getppid_scv_tm_suspended) - mflr r0 - std r0,16(r1) - stdu r1,-32(r1) + PUSH_BASIC_STACK(0) tbegin. beq 1f li r0, __NR_getppid @@ -62,13 +49,9 @@ FUNC_START(getppid_scv_tm_suspended) scv 0 tresume. tend. - addi r1,r1,32 - ld r0,16(r1) - mtlr r0 + POP_BASIC_STACK(0) blr 1: li r3, -1 - addi r1,r1,32 - ld r0,16(r1) - mtlr r0 + POP_BASIC_STACK(0) blr
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 + .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); }
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(-)