Re: [Xen-devel] [PATCH 12/13] vmx: Add a call-back to apply TSC scaling ratio to hardware

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

On Tue, Sep 29, 2015 at 10:33:14AM +0100, Andrew Cooper wrote:
> On 29/09/15 02:07, Haozhong Zhang wrote:
> > On Mon, Sep 28, 2015 at 12:02:08PM -0400, Boris Ostrovsky wrote:
> >> On 09/28/2015 03:13 AM, Haozhong Zhang wrote:
> >>> This patch adds a new call-back setup_tsc_scaling in struct
> >>> hvm_function_table to apply the TSC scaling ratio to hardware. For VMX,
> >>> it writes the TSC scaling ratio to VMCS field TSC_MULTIPLIER.
> >>>
> >>> Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
> >>> ---
> >>>  xen/arch/x86/hvm/hvm.c        | 1 +
> >>>  xen/arch/x86/hvm/svm/svm.c    | 5 +++++
> >>>  xen/arch/x86/hvm/vmx/vmx.c    | 8 ++++++++
> >>>  xen/include/asm-x86/hvm/hvm.h | 3 +++
> >>>  4 files changed, 17 insertions(+)
> >>>
> >>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >>> index 3522d20..2d8a148 100644
> >>> --- a/xen/arch/x86/hvm/hvm.c
> >>> +++ b/xen/arch/x86/hvm/hvm.c
> >>> @@ -376,6 +376,7 @@ void hvm_setup_tsc_scaling(struct vcpu *v)
> >>>      }
> >>>      v->arch.tsc_scaling_ratio = ratio;
> >>> +    hvm_funcs.setup_tsc_scaling(v);
> >>>  }
> >>>  void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
> >>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> >>> index 73bc863..d890c1f 100644
> >>> --- a/xen/arch/x86/hvm/svm/svm.c
> >>> +++ b/xen/arch/x86/hvm/svm/svm.c
> >>> @@ -2236,6 +2236,10 @@ static void svm_invlpg_intercept(unsigned long 
> >>> vaddr)
> >>>      svm_asid_g_invlpg(curr, vaddr);
> >>>  }
> >>> +static void svm_setup_tsc_scaling(struct vcpu *v)
> >>> +{
> >>> +}
> >>> +
> >> Should this be wrmsrl(MSR_AMD64_TSC_RATIO, v->arch.tsc_scaling_ratio) ?
> >>
> >> -boris
> >>
> > MSR_AMD64_TSC_RATIO is set in svm_ctxt_switch_to() before entering guest.
> >
> > For VMX, the ratio is set to a VMCS field TSC_MULTIPLIER and it's not
> > necessary to set it every time entering guest. Therefore, I introduce
> > the call-back setup_tsc_scaling() to do this. For SVM, as the ratio is
> > set every time entering guest, I leave the SVM version of 
> > setup_tsc_scaling()
> > empty.
> 
> VT-x has a per-VMCS scale, while SVM has a per-core MSR to adjust the
> scale.  These do require different modification algorithms.
>

Yes, this is what I mean.

> However, if there is any chance that any part of the system can update
> the ratio while an SVM VCPU is in context (which appears to be the
> case), then MSR_AMD64_TSC_RATIO needs updating synchronously, or it will
> be deferred until the next full context switch which could be an
> arbitrary time into the future.  This appears to be a latent bug in the
> SVM side.
>

In my patch, tsc ratio is set only when
 1. a domain is created (by arch_domain_create()),
 2. a vcpu's state is reset (by hvm_vcpu_reset_state()),
 3. a vcpu's context is restored (by hvm_load_cpu_ctxt()), or
 4. through the hypercall XEN_DOMCTL_settscinfo.

(Correct me if I'm wrong below)

For the first 3 cases, vcpu is definitely not in context, so it's safe
to set tsc ratio without any latent bug. For the last case,
arch_do_domctl() pauses the domain before updating tsc ratio, so it's
also safe.

> ~Andrew
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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