Re: [PATCH v4 1/8] RISC-V: KVM: Add support for Svadu FWFT features

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing Nested Virtualization (NACL) Synchronization in FWFT setter
- [High] State Corruption on Reset for SVADU-only guests
--

commit 3a9bb56abfad9f69ff748e7562ad8693be40c7f9
Author: Inochi Amaoto <inochiama@gmail.com>
RISC-V: KVM: Add support for Svadu FWFT features

This commit adds support for the Svadu FWFT feature in RISC-V KVM.
It exposes the PTE A/D hardware updating feature if both Svadu and Svade
are supported, and adds helper functions to get and set the ISA extension
state for the guest.

> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> index ab39ac464ffd8..7192c229a19e2 100644
> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
[ ... ]
> @@ -94,6 +94,45 @@ static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature)
>  	return false;
>  }
>  
> +static void kvm_sbi_fwft_env_flag_reset_helper(struct kvm_vcpu *vcpu,
> +					       u64 flag)
> +{
> +	vcpu->arch.cfg.henvcfg &= ~flag;
> +}
> +
> +static long kvm_sbi_fwft_env_flag_set_helper(struct kvm_vcpu *vcpu,
> +					     struct kvm_sbi_fwft_config *conf,
> +					     bool one_reg_access,
> +					     unsigned long value, u64 flag)
> +{
> +	struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
> +
> +	if (value == 0)
> +		cfg->henvcfg &= ~flag;
> +	else if (value == 1)
> +		cfg->henvcfg |= flag;
> +	else
> +		return SBI_ERR_INVALID_PARAM;
> +
> +	if (!one_reg_access) {
> +		csr_write(CSR_HENVCFG, vcpu->arch.cfg.henvcfg);
> +		if (IS_ENABLED(CONFIG_32BIT))
> +			csr_write(CSR_HENVCFGH, vcpu->arch.cfg.henvcfg >> 32);
> +	}

[Severity: High]
Will this cause state corruption when KVM nested acceleration (NACL) is
active?

When KVM acts as an L1 hypervisor using NACL, updating the CSR directly
with csr_write() fails to update the NACL shared memory page (nacl_shmem).

When L0 later switches context or resumes the L2 guest, it will reload
CSR_HENVCFG from the unmodified nacl_shmem, silently discarding the L2
guest's FWFT update.

Should this use ncsr_write() instead to ensure safe H-mode CSR
modifications for nested virtualization?

> +
> +	return SBI_SUCCESS;
> +}
[ ... ]
> @@ -137,6 +176,33 @@ static long kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
>  	return SBI_SUCCESS;
>  }
>  
> +static bool kvm_sbi_fwft_pte_ad_hw_updating_supported(struct kvm_vcpu *vcpu)
> +{
> +	return riscv_isa_extension_available(vcpu->arch.isa, SVADU) &&
> +		riscv_isa_extension_available(vcpu->arch.isa, SVADE);
> +}
> +
> +static void kvm_sbi_fwft_reset_pte_ad_hw_updating(struct kvm_vcpu *vcpu)
> +{
> +	kvm_sbi_fwft_env_flag_reset_helper(vcpu, ENVCFG_ADUE);
> +}

[Severity: High]
Could this break A/D bit updates for SVADU-only guests after a reboot?

A VCPU configured with SVADU but without SVADE requires ENVCFG_ADUE to be
set to 1 to function properly. This is correctly initialized the first
time in kvm_riscv_vcpu_config_ran_once().

However, when the guest initiates a system reset, this reset handler
unconditionally clears ENVCFG_ADUE. Because vcpu->arch.ran_atleast_once
is already true and is not cleared on reset,
kvm_riscv_vcpu_config_ran_once() will not run again.

Does this leave the SVADU-only VCPU resuming with ENVCFG_ADUE cleared,
causing hardware to fail updating A/D bits without SVADE to trap them?

> +
> +static long kvm_sbi_fwft_set_pte_ad_hw_updating(struct kvm_vcpu *vcpu,
> +						struct kvm_sbi_fwft_config *conf,
> +						bool one_reg_access, unsigned long value)
> +{
> +	return kvm_sbi_fwft_env_flag_set_helper(vcpu, conf, one_reg_access,
> +						value, ENVCFG_ADUE);
> +}
> +
> +static long kvm_sbi_fwft_get_pte_ad_hw_updating(struct kvm_vcpu *vcpu,
> +						struct kvm_sbi_fwft_config *conf,
> +						bool one_reg_access, unsigned long *value)
> +{
> +	return kvm_sbi_fwft_env_flag_get_helper(vcpu, conf, one_reg_access,
> +						value, ENVCFG_ADUE);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701020746.170157-1-inochiama@gmail.com?part=1