On Tue, 2026-10-27 at 19:59 -0700, Drew Fustini wrote: > 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?
Kind of! Working on it > > I can run local sashiko for my linux patches but I wasn't sure what > to > do for qemu. Yeah, it's Sashiko as well: https://github.com/sashiko-dev/sashiko/pull/753 Alistair > > > - [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
