Re: [PATCH v3] lib: sbi: Enable Ssqosid Ext using mstateen0

["Radim Krčmář" <rkrcmar@ventanamicro.com>]

2025-11-07T16:32:00+08:00, <cp0613@linux.alibaba.com>:
> From: Chen Pei <cp0613@linux.alibaba.com>
>
> The QoS Identifiers extension (Ssqosid) introduces the srmcfg register,
> which configures a hart with two identifiers: a Resource Control ID
> (RCID) and a Monitoring Counter ID (MCID). These identifiers accompany
> each request issued by the hart to shared resource controllers.
>
> If extension Smstateen is implemented together with Ssqosid, then
> Ssqosid also requires the SRMCFG bit in mstateen0 to be implemented. If
> mstateen0.SRMCFG is 0, attempts to access srmcfg in privilege modes less
> privileged than M-mode raise an illegal-instruction exception. If
> mstateen0.SRMCFG is 1 or if extension Smstateen is not implemented,
> attempts to access srmcfg when V=1 raise a virtual-instruction exception.
>
> This extension can be found in the RISC-V Instruction Set Manual:
> https://github.com/riscv/riscv-isa-manual
>
> Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
> ---
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> @@ -79,6 +79,8 @@ enum sbi_hart_extensions {
>  	SBI_HART_EXT_SMCTR,
>  	/** HART has CTR S-mode CSRs */
>  	SBI_HART_EXT_SSCTR,
> +	/** Hart has Ssqosid extension */
> +	SBI_HART_EXT_SSQOSID,

Don't we also need something like

  __check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
                  CSR_SRMCFG, SBI_HART_EXT_SSQOSID);

to detect the SSQOSID extension?

> diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
> @@ -143,8 +145,11 @@ static int switch_to_next_domain_context(struct hart_context *ctx,
>  	ctx->satp	= csr_swap(CSR_SATP, dom_ctx->satp);
>  	if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
>  		ctx->scounteren = csr_swap(CSR_SCOUNTEREN, dom_ctx->scounteren);
> -	if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_12)
> +	if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_12) {
>  		ctx->senvcfg	= csr_swap(CSR_SENVCFG, dom_ctx->senvcfg);
> +		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSQOSID))
> +			ctx->srmcfg	= csr_swap(CSR_SRMCFG, dom_ctx->srmcfg);
> +	}

Why would we want Ssqosid to depend on S >= 1.12?

> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> @@ -112,6 +112,11 @@ static void mstatus_init(struct sbi_scratch *scratch)
>  		else
>  			mstateen_val &= ~SMSTATEEN0_CTR;
>  
> +		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSQOSID))
> +			mstateen_val |= (SMSTATEEN0_SRMCFG);

(Style nit: please remove parentheses arounds SMSTATEEN0_SRMCFG.)

> +		else
> +			mstateen_val &= ~(SMSTATEEN0_SRMCFG);

The else branch is pointless, because the bit is known to be 0.

Thanks.

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi