Re: [BUG] Migration hv_time rollback

[Vitaly Kuznetsov]

Vitaly Kuznetsov <vkuznets@redhat.com> writes:

> Antoine Damhet <antoine.damhet@blade-group.com> writes:
>
>> On Wed, Sep 16, 2020 at 12:29:56PM +0100, Dr. David Alan Gilbert wrote:
>>> cc'ing in Vitaly who knows about the hv stuff.
>>
>> Thanks
>>
>>> 
>>> * Antoine Damhet (antoine.damhet@blade-group.com) wrote:
>>> > Hi,
>>> > 
>>> > We are experiencing timestamp rollbacks during live-migration of
>>> > Windows 10 guests with the following qemu configuration (linux 5.4.46
>>> > and qemu master):
>>> > ```
>>> > $ qemu-system-x86_64 -enable-kvm -cpu host,kvm=off,hv_time [...]
>>> > ```
>>> 
>>> How big a jump are you seeing, and how did you notice it in the guest?
>>
>> I'm seeing jumps of about the guest uptime (indicating a reset of the
>> counter). It's expected because we won't call `KVM_SET_CLOCK` to
>> restore any value.
>>
>> We first noticed it because after some migrations `dwm.exe` crashes with
>> the "(NTSTATUS) 0x8898009b - QueryPerformanceCounter returned a time in
>> the past." error code.
>>
>> I can also confirm the following hack makes the behavior disappear:
>>
>> ```
>> diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
>> index 64283358f9..f334bdf35f 100644
>> --- a/hw/i386/kvm/clock.c
>> +++ b/hw/i386/kvm/clock.c
>> @@ -332,11 +332,7 @@ void kvmclock_create(void)
>>  {
>>      X86CPU *cpu = X86_CPU(first_cpu);
>>
>> -    if (kvm_enabled() &&
>> -        cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
>> -                                       (1ULL << KVM_FEATURE_CLOCKSOURCE2))) 
>> {
>> -        sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL);
>> -    }
>> +    sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL);
>>  }
>>
>
>
> Oh, I think I see what's going on. When you add 'kvm=off'
> cpu->env.features[FEAT_KVM] is reset (see x86_cpu_expand_features()) so
> kvmclock QEMU device is not created and nobody calls KVM_SET_CLOCK on
> migration.
>
> In case we really want to support 'kvm=off' I think we can add Hyper-V
> features check here along with KVM, this should do the job.

Does the untested

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 64283358f91d..e03b2ca6d8f6 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -333,8 +333,9 @@ void kvmclock_create(void)
     X86CPU *cpu = X86_CPU(first_cpu);
 
     if (kvm_enabled() &&
-        cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
-                                       (1ULL << KVM_FEATURE_CLOCKSOURCE2))) {
+        ((cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
+                                         (1ULL << KVM_FEATURE_CLOCKSOURCE2))) 
||
+         (cpu->env.features[FEAT_HYPERV_EAX] & HV_TIME_REF_COUNT_AVAILABLE))) {
         sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL);
     }
 }

help?

(I don't think we need to remove all 'if (kvm_enabled())' checks from
machine types as 'kvm=off' should not be related).

-- 
Vitaly