Re: [PATCH v2 1/6] target/riscv: Add stubs for T-Head PMU CSRs

[Kuan-Wei Chiu <visitorckw@gmail.com>]

Hi Chao,

On Thu, May 14, 2026 at 10:48:07AM +0800, Chao Liu wrote:
> On Thu, May 14, 2026 at 01:15:23AM +0000, Kuan-Wei Chiu wrote:
> > T-Head CPUs use custom CSRs for performance monitoring, specifically
> > mcounterinten (0x7ca) and mcounterof (0x7cb). [1]
> > 
> > Since we don't implement these custom PMU registers yet, the system
> > crashes with an illegal instruction trap when OpenSBI like this:
> > 
> > system_opcode_insn: Failed to access CSR 0x7ca from M-mode
> > sbi_trap_error: hart0: trap1: illegal instruction handler failed (error -1)
> > 
> > Add simple read/write stubs for these two CSRs. By silently ignoring
> > writes and returning 0 on reads, we prevent the fatal exceptions and
> > allow to continue normally.
> > 
> > Link: https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource/XuanTie-OpenC906-UserManual.pdf [1]
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > ---
> >  target/riscv/th_csr.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
> > index 49eb7bbab5..b095364c31 100644
> > --- a/target/riscv/th_csr.c
> > +++ b/target/riscv/th_csr.c
> > @@ -21,12 +21,19 @@
> >  #include "cpu_vendorid.h"
> >  
> >  #define CSR_TH_SXSTATUS 0x5c0
> > +#define CSR_TH_MCOUNTERINTEN 0x7ca
> > +#define CSR_TH_MCOUNTEROF    0x7cb
> >  
> >  /* TH_SXSTATUS bits */
> >  #define TH_SXSTATUS_UCME        BIT(16)
> >  #define TH_SXSTATUS_MAEE        BIT(21)
> >  #define TH_SXSTATUS_THEADISAEE  BIT(22)
> >  
> > +static RISCVException mmode(CPURISCVState *env, int csrno)
> > +{
> > +    return RISCV_EXCP_NONE;
> > +}
> We can add an RVM extension check, similar in form to smode():
> 
> static RISCVException mmode(CPURISCVState *env, int csrno)
> {
>     if (riscv_has_ext(env, RVM)) {

Thanks for your review.
However, I think this might not be correct.

IIUC, RVM represents the standard M extension for integer
multiplication and division instructions, rather than indicating
whether M mode is implemented. Is it even possible for a riscv machine
implementation to not have M mode?

Looking at the misa csr description in the spec, the 'S' and 'U' bits
specifically denote the implementation of supervisor and user modes,
respectively, but there is no corresponding bit for M mode. I assume
this implies M mode is unconditionally required?

>         return RISCV_EXCP_NONE;
>     }
> 
>     return RISCV_EXCP_ILLEGAL_INST;
> }
> 
> For your reference, I have added more T-Head CSR support in the
> K230 patchset.
> 
> https://lore.kernel.org/qemu-devel/1c7319cd00a50bffeb41f9cc13339a8cd0c07350.1778516731.git.chao.liu.zevorn@gmail.com/

It looks like your patch covers much more ground than mine and has
already received quite a few reviews. I assume once your series is
merged, I can simply drop my patch #1.

So I guess the most sensible workflow here is for me to wait until your
patchset is merged into the subsystem maintainer's tree (assuming it
will route through Alistair's tree [1] I guess?, please correct me if
I'm wrong!), and then I'll rebase my series on top of it and send out
v3?

[1]: https://github.com/alistair23/qemu.git riscv-to-apply.next

Regards,
Kuan-Wei

> 
> As for this patch, I think the other changes look mostly fine.
> 
> Thanks,
> Chao
> > +
> >  static RISCVException smode(CPURISCVState *env, int csrno)
> >  {
> >      if (riscv_has_ext(env, RVS)) {
> > @@ -49,11 +56,34 @@ static RISCVException read_th_sxstatus(CPURISCVState *env, int csrno,
> >      return RISCV_EXCP_NONE;
> >  }
> >  
> > +static RISCVException read_th_pmu(CPURISCVState *env, int csrno,
> > +                                  target_ulong *val)
> > +{
> > +    *val = 0;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_th_pmu(CPURISCVState *env, int csrno,
> > +                                   target_ulong val, uintptr_t retaddr)
> > +{
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> >  const RISCVCSR th_csr_list[] = {
> >      {
> >          .csrno = CSR_TH_SXSTATUS,
> >          .insertion_test = test_thead_mvendorid,
> >          .csr_ops = { "th.sxstatus", smode, read_th_sxstatus }
> >      },
> > +    {
> > +        .csrno = CSR_TH_MCOUNTERINTEN,
> > +        .insertion_test = test_thead_mvendorid,
> > +        .csr_ops = { "th.mcounterinten", mmode, read_th_pmu, write_th_pmu }
> > +    },
> > +    {
> > +        .csrno = CSR_TH_MCOUNTEROF,
> > +        .insertion_test = test_thead_mvendorid,
> > +        .csr_ops = { "th.mcounterof", mmode, read_th_pmu, write_th_pmu }
> > +    },
> >      { }
> >  };
> > -- 
> > 2.54.0.563.g4f69b47b94-goog
> >