Re: [PATCH v1 3/4] LoongArch: kprobes: Fix single-stepping instruction slot preparation

[Lisa Robinson <lisa@bytefly.space>]

> In arch_prepare_ss_slot(), the original code directly assigns instructions
> to the buffer using raw memory stores. This approach has two significant
> drawbacks on LoongArch:
> 
> 1. Consistency: It skips the necessary instruction barrier synchronization
>    required by the architecture. Without a local barrier, the instruction
>    fetch unit might not observe the newly prepared instructions in the
>    single-step slot, even on the local CPU.
> 2. Atomicity: Raw memory assignments do not guarantee that the instruction
>    unit sees a complete instruction at all times, which is critical for
>    the integrity of single-step execution.
> 
> Like RISC-V and ARM64, use larch_insn_patch_text() for slot preparation to
> ensure the atomic instruction writes and proper local instruction barrier
> execution.
> 
> Note that global stop_machine synchronization is not required here because
> the single-step slot is executed only after a breakpoint exception, which
> inherently provides a context synchronization event for the CPU to observe
> the new instructions.
> 
> Fixes: 6d4cc40fb5f5 ("LoongArch: Add kprobes support")
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  arch/loongarch/kernel/kprobes.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/loongarch/kernel/kprobes.c b/arch/loongarch/kernel/kprobes.c
> index 04b5b05715cd..8e1b7a87c897 100644
> --- a/arch/loongarch/kernel/kprobes.c
> +++ b/arch/loongarch/kernel/kprobes.c
> @@ -12,8 +12,8 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>  
>  static void arch_prepare_ss_slot(struct kprobe *p)
>  {
> -	p->ainsn.insn[0] = *p->addr;
> -	p->ainsn.insn[1] = KPROBE_SSTEPBP_INSN;
> +	larch_insn_patch_text(p->ainsn.insn, *p->addr);
> +	larch_insn_patch_text(p->ainsn.insn + 1, KPROBE_SSTEPBP_INSN);

This instruction sequence is executed only after arch_arm_kprobe(), so any
instruction hazards are prevented.

--
Lisa

>  	p->ainsn.restore = (unsigned long)p->addr + LOONGARCH_INSN_SIZE;
>  }
>  NOKPROBE_SYMBOL(arch_prepare_ss_slot);
> -- 
> 2.42.0