Re: [PATCH v16 06/10] x86: collect global CMT information

[Chao Peng <chao.p.peng@linux.intel.com>]

On Thu, Sep 25, 2014 at 04:53:58PM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 25, 2014 at 06:19:06PM +0800, Chao Peng wrote:
> > This implementation tries to put all policies into user space, thus some
> > global CMT information needs to be exposed, such as the total RMID count,
> > L3 upscaling factor, etc.
> > 
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > Acked-by: Jan Beulich <jbeulich@suse.com>
> > ---
> >  xen/arch/x86/cpu/intel_cacheinfo.c |   49 ++----------------------------------
> >  xen/arch/x86/sysctl.c              |   43 +++++++++++++++++++++++++++++++
> >  xen/include/asm-x86/cpufeature.h   |   45 +++++++++++++++++++++++++++++++++
> >  xen/include/public/sysctl.h        |   14 +++++++++++
> >  4 files changed, 104 insertions(+), 47 deletions(-)
> > 
> > diff --git a/xen/arch/x86/cpu/intel_cacheinfo.c b/xen/arch/x86/cpu/intel_cacheinfo.c
> > index 430f939..48970c0 100644
> > --- a/xen/arch/x86/cpu/intel_cacheinfo.c
> > +++ b/xen/arch/x86/cpu/intel_cacheinfo.c
> > @@ -81,54 +81,9 @@ static struct _cache_table cache_table[] __cpuinitdata =
> >  	{ 0x00, 0, 0}
> >  };
> >  
> > -
> > -enum _cache_type
> > -{
> > -	CACHE_TYPE_NULL	= 0,
> > -	CACHE_TYPE_DATA = 1,
> > -	CACHE_TYPE_INST = 2,
> > -	CACHE_TYPE_UNIFIED = 3
> > -};
> > -
> > -union _cpuid4_leaf_eax {
> > -	struct {
> > -		enum _cache_type	type:5;
> > -		unsigned int		level:3;
> > -		unsigned int		is_self_initializing:1;
> > -		unsigned int		is_fully_associative:1;
> > -		unsigned int		reserved:4;
> > -		unsigned int		num_threads_sharing:12;
> > -		unsigned int		num_cores_on_die:6;
> > -	} split;
> > -	u32 full;
> > -};
> > -
> > -union _cpuid4_leaf_ebx {
> > -	struct {
> > -		unsigned int		coherency_line_size:12;
> > -		unsigned int		physical_line_partition:10;
> > -		unsigned int		ways_of_associativity:10;
> > -	} split;
> > -	u32 full;
> > -};
> > -
> > -union _cpuid4_leaf_ecx {
> > -	struct {
> > -		unsigned int		number_of_sets:32;
> > -	} split;
> > -	u32 full;
> > -};
> > -
> > -struct _cpuid4_info {
> > -	union _cpuid4_leaf_eax eax;
> > -	union _cpuid4_leaf_ebx ebx;
> > -	union _cpuid4_leaf_ecx ecx;
> > -	unsigned long size;
> > -};
> > -
> >  unsigned short			num_cache_leaves;
> >  
> > -static int __cpuinit cpuid4_cache_lookup(int index, struct _cpuid4_info *this_leaf)
> > +int cpuid4_cache_lookup(int index, struct cpuid4_info *this_leaf)
> >  {
> >  	union _cpuid4_leaf_eax 	eax;
> >  	union _cpuid4_leaf_ebx 	ebx;
> > @@ -185,7 +140,7 @@ unsigned int __cpuinit init_intel_cacheinfo(struct cpuinfo_x86 *c)
> >  		 * parameters cpuid leaf to find the cache details
> >  		 */
> >  		for (i = 0; i < num_cache_leaves; i++) {
> > -			struct _cpuid4_info this_leaf;
> > +			struct cpuid4_info this_leaf;
> >  
> >  			int retval;
> >  
> > diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> > index 15d4b91..b95408f 100644
> > --- a/xen/arch/x86/sysctl.c
> > +++ b/xen/arch/x86/sysctl.c
> > @@ -28,6 +28,7 @@
> >  #include <xen/nodemask.h>
> >  #include <xen/cpu.h>
> >  #include <xsm/xsm.h>
> > +#include <asm/psr.h>
> >  
> >  #define get_xen_guest_handle(val, hnd)  do { val = (hnd).p; } while (0)
> >  
> > @@ -101,6 +102,48 @@ long arch_do_sysctl(
> >      }
> >      break;
> >  
> > +    case XEN_SYSCTL_psr_cmt_op:
> > +        if ( !psr_cmt_enabled() )
> > +            return -ENODEV;
> > +
> > +        if ( sysctl->u.psr_cmt_op.flags != 0 )
> > +            return -EINVAL;
> > +
> > +        switch ( sysctl->u.psr_cmt_op.cmd )
> > +        {
> > +        case XEN_SYSCTL_PSR_CMT_enabled:
> > +            sysctl->u.psr_cmt_op.data =
> > +                (psr_cmt->features & PSR_RESOURCE_TYPE_L3) &&
> > +                (psr_cmt->l3.features & PSR_CMT_L3_OCCUPANCY);
> > +            break;
> > +        case XEN_SYSCTL_PSR_CMT_get_total_rmid:
> > +            sysctl->u.psr_cmt_op.data = psr_cmt->rmid_max;
> > +            break;
> > +        case XEN_SYSCTL_PSR_CMT_get_l3_upscaling_factor:
> > +            sysctl->u.psr_cmt_op.data = psr_cmt->l3.upscaling_factor;
> > +            break;
> > +        case XEN_SYSCTL_PSR_CMT_get_l3_cache_size:
> > +        {
> > +            struct cpuid4_info info;
> > +
> > +            ret = cpuid4_cache_lookup(3, &info);
> 
> Couldn't you use 'struct cpuinfo_x86' and extend it if you need to?
I can, indeed. Field 'x86_cache_size' is actully l3 cache size if it is
available. I still need to add a new field to indicate it's l3 to use
in this way.
> 
> 
> > +            if ( ret < 0 )
> > +                break;
> > +
> > +            sysctl->u.psr_cmt_op.data = info.size / 1024; /* in KB unit */
> 
> With the Haswell EP they have this weird setup where there
> are 8 cores on one side and 10 cores on another. Also the cache size is
> different (20MB LLC and 25MB LLC). With that wouldn't you want to enumerate
> exactly _which_ CPU cache you want instead of the one you running at?
> 
> Or is my reading of the diagrams wrong and OS never sees the split and
> gets 45MB?
Not sure as I don't have such machine. If this is the case, better to
use per-socket value here.
> 
> 
> > +        }
> > +        break;
> > +        default:
> > +            sysctl->u.psr_cmt_op.data = 0;
> > +            ret = -ENOSYS;
> > +            break;
> > +        }
> > +
> > +        if ( __copy_to_guest(u_sysctl, sysctl, 1) )
> > +            ret = -EFAULT;
> > +
> > +        break;
> > +
> >      default:
> >          ret = -ENOSYS;
> >          break;
> > diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
> > index 137d75c..d3bd14d 100644
> > --- a/xen/include/asm-x86/cpufeature.h
> > +++ b/xen/include/asm-x86/cpufeature.h
> > @@ -215,6 +215,51 @@
> >  #define cpu_has_vmx		boot_cpu_has(X86_FEATURE_VMXE)
> >  
> >  #define cpu_has_cpuid_faulting	boot_cpu_has(X86_FEATURE_CPUID_FAULTING)
> > +
> > +enum _cache_type {
> > +    CACHE_TYPE_NULL = 0,
> > +    CACHE_TYPE_DATA = 1,
> > +    CACHE_TYPE_INST = 2,
> > +    CACHE_TYPE_UNIFIED = 3
> > +};
> > +
> > +union _cpuid4_leaf_eax {
> > +    struct {
> > +        enum _cache_type type:5;
> > +        unsigned int level:3;
> > +        unsigned int is_self_initializing:1;
> > +        unsigned int is_fully_associative:1;
> > +        unsigned int reserved:4;
> > +        unsigned int num_threads_sharing:12;
> > +        unsigned int num_cores_on_die:6;
> > +    } split;
> > +    u32 full;
> > +};
> > +
> > +union _cpuid4_leaf_ebx {
> > +    struct {
> > +        unsigned int coherency_line_size:12;
> > +        unsigned int physical_line_partition:10;
> > +        unsigned int ways_of_associativity:10;
> > +    } split;
> > +    u32 full;
> > +};
> > +
> > +union _cpuid4_leaf_ecx {
> > +    struct {
> > +        unsigned int number_of_sets:32;
> > +    } split;
> > +    u32 full;
> > +};
> > +
> > +struct cpuid4_info {
> > +    union _cpuid4_leaf_eax eax;
> > +    union _cpuid4_leaf_ebx ebx;
> > +    union _cpuid4_leaf_ecx ecx;
> > +    unsigned long size;
> > +};
> > +
> > +int cpuid4_cache_lookup(int index, struct cpuid4_info *this_leaf);
> >  #endif
> >  
> >  #endif /* __ASM_I386_CPUFEATURE_H */
> > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> > index 3588698..66b6e47 100644
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -636,6 +636,18 @@ struct xen_sysctl_coverage_op {
> >  typedef struct xen_sysctl_coverage_op xen_sysctl_coverage_op_t;
> >  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_coverage_op_t);
> >  
> > +#define XEN_SYSCTL_PSR_CMT_get_total_rmid            0
> > +#define XEN_SYSCTL_PSR_CMT_get_l3_upscaling_factor   1
> > +/* The L3 cache size is returned in KB unit */
> > +#define XEN_SYSCTL_PSR_CMT_get_l3_cache_size         2
> > +#define XEN_SYSCTL_PSR_CMT_enabled                   3
> > +struct xen_sysctl_psr_cmt_op {
> > +    uint32_t cmd;
> > +    uint32_t flags;      /* padding variable, may be extended for future use */
> > +    uint64_t data;
> > +};
> > +typedef struct xen_sysctl_psr_cmt_op xen_sysctl_psr_cmt_op_t;
> > +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cmt_op_t);
> >  
> >  struct xen_sysctl {
> >      uint32_t cmd;
> > @@ -658,6 +670,7 @@ struct xen_sysctl {
> >  #define XEN_SYSCTL_cpupool_op                    18
> >  #define XEN_SYSCTL_scheduler_op                  19
> >  #define XEN_SYSCTL_coverage_op                   20
> > +#define XEN_SYSCTL_psr_cmt_op                    21
> >      uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
> >      union {
> >          struct xen_sysctl_readconsole       readconsole;
> > @@ -679,6 +692,7 @@ struct xen_sysctl {
> >          struct xen_sysctl_cpupool_op        cpupool_op;
> >          struct xen_sysctl_scheduler_op      scheduler_op;
> >          struct xen_sysctl_coverage_op       coverage_op;
> > +        struct xen_sysctl_psr_cmt_op        psr_cmt_op;
> >          uint8_t                             pad[128];
> >      } u;
> >  };
> > -- 
> > 1.7.9.5
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel