Re: [PATCH v9 1/3] riscv: implement this_cpu operations

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]

> riscv: implement this_cpu operations

TL;DR: this_cpu_write_N casts val without parentheses, so expressions like
"zero - 1" or "a < 0" may store different values than the generic code on
RISC-V; no in-tree caller found that triggers it.

> diff --git a/arch/riscv/include/asm/percpu.h b/arch/riscv/include/asm/percpu.h
> new file mode 100644
> --- /dev/null
> +++ b/arch/riscv/include/asm/percpu.h

[ ... ]

> +#define this_cpu_write_1(pcp, val)	_pcp_protect(__percpu_write_8, pcp, (unsigned long)val)
> +#define this_cpu_write_2(pcp, val)	_pcp_protect(__percpu_write_16, pcp, (unsigned long)val)
> +#define this_cpu_write_4(pcp, val)	_pcp_protect(__percpu_write_32, pcp, (unsigned long)val)
> +
> +#ifdef CONFIG_64BIT
> +#define this_cpu_write_8(pcp, val)	_pcp_protect(__percpu_write_64, pcp, (unsigned long)val)
> +#endif

Should these casts be (unsigned long)(val)?  this_cpu_write() passes val
through unparenthesized, so the cast binds only to the first operand of an
expression, which changes the stored value compared to the generic
implementation:

  u64 pcp; u32 zero = 0;
  this_cpu_write(pcp, zero - 1);
    generic:  0x00000000ffffffff
    here:     (unsigned long)zero - 1 = 0xffffffffffffffff

  int a = -1;
  this_cpu_write(int_pcp, a < 0);
    generic:  1
    here:     (unsigned long)a < 0, which is always 0

This is the same problem arm64 fixed in 885bff055a0f ("arm64: percpu: Fix
this_cpu_write() casting").  I did not find an in-tree caller that
currently triggers it, but a future caller passing a binary expression
would get a different value on RISC-V only.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/42475504268