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

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
> 

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

[Paolo Bonzini]

On 20/12/2017 10:33, Rhett Rhett wrote:
> tsc_sequence  had been set to 0 before,  so the tsc page won't function.
> guest has no chance to read tsc page before the last write.
> 

kvm_write_guest is not atomic.  The guest can see a non-zero sequence
before the other fields have been set.

Paolo