From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Yi Sun <yi.y.sun@linux.intel.com>
Cc: wei.liu2@citrix.com, he.chen@linux.intel.com,
andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com,
jbeulich@suse.com, chao.p.peng@linux.intel.com,
xen-devel@lists.xenproject.org
Subject: Re: [v2 2/3] x86: add support for L2 CAT in hypervisor.
Date: Fri, 30 Sep 2016 17:23:43 -0400 [thread overview]
Message-ID: <20160930212343.GD5382@char.us.oracle.com> (raw)
In-Reply-To: <1474510544-29571-1-git-send-email-yi.y.sun@linux.intel.com>
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@linux.intel.com>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>
> ---
> 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/
> + 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.
> +
> + /* 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.
> +
> + /* 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?
> + 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.
> + 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?
>
> /* 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?
> + 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@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-09-30 21:24 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-22 2:15 [v2 2/3] x86: add support for L2 CAT in hypervisor Yi Sun
2016-09-30 21:23 ` Konrad Rzeszutek Wilk [this message]
2016-10-09 6:59 ` Yi Sun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160930212343.GD5382@char.us.oracle.com \
--to=konrad.wilk@oracle.com \
--cc=andrew.cooper3@citrix.com \
--cc=chao.p.peng@linux.intel.com \
--cc=he.chen@linux.intel.com \
--cc=ian.jackson@eu.citrix.com \
--cc=jbeulich@suse.com \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xenproject.org \
--cc=yi.y.sun@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.