[PATCH v3 00/13] Add VMX TSC scaling support

[PATCH v3 00/13] Add VMX TSC scaling support

[Haozhong Zhang]

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 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 (13):
  x86/time.c: Use correct guest TSC frequency in tsc_set_info()
  x86/time.c: Use correct guest TSC frequency in tsc_get_info()
  x86/hvm: Scale host TSC when setting/getting guest TSC
  x86/time.c: Scale host TSC in pvclock properly
  svm: Remove redundant TSC scaling in svm_set_tsc_offset()
  x86/hvm: Collect information of TSC scaling ratio
  x86: Add functions for 64-bit integer arithmetic
  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
  x86/hvm: Detect TSC scaling through hvm_funcs
  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             |  73 ++++++++++++++++++++++----
 xen/arch/x86/hvm/svm/svm.c         |  30 ++++-------
 xen/arch/x86/hvm/vmx/vmcs.c        |  12 +++--
 xen/arch/x86/hvm/vmx/vmx.c         |  20 +++++--
 xen/arch/x86/time.c                |  52 +++++++++++++-----
 xen/include/asm-x86/domain.h       |   9 ++--
 xen/include/asm-x86/hvm/hvm.h      |  19 +++++++
 xen/include/asm-x86/hvm/svm/svm.h  |   3 --
 xen/include/asm-x86/hvm/vcpu.h     |   2 +
 xen/include/asm-x86/hvm/vmx/vmcs.h |   7 +++
 xen/include/asm-x86/math64.h       | 105 +++++++++++++++++++++++++++++++++++++
 13 files changed, 311 insertions(+), 56 deletions(-)
 create mode 100644 xen/include/asm-x86/math64.h

-- 
2.4.8

[PATCH v3 01/13] x86/time.c: Use correct guest TSC frequency in tsc_set_info()

[Haozhong Zhang]

When TSC_MODE_PVRDTSCP is used for a HVM container and TSC scaling is
available, use the non-zero value of argument gtsc_khz of tsc_set_info()
as the guest TSC frequency rather than using the host TSC
frequency. Otherwise, TSC scaling will not be able get the correct ratio
between the host and guest TSC frequencies.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v3:
 (addressing Boris Ostrovsky's comments)
 * Add the missing ! before d->arch.vtsc when initializing enable_tsc_scaling.
 * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.
 * Adapt comments for some TSC-related fields in struct arch_domain.

 xen/arch/x86/time.c          | 9 +++++++--
 xen/include/asm-x86/domain.h | 9 ++++++---
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index b5223cf..0059b6a 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1803,6 +1803,8 @@ void tsc_set_info(struct domain *d,
                   uint32_t tsc_mode, uint64_t elapsed_nsec,
                   uint32_t gtsc_khz, uint32_t incarnation)
 {
+    bool_t enable_tsc_scaling;
+
     if ( is_idle_domain(d) || is_hardware_domain(d) )
     {
         d->arch.vtsc = 0;
@@ -1864,7 +1866,9 @@ void tsc_set_info(struct domain *d,
     case TSC_MODE_PVRDTSCP:
         d->arch.vtsc = !boot_cpu_has(X86_FEATURE_RDTSCP) ||
                        !host_tsc_is_safe();
-        d->arch.tsc_khz = cpu_khz;
+        enable_tsc_scaling = has_hvm_container_domain(d) &&
+                             cpu_has_tsc_ratio && !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);
         if ( d->arch.vtsc )
@@ -1872,7 +1876,8 @@ void tsc_set_info(struct domain *d,
         else {
             /* when using native TSC, offset is nsec relative to power-on
              * of physical machine */
-            d->arch.vtsc_offset = scale_delta(rdtsc(), &d->arch.vtsc_to_ns) -
+            d->arch.vtsc_offset = scale_delta(rdtsc(),
+                                              &this_cpu(cpu_time).tsc_scale) -
                                   elapsed_nsec;
         }
         break;
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index e8f7037..405adef 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -347,9 +347,12 @@ struct arch_domain
     s_time_t vtsc_last;      /* previous TSC value (guarantee monotonicity) */
     spinlock_t vtsc_lock;
     uint64_t vtsc_offset;    /* adjustment for save/restore/migrate */
-    uint32_t tsc_khz;        /* cached khz for certain emulated cases */
-    struct time_scale vtsc_to_ns; /* scaling for certain emulated cases */
-    struct time_scale ns_to_vtsc; /* scaling for certain emulated cases */
+    uint32_t tsc_khz;        /* cached guest khz for certain emulated or
+                                hardware TSC scaling cases */
+    struct time_scale vtsc_to_ns; /* scaling for certain emulated or
+                                     hardware TSC scaling cases */
+    struct time_scale ns_to_vtsc; /* scaling for certain emulated or
+                                     hardware TSC scaling cases */
     uint32_t incarnation;    /* incremented every restore or live migrate
                                 (possibly other cases in the future */
 #if !defined(NDEBUG) || defined(PERF_COUNTERS)
-- 
2.4.8

[PATCH v3 02/13] x86/time.c: Use correct guest TSC frequency in tsc_get_info()

[Haozhong Zhang]

When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
frequency. However, tsc_set_info() may set the guest TSC frequency to a
value different than the host. In order to keep consistent to
tsc_set_info(), this patch makes tsc_get_info() use the value set by
tsc_set_info() as the guest TSC frequency.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v3:
 (addressing Boris Ostrovsky's comments)
 * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.

 xen/arch/x86/time.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 0059b6a..d83f068 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
                   uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
                   uint32_t *incarnation)
 {
+    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
+                                cpu_has_tsc_ratio;
+
     *incarnation = d->arch.incarnation;
     *tsc_mode = d->arch.tsc_mode;
 
@@ -1769,7 +1772,7 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
         }
         tsc = rdtsc();
         *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns);
-        *gtsc_khz = cpu_khz;
+        *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : cpu_khz;
         break;
     case TSC_MODE_PVRDTSCP:
         if ( d->arch.vtsc )
@@ -1780,9 +1783,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
         else
         {
             tsc = rdtsc();
-            *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
+            *elapsed_nsec = scale_delta(tsc, &this_cpu(cpu_time).tsc_scale) -
                             d->arch.vtsc_offset;
-            *gtsc_khz = 0; /* ignored by tsc_set_info */
+            *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : 0;
         }
         break;
     }
-- 
2.4.8

[PATCH v3 03/13] x86/hvm: Scale host TSC when setting/getting guest TSC

[Haozhong Zhang]

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>
---
 xen/arch/x86/hvm/hvm.c        | 17 +++++++----------
 xen/arch/x86/hvm/svm/svm.c    | 12 ++++++++++++
 xen/include/asm-x86/hvm/hvm.h |  2 ++
 3 files changed, 21 insertions(+), 10 deletions(-)

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..c538a29 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -804,6 +804,16 @@ static uint64_t scale_tsc(uint64_t host_tsc, uint64_t ratio)
     return scaled_host_tsc;
 }
 
+static uint64_t svm_scale_tsc(struct vcpu *v, uint64_t tsc)
+{
+    struct domain *d = v->domain;
+
+    if ( !cpu_has_tsc_ratio || d->arch.vtsc )
+        return tsc;
+
+    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 +2282,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..ba6259e 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)(struct vcpu *v, uint64_t tsc);
 };
 
 extern struct hvm_function_table hvm_funcs;
-- 
2.4.8

[PATCH v3 04/13] x86/time.c: Scale host TSC in pvclock properly

[Haozhong Zhang]

This patch makes the pvclock return the scaled host TSC and
corresponding scaling parameters to HVM domains if guest TSC is not
emulated and TSC scaling is enabled.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v3:
 (addressing Boris Ostrovsky's comments)
 * No changes in fact. tsc_set_info() does not set d->arch.vtsc to 0
   if host_tsc_is_safe() is not satisfied, so it is safe to use
   d->arch.vtsc_to_ns here.

 xen/arch/x86/time.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index d83f068..b31634a 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -815,10 +815,18 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
     }
     else
     {
-        tsc_stamp = t->local_tsc_stamp;
-
-        _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
-        _u.tsc_shift         = (s8)t->tsc_scale.shift;
+        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
+        {
+            tsc_stamp            = hvm_funcs.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;
+        }
+        else
+        {
+            tsc_stamp            = t->local_tsc_stamp;
+            _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
+            _u.tsc_shift         = (s8)t->tsc_scale.shift;
+        }
     }
 
     _u.tsc_timestamp = tsc_stamp;
-- 
2.4.8

[PATCH v3 05/13] svm: Remove redundant TSC scaling in svm_set_tsc_offset()

[Haozhong Zhang]

