[PATCH v2 0/2] riscv: kprobes: simulate nop and c.nop instructions

[PATCH v2 0/2] riscv: kprobes: simulate nop and c.nop instructions

[Xiaofeng Yuan]

nop and c.nop have no architectural effect, so handling them through
an out-of-line instruction slot and single-stepping is pure overhead.
This series simulates them directly in the breakpoint handler by
advancing the program counter, following the approach already used on
arm64, and extends the RISC-V kprobes KUnit test to cover both paths.

riscv_probe_decode_insn() is shared by kprobes and uprobes, so the
simulation also applies to uprobes placed on nop instructions. This is
relevant for USDT probe sites in user-space binaries, which are
nop/c.nop instructions on RISC-V, the same use case that motivated the
arm64 implementation. In kernel text, nops are also found at ftrace
mcount call sites and disabled jump_label sites.

Measured on QEMU (RISC-V virt, emulated), this reduces the per-hit
cost of a kprobe on a nop by roughly 40% compared to single-stepping
through the out-of-line slot.

Changes since v1:
- patch 1: add the above use-case justification and benchmark result
  to the commit message (per Nam Cao's review).
- patch 2: simplify the test functions to load KPROBE_TEST_MAGIC
  directly instead of splitting it into LOWER/UPPER halves (per Nam
  Cao's review).

Verified by cross-compiling for RISC-V and running the kprobes KUnit
test in QEMU (ok 1 kprobes_riscv).

Xiaofeng Yuan (2):
  riscv: kprobes: simulate nop and c.nop instructions
  riscv: kprobes: add nop and c.nop to the KUnit test

 arch/riscv/include/asm/insn.h                 | 11 +++++++++
 arch/riscv/kernel/probes/decode-insn.c        |  6 +++++
 arch/riscv/kernel/probes/simulate-insn.c      | 14 +++++++++++
 arch/riscv/kernel/probes/simulate-insn.h      |  2 ++
 .../kernel/tests/kprobes/test-kprobes-asm.S   | 23 +++++++++++++++++++
 5 files changed, 56 insertions(+)

-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 1/2] riscv: kprobes: simulate nop and c.nop instructions

[Xiaofeng Yuan]

nop and c.nop have no architectural effect, so allocating an
out-of-line instruction slot and single-stepping through them is
pure overhead. Simulate them directly in the breakpoint handler
by advancing the program counter, following the approach already
used on arm64.

This avoids slot allocation, text patching, IRQ flag save/restore
and the single-step exception for these instructions.

riscv_probe_decode_insn() is shared by kprobes and uprobes, so the
simulation also applies to uprobes placed on nop instructions. This
is relevant for USDT probe sites in user-space binaries, which are
nop/c.nop instructions, the same use case that motivated the arm64
implementation. In kernel text, nops are also found at ftrace
mcount call sites and disabled jump_label sites.

Measured on QEMU (RISC-V virt, emulated), this reduces the per-hit
cost of a kprobe on a nop by roughly 40% compared to single-stepping
through the out-of-line slot.

Compile tested on RISC-V, and verified with the RISC-V kprobes
KUnit test which now covers nop and c.nop.

Signed-off-by: Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com>
---
 arch/riscv/include/asm/insn.h            | 11 +++++++++++
 arch/riscv/kernel/probes/decode-insn.c   |  6 ++++++
 arch/riscv/kernel/probes/simulate-insn.c | 14 ++++++++++++++
 arch/riscv/kernel/probes/simulate-insn.h |  2 ++
 4 files changed, 33 insertions(+)

diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
index c3005573e8..5b3944a5fa 100644
--- a/arch/riscv/include/asm/insn.h
+++ b/arch/riscv/include/asm/insn.h
@@ -228,6 +228,15 @@
 #define RVG_MASK_EBREAK		0xffffffff
 #define RVG_MASK_SRET		0xffffffff
 
+/*
+ * NOP and C.NOP have no variable fields, so all bits must match.
+ * NOP is encoded as ADDI x0, x0, 0 (0x00000013), C.NOP as C.ADDI x0, 0 (0x0001).
+ */
+#define RVG_MATCH_NOP		0x00000013
+#define RVG_MASK_NOP		0xffffffff
+#define RVC_MATCH_C_NOP		0x0001
+#define RVC_MASK_C_NOP		0xffff
+
 #define __INSN_LENGTH_MASK	_UL(0x3)
 #define __INSN_LENGTH_GE_32	_UL(0x3)
 #define __INSN_OPCODE_MASK	_UL(0x7F)
@@ -262,6 +271,8 @@ __RISCV_INSN_FUNCS(c_ebreak, RVC_MASK_C_EBREAK, RVC_MATCH_C_EBREAK)
 __RISCV_INSN_FUNCS(ebreak, RVG_MASK_EBREAK, RVG_MATCH_EBREAK)
 __RISCV_INSN_FUNCS(sret, RVG_MASK_SRET, RVG_MATCH_SRET)
 __RISCV_INSN_FUNCS(fence, RVG_MASK_FENCE, RVG_MATCH_FENCE);
+__RISCV_INSN_FUNCS(nop, RVG_MASK_NOP, RVG_MATCH_NOP)
+__RISCV_INSN_FUNCS(c_nop, RVC_MASK_C_NOP, RVC_MATCH_C_NOP)
 
 /* special case to catch _any_ system instruction */
 static __always_inline bool riscv_insn_is_system(u32 code)
diff --git a/arch/riscv/kernel/probes/decode-insn.c b/arch/riscv/kernel/probes/decode-insn.c
index 65d9590bfb..d28408f0b7 100644
--- a/arch/riscv/kernel/probes/decode-insn.c
+++ b/arch/riscv/kernel/probes/decode-insn.c
@@ -44,5 +44,11 @@ riscv_probe_decode_insn(probe_opcode_t *addr, struct arch_probe_insn *api)
 	RISCV_INSN_SET_SIMULATE(auipc,		insn);
 	RISCV_INSN_SET_SIMULATE(branch,		insn);
 
+	/* Simulate NOP for better performance */
+	RISCV_INSN_SET_SIMULATE(nop,		insn);
+#ifdef CONFIG_RISCV_ISA_C
+	RISCV_INSN_SET_SIMULATE(c_nop,		insn);
+#endif
+
 	return INSN_GOOD;
 }
