All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/6] Add VMX TSC scaling support
@ 2016-02-23  2:04 Haozhong Zhang
  2016-02-23  2:04 ` [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio Haozhong Zhang
                   ` (5 more replies)
  0 siblings, 6 replies; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-23  2:04 UTC (permalink / raw)
  To: xen-devel, Jan Beulich, Boris Ostrovsky, Kevin Tian
  Cc: Haozhong Zhang, Keir Fraser, Suravee Suthikulpanit,
	Andrew Cooper, Aravind Gopalakrishnan, Jun Nakajima

This patchset adds support for VMX TSC scaling feature which is
available on Intel Skylake Server CPU. The specification of VMX TSC
scaling can be found at
http://www.intel.com/content/www/us/en/processors/timestamp-counter-scaling-virtualization-white-paper.html

VMX TSC scaling allows guest TSC which is read by guest rdtsc(p)
instructions increases in a rate that is customized by the hypervisor
and can be different than the host TSC frequency. Basically, VMX TSC
scaling adds a 64-bit field called TSC multiplier in VMCS so that, if
VMX TSC scaling is enabled, TSC read by guest rdtsc(p) instructions
will be calculated by the following formula:

  guest EDX:EAX = (Host TSC * TSC multiplier) >> 48 + VMX TSC Offset

where, Host TSC = Host MSR_IA32_TSC + Host MSR_IA32_TSC_ADJUST.

If the destination host supports VMX TSC scaling, this patchset allows
guest programs in a HVM container in the default TSC mode or PVRDTSCP
(native_paravirt) TSC mode to observe the same TSC frequency across
the migration.

Changes in v5:
 * v4 patch 1 - 3 have been committed so they are not included in v5.
 * v4 patch 5 "x86: Add functions for 64-bit integer arithmetic" is
   removed in v5. All math64 functions are inlined in v5.
 * v5 patch 1 corresponds to v4 patch 4.
   v5 patch 2 - 6 correspond to v4 patch 6 - 10.
 * Other changes are logged in each patch respectively.
 * Previous R-b and A-b (from Boris Ostrovsky, Jan Beulich and Kevin
   Tian) for all patches except patch 4 are removed because of above
   changes.

Changes in v4:
 * v3 patch 1&2 have been committed so they are not included in v4.
 * v3 patch 11 "x86/hvm: Detect TSC scaling through hvm_funcs" is merged
   early into v4 patch 4 "x86/hvm: Collect information of TSC scaling ratio".
 * v4 patch 1 - 8 correspond to v3 patch 3 - 10.
   v4 patch 9 - 10 correspond to v3 patch 12 - 13.
 * Other changes are logged in each patch respectively.

Changes in v3:
 * v2 patch 1&2 have been merged so they do not appear in v3.
 * Patch 1 - 6 correspond to v2 patch 3 - 8. Patch 7 is new.
   Patch 8 - 13 correspond to v2 patch 9 - 14.
 * Other changes are logged in each patch respectively.

Changes in v2:
 * Remove unnecessary v1 patch 1&13.
 * Add and move all bug-fix patches to the beginning of this series.
   (Patch 1 - 6)
 * Update changes in tsc_set_info() and tsc_get_info() to make both
   functions consistent with each other. (Patch 2 - 4)
 * Move a part of scaling logic out of [vmx|svm]_set_tsc_offset().
   (Patch 7)
 * Remove redundant hvm_funcs.tsc_scaling_ratio_rsvd. (Patch 8)
 * Reimplement functions that calculate TSC ratio and scale TSC.
   (Patch 9&10)
 * Merge setting VMX TSC multiplier into patch 13.
 * Move initialing tsc_scaling_ratio in VMX ahead to
   vmx_vcpu_initialise() so as to make construct_vmcs() naturally
   use this field instead of a constant. (Patch 13)
 * Update documents related to tsc_mode.
 * Other code cleanup and style fixes.

Haozhong Zhang (6):
  x86/hvm: Collect information of TSC scaling ratio
  x86/hvm: Setup TSC scaling ratio
  x86/hvm: Replace architecture TSC scaling by a common function
  x86/hvm: Move saving/loading vcpu's TSC to common code
  vmx: Add VMX RDTSC(P) scaling support
  docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt

 docs/man/xl.cfg.pod.5              | 14 ++++++++-
 docs/misc/tscmode.txt              | 21 +++++++++++++
 xen/arch/x86/hvm/hvm.c             | 62 +++++++++++++++++++++++++++++++++++---
 xen/arch/x86/hvm/svm/svm.c         | 21 +++++--------
 xen/arch/x86/hvm/vmx/vmcs.c        | 12 ++++++--
 xen/arch/x86/hvm/vmx/vmx.c         | 22 +++++++++++---
 xen/arch/x86/time.c                | 17 +++++++----
 xen/include/asm-x86/hvm/domain.h   |  2 ++
 xen/include/asm-x86/hvm/hvm.h      | 26 +++++++++++++++-
 xen/include/asm-x86/hvm/svm/svm.h  |  3 --
 xen/include/asm-x86/hvm/vmx/vmcs.h |  7 +++++
 11 files changed, 169 insertions(+), 38 deletions(-)