Now every caller passes an already scaled offset to
svm_set_tsc_offset(), so it's not necessary to recalculate a scaled TSC
offset in svm_set_tsc_offset().

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v3:
 (addressing Boris Ostrovsky's comments)
 * Move declaration of guest_tsc to its first use site.

 xen/arch/x86/hvm/svm/svm.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index c538a29..8891c78 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -826,19 +826,6 @@ static void svm_set_tsc_offset(struct vcpu *v, u64 offset, u64 at_tsc)
     struct vmcb_struct *n1vmcb, *n2vmcb;
     uint64_t n2_tsc_offset = 0;
     struct domain *d = v->domain;
-    uint64_t host_tsc, guest_tsc;
-
-    guest_tsc = hvm_get_guest_tsc_fixed(v, at_tsc);
-
-    /* Re-adjust the offset value when TSC_RATIO is available */
-    if ( cpu_has_tsc_ratio && !d->arch.vtsc )
-    {
-        if ( at_tsc )
-            host_tsc = at_tsc;
-        else
-            host_tsc = rdtsc();
-        offset = svm_get_tsc_offset(host_tsc, guest_tsc, vcpu_tsc_ratio(v));
-    }
 
     if ( !nestedhvm_enabled(d) ) {
         vmcb_set_tsc_offset(vmcb, offset);
@@ -854,6 +841,7 @@ static void svm_set_tsc_offset(struct vcpu *v, u64 offset, u64 at_tsc)
         n2_tsc_offset = vmcb_get_tsc_offset(n2vmcb) -
             vmcb_get_tsc_offset(n1vmcb);
         if ( svm->ns_tscratio != DEFAULT_TSC_RATIO ) {
+            uint64_t guest_tsc = hvm_get_guest_tsc_fixed(v, at_tsc);
             n2_tsc_offset = svm_get_tsc_offset(guest_tsc,
                 guest_tsc + n2_tsc_offset, svm->ns_tscratio);
         }
-- 
2.4.8

[PATCH v3 06/13] x86/hvm: Collect information of TSC scaling ratio

[Haozhong Zhang]

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>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v3:
 (addressing Kevin Tian's comments)
 * Fix a typo: maxmimum -> maximum

 xen/arch/x86/hvm/svm/svm.c    |  7 +++++++
 xen/include/asm-x86/hvm/hvm.h | 12 ++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 8891c78..95795ce 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1451,6 +1451,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_supported = 1;
+
 #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");
@@ -2272,6 +2275,10 @@ static struct hvm_function_table __initdata svm_function_table = {
     .nhvm_hap_walk_L1_p2m = nsvm_hap_walk_L1_p2m,
 
     .scale_tsc            = svm_scale_tsc,
+
+    .default_tsc_scaling_ratio   = DEFAULT_TSC_RATIO,
+    .max_tsc_scaling_ratio       = ~TSC_RATIO_RSVD_BITS,
+    .tsc_scaling_ratio_frac_bits = 32,
 };
 
 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 ba6259e..12f9f24 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -100,6 +100,18 @@ struct hvm_function_table {
     unsigned int hap_capabilities;
 
     /*
+     * Parameters of hardware-assisted TSC scaling.
+     */
+    /* is TSC scaling supported? */
+    bool_t   tsc_scaling_supported;
+    /* number of bits of the fractional part of TSC scaling ratio */
+    uint8_t  tsc_scaling_ratio_frac_bits;
+    /* default TSC scaling ratio (no scaling) */
+    uint64_t default_tsc_scaling_ratio;
+    /* maximum-allowed TSC scaling ratio */
+    uint64_t max_tsc_scaling_ratio;
+
+    /*
      * Initialise/destroy HVM domain/vcpu resources
      */
     int  (*domain_initialise)(struct domain *d);
-- 
2.4.8

[PATCH v3 07/13] x86: Add functions for 64-bit integer arithmetic

[Haozhong Zhang]

This patch adds several functions to take multiplication, division and
shifting involving 64-bit integers. Those functions are derived from
Linux kernel and will be used by later patches to calculate scaling
ratio and scaled TSC.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v3:
 (addressing Boris Ostrovsky's comments in v2 patch 9 & 10)
 * Move all math64 stuffs to this standalone patch.
 * Add comments to state these functions are derived from Linux kernel.

 xen/include/asm-x86/math64.h | 105 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 xen/include/asm-x86/math64.h

diff --git a/xen/include/asm-x86/math64.h b/xen/include/asm-x86/math64.h
new file mode 100644
index 0000000..f7624ef
--- /dev/null
+++ b/xen/include/asm-x86/math64.h
@@ -0,0 +1,105 @@
+#ifndef __X86_MATH64
+#define __X86_MATH64
+
+/*
+ * Functions defined in this file are derived from Linux kernel
+ * (include/linux/math64.h).
+ */
+
+/*
+ * (a * mul) / divisor
+ */
+static inline u64 mul_u64_u32_div(u64 a, u32 mul, u32 divisor)
+{
+    union {
+        u64 ll;
+        struct {
+            u32 low, high;
+        } l;
+    } u, rl, rh;
+
+    u.ll = a;
+    rl.ll = (u64)u.l.low * mul;
+    rh.ll = (u64)u.l.high * mul + rl.l.high;
+
+    /* Bits 32-63 of the result will be in rh.l.low. */
+    rl.l.high = do_div(rh.ll, divisor);
+
+    /* Bits 0-31 of the result will be in rl.l.low. */
+    do_div(rl.ll, divisor);
+
+    rl.l.high = rh.l.low;
+    return rl.ll;
+}
+
+/*
+ * Multiply two 64-bit unsigned integers a and b. The most and least
+ * significant 64 bits of the 128-bit result are returned through hi
+ * and lo respectively.
+ */
+static inline void mul64(u64 *lo, u64 *hi, u64 a, u64 b)
+{
+    typedef union {
+        u64 ll;
+        struct {
+            u32 low, high;
+        } l;
+    } LL;
+    LL rl, rm, rn, rh, a0, b0;
+    u64 c;
+
+    a0.ll = a;
+    b0.ll = b;
+
+    rl.ll = (u64)a0.l.low * b0.l.low;
+    rm.ll = (u64)a0.l.low * b0.l.high;
+    rn.ll = (u64)a0.l.high * b0.l.low;
+    rh.ll = (u64)a0.l.high * b0.l.high;
+
+    c = (u64)rl.l.high + rm.l.low + rn.l.low;
+    rl.l.high = c;
+    c >>= 32;
+    c = c + rm.l.high + rn.l.high + rh.l.low;
+    rh.l.low = c;
+    rh.l.high += (u32)(c >> 32);
+
+    *lo = rl.ll;
+    *hi = rh.ll;
+}
+
+/*
+ * Right shift a 128-bit unsigned integer by n bits. The most and
+ * least significant 64 bits of the 128-bit integer are passed through
+ * hi and lo respectively. The most and least significant 64 bits of
+ * the result are also returned through hi and lo respectively.
+ */
+static inline void rshift128(u64 *lo, u64 *hi, unsigned int n)
+{
+    u64 h;
+    if ( !n )
+        return;
+    h = *hi >> (n & 63);
+    if ( n >= 64 )
+    {
+        *hi = 0;
+        *lo = h;
+    }
+    else
+    {
+        *lo = (*lo >> n) | (*hi << (64 - n));
+        *hi = h;
+    }
+}
+
+/*
+ * (a * mul) >> n
+ */
+static inline u64 mul_u64_u64_shr(u64 a, u64 mul, unsigned int n)
+{
+    u64 lo, hi;
+    mul64(&lo, &hi, a, mul);
+    rshift128(&lo, &hi, n);
+    return lo;
+}
+
+#endif
-- 
2.4.8

[PATCH v3 08/13] x86/hvm: Setup TSC scaling ratio

[Haozhong Zhang]

This patch adds a field tsc_scaling_ratio in struct hvm_vcpu to
record the TSC scaling ratio, and sets it up when tsc_set_info() is
called for a vcpu or when a vcpu is restored or reset.

Before applying the TSC scaling ratio to CPU, 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>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v3:
 (addressing Boris Ostrovsky's comments)
 * Remove redundant checks.
 * Update comments in tsc_set_info(): during initial boot -> during domain creation.
 (addressing Kevin Tian's comments)
 * Add a validation step to check TSC scaling ratio before applying it
   to the hardware. If the validation fails, we will fallback to Xen's
   original way to handle guest TSC. hvm_setup_tsc_scaling() is always
   called when the validation succeeds, so it shoud never fail now.

 xen/arch/x86/hvm/hvm.c            | 41 +++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/hvm/svm/svm.c        |  6 ++++--
 xen/arch/x86/time.c               | 19 +++++++++++++++---
 xen/include/asm-x86/hvm/hvm.h     |  6 ++++++
 xen/include/asm-x86/hvm/svm/svm.h |  3 ---
 xen/include/asm-x86/hvm/vcpu.h    |  2 ++
 6 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3648a44..3bf99b1 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -65,6 +65,7 @@
 #include <asm/mtrr.h>
 #include <asm/apic.h>
 #include <asm/vm_event.h>
+#include <asm/math64.h>
 #include <public/sched.h>
 #include <public/hvm/ioreq.h>
 #include <public/version.h>
@@ -301,6 +302,42 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
     return 1;
 }
 
+bool_t hvm_validate_tsc_scaling_ratio(uint32_t gtsc_khz)
+{
+    u64 ratio;
+
+    if ( !hvm_funcs.tsc_scaling_supported )
+        return FALSE;
+
+    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
+                            gtsc_khz, cpu_khz);
+
+    return (!ratio || ratio > hvm_funcs.max_tsc_scaling_ratio) ? FALSE : TRUE;
+}
+
+void hvm_setup_tsc_scaling(struct vcpu *v)
+{
+    u64 ratio;
+
+    if ( !hvm_funcs.tsc_scaling_supported )
+        return;
+
+    /*
+     * The multiplication of the first two terms may overflow a 64-bit
+     * integer, so use mul_u64_u32_div() instead to keep precision.
+     */
+    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
+                            v->domain->arch.tsc_khz, cpu_khz);
+
+    if ( ratio == 0 || ratio > hvm_funcs.max_tsc_scaling_ratio )
+        return;
+
+    v->arch.hvm_vcpu.tsc_scaling_ratio = ratio;
+
+    if ( hvm_funcs.setup_tsc_scaling )
+        hvm_funcs.setup_tsc_scaling(v);
+}
+
 void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
 {
     uint64_t tsc;
@@ -2026,6 +2063,8 @@ 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;
 
+    hvm_setup_tsc_scaling(v);
+
     v->arch.hvm_vcpu.msr_tsc_aux = ctxt.msr_tsc_aux;
 
     seg.limit = ctxt.idtr_limit;
@@ -5504,6 +5543,8 @@ 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);
 
+    hvm_setup_tsc_scaling(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/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 95795ce..b58f1e8 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -811,7 +811,7 @@ static uint64_t svm_scale_tsc(struct vcpu *v, uint64_t tsc)
     if ( !cpu_has_tsc_ratio || d->arch.vtsc )
         return tsc;
 
-    return scale_tsc(tsc, vcpu_tsc_ratio(v));
+    return scale_tsc(tsc, v->arch.hvm_vcpu.tsc_scaling_ratio);
 }
 
 static uint64_t svm_get_tsc_offset(uint64_t host_tsc, uint64_t guest_tsc,
@@ -986,7 +986,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, v->arch.hvm_vcpu.tsc_scaling_ratio);
 }
 
 static void svm_ctxt_switch_from(struct vcpu *v)
@@ -1193,6 +1193,8 @@ static int svm_vcpu_initialise(struct vcpu *v)
 
     svm_guest_osvw_init(v);
 
+    v->arch.hvm_vcpu.tsc_scaling_ratio = DEFAULT_TSC_RATIO;
+
     return 0;
 }
 
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index b31634a..e69d9de 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1864,7 +1864,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 || cpu_has_tsc_ratio :
+              (d->arch.tsc_khz == cpu_khz ||
+               hvm_validate_tsc_scaling_ratio(d->arch.tsc_khz)) :
               incarnation == 0) )
         {
     case TSC_MODE_NEVER_EMULATE:
@@ -1878,7 +1879,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) &&
-                             cpu_has_tsc_ratio && !d->arch.vtsc;
+                             !d->arch.vtsc &&
+                             hvm_validate_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,9 +1899,20 @@ void tsc_set_info(struct domain *d,
     if ( has_hvm_container_domain(d) )
     {
         hvm_set_rdtsc_exiting(d, d->arch.vtsc);
-        if ( d->vcpu && d->vcpu[0] && incarnation == 0 )
+        if ( d->vcpu && d->vcpu[0] )
         {
             /*
+             * TSC scaling ratio on BSP is set here during domain
+             * creation, while the same TSC scaling ratio on APs will
+             * be set in hvm_vcpu_reset_state().
+             */
+            if ( !d->arch.vtsc )
+                hvm_setup_tsc_scaling(d->vcpu[0]);
+
+            if ( incarnation )
+                return;
+
+            /*
              * set_tsc_offset() is called from hvm_vcpu_initialise() before
              * tsc_set_info(). New vtsc mode may require recomputing TSC
              * offset.
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 12f9f24..6b49da8 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -226,6 +226,9 @@ struct hvm_function_table {
     int (*altp2m_vcpu_emulate_vmfunc)(struct cpu_user_regs *regs);
 
     uint64_t (*scale_tsc)(struct vcpu *v, uint64_t tsc);
+
+    /* Architecture function to setup TSC scaling ratio */
+    void (*setup_tsc_scaling)(struct vcpu *v);
 };
 
 extern struct hvm_function_table hvm_funcs;
@@ -261,6 +264,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)
 
+bool_t hvm_validate_tsc_scaling_ratio(uint32_t gtsc_khz);
+void hvm_setup_tsc_scaling(struct vcpu *v);
+
 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);
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index 152d9f3..901c988 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -175,6 +175,8 @@ struct hvm_vcpu {
     u64                 msr_tsc_adjust;
     u64                 msr_xss;
 
+    u64                 tsc_scaling_ratio;
+
     union {
         struct arch_vmx_struct vmx;
         struct arch_svm_struct svm;
-- 
2.4.8

[PATCH v3 09/13] x86/hvm: Replace architecture TSC scaling by a common function

[Haozhong Zhang]

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> (except the mul64 part)
---
Changes in v3:
 (addressing Boris Ostrovsky's comments)
 * Remove unnecessary assertions in hvm_scale_tsc().
 * Move math64 stuffs to a standalone patch and state their origination.

 xen/arch/x86/hvm/hvm.c        | 15 +++++++++++++--
 xen/arch/x86/hvm/svm/svm.c    | 12 ------------
 xen/arch/x86/time.c           |  2 +-
 xen/include/asm-x86/hvm/hvm.h |  3 +--
 4 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3bf99b1..a900b4c 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -302,6 +302,17 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
     return 1;
 }
 
+u64 hvm_scale_tsc(const struct vcpu *v, u64 tsc)
+{
+    u64 ratio = v->arch.hvm_vcpu.tsc_scaling_ratio;
+
+    if ( !hvm_funcs.tsc_scaling_supported ||
+         ratio == hvm_funcs.default_tsc_scaling_ratio )
+        return tsc;
+
+    return mul_u64_u64_shr(tsc, ratio, hvm_funcs.tsc_scaling_ratio_frac_bits);
+}
+
 bool_t hvm_validate_tsc_scaling_ratio(uint32_t gtsc_khz)
 {
     u64 ratio;
@@ -352,7 +363,7 @@ void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
     {
         tsc = at_tsc ?: rdtsc();
         if ( cpu_has_tsc_ratio )
-            tsc = hvm_funcs.scale_tsc(v, tsc);
+            tsc = hvm_scale_tsc(v, tsc);
     }
 
     delta_tsc = guest_tsc - tsc;
@@ -384,7 +395,7 @@ u64 hvm_get_guest_tsc_fixed(struct vcpu *v, uint64_t at_tsc)
     {
         tsc = at_tsc ?: rdtsc();
         if ( cpu_has_tsc_ratio )
-            tsc = hvm_funcs.scale_tsc(v, tsc);
+            tsc = hvm_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 b58f1e8..71538c8 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -804,16 +804,6 @@ static uint64_t scale_tsc(uint64_t host_tsc, uint64_t ratio)
     return scaled_host_tsc;
 }
 
-static uint64_t svm_scale_tsc(struct vcpu *v, uint64_t tsc)
-{
-    struct domain *d = v->domain;
-
-    if ( !cpu_has_tsc_ratio || d->arch.vtsc )
-        return tsc;
-
-    return scale_tsc(tsc, v->arch.hvm_vcpu.tsc_scaling_ratio);
-}
-
 static uint64_t svm_get_tsc_offset(uint64_t host_tsc, uint64_t guest_tsc,
     uint64_t ratio)
 {
@@ -2276,8 +2266,6 @@ 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,
-
     .default_tsc_scaling_ratio   = DEFAULT_TSC_RATIO,
     .max_tsc_scaling_ratio       = ~TSC_RATIO_RSVD_BITS,
     .tsc_scaling_ratio_frac_bits = 32,
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index e69d9de..8d391e0 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -817,7 +817,7 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
     {
         if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
         {
-            tsc_stamp            = hvm_funcs.scale_tsc(v, t->local_tsc_stamp);
+            tsc_stamp            = hvm_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;
         }
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 6b49da8..05894ee 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -225,8 +225,6 @@ 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)(struct vcpu *v, uint64_t tsc);
-
     /* Architecture function to setup TSC scaling ratio */
     void (*setup_tsc_scaling)(struct vcpu *v);
 };
@@ -264,6 +262,7 @@ 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)
 
+u64 hvm_scale_tsc(const struct vcpu *v, u64 tsc);
 bool_t hvm_validate_tsc_scaling_ratio(uint32_t gtsc_khz);
 void hvm_setup_tsc_scaling(struct vcpu *v);
 
-- 
2.4.8

[PATCH v3 10/13] x86/hvm: Move saving/loading vcpu's TSC to common code

[Haozhong Zhang]

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 a900b4c..7880b4c 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1783,6 +1783,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);
@@ -2078,6 +2080,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 71538c8..565b3c2 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 b918b8a..f26a1bc 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -587,9 +587,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)
@@ -604,8 +601,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.4.8

[PATCH v3 11/13] x86/hvm: Detect TSC scaling through hvm_funcs

[Haozhong Zhang]

This patch uses hvm_funcs.tsc_scaling_supported instead of the
architecture code to detect the TSC scaling support.

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

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 8d391e0..d14df13 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,7 +814,7 @@ 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_funcs.tsc_scaling_supported )
         {
             tsc_stamp            = hvm_scale_tsc(v, t->local_tsc_stamp);
             _u.tsc_to_system_mul = d->arch.vtsc_to_ns.mul_frac;
@@ -1758,7 +1757,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;
+                                hvm_funcs.tsc_scaling_supported;
 
     *incarnation = d->arch.incarnation;
     *tsc_mode = d->arch.tsc_mode;
-- 
2.4.8

[PATCH v3 12/13] vmx: Add VMX RDTSC(P) scaling support

[Haozhong Zhang]

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/arch/x86/hvm/vmx/vmcs.c        | 12 +++++++++---
 xen/arch/x86/hvm/vmx/vmx.c         | 15 +++++++++++++++
 xen/include/asm-x86/hvm/vmx/vmcs.h |  7 +++++++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index edd4c8d..8f16c3a 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 )
@@ -242,7 +243,8 @@ static int vmx_init_vmcs_config(void)
                SECONDARY_EXEC_ENABLE_INVPCID |
                SECONDARY_EXEC_ENABLE_VM_FUNCTIONS |
                SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS |
-               SECONDARY_EXEC_XSAVES);
+               SECONDARY_EXEC_XSAVES |
+               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;
@@ -999,7 +1001,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;
@@ -1281,6 +1283,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, v->arch.hvm_vcpu.tsc_scaling_ratio);
+
     vmx_vmcs_exit(v);
 
     /* PVH: paging mode is updated by arch_set_info_guest(). */
@@ -1863,7 +1868,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 f26a1bc..4c7214d 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -115,6 +115,7 @@ static int vmx_vcpu_initialise(struct vcpu *v)
     v->arch.schedule_tail    = vmx_do_resume;
     v->arch.ctxt_switch_from = vmx_ctxt_switch_from;
     v->arch.ctxt_switch_to   = vmx_ctxt_switch_to;
+    v->arch.hvm_vcpu.tsc_scaling_ratio = VMX_TSC_MULTIPLIER_DEFAULT;
 
     if ( (rc = vmx_create_vmcs(v)) != 0 )
     {
@@ -1105,6 +1106,13 @@ static void vmx_handle_cd(struct vcpu *v, unsigned long value)
     }
 }
 
+static void vmx_setup_tsc_scaling(struct vcpu *v)
+{
+    vmx_vmcs_enter(v);
+    __vmwrite(TSC_MULTIPLIER, v->arch.hvm_vcpu.tsc_scaling_ratio);
+    vmx_vmcs_exit(v);
+}
+
 static void vmx_set_tsc_offset(struct vcpu *v, u64 offset, u64 at_tsc)
 {
     vmx_vmcs_enter(v);
@@ -2003,6 +2011,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,
+    .default_tsc_scaling_ratio   = VMX_TSC_MULTIPLIER_DEFAULT,
+    .max_tsc_scaling_ratio       = VMX_TSC_MULTIPLIER_MAX,
+    .tsc_scaling_ratio_frac_bits = 48,
+    .setup_tsc_scaling           = vmx_setup_tsc_scaling,
 };
 
 /* Handle VT-d posted-interrupt when VCPU is running. */
@@ -2107,6 +2119,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_supported = 1;
+
     setup_vmcs_dump();
 
     return &vmx_function_table;
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index d1496b8..fdece44 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -236,6 +236,7 @@ extern u32 vmx_vmentry_control;
 #define SECONDARY_EXEC_ENABLE_PML               0x00020000
 #define SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS   0x00040000
 #define SECONDARY_EXEC_XSAVES                   0x00100000
