[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 20/01/2012 10:29, "Keir Fraser" <keir@xxxxxxx> wrote:

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

I'll preempt you saying that you need to depend on opt_vpmu_enabled by
saying you should get rid of that hacky global configuration flag, and allow
VPMU to be properly enabled per domain, via the toolstack. Then
configuration via libxc/xc_cpuid_x86.c will fall naturally into place.
Perhaps you can allow configuration straightforwardly via the existing
libxl_cpuid.c mechanism, or perhaps you need a higher-level config option
than that, and then cook it down into CPUID fiddling from within libxl, or
libxc. One of the toolstack maintainers (Ian Jackson for example) may have
better ideas than me on that.

Also, there are AMD- and Intel-specific sections to xc_cpuid_x86.c for HVM
guests -- {amd,intel}_xc_cpuid_policy(). You can use those to set flags
differently for the two vendors.

>  -- Keir
> 
>> Signed-off-by: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>
>> 
>> 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);
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.