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

[cp0613@linux.alibaba.com]

On 2025-11-07 11:13 AM, Radim Krčmář wrote:

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

Hi Radim,

I added this detection mechanism in the first patch, but Anup said, "For newer
extensions, detecting from ISA strings is sufficient, so there's no need to
trap-n-detect here." Therefore, it has been removed here.

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

I'm guessing you read the RISC-V Instruction Set Manual, which describes ssqoid as
requiring priv version 1.14. However, the highest priv version currently supported
by OpenSBI is 1.12. Therefore, this implementation is based on the current situation,
and the previous implementations such as CTR also follow this approach.

> > 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.)

Thank you for pointing this out.

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

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

> Thanks.

Here we need to consider the following scenario: even if HART supports the SSQOSID
extension, but the user doesn't want to use this feature and removes the ssqosid
field from the ISA string, then this bit needs to be explicitly cleared.

Thanks,
Pei

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