Re: [PATCH v4 03/21] arm64: percpu: Fix LSE operations on {8,16}-bit types

[Vladimir Murzin <vladimir.murzin@arm.com>]

On 9/8/26 16:17, Mark Rutland wrote:
> The assembly for __percpu_##name##_case_##sz() and
> __percpu_##name##_return_case_##sz() doesn't use the 'sfx' macro
> argument to form the LSE instruction. Without 'sfx', a W register
> argument will imply a 32-bit memory location, and consequently
> {8,16}-bit ops will erroneously read and write 32 bits of memory when
> the LSE instruction is used.
> 
> Fix this by appending 'sfx' to 'op_lse' to LSE instruction. It is not
> necessary (and not valid) to append 'sfx' to 'op_llsc', as 'op_llsc' is
> a register-register operation which does not access memory (and does not
> take a size suffix).
> 
> Test case:
> 
> | void outline_this_cpu_add_u8(u8 __percpu *p, u8 v)
> | {
> | 	this_cpu_add(*p, v);
> | }
> 
> Generated inline code (same before and after this patch):
> 
> | <outline_this_cpu_add_u8>:
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mrs     x2, sp_el0
> |        mov     x29, sp
> |        ldr     w3, [x2, #8]
> |        add     w3, w3, #0x1
> |        str     w3, [x2, #8]
> |        mrs     x3, tpidr_el1
> |        add     x0, x0, x3
> | 1:     ldxrb   w5, [x0]        // <----------------
> |        add     w5, w5, w1      // LL/SC alternative
> |        stxrb   w4, w5, [x0]    // instructions here
> |        cbnz    w4, 1b          // <----------------
> |        ldr     x0, [x2, #8]
> |        sub     x0, x0, #0x1
> |        str     w0, [x2, #8]
> |        cbz     x0, 2f
> |        ldr     x0, [x2, #8]
> |        cbnz    x0, 3f
> | 2:     bl      preempt_schedule_notrace
> | 3:     ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret
> 
> Generated LSE alternative before this patch:
> 
> |        ldadd   w1, w5, [x0]    // 32-bit LDADD
> |        nop
> |        nop
> |        nop
> 
> Generated LSE alternative after this patch:
> 
> |        ldaddb  w1, w5, [x0]    // 8-bit LDADDB
> |        nop
> |        nop
> |        nop
> 
> Fixes: 959bf2fd03b5 ("arm64: percpu: Rewrite per-cpu ops to allow use of LSE atomics")
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Vladimir Murzin <vladimir.murzin@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Yang Shi <yang@os.amperecomputing.com>
> Cc: stable@vger.kernel.org
> ---
>  arch/arm64/include/asm/percpu.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/percpu.h b/arch/arm64/include/asm/percpu.h
> index 31193bcf89a2b..8cf4068ce1b56 100644
> --- a/arch/arm64/include/asm/percpu.h
> +++ b/arch/arm64/include/asm/percpu.h
> @@ -77,7 +77,7 @@ __percpu_##name##_case_##sz(void *ptr, unsigned long val)		\
>  	"	stxr" #sfx "\t%w[loop], %" #w "[tmp], %[ptr]\n"		\
>  	"	cbnz	%w[loop], 1b",					\
>  	/* LSE atomics */						\
> -		#op_lse "\t%" #w "[val], %" #w "[tmp], %[ptr]\n"	\
> +		#op_lse #sfx "\t%" #w "[val], %" #w "[tmp], %[ptr]\n"	\
>  		__nops(3))						\
>  	: [loop] "=&r" (loop), [tmp] "=&r" (tmp),			\
>  	  [ptr] "+Q"(*(u##sz *)ptr)					\
> @@ -98,7 +98,7 @@ __percpu_##name##_return_case_##sz(void *ptr, unsigned long val)	\
>  	"	stxr" #sfx "\t%w[loop], %" #w "[ret], %[ptr]\n"		\
>  	"	cbnz	%w[loop], 1b",					\
>  	/* LSE atomics */						\
> -		#op_lse "\t%" #w "[val], %" #w "[ret], %[ptr]\n"	\
> +		#op_lse #sfx "\t%" #w "[val], %" #w "[ret], %[ptr]\n"	\
>  		#op_llsc "\t%" #w "[ret], %" #w "[ret], %" #w "[val]\n"	\
>  		__nops(2))						\
>  	: [loop] "=&r" (loop), [ret] "=&r" (ret),			\
> -- 2.30.2
> 

FWIW,

Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>