-- 
2.7.1

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-23  2:04 [PATCH v5 0/6] Add VMX TSC scaling support Haozhong Zhang
@ 2016-02-23  2:04 ` Haozhong Zhang
  2016-02-23 14:00   ` Boris Ostrovsky
                     ` (2 more replies)
  2016-02-23  2:05 ` [PATCH v5 2/6] x86/hvm: Setup " Haozhong Zhang
                   ` (4 subsequent siblings)
  5 siblings, 3 replies; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-23  2:04 UTC (permalink / raw)
  To: xen-devel, Jan Beulich, Boris Ostrovsky, Kevin Tian
  Cc: Haozhong Zhang, Keir Fraser, Suravee Suthikulpanit,
	Andrew Cooper, Aravind Gopalakrishnan, Jun Nakajima

Both VMX TSC scaling and SVM TSC ratio use the 64-bit TSC scaling ratio,
but the number of fractional bits of the ratio is different between VMX
and SVM. This patch adds the architecture code to collect the number of
fractional bits and other related information into fields of struct
hvm_function_table so that they can be used in the common code.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v5:
 * Drop "Reviewed-by" from Kevin Tian and Boris Ostrovsky because
   v4 and v5 are changed quite a lot.
 * Put all TSC scaling parameters and callbacks in a sub-structure 
   tsc_scaling in hvm_function_table.
 * Remove the redundant TSC scaling parameter for default TSC scaling
   ratio, which can be derived from the number of fractional bits of
   TSC scaling ratio.
 * Initialize all TSC scaling parameters and callbacks statically,
   except ratio_frac_bits that is initialized to a non-zero value
   if TSC scaling is available.
---
 xen/arch/x86/hvm/hvm.c        |  9 ++++-----
 xen/arch/x86/hvm/svm/svm.c    |  8 +++++++-
 xen/arch/x86/time.c           | 12 ++++++------
 xen/include/asm-x86/hvm/hvm.h | 16 +++++++++++++++-
 4 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index fe382ce..a72a6e7 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -60,7 +60,6 @@
 #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>
@@ -312,8 +311,8 @@ void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
     else
     {
         tsc = at_tsc ?: rdtsc();
-        if ( cpu_has_tsc_ratio )
-            tsc = hvm_funcs.scale_tsc(v, tsc);
+        if ( hvm_tsc_scaling_supported )
+            tsc = hvm_funcs.tsc_scaling.scale_tsc(v, tsc);
     }
 
     delta_tsc = guest_tsc - tsc;
@@ -344,8 +343,8 @@ u64 hvm_get_guest_tsc_fixed(struct vcpu *v, uint64_t at_tsc)
     else
     {
         tsc = at_tsc ?: rdtsc();
-        if ( cpu_has_tsc_ratio )
-            tsc = hvm_funcs.scale_tsc(v, tsc);
+        if ( hvm_tsc_scaling_supported )
+            tsc = hvm_funcs.tsc_scaling.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 e62dfa1..b22d4a1 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1465,6 +1465,9 @@ const struct hvm_function_table * __init start_svm(void)
     if ( !cpu_has_svm_nrips )
         clear_bit(SVM_FEATURE_DECODEASSISTS, &svm_feature_flags);
 
+    if ( cpu_has_tsc_ratio )
+        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
+
 #define P(p,s) if ( p ) { printk(" - %s\n", s); printed = 1; }
     P(cpu_has_svm_npt, "Nested Page Tables (NPT)");
     P(cpu_has_svm_lbrv, "Last Branch Record (LBR) Virtualisation");
@@ -2286,7 +2289,10 @@ static struct hvm_function_table __initdata svm_function_table = {
     .nhvm_intr_blocked = nsvm_intr_blocked,
     .nhvm_hap_walk_L1_p2m = nsvm_hap_walk_L1_p2m,
 
-    .scale_tsc            = svm_scale_tsc,
+    .tsc_scaling = {
+        .max_ratio = ~TSC_RATIO_RSVD_BITS,
+        .scale_tsc = svm_scale_tsc,
+    },
 };
 
 void svm_vmexit_handler(struct cpu_user_regs *regs)
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 5819b9f..2248dfa 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -37,7 +37,6 @@
 #include <asm/hpet.h>
 #include <io_ports.h>
 #include <asm/setup.h> /* for early_time_init */
-#include <asm/hvm/svm/svm.h> /* for cpu_has_tsc_ratio */
 #include <public/arch-x86/cpuid.h>
 
 /* opt_clocksource: Force clocksource to one of: pit, hpet, acpi. */
@@ -815,9 +814,10 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
     }
     else
     {
-        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
+        if ( has_hvm_container_domain(d) && hvm_tsc_scaling_supported )
         {
-            tsc_stamp            = hvm_funcs.scale_tsc(v, t->local_tsc_stamp);
+            tsc_stamp            =
+                hvm_funcs.tsc_scaling.scale_tsc(v, t->local_tsc_stamp);
             _u.tsc_to_system_mul = d->arch.vtsc_to_ns.mul_frac;
             _u.tsc_shift         = d->arch.vtsc_to_ns.shift;
         }
@@ -1758,7 +1758,7 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
                   uint32_t *incarnation)
 {
     bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
-                                cpu_has_tsc_ratio && !d->arch.vtsc;
+                                hvm_tsc_scaling_supported && !d->arch.vtsc;
 
     *incarnation = d->arch.incarnation;
     *tsc_mode = d->arch.tsc_mode;
@@ -1865,7 +1865,7 @@ void tsc_set_info(struct domain *d,
          */
         if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
              (has_hvm_container_domain(d) ?
-              d->arch.tsc_khz == cpu_khz || cpu_has_tsc_ratio :
+              d->arch.tsc_khz == cpu_khz || hvm_tsc_scaling_supported :
               incarnation == 0) )
         {
     case TSC_MODE_NEVER_EMULATE:
@@ -1879,7 +1879,7 @@ void tsc_set_info(struct domain *d,
         d->arch.vtsc = !boot_cpu_has(X86_FEATURE_RDTSCP) ||
                        !host_tsc_is_safe();
         enable_tsc_scaling = has_hvm_container_domain(d) &&
-                             cpu_has_tsc_ratio && !d->arch.vtsc;
+                             hvm_tsc_scaling_supported && !d->arch.vtsc;
         d->arch.tsc_khz = (enable_tsc_scaling && gtsc_khz) ? gtsc_khz : cpu_khz;
         set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 );
         d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 0b15616..7e87b10 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -222,7 +222,18 @@ struct hvm_function_table {
     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);
+    /*
+     * Parameters and callbacks for hardware-assisted TSC scaling,
+     * which are valid only when the hardware feature is available.
+     */
+    struct {
+        /* number of bits of the fractional part of TSC scaling ratio */
+        uint8_t  ratio_frac_bits;
+        /* maximum-allowed TSC scaling ratio */
+        uint64_t max_ratio;
+
+        uint64_t (*scale_tsc)(const struct vcpu *v, uint64_t tsc);
+    } tsc_scaling;
 };
 
 extern struct hvm_function_table hvm_funcs;
@@ -258,6 +269,9 @@ void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc);
 u64 hvm_get_guest_tsc_fixed(struct vcpu *v, u64 at_tsc);
 #define hvm_get_guest_tsc(v) hvm_get_guest_tsc_fixed(v, 0)
 
+#define hvm_tsc_scaling_supported \
+    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
+
 int hvm_set_mode(struct vcpu *v, int mode);
 void hvm_init_guest_time(struct domain *d);
 void hvm_set_guest_time(struct vcpu *v, u64 guest_time);
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 2/6] x86/hvm: Setup TSC scaling ratio
  2016-02-23  2:04 [PATCH v5 0/6] Add VMX TSC scaling support Haozhong Zhang
  2016-02-23  2:04 ` [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio Haozhong Zhang
@ 2016-02-23  2:05 ` Haozhong Zhang
  2016-02-24 15:01   ` Jan Beulich
  2016-02-23  2:05 ` [PATCH v5 3/6] x86/hvm: Replace architecture TSC scaling by a common function Haozhong Zhang
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-23  2:05 UTC (permalink / raw)
  To: xen-devel, Jan Beulich, Boris Ostrovsky, Kevin Tian
  Cc: Haozhong Zhang, Keir Fraser, Suravee Suthikulpanit,
	Andrew Cooper, Aravind Gopalakrishnan, Jun Nakajima

This patch adds a field tsc_scaling_ratio in struct hvm_domain to record
the per-domain TSC scaling ratio, and sets it in tsc_set_info().

Before setting the per-domain TSC scaling ratio, we check its validity
in tsc_set_info(). If an invalid ratio is given, we will leave the
default value in tsc_scaling_ratio (i.e. ratio = 1) and setup guest TSC
as if no TSC scaling is used:
* For TSC_MODE_FAULT,
  - if a user-specified TSC frequency is given, we will set the guest
    TSC frequency to it; otherwise, we set it to the host TSC frequency.
  - if guest TSC frequency does not equal to host TSC frequency, we will
    emulate guest TSC (i.e. d->arch.vtsc is set to 1). In both cases,
    guest TSC runs in the guest TSC frequency.
* For TSC_MODE_PVRDTSCP,
  - we set the guest TSC frequency to the host TSC frequency.
  - guest rdtsc is executed natively in the host TSC frequency as
    before.
  - if rdtscp is not available to guest, it will be emulated; otherwise,
    it will be executed natively. In both cases, guest rdtscp gets TSC
    in the host TSC frequency as before.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v5:
 * Drop "Reviewed-by" from Boris Ostrovsky because of following changes.
 * Rewrite 64-bit integer arithmetic in hvm_get_tsc_scaling_ratio()
   by inlined assembly.
 * Make the previously per-vcpu tsc_scaling_ratio to struct hvm_domain
   and setup it per-domain.
---
 xen/arch/x86/hvm/hvm.c            | 26 ++++++++++++++++++++++++++
 xen/arch/x86/hvm/svm/svm.c        |  4 ++--
 xen/arch/x86/time.c               | 10 ++++++++--
 xen/include/asm-x86/hvm/domain.h  |  2 ++
 xen/include/asm-x86/hvm/hvm.h     |  8 ++++++++
 xen/include/asm-x86/hvm/svm/svm.h |  3 ---
 6 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index a72a6e7..b239bdb 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -298,6 +298,29 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
     return 1;
 }
 
+/*
+ * Get the ratio to scale host TSC frequency to gtsc_khz. zero will be
+ * returned if TSC scaling is unavailable or ratio cannot be handled
+ * by host CPU. Otherwise, a non-zero ratio will be returned.
+ */
+u64 hvm_get_tsc_scaling_ratio(u32 gtsc_khz)
+{
+    u64 ratio = gtsc_khz;
+    u64 dummy = 0;
+
+    if ( !hvm_tsc_scaling_supported )
+        return 0;
+
+    /* ratio = (gtsc_khz << hvm_funcs.tsc_scaling.ratio_frac_bits) / cpu_khz */
+    asm (
+        "shldq %2,%1,%0; salq %2,%1; divq %3"
+        : "+&d" (dummy), "+&a" (ratio)
+        : "c" (hvm_funcs.tsc_scaling.ratio_frac_bits),
+          "rm" ((u64) cpu_khz) );
+
+    return ratio > hvm_funcs.tsc_scaling.max_ratio ? 0 : ratio;
+}
+
 void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
 {
     uint64_t tsc;
@@ -1643,6 +1666,9 @@ int hvm_domain_initialise(struct domain *d)
     register_portio_handler(d, 0xe9, 1, hvm_print_line);
     register_portio_handler(d, 0xcf8, 4, hvm_access_cf8);
 
+    if ( hvm_tsc_scaling_supported )
+        d->arch.hvm_domain.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
+
     rc = hvm_funcs.domain_initialise(d);
     if ( rc != 0 )
         goto fail2;
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index b22d4a1..1136937 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -823,7 +823,7 @@ 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));
+    return scale_tsc(tsc, hvm_vcpu_tsc_scaling_ratio(v));
 }
 
 static uint64_t svm_get_tsc_offset(uint64_t host_tsc, uint64_t guest_tsc,
@@ -1000,7 +1000,7 @@ static inline void svm_tsc_ratio_save(struct vcpu *v)
 static inline void svm_tsc_ratio_load(struct vcpu *v)
 {
     if ( cpu_has_tsc_ratio && !v->domain->arch.vtsc ) 
-        wrmsrl(MSR_AMD64_TSC_RATIO, vcpu_tsc_ratio(v));
+        wrmsrl(MSR_AMD64_TSC_RATIO, hvm_vcpu_tsc_scaling_ratio(v));
 }
 
 static void svm_ctxt_switch_from(struct vcpu *v)
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 2248dfa..fda9692 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1865,7 +1865,8 @@ void tsc_set_info(struct domain *d,
          */
         if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
              (has_hvm_container_domain(d) ?
-              d->arch.tsc_khz == cpu_khz || hvm_tsc_scaling_supported :
+              (d->arch.tsc_khz == cpu_khz ||
+               hvm_get_tsc_scaling_ratio(d->arch.tsc_khz)) :
               incarnation == 0) )
         {
     case TSC_MODE_NEVER_EMULATE:
@@ -1879,7 +1880,8 @@ void tsc_set_info(struct domain *d,
         d->arch.vtsc = !boot_cpu_has(X86_FEATURE_RDTSCP) ||
                        !host_tsc_is_safe();
         enable_tsc_scaling = has_hvm_container_domain(d) &&
-                             hvm_tsc_scaling_supported && !d->arch.vtsc;
+                             !d->arch.vtsc &&
+                             hvm_get_tsc_scaling_ratio(gtsc_khz ?: cpu_khz);
         d->arch.tsc_khz = (enable_tsc_scaling && gtsc_khz) ? gtsc_khz : cpu_khz;
         set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 );
         d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
@@ -1897,6 +1899,10 @@ void tsc_set_info(struct domain *d,
     d->arch.incarnation = incarnation + 1;
     if ( has_hvm_container_domain(d) )
     {
+        if ( hvm_tsc_scaling_supported && !d->arch.vtsc )
+            d->arch.hvm_domain.tsc_scaling_ratio =
+                hvm_get_tsc_scaling_ratio(d->arch.tsc_khz);
+
         hvm_set_rdtsc_exiting(d, d->arch.vtsc);
         if ( d->vcpu && d->vcpu[0] && incarnation == 0 )
         {
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 4b54c5d..678f10a 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -143,6 +143,8 @@ struct hvm_domain {
      */
     uint64_t sync_tsc;
 
+    uint64_t tsc_scaling_ratio;
+
     unsigned long *io_bitmap;
 
     /* List of permanently write-mapped pages. */
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 7e87b10..6264d9e 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -272,6 +272,14 @@ u64 hvm_get_guest_tsc_fixed(struct vcpu *v, u64 at_tsc);
 #define hvm_tsc_scaling_supported \
     (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
 
+#define hvm_default_tsc_scaling_ratio \
+    (1ULL << hvm_funcs.tsc_scaling.ratio_frac_bits)
+
+#define hvm_vcpu_tsc_scaling_ratio(v) \
+    ((v)->domain->arch.hvm_domain.tsc_scaling_ratio)
+
+u64 hvm_get_tsc_scaling_ratio(u32 gtsc_khz);
+
 int hvm_set_mode(struct vcpu *v, int mode);
 void hvm_init_guest_time(struct domain *d);
 void hvm_set_guest_time(struct vcpu *v, u64 guest_time);
diff --git a/xen/include/asm-x86/hvm/svm/svm.h b/xen/include/asm-x86/hvm/svm/svm.h
index d60ec23..c954b7e 100644
--- a/xen/include/asm-x86/hvm/svm/svm.h
+++ b/xen/include/asm-x86/hvm/svm/svm.h
@@ -97,9 +97,6 @@ extern u32 svm_feature_flags;
 /* TSC rate */
 #define DEFAULT_TSC_RATIO       0x0000000100000000ULL
 #define TSC_RATIO_RSVD_BITS     0xffffff0000000000ULL
-#define TSC_RATIO(g_khz, h_khz) ( (((u64)(g_khz)<<32)/(u64)(h_khz)) & \
-                                  ~TSC_RATIO_RSVD_BITS )
-#define vcpu_tsc_ratio(v)       TSC_RATIO((v)->domain->arch.tsc_khz, cpu_khz)
 
 extern void svm_host_osvw_reset(void);
 extern void svm_host_osvw_init(void);
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 3/6] x86/hvm: Replace architecture TSC scaling by a common function
  2016-02-23  2:04 [PATCH v5 0/6] Add VMX TSC scaling support Haozhong Zhang
  2016-02-23  2:04 ` [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio Haozhong Zhang
  2016-02-23  2:05 ` [PATCH v5 2/6] x86/hvm: Setup " Haozhong Zhang
@ 2016-02-23  2:05 ` Haozhong Zhang
  2016-02-24 15:07   ` Jan Beulich
  2016-02-26  4:30   ` Tian, Kevin
  2016-02-23  2:05 ` [PATCH v5 4/6] x86/hvm: Move saving/loading vcpu's TSC to common code Haozhong Zhang
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-23  2:05 UTC (permalink / raw)
  To: xen-devel, Jan Beulich, Boris Ostrovsky, Kevin Tian
  Cc: Haozhong Zhang, Keir Fraser, Suravee Suthikulpanit,
	Andrew Cooper, Aravind Gopalakrishnan, Jun Nakajima

This patch implements a common function hvm_scale_tsc() to scale TSC by
using TSC scaling information collected by architecture code.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v5:
 * Drop "Reviewed-by" from Kevin Tian and Boris Ostrovsky and "Acked-by" 
   from Jan Beulich because of following changes.
 * Rewrite 64-bit integer arithmetic in hvm_scale_tsc() by inlined assembly.
 * Change the first argument of hvm_scale_tsc() from 'const struct vcpu *' 
   to 'const struct domain *' because it does not relied on vcpu.
---
 xen/arch/x86/hvm/hvm.c        | 21 +++++++++++++++++++--
 xen/arch/x86/hvm/svm/svm.c    |  8 --------
 xen/arch/x86/time.c           |  3 +--
 xen/include/asm-x86/hvm/hvm.h |  3 +--
 4 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index b239bdb..c6a6198 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -321,6 +321,23 @@ u64 hvm_get_tsc_scaling_ratio(u32 gtsc_khz)
     return ratio > hvm_funcs.tsc_scaling.max_ratio ? 0 : ratio;
 }
 
+u64 hvm_scale_tsc(const struct domain *d, u64 tsc)
+{
+    u64 ratio = d->arch.hvm_domain.tsc_scaling_ratio;
+    u64 dummy;
+
+    if ( ratio == hvm_default_tsc_scaling_ratio )
+        return tsc;
+
+    /* tsc = (tsc * ratio) >> hvm_funcs.tsc_scaling.ratio_frac_bits */
+    asm (
+        "mulq %3; shrdq %2,%1,%0"
+        : "+a" (tsc), "=d" (dummy)
+        : "c" (hvm_funcs.tsc_scaling.ratio_frac_bits), "rm" (ratio) );
+
+    return tsc;
+}
+
 void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
 {
     uint64_t tsc;
@@ -335,7 +352,7 @@ void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
     {
         tsc = at_tsc ?: rdtsc();
         if ( hvm_tsc_scaling_supported )
-            tsc = hvm_funcs.tsc_scaling.scale_tsc(v, tsc);
+            tsc = hvm_scale_tsc(v->domain, tsc);
     }
 
     delta_tsc = guest_tsc - tsc;
@@ -367,7 +384,7 @@ u64 hvm_get_guest_tsc_fixed(struct vcpu *v, uint64_t at_tsc)
     {
         tsc = at_tsc ?: rdtsc();
         if ( hvm_tsc_scaling_supported )
-            tsc = hvm_funcs.tsc_scaling.scale_tsc(v, tsc);
+            tsc = hvm_scale_tsc(v->domain, 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 1136937..dbdfc5b 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -819,13 +819,6 @@ 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, hvm_vcpu_tsc_scaling_ratio(v));
-}
-
 static uint64_t svm_get_tsc_offset(uint64_t host_tsc, uint64_t guest_tsc,
     uint64_t ratio)
 {
@@ -2291,7 +2284,6 @@ static struct hvm_function_table __initdata svm_function_table = {
 
     .tsc_scaling = {
         .max_ratio = ~TSC_RATIO_RSVD_BITS,
-        .scale_tsc = svm_scale_tsc,
     },
 };
 
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index fda9692..687e39b 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -816,8 +816,7 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
     {
         if ( has_hvm_container_domain(d) && hvm_tsc_scaling_supported )
         {
-            tsc_stamp            =
-                hvm_funcs.tsc_scaling.scale_tsc(v, t->local_tsc_stamp);
+            tsc_stamp            = hvm_scale_tsc(d, t->local_tsc_stamp);
             _u.tsc_to_system_mul = d->arch.vtsc_to_ns.mul_frac;
             _u.tsc_shift         = d->arch.vtsc_to_ns.shift;
         }
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 6264d9e..fc9616a 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -231,8 +231,6 @@ struct hvm_function_table {
         uint8_t  ratio_frac_bits;
         /* maximum-allowed TSC scaling ratio */
         uint64_t max_ratio;
-
-        uint64_t (*scale_tsc)(const struct vcpu *v, uint64_t tsc);
     } tsc_scaling;
 };
 
@@ -278,6 +276,7 @@ u64 hvm_get_guest_tsc_fixed(struct vcpu *v, u64 at_tsc);
 #define hvm_vcpu_tsc_scaling_ratio(v) \
     ((v)->domain->arch.hvm_domain.tsc_scaling_ratio)
 
+u64 hvm_scale_tsc(const struct domain *d, u64 tsc);
 u64 hvm_get_tsc_scaling_ratio(u32 gtsc_khz);
 
 int hvm_set_mode(struct vcpu *v, int mode);
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 4/6] x86/hvm: Move saving/loading vcpu's TSC to common code
  2016-02-23  2:04 [PATCH v5 0/6] Add VMX TSC scaling support Haozhong Zhang
                   ` (2 preceding siblings ...)
  2016-02-23  2:05 ` [PATCH v5 3/6] x86/hvm: Replace architecture TSC scaling by a common function Haozhong Zhang
@ 2016-02-23  2:05 ` Haozhong Zhang
  2016-02-23  2:05 ` [PATCH v5 5/6] vmx: Add VMX RDTSC(P) scaling support Haozhong Zhang
  2016-02-23  2:05 ` [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt Haozhong Zhang
  5 siblings, 0 replies; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-23  2:05 UTC (permalink / raw)
  To: xen-devel, Jan Beulich, Boris Ostrovsky, Kevin Tian
  Cc: Haozhong Zhang, Keir Fraser, Suravee Suthikulpanit,
	Andrew Cooper, Aravind Gopalakrishnan, Jun Nakajima

Both VMX and SVM save/load vcpu's TSC when saving/loading vcpu's
context, so this patch moves saving/loading vcpu's TSC to the common
functions hvm_[save|load]_cpu_ctxt().

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 xen/arch/x86/hvm/hvm.c     | 4 ++++
 xen/arch/x86/hvm/svm/svm.c | 5 -----
 xen/arch/x86/hvm/vmx/vmx.c | 5 -----
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index c6a6198..3510302 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1808,6 +1808,8 @@ static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
         /* Architecture-specific vmcs/vmcb bits */
         hvm_funcs.save_cpu_ctxt(v, &ctxt);
 
+        ctxt.tsc = hvm_get_guest_tsc_fixed(v, d->arch.hvm_domain.sync_tsc);
+
         ctxt.msr_tsc_aux = hvm_msr_tsc_aux(v);
 
         hvm_get_segment_register(v, x86_seg_idtr, &seg);
@@ -2113,6 +2115,8 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
 
     v->arch.hvm_vcpu.msr_tsc_aux = ctxt.msr_tsc_aux;
 
+    hvm_set_guest_tsc_fixed(v, ctxt.tsc, d->arch.hvm_domain.sync_tsc);
+
     seg.limit = ctxt.idtr_limit;
     seg.base = ctxt.idtr_base;
     hvm_set_segment_register(v, x86_seg_idtr, &seg);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index dbdfc5b..a045f46 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -357,9 +357,6 @@ static void svm_save_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
     data->msr_syscall_mask = vmcb->sfmask;
     data->msr_efer         = v->arch.hvm_vcpu.guest_efer;
     data->msr_flags        = -1ULL;
-
-    data->tsc = hvm_get_guest_tsc_fixed(v,
-                                        v->domain->arch.hvm_domain.sync_tsc);
 }
 
 
@@ -374,8 +371,6 @@ static void svm_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
     vmcb->sfmask     = data->msr_syscall_mask;
     v->arch.hvm_vcpu.guest_efer = data->msr_efer;
     svm_update_guest_efer(v);
-
-    hvm_set_guest_tsc_fixed(v, data->tsc, v->domain->arch.hvm_domain.sync_tsc);
 }
 
 static void svm_save_vmcb_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 44e778f..c93fc28 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -604,9 +604,6 @@ static void vmx_save_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
     data->msr_lstar        = guest_state->msrs[VMX_INDEX_MSR_LSTAR];
     data->msr_star         = guest_state->msrs[VMX_INDEX_MSR_STAR];
     data->msr_syscall_mask = guest_state->msrs[VMX_INDEX_MSR_SYSCALL_MASK];
-
-    data->tsc = hvm_get_guest_tsc_fixed(v,
-                                        v->domain->arch.hvm_domain.sync_tsc);
 }
 
 static void vmx_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
