Re: [PATCH] KVM:Hyper-V reduce one kvm_write_guest operation

[Paolo Bonzini]

On 20/12/2017 08:46, rhett wrote:
> in function kvm_hv_setup_tsc_page , the old code write the full tsc_ref
> struct firstly, and write a
> tsc_sequence field later, it can be wirten once.

No, it cannot and this comment says exactly why:

> - /* Ensure sequence is zero before writing the rest of the struct. */
> -ÂÂÂÂÂÂ smp_wmb();
> -ÂÂÂÂÂÂ if (kvm_write_guest(kvm, gfn_to_gpa(gfn), &hv->tsc_ref,
> sizeof(hv->tsc_ref)))
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ goto out_unlock;
> -
> ÂÂÂÂÂÂÂ /*
> ÂÂÂÂÂÂÂÂ * Now switch to the TSC page mechanism by writing the sequence.
> ÂÂÂÂÂÂÂÂ */

The sequence is: disable TSC page, write TSC parameters, enable TSC
page.  If the guest can read a partially-written TSC page, it can return
a wrong time.

Paolo

> @@ -922,7 +917,7 @@ 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->tsc_ref, sizeof(hv->tsc_ref));
> Âout_unlock:
> ÂÂÂÂÂÂÂ mutex_unlock(&kvm->arch.hyperv.hv_lock);
> Â}
> -- 
> 1.8.3.1
>