Re: [PATCH] rseq/selftests: Fix riscv rseq_offset_deref_addv inline asm

[Stafford Horne <shorne@gmail.com>]

On Thu, Jan 09, 2025 at 06:29:01PM -0800, Charlie Jenkins wrote:
> On Fri, Jan 03, 2025 at 04:03:26AM +0000, Stafford Horne wrote:
> > When working on OpenRISC support for restartable sequences I noticed
> > and fixed these two issues with the riscv support bits.
> > 
> >  1 The 'inc' argument to RSEQ_ASM_OP_R_DEREF_ADDV was being implicitly
> >    passed to the macro.  Fix this by adding 'inc' to the list of macro
> >    arguments.
> >  2 The inline asm input constraints for 'inc' and 'off' use "er",  The
> >    riscv gcc port does not have an "e" constraint, this looks to be
> >    copied from the x86 port.  Fix this by just using an "r" constraint.
> > 
> > I have compile tested this only for riscv.  However, the same fixes I
> > use in the OpenRISC rseq selftests and everything passes with no issues.
> 
> Thank you for these changes! I suppose these tests hadn't been ran on
> riscv before... I ran the tests on QEMU and they all passed :)

Thanks for confirming.

> Tested-by: Charlie Jenkins <charlie@rivosinc.com>
> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
> 
> This should also have a fixes tag:
> 
> Fixes: 171586a6ab66 ("selftests/rseq: riscv: Template memory ordering and percpu access mode")

Right, If ok I think Palmer / the maintainer can add that when picking up the
patch.  If requested I can add that to a v2 though.

-Stafford

> > 
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > ---
> >  tools/testing/selftests/rseq/rseq-riscv-bits.h | 6 +++---
> >  tools/testing/selftests/rseq/rseq-riscv.h      | 2 +-
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/rseq/rseq-riscv-bits.h b/tools/testing/selftests/rseq/rseq-riscv-bits.h
> > index de31a0143139..f02f411d550d 100644
> > --- a/tools/testing/selftests/rseq/rseq-riscv-bits.h
> > +++ b/tools/testing/selftests/rseq/rseq-riscv-bits.h
> > @@ -243,7 +243,7 @@ int RSEQ_TEMPLATE_IDENTIFIER(rseq_offset_deref_addv)(intptr_t *ptr, off_t off, i
> >  #ifdef RSEQ_COMPARE_TWICE
> >  				  RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, "%l[error1]")
> >  #endif
> > -				  RSEQ_ASM_OP_R_DEREF_ADDV(ptr, off, 3)
> > +				  RSEQ_ASM_OP_R_DEREF_ADDV(ptr, off, inc, 3)
> >  				  RSEQ_INJECT_ASM(4)
> >  				  RSEQ_ASM_DEFINE_ABORT(4, abort)
> >  				  : /* gcc asm goto does not allow outputs */
> > @@ -251,8 +251,8 @@ int RSEQ_TEMPLATE_IDENTIFIER(rseq_offset_deref_addv)(intptr_t *ptr, off_t off, i
> >  				    [current_cpu_id]		"m" (rseq_get_abi()->RSEQ_TEMPLATE_CPU_ID_FIELD),
> >  				    [rseq_cs]			"m" (rseq_get_abi()->rseq_cs.arch.ptr),
> >  				    [ptr]			"r" (ptr),
> > -				    [off]			"er" (off),
> > -				    [inc]			"er" (inc)
> > +				    [off]			"r" (off),
> > +				    [inc]			"r" (inc)
> >  				    RSEQ_INJECT_INPUT
> >  				  : "memory", RSEQ_ASM_TMP_REG_1
> >  				    RSEQ_INJECT_CLOBBER
> > diff --git a/tools/testing/selftests/rseq/rseq-riscv.h b/tools/testing/selftests/rseq/rseq-riscv.h
> > index 37e598d0a365..67d544aaa9a3 100644
> > --- a/tools/testing/selftests/rseq/rseq-riscv.h
> > +++ b/tools/testing/selftests/rseq/rseq-riscv.h
> > @@ -158,7 +158,7 @@ do {									\
> >  	"bnez	" RSEQ_ASM_TMP_REG_1 ", 222b\n"				\
> >  	"333:\n"
> >  
> > -#define RSEQ_ASM_OP_R_DEREF_ADDV(ptr, off, post_commit_label)		\
> > +#define RSEQ_ASM_OP_R_DEREF_ADDV(ptr, off, inc, post_commit_label)	\
> >  	"mv	" RSEQ_ASM_TMP_REG_1 ", %[" __rseq_str(ptr) "]\n"	\
> >  	RSEQ_ASM_OP_R_ADD(off)						\
> >  	REG_L	  RSEQ_ASM_TMP_REG_1 ", 0(" RSEQ_ASM_TMP_REG_1 ")\n"	\
> > -- 
> > 2.47.0
> > 
> > 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv