Re: [PATCH v2 02/20] arm64: percpu: Fix this_cpu_and() mask generation

[Jinjie Ruan <ruanjinjie@huawei.com>]

在 2026/8/5 1:04, Mark Rutland 写道:
> The arm64 implementation of this_cpu_and(pcp, val) is built in terms of
> ANDNOT operations, which requires the 'val' argument to be bitwise
> negated. The bitwise negation is not implemented correctly, with two
> bugs described below.
> 
> (1) The bitwise negation is performed as '~val' rather than '~(val)'.
>     This won't always generate the expected value when 'val' is an
>     expression.
> 
>     For example, for this_cpu_and(pcp, 1 - 1):
> 
>     * 'val'    is  '1 - 1'   ===> (int) 0x00000000
>     * '~val'   is '~1 - 1'   ===> (int) 0xfffffffd
>     * '~(val)' is '~(1 - 1)' ===> (int) 0xffffffff
> 
>     ... and thus bit[1] of 'pcp' would be preserved unexpectedly by the
>     ANDNOT operation.
> 
> (2) The bitwise negation is performed on 'val' before it has been cast
>     to (at least) the width of 'pcp'. This won't always generate the
>     expected value for the upper bits.
> 
>     For example, for this_cpu_and(pcp, zero), where 'pcp' is a u64 and
>     'zero' is a u32:
> 
>     * 'zero'           ===> (u32) 0x00000000
>     * '~(zero)'        ===> (u32) 0xffffffff
>     * '(u64)~(zero)'   ===> (u64) 0x00000000ffffffff
>     * '~((u64)(zero))' ===> (u64) 0xffffffffffffffff
> 
>     ... and thus bits[63:32] of 'pcp' would be preserved unexpectedly by
>     the ANDNOT operation.
> 
> Fix these issues by adding brackets around 'val', and by casting 'val'
> to an appropriately-sized type before bitwise negation.
> 
> The bugs described above can be seen from the disassembly of the
> following test code:
> 
> | void this_cpu_and_u64_zero(u64 __percpu *pcp)
> | {
> |         u64 zero = 0;
> |         this_cpu_and(*pcp, zero);
> | }
> |
> | void this_cpu_and_u32_zero(u64 __percpu *pcp)
> | {
> |         u32 zero = 0;
> |         this_cpu_and(*pcp, zero);
> | }
> |
> | void this_cpu_and_expr_zero(u64 __percpu *pcp)
> | {
> |         this_cpu_and(*pcp, 1 - 1);
> | }
> |
> | void this_cpu_and_expr_zero_brackets(u64 __percpu *pcp)
> | {
> |         this_cpu_and(*pcp, (1 - 1));
> | }
> 
> Before this patch:
> 
> | <this_cpu_and_u64_zero>:
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mrs     x1, sp_el0
> |        mov     x29, sp
> |        ldr     w2, [x1, #8]
> |        add     w2, w2, #0x1
> |        str     w2, [x1, #8]
> |        mov     x3, #0xffffffffffffffff         // #-1
> |        mrs     x2, tpidr_el1
> |        add     x0, x0, x2
> | 1:     ldxr    x5, [x0]
> |        bic     x5, x5, x3
> |        stxr    w4, x5, [x0]
> |        cbnz    w4, 1b
> |        ldr     x0, [x1, #8]
> |        add     x0, x0, x3
> |        str     w0, [x1, #8]
> |        cbz     x0, 2f
> |        ldr     x0, [x1, #8]
> |        cbnz    x0, 3f
> | 2:     bl      preempt_schedule_notrace
> | 3:     ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret
> |
> | <this_cpu_and_u32_zero>:
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mrs     x1, sp_el0
> |        mov     x29, sp
> |        ldr     w2, [x1, #8]
> |        add     w2, w2, #0x1
> |        str     w2, [x1, #8]
> |        mov     x3, #0xffffffff                 // #4294967295
> |        mrs     x2, tpidr_el1
> |        add     x0, x0, x2
> | 1:     ldxr    x5, [x0]
> |        bic     x5, x5, x3
> |        stxr    w4, x5, [x0]
> |        cbnz    w4, 1b
> |        ldr     x0, [x1, #8]
> |        sub     x0, x0, #0x1
> |        str     w0, [x1, #8]
> |        cbz     x0, 2f
> |        ldr     x0, [x1, #8]
> |        cbnz    x0, 3f
> | 2:     bl      preempt_schedule_notrace
> | 3:     ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret
> |
> | <this_cpu_and_expr_zero>:
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mrs     x1, sp_el0
> |        mov     x29, sp
> |        ldr     w2, [x1, #8]
> |        add     w2, w2, #0x1
> |        str     w2, [x1, #8]
> |        mov     x3, #0xfffffffffffffffd         // #-3
> |        mrs     x2, tpidr_el1
> |        add     x0, x0, x2
> | 1:     ldxr    x5, [x0]
> |        bic     x5, x5, x3
> |        stxr    w4, x5, [x0]
> |        cbnz    w4, 1b
> |        ldr     x0, [x1, #8]
> |        sub     x0, x0, #0x1
> |        str     w0, [x1, #8]
> |        cbz     x0, 2f
> |        ldr     x0, [x1, #8]
> |        cbnz    x0, 3f
> | 2:     bl      preempt_schedule_notrace
> | 3:     ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret
> 
> After this patch:
> 
> | <this_cpu_and_u64_zero>:
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mrs     x1, sp_el0
> |        mov     x29, sp
> |        ldr     w2, [x1, #8]
> |        add     w2, w2, #0x1
> |        str     w2, [x1, #8]
> |        mov     x3, #0xffffffffffffffff         // #-1
> |        mrs     x2, tpidr_el1
> |        add     x0, x0, x2
> | 1:     ldxr    x5, [x0]
> |        bic     x5, x5, x3
> |        stxr    w4, x5, [x0]
> |        cbnz    w4, 1b
> |        ldr     x0, [x1, #8]
> |        add     x0, x0, x3
> |        str     w0, [x1, #8]
> |        cbz     x0, 2f
> |        ldr     x0, [x1, #8]
> |        cbnz    x0, 3f
> | 2:     bl      0 <preempt_schedule_notrace>
> | 3:     ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret
> |
> | <this_cpu_and_u32_zero>:
> |        b       this_cpu_and_u64_zero
> |
> | <this_cpu_and_expr_zero>:
> |        b       this_cpu_and_u64_zero
> |
> | <this_cpu_and_expr_zero_brackets>:
> |        b       this_cpu_and_u64_zero
> 
> Fixes: 959bf2fd03b5 ("arm64: percpu: Rewrite per-cpu ops to allow use of LSE atomics")
> Signed-off-by: Mark Rutland <mark.rutland@arm.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: Jinjie Ruan <ruanjinjie@huawei.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 | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/percpu.h b/arch/arm64/include/asm/percpu.h
> index 63bbfd4944a37..31193bcf89a2b 100644
> --- a/arch/arm64/include/asm/percpu.h
> +++ b/arch/arm64/include/asm/percpu.h
> @@ -206,13 +206,13 @@ PERCPU_RET_OP(add, add, ldadd)
>  	_pcp_protect_return(__percpu_add_return_case_64, pcp, val)
>  
>  #define this_cpu_and_1(pcp, val)	\
> -	_pcp_protect(__percpu_andnot_case_8, pcp, ~val)
> +	_pcp_protect(__percpu_andnot_case_8, pcp, ~(u8)(val))
>  #define this_cpu_and_2(pcp, val)	\
> -	_pcp_protect(__percpu_andnot_case_16, pcp, ~val)
> +	_pcp_protect(__percpu_andnot_case_16, pcp, ~(u16)(val))
>  #define this_cpu_and_4(pcp, val)	\
> -	_pcp_protect(__percpu_andnot_case_32, pcp, ~val)
> +	_pcp_protect(__percpu_andnot_case_32, pcp, ~(u32)(val))
>  #define this_cpu_and_8(pcp, val)	\
> -	_pcp_protect(__percpu_andnot_case_64, pcp, ~val)
> +	_pcp_protect(__percpu_andnot_case_64, pcp, ~(u64)(val))

Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>

>  
>  #define this_cpu_or_1(pcp, val)		\
>  	_pcp_protect(__percpu_or_case_8, pcp, val)