@@ -621,8 +618,6 @@ static void vmx_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
 
     v->arch.hvm_vmx.cstar     = data->msr_cstar;
     v->arch.hvm_vmx.shadow_gs = data->shadow_gs;
-
-    hvm_set_guest_tsc_fixed(v, data->tsc, v->domain->arch.hvm_domain.sync_tsc);
 }
 
 
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 5/6] vmx: Add VMX RDTSC(P) scaling support
  2016-02-23  2:04 [PATCH v5 0/6] Add VMX TSC scaling support Haozhong Zhang
                   ` (3 preceding siblings ...)
  2016-02-23  2:05 ` [PATCH v5 4/6] x86/hvm: Move saving/loading vcpu's TSC to common code Haozhong Zhang
@ 2016-02-23  2:05 ` Haozhong Zhang
  2016-02-24 15:09   ` Jan Beulich
  2016-02-26  4:31   ` Tian, Kevin
  2016-02-23  2:05 ` [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt Haozhong Zhang
  5 siblings, 2 replies; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-23  2:05 UTC (permalink / raw)
  To: xen-devel, Jan Beulich, Boris Ostrovsky, Kevin Tian
  Cc: Haozhong Zhang, Keir Fraser, Suravee Suthikulpanit,
	Andrew Cooper, Aravind Gopalakrishnan, Jun Nakajima

This patch adds the initialization and setup code for VMX TSC scaling.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v5:
 * Drop "Acked-by" from Kevin Tian because of following changes.
 * Adapt for data structure changes in patch 1.
---
 xen/arch/x86/hvm/hvm.c             |  6 ++++++
 xen/arch/x86/hvm/vmx/vmcs.c        | 12 +++++++++---
 xen/arch/x86/hvm/vmx/vmx.c         | 17 +++++++++++++++++
 xen/include/asm-x86/hvm/hvm.h      |  3 +++
 xen/include/asm-x86/hvm/vmx/vmcs.h |  7 +++++++
 5 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3510302..6f481d2 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2113,6 +2113,9 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
     if ( hvm_funcs.load_cpu_ctxt(v, &ctxt) < 0 )
         return -EINVAL;
 
+    if ( hvm_funcs.tsc_scaling.setup )
+        hvm_funcs.tsc_scaling.setup(v);
+
     v->arch.hvm_vcpu.msr_tsc_aux = ctxt.msr_tsc_aux;
 
     hvm_set_guest_tsc_fixed(v, ctxt.tsc, d->arch.hvm_domain.sync_tsc);
@@ -5627,6 +5630,9 @@ void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, uint16_t ip)
     hvm_set_segment_register(v, x86_seg_gdtr, &reg);
     hvm_set_segment_register(v, x86_seg_idtr, &reg);
 
