Re: [PATCH] KVM: x86: pvclock: Handle first-time write to pvclock-page contains random junk

[Liran Alon <LIRAN.ALON@ORACLE.COM>]

On 13/11/17 02:40, Wanpeng Li wrote:
> 2017-11-05 22:11 GMT+08:00 Liran Alon <liran.alon@oracle.com>:
>> When guest passes KVM it's pvclock-page GPA via WRMSR to
>> MSR_KVM_SYSTEM_TIME / MSR_KVM_SYSTEM_TIME_NEW, KVM don't initialize
>> pvclock-page to some start-values. It just requests a clock-update which
>
> KVM does, please refer to memset(hv_clock, 0, size) in kvmclock_init().

kvmclock_init() is the code that runs in KVM-guest. I was talking here 
about the code that handles the WRMSR in KVM hypervisor code.

The issue happens when the guest doesn't init pvclock-page as done in 
kvmclock_init(). Not all guests behave nicely :)

-Liran
>
> Regards,
> Wanpeng Li
>
>> will happen before entering to guest.
>>
>> The clock-update logic will call kvm_setup_pvclock_page() to update the
>> pvclock-page with info. However, kvm_setup_pvclock_page() *wrongly*
>> assumes that the version-field is initialized to an even number. This is
>> wrong because at first-time write, field could be any-value.
>>
>> Fix simply makes sure that if first-time version-field is odd, increment
>> it once more to make it even and only then start standard logic.
>> This follows same logic as done in other pvclock shared-pages (See
>> kvm_write_wall_clock() and record_steal_time()).
>>
>> Signed-off-by: Liran Alon <liran.alon@oracle.com>
>> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
>> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>>   arch/x86/kvm/x86.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 03869eb7fcd6..181106080e41 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -1830,6 +1830,9 @@ static void kvm_setup_pvclock_page(struct kvm_vcpu *v)
>>           */
>>          BUILD_BUG_ON(offsetof(struct pvclock_vcpu_time_info, version) != 0);
>>
>> +       if (guest_hv_clock.version & 1)
>> +               ++guest_hv_clock.version;  /* first time write, random junk */
>> +
>>          vcpu->hv_clock.version = guest_hv_clock.version + 1;
>>          kvm_write_guest_cached(v->kvm, &vcpu->pv_time,
>>                                  &vcpu->hv_clock,
>> --
>> 1.9.1
>>