Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function

[Keir Fraser <keir@xen.org>]

On 19/01/2012 13:54, "Dietmar Hahn" <dietmar.hahn@ts.fujitsu.com> wrote:

> 
>  tools/libxc/xc_cpuid_x86.c        |   1 +
>  xen/arch/x86/hvm/vmx/vmx.c        |   2 ++
>  xen/arch/x86/hvm/vmx/vpmu_core2.c |  38
> ++++++++++++++++++++++++++++++++++++--
>  xen/arch/x86/hvm/vpmu.c           |  25 ++++++++++++++++++-------
>  xen/include/asm-x86/hvm/vpmu.h    |   6 ++++++
>  5 files changed, 63 insertions(+), 9 deletions(-)
> 
> 
> The  "Debug Store" cpuid flag in the Intel processors gets enabled in the
> libxc.
> A new function call arch_vpmu_cpuid is added to the struct arch_vpmu_ops and
> for
> Intel processors the function core2_vpmu_cpuid() is added.
> The aim is that always a "struct arch_vpmu_ops" is accessible at least with
> a cpuid function to switch off special PMU cpuid flags dynamically depending
> on
> the boot variable opt_vpmu_enabled.

Our CPUID configuration is done per-domain, and from
tools/libxc/xc_cpuid_x86.c. CPUID adjustments implemented within the
hypervisor are generally not acceptable without very good reason.

 -- Keir

> Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
> 
> diff -r 7a82c2e2eb33 tools/libxc/xc_cpuid_x86.c
> --- a/tools/libxc/xc_cpuid_x86.c Thu Jan 19 13:14:02 2012 +0100
> +++ b/tools/libxc/xc_cpuid_x86.c Thu Jan 19 14:37:17 2012 +0100
> @@ -343,6 +343,7 @@ static void xc_cpuid_hvm_policy(
>                      bitmaskof(X86_FEATURE_CMOV) |
>                      bitmaskof(X86_FEATURE_PAT) |
>                      bitmaskof(X86_FEATURE_CLFLSH) |
> +                    bitmaskof(X86_FEATURE_DS) |
>                      bitmaskof(X86_FEATURE_PSE36) |
>                      bitmaskof(X86_FEATURE_MMX) |
>                      bitmaskof(X86_FEATURE_FXSR) |
> diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 13:14:02 2012 +0100
> +++ b/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 14:37:17 2012 +0100
> @@ -1603,6 +1603,8 @@ static void vmx_cpuid_intercept(
>              break;
>      }
>  
> +    vpmu_do_cpuid(input, eax, ebx, ecx, edx);
> +
>      HVMTRACE_5D (CPUID, input, *eax, *ebx, *ecx, *edx);
>  }
>  
> diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vpmu_core2.c
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 13:14:02 2012 +0100
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 14:37:17 2012 +0100
> @@ -598,6 +598,29 @@ static void core2_vpmu_destroy(struct vc
>      vpmu->flags &= ~VPMU_CONTEXT_ALLOCATED;
>  }
>  
> +/**
> + * core2_vpmu_cpuid - prepare special vpmu cpuid bits
> + * If emulation of vpmu is switched off, some bits are swtiched off,
> currently:
> + * - EAX[0x1].EAX[Bits 0-7]: PMC revision id.
> + * - EAX[0xa].EDX[Bit 21]:   Debug Store
> + */
> +#define bitmaskof(idx)  (1U << ((idx) & 31))
> +static void core2_vpmu_cpuid(unsigned int input,
> +                             unsigned int *eax, unsigned int *ebx,
> +                             unsigned int *ecx, unsigned int *edx)
> +{
> +    switch ( input )
> +    {
> +    case 0x1:
> +        *edx &= ~bitmaskof(X86_FEATURE_DS);    /* Debug Store not supported
> */
> +        break;
> +    case 0xa:
> +        if ( !opt_vpmu_enabled )
> +            *eax &= ~(unsigned int)0xff;       /* Clear pmc version id. */
> +        break;
> +    }
> +}
> +
>  struct arch_vpmu_ops core2_vpmu_ops = {
>      .do_wrmsr = core2_vpmu_do_wrmsr,
>      .do_rdmsr = core2_vpmu_do_rdmsr,
> @@ -605,7 +628,13 @@ struct arch_vpmu_ops core2_vpmu_ops = {
>      .arch_vpmu_initialise = core2_vpmu_initialise,
>      .arch_vpmu_destroy = core2_vpmu_destroy,
>      .arch_vpmu_save = core2_vpmu_save,
> -    .arch_vpmu_load = core2_vpmu_load
> +    .arch_vpmu_load = core2_vpmu_load,
> +    .arch_vpmu_cpuid = core2_vpmu_cpuid
> +};
> +
> +/* Used if vpmu is disabled. */
> +struct arch_vpmu_ops core2_vpmu_dis_ops = {
> +    .arch_vpmu_cpuid = core2_vpmu_cpuid
>  };
>  
>  int vmx_vpmu_initialize(struct vcpu *v)
> @@ -615,7 +644,7 @@ int vmx_vpmu_initialize(struct vcpu *v)
>      __u8 cpu_model = current_cpu_data.x86_model;
>  
>      if ( !opt_vpmu_enabled )
> -        return -EINVAL;
> +        goto func_out;
>  
>      if ( family == 6 )
>      {
> @@ -635,6 +664,11 @@ int vmx_vpmu_initialize(struct vcpu *v)
>      printk("VPMU: Initialization failed. "
>             "Intel processor family %d model %d has not "
>             "been supported\n", family, cpu_model);
> +
> +func_out:
> +
> +    vpmu->arch_vpmu_ops = &core2_vpmu_dis_ops;
> +
>      return -EINVAL;
>  }
>  
> diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vpmu.c
> --- a/xen/arch/x86/hvm/vpmu.c Thu Jan 19 13:14:02 2012 +0100
> +++ b/xen/arch/x86/hvm/vpmu.c Thu Jan 19 14:37:17 2012 +0100
> @@ -39,7 +39,7 @@ int vpmu_do_wrmsr(unsigned int msr, uint
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(current);
>  
> -    if ( vpmu->arch_vpmu_ops )
> +    if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
>          return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
>      return 0;
>  }
> @@ -48,7 +48,7 @@ int vpmu_do_rdmsr(unsigned int msr, uint
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(current);
>  
> -    if ( vpmu->arch_vpmu_ops )
> +    if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr )
>          return vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
>      return 0;
>  }
> @@ -57,7 +57,7 @@ int vpmu_do_interrupt(struct cpu_user_re
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(current);
>  
> -    if ( vpmu->arch_vpmu_ops )
> +    if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_interrupt )
>          return vpmu->arch_vpmu_ops->do_interrupt(regs);
>      return 0;
>  }
> @@ -66,7 +66,7 @@ void vpmu_save(struct vcpu *v)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>  
> -    if ( vpmu->arch_vpmu_ops )
> +    if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_save )
>          vpmu->arch_vpmu_ops->arch_vpmu_save(v);
>  }
>  
> @@ -74,7 +74,7 @@ void vpmu_load(struct vcpu *v)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>  
> -    if ( vpmu->arch_vpmu_ops )
> +    if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_load )
>          vpmu->arch_vpmu_ops->arch_vpmu_load(v);
>  }
>  
> @@ -109,7 +109,8 @@ void vpmu_initialise(struct vcpu *v)
>      {
>          vpmu->flags = 0;
>          vpmu->context = NULL;
> -        vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
> +        if ( vpmu->arch_vpmu_ops->arch_vpmu_initialise )
> +            vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
>      }
>  }
>  
> @@ -117,7 +118,17 @@ void vpmu_destroy(struct vcpu *v)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>  
> -    if ( vpmu->arch_vpmu_ops )
> +    if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_destroy )
>          vpmu->arch_vpmu_ops->arch_vpmu_destroy(v);
>  }
>  
> +void vpmu_do_cpuid(unsigned int input,
> +                   unsigned int *eax, unsigned int *ebx,
> +                   unsigned int *ecx, unsigned int *edx)
> +{
> +    struct vpmu_struct *vpmu = vcpu_vpmu(current);
> +
> +    if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_cpuid)
> +        vpmu->arch_vpmu_ops->arch_vpmu_cpuid(input, eax, ebx, ecx, edx);
> +}
> +
> diff -r 7a82c2e2eb33 xen/include/asm-x86/hvm/vpmu.h
> --- a/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 13:14:02 2012 +0100
> +++ b/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 14:37:17 2012 +0100
> @@ -56,6 +56,9 @@ struct arch_vpmu_ops {
>      void (*arch_vpmu_destroy)(struct vcpu *v);
>      void (*arch_vpmu_save)(struct vcpu *v);
>      void (*arch_vpmu_load)(struct vcpu *v);
> +    void (*arch_vpmu_cpuid)(unsigned int input,
> +                            unsigned int *eax, unsigned int *ebx,
> +                            unsigned int *ecx, unsigned int *edx);
>  };
>  
>  int vmx_vpmu_initialize(struct vcpu *v);
> @@ -78,6 +81,9 @@ void vpmu_initialise(struct vcpu *v);
>  void vpmu_destroy(struct vcpu *v);
>  void vpmu_save(struct vcpu *v);
>  void vpmu_load(struct vcpu *v);
> +void vpmu_do_cpuid(unsigned int input,
> +                   unsigned int *eax, unsigned int *ebx,
> +                   unsigned int *ecx, unsigned int *edx);
>  
>  extern int acquire_pmu_ownership(int pmu_ownership);
>  extern void release_pmu_ownership(int pmu_ownership);