| Message ID | 1487148579-7243-18-git-send-email-yi.y.sun@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 15, 2017 at 04:49:32PM +0800, Yi Sun wrote: > This patch implements the CPU init and free flow for L2 CAT including > L2 CAT initialization callback function. > > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > --- > xen/arch/x86/psr.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++ > xen/include/asm-x86/psr.h | 1 + > 2 files changed, 73 insertions(+) > > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c > index 0a0bab9..4489637 100644 > --- a/xen/arch/x86/psr.c > +++ b/xen/arch/x86/psr.c > @@ -98,6 +98,7 @@ struct feat_hw_info { > union { > struct psr_cat_hw_info l3_cat_info; > struct psr_cat_hw_info l3_cdp_info; > + struct psr_cat_hw_info l2_cat_info; > }; > }; > > @@ -235,6 +236,7 @@ static DEFINE_PER_CPU(struct psr_assoc, psr_assoc); > */ > static struct feat_node *feat_l3_cat; > static struct feat_node *feat_l3_cdp; > +static struct feat_node *feat_l2_cat; > > /* Common functions. */ > static void free_feature(struct psr_socket_info *info) > @@ -684,6 +686,53 @@ struct feat_ops l3_cdp_ops = { > .write_msr = l3_cdp_write_msr, > }; > > +/* L2 CAT callback functions implementation. */ > +static void l2_cat_init_feature(struct cpuid_leaf regs, > + struct feat_node *feat, > + struct psr_socket_info *info) This is exactly the same code as l3_cat_init_feature with s/l3/l2/. I think those two functions can somehow be merged, and I'm sure there's room for other merges, IMHO L2/L3 CAT implementations should share a bunch of code, there's too much duplication. Roger.
On 17-02-28 15:15:36, Roger Pau Monn� wrote: > On Wed, Feb 15, 2017 at 04:49:32PM +0800, Yi Sun wrote: > > This patch implements the CPU init and free flow for L2 CAT including > > L2 CAT initialization callback function. > > > > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > > --- > > xen/arch/x86/psr.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++ > > xen/include/asm-x86/psr.h | 1 + > > 2 files changed, 73 insertions(+) > > > > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c > > index 0a0bab9..4489637 100644 > > --- a/xen/arch/x86/psr.c > > +++ b/xen/arch/x86/psr.c > > @@ -98,6 +98,7 @@ struct feat_hw_info { > > union { > > struct psr_cat_hw_info l3_cat_info; > > struct psr_cat_hw_info l3_cdp_info; > > + struct psr_cat_hw_info l2_cat_info; > > }; > > }; > > > > @@ -235,6 +236,7 @@ static DEFINE_PER_CPU(struct psr_assoc, psr_assoc); > > */ > > static struct feat_node *feat_l3_cat; > > static struct feat_node *feat_l3_cdp; > > +static struct feat_node *feat_l2_cat; > > > > /* Common functions. */ > > static void free_feature(struct psr_socket_info *info) > > @@ -684,6 +686,53 @@ struct feat_ops l3_cdp_ops = { > > .write_msr = l3_cdp_write_msr, > > }; > > > > +/* L2 CAT callback functions implementation. */ > > +static void l2_cat_init_feature(struct cpuid_leaf regs, > > + struct feat_node *feat, > > + struct psr_socket_info *info) > > This is exactly the same code as l3_cat_init_feature with s/l3/l2/. I think > those two functions can somehow be merged, and I'm sure there's room for other > merges, IMHO L2/L3 CAT implementations should share a bunch of code, there's > too much duplication. > Thanks! Will consider to split common functions out. > Roger.
>>> On 15.02.17 at 09:49, <yi.y.sun@linux.intel.com> wrote: > @@ -684,6 +686,53 @@ struct feat_ops l3_cdp_ops = { > .write_msr = l3_cdp_write_msr, > }; > > +/* L2 CAT callback functions implementation. */ > +static void l2_cat_init_feature(struct cpuid_leaf regs, const struct cpuid_leaf * (and of course I should have noticed this already in earlier patches, but I didn't) > + struct feat_node *feat, > + struct psr_socket_info *info) > +{ > + struct psr_cat_hw_info l2_cat = { }; I realize Konrad did ask for the initializer here, but it's really pointless to have such when both structure fields get very obviously set right below. If you want an initializer, just set the fields to their final values. > + unsigned int socket; > + > + /* No valid values so do not enable the feature. */ > + if ( !regs.a || !regs.d ) > + return; > + > + l2_cat.cbm_len = (regs.a & CAT_CBM_LEN_MASK) + 1; > + l2_cat.cos_max = min(opt_cos_max, regs.d & CAT_COS_MAX_MASK); > + > + /* cos=0 is reserved as default cbm(all ones). */ > + feat->cos_reg_val[0] = (1ull << l2_cat.cbm_len) - 1; > + > + feat->feature = PSR_SOCKET_L2_CAT; > + ASSERT(!test_bit(PSR_SOCKET_L2_CAT, &info->feat_mask)); > + __set_bit(PSR_SOCKET_L2_CAT, &info->feat_mask); > + > + feat->info.l2_cat_info = l2_cat; > + > + info->nr_feat++; Is this field really used for anything? I don't recall seeing other than these increments. Jan
On 17-03-09 08:04:35, Jan Beulich wrote: > >>> On 15.02.17 at 09:49, <yi.y.sun@linux.intel.com> wrote: > > @@ -684,6 +686,53 @@ struct feat_ops l3_cdp_ops = { > > .write_msr = l3_cdp_write_msr, > > }; > > > > +/* L2 CAT callback functions implementation. */ > > +static void l2_cat_init_feature(struct cpuid_leaf regs, > > const struct cpuid_leaf * (and of course I should have noticed this > already in earlier patches, but I didn't) > Will fixed this from beginning. > > + struct feat_node *feat, > > + struct psr_socket_info *info) > > +{ > > + struct psr_cat_hw_info l2_cat = { }; > > I realize Konrad did ask for the initializer here, but it's really pointless > to have such when both structure fields get very obviously set right > below. If you want an initializer, just set the fields to their final values. > Got it. Thanks! > > + unsigned int socket; > > + > > + /* No valid values so do not enable the feature. */ > > + if ( !regs.a || !regs.d ) > > + return; > > + > > + l2_cat.cbm_len = (regs.a & CAT_CBM_LEN_MASK) + 1; > > + l2_cat.cos_max = min(opt_cos_max, regs.d & CAT_COS_MAX_MASK); > > + > > + /* cos=0 is reserved as default cbm(all ones). */ > > + feat->cos_reg_val[0] = (1ull << l2_cat.cbm_len) - 1; > > + > > + feat->feature = PSR_SOCKET_L2_CAT; > > + ASSERT(!test_bit(PSR_SOCKET_L2_CAT, &info->feat_mask)); > > + __set_bit(PSR_SOCKET_L2_CAT, &info->feat_mask); > > + > > + feat->info.l2_cat_info = l2_cat; > > + > > + info->nr_feat++; > > Is this field really used for anything? I don't recall seeing other than > these increments. > Just for a record. Will remove it. > Jan
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c index 0a0bab9..4489637 100644 --- a/xen/arch/x86/psr.c +++ b/xen/arch/x86/psr.c @@ -98,6 +98,7 @@ struct feat_hw_info { union { struct psr_cat_hw_info l3_cat_info; struct psr_cat_hw_info l3_cdp_info; + struct psr_cat_hw_info l2_cat_info; }; }; @@ -235,6 +236,7 @@ static DEFINE_PER_CPU(struct psr_assoc, psr_assoc); */ static struct feat_node *feat_l3_cat; static struct feat_node *feat_l3_cdp; +static struct feat_node *feat_l2_cat; /* Common functions. */ static void free_feature(struct psr_socket_info *info) @@ -684,6 +686,53 @@ struct feat_ops l3_cdp_ops = { .write_msr = l3_cdp_write_msr, }; +/* L2 CAT callback functions implementation. */ +static void l2_cat_init_feature(struct cpuid_leaf regs, + struct feat_node *feat, + struct psr_socket_info *info) +{ + struct psr_cat_hw_info l2_cat = { }; + unsigned int socket; + + /* No valid values so do not enable the feature. */ + if ( !regs.a || !regs.d ) + return; + + l2_cat.cbm_len = (regs.a & CAT_CBM_LEN_MASK) + 1; + l2_cat.cos_max = min(opt_cos_max, regs.d & CAT_COS_MAX_MASK); + + /* cos=0 is reserved as default cbm(all ones). */ + feat->cos_reg_val[0] = (1ull << l2_cat.cbm_len) - 1; + + feat->feature = PSR_SOCKET_L2_CAT; + ASSERT(!test_bit(PSR_SOCKET_L2_CAT, &info->feat_mask)); + __set_bit(PSR_SOCKET_L2_CAT, &info->feat_mask); + + feat->info.l2_cat_info = l2_cat; + + info->nr_feat++; + + /* Add this feature into list. */ + list_add_tail(&feat->list, &info->feat_list); + + socket = cpu_to_socket(smp_processor_id()); + if ( !opt_cpu_info ) + return; + + printk(XENLOG_INFO "L2 CAT: enabled on socket %u, cos_max:%u, cbm_len:%u.\n", + socket, feat->info.l2_cat_info.cos_max, + feat->info.l2_cat_info.cbm_len); +} + +static unsigned int l2_cat_get_cos_max(const struct feat_node *feat) +{ + return feat->info.l2_cat_info.cos_max; +} + +struct feat_ops l2_cat_ops = { + .get_cos_max = l2_cat_get_cos_max, +}; + static void __init parse_psr_bool(char *s, char *value, char *feature, unsigned int mask) { @@ -1456,6 +1505,18 @@ static void cpu_init_work(void) l3_cat_init_feature(regs, feat, info); } } + + cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 0, ®s); + if ( regs.b & PSR_RESOURCE_TYPE_L2 ) + { + cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 2, ®s); + + feat = feat_l2_cat; + /* psr_cpu_prepare will allocate it on subsequent CPU onlining. */ + feat_l2_cat = NULL; + feat->ops = l2_cat_ops; + l2_cat_init_feature(regs, feat, info); + } } static void cpu_fini_work(unsigned int cpu) @@ -1511,6 +1572,17 @@ static int psr_cpu_prepare(void) return -ENOMEM; } + if ( feat_l2_cat == NULL && + (feat_l2_cat = xzalloc(struct feat_node)) == NULL ) + { + xfree(feat_l3_cat); + feat_l3_cat = NULL; + + xfree(feat_l3_cdp); + feat_l3_cdp = NULL; + return -ENOMEM; + } + return 0; } diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h index fde7882..d7ed012 100644 --- a/xen/include/asm-x86/psr.h +++ b/xen/include/asm-x86/psr.h @@ -23,6 +23,7 @@ /* Resource Type Enumeration */ #define PSR_RESOURCE_TYPE_L3 0x2 +#define PSR_RESOURCE_TYPE_L2 0x4 /* L3 Monitoring Features */ #define PSR_CMT_L3_OCCUPANCY 0x1
This patch implements the CPU init and free flow for L2 CAT including L2 CAT initialization callback function. Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> --- xen/arch/x86/psr.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++ xen/include/asm-x86/psr.h | 1 + 2 files changed, 73 insertions(+)