+#define SECONDARY_EXEC_TSC_SCALING              0x02000000
 extern u32 vmx_secondary_exec_control;
 
 #define VMX_EPT_EXEC_ONLY_SUPPORTED                         0x00000001
@@ -258,6 +259,9 @@ extern u64 vmx_ept_vpid_cap;
 #define VMX_MISC_CR3_TARGET                     0x01ff0000
 #define VMX_MISC_VMWRITE_ALL                    0x20000000
 
+#define VMX_TSC_MULTIPLIER_DEFAULT              0x0001000000000000ULL
+#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 \
@@ -303,6 +307,8 @@ extern u64 vmx_ept_vpid_cap;
     (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_PML)
 #define cpu_has_vmx_xsaves \
     (vmx_secondary_exec_control & SECONDARY_EXEC_XSAVES)
+#define cpu_has_vmx_tsc_scaling \
+    (vmx_secondary_exec_control & SECONDARY_EXEC_TSC_SCALING)
 
 #define VMCS_RID_TYPE_MASK              0x80000000
 
@@ -378,6 +384,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.4.8

[PATCH v3 13/13] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt

[Haozhong Zhang]

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v3:
 (addressing Kevin Tian's comments)
 * Reorganize words to address Kevin's comments.

 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 8899f75..47aea0a 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.4.8

Re: [PATCH v3 01/13] x86/time.c: Use correct guest TSC frequency in tsc_set_info()

[Boris Ostrovsky]

On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> When TSC_MODE_PVRDTSCP is used for a HVM container and TSC scaling is
> available, use the non-zero value of argument gtsc_khz of tsc_set_info()
> as the guest TSC frequency rather than using the host TSC
> frequency. Otherwise, TSC scaling will not be able get the correct ratio
> between the host and guest TSC frequencies.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

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

Re: [PATCH v3 02/13] x86/time.c: Use correct guest TSC frequency in tsc_get_info()

[Boris Ostrovsky]

On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
> TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
> tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
> frequency. However, tsc_set_info() may set the guest TSC frequency to a
> value different than the host. In order to keep consistent to
> tsc_set_info(), this patch makes tsc_get_info() use the value set by
> tsc_set_info() as the guest TSC frequency.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> Changes in v3:
>   (addressing Boris Ostrovsky's comments)
>   * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.
>
>   xen/arch/x86/time.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index 0059b6a..d83f068 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>                     uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
>                     uint32_t *incarnation)
>   {
> +    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
> +                                cpu_has_tsc_ratio;
> +

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

although I still think that having !d->arch.vtsc would be better, even 
if not strictly required.



>       *incarnation = d->arch.incarnation;
>       *tsc_mode = d->arch.tsc_mode;
>   
> @@ -1769,7 +1772,7 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>           }
>           tsc = rdtsc();
>           *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns);
> -        *gtsc_khz = cpu_khz;
> +        *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : cpu_khz;
>           break;
>       case TSC_MODE_PVRDTSCP:
>           if ( d->arch.vtsc )
> @@ -1780,9 +1783,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>           else
>           {
>               tsc = rdtsc();
> -            *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
> +            *elapsed_nsec = scale_delta(tsc, &this_cpu(cpu_time).tsc_scale) -
>                               d->arch.vtsc_offset;
> -            *gtsc_khz = 0; /* ignored by tsc_set_info */
> +            *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : 0;
>           }
>           break;
>       }

Re: [PATCH v3 04/13] x86/time.c: Scale host TSC in pvclock properly

[Boris Ostrovsky]

On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> This patch makes the pvclock return the scaled host TSC and
> corresponding scaling parameters to HVM domains if guest TSC is not
> emulated and TSC scaling is enabled.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> Changes in v3:
>   (addressing Boris Ostrovsky's comments)
>   * No changes in fact. tsc_set_info() does not set d->arch.vtsc to 0
>     if host_tsc_is_safe() is not satisfied, so it is safe to use
>     d->arch.vtsc_to_ns here.

I don't think I understand your argument here. I think last time we 
decided that vtsc_to_ns can be used only if TSC is constant.

-boris

>
>   xen/arch/x86/time.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index d83f068..b31634a 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -815,10 +815,18 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
>       }
>       else
>       {
> -        tsc_stamp = t->local_tsc_stamp;
> -
> -        _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> -        _u.tsc_shift         = (s8)t->tsc_scale.shift;
> +        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
> +        {
> +            tsc_stamp            = hvm_funcs.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;
> +        }
> +        else
> +        {
> +            tsc_stamp            = t->local_tsc_stamp;
> +            _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> +            _u.tsc_shift         = (s8)t->tsc_scale.shift;
> +        }
>       }
>   
>       _u.tsc_timestamp = tsc_stamp;

Re: [PATCH v3 07/13] x86: Add functions for 64-bit integer arithmetic

[Boris Ostrovsky]

On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> This patch adds several functions to take multiplication, division and
> shifting involving 64-bit integers. Those functions are derived from
> Linux kernel and will be used by later patches to calculate scaling
> ratio and scaled TSC.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

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

(although I not sure why you decided to change Linux' code in mul64() below)



> +    c = (u64)rl.l.high + rm.l.low + rn.l.low;
> +    rl.l.high = c;
> +    c >>= 32;
> +    c = c + rm.l.high + rn.l.high + rh.l.low;
> +    rh.l.low = c;
> +    rh.l.high += (u32)(c >> 32);

Re: [PATCH v3 08/13] x86/hvm: Setup TSC scaling ratio

[Boris Ostrovsky]

On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> This patch adds a field tsc_scaling_ratio in struct hvm_vcpu to
> record the TSC scaling ratio, and sets it up when tsc_set_info() is
> called for a vcpu or when a vcpu is restored or reset.
>
> Before applying the TSC scaling ratio to CPU, 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>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> Changes in v3:
>   (addressing Boris Ostrovsky's comments)
>   * Remove redundant checks.
>   * Update comments in tsc_set_info(): during initial boot -> during domain creation.
>   (addressing Kevin Tian's comments)
>   * Add a validation step to check TSC scaling ratio before applying it
>     to the hardware. If the validation fails, we will fallback to Xen's
>     original way to handle guest TSC. hvm_setup_tsc_scaling() is always
>     called when the validation succeeds, so it shoud never fail now.
>
>   xen/arch/x86/hvm/hvm.c            | 41 +++++++++++++++++++++++++++++++++++++++
>   xen/arch/x86/hvm/svm/svm.c        |  6 ++++--
>   xen/arch/x86/time.c               | 19 +++++++++++++++---
>   xen/include/asm-x86/hvm/hvm.h     |  6 ++++++
>   xen/include/asm-x86/hvm/svm/svm.h |  3 ---
>   xen/include/asm-x86/hvm/vcpu.h    |  2 ++
>   6 files changed, 69 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 3648a44..3bf99b1 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -65,6 +65,7 @@
>   #include <asm/mtrr.h>
>   #include <asm/apic.h>
>   #include <asm/vm_event.h>
> +#include <asm/math64.h>
>   #include <public/sched.h>
>   #include <public/hvm/ioreq.h>
>   #include <public/version.h>
> @@ -301,6 +302,42 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
>       return 1;
>   }
>   
> +bool_t hvm_validate_tsc_scaling_ratio(uint32_t gtsc_khz)
> +{
> +    u64 ratio;
> +
> +    if ( !hvm_funcs.tsc_scaling_supported )
> +        return FALSE;
> +
> +    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
> +                            gtsc_khz, cpu_khz);
> +
> +    return (!ratio || ratio > hvm_funcs.max_tsc_scaling_ratio) ? FALSE : TRUE;
> +}
> +
> +void hvm_setup_tsc_scaling(struct vcpu *v)
> +{
> +    u64 ratio;
> +
> +    if ( !hvm_funcs.tsc_scaling_supported )
> +        return;
> +
> +    /*
> +     * The multiplication of the first two terms may overflow a 64-bit
> +     * integer, so use mul_u64_u32_div() instead to keep precision.
> +     */
> +    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
> +                            v->domain->arch.tsc_khz, cpu_khz);
> +
> +    if ( ratio == 0 || ratio > hvm_funcs.max_tsc_scaling_ratio )
> +        return;
> +
> +    v->arch.hvm_vcpu.tsc_scaling_ratio = ratio;
> +
> +    if ( hvm_funcs.setup_tsc_scaling )
> +        hvm_funcs.setup_tsc_scaling(v);
> +}

Would be nice to factor out common code. E.g. replace 
hvm_validate_tsc_scaling_ratio() with something like get_ratio() which 
returns zero if gtsc_khz is bogus?

-boris

Re: [PATCH v3 09/13] x86/hvm: Replace architecture TSC scaling by a common function

[Boris Ostrovsky]

On 12/30/2015 10:03 PM, Haozhong Zhang 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: Kevin Tian <kevin.tian@intel.com> (except the mul64 part)


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

Re: [PATCH v3 02/13] x86/time.c: Use correct guest TSC frequency in tsc_get_info()

[Haozhong Zhang]

