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

[Bibo Mao]

On 2026/7/16 下午12:46, Tao Cui wrote:
> 在 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.
Now pv_features is per VM, will set it as per CPU to avoid access 
contention in next version. Consistent checking should with per CPU 
pv_features will be done in VMM, rather than KVM, similar with per CPU 
cpucfg feature.

Regards
Bibo Mao
> Thanks,
> Tao
> > Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx>
> > ---
> >   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