Re: [PATCH v5 1/5] KVM: RISC-V: SBI FWFT: Mark vCPU CSRs dirty after setting feature value

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unconditional setting of csr_dirty contradicts the commit message design intent.
--

commit acd5b6b18e2d1bc390b68230f877d70bd8155368
Author: Yong-Xuan Wang <yongxuan.wang@sifive.com>

KVM: RISC-V: SBI FWFT: Mark vCPU CSRs dirty after setting feature value

This commit updates KVM on RISC-V to mark vCPU CSRs as dirty when an FWFT
feature value is updated. This prevents the guest from observing stale CSR
states after vCPU scheduling or migration by ensuring the modified state is
properly synchronized.

> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> index 2eab15339694f..5e4aafb0cbf16 100644
> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
> @@ -521,6 +521,7 @@ static int kvm_sbi_ext_fwft_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num
>  		break;
>  	case 2:
>  		ret = conf->feature->set(vcpu, conf, true, value);
> +		vcpu->arch.csr_dirty = true;

[Severity: Medium]
The commit message mentions marking the CSRs dirty after successfully setting
an FWFT feature value. Does kvm_sbi_ext_fwft_set_reg() unconditionally set
vcpu->arch.csr_dirty to true, even if conf->feature->set() fails and returns
an error?

Could this lead to unnecessary CSR hardware reloads on error paths since the
return value in ret is not checked before modifying the dirty flag?

>  		break;
>  	default:
>  		return -ENOENT;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-kvm-get_reg_list-v2-v5-0-415d08a2813b@sifive.com?part=1