Re: [PATCH 3/3] x86/kvm: implement Hyper-V reference TSC page clock

["Radim Krčmář" <rkrcmar@redhat.com>]

2016-04-21 20:11+0300, Roman Kagan:
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> @@ -797,23 +798,11 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data,
>  		mark_page_dirty(kvm, gfn);
>  		break;
>  	}
> +	case HV_X64_MSR_REFERENCE_TSC:

(Would be nicer to check for HV_X64_MSR_REFERENCE_TSC_AVAILABLE.)

>  		hv->hv_tsc_page = data;
> +		if (hv->hv_tsc_page & HV_X64_MSR_TSC_REFERENCE_ENABLE)
> +			kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);

The MSR value is global and will be seen by other VCPUs before we write
the page for the first time, which means there is an extremely unlikely
race that could read random data from a guest page and interpret it as
time.  Initialization before setting hv_tsc_page would be fine.

(Also, TLFS 4.0b says that the guest can pick any frame in the GPA
 space.  The guest could specify a frame that wouldn't be mapped in KVM
 and the guest would fail for no good reason.  HyperV's "overlay pages"
 likely don't read or overwrite content of mapped frames either.
 I think it would be safer to create a new mapping for the page ...)

> @@ -1143,3 +1132,107 @@ set_result:
> +static int pvclock_to_tscpage(struct pvclock_vcpu_time_info *hv_clock,
> +			       HV_REFERENCE_TSC_PAGE *tsc_ref)
> +{
| [...]
> +	 * tsc_scale = (tsc_to_system_mul << (tsc_shift + 32)) / 100
| [...]
> +	 * Note that although tsc_to_system_mul is 32 bit, we may need 128 bit
> +	 * division to calculate tsc_scale

Linux has a function that handles u96/u64 with 64 and even 32 bit
arithmetics, mul_u64_u32_div().  Do you think this would be ok?

  tsc_ref->tsc_scale = mul_u64_u32_div(1ULL << hv_clock->tsc_shift + 32,
                                       tsc_to_system_mul, 100);

Thanks.