On 01/04/16 12:48, Boris Ostrovsky wrote:
> On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> >When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
> >TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
> >tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
> >frequency. However, tsc_set_info() may set the guest TSC frequency to a
> >value different than the host. In order to keep consistent to
> >tsc_set_info(), this patch makes tsc_get_info() use the value set by
> >tsc_set_info() as the guest TSC frequency.
> >
> >Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> >---
> >Changes in v3:
> >  (addressing Boris Ostrovsky's comments)
> >  * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.
> >
> >  xen/arch/x86/time.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> >diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> >index 0059b6a..d83f068 100644
> >--- a/xen/arch/x86/time.c
> >+++ b/xen/arch/x86/time.c
> >@@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
> >                    uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
> >                    uint32_t *incarnation)
> >  {
> >+    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
> >+                                cpu_has_tsc_ratio;
> >+
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> although I still think that having !d->arch.vtsc would be better, even if
> not strictly required.
>
I'll add in the next version.

Haozhong

Re: [PATCH v3 04/13] x86/time.c: Scale host TSC in pvclock properly

[Haozhong Zhang]

On 01/04/16 13:09, Boris Ostrovsky wrote:
> On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> >This patch makes the pvclock return the scaled host TSC and
> >corresponding scaling parameters to HVM domains if guest TSC is not
> >emulated and TSC scaling is enabled.
> >
> >Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> >---
> >Changes in v3:
> >  (addressing Boris Ostrovsky's comments)
> >  * No changes in fact. tsc_set_info() does not set d->arch.vtsc to 0
> >    if host_tsc_is_safe() is not satisfied, so it is safe to use
> >    d->arch.vtsc_to_ns here.
> 
> I don't think I understand your argument here. I think last time we decided
> that vtsc_to_ns can be used only if TSC is constant.
> 
> -boris
>
In tsc_set_info(), if TSC scaling is available and host has
X86_FEATURE_TSC_RELIABLE (checked by host_tsc_is_safe()), vtsc is left
to 0. And looking at init_amd() and init_intel(),
X86_FEATURE_CONSTANT_TSC is available whenever
X86_FEATURE_TSC_RELIABLE is available as well. Therefore, when
vtsc_to_ns is used in __update_vcpu_system_time(), we can always
ensure that host has X86_FEATURE_CONSTANT_TSC and do not need to
recheck it there.

Haozhong

> >
> >  xen/arch/x86/time.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> >diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> >index d83f068..b31634a 100644
> >--- a/xen/arch/x86/time.c
> >+++ b/xen/arch/x86/time.c
> >@@ -815,10 +815,18 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
> >      }
> >      else
> >      {
> >-        tsc_stamp = t->local_tsc_stamp;
> >-
> >-        _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> >-        _u.tsc_shift         = (s8)t->tsc_scale.shift;
> >+        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
> >+        {
> >+            tsc_stamp            = hvm_funcs.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;
> >+        }
> >+        else
> >+        {
> >+            tsc_stamp            = t->local_tsc_stamp;
> >+            _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> >+            _u.tsc_shift         = (s8)t->tsc_scale.shift;
> >+        }
> >      }
> >      _u.tsc_timestamp = tsc_stamp;
> 

Re: [PATCH v3 07/13] x86: Add functions for 64-bit integer arithmetic

[Haozhong Zhang]

On 01/04/16 13:26, Boris Ostrovsky wrote:
> On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> >This patch adds several functions to take multiplication, division and
> >shifting involving 64-bit integers. Those functions are derived from
> >Linux kernel and will be used by later patches to calculate scaling
> >ratio and scaled TSC.
> >
> >Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> (although I not sure why you decided to change Linux' code in mul64() below)
> 
> 
> 
> >+    c = (u64)rl.l.high + rm.l.low + rn.l.low;
> >+    rl.l.high = c;
> >+    c >>= 32;
> >+    c = c + rm.l.high + rn.l.high + rh.l.low;
> >+    rh.l.low = c;
> >+    rh.l.high += (u32)(c >> 32);
> 

I took these code from my KVM patches, but it seemed it was refactored
later and also inlined with mul_u64_u64_shr(). However, above code is
still equivalent to the current Linux code (if we take mul64() from
mul_u64_u64_shr() in Linux), so I think it should be fine. Or do you
want me to make them consistent with Linux code?

Haozhong

Re: [PATCH v3 08/13] x86/hvm: Setup TSC scaling ratio

[Haozhong Zhang]

On 01/04/16 13:40, Boris Ostrovsky wrote:
> On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> >This patch adds a field tsc_scaling_ratio in struct hvm_vcpu to
> >record the TSC scaling ratio, and sets it up when tsc_set_info() is
> >called for a vcpu or when a vcpu is restored or reset.
> >
> >Before applying the TSC scaling ratio to CPU, 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>
> >Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> >---
> >Changes in v3:
> >  (addressing Boris Ostrovsky's comments)
> >  * Remove redundant checks.
> >  * Update comments in tsc_set_info(): during initial boot -> during domain creation.
> >  (addressing Kevin Tian's comments)
> >  * Add a validation step to check TSC scaling ratio before applying it
> >    to the hardware. If the validation fails, we will fallback to Xen's
> >    original way to handle guest TSC. hvm_setup_tsc_scaling() is always
> >    called when the validation succeeds, so it shoud never fail now.
> >
> >  xen/arch/x86/hvm/hvm.c            | 41 +++++++++++++++++++++++++++++++++++++++
> >  xen/arch/x86/hvm/svm/svm.c        |  6 ++++--
> >  xen/arch/x86/time.c               | 19 +++++++++++++++---
> >  xen/include/asm-x86/hvm/hvm.h     |  6 ++++++
> >  xen/include/asm-x86/hvm/svm/svm.h |  3 ---
> >  xen/include/asm-x86/hvm/vcpu.h    |  2 ++
> >  6 files changed, 69 insertions(+), 8 deletions(-)
> >
> >diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >index 3648a44..3bf99b1 100644
> >--- a/xen/arch/x86/hvm/hvm.c
> >+++ b/xen/arch/x86/hvm/hvm.c
> >@@ -65,6 +65,7 @@
> >  #include <asm/mtrr.h>
> >  #include <asm/apic.h>
> >  #include <asm/vm_event.h>
> >+#include <asm/math64.h>
> >  #include <public/sched.h>
> >  #include <public/hvm/ioreq.h>
> >  #include <public/version.h>
> >@@ -301,6 +302,42 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
> >      return 1;
> >  }
> >+bool_t hvm_validate_tsc_scaling_ratio(uint32_t gtsc_khz)
> >+{
> >+    u64 ratio;
> >+
> >+    if ( !hvm_funcs.tsc_scaling_supported )
> >+        return FALSE;
> >+
> >+    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
> >+                            gtsc_khz, cpu_khz);
> >+
> >+    return (!ratio || ratio > hvm_funcs.max_tsc_scaling_ratio) ? FALSE : TRUE;
> >+}
> >+
> >+void hvm_setup_tsc_scaling(struct vcpu *v)
> >+{
> >+    u64 ratio;
> >+
> >+    if ( !hvm_funcs.tsc_scaling_supported )
> >+        return;
> >+
> >+    /*
> >+     * The multiplication of the first two terms may overflow a 64-bit
> >+     * integer, so use mul_u64_u32_div() instead to keep precision.
> >+     */
> >+    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
> >+                            v->domain->arch.tsc_khz, cpu_khz);
> >+
> >+    if ( ratio == 0 || ratio > hvm_funcs.max_tsc_scaling_ratio )
> >+        return;
> >+
> >+    v->arch.hvm_vcpu.tsc_scaling_ratio = ratio;
> >+
> >+    if ( hvm_funcs.setup_tsc_scaling )
> >+        hvm_funcs.setup_tsc_scaling(v);
> >+}
> 
> Would be nice to factor out common code. E.g. replace
> hvm_validate_tsc_scaling_ratio() with something like get_ratio() which
> returns zero if gtsc_khz is bogus?
> 
> -boris

Yes, I'll change in the next version.

Haozhong

Re: [PATCH v3 04/13] x86/time.c: Scale host TSC in pvclock properly

[Boris Ostrovsky]

On 01/04/2016 07:59 PM, Haozhong Zhang wrote:
> On 01/04/16 13:09, Boris Ostrovsky wrote:
>> On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
>>> This patch makes the pvclock return the scaled host TSC and
>>> corresponding scaling parameters to HVM domains if guest TSC is not
>>> emulated and TSC scaling is enabled.
>>>
>>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>>> ---
>>> Changes in v3:
>>>   (addressing Boris Ostrovsky's comments)
>>>   * No changes in fact. tsc_set_info() does not set d->arch.vtsc to 0
>>>     if host_tsc_is_safe() is not satisfied, so it is safe to use
>>>     d->arch.vtsc_to_ns here.
>> I don't think I understand your argument here. I think last time we decided
>> that vtsc_to_ns can be used only if TSC is constant.
>>
>> -boris
>>
> In tsc_set_info(), if TSC scaling is available and host has
> X86_FEATURE_TSC_RELIABLE (checked by host_tsc_is_safe()), vtsc is left
> to 0. And looking at init_amd() and init_intel(),
> X86_FEATURE_CONSTANT_TSC is available whenever
> X86_FEATURE_TSC_RELIABLE is available as well. Therefore, when
> vtsc_to_ns is used in __update_vcpu_system_time(), we can always
> ensure that host has X86_FEATURE_CONSTANT_TSC and do not need to
> recheck it there.

OK --- I was looking at tsc_set_info()'s TSC_MODE_NEVER_EMULATE path but 
now I realize that we don't use scaling in that mode (right?).

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


>
> Haozhong
>
>>>   xen/arch/x86/time.c | 16 ++++++++++++----
>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
>>> index d83f068..b31634a 100644
>>> --- a/xen/arch/x86/time.c
>>> +++ b/xen/arch/x86/time.c
>>> @@ -815,10 +815,18 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
>>>       }
>>>       else
>>>       {
>>> -        tsc_stamp = t->local_tsc_stamp;
>>> -
>>> -        _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
>>> -        _u.tsc_shift         = (s8)t->tsc_scale.shift;
>>> +        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
>>> +        {
>>> +            tsc_stamp            = hvm_funcs.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;
>>> +        }
>>> +        else
>>> +        {
>>> +            tsc_stamp            = t->local_tsc_stamp;
>>> +            _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
>>> +            _u.tsc_shift         = (s8)t->tsc_scale.shift;
>>> +        }
>>>       }
>>>       _u.tsc_timestamp = tsc_stamp;

Re: [PATCH v3 07/13] x86: Add functions for 64-bit integer arithmetic

[Boris Ostrovsky]

On 01/04/2016 08:15 PM, Haozhong Zhang wrote:
> On 01/04/16 13:26, Boris Ostrovsky wrote:
>> On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
>>> This patch adds several functions to take multiplication, division and
>>> shifting involving 64-bit integers. Those functions are derived from
>>> Linux kernel and will be used by later patches to calculate scaling
>>> ratio and scaled TSC.
>>>
>>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>
>> (although I not sure why you decided to change Linux' code in mul64() below)
>>
>>
>>
>>> +    c = (u64)rl.l.high + rm.l.low + rn.l.low;
>>> +    rl.l.high = c;
>>> +    c >>= 32;
>>> +    c = c + rm.l.high + rn.l.high + rh.l.low;
>>> +    rh.l.low = c;
>>> +    rh.l.high += (u32)(c >> 32);
> I took these code from my KVM patches, but it seemed it was refactored
> later and also inlined with mul_u64_u64_shr(). However, above code is
> still equivalent to the current Linux code (if we take mul64() from
> mul_u64_u64_shr() in Linux), so I think it should be fine. Or do you
> want me to make them consistent with Linux code?

Yes, it is equivalent to what's in Linux. I don't think you need to do 
anything, I was just curious why the two routines look slightly different.


-boris

Re: [PATCH v3 04/13] x86/time.c: Scale host TSC in pvclock properly

[Haozhong Zhang]

On 01/05/16 11:15, Boris Ostrovsky wrote:
> On 01/04/2016 07:59 PM, Haozhong Zhang wrote:
> >On 01/04/16 13:09, Boris Ostrovsky wrote:
> >>On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> >>>This patch makes the pvclock return the scaled host TSC and
> >>>corresponding scaling parameters to HVM domains if guest TSC is not
> >>>emulated and TSC scaling is enabled.
> >>>
> >>>Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> >>>---
> >>>Changes in v3:
> >>>  (addressing Boris Ostrovsky's comments)
> >>>  * No changes in fact. tsc_set_info() does not set d->arch.vtsc to 0
> >>>    if host_tsc_is_safe() is not satisfied, so it is safe to use
> >>>    d->arch.vtsc_to_ns here.
> >>I don't think I understand your argument here. I think last time we decided
> >>that vtsc_to_ns can be used only if TSC is constant.
> >>
> >>-boris
> >>
> >In tsc_set_info(), if TSC scaling is available and host has
> >X86_FEATURE_TSC_RELIABLE (checked by host_tsc_is_safe()), vtsc is left
> >to 0. And looking at init_amd() and init_intel(),
> >X86_FEATURE_CONSTANT_TSC is available whenever
> >X86_FEATURE_TSC_RELIABLE is available as well. Therefore, when
> >vtsc_to_ns is used in __update_vcpu_system_time(), we can always
> >ensure that host has X86_FEATURE_CONSTANT_TSC and do not need to
> >recheck it there.
> 
> OK --- I was looking at tsc_set_info()'s TSC_MODE_NEVER_EMULATE path but now
> I realize that we don't use scaling in that mode (right?).
>

Right.

Haozhong

> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> 
> >
> >Haozhong
> >
> >>>  xen/arch/x86/time.c | 16 ++++++++++++----
> >>>  1 file changed, 12 insertions(+), 4 deletions(-)
> >>>
> >>>diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> >>>index d83f068..b31634a 100644
> >>>--- a/xen/arch/x86/time.c
> >>>+++ b/xen/arch/x86/time.c
> >>>@@ -815,10 +815,18 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
> >>>      }
> >>>      else
> >>>      {
> >>>-        tsc_stamp = t->local_tsc_stamp;
> >>>-
> >>>-        _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> >>>-        _u.tsc_shift         = (s8)t->tsc_scale.shift;
> >>>+        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
> >>>+        {
> >>>+            tsc_stamp            = hvm_funcs.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;
> >>>+        }
> >>>+        else
> >>>+        {
> >>>+            tsc_stamp            = t->local_tsc_stamp;
> >>>+            _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> >>>+            _u.tsc_shift         = (s8)t->tsc_scale.shift;
> >>>+        }
> >>>      }
> >>>      _u.tsc_timestamp = tsc_stamp;
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v3 02/13] x86/time.c: Use correct guest TSC frequency in tsc_get_info()

[Jan Beulich]

>>> On 04.01.16 at 18:48, <boris.ostrovsky@oracle.com> wrote:
> On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
>> When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
>> TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
>> tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
>> frequency. However, tsc_set_info() may set the guest TSC frequency to a
>> value different than the host. In order to keep consistent to
>> tsc_set_info(), this patch makes tsc_get_info() use the value set by
>> tsc_set_info() as the guest TSC frequency.
>>
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> ---
>> Changes in v3:
>>   (addressing Boris Ostrovsky's comments)
>>   * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.
>>
>>   xen/arch/x86/time.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
>> index 0059b6a..d83f068 100644
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>>                     uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
>>                     uint32_t *incarnation)
>>   {
>> +    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
>> +                                cpu_has_tsc_ratio;
>> +
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> although I still think that having !d->arch.vtsc would be better, even 
> if not strictly required.

Since Haozhong agreed, and for symmetry with patch 1 I'm going to
commit with this added, plus ...

>> @@ -1780,9 +1783,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>>           else
>>           {
>>               tsc = rdtsc();
>> -            *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
>> +            *elapsed_nsec = scale_delta(tsc, &this_cpu(cpu_time).tsc_scale) -
>>                               d->arch.vtsc_offset;
>> -            *gtsc_khz = 0; /* ignored by tsc_set_info */
>> +            *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : 0;

... with the comment here retained.

Jan

Re: [PATCH v3 02/13] x86/time.c: Use correct guest TSC frequency in tsc_get_info()

[Haozhong Zhang]

On 01/08/16 02:05, Jan Beulich wrote:
> >>> On 04.01.16 at 18:48, <boris.ostrovsky@oracle.com> wrote:
> > On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> >> When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
> >> TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
> >> tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
> >> frequency. However, tsc_set_info() may set the guest TSC frequency to a
> >> value different than the host. In order to keep consistent to
> >> tsc_set_info(), this patch makes tsc_get_info() use the value set by
> >> tsc_set_info() as the guest TSC frequency.
> >>
> >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> >> ---
> >> Changes in v3:
> >>   (addressing Boris Ostrovsky's comments)
> >>   * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.
> >>
> >>   xen/arch/x86/time.c | 9 ++++++---
> >>   1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> >> index 0059b6a..d83f068 100644
> >> --- a/xen/arch/x86/time.c
> >> +++ b/xen/arch/x86/time.c
> >> @@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
> >>                     uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
> >>                     uint32_t *incarnation)
> >>   {
> >> +    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
> >> +                                cpu_has_tsc_ratio;
> >> +
> > 
> > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > 
> > although I still think that having !d->arch.vtsc would be better, even 
> > if not strictly required.
> 
> Since Haozhong agreed, and for symmetry with patch 1 I'm going to
> commit with this added, plus ...
> 
> >> @@ -1780,9 +1783,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
> >>           else
> >>           {
> >>               tsc = rdtsc();
> >> -            *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
> >> +            *elapsed_nsec = scale_delta(tsc, &this_cpu(cpu_time).tsc_scale) -
> >>                               d->arch.vtsc_offset;
> >> -            *gtsc_khz = 0; /* ignored by tsc_set_info */
> >> +            *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : 0;
> 
> ... with the comment here retained.
>
And it's better to refine the comment like
/* ignored by tsc_set_info if TSC scaling disabled */

Haozhong

Re: [PATCH v3 03/13] x86/hvm: Scale host TSC when setting/getting guest TSC

[Jan Beulich]

>>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -804,6 +804,16 @@ static uint64_t scale_tsc(uint64_t host_tsc, uint64_t ratio)
>      return scaled_host_tsc;
>  }
>  
> +static uint64_t svm_scale_tsc(struct vcpu *v, uint64_t tsc)
> +{
> +    struct domain *d = v->domain;
> +
> +    if ( !cpu_has_tsc_ratio || d->arch.vtsc )

The left side of this check is redundant with those at both call sites.
It should either be removed altogether, or converted to an ASSERT().
Perhaps the right side should move into the callers too (as being
vendor independent), or if not at least the pointless local variable
should be eliminated.

Further I suppose this new hook really could/should have its
first argument const qualified (the hook isn't supposed to fiddle
with the vCPU).

Jan

Re: [PATCH v3 04/13] x86/time.c: Scale host TSC in pvclock properly

[Jan Beulich]

>>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> This patch makes the pvclock return the scaled host TSC and
> corresponding scaling parameters to HVM domains if guest TSC is not
> emulated and TSC scaling is enabled.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> Changes in v3:
>  (addressing Boris Ostrovsky's comments)
>  * No changes in fact. tsc_set_info() does not set d->arch.vtsc to 0
>    if host_tsc_is_safe() is not satisfied, so it is safe to use
>    d->arch.vtsc_to_ns here.
> 
>  xen/arch/x86/time.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index d83f068..b31634a 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -815,10 +815,18 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
>      }
>      else
>      {
> -        tsc_stamp = t->local_tsc_stamp;
> -
> -        _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> -        _u.tsc_shift         = (s8)t->tsc_scale.shift;
> +        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )

For symmetry with patches 1 and 2 I'd expect !d->arch.vtsc to be
added here too.

> +        {
> +            tsc_stamp            = hvm_funcs.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;
> +        }
> +        else
> +        {
> +            tsc_stamp            = t->local_tsc_stamp;
> +            _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> +            _u.tsc_shift         = (s8)t->tsc_scale.shift;

The case has been pointless anyway, and you don't add a similar
one in the if() branch - please delete it.

Jan

Re: [PATCH v3 05/13] svm: Remove redundant TSC scaling in svm_set_tsc_offset()

[Jan Beulich]

>>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> @@ -854,6 +841,7 @@ static void svm_set_tsc_offset(struct vcpu *v, u64 offset, u64 at_tsc)
>          n2_tsc_offset = vmcb_get_tsc_offset(n2vmcb) -
>              vmcb_get_tsc_offset(n1vmcb);
>          if ( svm->ns_tscratio != DEFAULT_TSC_RATIO ) {
> +            uint64_t guest_tsc = hvm_get_guest_tsc_fixed(v, at_tsc);
>              n2_tsc_offset = svm_get_tsc_offset(guest_tsc,

Blank line between declaration(s) and statement(s) please.

Also please consider taking the opportunity to fix the preceding
line's coding style violation.

Jan

Re: [PATCH v3 07/13] x86: Add functions for 64-bit integer arithmetic

[Jan Beulich]

>>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> --- /dev/null
> +++ b/xen/include/asm-x86/math64.h
> @@ -0,0 +1,105 @@
> +#ifndef __X86_MATH64
> +#define __X86_MATH64
> +
> +/*
> + * Functions defined in this file are derived from Linux kernel
> + * (include/linux/math64.h).
> + */

This is not true. At least mul64() doesn't exist in Linux (as of 4.4-rc8).

> +static inline void mul64(u64 *lo, u64 *hi, u64 a, u64 b)
> +{
> +    typedef union {
> +        u64 ll;
> +        struct {
> +            u32 low, high;
> +        } l;
> +    } LL;
> +    LL rl, rm, rn, rh, a0, b0;
> +    u64 c;
> +
> +    a0.ll = a;
> +    b0.ll = b;
> +
> +    rl.ll = (u64)a0.l.low * b0.l.low;
> +    rm.ll = (u64)a0.l.low * b0.l.high;
> +    rn.ll = (u64)a0.l.high * b0.l.low;
> +    rh.ll = (u64)a0.l.high * b0.l.high;
> +
> +    c = (u64)rl.l.high + rm.l.low + rn.l.low;
> +    rl.l.high = c;
> +    c >>= 32;
> +    c = c + rm.l.high + rn.l.high + rh.l.low;
> +    rh.l.low = c;
> +    rh.l.high += (u32)(c >> 32);
> +
> +    *lo = rl.ll;
> +    *hi = rh.ll;
> +}

For this one in particular, but likely also for the others: If this was
put in include/xen/math64.h I might buy it. As an x86-specific
implementation this could (and should) be done with a single asm()
statement using the MUL instruction (I didn't check whether gcc
also offers a built-in, which then could be used generically).

Jan

Re: [PATCH v3 08/13] x86/hvm: Setup TSC scaling ratio

[Jan Beulich]

>>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> @@ -301,6 +302,42 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
>      return 1;
>  }
>  
> +bool_t hvm_validate_tsc_scaling_ratio(uint32_t gtsc_khz)
> +{
> +    u64 ratio;
> +
> +    if ( !hvm_funcs.tsc_scaling_supported )
> +        return FALSE;

We use 0 and 1, not FALSE and TRUE (except in ACPI and EFI code).

> +    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
> +                            gtsc_khz, cpu_khz);
> +
> +    return (!ratio || ratio > hvm_funcs.max_tsc_scaling_ratio) ? FALSE : TRUE;

There no point in using a conditional expression here.

> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -226,6 +226,9 @@ struct hvm_function_table {
>      int (*altp2m_vcpu_emulate_vmfunc)(struct cpu_user_regs *regs);
>  
>      uint64_t (*scale_tsc)(struct vcpu *v, uint64_t tsc);
> +
> +    /* Architecture function to setup TSC scaling ratio */
> +    void (*setup_tsc_scaling)(struct vcpu *v);

SVM doesn't use this hook, and VMX gets to use it only later. I.e.
everything related to it is dead code right now, and would therefore
better go into the VMX (I suppose) patch actually making use of it.

Jan

Re: [PATCH v3 04/13] x86/time.c: Scale host TSC in pvclock properly

[Haozhong Zhang]

On 01/08/16 02:20, Jan Beulich wrote:
> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> > This patch makes the pvclock return the scaled host TSC and
> > corresponding scaling parameters to HVM domains if guest TSC is not
> > emulated and TSC scaling is enabled.
> > 
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > ---
> > Changes in v3:
> >  (addressing Boris Ostrovsky's comments)
> >  * No changes in fact. tsc_set_info() does not set d->arch.vtsc to 0
> >    if host_tsc_is_safe() is not satisfied, so it is safe to use
> >    d->arch.vtsc_to_ns here.
> > 
> >  xen/arch/x86/time.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> > index d83f068..b31634a 100644
> > --- a/xen/arch/x86/time.c
> > +++ b/xen/arch/x86/time.c
> > @@ -815,10 +815,18 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
> >      }
> >      else
> >      {
> > -        tsc_stamp = t->local_tsc_stamp;
> > -
> > -        _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> > -        _u.tsc_shift         = (s8)t->tsc_scale.shift;
> > +        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
> 
> For symmetry with patches 1 and 2 I'd expect !d->arch.vtsc to be
> added here too.
>

even though it's already in the !d->arch.vtsc branch? But I'm fine to
add it for symmetric.

> > +        {
> > +            tsc_stamp            = hvm_funcs.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;
> > +        }
> > +        else
> > +        {
> > +            tsc_stamp            = t->local_tsc_stamp;
> > +            _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> > +            _u.tsc_shift         = (s8)t->tsc_scale.shift;
> 
> The case has been pointless anyway, and you don't add a similar
> one in the if() branch - please delete it.
>

I don't understand why it's pointless. The inner if branch is to
ensure pvclock provides the scaled TSC information when TSC scaling is
used, and the inner else branch is to keep the existing behavior when
TSC scaling is not used. For the outer if branch, TSC scaling is never
used, so no change is needed.

Haozhong

Re: [PATCH v3 05/13] svm: Remove redundant TSC scaling in svm_set_tsc_offset()

[Haozhong Zhang]

On 01/08/16 02:22, Jan Beulich wrote:
> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> > @@ -854,6 +841,7 @@ static void svm_set_tsc_offset(struct vcpu *v, u64 offset, u64 at_tsc)
> >          n2_tsc_offset = vmcb_get_tsc_offset(n2vmcb) -
> >              vmcb_get_tsc_offset(n1vmcb);
> >          if ( svm->ns_tscratio != DEFAULT_TSC_RATIO ) {
> > +            uint64_t guest_tsc = hvm_get_guest_tsc_fixed(v, at_tsc);
> >              n2_tsc_offset = svm_get_tsc_offset(guest_tsc,
> 
> Blank line between declaration(s) and statement(s) please.
> 
> Also please consider taking the opportunity to fix the preceding
> line's coding style violation.
> 
> Jan

OK, both will be fixed in the next version.

Haozhong

Re: [PATCH v3 04/13] x86/time.c: Scale host TSC in pvclock properly

[Jan Beulich]

>>> On 08.01.16 at 14:22, <haozhong.zhang@intel.com> wrote:
> On 01/08/16 02:20, Jan Beulich wrote:
>> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
>> > --- a/xen/arch/x86/time.c
>> > +++ b/xen/arch/x86/time.c
>> > @@ -815,10 +815,18 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
>> >      }
>> >      else
>> >      {
>> > -        tsc_stamp = t->local_tsc_stamp;
>> > -
>> > -        _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
>> > -        _u.tsc_shift         = (s8)t->tsc_scale.shift;
>> > +        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
>> 
>> For symmetry with patches 1 and 2 I'd expect !d->arch.vtsc to be
>> added here too.
> 
> even though it's already in the !d->arch.vtsc branch? But I'm fine to
> add it for symmetric.

Ah, no, I didn't realize (only looking at the patch) that it's in the
else to a respective if(). No need to change it then.

>> > +        {
>> > +            tsc_stamp            = hvm_funcs.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;
>> > +        }
>> > +        else
>> > +        {
>> > +            tsc_stamp            = t->local_tsc_stamp;
>> > +            _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
>> > +            _u.tsc_shift         = (s8)t->tsc_scale.shift;
>> 
>> The case has been pointless anyway, and you don't add a similar
>> one in the if() branch - please delete it.
> 
> I don't understand why it's pointless. The inner if branch is to
> ensure pvclock provides the scaled TSC information when TSC scaling is
> used, and the inner else branch is to keep the existing behavior when
> TSC scaling is not used. For the outer if branch, TSC scaling is never
> used, so no change is needed.

Sorry, I see I typo-ed my reply. It was meant to start "The cast
has been ..."

Jan

Re: [PATCH v3 07/13] x86: Add functions for 64-bit integer arithmetic

[Haozhong Zhang]

On 01/08/16 02:34, Jan Beulich wrote:
> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> > --- /dev/null
> > +++ b/xen/include/asm-x86/math64.h
> > @@ -0,0 +1,105 @@
> > +#ifndef __X86_MATH64
> > +#define __X86_MATH64
> > +
> > +/*
> > + * Functions defined in this file are derived from Linux kernel
> > + * (include/linux/math64.h).
> > + */
> 
> This is not true. At least mul64() doesn't exist in Linux (as of 4.4-rc8).
>

My fault. I took them from my KVM patches and didn't notice they were
refactored a little when merging.

> > +static inline void mul64(u64 *lo, u64 *hi, u64 a, u64 b)
> > +{
> > +    typedef union {
> > +        u64 ll;
> > +        struct {
> > +            u32 low, high;
> > +        } l;
> > +    } LL;
> > +    LL rl, rm, rn, rh, a0, b0;
> > +    u64 c;
> > +
> > +    a0.ll = a;
> > +    b0.ll = b;
> > +
> > +    rl.ll = (u64)a0.l.low * b0.l.low;
> > +    rm.ll = (u64)a0.l.low * b0.l.high;
> > +    rn.ll = (u64)a0.l.high * b0.l.low;
> > +    rh.ll = (u64)a0.l.high * b0.l.high;
> > +
> > +    c = (u64)rl.l.high + rm.l.low + rn.l.low;
> > +    rl.l.high = c;
> > +    c >>= 32;
> > +    c = c + rm.l.high + rn.l.high + rh.l.low;
> > +    rh.l.low = c;
> > +    rh.l.high += (u32)(c >> 32);
> > +
> > +    *lo = rl.ll;
> > +    *hi = rh.ll;
> > +}
> 
> For this one in particular, but likely also for the others: If this was
> put in include/xen/math64.h I might buy it. As an x86-specific
> implementation this could (and should) be done with a single asm()
> statement using the MUL instruction (I didn't check whether gcc
> also offers a built-in, which then could be used generically).
>

gcc does provide a 128-bit integer type __int128. If my memory is
correct, it can be used for above mul64, but can not be compiled in
Xen if being used for division. Anyway, as above functions are
x86-specific so far, I'll reimplement them by assembly in the next
version.

Haozhong

Re: [PATCH v3 04/13] x86/time.c: Scale host TSC in pvclock properly

[Haozhong Zhang]

On 01/08/16 06:43, Jan Beulich wrote:
> >>> On 08.01.16 at 14:22, <haozhong.zhang@intel.com> wrote:
> > On 01/08/16 02:20, Jan Beulich wrote:
> >> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> >> > --- a/xen/arch/x86/time.c
> >> > +++ b/xen/arch/x86/time.c
> >> > @@ -815,10 +815,18 @@ static void __update_vcpu_system_time(struct vcpu *v, int force)
> >> >      }
> >> >      else
> >> >      {
> >> > -        tsc_stamp = t->local_tsc_stamp;
> >> > -
> >> > -        _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> >> > -        _u.tsc_shift         = (s8)t->tsc_scale.shift;
> >> > +        if ( has_hvm_container_domain(d) && cpu_has_tsc_ratio )
> >> 
> >> For symmetry with patches 1 and 2 I'd expect !d->arch.vtsc to be
> >> added here too.
> > 
> > even though it's already in the !d->arch.vtsc branch? But I'm fine to
> > add it for symmetric.
> 
> Ah, no, I didn't realize (only looking at the patch) that it's in the
> else to a respective if(). No need to change it then.
> 
> >> > +        {
> >> > +            tsc_stamp            = hvm_funcs.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;
> >> > +        }
> >> > +        else
> >> > +        {
> >> > +            tsc_stamp            = t->local_tsc_stamp;
> >> > +            _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
> >> > +            _u.tsc_shift         = (s8)t->tsc_scale.shift;
> >> 
> >> The case has been pointless anyway, and you don't add a similar
> >> one in the if() branch - please delete it.
> > 
> > I don't understand why it's pointless. The inner if branch is to
> > ensure pvclock provides the scaled TSC information when TSC scaling is
> > used, and the inner else branch is to keep the existing behavior when
> > TSC scaling is not used. For the outer if branch, TSC scaling is never
> > used, so no change is needed.
> 
> Sorry, I see I typo-ed my reply. It was meant to start "The cast
> has been ..."
> 
OK, I'll remove it.

Haozhong

Re: [PATCH v3 08/13] x86/hvm: Setup TSC scaling ratio

[Haozhong Zhang]

On 01/08/16 02:44, Jan Beulich wrote:
> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> > @@ -301,6 +302,42 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
> >      return 1;
> >  }
> >  
> > +bool_t hvm_validate_tsc_scaling_ratio(uint32_t gtsc_khz)
> > +{
> > +    u64 ratio;
> > +
> > +    if ( !hvm_funcs.tsc_scaling_supported )
> > +        return FALSE;
> 
> We use 0 and 1, not FALSE and TRUE (except in ACPI and EFI code).
>

I'll change in the next version.

> > +    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
> > +                            gtsc_khz, cpu_khz);
> > +
> > +    return (!ratio || ratio > hvm_funcs.max_tsc_scaling_ratio) ? FALSE : TRUE;
>
> There no point in using a conditional expression here.

Ah, right, I'll  change it to
         return !!(ratio && ratio <= hvm_funcs.max_tsc_scaling_ratio);
	 
> 
> > --- a/xen/include/asm-x86/hvm/hvm.h
> > +++ b/xen/include/asm-x86/hvm/hvm.h
> > @@ -226,6 +226,9 @@ struct hvm_function_table {
> >      int (*altp2m_vcpu_emulate_vmfunc)(struct cpu_user_regs *regs);
> >  
> >      uint64_t (*scale_tsc)(struct vcpu *v, uint64_t tsc);
> > +
> > +    /* Architecture function to setup TSC scaling ratio */
> > +    void (*setup_tsc_scaling)(struct vcpu *v);
> 
> SVM doesn't use this hook, and VMX gets to use it only later. I.e.
> everything related to it is dead code right now, and would therefore
> better go into the VMX (I suppose) patch actually making use of it.
> 

Yes, it's used in later VMX patch. I'll move it that patch.

Haozhong

Re: [PATCH v3 08/13] x86/hvm: Setup TSC scaling ratio

[Jan Beulich]

>>> On 08.01.16 at 14:55, <haozhong.zhang@intel.com> wrote:
> On 01/08/16 02:44, Jan Beulich wrote:
>> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
>> > +    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
>> > +                            gtsc_khz, cpu_khz);
>> > +
>> > +    return (!ratio || ratio > hvm_funcs.max_tsc_scaling_ratio) ? FALSE : TRUE;
>>
>> There no point in using a conditional expression here.
> 
> Ah, right, I'll  change it to
>          return !!(ratio && ratio <= hvm_funcs.max_tsc_scaling_ratio);

Except that you don't need the !!() here either.

Jan

Re: [PATCH v3 03/13] x86/hvm: Scale host TSC when setting/getting guest TSC

[Haozhong Zhang]

On 01/08/16 02:15, Jan Beulich wrote:
> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -804,6 +804,16 @@ static uint64_t scale_tsc(uint64_t host_tsc, uint64_t ratio)
> >      return scaled_host_tsc;
> >  }
> >  
> > +static uint64_t svm_scale_tsc(struct vcpu *v, uint64_t tsc)
> > +{
> > +    struct domain *d = v->domain;
> > +
> > +    if ( !cpu_has_tsc_ratio || d->arch.vtsc )
> 
> The left side of this check is redundant with those at both call sites.
> It should either be removed altogether, or converted to an ASSERT().
> Perhaps the right side should move into the callers too (as being
> vendor independent), or if not at least the pointless local variable
> should be eliminated.
>

Yes, I'll remove the left check and move the right check to callers. 
 
> Further I suppose this new hook really could/should have its
> first argument const qualified (the hook isn't supposed to fiddle
> with the vCPU).
>

I'll add 'const'.

Haozhong

Re: [PATCH v3 08/13] x86/hvm: Setup TSC scaling ratio

[Haozhong Zhang]

On 01/08/16 07:04, Jan Beulich wrote:
> >>> On 08.01.16 at 14:55, <haozhong.zhang@intel.com> wrote:
> > On 01/08/16 02:44, Jan Beulich wrote:
> >> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> >> > +    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
> >> > +                            gtsc_khz, cpu_khz);
> >> > +
> >> > +    return (!ratio || ratio > hvm_funcs.max_tsc_scaling_ratio) ? FALSE : TRUE;
> >>
> >> There no point in using a conditional expression here.
> > 
> > Ah, right, I'll  change it to
> >          return !!(ratio && ratio <= hvm_funcs.max_tsc_scaling_ratio);
> 
> Except that you don't need the !!() here either.
> 
> Jan
> 

Ah, yes.

Thanks,
Haozhong

Re: [PATCH v3 09/13] x86/hvm: Replace architecture TSC scaling by a common function

[Jan Beulich]

>>> On 31.12.15 at 04:03, <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: Kevin Tian <kevin.tian@intel.com> (except the mul64 part)

The tail of this should have been dropped with ...

> ---
> Changes in v3:
>  (addressing Boris Ostrovsky's comments)
>  * Remove unnecessary assertions in hvm_scale_tsc().
>  * Move math64 stuffs to a standalone patch and state their origination.

... this.

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

Re: [PATCH v3 12/13] vmx: Add VMX RDTSC(P) scaling support

[Jan Beulich]

>>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> @@ -2003,6 +2011,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,
> +    .default_tsc_scaling_ratio   = VMX_TSC_MULTIPLIER_DEFAULT,
> +    .max_tsc_scaling_ratio       = VMX_TSC_MULTIPLIER_MAX,
> +    .tsc_scaling_ratio_frac_bits = 48,
> +    .setup_tsc_scaling           = vmx_setup_tsc_scaling,
>  };
>  
>  /* Handle VT-d posted-interrupt when VCPU is running. */
> @@ -2107,6 +2119,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_supported = 1;

Do you actually still need this separate flag field? I.e. can't you
derive this from one of the other four fields?

Jan

Re: [PATCH v3 12/13] vmx: Add VMX RDTSC(P) scaling support

[Haozhong Zhang]

On 01/12/16 09:48, Jan Beulich wrote:
> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> > @@ -2003,6 +2011,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,
> > +    .default_tsc_scaling_ratio   = VMX_TSC_MULTIPLIER_DEFAULT,
> > +    .max_tsc_scaling_ratio       = VMX_TSC_MULTIPLIER_MAX,
> > +    .tsc_scaling_ratio_frac_bits = 48,
> > +    .setup_tsc_scaling           = vmx_setup_tsc_scaling,
> >  };
> >  
> >  /* Handle VT-d posted-interrupt when VCPU is running. */
> > @@ -2107,6 +2119,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_supported = 1;
> 
> Do you actually still need this separate flag field? I.e. can't you
> derive this from one of the other four fields?
>

Yes, unless other four fields are set conditionally instead of being
hardcoded in hvm_funcs.

Haozhong

> Jan
> 

Re: [PATCH v3 12/13] vmx: Add VMX RDTSC(P) scaling support

[Jan Beulich]

>>> On 14.01.16 at 05:52, <haozhong.zhang@intel.com> wrote:
> On 01/12/16 09:48, Jan Beulich wrote:
>> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
>> > @@ -2003,6 +2011,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,
>> > +    .default_tsc_scaling_ratio   = VMX_TSC_MULTIPLIER_DEFAULT,
>> > +    .max_tsc_scaling_ratio       = VMX_TSC_MULTIPLIER_MAX,
>> > +    .tsc_scaling_ratio_frac_bits = 48,
>> > +    .setup_tsc_scaling           = vmx_setup_tsc_scaling,
>> >  };
>> >  
>> >  /* Handle VT-d posted-interrupt when VCPU is running. */
>> > @@ -2107,6 +2119,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_supported = 1;
>> 
>> Do you actually still need this separate flag field? I.e. can't you
>> derive this from one of the other four fields?
> 
> Yes, unless other four fields are set conditionally instead of being
> hardcoded in hvm_funcs.

Indeed I'd expect (at least one of) them to be set (or zapped)
conditionally, just like is being done for some other ones.

Jan

Re: [PATCH v3 12/13] vmx: Add VMX RDTSC(P) scaling support

[Haozhong Zhang]

On 01/14/16 02:05, Jan Beulich wrote:
> >>> On 14.01.16 at 05:52, <haozhong.zhang@intel.com> wrote:
> > On 01/12/16 09:48, Jan Beulich wrote:
> >> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> >> > @@ -2003,6 +2011,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,
> >> > +    .default_tsc_scaling_ratio   = VMX_TSC_MULTIPLIER_DEFAULT,
> >> > +    .max_tsc_scaling_ratio       = VMX_TSC_MULTIPLIER_MAX,
> >> > +    .tsc_scaling_ratio_frac_bits = 48,
> >> > +    .setup_tsc_scaling           = vmx_setup_tsc_scaling,
> >> >  };
> >> >  
> >> >  /* Handle VT-d posted-interrupt when VCPU is running. */
> >> > @@ -2107,6 +2119,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_supported = 1;
> >> 
> >> Do you actually still need this separate flag field? I.e. can't you
> >> derive this from one of the other four fields?
> > 
> > Yes, unless other four fields are set conditionally instead of being
> > hardcoded in hvm_funcs.
> 
> Indeed I'd expect (at least one of) them to be set (or zapped)
> conditionally, just like is being done for some other ones.
>

OK, I'll set them conditionally and replace tsc_scaling_supported by
checking default_tsc_scaling_ratio.

Haozhong