|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v2 2/3] x86: add support for L2 CAT in hypervisor.
On 16-09-30 17:23:43, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 22, 2016 at 10:15:44AM +0800, Yi Sun wrote:
> > Add L2 CAT (Cache Allocation Technology) feature support in
> > hypervisor:
> > - Implement 'struct feat_ops' callback functions for L2 CAT
> > and initialize L2 CAT feature and add it into feature list.
> > - Add new sysctl to get L2 CAT information.
> > - Add new domctl to set/get L2 CAT CBM.
> >
> > Signed-off-by: He Chen <he.chen@xxxxxxxxxxxxxxx>
> > Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> >
> > ---
> > Changed since v1:
> > * psr.c
> > - Function and variables names are changed to express accurately.
> > - Fix code style issues.
> > - Fix imprecise comments.
> > - Add one callback function, get_cos_num(), to fulfill the
> > abstraction requirement.
> > - Replace custom list management to system.
> > - Use 'const' to make codes more safe.
> > ---
> > xen/arch/x86/domctl.c | 13 +++
> > xen/arch/x86/psr.c | 216
> > ++++++++++++++++++++++++++++++++++++++++
> > xen/arch/x86/sysctl.c | 13 +++
> > xen/include/asm-x86/msr-index.h | 1 +
> > xen/include/asm-x86/psr.h | 2 +
> > xen/include/public/domctl.h | 2 +
> > xen/include/public/sysctl.h | 6 ++
> > 7 files changed, 253 insertions(+)
> >
> > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> > index c53d819..24f85c7 100644
> > --- a/xen/arch/x86/domctl.c
> > +++ b/xen/arch/x86/domctl.c
> > @@ -1424,6 +1424,19 @@ long arch_do_domctl(
> > copyback = 1;
> > break;
> >
> > + case XEN_DOMCTL_PSR_CAT_OP_SET_L2_CBM:
> > + ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> > + domctl->u.psr_cat_op.data,
> > + PSR_MASK_TYPE_L2_CBM);
> > + break;
> > +
> > + case XEN_DOMCTL_PSR_CAT_OP_GET_L2_CBM:
> > + ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> > + &domctl->u.psr_cat_op.data,
> > + PSR_MASK_TYPE_L2_CBM);
> > + copyback = 1;
> > + break;
> > +
> > default:
> > ret = -EOPNOTSUPP;
> > break;
> > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> > index 99e4c78..5000a3c 100644
> > --- a/xen/arch/x86/psr.c
> > +++ b/xen/arch/x86/psr.c
> > @@ -137,6 +137,7 @@ struct psr_cat_lvl_info {
> > struct feat_info {
> > union {
> > struct psr_cat_lvl_info l3_info;
> > + struct psr_cat_lvl_info l2_info;
> > };
> > };
> >
> > @@ -158,12 +159,15 @@ struct psr_ref {
> > unsigned int ref;
> > };
> >
> > +
> > #define PSR_SOCKET_L3_CAT 0
> > #define PSR_SOCKET_L3_CDP 1
> > +#define PSR_SOCKET_L2_CAT 2
> >
> > struct psr_socket_info {
> > /*
> > * bit 1~0: [01]->L3 CAT-only, [10]->L3 CDP
> > + * bit 2: L2 CAT
> > */
> > unsigned int features;
> > unsigned int nr_feat;
> > @@ -190,6 +194,7 @@ static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
> > static struct psr_ref *temp_cos_ref;
> > /* Every feature has its own object. */
> > static struct feat_list *feat_l3;
> > +static struct feat_list *feat_l2;
> >
> > /* Common functions for supporting feature callback functions. */
> > static void free_feature(struct psr_socket_info *info)
> > @@ -212,6 +217,12 @@ static void free_feature(struct psr_socket_info *info)
> > xfree(feat_l3);
> > feat_l3 = NULL;
> > }
> > +
> > + if ( feat_l2 )
> > + {
> > + xfree(feat_l2);
> > + feat_l2 = NULL;
> > + }
> > }
> >
> > static bool psr_check_cbm(unsigned int cbm_len, uint64_t cbm)
> > @@ -241,6 +252,195 @@ static bool psr_check_cbm(unsigned int cbm_len,
> > uint64_t cbm)
> > * Features specific implementations.
> > */
> >
> > +/* L2 CAT callback functions implementation. */
> > +static void l2_cat_init_feature(unsigned int eax, unsigned int ebx,
> > + unsigned int ecx, unsigned int edx,
> > + struct feat_list *feat,
> > + struct psr_socket_info *info)
> > +{
> > + struct psr_cat_lvl_info l2_cat;
> > + unsigned int socket;
> > +
> > + /* No valid value so do not enable feature. */
>
> s/value/values/
> s/feature/the feature/
Thanks!
> > + if ( 0 == eax || 0 == edx )
> > + return;
> > +
> > + l2_cat.cbm_len = (eax & 0x1f) + 1;
> > + l2_cat.cos_max = min(opt_cos_max, edx & 0xffff);
>
> Hm, these 0x1f and 0xffff look same as L3. Perhaps make this a #define.
>
Ok, thanks!
> > +
> > + /* 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;
> > + __set_bit(PSR_SOCKET_L2_CAT, &info->features);
> > +
> > + feat->info.l2_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());
> > + printk(XENLOG_INFO "L2 CAT: enabled on socket %u, cos_max:%u,
> > cbm_len:%u.\n",
> > + socket, feat->info.l2_info.cos_max, feat->info.l2_info.cbm_len);
> > +}
> > +
> > +static int l2_cat_compare_val(const uint64_t val[],
> > + const struct feat_list *feat,
> > + unsigned int cos, bool *found)
> > +{
> > + uint64_t l2_def_cbm;
> > +
> > + l2_def_cbm = (1ull << feat->info.l2_info.cbm_len) - 1;
> > +
> > + /* L2 CAT */
> > + if ( cos > feat->info.l2_info.cos_max )
> > + {
> > + if ( val[0] != l2_def_cbm )
> > + {
> > + *found = false;
> > + return -ENOENT;
> > + }
> > + *found = true;
> > + }
> > + else
> > + *found = (val[0] == feat->cos_reg_val[cos]);
> > +
> > + /* L2 CAT occupies one COS. */
> > + return 1;
> > +}
> > +
> > +static unsigned int l2_cat_get_cos_max_from_type(const struct feat_list
> > *feat,
> > + enum psr_val_type type)
> > +{
> > + if ( type != PSR_MASK_TYPE_L2_CBM )
> > + return 0;
> > +
> > + return feat->info.l2_info.cos_max;
> > +}
> > +
> > +static unsigned int l2_cat_exceeds_cos_max(const uint64_t val[],
> > + const struct feat_list *feat,
> > + unsigned int cos)
> > +{
> > + uint64_t l2_def_cbm;
> > +
> > + l2_def_cbm = (1ull << feat->info.l2_info.cbm_len) - 1;
> > +
> > + /* L2 CAT */
> > + if ( cos > feat->info.l3_info.cos_max &&
> > + val[0] != l2_def_cbm )
> > + /*
> > + * Exceed cos_max and value to set is not default,
> > + * return error.
> > + */
> > + return 0;
> > +
> > + /* L2 CAT occupies one COS. */
> > + return 1;
> > +}
> > +
> > +static int l2_cat_write_msr(unsigned int cos, const uint64_t val[],
> > + struct feat_list *feat)
> > +{
> > + /* L2 CAT */
> > + if ( cos > feat->info.l2_info.cos_max )
> > + return 1;
> > +
> > + feat->cos_reg_val[cos] = val[0];
> > + wrmsrl(MSR_IA32_PSR_L2_MASK(cos), val[0]);
> > +
> > + /* L2 CAT occupies one COS. */
> > + return 1;
> > +}
> > +
> > +static unsigned int l2_cat_get_cos_num(const struct feat_list *feat)
> > +{
> > + /* L2 CAT occupies one COS. */
> > + return 1;
> > +}
> > +
> > +static int l2_cat_get_old_val(uint64_t val[],
> > + const struct feat_list *feat,
> > + unsigned int old_cos,
> > + enum psr_val_type type)
> > +{
> > + if ( old_cos > feat->info.l2_info.cos_max )
> > + /* Use default value. */
> > + old_cos = 0;
> > +
> > + val[0] = feat->cos_reg_val[old_cos];
>
> Extra space.
>
Oh, sorry. Thanks!
> > +
> > + /* L2 CAT occupies one COS. */
> > + return 1;
> > +}
> > +
> > +static int l2_cat_set_new_val(uint64_t val[],
> > + const struct feat_list *feat,
> > + unsigned int old_cos,
> > + enum psr_val_type type,
> > + uint64_t m)
> > +{
> > + if ( type == PSR_MASK_TYPE_L2_CBM )
> > + {
> > + if ( !psr_check_cbm(feat->info.l2_info.cbm_len, m) )
> > + return -EINVAL;
> > +
> > + val[0] = m;
> > + }
> > +
> > + /* L2 CAT occupies one COS. */
> > + return 1;
> > +}
> > +
> > +static int l2_cat_get_val(const struct feat_list *feat, unsigned int cos,
> > + enum psr_val_type type, uint64_t *val)
> > +{
> > + if ( type != PSR_MASK_TYPE_L2_CBM )
> > + return 0;
> > +
> > + if ( cos > feat->info.l3_info.cos_max )
> > + cos = 0;
> > +
> > + /* L2 CAT */
> > + *val = feat->cos_reg_val[cos];
> > +
> > + return 1;
> > +}
> > +
> > +static int l2_cat_get_feat_info(const struct feat_list *feat,
> > + enum psr_val_type type,
> > + uint32_t dat[], uint32_t array_len)
> > +{
> > + if ( type != PSR_MASK_TYPE_L2_CBM || !dat || 2 > array_len)
> > + return 0;
> > +
> > + dat[0] = feat->info.l2_info.cbm_len;
> > + dat[1] = feat->info.l2_info.cos_max;
> > +
> > + return 1;
> > +}
> > +
> > +static unsigned int l2_cat_get_max_cos_max(const struct feat_list *feat)
> > +{
> > + return feat->info.l2_info.cos_max;
> > +}
> > +
> > +struct feat_ops l2_cat_ops = {
> > + .init_feature = l2_cat_init_feature,
> > + .get_cos_num = l2_cat_get_cos_num,
> > + .get_old_val = l2_cat_get_old_val,
> > + .set_new_val = l2_cat_set_new_val,
> > + .compare_val = l2_cat_compare_val,
> > + .get_cos_max_from_type = l2_cat_get_cos_max_from_type,
> > + .exceeds_cos_max = l2_cat_exceeds_cos_max,
> > + .write_msr = l2_cat_write_msr,
> > + .get_val = l2_cat_get_val,
> > + .get_feat_info = l2_cat_get_feat_info,
> > + .get_max_cos_max = l2_cat_get_max_cos_max,
> > +};
> > +
> > /* L3 CAT/CDP callback functions implementation. */
> > static void l3_cat_init_feature(unsigned int eax, unsigned int ebx,
> > unsigned int ecx, unsigned int edx,
> > @@ -1220,6 +1420,10 @@ static int cpu_prepare_work(unsigned int cpu)
> > (feat_l3 = xzalloc(struct feat_list)) == NULL )
> > return -ENOMEM;
> >
> > + if ( feat_l2 == NULL &&
> > + (feat_l2 = xzalloc(struct feat_list)) == NULL )
>
> Hmm, and leak feat_l3 and the other one?
>
Sorry, will free them. Thanks!
> > + return -ENOMEM;
> > +
> > return 0;
> > }
> >
> > @@ -1255,6 +1459,18 @@ static void cpu_init_work(void)
> > feat_tmp->ops = l3_cat_ops;
> > feat_tmp->ops.init_feature(eax, ebx, ecx, edx, feat_tmp, info);
> > }
> > +
> > + cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
> > + if ( ebx & PSR_RESOURCE_TYPE_L2 )
> > + {
> > + feat_tmp = feat_l2;
> > + feat_l2 = NULL;
> > +
> > + /* Initialize L2 CAT according to CPUID. */
> > + cpuid_count(PSR_CPUID_LEVEL_CAT, 2, &eax, &ebx, &ecx, &edx);
> > + feat_tmp->ops = l2_cat_ops;
> > + feat_tmp->ops.init_feature(eax, ebx, ecx, edx, feat_tmp, info);
> > + }
> > }
> >
> > static void cpu_fini_work(unsigned int cpu)
> > diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> > index 8e17a9a..6bbe994 100644
> > --- a/xen/arch/x86/sysctl.c
> > +++ b/xen/arch/x86/sysctl.c
> > @@ -189,6 +189,19 @@ long arch_do_sysctl(
> > ret = -EFAULT;
> > break;
> > }
> > + case XEN_SYSCTL_PSR_CAT_get_l2_info:
> > + {
> > + uint32_t dat[2];
> > + ret = psr_get_info(sysctl->u.psr_cat_op.target,
> > + PSR_MASK_TYPE_L2_CBM,
> > + dat, 2);
> > + sysctl->u.psr_cat_op.u.l2_info.cbm_len = dat[0];
>
> Would it make sense to have 0 and 1 be an #define?
>
> That way here and in l2_cat_get_feat_info you can just
> reference by:
>
> dat[COS_MAX] = ..
>
> And then don't have to worry of accidently swapping the values.
>
Good suggestion. Thank you!
> > + sysctl->u.psr_cat_op.u.l2_info.cos_max = dat[1];
> > +
> > + if ( !ret && __copy_field_to_guest(u_sysctl, sysctl,
> > u.psr_cat_op) )
> > + ret = -EFAULT;
> > + break;
> > + }
> > default:
> > ret = -EOPNOTSUPP;
> > break;
> > diff --git a/xen/include/asm-x86/msr-index.h
> > b/xen/include/asm-x86/msr-index.h
> > index deb82a7..99202f3 100644
> > --- a/xen/include/asm-x86/msr-index.h
> > +++ b/xen/include/asm-x86/msr-index.h
> > @@ -342,6 +342,7 @@
> > #define MSR_IA32_PSR_L3_MASK(n) (0x00000c90 + (n))
> > #define MSR_IA32_PSR_L3_MASK_CODE(n) (0x00000c90 + (n) * 2 + 1)
> > #define MSR_IA32_PSR_L3_MASK_DATA(n) (0x00000c90 + (n) * 2)
> > +#define MSR_IA32_PSR_L2_MASK(n) (0x00000d10 + (n))
>
> Something is off here? Tabs?
The old codes uses tabs so I follow this style.
> >
> > /* Intel Model 6 */
> > #define MSR_P6_PERFCTR(n) (0x000000c1 + (n))
> > diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
> > index ef46d12..101a76a 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
> > @@ -50,6 +51,7 @@ enum psr_val_type {
> > PSR_MASK_TYPE_L3_CBM,
> > PSR_MASK_TYPE_L3_CODE,
> > PSR_MASK_TYPE_L3_DATA,
> > + PSR_MASK_TYPE_L2_CBM,
> > };
> >
> > extern struct psr_cmt *psr_cmt;
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index ddd3de4..8a30299 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -1136,6 +1136,8 @@ struct xen_domctl_psr_cat_op {
> > #define XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA 3
> > #define XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE 4
> > #define XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA 5
> > +#define XEN_DOMCTL_PSR_CAT_OP_SET_L2_CBM 6
> > +#define XEN_DOMCTL_PSR_CAT_OP_GET_L2_CBM 7
> > uint32_t cmd; /* IN: XEN_DOMCTL_PSR_CAT_OP_* */
> > uint32_t target; /* IN */
> > uint64_t data; /* IN/OUT */
> > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> > index 8197c14..50ff61c 100644
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -734,6 +734,7 @@ typedef struct xen_sysctl_pcitopoinfo
> > xen_sysctl_pcitopoinfo_t;
> > DEFINE_XEN_GUEST_HANDLE(xen_sysctl_pcitopoinfo_t);
> >
> > #define XEN_SYSCTL_PSR_CAT_get_l3_info 0
> > +#define XEN_SYSCTL_PSR_CAT_get_l2_info 1
> > struct xen_sysctl_psr_cat_op {
> > uint32_t cmd; /* IN: XEN_SYSCTL_PSR_CAT_* */
> > uint32_t target; /* IN */
> > @@ -744,6 +745,11 @@ struct xen_sysctl_psr_cat_op {
> > #define XEN_SYSCTL_PSR_CAT_L3_CDP (1u << 0)
> > uint32_t flags; /* OUT: CAT flags */
> > } l3_info;
> > +
>
> Extra space?
>
This is an extra line to separate l3_info and l2_info.
> > + struct {
> > + uint32_t cbm_len; /* OUT: CBM length */
> > + uint32_t cos_max; /* OUT: Maximum COS */
> > + } l2_info;
> > } u;
> > };
> > typedef struct xen_sysctl_psr_cat_op xen_sysctl_psr_cat_op_t;
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |