Re: [PATCH v7 09/22] RISC-V: Add Ssccfg/Smcdeleg ISA extension definition and parsing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] `riscv_ext_smcdeleg_validate` incorrectly checks for the Supervisor-level extension `SSCSRIND` instead of the Machine-level extension `SMCSRIND`.
- [High] The supervisor-level extension `Ssccfg` improperly mandates the presence of the Machine-level `Smcdeleg` extension and relies on flawed dependency logic, breaking `Ssccfg` in S-mode guests.
--

commit ad250a6901a4cd821647231579298f446a592807
Author: Atish Patra <atishp@rivosinc.com>

RISC-V: Add Ssccfg/Smcdeleg ISA extension definition and parsing

Smcdeleg extension allows the M-mode to delegate selected counters
to S-mode so that it can access those counters and correpsonding
hpmevent CSRs without M-mode. Ssccfg provides access to delegated
counters and new supervisor-level state.

> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1452521d740a6..1fe647e035157 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -330,6 +330,27 @@ static const unsigned int riscv_a_exts[] = {
>  	RISCV_ISA_EXT_ZKNE,	\
>  	RISCV_ISA_EXT_ZKNH
>  
> +static int riscv_ext_smcdeleg_validate(const struct riscv_isa_ext_data *data,
> +				       const unsigned long *isa_bitmap)
> +{
> +	if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_SSCSRIND) &&

[Severity: High]
Should this validate against the Machine-level extension SMCSRIND instead of
the Supervisor-level SSCSRIND?

Since Smcdeleg is a Machine-mode extension, checking the S-mode equivalent
might cause Smcdeleg to incorrectly fail validation on hardware that exposes
SMCSRIND but not SSCSRIND. It could also erroneously pass if SMCSRIND is
missing.

> +	    __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZIHPM) &&
> +	    __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZICNTR))
> +		return 0;
> +
> +	return -EPROBE_DEFER;
> +}
> +
> +static int riscv_ext_ssccfg_validate(const struct riscv_isa_ext_data *data,
> +				     const unsigned long *isa_bitmap)
> +{
> +	if (!riscv_ext_smcdeleg_validate(data, isa_bitmap) &&
> +	    __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_SMCDELEG))

[Severity: High]
Does enforcing a Machine-mode extension dependency (SMCDELEG) here break
Ssccfg in virtualized environments?

In a guest OS, the hypervisor typically exposes S-mode extensions like Ssccfg
but filters out M-mode extensions like Smcdeleg. Enforcing the presence of
SMCDELEG means the guest kernel will silently fail to enable Ssccfg.

Also, is it necessary to call riscv_ext_smcdeleg_validate() directly here?
Reusing the Machine-mode validation logic prevents Ssccfg from independently
checking its own software-visible S-mode dependencies (SSCSRIND, ZIHPM,
and ZICNTR).

> +		return 0;
> +
> +	return -EPROBE_DEFER;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-counter_delegation-v7-0-0ba2fd34614e@meta.com?part=9