+    if ( hvm_funcs.tsc_scaling.setup )
+        hvm_funcs.tsc_scaling.setup(v);
+
     /* Sync AP's TSC with BSP's. */
     v->arch.hvm_vcpu.cache_tsc_offset =
         v->domain->vcpu[0]->arch.hvm_vcpu.cache_tsc_offset;
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 5bc3c74..d506f80 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -149,6 +149,7 @@ static void __init vmx_display_features(void)
     P(cpu_has_vmx_vmfunc, "VM Functions");
     P(cpu_has_vmx_virt_exceptions, "Virtualisation Exceptions");
     P(cpu_has_vmx_pml, "Page Modification Logging");
+    P(cpu_has_vmx_tsc_scaling, "TSC Scaling");
 #undef P
 
     if ( !printed )
@@ -243,7 +244,8 @@ static int vmx_init_vmcs_config(void)
                SECONDARY_EXEC_ENABLE_VM_FUNCTIONS |
                SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS |
                SECONDARY_EXEC_XSAVES |
-               SECONDARY_EXEC_PCOMMIT);
+               SECONDARY_EXEC_PCOMMIT |
+               SECONDARY_EXEC_TSC_SCALING);
         rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
         if ( _vmx_misc_cap & VMX_MISC_VMWRITE_ALL )
             opt |= SECONDARY_EXEC_ENABLE_VMCS_SHADOWING;
@@ -1000,7 +1002,7 @@ static int construct_vmcs(struct vcpu *v)
     __vmwrite(PIN_BASED_VM_EXEC_CONTROL, vmx_pin_based_exec_control);
 
     v->arch.hvm_vmx.exec_control = vmx_cpu_based_exec_control;
-    if ( d->arch.vtsc )
+    if ( d->arch.vtsc && !cpu_has_vmx_tsc_scaling )
         v->arch.hvm_vmx.exec_control |= CPU_BASED_RDTSC_EXITING;
 
     v->arch.hvm_vmx.secondary_exec_control = vmx_secondary_exec_control;
@@ -1288,6 +1290,9 @@ static int construct_vmcs(struct vcpu *v)
     if ( cpu_has_vmx_xsaves )
         __vmwrite(XSS_EXIT_BITMAP, 0);
 
+    if ( cpu_has_vmx_tsc_scaling )
+        __vmwrite(TSC_MULTIPLIER, d->arch.hvm_domain.tsc_scaling_ratio);
+
     vmx_vmcs_exit(v);
 
     /* PVH: paging mode is updated by arch_set_info_guest(). */
@@ -1870,7 +1875,8 @@ void vmcs_dump_vcpu(struct vcpu *v)
            vmr32(VM_EXIT_REASON), vmr(EXIT_QUALIFICATION));
     printk("IDTVectoring: info=%08x errcode=%08x\n",
            vmr32(IDT_VECTORING_INFO), vmr32(IDT_VECTORING_ERROR_CODE));
-    printk("TSC Offset = 0x%016lx\n", vmr(TSC_OFFSET));
+    printk("TSC Offset = 0x%016lx  TSC Multiplier = 0x%016lx\n",
+           vmr(TSC_OFFSET), vmr(TSC_MULTIPLIER));
     if ( (v->arch.hvm_vmx.exec_control & CPU_BASED_TPR_SHADOW) ||
          (vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT) )
         printk("TPR Threshold = 0x%02x  PostedIntrVec = 0x%02x\n",
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index c93fc28..57071ac 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1122,6 +1122,16 @@ static void vmx_handle_cd(struct vcpu *v, unsigned long value)
     }
 }
 
+static void vmx_setup_tsc_scaling(struct vcpu *v)
+{
+    if ( !hvm_tsc_scaling_supported || v->domain->arch.vtsc )
+        return;
+
+    vmx_vmcs_enter(v);
+    __vmwrite(TSC_MULTIPLIER, hvm_vcpu_tsc_scaling_ratio(v));
+    vmx_vmcs_exit(v);
+}
+
 static void vmx_set_tsc_offset(struct vcpu *v, u64 offset, u64 at_tsc)
 {
     vmx_vmcs_enter(v);
@@ -2016,6 +2026,10 @@ static struct hvm_function_table __initdata vmx_function_table = {
     .altp2m_vcpu_update_vmfunc_ve = vmx_vcpu_update_vmfunc_ve,
     .altp2m_vcpu_emulate_ve = vmx_vcpu_emulate_ve,
     .altp2m_vcpu_emulate_vmfunc = vmx_vcpu_emulate_vmfunc,
+    .tsc_scaling = {
+        .max_ratio = VMX_TSC_MULTIPLIER_MAX,
+        .setup     = vmx_setup_tsc_scaling,
+    },
 };
 
 /* Handle VT-d posted-interrupt when VCPU is running. */
@@ -2120,6 +2134,9 @@ const struct hvm_function_table * __init start_vmx(void)
          && cpu_has_vmx_secondary_exec_control )
         vmx_function_table.pvh_supported = 1;
 
+    if ( cpu_has_vmx_tsc_scaling )
+        vmx_function_table.tsc_scaling.ratio_frac_bits = 48;
+
     setup_vmcs_dump();
 
     return &vmx_function_table;
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index fc9616a..c98df96 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -231,6 +231,9 @@ struct hvm_function_table {
         uint8_t  ratio_frac_bits;
         /* maximum-allowed TSC scaling ratio */
         uint64_t max_ratio;
+
+        /* Architecture function to setup TSC scaling ratio */
+        void (*setup)(struct vcpu *v);
     } tsc_scaling;
 };
 
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index a5e7aee..0d7c6d2 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -237,6 +237,7 @@ extern u32 vmx_vmentry_control;
 #define SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS   0x00040000
 #define SECONDARY_EXEC_XSAVES                   0x00100000
 #define SECONDARY_EXEC_PCOMMIT                  0x00200000
+#define SECONDARY_EXEC_TSC_SCALING              0x02000000
 extern u32 vmx_secondary_exec_control;
 
 #define VMX_EPT_EXEC_ONLY_SUPPORTED                         0x00000001
@@ -259,6 +260,8 @@ extern u64 vmx_ept_vpid_cap;
 #define VMX_MISC_CR3_TARGET                     0x01ff0000
 #define VMX_MISC_VMWRITE_ALL                    0x20000000
 
+#define VMX_TSC_MULTIPLIER_MAX                  0xffffffffffffffffULL
+
 #define cpu_has_wbinvd_exiting \
     (vmx_secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING)
 #define cpu_has_vmx_virtualize_apic_accesses \
@@ -306,6 +309,9 @@ extern u64 vmx_ept_vpid_cap;
     (vmx_secondary_exec_control & SECONDARY_EXEC_XSAVES)
 #define cpu_has_vmx_pcommit \
     (vmx_secondary_exec_control & SECONDARY_EXEC_PCOMMIT)
+#define cpu_has_vmx_tsc_scaling \
+    (vmx_secondary_exec_control & SECONDARY_EXEC_TSC_SCALING)
+
 #define VMCS_RID_TYPE_MASK              0x80000000
 
 /* GUEST_INTERRUPTIBILITY_INFO flags. */
