From mboxrd@z Thu Jan 1 00:00:00 1970 From: Haozhong Zhang Subject: Re: [PATCH 11/13] vmx: Use scaled host TSC to calculate TSC offset Date: Fri, 23 Oct 2015 08:52:33 +0800 Message-ID: <20151023005233.GJ16418@hzzhang-OptiPlex-9020.sh.intel.com> References: <1443424438-13404-1-git-send-email-haozhong.zhang@intel.com> <1443424438-13404-12-git-send-email-haozhong.zhang@intel.com> <562906D4.3090700@oracle.com> <20151022171203.GH16418@hzzhang-OptiPlex-9020.sh.intel.com> <562936B7.3030909@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <562936B7.3030909@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky Cc: Kevin Tian , Wei Liu , Ian Campbell , Stefano Stabellini , Andrew Cooper , Ian Jackson , xen-devel@lists.xen.org, Jan Beulich , Aravind Gopalakrishnan , Jun Nakajima , Keir Fraser , Suravee Suthikulpanit List-Id: xen-devel@lists.xenproject.org On Thu, Oct 22, 2015 at 03:19:19PM -0400, Boris Ostrovsky wrote: > On 10/22/2015 01:12 PM, Haozhong Zhang wrote: > >On Thu, Oct 22, 2015 at 11:55:00AM -0400, Boris Ostrovsky wrote: > >>On 09/28/2015 03:13 AM, Haozhong Zhang wrote: > >>>If VMX TSC scaling is enabled and no TSC emulation is used, > >>>vmx_set_tsc_offset() will calculate the TSC offset by substracting the > >>>scaled host TSC from the current guest TSC. > >>> > >>>Signed-off-by: Haozhong Zhang > >>>--- > >>> xen/arch/x86/hvm/vmx/vmx.c | 15 +++++++++++++++ > >>> 1 file changed, 15 insertions(+) > >>> > >>>diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > >>>index 454440e..163974d 100644 > >>>--- a/xen/arch/x86/hvm/vmx/vmx.c > >>>+++ b/xen/arch/x86/hvm/vmx/vmx.c > >>>@@ -1102,11 +1102,26 @@ static void vmx_handle_cd(struct vcpu *v, unsigned long value) > >>> static void vmx_set_tsc_offset(struct vcpu *v, u64 offset, u64 at_tsc) > >>> { > >>>+ uint64_t host_tsc, guest_tsc; > >>>+ struct domain *d = v->domain; > >>>+ > >>>+ guest_tsc = hvm_get_guest_tsc_fixed(v, at_tsc); > >>>+ > >>>+ if ( cpu_has_vmx_tsc_scaling && !d->arch.vtsc ) > >>>+ { > >>>+ host_tsc = at_tsc ? at_tsc : rdtsc(); > >>>+ offset = guest_tsc - hvm_scale_tsc(v, host_tsc); > >>>+ } > >>>+ > >>> vmx_vmcs_enter(v); > >>>+ if ( !nestedhvm_enabled(d) ) > >>>+ goto out; > >>>+ > >>> if ( nestedhvm_vcpu_in_guestmode(v) ) > >>> offset += nvmx_get_tsc_offset(v); > >>>+out: > >>> __vmwrite(TSC_OFFSET, offset); > >>> vmx_vmcs_exit(v); > >>> } > >> > >>This (and corresponding SVM code) looks somewhat suspect to me: if the > >>processor supports scaling we are ignoring caller-provided offset. > >> > >Yes, if TSC scaling is available, [svm|vmx]_set_tsc_offset() do not > >trust the offset from callers and calculate a scaled version by itself > >instead. > > > >The original svm_set_tsc_offset() did so and I keep its semantics > >here. > > > >Maybe moving the scaling logic out of [svm|vmx]_set_tsc_offset() could > >make the semantics more clear. > > > Yes: since scaling is now architectural, offsets are (or at least can be) > calculated correctly at HVM level and so arch-specific handlers should not > have to do this. In case of hvm_set_guest_tsc_fixed() we definitely doing it > twice. > > -boris > I'll move the scaling logic out of [svm|vmx]_set_tsc_offset() in the next version. > > > >>Besides, at least when called from hvm_set_guest_tsc_fixed() --- we've > >>already taken scaling into account, that's what patch 6 does, doesn't it? > >> > >Yes. > > > >>-boris > >> > >>_______________________________________________ > >>Xen-devel mailing list > >>Xen-devel@lists.xen.org > >>http://lists.xen.org/xen-devel > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel