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

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Ssccfg validation incorrectly depends on the machine-mode extension SMCDELEG, breaking virtualization.
- [Medium] Smcdeleg validation incorrectly checks for the supervisor-mode extension SSCSRIND instead of the machine-mode variant SMCSRIND.
--

--- Patch [9]: [PATCH v9 09/20] RISC-V: Add Ssccfg/Smcdeleg ISA extension definition and parsing ---
commit e656dfc58bff7f1b23f61ced28dbec6228def44b
Author: Atish Patra <[email protected]>

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. This patch enables these
definitions and parsing.

Link: https://patch.msgid.link/[email protected]

> 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: Medium]
Should this check for the machine-mode extension RISCV_ISA_EXT_SMCSRIND
instead?

Validating a machine-mode extension (Smcdeleg) against a supervisor-mode
dependency (SSCSRIND) seems semantically incorrect according to the RISC-V
specification, which usually ties M-mode extensions to M-mode prerequisites.

> +	    __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]
Will this check cause the Ssccfg extension to fail validation when running as
a guest VM?

Since a hypervisor (HS-mode) exposes the supervisor-level Ssccfg extension to
the guest (VS-mode) but omits machine-mode extensions like SMCDELEG,
requiring SMCDELEG to be present in the ISA bitmap means Ssccfg will return
-EPROBE_DEFER.

This appears to break performance counter configuration in virtualized
environments where M-mode extensions are structurally impossible for the guest
to possess.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9