@@ -380,6 +386,7 @@ enum vmcs_field {
     VMWRITE_BITMAP                  = 0x00002028,
     VIRT_EXCEPTION_INFO             = 0x0000202a,
     XSS_EXIT_BITMAP                 = 0x0000202c,
+    TSC_MULTIPLIER                  = 0x00002032,
     GUEST_PHYSICAL_ADDRESS          = 0x00002400,
     VMCS_LINK_POINTER               = 0x00002800,
     GUEST_IA32_DEBUGCTL             = 0x00002802,
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt
  2016-02-23  2:04 [PATCH v5 0/6] Add VMX TSC scaling support Haozhong Zhang
                   ` (4 preceding siblings ...)
  2016-02-23  2:05 ` [PATCH v5 5/6] vmx: Add VMX RDTSC(P) scaling support Haozhong Zhang
@ 2016-02-23  2:05 ` Haozhong Zhang
  2016-02-26  4:37   ` Tian, Kevin
  5 siblings, 1 reply; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-23  2:05 UTC (permalink / raw)
  To: xen-devel, Jan Beulich, Boris Ostrovsky, Kevin Tian
  Cc: Haozhong Zhang, Keir Fraser, Suravee Suthikulpanit,
	Andrew Cooper, Aravind Gopalakrishnan, Jun Nakajima

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 docs/man/xl.cfg.pod.5 | 14 +++++++++++++-
 docs/misc/tscmode.txt | 21 +++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 40690bd..56b1117 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1313,9 +1313,17 @@ deprecated. Options are:
 
 =item B<"default">
 
-Guest rdtsc/p executed natively when monotonicity can be guaranteed
+Guest rdtsc/p is executed natively when monotonicity can be guaranteed
 and emulated otherwise (with frequency scaled if necessary).
 
+If a HVM container in B<default> TSC mode is created on a host that
+provides constant host TSC, its guest TSC frequency will be the same
+as the host. If it is later migrated to another host that provide
+constant host TSC and supports Intel VMX TSC scaling/AMD SVM TSC
+ratio, its guest TSC frequency will be the same before and after
+migration, and guest rdtsc/p will be executed natively as well after
+migration.
+
 =item B<"always_emulate">
 
 Guest rdtsc/p always emulated at 1GHz (kernel and user). Guest rdtsc/p
@@ -1337,6 +1345,10 @@ determine when a restore/migration has occurred and assumes guest
 obtains/uses pvclock-like mechanism to adjust for monotonicity and
 frequency changes.
 
+If a HVM container in B<native_paravirt> TSC mode can execute both guest
+rdtsc and guest rdtscp natively, then the guest TSC frequency will be
+determined in the similar way to that of B<default> TSC mode.
+
 =back
 
 Please see F<docs/misc/tscmode.txt> for more information on this option.
diff --git a/docs/misc/tscmode.txt b/docs/misc/tscmode.txt
index e8c84e8..01ee060 100644
--- a/docs/misc/tscmode.txt
+++ b/docs/misc/tscmode.txt
@@ -297,3 +297,24 @@ and also much faster than nearly all OS-provided time mechanisms.
 While pvrtscp is too complex for most apps, certain enterprise
 TSC-sensitive high-TSC-frequency apps may find it useful to
 obtain a significant performance gain.
+
+Hardware TSC Scaling
+
+Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
+by guest rdtsc/p increasing in a different frequency than the host
+TSC frequency.
+
+If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode
+(tsc_mode=3) is created on a host that provides constant TSC, its
+guest TSC frequency will be the same as the host. If it is later
+migrated to another host that provides constant TSC and supports Intel
+VMX TSC scaling/AMD SVM TSC ratio, its guest TSC frequency will be the
+same before and after migration.
+
+For above HVM container in default TSC mode (tsc_mode=0), if above
+hosts support rdtscp, both guest rdtsc and rdtscp instructions will be
+executed natively before and after migration.
+
+For above HVM container in PVRDTSCP mode (tsc_mode=3), if the
+destination host does not support rdtscp, the guest rdtscp instruction
+will be emulated with the guest TSC frequency.
-- 
2.7.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-23  2:04 ` [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio Haozhong Zhang
@ 2016-02-23 14:00   ` Boris Ostrovsky
  2016-02-23 14:10     ` Jan Beulich
  2016-02-24 14:36   ` Jan Beulich
  2016-02-26  4:27   ` Tian, Kevin
  2 siblings, 1 reply; 32+ messages in thread
From: Boris Ostrovsky @ 2016-02-23 14:00 UTC (permalink / raw)
  To: Haozhong Zhang, xen-devel, Jan Beulich, Kevin Tian
  Cc: Jun Nakajima, Andrew Cooper, Keir Fraser, Aravind Gopalakrishnan,
	Suravee Suthikulpanit

On 02/22/2016 09:04 PM, Haozhong Zhang wrote:

>   
> +    if ( cpu_has_tsc_ratio )
> +        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
> +


>   
> +#define hvm_tsc_scaling_supported \
> +    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
> +

What is the difference (in usage) between cpu_has_tsc_ratio and 
hvm_tsc_scaling_supported? Isn't the first imply the second (and if yes 
then what's the reason for having the latter)?

-boris

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-23 14:00   ` Boris Ostrovsky
@ 2016-02-23 14:10     ` Jan Beulich
  2016-02-23 14:16       ` Boris Ostrovsky
  0 siblings, 1 reply; 32+ messages in thread
From: Jan Beulich @ 2016-02-23 14:10 UTC (permalink / raw)
  To: Haozhong Zhang, Boris Ostrovsky
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima

>>> On 23.02.16 at 15:00, <boris.ostrovsky@oracle.com> wrote:
> On 02/22/2016 09:04 PM, Haozhong Zhang wrote:
> 
>>   
>> +    if ( cpu_has_tsc_ratio )
>> +        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
>> +
> 
> 
>>   
>> +#define hvm_tsc_scaling_supported \
>> +    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
>> +
> 
> What is the difference (in usage) between cpu_has_tsc_ratio and 
> hvm_tsc_scaling_supported? Isn't the first imply the second (and if yes 
> then what's the reason for having the latter)?

Iiuc cpu_has_tsc_ratio is AMD/SVM specific, while
hvm_tsc_scaling_supported is meant to be vendor independent.

Jan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-23 14:10     ` Jan Beulich
@ 2016-02-23 14:16       ` Boris Ostrovsky
  2016-02-23 15:37         ` Jan Beulich
  0 siblings, 1 reply; 32+ messages in thread
From: Boris Ostrovsky @ 2016-02-23 14:16 UTC (permalink / raw)
  To: Jan Beulich, Haozhong Zhang
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima

On 02/23/2016 09:10 AM, Jan Beulich wrote:
>>>> On 23.02.16 at 15:00, <boris.ostrovsky@oracle.com> wrote:
>> On 02/22/2016 09:04 PM, Haozhong Zhang wrote:
>>
>>>    
>>> +    if ( cpu_has_tsc_ratio )
>>> +        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
>>> +
>>
>>>    
>>> +#define hvm_tsc_scaling_supported \
>>> +    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
>>> +
>> What is the difference (in usage) between cpu_has_tsc_ratio and
>> hvm_tsc_scaling_supported? Isn't the first imply the second (and if yes
>> then what's the reason for having the latter)?
> Iiuc cpu_has_tsc_ratio is AMD/SVM specific, while
> hvm_tsc_scaling_supported is meant to be vendor independent.

Ah, OK. Can we then

#define hvm_tsc_scaling_supported (cpu_has_vmx_tsc_scaling || 
cpu_has_tsc_ratio)

-boris

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-23 14:16       ` Boris Ostrovsky
@ 2016-02-23 15:37         ` Jan Beulich
  2016-02-24  6:00           ` Haozhong Zhang
  0 siblings, 1 reply; 32+ messages in thread
From: Jan Beulich @ 2016-02-23 15:37 UTC (permalink / raw)
  To: Haozhong Zhang, Boris Ostrovsky
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima

>>> On 23.02.16 at 15:16, <boris.ostrovsky@oracle.com> wrote:
> On 02/23/2016 09:10 AM, Jan Beulich wrote:
>>>>> On 23.02.16 at 15:00, <boris.ostrovsky@oracle.com> wrote:
>>> On 02/22/2016 09:04 PM, Haozhong Zhang wrote:
>>>
>>>>    
>>>> +    if ( cpu_has_tsc_ratio )
>>>> +        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
>>>> +
>>>
>>>>    
>>>> +#define hvm_tsc_scaling_supported \
>>>> +    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
>>>> +
>>> What is the difference (in usage) between cpu_has_tsc_ratio and
>>> hvm_tsc_scaling_supported? Isn't the first imply the second (and if yes
>>> then what's the reason for having the latter)?
>> Iiuc cpu_has_tsc_ratio is AMD/SVM specific, while
>> hvm_tsc_scaling_supported is meant to be vendor independent.
> 
> Ah, OK. Can we then
> 
> #define hvm_tsc_scaling_supported (cpu_has_vmx_tsc_scaling || 
> cpu_has_tsc_ratio)

Why would we? The above is doing precisely (but implicitly) that,
just with only one memory access instead of two.

Jan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-23 15:37         ` Jan Beulich
@ 2016-02-24  6:00           ` Haozhong Zhang
  2016-02-24 13:46             ` Haozhong Zhang
  0 siblings, 1 reply; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-24  6:00 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima

On 02/23/16 08:37, Jan Beulich wrote:
> >>> On 23.02.16 at 15:16, <boris.ostrovsky@oracle.com> wrote:
> > On 02/23/2016 09:10 AM, Jan Beulich wrote:
> >>>>> On 23.02.16 at 15:00, <boris.ostrovsky@oracle.com> wrote:
> >>> On 02/22/2016 09:04 PM, Haozhong Zhang wrote:
> >>>
> >>>>    
> >>>> +    if ( cpu_has_tsc_ratio )
> >>>> +        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
> >>>> +
> >>>
> >>>>    
> >>>> +#define hvm_tsc_scaling_supported \
> >>>> +    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
> >>>> +
> >>> What is the difference (in usage) between cpu_has_tsc_ratio and
> >>> hvm_tsc_scaling_supported? Isn't the first imply the second (and if yes
> >>> then what's the reason for having the latter)?
> >> Iiuc cpu_has_tsc_ratio is AMD/SVM specific, while
> >> hvm_tsc_scaling_supported is meant to be vendor independent.
> >

Yes, it's to be vendor independent. Earlier versions of this patch
series set a field tsc_scaling_supported in hvm_function_table if
cpu_has_vmx_tsc_scaling or cpu_has_tsc_ratio. Jan suggested we could get
the same information if some of other fields are initialized
conditionally, and no extra field (tsc_scaling_supported) would be
needed any more.

> > Ah, OK. Can we then
> > 
> > #define hvm_tsc_scaling_supported (cpu_has_vmx_tsc_scaling || 
> > cpu_has_tsc_ratio)
> 
> Why would we? The above is doing precisely (but implicitly) that,
> just with only one memory access instead of two.
>

Boris, does the current one look fine for you, as it does the same thing
as your suggested one?

Thanks,
Haozhong

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-24  6:00           ` Haozhong Zhang
@ 2016-02-24 13:46             ` Haozhong Zhang
  2016-02-24 14:49               ` Boris Ostrovsky
  0 siblings, 1 reply; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-24 13:46 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Jan Beulich, Aravind Gopalakrishnan, Jun Nakajima

Sorry, forgot sending the last reply to Boris.

On 02/24/16 14:00, Haozhong Zhang wrote:
> On 02/23/16 08:37, Jan Beulich wrote:
> > >>> On 23.02.16 at 15:16, <boris.ostrovsky@oracle.com> wrote:
> > > On 02/23/2016 09:10 AM, Jan Beulich wrote:
> > >>>>> On 23.02.16 at 15:00, <boris.ostrovsky@oracle.com> wrote:
> > >>> On 02/22/2016 09:04 PM, Haozhong Zhang wrote:
> > >>>
> > >>>>    
> > >>>> +    if ( cpu_has_tsc_ratio )
> > >>>> +        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
> > >>>> +
> > >>>
> > >>>>    
> > >>>> +#define hvm_tsc_scaling_supported \
> > >>>> +    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
> > >>>> +
> > >>> What is the difference (in usage) between cpu_has_tsc_ratio and
> > >>> hvm_tsc_scaling_supported? Isn't the first imply the second (and if yes
> > >>> then what's the reason for having the latter)?
> > >> Iiuc cpu_has_tsc_ratio is AMD/SVM specific, while
> > >> hvm_tsc_scaling_supported is meant to be vendor independent.
> > >
> 
> Yes, it's to be vendor independent. Earlier versions of this patch
> series set a field tsc_scaling_supported in hvm_function_table if
> cpu_has_vmx_tsc_scaling or cpu_has_tsc_ratio. Jan suggested we could get
> the same information if some of other fields are initialized
> conditionally, and no extra field (tsc_scaling_supported) would be
> needed any more.
> 
> > > Ah, OK. Can we then
> > > 
> > > #define hvm_tsc_scaling_supported (cpu_has_vmx_tsc_scaling || 
> > > cpu_has_tsc_ratio)
> > 
> > Why would we? The above is doing precisely (but implicitly) that,
> > just with only one memory access instead of two.
> >
> 
> Boris, does the current one look fine for you, as it does the same thing
> as your suggested one?
> 
> Thanks,
> Haozhong
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-23  2:04 ` [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio Haozhong Zhang
  2016-02-23 14:00   ` Boris Ostrovsky
