Message ID | 1451531020-29964-9-git-send-email-haozhong.zhang@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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
>>> 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
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
>>> 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
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
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, ®); hvm_set_segment_register(v, x86_seg_idtr, ®); + 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;