Re: [RFC PATCH] riscv: uprobe: Relax SYSTEM instruction validation

[daichengrong]

On 7/20/26 16:00, daichengrong wrote:
> RISC-V uprobe currently rejects all SYSTEM instructions during
> instruction validation.
> 
> This policy is conservative, but it also rejects CSR instructions that
> only read architectural state, preventing uprobe from instrumenting
> user applications containing such instructions.
> 
> This RFC proposes refining the SYSTEM instruction validation by
> distinguishing read-only CSR instructions from CSR operations that may
> modify processor state. Read-only CSR instructions are allowed, while
> CSR write operations and other SYSTEM instructions continue to be
> rejected.
> 
> The RISC-V ISA permits implementation-defined side effects on some CSR
> reads. Feedback is welcome on whether instruction-level validation is
> sufficient, or whether additional validation based on CSR properties
> would be more appropriate.
> 
> Signed-off-by: daichengrong <daichengrong@xxxxxxxxxxx>
> ---
>  arch/riscv/include/asm/insn.h          | 15 +++++++++++++++
>  arch/riscv/kernel/probes/decode-insn.c |  2 +-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> index c3005573e8c9..08c6be4609ea 100644
> --- a/arch/riscv/include/asm/insn.h
> +++ b/arch/riscv/include/asm/insn.h
> @@ -600,4 +600,19 @@ static inline void riscv_insn_insert_utype_itype_imm(u32 *utype_insn, u32 *itype
>  	*utype_insn |= (imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1);
>  	*itype_insn |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF);
>  }
> +
> +static __always_inline bool riscv_insn_is_csr_read(u32 code)
> +{
> +	return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_SYSTEM &&
> +			(GET_FUNCT3(code) == GET_FUNCT3(INSN_MATCH_CSRRS)
> +				 || GET_FUNCT3(code) == GET_FUNCT3(INSN_MATCH_CSRRC)
> +				 || GET_FUNCT3(code) == GET_FUNCT3(INSN_MATCH_CSRRSI)
> +				 || GET_FUNCT3(code) == GET_FUNCT3(INSN_MATCH_CSRRCI)) &&
> +			 RV_EXTRACT_RS1_REG(code) == 0;
> +}
> +
> +static __always_inline bool riscv_insn_is_system_except_csr_read(u32 code)
> +{
> +	return riscv_insn_is_system(code) && !riscv_insn_is_csr_read(code);
> +}
>  #endif /* _ASM_RISCV_INSN_H */
> diff --git a/arch/riscv/kernel/probes/decode-insn.c b/arch/riscv/kernel/probes/decode-insn.c
> index 65d9590bfb9f..4a6a33b95159 100644
> --- a/arch/riscv/kernel/probes/decode-insn.c
> +++ b/arch/riscv/kernel/probes/decode-insn.c
> @@ -21,7 +21,7 @@ riscv_probe_decode_insn(probe_opcode_t *addr, struct arch_probe_insn *api)
>  	/*
>  	 * Reject instructions list:
>  	 */
> -	RISCV_INSN_REJECTED(system,		insn);
> +	RISCV_INSN_REJECTED(system_except_csr_read,		insn);
>  	RISCV_INSN_REJECTED(fence,		insn);
>  
>  	/*

PING.

I'd like to add some motivation for this change.

Direct CSR accesses have traditionally been relatively uncommon in ordinary
userspace applications. However, as more RISC-V architectural extensions are
used by userspace software, reading architectural state through CSRs is
becoming more relevant.

The Vector extension is one example. Vector-enabled applications, runtime
libraries, and compiler-generated code may contain CSR reads as part of
querying or managing architectural state. Similar cases may also appear with
other extensions that expose useful state to userspace.

Therefore, rejecting all CSR instructions in uprobe validation can prevent
instrumentation of otherwise valid userspace code even when the instruction
only observes architectural state and does not modify it.

The intent of this RFC is to distinguish such read-only CSR accesses from
operations that modify processor state. CSR writes and other SYSTEM
instructions would remain rejected.

There is still the question of CSR read side effects permitted by the ISA.
Feedback would be appreciated on whether checking the instruction semantics
is sufficient here, or whether we should additionally restrict the set of
CSRs that uprobes may accept.

Thanks,
Chengrong