Re: [PATCH v5 3/5] KVM: RISC-V: SBI FWFT: Fix stale feature exposure after runtime extension changes

[Anup Patel]

On Mon, Jun 1, 2026 at 3:56 PM Yong-Xuan Wang <yongxuan.wang@xxxxxxxxxx> wrote:
>
> Fix a bug where FWFT features could be incorrectly exposed to guests
> after userspace disables their dependent ISA extensions at runtime.
>
> The 'supported' field in kvm_sbi_fwft_config was set once during vCPU
> initialization based on the initial hardware/extension availability.
> However, when userspace subsequently disables ISA extensions via the KVM
> ONE_REG interface, the 'supported' field was not updated. This caused
> the following issues:
> 1. FWFT features would remain visible and accessible to guests even
>    after their prerequisite ISA extensions were disabled
> 2. Guests could configure FWFT features that depend on disabled
>    extensions, leading to undefined behavior
> 3. The static 'supported' flag and the dynamic supported() callback
>    could disagree about feature availability
>
> The fix introduces a two-layer checking mechanism:
> 1. Add an optional init() callback to the kvm_sbi_fwft_feature structure
>    for features that require hardware probing during initialization. This
>    separates the one-time hardware detection logic from the runtime
>    availability check.
> 2. Add runtime checks in all FWFT-related functions that call
>    feature->supported(vcpu) if the callback exists. This ensures feature
>    availability is re-evaluated based on the current ISA extension state.
>
> This approach maintains the cached 'supported' field for initialization-
> time decisions while ensuring runtime availability is always determined
> by the current vCPU configuration, not initialization-time snapshots.
>
> Fixes: 6b72fd170592 ("RISC-V: KVM: add support for FWFT SBI extension")
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@xxxxxxxxxx>

LGTM.

Reviewed-by: Anup Patel <anup@xxxxxxxxxxxxxx>

Thanks,
Anup

> ---
>  arch/riscv/kvm/vcpu_sbi_fwft.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> index aee951f2b8e6..ab39ac464ffd 100644
> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
> @@ -303,6 +303,8 @@ static int kvm_fwft_get_feature(struct kvm_vcpu *vcpu, u32 feature,
>
>         if (!tconf->supported || !tconf->enabled)
>                 return SBI_ERR_NOT_SUPPORTED;
> +       else if (tconf->feature->supported && !tconf->feature->supported(vcpu))
> +               return SBI_ERR_NOT_SUPPORTED;
>
>         *conf = tconf;
>
> @@ -433,6 +435,8 @@ static unsigned long kvm_sbi_ext_fwft_get_reg_count(struct kvm_vcpu *vcpu)
>                 conf = kvm_sbi_fwft_get_config(vcpu, feature->id);
>                 if (!conf || !conf->supported)
>                         continue;
> +               else if (conf->feature->supported && !conf->feature->supported(vcpu))
> +                       continue;
>
>                 ret++;
>         }
> @@ -455,6 +459,8 @@ static int kvm_sbi_ext_fwft_get_reg_id(struct kvm_vcpu *vcpu, int index, u64 *re
>                 conf = kvm_sbi_fwft_get_config(vcpu, feature->id);
>                 if (!conf || !conf->supported)
>                         continue;
> +               else if (conf->feature->supported && !conf->feature->supported(vcpu))
> +                       continue;
>
>                 if (index == idx) {
>                         *reg_id = KVM_REG_RISCV |
> @@ -490,6 +496,8 @@ static int kvm_sbi_ext_fwft_get_reg(struct kvm_vcpu *vcpu, unsigned long reg_num
>         conf = kvm_sbi_fwft_get_config(vcpu, feature->id);
>         if (!conf || !conf->supported)
>                 return -ENOENT;
> +       else if (conf->feature->supported && !conf->feature->supported(vcpu))
> +               return -ENOENT;
>
>         switch (reg_num - feature->first_reg_num) {
>         case 0:
> @@ -527,6 +535,8 @@ static int kvm_sbi_ext_fwft_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num
>         conf = kvm_sbi_fwft_get_config(vcpu, feature->id);
>         if (!conf || !conf->supported)
>                 return -ENOENT;
> +       else if (conf->feature->supported && !conf->feature->supported(vcpu))
> +               return -ENOENT;
>
>         switch (reg_num - feature->first_reg_num) {
>         case 0:
>
> --
> 2.43.7
>