diff --git a/arch/riscv/kernel/probes/simulate-insn.c b/arch/riscv/kernel/probes/simulate-insn.c
index fa581590c1..3b22d3ad53 100644
--- a/arch/riscv/kernel/probes/simulate-insn.c
+++ b/arch/riscv/kernel/probes/simulate-insn.c
@@ -237,3 +237,17 @@ bool __kprobes simulate_c_beqz(u32 opcode, unsigned long addr, struct pt_regs *r
 {
 	return simulate_c_bnez_beqz(opcode, addr, regs, false);
 }
+
+bool __kprobes simulate_nop(u32 opcode, unsigned long addr, struct pt_regs *regs)
+{
+	instruction_pointer_set(regs, addr + 4);
+
+	return true;
+}
+
+bool __kprobes simulate_c_nop(u32 opcode, unsigned long addr, struct pt_regs *regs)
+{
+	instruction_pointer_set(regs, addr + 2);
+
+	return true;
+}
diff --git a/arch/riscv/kernel/probes/simulate-insn.h b/arch/riscv/kernel/probes/simulate-insn.h
index 44ebbc444d..7e0936613f 100644
--- a/arch/riscv/kernel/probes/simulate-insn.h
+++ b/arch/riscv/kernel/probes/simulate-insn.h
@@ -29,5 +29,7 @@ bool simulate_c_jr(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_jalr(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_bnez(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_beqz(u32 opcode, unsigned long addr, struct pt_regs *regs);
+bool simulate_nop(u32 opcode, unsigned long addr, struct pt_regs *regs);
+bool simulate_c_nop(u32 opcode, unsigned long addr, struct pt_regs *regs);
 
 #endif /* _RISCV_KERNEL_PROBES_SIMULATE_INSN_H */
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 2/2] riscv: kprobes: add nop and c.nop to the KUnit test

[Xiaofeng Yuan]

Extend the RISC-V kprobes KUnit test with test_kprobes_nop and
test_kprobes_c_nop, covering both the 32-bit nop and the
compressed c.nop simulation paths.

Signed-off-by: Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com>
---
 .../kernel/tests/kprobes/test-kprobes-asm.S   | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S
index f16deee9e0..06a9cb35c9 100644
--- a/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S
+++ b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S
@@ -181,6 +181,25 @@ SYM_FUNC_END(test_kprobes_c_bnez)
 
 #endif /* CONFIG_RISCV_ISA_C */
 
+SYM_FUNC_START(test_kprobes_nop)
+	.option push
+	.option norvc
+test_kprobes_nop_addr:
+	nop
+	.option pop
+	li a0, KPROBE_TEST_MAGIC
+	ret
+SYM_FUNC_END(test_kprobes_nop)
+
+#ifdef CONFIG_RISCV_ISA_C
+SYM_FUNC_START(test_kprobes_c_nop)
+test_kprobes_c_nop_addr:
+	c.nop
+	li a0, KPROBE_TEST_MAGIC
+	ret
+SYM_FUNC_END(test_kprobes_c_nop)
+#endif /* CONFIG_RISCV_ISA_C */
+
 .section .rodata
 SYM_DATA_START(test_kprobes_addresses)
 	RISCV_PTR test_kprobes_add_addr1
@@ -197,7 +216,9 @@ SYM_DATA_START(test_kprobes_addresses)
 	RISCV_PTR test_kprobes_branch_addr6
 	RISCV_PTR test_kprobes_branch_addr7
 	RISCV_PTR test_kprobes_branch_addr8
+	RISCV_PTR test_kprobes_nop_addr
 #ifdef CONFIG_RISCV_ISA_C
+	RISCV_PTR test_kprobes_c_nop_addr
 	RISCV_PTR test_kprobes_branch_c_j_addr1
 	RISCV_PTR test_kprobes_branch_c_j_addr2
 	RISCV_PTR test_kprobes_c_jr_addr1
@@ -220,7 +241,9 @@ SYM_DATA_START(test_kprobes_functions)
 	RISCV_PTR test_kprobes_jalr
 	RISCV_PTR test_kprobes_auipc
 	RISCV_PTR test_kprobes_branch
+	RISCV_PTR test_kprobes_nop
 #ifdef CONFIG_RISCV_ISA_C
+	RISCV_PTR test_kprobes_c_nop
 	RISCV_PTR test_kprobes_c_j
 	RISCV_PTR test_kprobes_c_jr
 	RISCV_PTR test_kprobes_c_jalr
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 1/2] riscv: kprobes: simulate nop and c.nop instructions

[Nam Cao]

Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> writes:
> nop and c.nop have no architectural effect, so allocating an
> out-of-line instruction slot and single-stepping through them is
> pure overhead. Simulate them directly in the breakpoint handler
> by advancing the program counter, following the approach already
> used on arm64.

That is not the reason why arm64 simulate nop. And how is single
stepping slower than simulating?

> This avoids slot allocation, text patching, IRQ flag save/restore
> and the single-step exception for these instructions.
>
> riscv_probe_decode_insn() is shared by kprobes and uprobes, so the
> simulation also applies to uprobes placed on nop instructions. This
> is relevant for USDT probe sites in user-space binaries, which are
> nop/c.nop instructions, the same use case that motivated the arm64
> implementation.

I can only find https://github.com/chrisa/libusdt, which does not have
riscv support. Which USDT are you referring to?

> In kernel text, nops are also found at ftrace mcount call sites and
> disabled jump_label sites.

Not sure about mcount, but isn't installing kprobe on jump labels forbidden?

> Measured on QEMU (RISC-V virt, emulated), this reduces the per-hit
> cost of a kprobe on a nop by roughly 40% compared to single-stepping
> through the out-of-line slot.

Reading the arm64's commit, simulating nop should only improve uprobe,
not kprobe; or am I confused somewhere?

Nam

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: Re: [PATCH v2 1/2] riscv: kprobes: simulate nop and c.nop instructions

[袁晓峰]

From: Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com>
To: Nam Cao <namcao@linutronix.de>
Cc: Paul Walmsley <pjw@kernel.org>, Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>, Nam Cao <namcaov@gmail.com>,
linux-riscv@lists.infradead.org
Subject: Re: [PATCH v2 1/2] riscv: kprobes: simulate nop and c.nop instructions
In-Reply-To: <87wltdt4md.fsf@yellow.woof>


Hi Nam,


Thanks for the detailed review. You were right on several points; let
me take them in order.


Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> writes:
> > nop and c.nop have no architectural effect, so allocating an
> [..]
> > pure overhead. [...] following the approach already
> > used on arm64.
>
> That is not the reason why arm64 simulate nop. And how is single
> stepping slower than simulating?


You are right about the first part: I should not have paraphrased the
arm64 motivation.  ac4ad5c09b34 ("arm64: insn: Simulate nop
instruction for better uprobe performance") was motivated by Andrii's
uprobe benchmarks, where a probed nop was about 2x slower than an
already-emulated instruction.


On the second question, concretely what the extra cost is on riscv:
the "single-stepping" here is done by the XOL slot -- the breakpoint
trap runs the handler and redirects execution to a copy of the
probed instruction with another ebreak appended after it, which
traps back into the kernel a second time just to complete the step.
So each hit of a probed nop costs two exception round-trips (full
pt_regs + sret each) for the slot path versus one for simulation.
For uprobes it is worse still: the slot runs in user mode, so the
re-trap is a full kernel<->userspace round-trip.  I measured
this on QEMU (RISC-V virt): a ftrace uprobe event on a
USDT-style nop site reduces the per-hit cost by roughly 80
percent compared to the XOL path; a kprobe on a nop by roughly
40 percent.  (arm64 commit ac4ad5c09b34 measured ~2x on real
hardware for the same class of change.)  v3 carries these
measurements instead of the arm64 rationale I mis-stated.


> > [...] This
> > is relevant for USDT probe sites in user-space binaries, which are
> [...]
>
> I can only find https://github.com/chrisa/libusdt, which does not have
> riscv support. Which USDT are you referring to?


Sorry for the vague shorthand -- by USDT I mean SystemTap-style SDT
probes (the .note.stapsdt format parsed by libbpf and used by tools
like bpftrace).  On riscv a probe site is an SDT note whose recorded
Location is a plain nop by construction.  In-tree evidence:


- tools/testing/selftests/bpf/sdt.h emits the location as
  "990: _SDT_NOP"; _SDT_NOP is specialized only for ia64/s390
  ("nop"/"nop 0"), so riscv takes the generic "nop" path;
- libbpf documents the model: "USDT call is actually not a function
  call, but is instead replaced by a single NOP instruction ... [the]
  NOP instruction that kernel can replace with an interrupt
  instruction" (tools/lib/bpf/usdt.c), and it already has riscv
  specific note/argument parsing (the register map in
  tools/lib/bpf/usdt.c);
- the bpf selftests USDT provider (urandom_read.c: STAP_PROBE1/3)
  has riscv-specific build support
  (tools/testing/selftests/bpf/Makefile);
- and the arm64 series ac4ad5c09b34 states it itself: "Typicall uprobe
  is installed on 'nop' for USDT".


To make it concrete I cross-compiled a provider with the in-tree
header (riscv64-linux-musl-gcc -O2 -static):


  $ readelf -n usdt_probe | grep -A4 stapsdt
  [...]
      Provider: "bench"
      Name: "nop_site"
      Location: 0x00000000000006ba, Base: 0x0000000000001052
  $ objdump -d usdt_probe | grep -A2 '<probe_site>:'
  00000000000006ba <probe_site>:
    6ba:  0001      nop          (rv64gc: compressed c.nop)
    6bc:  8082      ret


and with -march=rv64imafd (no RVC):


    6c8:  00000013  nop          (4-byte nop)


so this is the ABI-defined instruction at every riscv USDT site --
exactly the two instructions this patch simulates.


To be honest about scope: I have not verified how widely USDT is
deployed on riscv workloads today.  My claim is the narrower one:
libbpf parses riscv SDT notes, the bpf selftests' USDT provider has
riscv-specific build handling, and the sdt.h macro body emits the
note location as a plain nop/c.nop by construction -- so USDT is the
intended consumer of probed nops on riscv, the same model the arm64
series describes: "Typicall uprobe is installed on 'nop' for USDT".


> > In kernel text, nops are also found at ftrace
> > mcount call sites and disabled jump_label sites.
>
> Not sure about mcount, but isn't installing kprobe on jump labels
> forbidden?


You are right, and I dropped both claims in v3.  register_kprobe()
rejects jump_label text sites via jump_label_text_reserved()
(kernel/kprobes.c), which covers the site regardless of whether it
currently holds a nop or a JAL (and static_call sites likewise), so
those nops are not natural probe candidates at all.  The mcount claim
was misleading even where it is allowed: on riscv the function entry
instruction is the auipc of the mcount pair; the nop is only at +4.


> > Measured on QEMU [...]
> > roughly a 2x [...]


> Reading the arm64's commit, simulating nop should only improve uprobe,
> not kprobe; or am I confused somewhere?


Not confused about the motivation -- on arm64 the benchmark and the
title are uprobe-specific, and the common real-world producer of
probed nops is also uprobes (USDT sites), as above.  The cost model
just doesn't stop at kprobes on our side: riscv_probe_decode_insn()
is shared by arch_prepare_kprobe() and arch_uprobe_analyze_insn(),
and the kprobe path is likewise a slot-replay-with-re-trap (second
breakpoint round-trip), so simulating the nop removes one exception
round-trip per hit of a probed nop for kprobes too.  But your
framing is fair: the natural producer of this traffic is uprobes
on USDT sites, so v3 leads with the uprobe measurement (~50us ->
~9us per hit, QEMU) and keeps the kprobe reduction (~40 percent)
as the secondary number for the shared decode path.


v3 is out with these corrections; the diff itself is unchanged.


Best regards,
Xiaofeng Yuan
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv