Re: [PATCH v2 2/2] riscv: probes: reject probes inside an LR/SC sequence

[Vladimir Kondratiev <Vladimir.Kondratiev@mobileye.com>]

On 11/12/26 09:54, Nam Cao wrote:
> EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe.
> 
> Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> writes:
>> A kprobe or uprobe replaces an instruction with a breakpoint. The trap
>> clears the hart's reservation, so if the breakpoint sits on the lr, on
>> the sc, or on any instruction between them, the sc fails, the loop
>> branches back to the lr and the probe fires again. The sequence never
>> completes.
> 
> Xiaofeng sent a series addressing the same issue:
> https://lore.kernel.org/linux-riscv/20261019134121.1763-1-yuanxiaofeng@eswincomputing.com/#t
> 
> I have only skimmed this patch, but this patch has an important
> difference: riscv_probe_in_lrsc_body(), while the other series
> implements a kunit.
> 
> Perhaps you two should join forces?
> 
> Nam
> 
Ups, I have already sent v3.

To my understanding of the risc-v spec, one can't put a breakpoint
within a valid constrained LR/SC sequence. However if lr is not part of
such a sequence, there;s no restrictions

I'd be glad if someone can comment on this because this is the core
point. All the text below assumes this is a correct interpretation of
the spec.

Within the lr/sc loop, probe shall be rejected at positions from lr to
sc inclusive; but if lr is not bounded to sc, there's no restrictions.
Good example would be __cmpwait() where lr is followed by wrs.nto, in
this function probe can be placed everywhere, including lr itself.

As a result, one can't detect lr/sc loop by searching for preceding lr;
it should instead search for the following sc.

Xiaofeng's patch interprets this different, it searches for lr between
nearest symbol and probe position.

Difference best seen in KUnit test. See this case:

SYM_FUNC_START(test_kprobes_lrsc_gap)
.Llrsc_gap_lr:
	lr.w t0, 0(x0)
	.rept 13
	nop
	.endr
.Llrsc_gap_pos15:
	nop
.Llrsc_gap_pos16:
	nop
	ret
SYM_FUNC_END(test_kprobes_lrsc_gap)

In this fragment, in my understanding, location .Llrsc_gap_pos15 should
be allowed because it is not inside a valid constrained LR/SC sequence
while Xiaofeng's patch would reject it.

Xiaofeng: let's join the forces, See my patch addressing the same issue
here:
https://patch.msgid.link/20261112-probes-in-lrsc-v3-0-a95089852a17@mobileye.com

If we agree on the risc-v spec interpretation, let's stick with my patch
for the [ku]probes itself; and adopt your KUnit stuff for the testing?

Thanks, Vladimir