On Tue, Oct 27, 2026 at 12:22:02AM +0000, Alistair Francis wrote: > On Mon, 2026-10-26 at 10:47 -0700, Drew Fustini wrote: > > From: Kornel Dulęba <[email protected]> > > > > Implement the srmcfg CSR defined by the Ssqosid ISA extension > > (Supervisor-mode Quality of Service ID). The CSR contains two fields: > > > > - Resource Control ID (RCID) used determine resource allocation > > - Monitoring Counter ID (MCID) used to track resource usage > > > > The CSR is defined for S-mode, so check mstateen0.srmcfg to determine > > if s-mode is allowed to access it. In addition, accessing it when V=1 > > shall cause a virtual instruction exception. > > > > Link: > > https://github.com/riscv-non-isa/riscv-cbqri/releases/download/v1.0/riscv-cbqri.pdf > > Link: https://docs.riscv.org/reference/isa/priv/smstateen.html > > Signed-off-by: Kornel Dulęba <[email protected]> > > [fustini: rebase on v10.1.50, fix check_srmcfg] > > Reviewed-by: Radim Krčmář <[email protected]> > > Signed-off-by: Drew Fustini <[email protected]> > > I'm just meat proxying this one, but it looks believable, let me know > if it's wrong though:
Is this something I can run myself in the future? I can run local sashiko for my linux patches but I wasn't sure what to do for qemu. > - [HIGH] In target/riscv/tcg/csr.c, none of write_mstateen0, > write_mstateen0h, write_hstateen0, or write_hstateen0h ever ORs > SMSTATEEN0_SRMCFG into wr_mask when ext_ssqosid is set, so bit 55 of > mstateen0/hstateen0 stays read-only zero and M-mode firmware writes > that enable srmcfg access are silently dropped, violating the > Smstateen spec (a bit for an implemented extension must not be > read-only zero). check_srmcfg gates the new CSR on > smstateen_acc_ok(..., SMSTATEEN0_SRMCFG), and Smstateen is enabled by > default on the same dynamic CPUs that get ext_ssqosid by default, so > on the stock virt machine every S-mode (V=0) srmcfg access traps > illegal instruction and VS-mode additionally takes a virtual > instruction fault: the QoS feature this series exists to expose is > systematically unusable. Fix by ORing the bit into the stateen write > masks like the existing CTR, P1P13, and SVSLCT cases. This is a real issue for mstateen0 and mstateen0h. I will add SMSTATEEN0_SRMCFG to wr_mask in write_mstateen0() and write_mstateen0h(). However, I believe hstateen0 bit 55 is WPRI so I think the current behavior is correct. > - [MEDIUM] In target/riscv/cpu.h, env->srmcfg is new guest-writable > architected state (written via write_srmcfg in > target/riscv/tcg/csr.c) but target/riscv/machine.c never migrates > it: vmstate_riscv_cpu has no ssqosid/srmcfg field or subsection. > After live migration, savevm/loadvm, or snapshot restore the > destination CPU resumes with srmcfg = 0 while the guest believes its > programmed RCID/MCID are still in effect. Add a versioned > vmstate_ssqosid subsection with a .needed predicate on ext_ssqosid, > following the vmstate_smstateen/vmstate_envcfg pattern. I'll add a VMStateDescription for Ssqosid. > - [MEDIUM] In target/riscv/cpu.c (isa_edata_arr), Ssqosid is > registered as a user-facing extension and enabled by default on > BASE32/BASE64 with no tests/tcg coverage anywhere in the tree. The > RISC-V upstreaming rules require unit tests for new extensions' CSR > accesses, and the stateen gating and V=1 trap paths added in > check_srmcfg are exactly what such tests would exercise -- they would > have caught the broken SRMCFG stateen bit above. Add tests/tcg cases > for srmcfg read/write, WARL masking, and the > stateen/virtual-instruction-fault gating. I'll add tests for srmcfg. > - [LOW] nit: in target/riscv/tcg/csr.c, the check_srmcfg comment says > "we need to throw and virt instruction fault"; s/and virt/a virtual/. I'll fix. Thanks, Drew
