Re: [PATCH v2 1/3] target/loongarch/kvm: advertise pv features per vCPU

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

在 2026/7/10 10:58, Bibo Mao 写道:
> 
> 
> On 2026/7/10 上午8:50, Tao Cui wrote:
>> From: Tao Cui <cuitao@kylinos.cn>
>>
>> kvm_set_pv_features() programs the KVM_FEATURE cpucfg attribute, which is a
>> per-vCPU setting. It was called from kvm_arch_put_registers() under a
>> function-local static guard, so it ran only once for the whole VM: only the
>> first vCPU got its pv features pushed to KVM, and on SMP guests the others
>> never saw KVM_FEATURE_IPI / KVM_FEATURE_STEAL_TIME.
>>
>> Drop the static guard and push pv features per vCPU under
>> KVM_PUT_FULL_STATE, the same gate kvm_set_stealtime() already uses. Host
>> feature detection stays in kvm_arch_init_vcpu(); the per-vCPU state write
>> belongs in kvm_arch_put_registers().
>>
>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>> ---
>>   target/loongarch/kvm/kvm.c | 15 ++++++---------
>>   1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
>> index d6539c12ac..c557ee3c3d 100644
>> --- a/target/loongarch/kvm/kvm.c
>> +++ b/target/loongarch/kvm/kvm.c
>> @@ -816,7 +816,6 @@ int kvm_arch_get_registers(CPUState *cs, Error **errp)
>>   int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
>>   {
>>       int ret;
>> -    static int once;
>>         ret = kvm_loongarch_put_regs_core(cs);
>>       if (ret) {
>> @@ -843,19 +842,17 @@ int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
>>           return ret;
>>       }
>>   -    if (!once) {
>> +    if (level >= KVM_PUT_FULL_STATE) {
>> +        /*
>> +         * pv_features and steal time are per-vCPU state. Push them on
>> +         * full-state sync so every vCPU gets its own settings; the kernel
>> +         * clears the steal-time guest_addr on KVM_PUT_RESET_STATE.
>> +         */
>>           ret = kvm_set_pv_features(cs);
> pv feature is a little different from steal-time. steal-time guest_addr is created from guest OS, pv feature is created from VMM at beginning. steal-time guest_addr can be set for many times, and there is bit KVM_STEAL_PHYS_VALID checking with steal-time guest_addr, however pv feature can be set only once with existing method.
> 

Hi Bibo,

Thanks for catching this — I hadn't fully considered the double-call path.

KVM_PUT_FULL_STATE fires both at realize (cpu_synchronize_post_init) and
on incoming migration load (cpu_synchronize_all_post_init), so
kvm_set_pv_features() runs twice on the destination.

> Although I do not understand flow of VM migration, with KVM_PUT_FULL_STATE state changing, there are at least two places where this state is set, one is from cpu_common_realizefn() which calls cpu_synchronize_post_init(), the other is qemu_loadvm_state()/qemu_loadvm_state_main() which calls cpu_synchronize_all_post_init().
> 
> It seems that VM will fail to migrate since kvm_set_pv_features is called twice at least here. Do you test VM migration with this patch?

I did test migration (virt-11.2 -> virt-11.2, virt-11.1 -> virt-11.1) and
it passed, but that was on a single host — source and destination computed
the same pv_features. The kernel (kvm_loongarch_cpucfg_set_attr) only
rejects a re-set when the value differs:

    if ((kvm->arch.pv_features & LOONGARCH_PV_FEAT_UPDATED) &&
        ((kvm->arch.pv_features & valid) != val))
        return -EINVAL;

So a cross-host migration where the two sides compute different pv_features
would indeed fail on the second set.

I'll add a per-vCPU guard so the push happens exactly once (at the first
FULL_STATE sync); subsequent syncs are skipped and the destination keeps
advertising the features its own host supports. Does that sound like the
right direction?

Thanks,
Tao

> 
> Regards
> Bibo Mao
>>           if (ret) {
>>               return ret;
>>           }
>> -        once = 1;
>> -    }
>>   -    if (level >= KVM_PUT_FULL_STATE) {
>> -        /*
>> -         * only KVM_PUT_FULL_STATE is required, kvm kernel will clear
>> -         * guest_addr for KVM_PUT_RESET_STATE
>> -         */
>>           ret = kvm_set_stealtime(cs);
>>           if (ret) {
>>               return ret;
>>
>