On Fri, May 15, 2026 at 02:07:18AM +0800, Kuan-Wei Chiu wrote:
> 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 <[email protected]>
> > > ---
> > >  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?
> 
Good catch, your analysis is correct. I will fix this in the next version.

> >         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.zev...@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?
> 
I think both are fine, we need to see what Alistair suggests.

Thanks,
Chao

> [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
> > > 

Reply via email to