Re: [RFC PATCH 06/13] arm64: percpu: Add infrastructure for preemptible this_cpu_*() ops

[Vladimir Murzin]

On 7/28/26 13:38, Mark Rutland wrote:
> Currently arm64's this_cpu_*() ops transiently disable preemption in
> order to guarantee that the address generation and memory access(es)
> occur on the same CPU.
> 
> Transiently disabling preemption can be  expensive. When re-enabling
> preemption it is necessary to make a conditional function call to
> preempt_schedule[_notrace]() in order to handle the rare case that the
> task needs to be rescheduled. The potential function call has a number
> of negative effects on code generation (e.g. due to the need to create a
> stack frame and spill registers), and the conditionality can result in
> poor code generation and/or poor branch prediction.
> 
> This patch adds infrastructure for a scheme where this_cpu_*() ops do
> not need to transiently disable preemption, avoiding the negative
> impacts described above.
> 
> Each operation registers a critical section during which the exception
> return code will adjust the offset and addresses if preemption occurs
> mid-sequence. The critical section is registered/unregistered with a
> small prologue and epilogue which encodes three distinct GPRRs (<pcp>,
> <off>, <addr>) into a new thread_info::pcp_gprs field:
> 
>          // Prologue. Enable fixups for <off> and <addr>.
>          mrs	<tsk>, sp_el0
>          mov	<tmp>, #__VAL_PCPU_GPRS(<pcp>, <off>, <addr>)
>          strh	<tmp>, [<tsk>, #TSK_TI_PCPU_GPRS]
> 
>          // Generate cpu-specific address
>          mrs	<off>, TPIDR_ELx
>          add	<addr>, <pcp>, <off>
> 
>          // Perform access sequence
>          ldr	<val>, [<addr>]
> 
>          // Epilogue. Disable fixups
>          strh	wzr, [<tsk>, #TSK_TI_PCPU_GPRS]
> 
> If an exception is taken from within the critical section, the exception
> return code will adjust <off> to be the current CPU's offset, and will
> adjust <addr> to be (<pcp> + <off>). Distinct registers are used for
> <pcp>, <off>, and <addr>, so that the fixup can be applied safely at any
> point during the critical section.
> 
> To ensure that this_cpu_*() operations within exception handlers work
> correctly and do not corrupt state, thread_info::pcpu_gprs is saved
> into a new pt_regs::pcpu_gprs field upon exception entry, and restored
> upon exception return.
> 
> Looking at a simple this_cpu_operation:
> 
> | void outline_this_cpu_add_u64(u64 __percpu *p, u64 v)
> | {
> | 	this_cpu_add(*p, v);
> | }
> 
> Atop v7.2-rc4, with GCC 15.2.0 and defconfig, this is compiled as:
> 
> | <outline_this_cpu_add_u64>:
> |        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:     ldxr    x5, [x0]
> |        add     x5, x5, x1
> |        stxr    w4, x5, [x0]
> |        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
> 
> With the scheme added in this patch, this can be compiled as:
> 
> | <outline_this_cpu_add_u64>:
> |        mrs     x2, sp_el0
> |        mov     x4, #0xc80
> |        strh    w4, [x2, #20]
> |        mrs     x4, tpidr_el1
> |        add     x3, x0, x4
> | 1:     ldxr    x6, [x3]
> |        add     x6, x6, x1
> |        stxr    w5, x6, [x3]
> |        cbnz    w5, 1b
> |        strh    wzr, [x2, #20]
> |        ret
> 
> TODO: Save/restore the PCPU GPRs in __sdei_asm_handler(). This will
> require some mechanical rework to the __sdei_asm_handler() assembly.
> 
> Signed-off-by: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: Ada Couprie Diaz <ada.coupriediaz@xxxxxxx>
> Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
> Cc: Marc Zyngier <maz@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Vladimir Murzin <vladimir.murzin@xxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Yang Shi <yang@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>  arch/arm64/include/asm/percpu.h      | 43 ++++++++++++++++++++++++++++
>  arch/arm64/include/asm/ptrace.h      |  5 ++++
>  arch/arm64/include/asm/thread_info.h |  1 +
>  arch/arm64/kernel/asm-offsets.c      |  2 ++
>  arch/arm64/kernel/entry-common.c     | 38 ++++++++++++++++++++++++
>  arch/arm64/kernel/entry.S            | 13 +++++++++
>  6 files changed, 102 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/percpu.h b/arch/arm64/include/asm/percpu.h
> index 98823c97d534c..0871dcc41d759 100644
> --- a/arch/arm64/include/asm/percpu.h
> +++ b/arch/arm64/include/asm/percpu.h
> @@ -5,10 +5,12 @@
>  #ifndef __ASM_PERCPU_H
>  #define __ASM_PERCPU_H
>  
> +#include <linux/bits.h>
>  #include <linux/preempt.h>
>  
>  #include <asm/alternative.h>
>  #include <asm/cmpxchg.h>
> +#include <asm/gpr-num.h>
>  #include <asm/stack_pointer.h>
>  #include <asm/sysreg.h>
>  
> @@ -51,6 +53,47 @@ static inline unsigned long __kern_my_cpu_offset(void)
>  	return off;
>  }
>  
> +#define PCPU_GPR_PCP			GENMASK(4, 0)
> +#define PCPU_GPR_OFF			GENMASK(9, 5)
> +#define PCPU_GPR_ADDR			GENMASK(14, 10)
> +
> +#define __VAL_PCPU_GPRS(pcp, off, addr)				\
> +	"("							\
> +		"(.L__gpr_num_" pcp  " << 0) | "		\
> +		"(.L__gpr_num_" off  " << 5) | "		\
> +		"(.L__gpr_num_" addr " << 10)"			\
> +	")"

Later in the patch, there is a comment stating that these registers
are not expected to overlap. I can also see that early-clobber
constraints are applied to the output registers later in the patch
series, so the code is correct.

However, would it be possible to add assertions that detect register
overlap, perhaps something like:

".if (.L__gpr_num_" pcp "== .L__gpr_num_" off ") ||"	\
"    (.L__gpr_num_" pcp "== .L__gpr_num_" addr ") ||"	\
"    (.L__gpr_num_" off "== .L__gpr_num_" addr")"	\
".error "inline asm registers overlap"			\
".endif"						\

or any other (better) way.

Such assertions would serve both as documentation and as a strong
guarantee that the registers are distinct.


Cheers
Vladimir