diff mbox

[v4,01/10] x86/hvm: Scale host TSC when setting/getting guest TSC

Message ID 1453067939-9121-2-git-send-email-haozhong.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Haozhong Zhang Jan. 17, 2016, 9:58 p.m. UTC
The existing hvm_[set|get]_guest_tsc_fixed() calculate the guest TSC by
adding the TSC offset to the host TSC. When the TSC scaling is enabled,
the host TSC should be scaled first. This patch adds the scaling logic
to those two functions.

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v4:
 (addressing Jan Beulich's comments)
 * Remove redundant check of cpu_has_tsc_ratio in svm_scale_tsc().
 * Move check of d->arch.vtsc in svm_scale_tsc() to callers.

 xen/arch/x86/hvm/hvm.c        | 17 +++++++----------
 xen/arch/x86/hvm/svm/svm.c    |  9 +++++++++
 xen/include/asm-x86/hvm/hvm.h |  2 ++
 3 files changed, 18 insertions(+), 10 deletions(-)

Comments

Jan Beulich Jan. 18, 2016, 1:39 p.m. UTC | #1
>>> On 17.01.16 at 22:58, <haozhong.zhang@intel.com> wrote:
> The existing hvm_[set|get]_guest_tsc_fixed() calculate the guest TSC by
> adding the TSC offset to the host TSC. When the TSC scaling is enabled,
> the host TSC should be scaled first. This patch adds the scaling logic
> to those two functions.
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> Changes in v4:
>  (addressing Jan Beulich's comments)
>  * Remove redundant check of cpu_has_tsc_ratio in svm_scale_tsc().

This would better have caused Boris' R-b tag to get dropped, or
at least I don't recall him having offered it to stay despite the
change. Boris - can you confirm one way or the other?

>  * Move check of d->arch.vtsc in svm_scale_tsc() to callers.

No d->arch.vtsc instances appear anywhere in this patch, so the
wording is confusing. Indeed the two callers already check this (or
really sit on suitable "else" paths), so at least the patch is correct.

Jan
Haozhong Zhang Jan. 19, 2016, 12:17 a.m. UTC | #2
On 01/18/16 06:39, Jan Beulich wrote:
> >>> On 17.01.16 at 22:58, <haozhong.zhang@intel.com> wrote:
> > The existing hvm_[set|get]_guest_tsc_fixed() calculate the guest TSC by
> > adding the TSC offset to the host TSC. When the TSC scaling is enabled,
> > the host TSC should be scaled first. This patch adds the scaling logic
> > to those two functions.
> > 
> > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > ---
> > Changes in v4:
> >  (addressing Jan Beulich's comments)
> >  * Remove redundant check of cpu_has_tsc_ratio in svm_scale_tsc().
> 
> This would better have caused Boris' R-b tag to get dropped, or
> at least I don't recall him having offered it to stay despite the
> change. Boris - can you confirm one way or the other?
>
> >  * Move check of d->arch.vtsc in svm_scale_tsc() to callers.
> 
> No d->arch.vtsc instances appear anywhere in this patch, so the
> wording is confusing. Indeed the two callers already check this (or
> really sit on suitable "else" paths), so at least the patch is correct.
>

Sorry for the confusion. It should be "Remove check of d->arch.vtsc in
svm_scale_tsc() as it has been checked by callers."

Haozhong
Boris Ostrovsky Jan. 19, 2016, 2:27 p.m. UTC | #3
On 01/18/2016 08:39 AM, Jan Beulich wrote:
>>>> On 17.01.16 at 22:58, <haozhong.zhang@intel.com> wrote:
>> The existing hvm_[set|get]_guest_tsc_fixed() calculate the guest TSC by
>> adding the TSC offset to the host TSC. When the TSC scaling is enabled,
>> the host TSC should be scaled first. This patch adds the scaling logic
>> to those two functions.
>>
>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> ---
>> Changes in v4:
>>   (addressing Jan Beulich's comments)
>>   * Remove redundant check of cpu_has_tsc_ratio in svm_scale_tsc().
> This would better have caused Boris' R-b tag to get dropped, or
> at least I don't recall him having offered it to stay despite the
> change. Boris - can you confirm one way or the other?

Yes, that's fine, my tag still stands.

-boris

>
>>   * Move check of d->arch.vtsc in svm_scale_tsc() to callers.
> No d->arch.vtsc instances appear anywhere in this patch, so the
> wording is confusing. Indeed the two callers already check this (or
> really sit on suitable "else" paths), so at least the patch is correct.
>
> Jan
>
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 21470ec..3648a44 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -60,6 +60,7 @@ 
 #include <asm/hvm/nestedhvm.h>
 #include <asm/hvm/event.h>
 #include <asm/hvm/vmx/vmx.h>
+#include <asm/hvm/svm/svm.h> /* for cpu_has_tsc_ratio */
 #include <asm/altp2m.h>
 #include <asm/mtrr.h>
 #include <asm/apic.h>
@@ -310,13 +311,11 @@  void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
         tsc = hvm_get_guest_time_fixed(v, at_tsc);
         tsc = gtime_to_gtsc(v->domain, tsc);
     }
-    else if ( at_tsc )
-    {
-        tsc = at_tsc;
-    }
     else
     {
-        tsc = rdtsc();
+        tsc = at_tsc ?: rdtsc();
+        if ( cpu_has_tsc_ratio )
+            tsc = hvm_funcs.scale_tsc(v, tsc);
     }
 
     delta_tsc = guest_tsc - tsc;
@@ -344,13 +343,11 @@  u64 hvm_get_guest_tsc_fixed(struct vcpu *v, uint64_t at_tsc)
         tsc = hvm_get_guest_time_fixed(v, at_tsc);
         tsc = gtime_to_gtsc(v->domain, tsc);
     }
-    else if ( at_tsc )
-    {
-        tsc = at_tsc;
-    }
     else
     {
-        tsc = rdtsc();
+        tsc = at_tsc ?: rdtsc();
+        if ( cpu_has_tsc_ratio )
+            tsc = hvm_funcs.scale_tsc(v, tsc);
     }
 
     return tsc + v->arch.hvm_vcpu.cache_tsc_offset;
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index a66d854..a46bc98 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -804,6 +804,13 @@  static uint64_t scale_tsc(uint64_t host_tsc, uint64_t ratio)
     return scaled_host_tsc;
 }
 
+static uint64_t svm_scale_tsc(const struct vcpu *v, uint64_t tsc)
+{
+    ASSERT(cpu_has_tsc_ratio && !v->domain->arch.vtsc);
+
+    return scale_tsc(tsc, vcpu_tsc_ratio(v));
+}
+
 static uint64_t svm_get_tsc_offset(uint64_t host_tsc, uint64_t guest_tsc,
     uint64_t ratio)
 {
@@ -2272,6 +2279,8 @@  static struct hvm_function_table __initdata svm_function_table = {
     .nhvm_vmcx_hap_enabled = nsvm_vmcb_hap_enabled,
     .nhvm_intr_blocked = nsvm_intr_blocked,
     .nhvm_hap_walk_L1_p2m = nsvm_hap_walk_L1_p2m,
+
+    .scale_tsc            = svm_scale_tsc,
 };
 
 void svm_vmexit_handler(struct cpu_user_regs *regs)
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index b9d893d..a87224b 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -212,6 +212,8 @@  struct hvm_function_table {
     void (*altp2m_vcpu_update_vmfunc_ve)(struct vcpu *v);
     bool_t (*altp2m_vcpu_emulate_ve)(struct vcpu *v);
     int (*altp2m_vcpu_emulate_vmfunc)(struct cpu_user_regs *regs);
+
+    uint64_t (*scale_tsc)(const struct vcpu *v, uint64_t tsc);
 };
 
 extern struct hvm_function_table hvm_funcs;