Re: [PATCH v7 5/8] RISC-V: Add fetch and decode helpers to a common file

[Jesse Taube]

On Wed, Sep 30, 2026 at 2:40 AM Himanshu Chauhan
<himanshu.chauhan@xxxxxxxxxxxxxxxx> wrote:
>
> Move get_insn() and __read_insn() from traps_misaligned.c
>
> Add helpers for RISC-V register access and control-flow emulation
>
> Add helpers to retrieve RISC-V register values, calcuate the next
> execution address based on current pc, register states and by evaluating
> conditional branch instructions. the helpers accommodate both compressed
> and standard instructions. Add helper to determine is brach will be taken
> by current pc. It supports register-based jumps, immediate jumps, BEQZ/BNEZ
> conditional branches, and signed and unsigned comparisons for standard
> conditional branch instructions.
>
> Signed-off-by: Himanshu Chauhan <himanshu.chauhan@xxxxxxxxxxxxxxxx>
> Co-developed-by: Jesse Taube <jtaubepe@xxxxxxxxxx>
> ---
>  arch/riscv/include/asm/insn.h        |  15 ++
>  arch/riscv/kernel/traps_misaligned.c |  52 +-----
>  arch/riscv/lib/Makefile              |   1 +
>  arch/riscv/lib/insn.c                | 263 +++++++++++++++++++++++++++
>  4 files changed, 280 insertions(+), 51 deletions(-)
>  create mode 100644 arch/riscv/lib/insn.c
>
> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> index c3005573e8c9..b2125728d4e9 100644
> --- a/arch/riscv/include/asm/insn.h
> +++ b/arch/riscv/include/asm/insn.h
> @@ -227,9 +227,16 @@
>  #define RVC_MASK_C_EBREAK      0xffff
>  #define RVG_MASK_EBREAK                0xffffffff
>  #define RVG_MASK_SRET          0xffffffff
> +#define RVC_MASK_INSN          GENMASK(15, 0)
>
>  #define __INSN_LENGTH_MASK     _UL(0x3)
>  #define __INSN_LENGTH_GE_32    _UL(0x3)
> +
> +static __always_inline bool riscv_insn_is_compressed(u32 code)
> +{
> +       return (code & (__INSN_LENGTH_MASK)) != (__INSN_LENGTH_GE_32);
> +}
> +
>  #define __INSN_OPCODE_MASK     _UL(0x7F)
>  #define __INSN_BRANCH_OPCODE   _UL(RVG_OPCODE_BRANCH)
>
> @@ -600,4 +607,12 @@ 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);
>  }
> +#ifndef __ASSEMBLY__
> +#include <asm/ptrace.h>
> +
> +int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn);
> +int get_insn_safe(struct pt_regs *regs, ulong epc, ulong *r_insn);
> +unsigned long get_next_insn_address(struct pt_regs *regs, ulong insn, ulong pc);
> +#endif /* __ASSEMBLY__ */
> +
>  #endif /* _ASM_RISCV_INSN_H */
> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
> index 6e8ae6c66322..541e6a5b8039 100644
> --- a/arch/riscv/kernel/traps_misaligned.c
> +++ b/arch/riscv/kernel/traps_misaligned.c
> @@ -10,6 +10,7 @@
>  #include <linux/irq.h>
>  #include <linux/stringify.h>
>
> +#include <asm/insn.h>
>  #include <asm/processor.h>
>  #include <asm/ptrace.h>
>  #include <asm/csr.h>
> @@ -129,57 +130,6 @@ static unsigned long get_f32_rs(unsigned long insn, u8 fp_reg_offset,
>  #define GET_F32_RS2C(insn, regs) (get_f32_rs(insn, 2, regs))
>  #define GET_F32_RS2S(insn, regs) (get_f32_rs(RVC_RS2S(insn), 0, regs))
>
> -#define __read_insn(regs, insn, insn_addr, type)       \
> -({                                                     \
> -       int __ret;                                      \
> -                                                       \
> -       if (user_mode(regs)) {                          \
> -               __ret = get_user(insn, (type __user *) insn_addr); \
> -       } else {                                        \
> -               insn = *(type *)insn_addr;              \
> -               __ret = 0;                              \
> -       }                                               \
> -                                                       \
> -       __ret;                                          \
> -})
> -
> -static inline int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn)
> -{
> -       ulong insn = 0;
> -
> -       if (epc & 0x2) {
> -               ulong tmp = 0;
> -
> -               if (__read_insn(regs, insn, epc, u16))
> -                       return -EFAULT;
> -               /* __get_user() uses regular "lw" which sign extend the loaded
> -                * value make sure to clear higher order bits in case we "or" it
> -                * below with the upper 16 bits half.
> -                */
> -               insn &= GENMASK(15, 0);
> -               if ((insn & __INSN_LENGTH_MASK) != __INSN_LENGTH_32) {
> -                       *r_insn = insn;
> -                       return 0;
> -               }
> -               epc += sizeof(u16);
> -               if (__read_insn(regs, tmp, epc, u16))
> -                       return -EFAULT;
> -               *r_insn = (tmp << 16) | insn;
> -
> -               return 0;
> -       } else {
> -               if (__read_insn(regs, insn, epc, u32))
> -                       return -EFAULT;
> -               if ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_32) {
> -                       *r_insn = insn;
> -                       return 0;
> -               }
> -               insn &= GENMASK(15, 0);
> -               *r_insn = insn;
> -
> -               return 0;
> -       }
> -}
>
>  union reg_data {
>         u8 data_bytes[8];
> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> index f668b98970bd..84cdd35afa42 100644
> --- a/arch/riscv/lib/Makefile
> +++ b/arch/riscv/lib/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  lib-y                  += delay.o
> +lib-y                  += insn.o
>  lib-y                  += memcpy.o
>  lib-y                  += memset.o
>  lib-y                  += memmove.o
> diff --git a/arch/riscv/lib/insn.c b/arch/riscv/lib/insn.c
> new file mode 100644
> index 000000000000..361cac7abe10
> --- /dev/null
> +++ b/arch/riscv/lib/insn.c
> @@ -0,0 +1,263 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2026 Qualcomm Technoloies, Inc.
> + */
> +
> +#include <linux/uaccess.h>
> +
> +#include <asm/insn.h>
> +#include <asm/ptrace.h>
> +#include <asm/uaccess.h>
> +
> +/**
> + * __fetch_insn() - Fetch a RISC-V instruction parcel from memory
> + * @regs: Register state used to determine the access context
> + * @insn: Variable receiving the instruction value
> + * @insn_addr: Address from which to read the instruction parcel
> + * @type: Instruction parcel type, either @u16 or @u32
> + *
> + * Fetches an instruction parcel from user or kernel memory, depending on the
> + * execution context indicated by @regs. RISC-V instruction parcels are
> + * stored in little-endian byte order, so the fetched value is converted from
> + * little-endian to the native CPU representation before being assigned to
> + * @insn.
> + *
> + * The conversion is a no-op on little-endian targets and performs the
> + * required byte swap on big-endian targets.
> + *
> + * Return: 0 on success, or a negative error code if reading user memory
> + *         fails.
> + */
> +#define __fetch_insn(regs, insn, insn_addr, type)              \
> +({                                                             \
> +       type __val;                                             \
> +       int __ret;                                              \
> +                                                               \
> +       if (user_mode(regs))                                    \
> +               __ret = get_user(__val,                         \
> +                                (type __user *)(insn_addr));   \
> +       else {                                                  \
> +               __val = *(type *)(insn_addr);                   \
> +               __ret = 0;                                      \
> +       }                                                       \
> +                                                               \
> +       if (!__ret) {                                           \
> +               if (sizeof(type) == sizeof(u16))                        \
> +                       (insn) = le16_to_cpu((__force __le16)__val);    \
> +               else                                                    \
> +                       (insn) = le32_to_cpu((__force __le32)__val);    \
> +       }                                                       \
> +                                                               \
> +       __ret;                                                  \
> +})
> +
> +/**
> + * get_insn() - Fetch and decode a RISC-V instruction
> + * @regs: Register state used for instruction access
> + * @epc: Address of the instruction to fetch
> + * @r_insn: Pointer to store the fetched instruction encoding
> + *
> + * Fetches the instruction at @epc and stores its encoding in @r_insn.
> + * Both standard 32-bit instructions and compressed 16-bit instructions are
> + * supported. If a 32-bit instruction is split across two 16-bit instruction
> + * accesses, the halfwords are combined into a single instruction encoding.
> + *
> + * Return: 0 on success, or %-EFAULT if instruction access fails.
> + */
> +int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn)
> +{
> +       ulong insn, tmp;
> +
> +       if (!(epc & 0x2)) {
> +               if (__fetch_insn(regs, insn, epc, u32))
> +                       return -EFAULT;
> +
> +               if (riscv_insn_is_compressed(insn))
> +                       insn &= RVC_MASK_INSN;
> +
> +               *r_insn = insn;
> +               return 0;
> +       }
> +
> +       if (__fetch_insn(regs, insn, epc, u16))
> +               return -EFAULT;
> +
> +       insn &= RVC_MASK_INSN;
> +       if (riscv_insn_is_compressed(insn)) {
> +               *r_insn = insn;
> +               return 0;
> +       }
> +
> +       if (__fetch_insn(regs, tmp, epc + sizeof(u16), u16))
> +               return -EFAULT;
> +
> +       *r_insn = (tmp << 16) | insn;
> +
> +       return 0;
> +}
> +
> +int get_insn_safe(struct pt_regs *regs, ulong epc, ulong *r_insn)
> +{
> +       int ret;
> +
> +       pagefault_disable();
> +       ret = get_insn(regs, epc, r_insn);
> +       pagefault_enable();
> +
> +       return ret;
> +}
> +
> +/**
> + * riscv_get_reg_value() - Get the value stored the given RISC-V register number
> + * @regs: Register state containing the saved register values
> + * @regno: RISC-V register number
> + *
> + * Returns the value of the RISC-V register identified by @regno. Register
> + * x0 always returns zero, as required by the RISC-V ISA.
> + *
> + * Return: The register value, or zero if @regno is zero.
> + */
> +static unsigned long riscv_get_reg_value(struct pt_regs *regs, unsigned int regno)
> +{
> +       return regno ? regs_get_register(regs, regno * sizeof(unsigned long)) : 0;
> +}
> +
> +/**
> + * get_next_insn_address_compressed() - Calculate the next address for a compressed
> + * RISC-V instruction
> + * @regs: Register state used to evaluate indirect jumps and conditional
> + *        branches
> + * @insn: Compressed RISC-V instruction encoding
> + * @pc: Current program counter
> + *
> + * Determines the address at which execution should continue after processing
> + * the compressed instruction in @insn. Indirect jumps use the value of the
> + * instruction's source register, unconditional jumps use the encoded
> + * immediate, and conditional branches evaluate the relevant register value.
> + *
> + * For a branch that is not taken, or for an unsupported compressed
> + * instruction, the address of the following 16-bit instruction is returned.
> + *
> + * Return: The next instruction address.
> + */
> +static unsigned long get_next_insn_address_compressed(struct pt_regs *regs, u32 insn,
> +                                                     unsigned long pc)
> +{
> +       unsigned int rs1_num;
> +
> +       if (riscv_insn_is_c_jalr(insn) || riscv_insn_is_c_jr(insn)) {
> +               rs1_num = RV_X(insn, RVC_C2_RS1_OPOFF, 5);
Use RVC_EXTRACT_C2_RS1_REG here.
In the other places where it extracts the register it should use
RVC/RV_EXTRACT_*_REG

Otherwise LGTM.

Thanks,
Jesse Taube

> +               return regs_get_register(regs, rs1_num * sizeof(unsigned long));
> +       }
> +
> +       if (riscv_insn_is_c_j(insn) || riscv_insn_is_c_jal(insn))
> +               return RVC_EXTRACT_JTYPE_IMM(insn) + pc;
> +
> +       if (riscv_insn_is_c_beqz(insn)) {
> +               rs1_num = RV_X(insn, RVC_C1_RS1_OPOFF, 3) + 8;
> +               if (!rs1_num || riscv_get_reg_value(regs, rs1_num) == 0)
> +                       return RVC_EXTRACT_BTYPE_IMM(insn) + pc;
> +               return pc + 2;
> +       }
> +
> +       if (riscv_insn_is_c_bnez(insn)) {
> +               rs1_num = RV_X(insn, RVC_C1_RS1_OPOFF, 3) + 8;
> +               if (rs1_num && riscv_get_reg_value(regs, rs1_num) != 0)
> +                       return RVC_EXTRACT_BTYPE_IMM(insn) + pc;
> +               return pc + 2;
> +       }
> +
> +       return pc + 2;
> +}
> +
> +/**
> + * riscv_branch_taken() - Determine whether a RISC-V conditional branch is taken
> + * @regs: Register state used to obtain the source operand values
> + * @insn: RISC-V branch instruction encoding
> + *
> + * Evaluates the branch condition encoded in @insn using the values of its
> + * source registers from @regs. Signed comparisons are used for BLT and BGE,
> + * while unsigned comparisons are used for BLTU and BGEU.
> + *
> + * Return: %true if the branch condition is satisfied, or %false otherwise.
> + *         Unsupported branch instructions also return %false.
> + */
> +static bool riscv_branch_taken(struct pt_regs *regs, u32 insn)
> +{
> +       unsigned int rs1_num = RV_X(insn, RVG_RS1_OPOFF, 5);
> +       unsigned int rs2_num = RV_X(insn, RVG_RS2_OPOFF, 5);
> +       unsigned long rs1_val = riscv_get_reg_value(regs, rs1_num);
> +       unsigned long rs2_val = riscv_get_reg_value(regs, rs2_num);
> +
> +       if (riscv_insn_is_beq(insn))
> +               return rs1_val == rs2_val;
> +       if (riscv_insn_is_bne(insn))
> +               return rs1_val != rs2_val;
> +       if (riscv_insn_is_blt(insn))
> +               return (long)rs1_val < (long)rs2_val;
> +       if (riscv_insn_is_bge(insn))
> +               return (long)rs1_val >= (long)rs2_val;
> +       if (riscv_insn_is_bltu(insn))
> +               return rs1_val < rs2_val;
> +       if (riscv_insn_is_bgeu(insn))
> +               return rs1_val >= rs2_val;
> +
> +       return false;
> +}
> +
> +/**
> +* get_next_insn_address_standard() - Compute the next PC for standard RISC-V
> +* control-flow instructions
> +*
> +* @regs: Register state of the current context.
> +* @insn: Instruction located at @pc.
> +* @pc: Address of the current instruction.
> +*
> +* Determine the address of the next instruction to be executed after @insn.
> +* The function evaluates control-flow instructions whose target cannot be
> +* obtained by simply advancing the program counter:
> +*
> +* - Conditional branches: returns either the branch target or @pc + 4
> +* depending on the branch outcome.
> +* - JAL: returns the jump target encoded in the instruction.
> +* - JALR: returns the computed indirect jump target using the instruction
> +* immediate and the value of the source register.
> +* - SRET: returns @pc, as control transfer is handled by the trap return
> +* mechanism.
> +*
> +* For all other instructions, execution is assumed to continue at the next
> +* sequential 32-bit instruction and @pc + 4 is returned.
> +*
> +* Return: Address of the next instruction to be executed.
> +*/
> +static unsigned long get_next_insn_address_standard(struct pt_regs *regs, u32 insn,
> +                                                   unsigned long pc)
> +{
> +       unsigned int rs1_num;
> +
> +       if ((insn & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE)
> +               return riscv_branch_taken(regs, insn) ?
> +                       RV_EXTRACT_BTYPE_IMM(insn) + pc : pc + 4;
> +
> +       if (riscv_insn_is_jal(insn))
> +               return RV_EXTRACT_JTYPE_IMM(insn) + pc;
> +
> +       if (riscv_insn_is_jalr(insn)) {
> +               rs1_num = RV_X(insn, RVG_RS1_OPOFF, 5);
> +               return RV_EXTRACT_ITYPE_IMM(insn) + riscv_get_reg_value(regs, rs1_num);
> +       }
> +
> +       if (riscv_insn_is_sret(insn))
> +               return pc;
> +
> +       return pc + 4;
> +}
> +
> +/* Calculate the new address for after a step */
> +unsigned long get_next_insn_address(struct pt_regs *regs, ulong insn, ulong pc)
> +{
> +       if (riscv_insn_is_compressed(insn))
> +               return get_next_insn_address_compressed(regs, insn, pc);
> +
> +       return get_next_insn_address_standard(regs, insn, pc);
> +}
> --
> 2.43.0
>