@ 2016-02-24 14:36   ` Jan Beulich
  2016-02-24 15:03     ` Haozhong Zhang
  2016-02-26  4:27   ` Tian, Kevin
  2 siblings, 1 reply; 32+ messages in thread
From: Jan Beulich @ 2016-02-24 14:36 UTC (permalink / raw)
  To: Haozhong Zhang
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

>>> On 23.02.16 at 03:04, <haozhong.zhang@intel.com> wrote:
> Both VMX TSC scaling and SVM TSC ratio use the 64-bit TSC scaling ratio,
> but the number of fractional bits of the ratio is different between VMX
> and SVM. This patch adds the architecture code to collect the number of
> fractional bits and other related information into fields of struct
> hvm_function_table so that they can be used in the common code.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

albeit I would have wished ...

> @@ -312,8 +311,8 @@ void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
>      else
>      {
>          tsc = at_tsc ?: rdtsc();
> -        if ( cpu_has_tsc_ratio )
> -            tsc = hvm_funcs.scale_tsc(v, tsc);
> +        if ( hvm_tsc_scaling_supported )
> +            tsc = hvm_funcs.tsc_scaling.scale_tsc(v, tsc);

... for these to get their redundancy reduced, .e.g

            tsc = hvm_funcs.tsc_scaling.scale(v, tsc);

The R-b may be retained if you elect to make that adjustment.

Jan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-24 13:46             ` Haozhong Zhang
@ 2016-02-24 14:49               ` Boris Ostrovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Boris Ostrovsky @ 2016-02-24 14:49 UTC (permalink / raw)
  To: Jan Beulich, Aravind Gopalakrishnan, Suravee Suthikulpanit,
	Andrew Cooper, Jun Nakajima, Kevin Tian, xen-devel, Keir Fraser

On 02/24/2016 08:46 AM, Haozhong Zhang wrote:
> Sorry, forgot sending the last reply to Boris.
>
> On 02/24/16 14:00, Haozhong Zhang wrote:
>> On 02/23/16 08:37, Jan Beulich wrote:
>>>>>> On 23.02.16 at 15:16, <boris.ostrovsky@oracle.com> wrote:
>>>> On 02/23/2016 09:10 AM, Jan Beulich wrote:
>>>>>>>> On 23.02.16 at 15:00, <boris.ostrovsky@oracle.com> wrote:
>>>>>> On 02/22/2016 09:04 PM, Haozhong Zhang wrote:
>>>>>>
>>>>>>>     
>>>>>>> +    if ( cpu_has_tsc_ratio )
>>>>>>> +        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
>>>>>>> +
>>>>>>>     
>>>>>>> +#define hvm_tsc_scaling_supported \
>>>>>>> +    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
>>>>>>> +
>>>>>> What is the difference (in usage) between cpu_has_tsc_ratio and
>>>>>> hvm_tsc_scaling_supported? Isn't the first imply the second (and if yes
>>>>>> then what's the reason for having the latter)?
>>>>> Iiuc cpu_has_tsc_ratio is AMD/SVM specific, while
>>>>> hvm_tsc_scaling_supported is meant to be vendor independent.
>> Yes, it's to be vendor independent. Earlier versions of this patch
>> series set a field tsc_scaling_supported in hvm_function_table if
>> cpu_has_vmx_tsc_scaling or cpu_has_tsc_ratio. Jan suggested we could get
>> the same information if some of other fields are initialized
>> conditionally, and no extra field (tsc_scaling_supported) would be
>> needed any more.
>>
>>>> Ah, OK. Can we then
>>>>
>>>> #define hvm_tsc_scaling_supported (cpu_has_vmx_tsc_scaling ||
>>>> cpu_has_tsc_ratio)
>>> Why would we? The above is doing precisely (but implicitly) that,
>>> just with only one memory access instead of two.
>>>
>> Boris, does the current one look fine for you, as it does the same thing
>> as your suggested one?

Yes, this is fine. IMO using cpu_has_* would be slightly  more logical 
but as Jan pointed it is also potentially more costly.

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

-boris

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/6] x86/hvm: Setup TSC scaling ratio
  2016-02-23  2:05 ` [PATCH v5 2/6] x86/hvm: Setup " Haozhong Zhang
@ 2016-02-24 15:01   ` Jan Beulich
  2016-02-24 15:42     ` Haozhong Zhang
  0 siblings, 1 reply; 32+ messages in thread
From: Jan Beulich @ 2016-02-24 15:01 UTC (permalink / raw)
  To: Haozhong Zhang
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

