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

[Mark Rutland <mark.rutland@arm.com>]

On Thu, Aug 06, 2026 at 09:28:15AM +0100, David Laight wrote:
> On Wed, 5 Aug 2026 14:02:03 +0100
> Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Aug 05, 2026 at 10:14:16AM +0100, David Laight wrote:
> > > On Tue,  4 Aug 2026 18:04:45 +0100
> > > Mark Rutland <mark.rutland@arm.com> wrote:
> > > >  #define this_cpu_and_8(pcp, val)	\
> > > > -	_pcp_protect(__percpu_andnot_case_64, pcp, ~val)
> > > > +	_pcp_protect(__percpu_andnot_case_64, pcp, ~(u64)(val))  
> > > 
> > > This one still isn't right.
> > > If val is a signed int with a negative value then it is sign extended
> > > before being inverted.
> > > 	val             (int)0x80000000
> > > 	(u64)(val)   0xffffffff80000000
> > > 	~(u64)(val)  0x000000007fffffff
> > > Something like ~(u64)((val) + 0u) will DTRT.  
> > 
> > As above, where have you got that idea from?
> > 
> > AFAICT, a smaller signed type *should* be sign extended, and that must
> > happen before bitwise negation, since that bitwise negation is to cancel
> > out the NOT part of the ANDNOT operation.
> >  
> > Think:
> > 
> >     'pcp'                    is (u64) 0x0123456789abcdef
> >     'val'                    is (int) 0x800000000
> >     '(u64)(val)'             is (u64) 0xffffffff80000000
> >     'pcp & (u64)(val)'       is (u64) 0x0123456780000000
> > 
> >     '~(u64)(val)'            is (u64) 0x000000007fffffff
> >     'pcp ANDNOT ~(u64)(val)' is (u64) 0x0123456780000000
> 
> The problem tends to arise with (u8)128 << 24 which is signed even
> though that is never intended.

Never intended by whom?

The expression '(u8)128 << 24' has int type. If an int is passed into a
u64 function parameter, it will be sign extended to 64 bits. If that's
passed into a binary expression against a u64, it will be sign extended.
That's the way __this_cpu_and() evaluates its argument too.

This is no different from calling any other function. If that caller
doesn't want the int value sign-extended to 64 bits to match the 64-bit
argument type, the caller needs to cast to an unsigned type.

See:

| void some_callee(u64 arg);
| 
| void some_caller(u8 arg)
| {
|         some_caller(arg << 24).
| }
|
| u64 some_inline_and(u8 arg)
| {
|         u64 lhs = 0x0123456789abcdef;
|         return lhs & (arg << 24);
| }
|
| void outline___this_cpu_and_expr(u64 __percpu *p)
| {
|        __this_cpu_and(*p, (u8)128 << 24);
| }

Generated code:

| <some_caller>:
|        paciasp
|        stp     x29, x30, [sp, #-16]!
|        lsl     w0, w0, #24           // left-shift 24
|        mov     x29, sp
|        sxtw    x0, w0                // sign-extend from 32-bit to 64-bit
|        bl      some_callee
|        ldp     x29, x30, [sp], #16
|        autiasp
|        ret
|
| <some_inline_and>:
|        lsl     w0, w0, #24           // left-shift 24
|        mov     x1, #0xcdef
|        movk    x1, #0x89ab, lsl #16
|        sxtw    x0, w0                // sign-extend from 32-bit to 64-bit
|        movk    x1, #0x4567, lsl #32
|        movk    x1, #0x123, lsl #48
|        and     x0, x0, x1
|        ret
|
| <outline___this_cpu_and_expr>:
|        mrs     x2, tpidr_el1
|        ldr     x1, [x0, x2]
|        and     x1, x1, #0xffffffff80000000
|        str     x1, [x0, x2]
|        ret

> To my mind sign extension prior to and/or operations is almost
> certainly unexpected.

As above, I disagree. I think you're starting from an incorrect
assumption about how these should work.

Having sign extension here is is entirely consistent with the usual way
type promotions are applied. It's the semantic of the __this_cpu_*()
operations that should match the this_cpu_*() operations.

Applying an arbitrarily different rule here doesn't help, and would be a
bug. I am not going to change this.

Mark.