Re: [PATCH v2 08/16] riscv: kvm: Use generated instruction headers for csr code

Charlie Jenkins <[email protected]>
Newsgroups org.infradead.lists.kvm-riscv,org.infradead.lists.linux-riscv,org.kernel.vger.kvm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest
Message-ID <akNfjsbVVcPGNg5a@blinky>
On Mon, Jun 29, 2026 at 04:25:53PM -0400, Jesse T wrote:
> On Mon, Jun 22, 2026 at 1:08 AM Charlie Jenkins via B4 Relay
> <[email protected]> wrote:
> >
> > From: Charlie Jenkins <[email protected]>
> >
> > Migrate the csr parsing code to use the generated instruction headers
> > instead of the hand-written instruction composition functions.
> >
> > Signed-off-by: Charlie Jenkins <[email protected]>
> >
> > ---
> >
> > This is a simple transformation that I have again validated through
> > brute force.
> > ---
> >  arch/riscv/kvm/vcpu_insn.c | 47 +++++++++++++++++++++++-----------------------
> >  1 file changed, 24 insertions(+), 23 deletions(-)
> >
> > diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
> > index f09f9251d1f0..8ccf6ec722f0 100644
> > --- a/arch/riscv/kvm/vcpu_insn.c
> > +++ b/arch/riscv/kvm/vcpu_insn.c
> > @@ -146,43 +146,44 @@ int kvm_riscv_vcpu_csr_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
> >
> >  static int csr_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, ulong insn)
> >  {
> > +       #define GET_REG(_rd) (*((unsigned long *)(&vcpu->arch.guest_context) + _rd))
> > +
> 
> IMO it would be better to make this a static inline function, otherwise.

Yeah that's fair, I'll make that change!

> 
> Reviewed-by: Jesse Taube <[email protected]>
> 
> >         int i, rc = KVM_INSN_ILLEGAL_TRAP;
> > -       unsigned int csr_num = insn >> SH_RS2;
> > -       unsigned int rs1_num = (insn >> SH_RS1) & MASK_RX;
> > -       ulong rs1_val = GET_RS1(insn, &vcpu->arch.guest_context);
> > +       unsigned int csr_num;
> >         const struct csr_func *tcfn, *cfn = NULL;
> >         ulong val = 0, wr_mask = 0, new_val = 0;
> >
> >         /* Decode the CSR instruction */
> > -       switch (GET_FUNCT3(insn)) {
> > -       case GET_FUNCT3(INSN_MATCH_CSRRW):
> > +       if (riscv_insn_is_csrrw(insn)) {
> >                 wr_mask = -1UL;
> > -               new_val = rs1_val;
> > -               break;
> > -       case GET_FUNCT3(INSN_MATCH_CSRRS):
> > -               wr_mask = rs1_val;
> > +               new_val = GET_REG(riscv_insn_csrrw_extract_xs1(insn));
> > +               csr_num = riscv_insn_csrrw_extract_csr(insn);
> > +       } else if (riscv_insn_is_csrrs(insn)) {
> > +               wr_mask = GET_REG(riscv_insn_csrrs_extract_xs1(insn));
> >                 new_val = -1UL;
> > -               break;
> > -       case GET_FUNCT3(INSN_MATCH_CSRRC):
> > -               wr_mask = rs1_val;
> > +               csr_num = riscv_insn_csrrs_extract_csr(insn);
> > +       } else if (riscv_insn_is_csrrc(insn)) {
> > +               wr_mask = GET_REG(riscv_insn_csrrs_extract_xs1(insn));
> >                 new_val = 0;
> > -               break;
> > -       case GET_FUNCT3(INSN_MATCH_CSRRWI):
> > +               csr_num = riscv_insn_csrrc_extract_csr(insn);
> > +       } else if (riscv_insn_is_csrrwi(insn)) {
> >                 wr_mask = -1UL;
> > -               new_val = rs1_num;
> > -               break;
> > -       case GET_FUNCT3(INSN_MATCH_CSRRSI):
> > -               wr_mask = rs1_num;
> > +               new_val = riscv_insn_csrrwi_extract_imm(insn);
> > +               csr_num = riscv_insn_csrrwi_extract_csr(insn);
> > +       } else if (riscv_insn_is_csrrsi(insn)) {
> > +               wr_mask = riscv_insn_csrrwi_extract_imm(insn);
> >                 new_val = -1UL;
> > -               break;
> > -       case GET_FUNCT3(INSN_MATCH_CSRRCI):
> > -               wr_mask = rs1_num;
> > +               csr_num = riscv_insn_csrrsi_extract_csr(insn);
> > +       } else if (riscv_insn_is_csrrci(insn)) {
> > +               wr_mask = GET_REG(riscv_insn_csrrwi_extract_imm(insn));
> >                 new_val = 0;
> > -               break;
> > -       default:
> > +               csr_num = riscv_insn_csrrwi_extract_csr(insn);
> > +       } else {
> >                 return rc;
> >         }
> >
> > +       #undef GET_REG
> > +
> >         /* Save instruction decode info */
> >         vcpu->arch.csr_decode.insn = insn;
> >         vcpu->arch.csr_decode.return_handled = 0;
> >
> > --
> > 2.54.0
> >
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

-- 
kvm-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kvm-riscv
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.