Re: [PATCH v8 2/3] rseq/selftests: add rseq_offset_deref_addv

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

----- On Sep 24, 2020, at 9:33 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> ----- On Sep 23, 2020, at 7:36 PM, Peter Oskolkov posk@google.com wrote:
> 
> The patch title should state that it only adds rseq_offset_deref_addv
> to x86-64. Considering that other architecture maintainers will look
> at it as inspiration for other architectures, we should also state in
> the commit message that architectures implementing it should define
> "RSEQ_ARCH_HAS_OFFSET_DEREF_ADDV".

With those changes applied, you can also add my:

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>


Thanks,

Mathieu

> 
> Thanks,
> 
> Mathieu
> 
>> This patch adds rseq_offset_deref_addv function to
>> tools/testing/selftests/rseq/rseq-x86.h, to be used
>> in a selftest in the next patch in the patchset.
>> 
>> v7->v8: this patch split out of the v7 selftest patch.
>> 
>> Signed-off-by: Peter Oskolkov <posk@google.com>
>> ---
>> tools/testing/selftests/rseq/rseq-x86.h | 57 +++++++++++++++++++++++++
>> 1 file changed, 57 insertions(+)
>> 
>> diff --git a/tools/testing/selftests/rseq/rseq-x86.h
>> b/tools/testing/selftests/rseq/rseq-x86.h
>> index b2da6004fe30..640411518e46 100644
>> --- a/tools/testing/selftests/rseq/rseq-x86.h
>> +++ b/tools/testing/selftests/rseq/rseq-x86.h
>> @@ -279,6 +279,63 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
>> #endif
>> }
>> 
>> +#define RSEQ_ARCH_HAS_OFFSET_DEREF_ADDV
>> +
>> +/*
>> + *   pval = *(ptr+off)
>> + *  *pval += inc;
>> + */
>> +static inline __attribute__((always_inline))
>> +int rseq_offset_deref_addv(intptr_t *ptr, off_t off, intptr_t inc, int cpu)
>> +{
>> +	RSEQ_INJECT_C(9)
>> +
>> +	__asm__ __volatile__ goto (
>> +		RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
>> +#ifdef RSEQ_COMPARE_TWICE
>> +		RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
>> +#endif
>> +		/* Start rseq by storing table entry pointer into rseq_cs. */
>> +		RSEQ_ASM_STORE_RSEQ_CS(1, 3b, RSEQ_CS_OFFSET(%[rseq_abi]))
>> +		RSEQ_ASM_CMP_CPU_ID(cpu_id, RSEQ_CPU_ID_OFFSET(%[rseq_abi]), 4f)
>> +		RSEQ_INJECT_ASM(3)
>> +#ifdef RSEQ_COMPARE_TWICE
>> +		RSEQ_ASM_CMP_CPU_ID(cpu_id, RSEQ_CPU_ID_OFFSET(%[rseq_abi]), %l[error1])
>> +#endif
>> +		/* get p+v */
>> +		"movq %[ptr], %%rbx\n\t"
>> +		"addq %[off], %%rbx\n\t"
>> +		/* get pv */
>> +		"movq (%%rbx), %%rcx\n\t"
>> +		/* *pv += inc */
>> +		"addq %[inc], (%%rcx)\n\t"
>> +		"2:\n\t"
>> +		RSEQ_INJECT_ASM(4)
>> +		RSEQ_ASM_DEFINE_ABORT(4, "", abort)
>> +		: /* gcc asm goto does not allow outputs */
>> +		: [cpu_id]		"r" (cpu),
>> +		  [rseq_abi]		"r" (&__rseq_abi),
>> +		  /* final store input */
>> +		  [ptr]			"m" (*ptr),
>> +		  [off]			"er" (off),
>> +		  [inc]			"er" (inc)
>> +		: "memory", "cc", "rax", "rbx", "rcx"
>> +		  RSEQ_INJECT_CLOBBER
>> +		: abort
>> +#ifdef RSEQ_COMPARE_TWICE
>> +		  , error1
>> +#endif
>> +	);
>> +	return 0;
>> +abort:
>> +	RSEQ_INJECT_FAILED
>> +	return -1;
>> +#ifdef RSEQ_COMPARE_TWICE
>> +error1:
>> +	rseq_bug("cpu_id comparison failed");
>> +#endif
>> +}
>> +
>> static inline __attribute__((always_inline))
>> int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
>> 				 intptr_t *v2, intptr_t newv2,
>> --
>> 2.28.0.709.gb0816b6eb0-goog
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com