Re: [PATCH v8 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] The Supervisor-level `Ssccfg` extension validation strictly requires the Machine-level `Smcdeleg` extension, breaking S-mode guest VMs.
- [Medium] riscv_ext_ssccfg_validate passes the wrong context pointer to riscv_ext_smcdeleg_validate and performs a logically redundant check.
--

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

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

This commit introduces definitions and parsing for the Smcdeleg and Ssccfg
ISA extensions on RISC-V. It adds validation logic to ensure that required
dependencies are present before enabling these extensions.

> 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) &&
> +	    __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 this code inadvertently break Ssccfg support in S-mode guest VMs?

When Linux runs as a guest VM under a hypervisor, M-mode extensions like
Smcdeleg are hidden from the guest OS. Since this validation strictly requires
RISCV_ISA_EXT_SMCDELEG to be present in the ISA string, Ssccfg will always
return -EPROBE_DEFER and be disabled.

Should a Supervisor-level extension depend on the presence of a Machine-level
extension?

[Severity: Medium]
Is it intentional to pass the SSCCFG data pointer to a function expecting
SMCDELEG's context?

While it currently doesn't crash because riscv_ext_smcdeleg_validate() ignores
the data pointer, this seems to violate the validation callback API contract.

Additionally, is it necessary to call riscv_ext_smcdeleg_validate() directly
here? If RISCV_ISA_EXT_SMCDELEG is present in the resolved isa_bitmap, it
should already be guaranteed to have passed its own validation function during
a previous loop iteration.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-counter_delegation-v8-0-7909f863a645@meta.com?part=9