* [PATCH v2 1/4] x86: Support enable CDP by boot parameter and add get CDP status
2015-09-09 5:16 [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling He Chen
@ 2015-09-09 5:16 ` He Chen
2015-09-09 7:04 ` Chao Peng
2015-09-09 5:16 ` [PATCH v2 2/4] x86: add domctl cmd to set/get CDP code/data CBM He Chen
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: He Chen @ 2015-09-09 5:16 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, chao.p.peng, keir
Intel Code/Data Prioritization(CDP) feature is based on CAT. cdp_enabled
is added to CAT socket info to indicate CDP is on or off on the socket,
note that cos_max would be half when CDP is on. struct psr_cat_cbm is
extended to support CDP operation. Extend psr_get_cat_l3_info sysctl to
get CDP status.
Signed-off-by: He Chen <he.chen@linux.intel.com>
---
xen/arch/x86/psr.c | 84 ++++++++++++++++++++++++++++++++++-------
xen/arch/x86/sysctl.c | 5 ++-
xen/include/asm-x86/msr-index.h | 3 ++
xen/include/asm-x86/psr.h | 11 +++++-
xen/include/public/sysctl.h | 2 +
5 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index c0daa2e..ba0f726 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -17,13 +17,21 @@
#include <xen/cpu.h>
#include <xen/err.h>
#include <xen/sched.h>
+#include <xen/domain.h>
#include <asm/psr.h>
#define PSR_CMT (1<<0)
#define PSR_CAT (1<<1)
+#define PSR_CDP (1<<2)
struct psr_cat_cbm {
- uint64_t cbm;
+ union {
+ uint64_t cbm;
+ struct {
+ uint64_t code;
+ uint64_t data;
+ } cdp;
+ } u;
unsigned int ref;
};
@@ -32,6 +40,7 @@ struct psr_cat_socket_info {
unsigned int cos_max;
struct psr_cat_cbm *cos_to_cbm;
spinlock_t cbm_lock;
+ bool_t cdp_enabled;
};
struct psr_assoc {
@@ -43,6 +52,7 @@ struct psr_cmt *__read_mostly psr_cmt;
static unsigned long *__read_mostly cat_socket_enable;
static struct psr_cat_socket_info *__read_mostly cat_socket_info;
+static unsigned long *__read_mostly cdp_socket_enable;
static unsigned int __initdata opt_psr;
static unsigned int __initdata opt_rmid_max = 255;
@@ -94,6 +104,7 @@ static void __init parse_psr_param(char *s)
parse_psr_bool(s, val_str, "cmt", PSR_CMT);
parse_psr_bool(s, val_str, "cat", PSR_CAT);
+ parse_psr_bool(s, val_str, "cdp", PSR_CDP);
if ( val_str && !strcmp(s, "rmid_max") )
opt_rmid_max = simple_strtoul(val_str, NULL, 0);
@@ -262,7 +273,7 @@ static struct psr_cat_socket_info *get_cat_socket_info(unsigned int socket)
}
int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
- uint32_t *cos_max)
+ uint32_t *cos_max, uint32_t *flags)
{
struct psr_cat_socket_info *info = get_cat_socket_info(socket);
@@ -272,6 +283,10 @@ int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
*cbm_len = info->cbm_len;
*cos_max = info->cos_max;
+ *flags = 0;
+ if ( info->cdp_enabled )
+ *flags |= PSR_CAT_FLAG_L3_CDP;
+
return 0;
}
@@ -282,7 +297,7 @@ int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
if ( IS_ERR(info) )
return PTR_ERR(info);
- *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].cbm;
+ *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
return 0;
}
@@ -313,19 +328,34 @@ static bool_t psr_check_cbm(unsigned int cbm_len, uint64_t cbm)
struct cos_cbm_info
{
unsigned int cos;
- uint64_t cbm;
+ uint64_t cbm_code;
+ uint64_t cbm_data;
+ bool_t cdp;
};
static void do_write_l3_cbm(void *data)
{
struct cos_cbm_info *info = data;
- wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos), info->cbm);
+ if ( info->cdp )
+ {
+ wrmsrl(MSR_IA32_PSR_L3_MASK_CBM_CODE(info->cos), info->cbm_code);
+ wrmsrl(MSR_IA32_PSR_L3_MASK_CBM_DATA(info->cos), info->cbm_data);
+ }
+ else
+ wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos), info->cbm_code);
}
-static int write_l3_cbm(unsigned int socket, unsigned int cos, uint64_t cbm)
+static int write_l3_cbm(unsigned int socket, unsigned int cos,
+ uint64_t cbm_code, uint64_t cbm_data, bool_t cdp)
{
- struct cos_cbm_info info = { .cos = cos, .cbm = cbm };
+ struct cos_cbm_info info =
+ {
+ .cos = cos,
+ .cbm_code = cbm_code,
+ .cbm_data = cbm_data,
+ .cdp = cdp,
+ };
if ( socket == cpu_to_socket(smp_processor_id()) )
do_write_l3_cbm(&info);
@@ -363,7 +393,7 @@ int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
/* If still not found, then keep unused one. */
if ( !found && cos != 0 && map[cos].ref == 0 )
found = map + cos;
- else if ( map[cos].cbm == cbm )
+ else if ( map[cos].u.cbm == cbm )
{
if ( unlikely(cos == old_cos) )
{
@@ -387,16 +417,16 @@ int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
}
cos = found - map;
- if ( found->cbm != cbm )
+ if ( found->u.cbm != cbm )
{
- int ret = write_l3_cbm(socket, cos, cbm);
+ int ret = write_l3_cbm(socket, cos, cbm, 0, 0);
if ( ret )
{
spin_unlock(&info->cbm_lock);
return ret;
}
- found->cbm = cbm;
+ found->u.cbm = cbm;
}
found->ref++;
@@ -470,6 +500,7 @@ static void cat_cpu_init(void)
struct psr_cat_socket_info *info;
unsigned int socket;
unsigned int cpu = smp_processor_id();
+ uint64_t val;
const struct cpuinfo_x86 *c = cpu_data + cpu;
if ( !cpu_has(c, X86_FEATURE_CAT) || c->cpuid_level < PSR_CPUID_LEVEL_CAT )
@@ -490,13 +521,34 @@ static void cat_cpu_init(void)
info->cos_to_cbm = temp_cos_to_cbm;
temp_cos_to_cbm = NULL;
/* cos=0 is reserved as default cbm(all ones). */
- info->cos_to_cbm[0].cbm = (1ull << info->cbm_len) - 1;
+ info->cos_to_cbm[0].u.cbm = (1ull << info->cbm_len) - 1;
spin_lock_init(&info->cbm_lock);
set_bit(socket, cat_socket_enable);
printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
socket, info->cos_max, info->cbm_len);
+
+ if ( (ecx & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
+ {
+ if ( test_bit(socket, cdp_socket_enable) )
+ return;
+
+ rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
+ wrmsrl(MSR_IA32_PSR_L3_QOS_CFG, val | 1 << PSR_L3_QOS_CDP_ENABLE_BIT);
+
+ /* No need to write register since CBMs are fully open as default by HW */
+ info->cos_to_cbm[0].u.cdp.code = (1ull << info->cbm_len) - 1;
+ info->cos_to_cbm[0].u.cdp.data = (1ull << info->cbm_len) - 1;
+
+ /* Cut half of cos_max when CDP enabled */
+ info->cos_max = info->cos_max / 2;
+
+ info->cdp_enabled = 1;
+ set_bit(socket, cdp_socket_enable);
+ printk(XENLOG_INFO "CDP: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
+ socket, info->cos_max, info->cbm_len);
+ }
}
}
@@ -508,6 +560,9 @@ static void cat_cpu_fini(unsigned int cpu)
{
struct psr_cat_socket_info *info = cat_socket_info + socket;
+ info->cdp_enabled = 0;
+ clear_bit(socket, cdp_socket_enable);
+
if ( info->cos_to_cbm )
{
xfree(info->cos_to_cbm);
@@ -523,6 +578,8 @@ static void __init psr_cat_free(void)
cat_socket_enable = NULL;
xfree(cat_socket_info);
cat_socket_info = NULL;
+ xfree(cdp_socket_enable);
+ cdp_socket_enable = NULL;
}
static void __init init_psr_cat(void)
@@ -535,8 +592,9 @@ static void __init init_psr_cat(void)
cat_socket_enable = xzalloc_array(unsigned long, BITS_TO_LONGS(nr_sockets));
cat_socket_info = xzalloc_array(struct psr_cat_socket_info, nr_sockets);
+ cdp_socket_enable = xzalloc_array(unsigned long, BITS_TO_LONGS(nr_sockets));
- if ( !cat_socket_enable || !cat_socket_info )
+ if ( !cat_socket_enable || !cat_socket_info || !cdp_socket_enable )
psr_cat_free();
}
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index f36b52f..e42c5e7 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -177,12 +177,13 @@ long arch_do_sysctl(
case XEN_SYSCTL_PSR_CAT_get_l3_info:
ret = psr_get_cat_l3_info(sysctl->u.psr_cat_op.target,
&sysctl->u.psr_cat_op.u.l3_info.cbm_len,
- &sysctl->u.psr_cat_op.u.l3_info.cos_max);
+ &sysctl->u.psr_cat_op.u.l3_info.cos_max,
+ &sysctl->u.psr_cat_op.u.l3_info.flags);
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 e9c4723..402e7a7 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -328,7 +328,10 @@
#define MSR_IA32_CMT_EVTSEL 0x00000c8d
#define MSR_IA32_CMT_CTR 0x00000c8e
#define MSR_IA32_PSR_ASSOC 0x00000c8f
+#define MSR_IA32_PSR_L3_QOS_CFG 0x00000c81
#define MSR_IA32_PSR_L3_MASK(n) (0x00000c90 + (n))
+#define MSR_IA32_PSR_L3_MASK_CBM_CODE(n) (0x00000c90 + (n * 2 + 1))
+#define MSR_IA32_PSR_L3_MASK_CBM_DATA(n) (0x00000c90 + (n * 2))
/* 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 081750f..422b053 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -27,6 +27,15 @@
/* L3 Monitoring Features */
#define PSR_CMT_L3_OCCUPANCY 0x1
+/* CDP Capability */
+#define PSR_CAT_CDP_CAPABILITY (1u << 2)
+
+/* L3 CDP Enable bit*/
+#define PSR_L3_QOS_CDP_ENABLE_BIT 0x0
+
+/* L3 CDP flag bit */
+#define PSR_CAT_FLAG_L3_CDP (1u << 0)
+
struct psr_cmt_l3 {
unsigned int features;
unsigned int upscaling_factor;
@@ -52,7 +61,7 @@ void psr_free_rmid(struct domain *d);
void psr_ctxt_switch_to(struct domain *d);
int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
- uint32_t *cos_max);
+ uint32_t *cos_max, uint32_t *flags);
int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm);
int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm);
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 58c9be2..4102cd2 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -704,6 +704,8 @@ struct xen_sysctl_psr_cat_op {
struct {
uint32_t cbm_len; /* OUT: CBM length */
uint32_t cos_max; /* OUT: Maximum COS */
+ #define XEN_SYSCTL_PSR_CAT_L3_CDP (1u << 0)
+ uint32_t flags; /* OUT: CAT flags */
} l3_info;
} u;
};
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/4] x86: Support enable CDP by boot parameter and add get CDP status
2015-09-09 5:16 ` [PATCH v2 1/4] x86: Support enable CDP by boot parameter and add get CDP status He Chen
@ 2015-09-09 7:04 ` Chao Peng
2015-09-09 7:44 ` He Chen
0 siblings, 1 reply; 15+ messages in thread
From: Chao Peng @ 2015-09-09 7:04 UTC (permalink / raw)
To: He Chen
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
ian.jackson, jbeulich, xen-devel, keir
On Wed, Sep 09, 2015 at 01:16:45PM +0800, He Chen wrote:
> Intel Code/Data Prioritization(CDP) feature is based on CAT. cdp_enabled
> is added to CAT socket info to indicate CDP is on or off on the socket,
> note that cos_max would be half when CDP is on. struct psr_cat_cbm is
> extended to support CDP operation. Extend psr_get_cat_l3_info sysctl to
> get CDP status.
>
> Signed-off-by: He Chen <he.chen@linux.intel.com>
> ---
> xen/arch/x86/psr.c | 84 ++++++++++++++++++++++++++++++++++-------
> xen/arch/x86/sysctl.c | 5 ++-
> xen/include/asm-x86/msr-index.h | 3 ++
> xen/include/asm-x86/psr.h | 11 +++++-
> xen/include/public/sysctl.h | 2 +
> 5 files changed, 89 insertions(+), 16 deletions(-)
>
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index c0daa2e..ba0f726 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -17,13 +17,21 @@
> #include <xen/cpu.h>
> #include <xen/err.h>
> #include <xen/sched.h>
> +#include <xen/domain.h>
Is this still needed?
> #include <asm/psr.h>
>
> #define PSR_CMT (1<<0)
> #define PSR_CAT (1<<1)
> +#define PSR_CDP (1<<2)
>
> struct psr_cat_cbm {
> - uint64_t cbm;
> + union {
> + uint64_t cbm;
> + struct {
> + uint64_t code;
> + uint64_t data;
> + } cdp;
> + } u;
> unsigned int ref;
> };
>
> @@ -32,6 +40,7 @@ struct psr_cat_socket_info {
> unsigned int cos_max;
> struct psr_cat_cbm *cos_to_cbm;
> spinlock_t cbm_lock;
As you defined the cdp stauts for each socket here ...
> + bool_t cdp_enabled;
> };
>
> struct psr_assoc {
> @@ -43,6 +52,7 @@ struct psr_cmt *__read_mostly psr_cmt;
>
> static unsigned long *__read_mostly cat_socket_enable;
> static struct psr_cat_socket_info *__read_mostly cat_socket_info;
> +static unsigned long *__read_mostly cdp_socket_enable;
... this one then perhaps is redundency. If only one is need then I really
like to keep the latter one.
> @@ -470,6 +500,7 @@ static void cat_cpu_init(void)
> struct psr_cat_socket_info *info;
> unsigned int socket;
> unsigned int cpu = smp_processor_id();
> + uint64_t val;
> const struct cpuinfo_x86 *c = cpu_data + cpu;
>
> if ( !cpu_has(c, X86_FEATURE_CAT) || c->cpuid_level < PSR_CPUID_LEVEL_CAT )
> @@ -490,13 +521,34 @@ static void cat_cpu_init(void)
> info->cos_to_cbm = temp_cos_to_cbm;
> temp_cos_to_cbm = NULL;
> /* cos=0 is reserved as default cbm(all ones). */
> - info->cos_to_cbm[0].cbm = (1ull << info->cbm_len) - 1;
> + info->cos_to_cbm[0].u.cbm = (1ull << info->cbm_len) - 1;
>
> spin_lock_init(&info->cbm_lock);
>
> set_bit(socket, cat_socket_enable);
> printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> socket, info->cos_max, info->cbm_len);
If CDP is enable below, then this information is quite misleading.
> +
> + if ( (ecx & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
> + {
> + if ( test_bit(socket, cdp_socket_enable) )
> + return;
> +
> + rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
> + wrmsrl(MSR_IA32_PSR_L3_QOS_CFG, val | 1 << PSR_L3_QOS_CDP_ENABLE_BIT);
> +
> + /* No need to write register since CBMs are fully open as default by HW */
> + info->cos_to_cbm[0].u.cdp.code = (1ull << info->cbm_len) - 1;
> + info->cos_to_cbm[0].u.cdp.data = (1ull << info->cbm_len) - 1;
If I remember correctly, HW is supposed to boot as CAT mode by default and
only IA32_L3_QOS_Mask_0 is all ones, other mask msrs however are not mentioned
in the spec which then may contain arbitrary value. So I guesss as
least you need write all ones to IA32_L3_QOS_Mask_1 explicitly and that
should be done before you enabled CDP.
> +
> + /* Cut half of cos_max when CDP enabled */
> + info->cos_max = info->cos_max / 2;
> +
> + info->cdp_enabled = 1;
> + set_bit(socket, cdp_socket_enable);
> + printk(XENLOG_INFO "CDP: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> + socket, info->cos_max, info->cbm_len);
> + }
> }
> }
>
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index e9c4723..402e7a7 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -328,7 +328,10 @@
> #define MSR_IA32_CMT_EVTSEL 0x00000c8d
> #define MSR_IA32_CMT_CTR 0x00000c8e
> #define MSR_IA32_PSR_ASSOC 0x00000c8f
> +#define MSR_IA32_PSR_L3_QOS_CFG 0x00000c81
> #define MSR_IA32_PSR_L3_MASK(n) (0x00000c90 + (n))
> +#define MSR_IA32_PSR_L3_MASK_CBM_CODE(n) (0x00000c90 + (n * 2 + 1))
> +#define MSR_IA32_PSR_L3_MASK_CBM_DATA(n) (0x00000c90 + (n * 2))
I guess 'CBM' can be removed from these two macros, e.g.
MSR_IA32_PSR_L3_MASK_CODE & MSR_IA32_PSR_L3_MASK_DATA
Chao
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/4] x86: Support enable CDP by boot parameter and add get CDP status
2015-09-09 7:04 ` Chao Peng
@ 2015-09-09 7:44 ` He Chen
0 siblings, 0 replies; 15+ messages in thread
From: He Chen @ 2015-09-09 7:44 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
ian.jackson, jbeulich, xen-devel, keir
On Wed, Sep 09, 2015 at 03:04:35PM +0800, Chao Peng wrote:
> On Wed, Sep 09, 2015 at 01:16:45PM +0800, He Chen wrote:
> > Intel Code/Data Prioritization(CDP) feature is based on CAT. cdp_enabled
> > is added to CAT socket info to indicate CDP is on or off on the socket,
> > note that cos_max would be half when CDP is on. struct psr_cat_cbm is
> > extended to support CDP operation. Extend psr_get_cat_l3_info sysctl to
> > get CDP status.
> >
> > Signed-off-by: He Chen <he.chen@linux.intel.com>
> > ---
> > xen/arch/x86/psr.c | 84 ++++++++++++++++++++++++++++++++++-------
> > xen/arch/x86/sysctl.c | 5 ++-
> > xen/include/asm-x86/msr-index.h | 3 ++
> > xen/include/asm-x86/psr.h | 11 +++++-
> > xen/include/public/sysctl.h | 2 +
> > 5 files changed, 89 insertions(+), 16 deletions(-)
> >
> > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> > index c0daa2e..ba0f726 100644
> > --- a/xen/arch/x86/psr.c
> > +++ b/xen/arch/x86/psr.c
> > @@ -17,13 +17,21 @@
> > #include <xen/cpu.h>
> > #include <xen/err.h>
> > #include <xen/sched.h>
> > +#include <xen/domain.h>
>
> Is this still needed?
>
Obviously not, my mistake.
> > #include <asm/psr.h>
> >
> > #define PSR_CMT (1<<0)
> > #define PSR_CAT (1<<1)
> > +#define PSR_CDP (1<<2)
> >
> > struct psr_cat_cbm {
> > - uint64_t cbm;
> > + union {
> > + uint64_t cbm;
> > + struct {
> > + uint64_t code;
> > + uint64_t data;
> > + } cdp;
> > + } u;
> > unsigned int ref;
> > };
> >
> > @@ -32,6 +40,7 @@ struct psr_cat_socket_info {
> > unsigned int cos_max;
> > struct psr_cat_cbm *cos_to_cbm;
> > spinlock_t cbm_lock;
>
> As you defined the cdp stauts for each socket here ...
> > + bool_t cdp_enabled;
> > };
> >
> > struct psr_assoc {
> > @@ -43,6 +52,7 @@ struct psr_cmt *__read_mostly psr_cmt;
> >
> > static unsigned long *__read_mostly cat_socket_enable;
> > static struct psr_cat_socket_info *__read_mostly cat_socket_info;
> > +static unsigned long *__read_mostly cdp_socket_enable;
>
> ... this one then perhaps is redundency. If only one is need then I really
> like to keep the latter one.
>
You're right. I would refine this.
> > @@ -470,6 +500,7 @@ static void cat_cpu_init(void)
> > struct psr_cat_socket_info *info;
> > unsigned int socket;
> > unsigned int cpu = smp_processor_id();
> > + uint64_t val;
> > const struct cpuinfo_x86 *c = cpu_data + cpu;
> >
> > if ( !cpu_has(c, X86_FEATURE_CAT) || c->cpuid_level < PSR_CPUID_LEVEL_CAT )
> > @@ -490,13 +521,34 @@ static void cat_cpu_init(void)
> > info->cos_to_cbm = temp_cos_to_cbm;
> > temp_cos_to_cbm = NULL;
> > /* cos=0 is reserved as default cbm(all ones). */
> > - info->cos_to_cbm[0].cbm = (1ull << info->cbm_len) - 1;
> > + info->cos_to_cbm[0].u.cbm = (1ull << info->cbm_len) - 1;
> >
> > spin_lock_init(&info->cbm_lock);
> >
> > set_bit(socket, cat_socket_enable);
> > printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> > socket, info->cos_max, info->cbm_len);
>
> If CDP is enable below, then this information is quite misleading.
>
Is it proper to remove CAT printk info?
> > +
> > + if ( (ecx & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
> > + {
> > + if ( test_bit(socket, cdp_socket_enable) )
> > + return;
> > +
> > + rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
> > + wrmsrl(MSR_IA32_PSR_L3_QOS_CFG, val | 1 << PSR_L3_QOS_CDP_ENABLE_BIT);
> > +
> > + /* No need to write register since CBMs are fully open as default by HW */
> > + info->cos_to_cbm[0].u.cdp.code = (1ull << info->cbm_len) - 1;
> > + info->cos_to_cbm[0].u.cdp.data = (1ull << info->cbm_len) - 1;
>
> If I remember correctly, HW is supposed to boot as CAT mode by default and
> only IA32_L3_QOS_Mask_0 is all ones, other mask msrs however are not mentioned
> in the spec which then may contain arbitrary value. So I guesss as
> least you need write all ones to IA32_L3_QOS_Mask_1 explicitly and that
> should be done before you enabled CDP.
>
I have tested it at Broadwell server. I mean, I have used rdmsr to get mask0
and mask1 during booting and then check them, I found that they all ones by
default.
But since spec doesn't mention it as you said, I would write all ones to mask1
for better compatibility.
> > +
> > + /* Cut half of cos_max when CDP enabled */
> > + info->cos_max = info->cos_max / 2;
> > +
> > + info->cdp_enabled = 1;
> > + set_bit(socket, cdp_socket_enable);
> > + printk(XENLOG_INFO "CDP: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> > + socket, info->cos_max, info->cbm_len);
> > + }
> > }
> > }
> >
> > diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> > index e9c4723..402e7a7 100644
> > --- a/xen/include/asm-x86/msr-index.h
> > +++ b/xen/include/asm-x86/msr-index.h
> > @@ -328,7 +328,10 @@
> > #define MSR_IA32_CMT_EVTSEL 0x00000c8d
> > #define MSR_IA32_CMT_CTR 0x00000c8e
> > #define MSR_IA32_PSR_ASSOC 0x00000c8f
> > +#define MSR_IA32_PSR_L3_QOS_CFG 0x00000c81
> > #define MSR_IA32_PSR_L3_MASK(n) (0x00000c90 + (n))
> > +#define MSR_IA32_PSR_L3_MASK_CBM_CODE(n) (0x00000c90 + (n * 2 + 1))
> > +#define MSR_IA32_PSR_L3_MASK_CBM_DATA(n) (0x00000c90 + (n * 2))
>
> I guess 'CBM' can be removed from these two macros, e.g.
> MSR_IA32_PSR_L3_MASK_CODE & MSR_IA32_PSR_L3_MASK_DATA
>
> Chao
Sure and thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/4] x86: add domctl cmd to set/get CDP code/data CBM
2015-09-09 5:16 [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling He Chen
2015-09-09 5:16 ` [PATCH v2 1/4] x86: Support enable CDP by boot parameter and add get CDP status He Chen
@ 2015-09-09 5:16 ` He Chen
2015-09-09 7:24 ` Chao Peng
2015-09-09 5:16 ` [PATCH v2 3/4] tools: add tools support for Intel CDP He Chen
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: He Chen @ 2015-09-09 5:16 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, chao.p.peng, keir
CDP extends CAT and provides the capacity to control L3 code & data
cache. With CDP, one COS correspond to two CMBs(code & data). cbm_type
is added to support distinguish different CBM operation. Besides, new
domctl cmds are introdunced to support set/get CDP CBM. Some CAT
functions to operation CBMs are extended to support CDP.
Signed-off-by: He Chen <he.chen@linux.intel.com>
---
xen/arch/x86/domctl.c | 32 ++++++++-
xen/arch/x86/psr.c | 163 ++++++++++++++++++++++++++++++++++----------
xen/include/asm-x86/psr.h | 12 +++-
xen/include/public/domctl.h | 4 ++
4 files changed, 170 insertions(+), 41 deletions(-)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index bf62a88..734fddb 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1167,12 +1167,40 @@ long arch_do_domctl(
{
case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
- domctl->u.psr_cat_op.data);
+ domctl->u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3);
+ break;
+
+ case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE:
+ ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
+ domctl->u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3_CODE);
+ break;
+
+ case XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA:
+ ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
+ domctl->u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3_DATA);
break;
case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
- &domctl->u.psr_cat_op.data);
+ &domctl->u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3);
+ copyback = 1;
+ break;
+
+ case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE:
+ ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
+ &domctl->u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3_CODE);
+ copyback = 1;
+ break;
+
+ case XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA:
+ ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
+ &domctl->u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3_DATA);
copyback = 1;
break;
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index ba0f726..c7d8356 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -290,14 +290,32 @@ int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
return 0;
}
-int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
+int psr_get_l3_cbm(struct domain *d, unsigned int socket,
+ uint64_t *cbm, enum cbm_type type)
{
struct psr_cat_socket_info *info = get_cat_socket_info(socket);
if ( IS_ERR(info) )
return PTR_ERR(info);
- *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
+ if ( type == PSR_CBM_TYPE_L3 )
+ {
+ if ( info->cdp_enabled )
+ return -EXDEV;
+ *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
+ }
+ else if ( type == PSR_CBM_TYPE_L3_CODE )
+ {
+ if ( !info->cdp_enabled )
+ return -EXDEV;
+ *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
+ }
+ else
+ {
+ if ( !info->cdp_enabled )
+ return -EXDEV;
+ *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
+ }
return 0;
}
@@ -371,65 +389,136 @@ static int write_l3_cbm(unsigned int socket, unsigned int cos,
return 0;
}
-int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
+static int find_cos(struct psr_cat_cbm *map, int cos_max,
+ uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
{
- unsigned int old_cos, cos;
- struct psr_cat_cbm *map, *found = NULL;
+ unsigned int cos;
+
+ if ( !cdp_enabled )
+ {
+ for ( cos = 0; cos <= cos_max; cos++ )
+ if ( map[cos].ref && map[cos].u.cbm == cbm_code )
+ return cos;
+ }
+ else
+ {
+ for ( cos = 0; cos <= cos_max; cos++ )
+ if ( map[cos].ref && map[cos].u.cdp.code == cbm_code &&
+ map[cos].u.cdp.data == cbm_data )
+ return cos;
+ }
+
+ return -ENOENT;
+}
+
+static int pick_avail_cos(struct psr_cat_cbm *map, int cos_max, int old_cos)
+{
+ int cos;
+
+ /* If old cos is referred only by the domain, then use it. */
+ if ( map[old_cos].ref == 1 )
+ return old_cos;
+
+ /* Then we pick an unused one, never pick 0 */
+ for ( cos = 1; cos <= cos_max; cos++ )
+ if ( map[cos].ref == 0 )
+ return cos;
+
+ return -ENOENT;
+}
+
+int psr_set_l3_cbm(struct domain *d, unsigned int socket,
+ uint64_t cbm, enum cbm_type type)
+{
+ unsigned int old_cos, cos_max;
+ int cos, ret;
+ uint64_t cbm_data, cbm_code;
+ bool_t need_write = 1;
+ struct psr_cat_cbm *map;
struct psr_cat_socket_info *info = get_cat_socket_info(socket);
if ( IS_ERR(info) )
return PTR_ERR(info);
+ cbm_code = (1ull << info->cbm_len) - 1;
+ cbm_data = (1ull << info->cbm_len) - 1;
if ( !psr_check_cbm(info->cbm_len, cbm) )
return -EINVAL;
+ cos_max = info->cos_max;
old_cos = d->arch.psr_cos_ids[socket];
map = info->cos_to_cbm;
- spin_lock(&info->cbm_lock);
-
- for ( cos = 0; cos <= info->cos_max; cos++ )
+ switch( type )
{
- /* If still not found, then keep unused one. */
- if ( !found && cos != 0 && map[cos].ref == 0 )
- found = map + cos;
- else if ( map[cos].u.cbm == cbm )
- {
- if ( unlikely(cos == old_cos) )
- {
- ASSERT(cos == 0 || map[cos].ref != 0);
- spin_unlock(&info->cbm_lock);
- return 0;
- }
- found = map + cos;
+ case PSR_CBM_TYPE_L3:
+ if ( info->cdp_enabled )
+ return -EINVAL;
+ cbm_code = cbm;
+ cbm_data = cbm;
break;
- }
- }
- /* If old cos is referred only by the domain, then use it. */
- if ( !found && map[old_cos].ref == 1 )
- found = map + old_cos;
+ case PSR_CBM_TYPE_L3_CODE:
+ if ( !info->cdp_enabled )
+ return -EINVAL;
+ cbm_code = cbm;
+ cbm_data = map[old_cos].u.cdp.data;
+ break;
- if ( !found )
- {
- spin_unlock(&info->cbm_lock);
- return -EOVERFLOW;
+ case PSR_CBM_TYPE_L3_DATA:
+ if ( !info->cdp_enabled )
+ return -EINVAL;
+ cbm_code = map[old_cos].u.cdp.code;
+ cbm_data = cbm;
+ break;
+
+ default:
+ return -EINVAL;
}
- cos = found - map;
- if ( found->u.cbm != cbm )
+ spin_lock(&info->cbm_lock);
+ cos = find_cos(map, cos_max, cbm_code, cbm_data, info->cdp_enabled);
+ if ( cos >= 0 )
{
- int ret = write_l3_cbm(socket, cos, cbm, 0, 0);
-
- if ( ret )
+ if ( unlikely(cos == old_cos) )
{
spin_unlock(&info->cbm_lock);
- return ret;
+ return 0;
+ }
+ }
+ else
+ {
+ cos = pick_avail_cos(map, cos_max, old_cos);
+ if ( cos < 0 )
+ {
+ spin_unlock(&info->cbm_lock);
+ return cos;
+ }
+
+ /* We try to avoid writing MSR */
+ if ( info->cdp_enabled )
+ {
+ if ( map[cos].u.cdp.code == cbm_code &&
+ map[cos].u.cdp.data == cbm_data )
+ need_write = 0;
+ }
+ else
+ need_write = map[cos].u.cbm == cbm_code ? 0 : 1;
+
+ if ( need_write )
+ {
+ ret = write_l3_cbm(socket, cos, cbm_code, cbm_data, info->cdp_enabled);
+ if ( ret )
+ {
+ spin_unlock(&info->cbm_lock);
+ return ret;
+ }
+ map[cos].u.cdp.code = cbm_code;
+ map[cos].u.cdp.data = cbm_data;
}
- found->u.cbm = cbm;
}
- found->ref++;
+ map[cos].ref++;
map[old_cos].ref--;
spin_unlock(&info->cbm_lock);
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 422b053..c0b3f66 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -49,6 +49,12 @@ struct psr_cmt {
struct psr_cmt_l3 l3;
};
+enum cbm_type {
+ PSR_CBM_TYPE_L3 = 1,
+ PSR_CBM_TYPE_L3_CODE = 2,
+ PSR_CBM_TYPE_L3_DATA = 3,
+};
+
extern struct psr_cmt *psr_cmt;
static inline bool_t psr_cmt_enabled(void)
@@ -62,8 +68,10 @@ void psr_ctxt_switch_to(struct domain *d);
int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
uint32_t *cos_max, uint32_t *flags);
-int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm);
-int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm);
+int psr_get_l3_cbm(struct domain *d, unsigned int socket,
+ uint64_t *cbm, enum cbm_type type);
+int psr_set_l3_cbm(struct domain *d, unsigned int socket,
+ uint64_t cbm, enum cbm_type type);
int psr_domain_init(struct domain *d);
void psr_domain_free(struct domain *d);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 675f021..f438cae 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1056,6 +1056,10 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_monitor_op_t);
struct xen_domctl_psr_cat_op {
#define XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM 0
#define XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM 1
+#define XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE 2
+#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
uint32_t cmd; /* IN: XEN_DOMCTL_PSR_CAT_OP_* */
uint32_t target; /* IN */
uint64_t data; /* IN/OUT */
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/4] x86: add domctl cmd to set/get CDP code/data CBM
2015-09-09 5:16 ` [PATCH v2 2/4] x86: add domctl cmd to set/get CDP code/data CBM He Chen
@ 2015-09-09 7:24 ` Chao Peng
2015-09-09 7:35 ` Jan Beulich
0 siblings, 1 reply; 15+ messages in thread
From: Chao Peng @ 2015-09-09 7:24 UTC (permalink / raw)
To: He Chen
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
ian.jackson, jbeulich, xen-devel, keir
On Wed, Sep 09, 2015 at 01:16:46PM +0800, He Chen wrote:
> CDP extends CAT and provides the capacity to control L3 code & data
> cache. With CDP, one COS correspond to two CMBs(code & data). cbm_type
> is added to support distinguish different CBM operation. Besides, new
> domctl cmds are introdunced to support set/get CDP CBM. Some CAT
> functions to operation CBMs are extended to support CDP.
>
> Signed-off-by: He Chen <he.chen@linux.intel.com>
> ---
> xen/arch/x86/domctl.c | 32 ++++++++-
> xen/arch/x86/psr.c | 163 ++++++++++++++++++++++++++++++++++----------
> xen/include/asm-x86/psr.h | 12 +++-
> xen/include/public/domctl.h | 4 ++
> 4 files changed, 170 insertions(+), 41 deletions(-)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index bf62a88..734fddb 100644
> --- a/xen/arch/x86/domctl.c
>
> -int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
> +int psr_get_l3_cbm(struct domain *d, unsigned int socket,
> + uint64_t *cbm, enum cbm_type type)
> {
> struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>
> if ( IS_ERR(info) )
> return PTR_ERR(info);
>
> - *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
> + if ( type == PSR_CBM_TYPE_L3 )
> + {
> + if ( info->cdp_enabled )
> + return -EXDEV;
> + *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
> + }
> + else if ( type == PSR_CBM_TYPE_L3_CODE )
> + {
> + if ( !info->cdp_enabled )
> + return -EXDEV;
> + *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
> + }
> + else
> + {
> + if ( !info->cdp_enabled )
> + return -EXDEV;
> + *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
> + }
Could I suggest to use switch? And the check can be done in advance.
>
> return 0;
> }
> @@ -371,65 +389,136 @@ static int write_l3_cbm(unsigned int socket, unsigned int cos,
> return 0;
> }
>
> -int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
> +static int find_cos(struct psr_cat_cbm *map, int cos_max,
> + uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
> {
> - unsigned int old_cos, cos;
> - struct psr_cat_cbm *map, *found = NULL;
> + unsigned int cos;
> +
> + if ( !cdp_enabled )
> + {
> + for ( cos = 0; cos <= cos_max; cos++ )
> + if ( map[cos].ref && map[cos].u.cbm == cbm_code )
> + return cos;
> + }
> + else
> + {
> + for ( cos = 0; cos <= cos_max; cos++ )
> + if ( map[cos].ref && map[cos].u.cdp.code == cbm_code &&
> + map[cos].u.cdp.data == cbm_data )
> + return cos;
> + }
I guess the '{' and '}' can be omitted if the body contains only one
sentence.
> +
> + return -ENOENT;
> +}
> +
> +static int pick_avail_cos(struct psr_cat_cbm *map, int cos_max, int old_cos)
> +{
> + int cos;
> +
> + /* If old cos is referred only by the domain, then use it. */
> + if ( map[old_cos].ref == 1 )
> + return old_cos;
> +
> + /* Then we pick an unused one, never pick 0 */
Find an unused one other than cos0.
> + for ( cos = 1; cos <= cos_max; cos++ )
> + if ( map[cos].ref == 0 )
> + return cos;
> +
> + return -ENOENT;
> +}
> +
> +int psr_set_l3_cbm(struct domain *d, unsigned int socket,
> + uint64_t cbm, enum cbm_type type)
> +{
> + unsigned int old_cos, cos_max;
> + int cos, ret;
> + uint64_t cbm_data, cbm_code;
> + bool_t need_write = 1;
> + struct psr_cat_cbm *map;
> struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>
> if ( IS_ERR(info) )
> return PTR_ERR(info);
>
> + cbm_code = (1ull << info->cbm_len) - 1;
> + cbm_data = (1ull << info->cbm_len) - 1;
Why this is needed, as you will give them the value eventually.
> if ( !psr_check_cbm(info->cbm_len, cbm) )
> return -EINVAL;
>
> + cos_max = info->cos_max;
> old_cos = d->arch.psr_cos_ids[socket];
> map = info->cos_to_cbm;
>
> - spin_lock(&info->cbm_lock);
> -
> - for ( cos = 0; cos <= info->cos_max; cos++ )
> + switch( type )
Coding style.
Chao
> {
> - /* If still not found, then keep unused one. */
> - if ( !found && cos != 0 && map[cos].ref == 0 )
> - found = map + cos;
> - else if ( map[cos].u.cbm == cbm )
> - {
> - if ( unlikely(cos == old_cos) )
> - {
> - ASSERT(cos == 0 || map[cos].ref != 0);
> - spin_unlock(&info->cbm_lock);
> - return 0;
> - }
> - found = map + cos;
> + case PSR_CBM_TYPE_L3:
> + if ( info->cdp_enabled )
> + return -EINVAL;
> + cbm_code = cbm;
> + cbm_data = cbm;
> break;
> - }
> - }
>
> - /* If old cos is referred only by the domain, then use it. */
> - if ( !found && map[old_cos].ref == 1 )
> - found = map + old_cos;
> + case PSR_CBM_TYPE_L3_CODE:
> + if ( !info->cdp_enabled )
> + return -EINVAL;
> + cbm_code = cbm;
> + cbm_data = map[old_cos].u.cdp.data;
> + break;
>
> - if ( !found )
> - {
> - spin_unlock(&info->cbm_lock);
> - return -EOVERFLOW;
> + case PSR_CBM_TYPE_L3_DATA:
> + if ( !info->cdp_enabled )
> + return -EINVAL;
> + cbm_code = map[old_cos].u.cdp.code;
> + cbm_data = cbm;
> + break;
> +
> + default:
> + return -EINVAL;
> }
>
> - cos = found - map;
> - if ( found->u.cbm != cbm )
> + spin_lock(&info->cbm_lock);
> + cos = find_cos(map, cos_max, cbm_code, cbm_data, info->cdp_enabled);
> + if ( cos >= 0 )
> {
> - int ret = write_l3_cbm(socket, cos, cbm, 0, 0);
> -
> - if ( ret )
> + if ( unlikely(cos == old_cos) )
> {
> spin_unlock(&info->cbm_lock);
> - return ret;
> + return 0;
> + }
> + }
> + else
> + {
> + cos = pick_avail_cos(map, cos_max, old_cos);
> + if ( cos < 0 )
> + {
> + spin_unlock(&info->cbm_lock);
> + return cos;
> + }
> +
> + /* We try to avoid writing MSR */
> + if ( info->cdp_enabled )
> + {
> + if ( map[cos].u.cdp.code == cbm_code &&
> + map[cos].u.cdp.data == cbm_data )
> + need_write = 0;
> + }
> + else
> + need_write = map[cos].u.cbm == cbm_code ? 0 : 1;
> +
> + if ( need_write )
> + {
> + ret = write_l3_cbm(socket, cos, cbm_code, cbm_data, info->cdp_enabled);
> + if ( ret )
> + {
> + spin_unlock(&info->cbm_lock);
> + return ret;
> + }
> + map[cos].u.cdp.code = cbm_code;
> + map[cos].u.cdp.data = cbm_data;
> }
> - found->u.cbm = cbm;
> }
>
> - found->ref++;
> + map[cos].ref++;
> map[old_cos].ref--;
> spin_unlock(&info->cbm_lock);
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/4] x86: add domctl cmd to set/get CDP code/data CBM
2015-09-09 7:24 ` Chao Peng
@ 2015-09-09 7:35 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2015-09-09 7:35 UTC (permalink / raw)
To: Chao Peng, He Chen
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
ian.jackson, xen-devel, keir
>>> On 09.09.15 at 09:24, <chao.p.peng@linux.intel.com> wrote:
> On Wed, Sep 09, 2015 at 01:16:46PM +0800, He Chen wrote:
>> -int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
>> +int psr_get_l3_cbm(struct domain *d, unsigned int socket,
>> + uint64_t *cbm, enum cbm_type type)
>> {
>> struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>>
>> if ( IS_ERR(info) )
>> return PTR_ERR(info);
>>
>> - *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
>> + if ( type == PSR_CBM_TYPE_L3 )
>> + {
>> + if ( info->cdp_enabled )
>> + return -EXDEV;
>> + *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
>> + }
>> + else if ( type == PSR_CBM_TYPE_L3_CODE )
>> + {
>> + if ( !info->cdp_enabled )
>> + return -EXDEV;
>> + *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
>> + }
>> + else
>> + {
>> + if ( !info->cdp_enabled )
>> + return -EXDEV;
>> + *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
>> + }
>
> Could I suggest to use switch? And the check can be done in advance.
Indeed. And please consider carefully what the default case ought
to be.
>> @@ -371,65 +389,136 @@ static int write_l3_cbm(unsigned int socket, unsigned int cos,
>> return 0;
>> }
>>
>> -int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
>> +static int find_cos(struct psr_cat_cbm *map, int cos_max,
>> + uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
>> {
>> - unsigned int old_cos, cos;
>> - struct psr_cat_cbm *map, *found = NULL;
>> + unsigned int cos;
>> +
>> + if ( !cdp_enabled )
>> + {
>> + for ( cos = 0; cos <= cos_max; cos++ )
>> + if ( map[cos].ref && map[cos].u.cbm == cbm_code )
>> + return cos;
>> + }
>> + else
>> + {
>> + for ( cos = 0; cos <= cos_max; cos++ )
>> + if ( map[cos].ref && map[cos].u.cdp.code == cbm_code &&
>> + map[cos].u.cdp.data == cbm_data )
>> + return cos;
>> + }
>
> I guess the '{' and '}' can be omitted if the body contains only one
> sentence.
Not really: They are required on the outer if(), and hence for
consistency they should be retained on the corresponding else.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/4] tools: add tools support for Intel CDP
2015-09-09 5:16 [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling He Chen
2015-09-09 5:16 ` [PATCH v2 1/4] x86: Support enable CDP by boot parameter and add get CDP status He Chen
2015-09-09 5:16 ` [PATCH v2 2/4] x86: add domctl cmd to set/get CDP code/data CBM He Chen
@ 2015-09-09 5:16 ` He Chen
2015-09-09 7:32 ` Chao Peng
2015-09-09 5:16 ` [PATCH v2 4/4] docs: add document to introduce CDP command He Chen
2015-09-09 7:37 ` [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling Chao Peng
4 siblings, 1 reply; 15+ messages in thread
From: He Chen @ 2015-09-09 5:16 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, chao.p.peng, keir
This is the xl/xc changes to support Intel Code/Data Prioritization.
CAT xl commands to set/get CBMs are extended to support CDP.
Signed-off-by: He Chen <he.chen@linux.intel.com>
---
tools/libxc/include/xenctrl.h | 7 ++++--
tools/libxc/xc_psr.c | 17 +++++++++++++-
tools/libxl/libxl.h | 10 +++++++++
tools/libxl/libxl_psr.c | 32 ++++++++++++++++++++++++--
tools/libxl/libxl_types.idl | 3 +++
tools/libxl/xl.h | 2 ++
tools/libxl/xl_cmdimpl.c | 52 ++++++++++++++++++++++++++++++++++---------
tools/libxl/xl_cmdtable.c | 5 +++++
8 files changed, 113 insertions(+), 15 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index de3c0ad..54f2069 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2798,7 +2798,9 @@ enum xc_psr_cmt_type {
typedef enum xc_psr_cmt_type xc_psr_cmt_type;
enum xc_psr_cat_type {
- XC_PSR_CAT_L3_CBM = 1,
+ XC_PSR_CAT_L3_CBM = 1,
+ XC_PSR_CAT_L3_CODE = 2,
+ XC_PSR_CAT_L3_DATA = 3,
};
typedef enum xc_psr_cat_type xc_psr_cat_type;
@@ -2824,7 +2826,8 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
xc_psr_cat_type type, uint32_t target,
uint64_t *data);
int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
- uint32_t *cos_max, uint32_t *cbm_len);
+ uint32_t *cos_max, uint32_t *cbm_len,
+ bool *cdp_enabled);
#endif
#endif /* XENCTRL_H */
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index d8b3a51..5bbe950 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -260,6 +260,12 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
case XC_PSR_CAT_L3_CBM:
cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM;
break;
+ case XC_PSR_CAT_L3_CODE:
+ cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE;
+ break;
+ case XC_PSR_CAT_L3_DATA:
+ cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA;
+ break;
default:
errno = EINVAL;
return -1;
@@ -287,6 +293,12 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
case XC_PSR_CAT_L3_CBM:
cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM;
break;
+ case XC_PSR_CAT_L3_CODE:
+ cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE;
+ break;
+ case XC_PSR_CAT_L3_DATA:
+ cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA;
+ break;
default:
errno = EINVAL;
return -1;
@@ -306,7 +318,8 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
}
int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
- uint32_t *cos_max, uint32_t *cbm_len)
+ uint32_t *cos_max, uint32_t *cbm_len,
+ bool *cdp_enabled)
{
int rc;
DECLARE_SYSCTL;
@@ -320,6 +333,8 @@ int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
{
*cos_max = sysctl.u.psr_cat_op.u.l3_info.cos_max;
*cbm_len = sysctl.u.psr_cat_op.u.l3_info.cbm_len;
+ *cdp_enabled = sysctl.u.psr_cat_op.u.l3_info.flags &
+ XEN_SYSCTL_PSR_CAT_L3_CDP;
}
return rc;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5f9047c..e4eb4df 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -796,6 +796,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
* If this is defined, the Cache Allocation Technology feature is supported.
*/
#define LIBXL_HAVE_PSR_CAT 1
+
+/*
+ * LIBXL_HAVE_PSR_CDP
+ *
+ * If this is defined, the Code/Data Prioritization feature is supported.
+ */
+#define LIBXL_HAVE_PSR_CDP 1
#endif
/*
@@ -1729,6 +1736,9 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
#endif
+#ifdef LIBXL_HAVE_PSR_CDP
+#endif
+
/* misc */
/* Each of these sets or clears the flag according to whether the
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 3378239..26939a5 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -297,6 +297,7 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
GC_INIT(ctx);
int rc;
int socketid, nr_sockets;
+ libxl_psr_cat_info *info;
rc = libxl__count_physical_sockets(gc, &nr_sockets);
if (rc) {
@@ -304,14 +305,41 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
goto out;
}
+ rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
+ if (rc) {
+ LOGE(ERROR, "Failed to get cat info");
+ goto out;
+ }
+
+ if (!info->cdp_enabled &&
+ (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
+ type == LIBXL_PSR_CBM_TYPE_L3_DATA))
+ {
+ LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
+ rc = EINVAL;
+ free(info);
+ goto out;
+ }
+
libxl_for_each_set_bit(socketid, *target_map) {
if (socketid >= nr_sockets)
break;
- if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
+ if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
+ {
+ if(xc_psr_cat_set_domain_data(ctx->xch, domid,
+ LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
+ xc_psr_cat_set_domain_data(ctx->xch, domid,
+ LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
+ {
+ libxl__psr_cat_log_err_msg(gc, errno);
+ rc = ERROR_FAIL;
+ }
+ } else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
libxl__psr_cat_log_err_msg(gc, errno);
rc = ERROR_FAIL;
}
}
+ free(info);
out:
GC_FREE;
@@ -352,7 +380,7 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
for (i = 0; i < nr_sockets; i++) {
if (xc_psr_cat_get_l3_info(ctx->xch, i, &ptr[i].cos_max,
- &ptr[i].cbm_len)) {
+ &ptr[i].cbm_len, &ptr[i].cdp_enabled)) {
libxl__psr_cat_log_err_msg(gc, errno);
rc = ERROR_FAIL;
free(ptr);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ef346e7..fa017ad 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -787,9 +787,12 @@ libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
(0, "UNKNOWN"),
(1, "L3_CBM"),
+ (2, "L3_CODE"),
+ (3, "L3_DATA"),
])
libxl_psr_cat_info = Struct("psr_cat_info", [
("cos_max", uint32),
("cbm_len", uint32),
+ ("cdp_enabled", bool),
])
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 13bccba..8aefccf 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -121,6 +121,8 @@ int main_psr_cmt_show(int argc, char **argv);
int main_psr_cat_cbm_set(int argc, char **argv);
int main_psr_cat_show(int argc, char **argv);
#endif
+#ifdef LIBXL_HAVE_PSR_CDP
+#endif
void help(const char *command);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index ebbb9a5..ec8fffa 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8390,6 +8390,10 @@ static int psr_cat_hwinfo(void)
}
printf("%-16s: %u\n", "Socket ID", socketid);
printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
+ if (info->cdp_enabled)
+ printf("%-16s: Enabled\n", "CDP Status");
+ else
+ printf("%-16s: Disabled\n", "CDP Status");
printf("%-16s: %u\n", "Maximum COS", info->cos_max);
printf("%-16s: %u\n", "CBM length", info->cbm_len);
printf("%-16s: %#llx\n", "Default CBM",
@@ -8401,7 +8405,8 @@ out:
return rc;
}
-static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid)
+static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid,
+ bool cdp_enabled)
{
char *domain_name;
uint64_t cbm;
@@ -8410,20 +8415,29 @@ static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid)
printf("%5d%25s", domid, domain_name);
free(domain_name);
- if (!libxl_psr_cat_get_cbm(ctx, domid, LIBXL_PSR_CBM_TYPE_L3_CBM,
- socketid, &cbm))
- printf("%#16"PRIx64, cbm);
-
+ if (!cdp_enabled) {
+ if (!libxl_psr_cat_get_cbm(ctx, domid, LIBXL_PSR_CBM_TYPE_L3_CBM,
+ socketid, &cbm))
+ printf("%#16"PRIx64, cbm);
+ } else {
+ if (!libxl_psr_cat_get_cbm(ctx, domid, LIBXL_PSR_CBM_TYPE_L3_CODE,
+ socketid, &cbm))
+ printf("%10s%#8"PRIx64, "code:", cbm);
+ if (!libxl_psr_cat_get_cbm(ctx, domid, LIBXL_PSR_CBM_TYPE_L3_DATA,
+ socketid, &cbm))
+ printf("%10s%#8"PRIx64, "data:", cbm);
+ }
printf("\n");
}
-static int psr_cat_print_domain_cbm(uint32_t domid, uint32_t socketid)
+static int psr_cat_print_domain_cbm(uint32_t domid, uint32_t socketid,
+ bool cdp_enabled)
{
int i, nr_domains;
libxl_dominfo *list;
if (domid != INVALID_DOMID) {
- psr_cat_print_one_domain_cbm(domid, socketid);
+ psr_cat_print_one_domain_cbm(domid, socketid, cdp_enabled);
return 0;
}
@@ -8433,7 +8447,7 @@ static int psr_cat_print_domain_cbm(uint32_t domid, uint32_t socketid)
}
for (i = 0; i < nr_domains; i++)
- psr_cat_print_one_domain_cbm(list[i].domid, socketid);
+ psr_cat_print_one_domain_cbm(list[i].domid, socketid, cdp_enabled);
libxl_dominfo_list_free(list, nr_domains);
return 0;
@@ -8457,7 +8471,7 @@ static int psr_cat_print_socket(uint32_t domid, uint32_t socketid,
printf("%-16s: %#llx\n", "Default CBM", (1ull << info->cbm_len) - 1);
printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
- return psr_cat_print_domain_cbm(domid, socketid);
+ return psr_cat_print_domain_cbm(domid, socketid, info->cdp_enabled);
}
static int psr_cat_show(uint32_t domid)
@@ -8489,6 +8503,8 @@ int main_psr_cat_cbm_set(int argc, char **argv)
libxl_psr_cbm_type type = LIBXL_PSR_CBM_TYPE_L3_CBM;
uint64_t cbm;
int ret, opt = 0;
+ int opt_data = 0;
+ int opt_code = 0;
libxl_bitmap target_map;
char *value;
libxl_string_list socket_list;
@@ -8497,13 +8513,15 @@ int main_psr_cat_cbm_set(int argc, char **argv)
static struct option opts[] = {
{"socket", 1, 0, 's'},
+ {"data", 0, 0, 'd'},
+ {"code", 0, 0, 'c'},
COMMON_LONG_OPTS
};
libxl_socket_bitmap_alloc(ctx, &target_map, 0);
libxl_bitmap_set_none(&target_map);
- SWITCH_FOREACH_OPT(opt, "s:", opts, "psr-cat-cbm-set", 2) {
+ SWITCH_FOREACH_OPT(opt, "s:cd", opts, "psr-cat-cbm-set", 2) {
case 's':
trim(isspace, optarg, &value);
split_string_into_string_list(value, ",", &socket_list);
@@ -8517,8 +8535,19 @@ int main_psr_cat_cbm_set(int argc, char **argv)
libxl_string_list_dispose(&socket_list);
free(value);
break;
+ case 'd':
+ type = LIBXL_PSR_CBM_TYPE_L3_DATA;
+ opt_data = 1;
+ break;
+ case 'c':
+ type = LIBXL_PSR_CBM_TYPE_L3_CODE;
+ opt_code = 1;
+ break;
}
+ if (opt_data && opt_code)
+ type = LIBXL_PSR_CBM_TYPE_L3_CBM;
+
if (libxl_bitmap_is_empty(&target_map))
libxl_bitmap_set_any(&target_map);
@@ -8585,6 +8614,9 @@ int main_psr_hwinfo(int argc, char **argv)
return ret;
}
+#ifdef LIBXL_HAVE_PSR_CDP
+#endif
+
#endif
/*
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 0071f12..1c99b9b 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -543,6 +543,8 @@ struct cmd_spec cmd_table[] = {
"Set cache capacity bitmasks(CBM) for a domain",
"[options] <Domain> <CBM>",
"-s <socket> Specify the socket to process, otherwise all sockets are processed\n"
+ "-c Set code CBM if CDP is supported\n"
+ "-d Set data CBM if CDP is supported\n"
},
{ "psr-cat-show",
&main_psr_cat_show, 0, 1,
@@ -551,6 +553,9 @@ struct cmd_spec cmd_table[] = {
},
#endif
+
+#ifdef LIBXL_HAVE_PSR_CDP
+#endif
};
int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 3/4] tools: add tools support for Intel CDP
2015-09-09 5:16 ` [PATCH v2 3/4] tools: add tools support for Intel CDP He Chen
@ 2015-09-09 7:32 ` Chao Peng
2015-09-09 8:10 ` He Chen
0 siblings, 1 reply; 15+ messages in thread
From: Chao Peng @ 2015-09-09 7:32 UTC (permalink / raw)
To: He Chen
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
ian.jackson, jbeulich, xen-devel, keir
On Wed, Sep 09, 2015 at 01:16:47PM +0800, He Chen wrote:
> This is the xl/xc changes to support Intel Code/Data Prioritization.
> CAT xl commands to set/get CBMs are extended to support CDP.
>
> Signed-off-by: He Chen <he.chen@linux.intel.com>
> ---
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 5f9047c..e4eb4df 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -796,6 +796,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
> * If this is defined, the Cache Allocation Technology feature is supported.
> */
> #define LIBXL_HAVE_PSR_CAT 1
> +
> +/*
> + * LIBXL_HAVE_PSR_CDP
> + *
> + * If this is defined, the Code/Data Prioritization feature is supported.
> + */
> +#define LIBXL_HAVE_PSR_CDP 1
> #endif
>
> /*
> @@ -1729,6 +1736,9 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
> void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
> #endif
>
> +#ifdef LIBXL_HAVE_PSR_CDP
> +#endif
> +
Why this? There are several of them in this patch.
> /* misc */
>
> /* Each of these sets or clears the flag according to whether the
> diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> index 3378239..26939a5 100644
> --- a/tools/libxl/libxl_psr.c
> +++ b/tools/libxl/libxl_psr.c
> @@ -297,6 +297,7 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> GC_INIT(ctx);
> int rc;
> int socketid, nr_sockets;
> + libxl_psr_cat_info *info;
>
> rc = libxl__count_physical_sockets(gc, &nr_sockets);
> if (rc) {
> @@ -304,14 +305,41 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> goto out;
> }
>
> + rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
> + if (rc) {
> + LOGE(ERROR, "Failed to get cat info");
> + goto out;
> + }
> +
> + if (!info->cdp_enabled &&
> + (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
> + type == LIBXL_PSR_CBM_TYPE_L3_DATA))
> + {
> + LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
> + rc = EINVAL;
> + free(info);
> + goto out;
> + }
> +
> libxl_for_each_set_bit(socketid, *target_map) {
> if (socketid >= nr_sockets)
> break;
> - if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
> + if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
> + {
> + if(xc_psr_cat_set_domain_data(ctx->xch, domid,
> + LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
> + xc_psr_cat_set_domain_data(ctx->xch, domid,
> + LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
> + {
> + libxl__psr_cat_log_err_msg(gc, errno);
> + rc = ERROR_FAIL;
> + }
Will you merge the two if's?
Chao
> + } else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
> libxl__psr_cat_log_err_msg(gc, errno);
> rc = ERROR_FAIL;
> }
> }
> + free(info);
>
> out:
> GC_FREE;
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 3/4] tools: add tools support for Intel CDP
2015-09-09 7:32 ` Chao Peng
@ 2015-09-09 8:10 ` He Chen
2015-09-09 8:37 ` Jan Beulich
0 siblings, 1 reply; 15+ messages in thread
From: He Chen @ 2015-09-09 8:10 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
ian.jackson, jbeulich, xen-devel, keir
On Wed, Sep 09, 2015 at 03:32:11PM +0800, Chao Peng wrote:
> On Wed, Sep 09, 2015 at 01:16:47PM +0800, He Chen wrote:
> > This is the xl/xc changes to support Intel Code/Data Prioritization.
> > CAT xl commands to set/get CBMs are extended to support CDP.
> >
> > Signed-off-by: He Chen <he.chen@linux.intel.com>
> > ---
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index 5f9047c..e4eb4df 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -796,6 +796,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
> > * If this is defined, the Cache Allocation Technology feature is supported.
> > */
> > #define LIBXL_HAVE_PSR_CAT 1
> > +
> > +/*
> > + * LIBXL_HAVE_PSR_CDP
> > + *
> > + * If this is defined, the Code/Data Prioritization feature is supported.
> > + */
> > +#define LIBXL_HAVE_PSR_CDP 1
> > #endif
> >
> > /*
> > @@ -1729,6 +1736,9 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
> > void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
> > #endif
> >
> > +#ifdef LIBXL_HAVE_PSR_CDP
> > +#endif
> > +
>
> Why this? There are several of them in this patch.
>
My carelessness. I would remove the redundant code lines.
> > /* misc */
> >
> > /* Each of these sets or clears the flag according to whether the
> > diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> > index 3378239..26939a5 100644
> > --- a/tools/libxl/libxl_psr.c
> > +++ b/tools/libxl/libxl_psr.c
> > @@ -297,6 +297,7 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> > GC_INIT(ctx);
> > int rc;
> > int socketid, nr_sockets;
> > + libxl_psr_cat_info *info;
> >
> > rc = libxl__count_physical_sockets(gc, &nr_sockets);
> > if (rc) {
> > @@ -304,14 +305,41 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> > goto out;
> > }
> >
> > + rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
> > + if (rc) {
> > + LOGE(ERROR, "Failed to get cat info");
> > + goto out;
> > + }
> > +
> > + if (!info->cdp_enabled &&
> > + (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
> > + type == LIBXL_PSR_CBM_TYPE_L3_DATA))
> > + {
> > + LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
> > + rc = EINVAL;
> > + free(info);
> > + goto out;
> > + }
> > +
> > libxl_for_each_set_bit(socketid, *target_map) {
> > if (socketid >= nr_sockets)
> > break;
> > - if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
> > + if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
> > + {
> > + if(xc_psr_cat_set_domain_data(ctx->xch, domid,
> > + LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
> > + xc_psr_cat_set_domain_data(ctx->xch, domid,
> > + LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
> > + {
> > + libxl__psr_cat_log_err_msg(gc, errno);
> > + rc = ERROR_FAIL;
> > + }
>
> Will you merge the two if's?
>
> Chao
Surely.
> > + } else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
> > libxl__psr_cat_log_err_msg(gc, errno);
> > rc = ERROR_FAIL;
> > }
> > }
> > + free(info);
> >
> > out:
> > GC_FREE;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 3/4] tools: add tools support for Intel CDP
2015-09-09 8:10 ` He Chen
@ 2015-09-09 8:37 ` Jan Beulich
2015-09-09 9:17 ` Chao Peng
0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2015-09-09 8:37 UTC (permalink / raw)
To: Chao Peng, He Chen
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
ian.jackson, xen-devel, keir
>>> On 09.09.15 at 10:10, <he.chen@linux.intel.com> wrote:
> On Wed, Sep 09, 2015 at 03:32:11PM +0800, Chao Peng wrote:
>> On Wed, Sep 09, 2015 at 01:16:47PM +0800, He Chen wrote:
>> > @@ -304,14 +305,41 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
>> > goto out;
>> > }
>> >
>> > + rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
>> > + if (rc) {
>> > + LOGE(ERROR, "Failed to get cat info");
>> > + goto out;
>> > + }
>> > +
>> > + if (!info->cdp_enabled &&
>> > + (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
>> > + type == LIBXL_PSR_CBM_TYPE_L3_DATA))
>> > + {
>> > + LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
>> > + rc = EINVAL;
>> > + free(info);
>> > + goto out;
>> > + }
>> > +
>> > libxl_for_each_set_bit(socketid, *target_map) {
>> > if (socketid >= nr_sockets)
>> > break;
>> > - if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
>> > + if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
>> > + {
>> > + if(xc_psr_cat_set_domain_data(ctx->xch, domid,
>> > + LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
>> > + xc_psr_cat_set_domain_data(ctx->xch, domid,
>> > + LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
>> > + {
>> > + libxl__psr_cat_log_err_msg(gc, errno);
>> > + rc = ERROR_FAIL;
>> > + }
>>
>> Will you merge the two if's?
>
> Surely.
Surely not you mean, or else how will this ...
>> > + } else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
... work?
Looking at this I'm surprised though that consumers is required to do
two calls (and know whether CDP is enabled) when they want to set
things up without caring for the code/data distinction. I think this
should be taken care of in the hypervisor.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 3/4] tools: add tools support for Intel CDP
2015-09-09 8:37 ` Jan Beulich
@ 2015-09-09 9:17 ` Chao Peng
0 siblings, 0 replies; 15+ messages in thread
From: Chao Peng @ 2015-09-09 9:17 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, xen-devel, keir
On Wed, Sep 09, 2015 at 02:37:36AM -0600, Jan Beulich wrote:
> >>> On 09.09.15 at 10:10, <he.chen@linux.intel.com> wrote:
> > On Wed, Sep 09, 2015 at 03:32:11PM +0800, Chao Peng wrote:
> >> On Wed, Sep 09, 2015 at 01:16:47PM +0800, He Chen wrote:
> >> > @@ -304,14 +305,41 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> >> > goto out;
> >> > }
> >> >
> >> > + rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
> >> > + if (rc) {
> >> > + LOGE(ERROR, "Failed to get cat info");
> >> > + goto out;
> >> > + }
> >> > +
> >> > + if (!info->cdp_enabled &&
> >> > + (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
> >> > + type == LIBXL_PSR_CBM_TYPE_L3_DATA))
> >> > + {
> >> > + LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
> >> > + rc = EINVAL;
> >> > + free(info);
> >> > + goto out;
> >> > + }
> >> > +
> >> > libxl_for_each_set_bit(socketid, *target_map) {
> >> > if (socketid >= nr_sockets)
> >> > break;
> >> > - if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
> >> > + if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
> >> > + {
> >> > + if(xc_psr_cat_set_domain_data(ctx->xch, domid,
> >> > + LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
> >> > + xc_psr_cat_set_domain_data(ctx->xch, domid,
> >> > + LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
> >> > + {
> >> > + libxl__psr_cat_log_err_msg(gc, errno);
> >> > + rc = ERROR_FAIL;
> >> > + }
> >>
> >> Will you merge the two if's?
> >
> > Surely.
>
> Surely not you mean, or else how will this ...
>
> >> > + } else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
>
> ... work?
Indeed. Then just ignore it.
>
> Looking at this I'm surprised though that consumers is required to do
> two calls (and know whether CDP is enabled) when they want to set
> things up without caring for the code/data distinction. I think this
> should be taken care of in the hypervisor.
Agreed, hypervisor should take care of it for this case.
Chao
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 4/4] docs: add document to introduce CDP command
2015-09-09 5:16 [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling He Chen
` (2 preceding siblings ...)
2015-09-09 5:16 ` [PATCH v2 3/4] tools: add tools support for Intel CDP He Chen
@ 2015-09-09 5:16 ` He Chen
2015-09-09 7:37 ` [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling Chao Peng
4 siblings, 0 replies; 15+ messages in thread
From: He Chen @ 2015-09-09 5:16 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, chao.p.peng, keir
Add CDP command in xl interface man page and add description of CDP
in xl-psr.markdown.
Signed-off-by: He Chen <he.chen@linux.intel.com>
---
docs/man/xl.pod.1 | 14 ++++++++++++++
docs/misc/xl-psr.markdown | 44 +++++++++++++++++++++++++++++++++++++++-----
2 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index f22c3f3..3801039 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1530,6 +1530,12 @@ applications. In the Xen implementation, CAT is used to control cache allocation
on VM basis. To enforce cache on a specific domain, just set capacity bitmasks
(CBM) for the domain.
+Intel Broadwell and later server platforms also offer Code/Data Prioritization
+(CDP) for cache allocation, which support specify code or data cache for
+applications. CDP is used on VM basis in the Xen implementation. To specify
+code or data CBM for the domain, CDP feature must be enabled and CBM type
+options need to be specified when setting CBM.
+
=over 4
=item B<psr-cat-cbm-set> [I<OPTIONS>] I<domain-id> I<cbm>
@@ -1545,6 +1551,14 @@ B<OPTIONS>
Specify the socket to process, otherwise all sockets are processed.
+=item B<-c>, B<--code>
+
+Set code CBM with CDP enabled.
+
+=item B<-d>, B<--data>
+
+Set data CBM with CDP enabled.
+
=back
=item B<psr-cat-show> [I<domain-id>]
diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
index 3545912..4c77737 100644
--- a/docs/misc/xl-psr.markdown
+++ b/docs/misc/xl-psr.markdown
@@ -14,7 +14,7 @@ tracks cache utilization of memory accesses according to the RMID and reports
monitored data via a counter register.
For more detailed information please refer to Intel SDM chapter
-"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
+"17.15 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
In Xen's implementation, each domain in the system can be assigned a RMID
independently, while RMID=0 is reserved for monitoring domains that don't
@@ -91,17 +91,44 @@ For example, assuming a system with 8 portions and 3 domains:
first domain exclusive access to half the cache, and the other two exclusive
access to one quarter each.
-For more detailed information please refer to Intel SDM chapter
-"17.15 - Platform Shared Resource Control: Cache Allocation Technology".
-
In Xen's implementation, CBM can be configured with libxl/xl interfaces but
COS is maintained in hypervisor only. The cache partition granularity is per
domain, each domain has COS=0 assigned by default, the corresponding CBM is
all-ones, which means all the cache resource can be used by default.
+Code/Data Prioritization (CDP) Technology is an extension of CAT, which is
+available on Intel Broadwell and later server platforms. CDP enables isolation
+and separate prioritization of code and data fetches to the L3 cache in a
+software configurable manner, which can enable workload prioritization and
+tuning of cache capacity to the characteristics of the workload. CDP extends
+Cache Allocation Technology (CAT) by providing separate code and data masks
+per Class of Service (COS).
+
+CDP is disabled on the processor by default. If the CAT MSRs are used without
+enabling CDP, the processor operates in a traditional CAT-only mode.
+
+When CDP is enabled,
+
+ * the CAT mask MSRs are re-mapped into interleaved pairs of mask MSRs for
+ data or code fetches.
+
+ * the range of COS for CAT is re-indexed, with the lower-half of the COS
+ range available for CDP.
+
+CDP allows OS or Hypervisor to partition cache allocation more fine-grained,
+code cache and data cache can be specified respectively. To enable CDP on
+platform, all sockets in the platform must have CDP either enabled or
+disabled, not a mix. With CDP enabled, one COS corresponds to two CBMs (code
+CBM & data CBM), which means the number of available COS will reduce to half
+when CDP is on.
+
+For more detailed information please refer to Intel SDM chapter
+"17.16 - Platform Shared Resource Control: Cache Allocation Technology".
+
### xl interfaces
-System CAT information such as maximum COS and CBM length can be obtained by:
+System CAT information such as maximum COS, CBM length and CDP status can be
+obtained by:
`xl psr-hwinfo --cat`
@@ -119,6 +146,13 @@ A cbm is valid only when:
In a multi-socket system, the same cbm will be set on each socket by default.
Per socket cbm can be specified with the `--socket SOCKET` option.
+To specify code CBM for the domain, `-c` or `--code` option is needed.
+
+To specify data CBM for the domain, `-d` or `--data` option is needed.
+
+If neither `-c` nor `-d` option is specified when CDP is on, the same code CBM
+and data CBM will be set for the domain.
+
Setting the CBM may not be successful if insufficient COS is available. In
such case unused COS(es) may be freed by setting CBM of all related domains to
its default value(all-ones).
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling
2015-09-09 5:16 [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling He Chen
` (3 preceding siblings ...)
2015-09-09 5:16 ` [PATCH v2 4/4] docs: add document to introduce CDP command He Chen
@ 2015-09-09 7:37 ` Chao Peng
2015-09-09 12:36 ` Ian Campbell
4 siblings, 1 reply; 15+ messages in thread
From: Chao Peng @ 2015-09-09 7:37 UTC (permalink / raw)
To: He Chen
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
ian.jackson, jbeulich, xen-devel, keir
On Wed, Sep 09, 2015 at 01:16:44PM +0800, He Chen wrote:
> Changes in v2:
> - x86: Enable CDP by boot parameter instead of enabling/disabling CDP at
> runtime (suggested by Andrew)
As you added a new boot option, you also need a patch for
docs/misc/xen-command-line.markdown.
Chao
> - tools: remove psr-cat-cdp-enable/disable xl commands
> - code style
>
> Code/Data Prioritization(CDP) is offered in Intel Broadwell and later server
> platforms, which is an extension of CAT. CDP enables isolation and separate
> prioritization of code and data fetches to the L3 cache in a software
> configurable manner, which can enable workload prioritization and tuning of
> cache capacity to the characteristics of the workload. CDP extends Cache
> Allocation Technology (CAT) by providing separate code and data capacity bit
> masks(CBM) per Class of Service (COS). CDP is used on VM basis in the Xen
> implementation.
>
> More information about CDP, please refer to Intel SDM, Volumn 3, section 17.16
> http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf
>
> This patch series enables CDP feature in Xen based on CAT code, and extends
> CBM operation functions to support CDP. For all the changes, please see in
> each patch.
>
> This patchset has been tested on Intel Broadwell server platform.
>
> To make this patchset better, any comment or suggestion is welcomed, I would
> really appreciate it.
>
> Thanks
>
> He Chen (4):
> x86: Support enable CDP by boot parameter and add get CDP status
> x86: add domctl cmd to set/get CDP code/data CBM
> tools: add tools support for Intel CDP
> docs: add document to introduce CDP command
>
> docs/man/xl.pod.1 | 14 +++
> docs/misc/xl-psr.markdown | 44 +++++++-
> tools/libxc/include/xenctrl.h | 7 +-
> tools/libxc/xc_psr.c | 17 ++-
> tools/libxl/libxl.h | 10 ++
> tools/libxl/libxl_psr.c | 32 +++++-
> tools/libxl/libxl_types.idl | 3 +
> tools/libxl/xl.h | 2 +
> tools/libxl/xl_cmdimpl.c | 52 +++++++--
> tools/libxl/xl_cmdtable.c | 5 +
> xen/arch/x86/domctl.c | 32 +++++-
> xen/arch/x86/psr.c | 237 ++++++++++++++++++++++++++++++++--------
> xen/arch/x86/sysctl.c | 5 +-
> xen/include/asm-x86/msr-index.h | 3 +
> xen/include/asm-x86/psr.h | 23 +++-
> xen/include/public/domctl.h | 4 +
> xen/include/public/sysctl.h | 2 +
> 17 files changed, 420 insertions(+), 72 deletions(-)
>
> --
> 1.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling
2015-09-09 7:37 ` [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling Chao Peng
@ 2015-09-09 12:36 ` Ian Campbell
0 siblings, 0 replies; 15+ messages in thread
From: Ian Campbell @ 2015-09-09 12:36 UTC (permalink / raw)
To: Chao Peng, He Chen
Cc: wei.liu2, stefano.stabellini, andrew.cooper3, ian.jackson,
jbeulich, xen-devel, keir
On Wed, 2015-09-09 at 15:37 +0800, Chao Peng wrote:
> On Wed, Sep 09, 2015 at 01:16:44PM +0800, He Chen wrote:
> > Changes in v2:
> > - x86: Enable CDP by boot parameter instead of enabling/disabling CDP
> > at
> > runtime (suggested by Andrew)
>
> As you added a new boot option, you also need a patch for
> docs/misc/xen-command-line.markdown.
FYI this is (usually) best done in the patch which adds the option, rather
than in a separate patch.
Ian.
^ permalink raw reply [flat|nested] 15+ messages in thread