From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754691AbdLTJB1 (ORCPT ); Wed, 20 Dec 2017 04:01:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54684 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752128AbdLTJBU (ORCPT ); Wed, 20 Dec 2017 04:01:20 -0500 Subject: Re: [PATCH] KVM:Hyper-V reduce one kvm_write_guest operation To: rhett , rkrcmar@redhat.com, tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org References: From: Paolo Bonzini Message-ID: <1c25be48-32a2-4ec3-d396-a52cbda568e2@redhat.com> Date: Wed, 20 Dec 2017 10:01:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Wed, 20 Dec 2017 09:01:19 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >