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

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 <[email protected]>
>> ---
>>   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
>