From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH 11/13] vmx: Use scaled host TSC to calculate TSC offset Date: Thu, 22 Oct 2015 15:19:19 -0400 Message-ID: <562936B7.3030909@oracle.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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20151022171203.GH16418@hzzhang-OptiPlex-9020.sh.intel.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: xen-devel@lists.xen.org, Kevin Tian , Wei Liu , Ian Campbell , Stefano Stabellini , Jun Nakajima , Andrew Cooper , Ian Jackson , Aravind Gopalakrishnan , Jan Beulich , Keir Fraser , Suravee Suthikulpanit List-Id: xen-devel@lists.xenproject.org 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 > >> 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