Re: [PATCH 2/4] KVM: x86: hyper-v: Prevent using not-yet-updated TSC page by secondary CPUs

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 15/03/21 15:37, Vitaly Kuznetsov wrote:
>> When KVM_REQ_MASTERCLOCK_UPDATE request is issued (e.g. after migration)
>> we need to make sure no vCPU sees stale values in PV clock structures and
>> thus all vCPUs are kicked with KVM_REQ_CLOCK_UPDATE. Hyper-V TSC page
>> clocksource is global and kvm_guest_time_update() only updates in on vCPU0
>> but this is not entirely correct: nothing blocks some other vCPU from
>> entering the guest before we finish the update on CPU0 and it can read
>> stale values from the page.
>> 
>> Call kvm_hv_setup_tsc_page() on all vCPUs. Normally, KVM_REQ_CLOCK_UPDATE
>> should be very rare so we may not care much about being wasteful.
>
> I think we should instead write 0 to the page in kvm_gen_update_masterclock.
>

We can do that but we will also need to invalidate
hv->tsc_ref.tsc_sequence to prevent MSR based clocksource
(HV_X64_MSR_TIME_REF_COUNT -> get_time_ref_counter()) from using stale
hv->tsc_ref.tsc_scale/tsc_offset values (in case we had them
calculated).

Also, we can't really disable TSC page for nested scenario when guest
opted for reenlightenment (PATCH4) but we're not going to update the
page anyway so there's not much different.

> Paolo
>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>   arch/x86/kvm/x86.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 47e021bdcc94..882c509bfc86 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -2748,8 +2748,9 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
>>   				       offsetof(struct compat_vcpu_info, time));
>>   	if (vcpu->xen.vcpu_time_info_set)
>>   		kvm_setup_pvclock_page(v, &vcpu->xen.vcpu_time_info_cache, 0);
>> -	if (v == kvm_get_vcpu(v->kvm, 0))
>> -		kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
>> +
>> +	kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
>> +
>>   	return 0;
>>   }
>>   
>> 
>

-- 
Vitaly