Re: [PATCH 3/3] riscv: add trap and emulation for RDCYCLE

Andrew Jones
2024-09-18T06:46:48Z

On Tue, Sep 17, 2024 at 02:08:53PM GMT, Ben Dooks wrote:
> Add a trap for RDCYCLE and emulate it as RDTIME instruciton.
>
> This is an initial PoC and should probably be made more generic
> way of trapping and dealing with bad instructions
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> arch/riscv/kernel/traps.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index 1c3fab272fd1..51ea28ebf54d 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -167,6 +167,35 @@ DO_ERROR_INFO(do_trap_insn_misaligned,
> DO_ERROR_INFO(do_trap_insn_fault,
> SIGSEGV, SEGV_ACCERR, "instruction access fault");
>
> +#define is_system(__i) (((__i) & 0x7f) == RVG_OPCODE_SYSTEM)

We have riscv_insn_is_system()

> +
> +static bool riscv_try_csr_fixup_user(struct pt_regs *regs, u32 insn)
> +{
> + /* expecting a 4 byte CSR instruction (*/
> + if (unlikely(GET_INSN_LENGTH(insn) != 4))
> + return false;
> +
> + if (is_system(insn)) {
> + u32 csr = insn >> RVG_SYSTEM_CSR_OFF;
> + u32 rd = (insn >> RVG_RD_OPOFF) & RVG_RD_MASK;
> + u32 rs = (insn >> RVG_RS1_OPOFF) & RVG_RS1_MASK;
> + u32 funct3 = (insn >> RV_INSN_FUNCT3_OPOFF) & 0x7;

There are are a few other macros in asm/insn.h that can be applied, such
as RV_EXTRACT_RD_REG(), and more could be added if necessary.

> +
> + if (rs == 0 && funct3 == 2 && csr == CSR_CYCLE) {

We could probably create a riscv_insn_is_csr_read() and RV_EXTRACT_CSR()
for this.

> + u64 val = csr_read(CSR_TIME);
> + /* we've got a RDCCLYE, emulated it with CSR_TIME */
> +
> + printk_ratelimited("PID %d: process using RDCYCLE, emulating with RDTIME\n", current->pid);

If we add current->comm it may be easier to find applications that should
be converted to CSR_TIME.

> +
> + regs_set_register(regs, rd*sizeof(unsigned long), val);

nit: spaces around the '*'

> + regs->epc += 4;
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *regs)
> {
> bool handled;
> @@ -186,6 +215,8 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
> }
>
> handled = riscv_v_first_use_handler(regs, insn);
> + if (!handle)
^handled?

> + handled = riscv_try_csr_fixup_user(regs, insn);
>
> local_irq_disable();
>
> --
> 2.37.2.352.g3c44437643
>
>

Thanks,
drew