Re: [PATCH v3 3/4] openrisc: Add unit tests for KProbes on branch instructions

[Stafford Horne <shorne@gmail.com>]

On Fri, Nov 13, 2026 at 10:45:03PM +0530, Sahil wrote:
> On 11/8/26 1:33 PM, Stafford Horne wrote:
> > On Tue, Oct 27, 2026 at 01:42:39AM +0530, Sahil Siddiq wrote:
> > > Tests already exist lib/tests to verify the generic functionality of
> > > KProbes. Add unit tests to verify that KProbes work on instructions
> > > that require simulation in the OpenRISC ISA, and that the corresponding
> > > delay slots are handled correctly.
> > > 
> > > Signed-off-by: Sahil Siddiq <sahilcdq0@gmail.com>
> > > ---
> > > Changes v2 -> v3:
> > > - arch/openrisc/kernel/tests/test_kprobes_asm.S:
> > >    - (test_kprobes_*): Set value in r11 register to 0 prior to running tests
> > >    - (test_kprobes_adrp):
> > >      Place function at page-aligned address
> > >      Use PAGE_SIZE macro instead of magic number
> > >      Move value in r13 to r11 before exiting
> > > 
> > > Changes v1 -> v2: New commit
> > > 
> > >   arch/openrisc/Kconfig.debug                   |  30 +++-
> > >   arch/openrisc/kernel/Makefile                 |   1 +
> > >   arch/openrisc/kernel/tests/Makefile           |   3 +
> > >   arch/openrisc/kernel/tests/test_kprobes.c     |  59 +++++++
> > >   arch/openrisc/kernel/tests/test_kprobes.h     |  24 +++
> > >   arch/openrisc/kernel/tests/test_kprobes_asm.S | 157 ++++++++++++++++++
> > >   6 files changed, 273 insertions(+), 1 deletion(-)
> > >   create mode 100644 arch/openrisc/kernel/tests/Makefile
> > >   create mode 100644 arch/openrisc/kernel/tests/test_kprobes.c
> > >   create mode 100644 arch/openrisc/kernel/tests/test_kprobes.h
> > >   create mode 100644 arch/openrisc/kernel/tests/test_kprobes_asm.S
> > > 
> > > [...]
> > > diff --git a/arch/openrisc/kernel/tests/test_kprobes_asm.S b/arch/openrisc/kernel/tests/test_kprobes_asm.S
> > > new file mode 100644
> > > index 000000000000..bc87a5fb00d5
> > > --- /dev/null
> > > +++ b/arch/openrisc/kernel/tests/test_kprobes_asm.S
> > > @@ -0,0 +1,157 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +/*
> > > + * KProbes unit tests for OpenRISC
> > > + *
> > > + * Unit tests borrowed liberally from similar work of RISC-V. All
> > > + * original copyrights apply as per the original source declaration.
> > > + */
> > > +
> > > +#include <linux/linkage.h>
> > > +#include <asm/asm.h>
> > > +#include <asm/page.h>
> > > +
> > > +#include "test_kprobes.h"
> > > +
> > > +SYM_FUNC_START(test_kprobes_j)
> > > +test_kprobes_j_addr1:
> > > +	l.addi	r11,r0,0
> > > +	l.j	2f
> > > +	l.nop
> > 
> > Just a tiny comment here about indenting the delay slots.
> 
> Got it, I'll indent this.
> 
> > You could also use the delayslots here, for example:
> > 
> > 	l.j	2f
> > 	 l.addi	r11,r0,0
> 
> I initially thought of that too. But then I thought by keeping l.addi
> before l.j, I would also be able to test this for the case when
> there are no delay slots. I can reshuffle the instructions if that
> test isn't required. Please let me know what your thoughts are.

I don't think it matters either way since these are tests.  But, it would be good
to note that in a comment at the top of the files where you want to maintain
that convention.

However, thinking a bit further that may be hard to enforce down the line just
for a handful of files.  We should assume the kernel code uses delayslots.

-Stafford

> > > +1:	l.ori	r13,r13,KPROBE_TEST_MAGIC_LOWER
> > > +	l.addi	r11,r13,0
> > > +	l.jr	r9
> > > +	l.nop
> > > +2:	l.movhi	r13,KPROBE_TEST_MAGIC_UPPER
> > > +test_kprobes_j_addr2:
> > > +	l.j	1b
> > > +	l.nop
> > > +SYM_FUNC_END(test_kprobes_j)
> > > [...]
> > > -- 
> > > 2.55.0
> > > 
> 
> Regards,
> Sahil