Re: [PATCH] LoongArch: KVM: Allow to set pv_feature until vCPU run

[Tao Cui <cui.tao@linux.dev>]

在 2026/7/16 09:38, Bibo Mao 写道:
> Now pv_feature can be set only once, there is problem with VM migration.
> Where it is set when vCPU is created and after migration, here it is
> allow to set for many times, until vCPU starts to run.
> 
Hi Bibo,

The Sashiko AI review raised two concerns on this patch that I think
are valid:

1. Since ran_atleast_once is per-vCPU but pv_features is VM-wide,
   userspace could run vCPU 0 and then use vCPU 1 (whose
   ran_atleast_once is still false) to change pv_features while
   vCPU 0 is executing.

2. Without the old LOONGARCH_PV_FEAT_UPDATED latch, setting different
   PV features on different un-run vCPUs silently overwrites
   pv_features instead of returning -EINVAL.

Both stem from using a per-vCPU flag to protect VM-wide state.
Would a VM-level bool in kvm_arch (e.g. pv_features_configured),
set on first SET_ATTR or first RUN of any vCPU, work?  It would
not be migrated since it is not exposed via any ioctl.

Thanks,
Tao
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  arch/loongarch/include/asm/kvm_host.h |  4 +++-
>  arch/loongarch/kvm/vcpu.c             | 15 +++++++++++----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
> index 23cfbecebbd7..af376fc44c44 100644
> --- a/arch/loongarch/include/asm/kvm_host.h
> +++ b/arch/loongarch/include/asm/kvm_host.h
> @@ -163,7 +163,6 @@ enum emulation_result {
>  #define KVM_LARCH_SWCSR_LATEST	(0x1 << 3)
>  #define KVM_LARCH_HWCSR_USABLE	(0x1 << 4)
>  
> -#define LOONGARCH_PV_FEAT_UPDATED	BIT_ULL(63)
>  #define LOONGARCH_PV_FEAT_MASK		(BIT(KVM_FEATURE_IPI) |		\
>  					 BIT(KVM_FEATURE_PREEMPT) |	\
>  					 BIT(KVM_FEATURE_STEAL_TIME) |	\
> @@ -250,6 +249,9 @@ struct kvm_vcpu_arch {
>  	/* cpucfg */
>  	u32 cpucfg[KVM_MAX_CPUCFG_REGS];
>  
> +	/* VCPU ran at least once */
> +	bool ran_atleast_once;
> +
>  	/* paravirt steal time */
>  	struct {
>  		u64 guest_addr;
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 20c207d80e31..ce6a1b06d50d 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -1164,11 +1164,14 @@ static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu,
>  		if (val & ~valid)
>  			return -EINVAL;
>  
> -		/* All vCPUs need set the same PV features */
> -		if ((kvm->arch.pv_features & LOONGARCH_PV_FEAT_UPDATED)
> -				&& ((kvm->arch.pv_features & valid) != val))
> +		if ((kvm->arch.pv_features & valid) == val)
> +			return 0;
> +
> +		if (vcpu->arch.ran_atleast_once)
>  			return -EINVAL;
> -		kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
> +
> +		/* All vCPUs need set the same PV features */
> +		kvm->arch.pv_features = val;
>  		return 0;
>  	default:
>  		return -ENXIO;
> @@ -1851,6 +1854,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>  	int r = -EINTR;
>  	struct kvm_run *run = vcpu->run;
>  
> +	/* Mark this VCPU ran at least once */
> +	if (!vcpu->arch.ran_atleast_once)
> +		vcpu->arch.ran_atleast_once = true;
> +
>  	if (vcpu->mmio_needed) {
>  		if (!vcpu->mmio_is_write)
>  			kvm_complete_mmio_read(vcpu, run);
> 
> base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa