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 11:25:56AM +0100, Andrew Cooper wrote:
> On 29/09/15 11:02, Haozhong Zhang wrote:
> > 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.
> 
> That logic appears to be correct, which would suggest that there isn't
> actually a latent bug.
> 
> In such a case, we would typically make the hvm_funcs pointer optional,
> and omit an empty stub on the SVM side.
>

Yes, I'll add the following check in hvm_setup_tsc_scaling():

  if ( !hvm_funcs.setup_tsc_scaling )
      return;

- Haozhong

> ~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