Re: [Xen-devel] [PATCH v5 3/3] x86/viridian: Add partition time reference counter MSR support

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Egger, Christoph [mailto:chegger@xxxxxxxxx]
> Sent: 13 August 2014 16:28
> To: Paul Durrant; xen-devel@xxxxxxxxxxxxx
> Cc: Keir (Xen.org); Ian Campbell; Stefano Stabellini; Ian Jackson; Jan Beulich
> Subject: Re: [Xen-devel] [PATCH v5 3/3] x86/viridian: Add partition time
> reference counter MSR support
> 
> On 05.08.14 15:07, Paul Durrant wrote:
> > This patch optionally re-instates support for the partition time reference
> > counter that was previously introduced by commit
> > e36cd2cdc9674a7a4855d21fb7b3e6e17c4bb33b and reverted by commit
> > 1cd4fab14ce25859efa4a2af13475e6650a5506c. The previous
> implementation was
> > non-optional and flawed.
> >
> > This implementation uses the tsc of vcpu0, which is preserved across
> > save/restore as part of the architectural state, and then converts that
> > to a 100ns tick using the domain's tsc_khz.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Cc: Keir Fraser <keir@xxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > ---
> >  docs/man/xl.cfg.pod.5            |    7 +++++++
> >  tools/libxl/libxl_dom.c          |    2 ++
> >  xen/arch/x86/hvm/viridian.c      |   24 +++++++++++++++++++-----
> >  xen/include/asm-x86/perfc_defn.h |    1 +
> >  xen/include/public/hvm/params.h  |    9 ++++++++-
> >  5 files changed, 37 insertions(+), 6 deletions(-)
> >
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > index a05e831..e4692982 100644
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -1109,6 +1109,13 @@ This set incorporates the TSC and APIC
> frequency MSRs.
> >  This enlightenment can improve performance of Windows 7 and Windows
> >  Server 2008 R2 onwards.
> >
> > +=item B<time-ref-count>
> > +
> > +This set incorporates Partition Time Reference Counter MSR.
> > +
> > +This enlightenment can improve performance of Windows 8 and Windows
> > +Server 2012 onwards.
> > +
> >  =back
> >
> >  See the latest version of Microsoft's Hypervisor Top-Level Functional
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index cc57931..224a4d9 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -222,6 +222,8 @@ static void hvm_set_viridian_features(libxl__gc *gc,
> uint32_t domid,
> >              feature_mask |= HVMPV_base_freq;
> >          else if (strcmp(*p, "freq") == 0)
> >              feature_mask &= ~HVMPV_no_freq;
> > +        else if (strcmp(*p, "time-ref-count") == 0)
> > +            feature_mask |= HVMPV_time_ref_count;
> >          p++;
> >      }
> >
> > diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> > index 6fcfa96..5e99af2 100644
> > --- a/xen/arch/x86/hvm/viridian.c
> > +++ b/xen/arch/x86/hvm/viridian.c
> > @@ -36,11 +36,11 @@
> >  #define HvNotifyLongSpinWait    8
> >
> >  /* Viridian CPUID 4000003, Viridian MSR availability. */
> > -#define CPUID3A_MSR_REF_COUNT   (1 << 1)
> > -#define CPUID3A_MSR_APIC_ACCESS (1 << 4)
> > -#define CPUID3A_MSR_HYPERCALL   (1 << 5)
> > -#define CPUID3A_MSR_VP_INDEX    (1 << 6)
> > -#define CPUID3A_MSR_FREQ        (1 << 11)
> > +#define CPUID3A_MSR_TIME_REF_COUNT (1 << 1)
> > +#define CPUID3A_MSR_APIC_ACCESS    (1 << 4)
> > +#define CPUID3A_MSR_HYPERCALL      (1 << 5)
> > +#define CPUID3A_MSR_VP_INDEX       (1 << 6)
> > +#define CPUID3A_MSR_FREQ           (1 << 11)
> >
> >  /* Viridian CPUID 4000004, Implementation Recommendations. */
> >  #define CPUID4A_MSR_BASED_APIC  (1 << 3)
> > @@ -93,6 +93,8 @@ int cpuid_viridian_leaves(unsigned int leaf, unsigned
> int *eax,
> >                  CPUID3A_MSR_VP_INDEX);
> >          if ( !(viridian_feature_mask(d) & HVMPV_no_freq) )
> >              *eax |= CPUID3A_MSR_FREQ;
> > +        if ( viridian_feature_mask(d) & HVMPV_time_ref_count )
> > +            *eax |= CPUID3A_MSR_TIME_REF_COUNT;
> >          break;
> >      case 4:
> >          /* Recommended hypercall usage. */
> > @@ -349,6 +351,18 @@ int rdmsr_viridian_regs(uint32_t idx, uint64_t *val)
> >          *val = v->arch.hvm_vcpu.viridian.apic_assist.raw;
> >          break;
> >
> > +    case VIRIDIAN_MSR_TIME_REF_COUNT:
> > +    {
> > +        uint64_t tsc;
> > +
> > +        if ( !(viridian_feature_mask(d) & HVMPV_time_ref_count) )
> > +            return 0;
> > +
> > +        perfc_incr(mshv_rdmsr_time_ref_count);
> > +        tsc = hvm_get_guest_tsc(pt_global_vcpu_target(d));
> > +        *val = (tsc * 10000ull) / d->arch.tsc_khz;
> 
> This is correct by math but not in implementation.
> You need to stay within 64bits. Please add the math formula
> in a comment.
> 

Good point. I'll use the tsc scale code to do the calculation.

  Paul

> Christoph
> 
> > +        break;
> > +    }
> >      default:
> >          return 0;
> >      }
> > diff --git a/xen/include/asm-x86/perfc_defn.h b/xen/include/asm-
> x86/perfc_defn.h
> > index 7d802cc..170da00 100644
> > --- a/xen/include/asm-x86/perfc_defn.h
> > +++ b/xen/include/asm-x86/perfc_defn.h
> > @@ -120,6 +120,7 @@ PERFCOUNTER(mshv_rdmsr_hc_page,         "MS Hv
> rdmsr hypercall page")
> >  PERFCOUNTER(mshv_rdmsr_vp_index,        "MS Hv rdmsr vp index")
> >  PERFCOUNTER(mshv_rdmsr_tsc_frequency,   "MS Hv rdmsr TSC
> frequency")
> >  PERFCOUNTER(mshv_rdmsr_apic_frequency,  "MS Hv rdmsr APIC
> frequency")
> > +PERFCOUNTER(mshv_rdmsr_time_ref_count,  "MS Hv rdmsr time ref
> count")
> >  PERFCOUNTER(mshv_rdmsr_icr,             "MS Hv rdmsr icr")
> >  PERFCOUNTER(mshv_rdmsr_tpr,             "MS Hv rdmsr tpr")
> >  PERFCOUNTER(mshv_rdmsr_apic_assist,     "MS Hv rdmsr APIC assist")
> > diff --git a/xen/include/public/hvm/params.h
> b/xen/include/public/hvm/params.h
> > index 68d26fd..3c51072 100644
> > --- a/xen/include/public/hvm/params.h
> > +++ b/xen/include/public/hvm/params.h
> > @@ -88,7 +88,14 @@
> >  #define _HVMPV_no_freq 1
> >  #define HVMPV_no_freq  (1 << _HVMPV_no_freq)
> >
> > -#define HVMPV_feature_mask (HVMPV_base_freq|HVMPV_no_freq)
> > +/* Enable Partition Time Reference Counter
> (HV_X64_MSR_TIME_REF_COUNT) */
> > +#define _HVMPV_time_ref_count 2
> > +#define HVMPV_time_ref_count  (1 << _HVMPV_time_ref_count)
> > +
> > +#define HVMPV_feature_mask \
> > +   (HVMPV_base_freq | \
> > +    HVMPV_no_freq | \
> > +    HVMPV_time_ref_count)
> >
> >  #endif
> >
> >


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