>>> On 23.02.16 at 03:05, <haozhong.zhang@intel.com> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -298,6 +298,29 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
>      return 1;
>  }
>  
> +/*
> + * Get the ratio to scale host TSC frequency to gtsc_khz. zero will be
> + * returned if TSC scaling is unavailable or ratio cannot be handled
> + * by host CPU. Otherwise, a non-zero ratio will be returned.
> + */
> +u64 hvm_get_tsc_scaling_ratio(u32 gtsc_khz)
> +{
> +    u64 ratio = gtsc_khz;
> +    u64 dummy = 0;

"dummy" suggests it is unused, which it isn't. "tmp" or "hi" might be
a little better, but since the meanings of the variables (also "ratio")
differ for their roles an inputs and outputs, splitting inputs and
outputs below would seem even better. In which case "dummy"
become would an appropriate name again.

> +    if ( !hvm_tsc_scaling_supported )
> +        return 0;
> +
> +    /* ratio = (gtsc_khz << hvm_funcs.tsc_scaling.ratio_frac_bits) / cpu_khz */
> +    asm (
> +        "shldq %2,%1,%0; salq %2,%1; divq %3"
> +        : "+&d" (dummy), "+&a" (ratio)
> +        : "c" (hvm_funcs.tsc_scaling.ratio_frac_bits),
> +          "rm" ((u64) cpu_khz) );

And this DIVQ can't possibly #DE, e.g. when gtsc_khz is much larger
than cpu_khz?

I'd also prefer if the instruction got put on the same line as the
"asm (". Considering that we're dealing with unsigned quantities
here I'd further prefer if SHLQ was used instead of SALQ. And
finally I'd suggest using named rather than numbered asm()
arguments.

> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -272,6 +272,14 @@ u64 hvm_get_guest_tsc_fixed(struct vcpu *v, u64 at_tsc);
>  #define hvm_tsc_scaling_supported \
>      (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
>  
> +#define hvm_default_tsc_scaling_ratio \
> +    (1ULL << hvm_funcs.tsc_scaling.ratio_frac_bits)
> +
> +#define hvm_vcpu_tsc_scaling_ratio(v) \
> +    ((v)->domain->arch.hvm_domain.tsc_scaling_ratio)

Since this is now a per-domain property I think it is misleading (and
potentially hindering) for the called to pass in a struct vcpu * here.
Please make this a struct domain *.

Jan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-24 14:36   ` Jan Beulich
@ 2016-02-24 15:03     ` Haozhong Zhang
  0 siblings, 0 replies; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-24 15:03 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

On 02/24/16 07:36, Jan Beulich wrote:
> >>> On 23.02.16 at 03:04, <haozhong.zhang@intel.com> wrote:
> > Both VMX TSC scaling and SVM TSC ratio use the 64-bit TSC scaling ratio,
> > but the number of fractional bits of the ratio is different between VMX
> > and SVM. This patch adds the architecture code to collect the number of
> > fractional bits and other related information into fields of struct
> > hvm_function_table so that they can be used in the common code.
> > 
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> albeit I would have wished ...
> 
> > @@ -312,8 +311,8 @@ void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
> >      else
> >      {
> >          tsc = at_tsc ?: rdtsc();
> > -        if ( cpu_has_tsc_ratio )
> > -            tsc = hvm_funcs.scale_tsc(v, tsc);
> > +        if ( hvm_tsc_scaling_supported )
> > +            tsc = hvm_funcs.tsc_scaling.scale_tsc(v, tsc);
> 
> ... for these to get their redundancy reduced, .e.g
> 
>             tsc = hvm_funcs.tsc_scaling.scale(v, tsc);
> 
> The R-b may be retained if you elect to make that adjustment.
>

The callback scale_tsc() is later dropped in patch 3. If a new
version of this patch series is needed, I'll make the adjustment
then. Otherwise, I would like to leave it as is.

Thanks,
Haozhong

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 3/6] x86/hvm: Replace architecture TSC scaling by a common function
  2016-02-23  2:05 ` [PATCH v5 3/6] x86/hvm: Replace architecture TSC scaling by a common function Haozhong Zhang
@ 2016-02-24 15:07   ` Jan Beulich
  2016-02-26  4:30   ` Tian, Kevin
  1 sibling, 0 replies; 32+ messages in thread
From: Jan Beulich @ 2016-02-24 15:07 UTC (permalink / raw)
  To: Haozhong Zhang
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

>>> On 23.02.16 at 03:05, <haozhong.zhang@intel.com> wrote:
> This patch implements a common function hvm_scale_tsc() to scale TSC by
> using TSC scaling information collected by architecture code.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Also I note that

> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -231,8 +231,6 @@ struct hvm_function_table {
>          uint8_t  ratio_frac_bits;
>          /* maximum-allowed TSC scaling ratio */
>          uint64_t max_ratio;
> -
> -        uint64_t (*scale_tsc)(const struct vcpu *v, uint64_t tsc);
>      } tsc_scaling;

... this field disappears here, so never mind the renaming comment
on the earlier patch.

Jan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 5/6] vmx: Add VMX RDTSC(P) scaling support
  2016-02-23  2:05 ` [PATCH v5 5/6] vmx: Add VMX RDTSC(P) scaling support Haozhong Zhang
@ 2016-02-24 15:09   ` Jan Beulich
  2016-02-26  4:31   ` Tian, Kevin
  1 sibling, 0 replies; 32+ messages in thread
From: Jan Beulich @ 2016-02-24 15:09 UTC (permalink / raw)
  To: Haozhong Zhang
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

>>> On 23.02.16 at 03:05, <haozhong.zhang@intel.com> wrote:
> This patch adds the initialization and setup code for VMX TSC scaling.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/6] x86/hvm: Setup TSC scaling ratio
  2016-02-24 15:01   ` Jan Beulich
@ 2016-02-24 15:42     ` Haozhong Zhang
  2016-02-24 15:51       ` Jan Beulich
  0 siblings, 1 reply; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-24 15:42 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Kevin Tian, Keir Fraser, Jun Nakajima, Andrew Cooper, xen-devel,
	Aravind Gopalakrishnan, Suravee Suthikulpanit, Boris Ostrovsky

On 02/24/16 08:01, Jan Beulich wrote:
> >>> On 23.02.16 at 03:05, <haozhong.zhang@intel.com> wrote:
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -298,6 +298,29 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
> >      return 1;
> >  }
> >  
> > +/*
> > + * Get the ratio to scale host TSC frequency to gtsc_khz. zero will be
> > + * returned if TSC scaling is unavailable or ratio cannot be handled
> > + * by host CPU. Otherwise, a non-zero ratio will be returned.
> > + */
> > +u64 hvm_get_tsc_scaling_ratio(u32 gtsc_khz)
> > +{
> > +    u64 ratio = gtsc_khz;
> > +    u64 dummy = 0;
> 
> "dummy" suggests it is unused, which it isn't. "tmp" or "hi" might be
> a little better, but since the meanings of the variables (also "ratio")
> differ for their roles an inputs and outputs, splitting inputs and
> outputs below would seem even better. In which case "dummy"
> become would an appropriate name again.
>

Yes, I'll split input and outputs for dummy and ratio.

> > +    if ( !hvm_tsc_scaling_supported )
> > +        return 0;
> > +
> > +    /* ratio = (gtsc_khz << hvm_funcs.tsc_scaling.ratio_frac_bits) / cpu_khz */
> > +    asm (
> > +        "shldq %2,%1,%0; salq %2,%1; divq %3"
> > +        : "+&d" (dummy), "+&a" (ratio)
> > +        : "c" (hvm_funcs.tsc_scaling.ratio_frac_bits),
> > +          "rm" ((u64) cpu_khz) );
> 
> And this DIVQ can't possibly #DE, e.g. when gtsc_khz is much larger
> than cpu_khz?
>

Oops, it could. Following check should be added before asm():
        /* the quotient is too large to fit in the integral part of TSC scaling ratio */
        if ( gtsc_khz / cpu_khz >
             (hvm_funcs.tsc_scaling.max_ratio >> hvm_funcs.tsc_scaling.ratio_frac_bits )
            return 0;

> I'd also prefer if the instruction got put on the same line as the
> "asm (". Considering that we're dealing with unsigned quantities
> here I'd further prefer if SHLQ was used instead of SALQ. And
> finally I'd suggest using named rather than numbered asm()
> arguments.
>

I'll make all these three changes.

> > --- a/xen/include/asm-x86/hvm/hvm.h
> > +++ b/xen/include/asm-x86/hvm/hvm.h
> > @@ -272,6 +272,14 @@ u64 hvm_get_guest_tsc_fixed(struct vcpu *v, u64 at_tsc);
> >  #define hvm_tsc_scaling_supported \
> >      (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
> >  
> > +#define hvm_default_tsc_scaling_ratio \
> > +    (1ULL << hvm_funcs.tsc_scaling.ratio_frac_bits)
> > +
> > +#define hvm_vcpu_tsc_scaling_ratio(v) \
> > +    ((v)->domain->arch.hvm_domain.tsc_scaling_ratio)
> 
> Since this is now a per-domain property I think it is misleading (and
> potentially hindering) for the called to pass in a struct vcpu * here.
> Please make this a struct domain *.
>

I'll change to struct domain *, and also remove _vcpu in the name.

Thanks,
Haozhong

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/6] x86/hvm: Setup TSC scaling ratio
  2016-02-24 15:42     ` Haozhong Zhang
@ 2016-02-24 15:51       ` Jan Beulich
  2016-02-24 16:05         ` Haozhong Zhang
  0 siblings, 1 reply; 32+ messages in thread
From: Jan Beulich @ 2016-02-24 15:51 UTC (permalink / raw)
  To: Haozhong Zhang
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

>>> On 24.02.16 at 16:42, <haozhong.zhang@intel.com> wrote:
> On 02/24/16 08:01, Jan Beulich wrote:
>> >>> On 23.02.16 at 03:05, <haozhong.zhang@intel.com> wrote:
>> > +    /* ratio = (gtsc_khz << hvm_funcs.tsc_scaling.ratio_frac_bits) / cpu_khz */
>> > +    asm (
>> > +        "shldq %2,%1,%0; salq %2,%1; divq %3"
>> > +        : "+&d" (dummy), "+&a" (ratio)
>> > +        : "c" (hvm_funcs.tsc_scaling.ratio_frac_bits),
>> > +          "rm" ((u64) cpu_khz) );
>> 
>> And this DIVQ can't possibly #DE, e.g. when gtsc_khz is much larger
>> than cpu_khz?
>>
> 
> Oops, it could. Following check should be added before asm():
>         /* the quotient is too large to fit in the integral part of TSC 
> scaling ratio */
>         if ( gtsc_khz / cpu_khz >
>              (hvm_funcs.tsc_scaling.max_ratio >> 
> hvm_funcs.tsc_scaling.ratio_frac_bits )
>             return 0;

Well, wouldn't that need to be >= then, since the division truncates?

Jan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/6] x86/hvm: Setup TSC scaling ratio
  2016-02-24 15:51       ` Jan Beulich
@ 2016-02-24 16:05         ` Haozhong Zhang
  0 siblings, 0 replies; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-24 16:05 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

On 02/24/16 08:51, Jan Beulich wrote:
> >>> On 24.02.16 at 16:42, <haozhong.zhang@intel.com> wrote:
> > On 02/24/16 08:01, Jan Beulich wrote:
> >> >>> On 23.02.16 at 03:05, <haozhong.zhang@intel.com> wrote:
> >> > +    /* ratio = (gtsc_khz << hvm_funcs.tsc_scaling.ratio_frac_bits) / cpu_khz */
> >> > +    asm (
> >> > +        "shldq %2,%1,%0; salq %2,%1; divq %3"
> >> > +        : "+&d" (dummy), "+&a" (ratio)
> >> > +        : "c" (hvm_funcs.tsc_scaling.ratio_frac_bits),
> >> > +          "rm" ((u64) cpu_khz) );
> >> 
> >> And this DIVQ can't possibly #DE, e.g. when gtsc_khz is much larger
> >> than cpu_khz?
> >>
> > 
> > Oops, it could. Following check should be added before asm():
> >         /* the quotient is too large to fit in the integral part of TSC 
> > scaling ratio */
> >         if ( gtsc_khz / cpu_khz >
> >              (hvm_funcs.tsc_scaling.max_ratio >> 
> > hvm_funcs.tsc_scaling.ratio_frac_bits )
> >             return 0;
> 
> Well, wouldn't that need to be >= then, since the division truncates?
>

No. The division truncation on (gtsc_khz/cpu_khz) gets the integral part
that is the part I want to check whether can fit in the integral part of
TSC scaling ratio.

Haozhong

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio
  2016-02-23  2:04 ` [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio Haozhong Zhang
  2016-02-23 14:00   ` Boris Ostrovsky
  2016-02-24 14:36   ` Jan Beulich
@ 2016-02-26  4:27   ` Tian, Kevin
  2 siblings, 0 replies; 32+ messages in thread
From: Tian, Kevin @ 2016-02-26  4:27 UTC (permalink / raw)
  To: Zhang, Haozhong, xen-devel, Jan Beulich, Boris Ostrovsky
  Cc: Nakajima, Jun, Andrew Cooper, Keir Fraser,
	Aravind Gopalakrishnan, Suravee Suthikulpanit

> From: Zhang, Haozhong
> Sent: Tuesday, February 23, 2016 10:05 AM
> 
> Both VMX TSC scaling and SVM TSC ratio use the 64-bit TSC scaling ratio,
> but the number of fractional bits of the ratio is different between VMX
> and SVM. This patch adds the architecture code to collect the number of
> fractional bits and other related information into fields of struct
> hvm_function_table so that they can be used in the common code.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 3/6] x86/hvm: Replace architecture TSC scaling by a common function
  2016-02-23  2:05 ` [PATCH v5 3/6] x86/hvm: Replace architecture TSC scaling by a common function Haozhong Zhang
  2016-02-24 15:07   ` Jan Beulich
@ 2016-02-26  4:30   ` Tian, Kevin
  1 sibling, 0 replies; 32+ messages in thread
From: Tian, Kevin @ 2016-02-26  4:30 UTC (permalink / raw)
  To: Zhang, Haozhong, xen-devel, Jan Beulich, Boris Ostrovsky
  Cc: Nakajima, Jun, Andrew Cooper, Keir Fraser,
	Aravind Gopalakrishnan, Suravee Suthikulpanit

> From: Zhang, Haozhong
> Sent: Tuesday, February 23, 2016 10:05 AM
> 
> This patch implements a common function hvm_scale_tsc() to scale TSC by
> using TSC scaling information collected by architecture code.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 5/6] vmx: Add VMX RDTSC(P) scaling support
  2016-02-23  2:05 ` [PATCH v5 5/6] vmx: Add VMX RDTSC(P) scaling support Haozhong Zhang
  2016-02-24 15:09   ` Jan Beulich
@ 2016-02-26  4:31   ` Tian, Kevin
  1 sibling, 0 replies; 32+ messages in thread
From: Tian, Kevin @ 2016-02-26  4:31 UTC (permalink / raw)
  To: Zhang, Haozhong, xen-devel, Jan Beulich, Boris Ostrovsky
  Cc: Nakajima, Jun, Andrew Cooper, Keir Fraser,
	Aravind Gopalakrishnan, Suravee Suthikulpanit

> From: Zhang, Haozhong
> Sent: Tuesday, February 23, 2016 10:05 AM
> 
> This patch adds the initialization and setup code for VMX TSC scaling.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt
  2016-02-23  2:05 ` [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt Haozhong Zhang
@ 2016-02-26  4:37   ` Tian, Kevin
  2016-02-26  4:44     ` Zhang, Haozhong
  2016-02-26  8:01     ` Jan Beulich
  0 siblings, 2 replies; 32+ messages in thread
From: Tian, Kevin @ 2016-02-26  4:37 UTC (permalink / raw)
  To: Zhang, Haozhong, xen-devel, Jan Beulich, Boris Ostrovsky
  Cc: Nakajima, Jun, Andrew Cooper, Keir Fraser,
	Aravind Gopalakrishnan, Suravee Suthikulpanit

> From: Zhang, Haozhong
> Sent: Tuesday, February 23, 2016 10:05 AM
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>, except:

> +
> +Hardware TSC Scaling
> +
> +Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
> +by guest rdtsc/p increasing in a different frequency than the host
> +TSC frequency.
> +
> +If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode

'HVM container' means something different. We usually use "HVM domain"
as you may see in other places in this doc.

Thanks
Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt
  2016-02-26  4:37   ` Tian, Kevin
@ 2016-02-26  4:44     ` Zhang, Haozhong
  2016-02-26  8:01     ` Jan Beulich
  1 sibling, 0 replies; 32+ messages in thread
From: Zhang, Haozhong @ 2016-02-26  4:44 UTC (permalink / raw)
  To: Tian, Kevin
  Cc: Keir Fraser, Jan Beulich, Nakajima, Jun, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Suravee Suthikulpanit,
	Boris Ostrovsky

On 02/26/16 12:37, Tian, Kevin wrote:
> > From: Zhang, Haozhong
> > Sent: Tuesday, February 23, 2016 10:05 AM
> > 
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>, except:
> 
> > +
> > +Hardware TSC Scaling
> > +
> > +Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
> > +by guest rdtsc/p increasing in a different frequency than the host
> > +TSC frequency.
> > +
> > +If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode
> 
> 'HVM container' means something different. We usually use "HVM domain"
> as you may see in other places in this doc.
>

I'll change to 'HVM domain'.

Thanks,
Haozhong

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt
  2016-02-26  4:37   ` Tian, Kevin
  2016-02-26  4:44     ` Zhang, Haozhong
@ 2016-02-26  8:01     ` Jan Beulich
  2016-02-26  8:05       ` Haozhong Zhang
  2016-02-29  2:02       ` Tian, Kevin
  1 sibling, 2 replies; 32+ messages in thread
From: Jan Beulich @ 2016-02-26  8:01 UTC (permalink / raw)
  To: Haozhong Zhang, Kevin Tian
  Cc: Keir Fraser, Suravee Suthikulpanit, Andrew Cooper, xen-devel,
	Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

>>> On 26.02.16 at 05:37, <kevin.tian@intel.com> wrote:
>>  From: Zhang, Haozhong
>> Sent: Tuesday, February 23, 2016 10:05 AM
>> 
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>, except:
> 
>> +
>> +Hardware TSC Scaling
>> +
>> +Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
>> +by guest rdtsc/p increasing in a different frequency than the host
>> +TSC frequency.
>> +
>> +If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode
> 
> 'HVM container' means something different. We usually use "HVM domain"
> as you may see in other places in this doc.

But I think this is specifically meant to refer to both HVM and PVH
domains.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt
  2016-02-26  8:01     ` Jan Beulich
@ 2016-02-26  8:05       ` Haozhong Zhang
  2016-02-29  2:02       ` Tian, Kevin
  1 sibling, 0 replies; 32+ messages in thread
From: Haozhong Zhang @ 2016-02-26  8:05 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Andrew Cooper,
	xen-devel, Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

On 02/26/16 01:01, Jan Beulich wrote:
> >>> On 26.02.16 at 05:37, <kevin.tian@intel.com> wrote:
> >>  From: Zhang, Haozhong
> >> Sent: Tuesday, February 23, 2016 10:05 AM
> >> 
> >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > 
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>, except:
> > 
> >> +
> >> +Hardware TSC Scaling
> >> +
> >> +Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
> >> +by guest rdtsc/p increasing in a different frequency than the host
> >> +TSC frequency.
> >> +
> >> +If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode
> > 
> > 'HVM container' means something different. We usually use "HVM domain"
> > as you may see in other places in this doc.
> 
> But I think this is specifically meant to refer to both HVM and PVH
> domains.
>

Yes, that is what I mean. So 'HVM container' is the correct name?

Haozhong

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt
  2016-02-26  8:01     ` Jan Beulich
  2016-02-26  8:05       ` Haozhong Zhang
@ 2016-02-29  2:02       ` Tian, Kevin
  2016-02-29  2:45         ` Zhang, Haozhong
  1 sibling, 1 reply; 32+ messages in thread
From: Tian, Kevin @ 2016-02-29  2:02 UTC (permalink / raw)
  To: Jan Beulich, Zhang, Haozhong
  Cc: Keir Fraser, Suravee Suthikulpanit, Andrew Cooper, xen-devel,
	Aravind Gopalakrishnan, Nakajima, Jun, Boris Ostrovsky

> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Friday, February 26, 2016 4:01 PM
> 
> >>> On 26.02.16 at 05:37, <kevin.tian@intel.com> wrote:
> >>  From: Zhang, Haozhong
> >> Sent: Tuesday, February 23, 2016 10:05 AM
> >>
> >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> >
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>, except:
> >
> >> +
> >> +Hardware TSC Scaling
> >> +
> >> +Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
> >> +by guest rdtsc/p increasing in a different frequency than the host
> >> +TSC frequency.
> >> +
> >> +If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode
> >
> > 'HVM container' means something different. We usually use "HVM domain"
> > as you may see in other places in this doc.
> 
> But I think this is specifically meant to refer to both HVM and PVH
> domains.
> 

First, I have a feeling that many people today refer to containers
running within a VM as 'VM container', which is a bit confusing to
'HVM container' purpose here. Couldn't we use 'HVM domains'
to cover both HVM and PVH (which is PV-HVM)? Curious whether
there is formal definition of those terminologies...

Second, even when 'HVM container' can be used as you explained,
it's inconsistent with other places in same doc, where only 'HVM
domain' is used. I'd think consistency is more important in this
patch series, and then if 'HVM container' is really preferred which
should be a separate patch to update all related docs.

Thanks
Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt
  2016-02-29  2:02       ` Tian, Kevin
@ 2016-02-29  2:45         ` Zhang, Haozhong
  2016-02-29  9:04           ` Jan Beulich
  0 siblings, 1 reply; 32+ messages in thread
From: Zhang, Haozhong @ 2016-02-29  2:45 UTC (permalink / raw)
  To: Tian, Kevin, Jan Beulich
  Cc: Keir Fraser, Suravee Suthikulpanit, Andrew Cooper, xen-devel,
	Aravind Gopalakrishnan, Nakajima, Jun, Boris Ostrovsky

On 02/29/16 10:02, Tian, Kevin wrote:
> > From: Jan Beulich [mailto:JBeulich@suse.com]
> > Sent: Friday, February 26, 2016 4:01 PM
> > 
> > >>> On 26.02.16 at 05:37, <kevin.tian@intel.com> wrote:
> > >>  From: Zhang, Haozhong
> > >> Sent: Tuesday, February 23, 2016 10:05 AM
> > >>
> > >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > >
> > > Reviewed-by: Kevin Tian <kevin.tian@intel.com>, except:
> > >
> > >> +
> > >> +Hardware TSC Scaling
> > >> +
> > >> +Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
> > >> +by guest rdtsc/p increasing in a different frequency than the host
> > >> +TSC frequency.
> > >> +
> > >> +If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode
> > >
> > > 'HVM container' means something different. We usually use "HVM domain"
> > > as you may see in other places in this doc.
> > 
> > But I think this is specifically meant to refer to both HVM and PVH
> > domains.
> > 
> 
> First, I have a feeling that many people today refer to containers
> running within a VM as 'VM container', which is a bit confusing to
> 'HVM container' purpose here. Couldn't we use 'HVM domains'
> to cover both HVM and PVH (which is PV-HVM)? Curious whether
> there is formal definition of those terminologies...
>

I call it 'HVM container' because I use has_hvm_container_domain(d)
| #define has_hvm_container_domain(d) ((d)->guest_type != guest_type_pv)
to check whether TSC scaling can be used by a domain, which, in current
implementation, is either a HVM domain (d->guest_type == guest_type_hvm)
or a PVH domain (d->guest_type == guest_type_pvh).

And I also noticed another macro is_hvm_domain(d)
| #define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
so I think 'HVM domain' can not be used to refer to both HVM and PVH
domains.

> Second, even when 'HVM container' can be used as you explained,
> it's inconsistent with other places in same doc, where only 'HVM
> domain' is used. I'd think consistency is more important in this
> patch series, and then if 'HVM container' is really preferred which
> should be a separate patch to update all related docs.
>

Or, maybe I should make it explicit, i.e. using 'HVM and PVH domains'
rather than 'HVM container'.

Haozhong

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt
  2016-02-29  2:45         ` Zhang, Haozhong
@ 2016-02-29  9:04           ` Jan Beulich
  0 siblings, 0 replies; 32+ messages in thread
From: Jan Beulich @ 2016-02-29  9:04 UTC (permalink / raw)
  To: Haozhong Zhang, Kevin Tian
  Cc: Keir Fraser, Suravee Suthikulpanit, Andrew Cooper, xen-devel,
	Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

>>> On 29.02.16 at 03:45, <haozhong.zhang@intel.com> wrote:
> On 02/29/16 10:02, Tian, Kevin wrote:
>> > From: Jan Beulich [mailto:JBeulich@suse.com]
>> > Sent: Friday, February 26, 2016 4:01 PM
>> > 
>> > >>> On 26.02.16 at 05:37, <kevin.tian@intel.com> wrote:
>> > >>  From: Zhang, Haozhong
>> > >> Sent: Tuesday, February 23, 2016 10:05 AM
>> > >>
>> > >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> > >
>> > > Reviewed-by: Kevin Tian <kevin.tian@intel.com>, except:
>> > >
>> > >> +
>> > >> +Hardware TSC Scaling
>> > >> +
>> > >> +Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
>> > >> +by guest rdtsc/p increasing in a different frequency than the host
>> > >> +TSC frequency.
>> > >> +
>> > >> +If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode
>> > >
>> > > 'HVM container' means something different. We usually use "HVM domain"
>> > > as you may see in other places in this doc.
>> > 
>> > But I think this is specifically meant to refer to both HVM and PVH
>> > domains.
>> > 
>> 
>> First, I have a feeling that many people today refer to containers
>> running within a VM as 'VM container', which is a bit confusing to
>> 'HVM container' purpose here. Couldn't we use 'HVM domains'
>> to cover both HVM and PVH (which is PV-HVM)? Curious whether
>> there is formal definition of those terminologies...
>>
> 
> I call it 'HVM container' because I use has_hvm_container_domain(d)
> | #define has_hvm_container_domain(d) ((d)->guest_type != guest_type_pv)
> to check whether TSC scaling can be used by a domain, which, in current
> implementation, is either a HVM domain (d->guest_type == guest_type_hvm)
> or a PVH domain (d->guest_type == guest_type_pvh).
> 
> And I also noticed another macro is_hvm_domain(d)
> | #define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
> so I think 'HVM domain' can not be used to refer to both HVM and PVH
> domains.

Indeed.

>> Second, even when 'HVM container' can be used as you explained,
>> it's inconsistent with other places in same doc, where only 'HVM
>> domain' is used. I'd think consistency is more important in this
>> patch series, and then if 'HVM container' is really preferred which
>> should be a separate patch to update all related docs.

Yes, inconsistencies in the docs are likely a result of them not
having got updated when PVH got introduced, of PVH existence
being neglected at the time they got put in.

> Or, maybe I should make it explicit, i.e. using 'HVM and PVH domains'
> rather than 'HVM container'.

That's an option, but personally I think a worse one than the
"HVM container" term.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2016-02-29  9:04 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-23  2:04 [PATCH v5 0/6] Add VMX TSC scaling support Haozhong Zhang
2016-02-23  2:04 ` [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio Haozhong Zhang
2016-02-23 14:00   ` Boris Ostrovsky
2016-02-23 14:10     ` Jan Beulich
2016-02-23 14:16       ` Boris Ostrovsky
2016-02-23 15:37         ` Jan Beulich
2016-02-24  6:00           ` Haozhong Zhang
2016-02-24 13:46             ` Haozhong Zhang
2016-02-24 14:49               ` Boris Ostrovsky
2016-02-24 14:36   ` Jan Beulich
2016-02-24 15:03     ` Haozhong Zhang
2016-02-26  4:27   ` Tian, Kevin
2016-02-23  2:05 ` [PATCH v5 2/6] x86/hvm: Setup " Haozhong Zhang
2016-02-24 15:01   ` Jan Beulich
2016-02-24 15:42     ` Haozhong Zhang
2016-02-24 15:51       ` Jan Beulich
2016-02-24 16:05         ` Haozhong Zhang
2016-02-23  2:05 ` [PATCH v5 3/6] x86/hvm: Replace architecture TSC scaling by a common function Haozhong Zhang
2016-02-24 15:07   ` Jan Beulich
2016-02-26  4:30   ` Tian, Kevin
2016-02-23  2:05 ` [PATCH v5 4/6] x86/hvm: Move saving/loading vcpu's TSC to common code Haozhong Zhang
2016-02-23  2:05 ` [PATCH v5 5/6] vmx: Add VMX RDTSC(P) scaling support Haozhong Zhang
2016-02-24 15:09   ` Jan Beulich
2016-02-26  4:31   ` Tian, Kevin
2016-02-23  2:05 ` [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt Haozhong Zhang
2016-02-26  4:37   ` Tian, Kevin
2016-02-26  4:44     ` Zhang, Haozhong
2016-02-26  8:01     ` Jan Beulich
2016-02-26  8:05       ` Haozhong Zhang
2016-02-29  2:02       ` Tian, Kevin
2016-02-29  2:45         ` Zhang, Haozhong
2016-02-29  9:04           ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.