Re: [PATCH 3/4] KVM: x86: hyper-v: Track Hyper-V TSC page status

[Sean Christopherson <seanjc@google.com>]

On Mon, Mar 15, 2021, Vitaly Kuznetsov wrote:
> Create an infrastructure for tracking Hyper-V TSC page status, i.e. if it
> was updated from guest/host side or if we've failed to set it up (because
> e.g. guest wrote some garbage to HV_X64_MSR_REFERENCE_TSC) and there's no
> need to retry.
> 
> Also, in a hypothetical situation when we are in 'always catchup' mode for
> TSC we can now avoid contending 'hv->hv_lock' on every guest enter by
> setting the state to HV_TSC_PAGE_BROKEN after compute_tsc_page_parameters()
> returns false.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index eefb85b86fe8..2a8d078b16cb 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1087,7 +1087,7 @@ void kvm_hv_setup_tsc_page(struct kvm *kvm,
>  	BUILD_BUG_ON(sizeof(tsc_seq) != sizeof(hv->tsc_ref.tsc_sequence));
>  	BUILD_BUG_ON(offsetof(struct ms_hyperv_tsc_page, tsc_sequence) != 0);
>  
> -	if (!(hv->hv_tsc_page & HV_X64_MSR_TSC_REFERENCE_ENABLE))
> +	if (hv->hv_tsc_page_status == HV_TSC_PAGE_BROKEN)
>  		return;
>  
>  	mutex_lock(&hv->hv_lock);

...

> @@ -1133,6 +1133,12 @@ void kvm_hv_setup_tsc_page(struct kvm *kvm,
>  	hv->tsc_ref.tsc_sequence = tsc_seq;
>  	kvm_write_guest(kvm, gfn_to_gpa(gfn),
>  			&hv->tsc_ref, sizeof(hv->tsc_ref.tsc_sequence));
> +
> +	hv->hv_tsc_page_status = HV_TSC_PAGE_SET;
> +	goto out_unlock;
> +
> +out_err:
> +	hv->hv_tsc_page_status = HV_TSC_PAGE_BROKEN;
>  out_unlock:
>  	mutex_unlock(&hv->hv_lock);
>  }
> @@ -1193,8 +1199,13 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data,
>  	}
>  	case HV_X64_MSR_REFERENCE_TSC:
>  		hv->hv_tsc_page = data;
> -		if (hv->hv_tsc_page & HV_X64_MSR_TSC_REFERENCE_ENABLE)
> +		if (hv->hv_tsc_page & HV_X64_MSR_TSC_REFERENCE_ENABLE) {
> +			if (!host)
> +				hv->hv_tsc_page_status = HV_TSC_PAGE_GUEST_CHANGED;
> +			else
> +				hv->hv_tsc_page_status = HV_TSC_PAGE_HOST_CHANGED;

Writing the status without taking hv->hv_lock could cause the update to be lost,
e.g. if a different vCPU fails kvm_hv_setup_tsc_page() at the same time, its
write to set status to HV_TSC_PAGE_BROKEN would race with this write.

>  			kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);
> +		}
>  		break;
>  	case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
>  		return kvm_hv_msr_set_crash_data(kvm,
> -- 
> 2.30.2
>