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:
- [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.
- [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.
- [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.
- [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/.
Alistair
> ---
> disas/riscv.c | 1 +
> target/riscv/cpu.c | 3 +++
> target/riscv/cpu.h | 3 +++
> target/riscv/cpu_bits.h | 9 +++++++++
> target/riscv/cpu_cfg_fields.h.inc | 1 +
> target/riscv/tcg/csr.c | 37
> +++++++++++++++++++++++++++++++++++++
> 6 files changed, 54 insertions(+)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index ca0d6e90b373..bce5f58305dd 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -703,6 +703,7 @@ static const char *csr_name(int csrno)
> case 0x0143: return "stval";
> case 0x0144: return "sip";
> case 0x0180: return "satp";
> + case 0x0181: return "srmcfg";
> case 0x0200: return "hstatus";
> case 0x0202: return "hedeleg";
> case 0x0203: return "hideleg";
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 5a0529fbdade..3432fea38ba0 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -284,6 +284,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(ssdbltrp, PRIV_VERSION_1_13_0, ext_ssdbltrp),
> ISA_EXT_DATA_ENTRY(ssnpm, PRIV_VERSION_1_13_0, ext_ssnpm),
> ISA_EXT_DATA_ENTRY(sspm, PRIV_VERSION_1_13_0, ext_sspm),
> + ISA_EXT_DATA_ENTRY(ssqosid, PRIV_VERSION_1_13_0, ext_ssqosid),
> ISA_INTERNAL_EXT_DATA_ENTRY(ssstateen, PRIV_VERSION_1_12_0,
> ext_ssstateen),
> ISA_INTERNAL_EXT_DATA_ENTRY(ssstrict, PRIV_VERSION_1_12_0,
> has_priv_1_12),
> ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
> @@ -3504,6 +3505,7 @@ static const TypeInfo riscv_cpu_type_infos[] =
> {
> .cfg.ext_zbc = true,
> .cfg.ext_zbs = true,
> .cfg.ext_sstc = true,
> + .cfg.ext_ssqosid = true,
> .cfg.ext_svadu = true,
> .cfg.ext_svvptc = true,
> ),
> @@ -3577,6 +3579,7 @@ static const TypeInfo riscv_cpu_type_infos[] =
> {
> .cfg.ext_zbc = true,
> .cfg.ext_zbs = true,
> .cfg.ext_sstc = true,
> + .cfg.ext_ssqosid = true,
> .cfg.ext_svadu = true,
> .cfg.ext_svvptc = true,
> ),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 49a1186bdbb2..0dfb280d1f33 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -369,6 +369,9 @@ struct CPUArchState {
> uint64_t ctr_dst[16 << SCTRDEPTH_MAX];
> uint64_t ctr_data[16 << SCTRDEPTH_MAX];
>
> + /* Ssqosid extension */
> + target_ulong srmcfg;
> +
> /* Machine and Supervisor interrupt priorities */
> uint8_t miprio[64];
> uint8_t siprio[64];
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 3579789a1f56..628183ae0fe2 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -228,6 +228,9 @@
> #define CSR_SPTBR 0x180
> #define CSR_SATP 0x180
>
> +/* Ssqosid extension */
> +#define CSR_SRMCFG 0x181
> +
> /* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> #define CSR_SISELECT 0x150
> #define CSR_SIREG 0x151
> @@ -356,6 +359,7 @@
> #define SMSTATEEN0_FCSR (1ULL << 1)
> #define SMSTATEEN0_JVT (1ULL << 2)
> #define SMSTATEEN0_CTR (1ULL << 54)
> +#define SMSTATEEN0_SRMCFG (1ULL << 55)
> #define SMSTATEEN0_P1P13 (1ULL << 56)
> #define SMSTATEEN0_HSCONTXT (1ULL << 57)
> #define SMSTATEEN0_IMSIC (1ULL << 58)
> @@ -1199,4 +1203,9 @@ typedef enum CTRType {
> #define MCONTEXT64 0x0000000000001FFFULL
> #define MCONTEXT32_HCONTEXT 0x0000007F
> #define MCONTEXT64_HCONTEXT 0x0000000000003FFFULL
> +
> +/* SRMCFG CSR field masks (Ssqosid extensions) */
> +#define SRMCFG_RCID 0x00000FFF
> +#define SRMCFG_MCID 0x0FFF0000
> +
> #endif
> diff --git a/target/riscv/cpu_cfg_fields.h.inc
> b/target/riscv/cpu_cfg_fields.h.inc
> index 85fc3ad8bb8c..1ecdf9f393b4 100644
> --- a/target/riscv/cpu_cfg_fields.h.inc
> +++ b/target/riscv/cpu_cfg_fields.h.inc
> @@ -113,6 +113,7 @@ BOOL_FIELD(ext_ssaia)
> BOOL_FIELD(ext_smctr)
> BOOL_FIELD(ext_ssctr)
> BOOL_FIELD(ext_sscofpmf)
> +BOOL_FIELD(ext_ssqosid)
> BOOL_FIELD(ext_smepmp)
> BOOL_FIELD(ext_smrnmi)
> BOOL_FIELD(ext_ssnpm)
> diff --git a/target/riscv/tcg/csr.c b/target/riscv/tcg/csr.c
> index af444e3e6f32..fc94f569ea8a 100644
> --- a/target/riscv/tcg/csr.c
> +++ b/target/riscv/tcg/csr.c
> @@ -1805,6 +1805,40 @@ static RISCVException
> write_stimecmph(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException check_srmcfg(CPURISCVState *env, int csrno)
> +{
> + if (!riscv_cpu_cfg(env)->ext_ssqosid) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + RISCVException ret = smstateen_acc_ok(env, 0,
> SMSTATEEN0_SRMCFG);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + /*
> + * Even though this is an S-mode CSR the spec says that we need
> to throw
> + * and virt instruction fault if a guest tries to access it.
> + */
> + return env->virt_enabled ?
> + RISCV_EXCP_VIRT_INSTRUCTION_FAULT : smode(env, csrno);
> +}
> +
> +static RISCVException read_srmcfg(CPURISCVState *env, int csrno,
> + target_ulong *val)
> +{
> + *val = env->srmcfg;
> + return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_srmcfg(CPURISCVState *env, int csrno,
> + target_ulong val, uintptr_t ra)
> +{
> + env->srmcfg = val & (SRMCFG_RCID | SRMCFG_MCID);
> + return RISCV_EXCP_NONE;
> +}
> +
> +
> #define VSTOPI_NUM_SRCS 5
>
> /*
> @@ -6377,6 +6411,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] =
> {
> /* Supervisor Protection and Translation */
> [CSR_SATP] = { "satp", satp, read_satp,
> write_satp },
>
> + /* Supervisor-Level Quality-of-Service Identifiers (Ssqosid) */
> + [CSR_SRMCFG] = { "srmcfg", check_srmcfg, read_srmcfg,
> write_srmcfg },
> +
> /* Supervisor-Level Window to Indirectly Accessed Registers
> (AIA) */
> [CSR_SISELECT] = { "siselect", csrind_or_aia_smode, NULL,
> NULL,
>
> rmw_xiselect },
