Re: [PATCH v6 21/51] x86/kvm: Obtain TSC frequency from PV CPUID if present

On 8/18/26 01:25, Jim Mattson wrote:
> On Mon, Aug 17, 2026 at 3:20 PM Maksim Davydov
> <[email protected]> wrote:
>>
>>
>>
>> On 8/17/26 17:08, Sean Christopherson wrote:
>>> On Mon, Aug 17, 2026, Maksim Davydov wrote:
>>>> On 8/7/26 02:35, Sean Christopherson wrote:
>>>>> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
>>>>> index 29ca37e9a3bc..f55d0305d1f3 100644
>>>>> --- a/arch/x86/kernel/kvmclock.c
>>>>> +++ b/arch/x86/kernel/kvmclock.c
>>>>> @@ -342,8 +342,10 @@ void __init kvmclock_init(void)
>>>>>     flags = pvclock_read_flags(&hv_clock_boot[0].pvti);
>>>>>     kvm_sched_clock_init(flags & PVCLOCK_TSC_STABLE_BIT);
>>>>>
>>>>> -   x86_init.hyper.get_tsc_khz = kvmclock_get_tsc_khz;
>>>>> -   x86_init.hyper.get_cpu_khz = kvmclock_get_tsc_khz;
>>>>> +   if (!x86_init.hyper.get_tsc_khz)
>>>>> +           x86_init.hyper.get_tsc_khz = kvmclock_get_tsc_khz;
>>>>> +   if (!x86_init.hyper.get_cpu_khz)
>>>>> +           x86_init.hyper.get_cpu_khz = kvmclock_get_tsc_khz;
>>>>>     x86_platform.get_wallclock = kvm_get_wallclock;
>>>>>     x86_platform.set_wallclock = kvm_set_wallclock;
>>>>>  #ifdef CONFIG_X86_LOCAL_APIC
>>>>
>>>>
>>>> I cannot test this right now as I lack two servers with different CPU
>>>> base frequencies, but it seems that this patch might break something in
>>>> guests:
>>>> After migrating a VM (QEMU + KVM) from a host with one base frequency to
>>>> another host with a different base frequency, the value in CPUID leaf
>>>> 0x40000010 EAX changes and becomes the same as the destination host base
>>>> frequency instead of remaining the source base frequency.
>>>
>>> That's a bug in whatever is orchestrating the migration, and/or QEMU if QEMU is
>>> handing the upper layers a loaded footgun.
>>>
>>>> The main reason for this behaviour is that setting the TSC frequency via
>>>> ioctl(KVM_SET_TSC_KHZ) doesn't change the value in CPUID leaf 0x40000010
>>>> EAX and these two entities are still not connected.
>>>
>>> And they never will be.  It's userspace's responsibility to fill the correct
>>> values for 0x40000010.
>>>
>>> But AFAICT, QEMU does the right thing.  env->tsc_khz is used for both the CPUID
>>> leaf and for KVM_SET_TSC_KHZ.
>>>
>>> kvm_arch_init_vcpu():
>>>
>>>         c = &cpuid_data.entries[cpuid_i++];
>>>         c->function = KVM_CPUID_SIGNATURE | 0x10;
>>>         c->eax = env->tsc_khz;
>>>         c->ebx = env->apic_bus_freq / 1000; /* Hz to KHz */
>>>         c->ecx = c->edx = 0;
>>>
>>> kvm_arch_set_tsc_khz():
>>>
>>>     r = set_ioctl ?
>>>         kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) :
>>>         -ENOTSUP;
>>>
>>
>> Agreed.
>> However, kvm_arch_set_tsc_khz() is also used at the end of migration
>> (qemu_loadvm_state -> cpu_synchronize_post_init).
>> So, the new value of env->tsc_khz from the source host can be loaded
>> during migration via KVM_SET_TSC_KHZ. However, the frequency in CPUID
>> leaf 0x40000010 remains the same as on the destination host (the TSC
>> frequency can differ from the source host's frequency). Thus, it's
>> possible to have unsynchronized CPUID leaf 0x40000010 when using default
>> QEMU parameters.
> 
> I don't use qemu, but isn't the qemu solution for this general
> situation to mark leaf 40000010H as 'unmigratable'?

I don't think this leaf should be unmigratable by default. I think the
same logic as for `invtsc` can be implemented:
If the TSC frequency is explicitly specified via `-cpu`, CPUID leaf
0x40000010 is migratable because `env->tsc_khz` during
`kvm_arch_init_vcpu()` on the destination host will be the same as on
the source host. But if the TSC frequency isn't specified, CPUID leaf
0x40000010 has to be omitted from `KVM_SET_CPUID2`.