* [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling
@ 2015-09-02 8:27 He Chen
2015-09-02 8:27 ` [PATCH 1/5] x86: detect Intel CDP feature He Chen
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: He Chen @ 2015-09-02 8:27 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, keir
Hi all,
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, including
extending CBM operation functions and introducing new commands to enable/disable
CDP dynamically. 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 (5):
x86: detect Intel CDP feature
x86: Support enable/disable CDP dynamically and 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 | 22 +++
docs/misc/xl-psr.markdown | 56 +++++++-
tools/libxc/include/xenctrl.h | 10 +-
tools/libxc/xc_psr.c | 42 +++++-
tools/libxl/libxl.h | 12 ++
tools/libxl/libxl_psr.c | 64 ++++++++-
tools/libxl/libxl_types.idl | 3 +
tools/libxl/xl.h | 4 +
tools/libxl/xl_cmdimpl.c | 81 +++++++++--
tools/libxl/xl_cmdtable.c | 15 ++
xen/arch/x86/domctl.c | 33 ++++-
xen/arch/x86/psr.c | 309 ++++++++++++++++++++++++++++++++++++------
xen/arch/x86/sysctl.c | 9 +-
xen/include/asm-x86/psr.h | 26 +++-
xen/include/public/domctl.h | 4 +
xen/include/public/sysctl.h | 5 +
16 files changed, 625 insertions(+), 70 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5] x86: detect Intel CDP feature
2015-09-02 8:27 [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling He Chen
@ 2015-09-02 8:27 ` He Chen
2015-09-02 11:12 ` Andrew Cooper
2015-09-02 8:27 ` [PATCH 2/5] x86: Support enable/disable CDP dynamically and get CDP status He Chen
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: He Chen @ 2015-09-02 8:27 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, keir
Detect Intel Code/Data Prioritization(CDP) feature and store cpuid
information for later use. CDP feature is based on CAT, note that all
sockets in the platform must have CDP either enabled or disabled, not
a mix. cdp_socket_avail saves CDP capability of every socket so that
we can determine if CDP is supported in the platform.
Signed-off-by: He Chen <he.chen@linux.intel.com>
---
xen/arch/x86/psr.c | 13 ++++++++++++-
xen/include/asm-x86/psr.h | 4 ++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index c0daa2e..b357816 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -43,6 +43,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_avail;
static unsigned int __initdata opt_psr;
static unsigned int __initdata opt_rmid_max = 255;
@@ -498,6 +499,13 @@ static void cat_cpu_init(void)
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 )
+ {
+ set_bit(socket, cdp_socket_avail);
+ printk(XENLOG_INFO "CDP: available on socket %u\n", socket);
+ }
+
}
static void cat_cpu_fini(unsigned int cpu)
@@ -523,6 +531,8 @@ static void __init psr_cat_free(void)
cat_socket_enable = NULL;
xfree(cat_socket_info);
cat_socket_info = NULL;
+ xfree(cdp_socket_avail);
+ cdp_socket_avail = NULL;
}
static void __init init_psr_cat(void)
@@ -535,8 +545,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_avail = 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_avail )
psr_cat_free();
}
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 081750f..a6b83df 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -27,6 +27,10 @@
/* L3 Monitoring Features */
#define PSR_CMT_L3_OCCUPANCY 0x1
+/* CDP Capability */
+#define PSR_CAT_CDP_CAPABILITY 0x4
+
+
struct psr_cmt_l3 {
unsigned int features;
unsigned int upscaling_factor;
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/5] x86: Support enable/disable CDP dynamically and get CDP status
2015-09-02 8:27 [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling He Chen
2015-09-02 8:27 ` [PATCH 1/5] x86: detect Intel CDP feature He Chen
@ 2015-09-02 8:27 ` He Chen
2015-09-02 11:39 ` Andrew Cooper
2015-09-02 8:28 ` [PATCH 3/5] x86: add domctl cmd to set/get CDP code/data CBM He Chen
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: He Chen @ 2015-09-02 8:27 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, keir
cdp_enabled is added to CAT socket info to indicate CDP is on or off on
the socket and struct psr_cat_cbm is extended to support CDP operation.
IA32_L3_QOS_CFG is a new MSR to enable/disable L3 CDP, when enable or
disable CDP, all domains will reset to COS0 with fully access all L3
cache.
Signed-off-by: He Chen <he.chen@linux.intel.com>
---
xen/arch/x86/psr.c | 164 ++++++++++++++++++++++++++++++++++++++++----
xen/arch/x86/sysctl.c | 9 ++-
xen/include/asm-x86/psr.h | 10 ++-
xen/include/public/sysctl.h | 5 ++
4 files changed, 174 insertions(+), 14 deletions(-)
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index b357816..26596dd 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -17,13 +17,20 @@
#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)
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 +39,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 {
@@ -263,7 +271,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, uint8_t *cdp_enabled)
{
struct psr_cat_socket_info *info = get_cat_socket_info(socket);
@@ -272,6 +280,7 @@ int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
*cbm_len = info->cbm_len;
*cos_max = info->cos_max;
+ *cdp_enabled = (uint8_t)info->cdp_enabled;
return 0;
}
@@ -283,7 +292,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;
}
@@ -314,19 +323,33 @@ 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_mode;
};
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_mode == 0 )
+ wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos), info->cbm_code);
+ else
+ {
+ wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos*2+1), info->cbm_code);
+ wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos*2), info->cbm_data);
+ }
}
-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_mode)
{
- 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_mode = cdp_mode,
+ };
if ( socket == cpu_to_socket(smp_processor_id()) )
do_write_l3_cbm(&info);
@@ -364,7 +387,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) )
{
@@ -388,16 +411,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++;
@@ -491,7 +514,7 @@ 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);
@@ -556,6 +579,123 @@ static int psr_cpu_prepare(unsigned int cpu)
return cat_cpu_prepare(cpu);
}
+static void do_write_l3_config(void *data)
+{
+ uint64_t val;
+ uint64_t *mask = data;
+
+ rdmsrl(PSR_L3_QOS_CFG, val);
+ wrmsrl(PSR_L3_QOS_CFG, val | *mask);
+}
+
+static int config_socket_l3_cdp(unsigned int socket, uint64_t cdp_mask)
+{
+ unsigned int cpu;
+ uint64_t mask = cdp_mask;
+
+ if ( socket == cpu_to_socket(smp_processor_id()) )
+ do_write_l3_config(&mask);
+ else
+ {
+ cpu = get_socket_cpu(socket);
+ if ( cpu >= nr_cpu_ids )
+ return -ENOTSOCK;
+ on_selected_cpus(cpumask_of(cpu), do_write_l3_config, &mask, 1);
+ }
+ return 0;
+}
+
+int psr_cat_enable_cdp(void)
+{
+ int i, ret;
+ struct psr_cat_socket_info *info;
+ unsigned int socket;
+ struct domain *d;
+
+ for_each_set_bit(socket, cat_socket_enable, nr_sockets)
+ {
+ ret = config_socket_l3_cdp(socket, 1 << PSR_L3_QOS_CDP_ENABLE_BIT);
+ if ( ret )
+ return ret;
+
+ info = cat_socket_info + socket;
+ if (info->cdp_enabled)
+ return 0;
+
+ /* Cut half of cos_max when CDP enabled */
+ info->cos_max = info->cos_max / 2;
+
+ spin_lock(&info->cbm_lock);
+
+ /* Reset COS0 code CBM & data CBM for all domain */
+ 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;
+
+ for ( i = 0; i <= info->cos_max; i++ )
+ info->cos_to_cbm[0].ref = 0;
+
+ /* Only write mask1 since mask0 is always all 1 by CAT. */
+ ret = write_l3_cbm(socket, 1, info->cos_to_cbm[0].u.cdp.code, 0, 0);
+ if ( ret )
+ {
+ spin_unlock(&info->cbm_lock);
+ return ret;
+ }
+
+ /* Reset all domain to COS0 */
+ for_each_domain( d )
+ {
+ d->arch.psr_cos_ids[socket] = 0;
+ info->cos_to_cbm[0].ref++;
+ }
+
+ spin_unlock(&info->cbm_lock);
+ info->cdp_enabled = 1;
+ }
+
+ return 0;
+}
+
+int psr_cat_disable_cdp(void)
+{
+ int i, ret;
+ struct psr_cat_socket_info *info;
+ unsigned int socket;
+ struct domain *d;
+
+ for_each_set_bit(socket, cat_socket_enable, nr_sockets)
+ {
+ ret = config_socket_l3_cdp(socket, 0 << PSR_L3_QOS_CDP_ENABLE_BIT);
+ if ( ret )
+ return ret;
+
+ info = cat_socket_info + socket;
+ if( !info->cdp_enabled )
+ return 0;
+
+ /* Restore CAT cos_max when CDP disabled */
+ info->cos_max = info->cos_max * 2 + 1;
+ spin_lock(&info->cbm_lock);
+
+ /* Reset all CBMs and set fully open COS0 for all domains */
+ info->cos_to_cbm[0].u.cbm = (1ull << info->cbm_len) - 1;
+ for ( i = 0; i <= info->cos_max; i++ )
+ info->cos_to_cbm[i].ref = 0;
+
+ /* Reset all domains to COS0 */
+ for_each_domain( d )
+ {
+ d->arch.psr_cos_ids[socket] = 0;
+ info->cos_to_cbm[0].ref++;
+ }
+
+ spin_unlock(&info->cbm_lock);
+ info->cdp_enabled = 0;
+ }
+
+ return 0;
+}
+
static void psr_cpu_init(void)
{
if ( cat_socket_info )
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index f36b52f..51b03a4 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -177,12 +177,19 @@ 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.cdp_enabled);
if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) )
ret = -EFAULT;
break;
+ case XEN_SYSCTL_PSR_CAT_enable_cdp:
+ ret = psr_cat_enable_cdp();
+ break;
+ case XEN_SYSCTL_PSR_CAT_disable_cdp:
+ ret = psr_cat_disable_cdp();
+ break;
default:
ret = -EOPNOTSUPP;
break;
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index a6b83df..2b79a92 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -30,6 +30,11 @@
/* CDP Capability */
#define PSR_CAT_CDP_CAPABILITY 0x4
+/* L3 QoS Config MSR Address */
+#define PSR_L3_QOS_CFG 0xc81
+
+/* L3 CDP Enable bit*/
+#define PSR_L3_QOS_CDP_ENABLE_BIT 0x0
struct psr_cmt_l3 {
unsigned int features;
@@ -56,13 +61,16 @@ 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, uint8_t *cdp_enabled);
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_domain_init(struct domain *d);
void psr_domain_free(struct domain *d);
+int psr_cat_enable_cdp(void);
+int psr_cat_disable_cdp(void);
+
#endif /* __ASM_PSR_H__ */
/*
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 58c9be2..8b97137 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -697,6 +697,8 @@ 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_enable_cdp 1
+#define XEN_SYSCTL_PSR_CAT_disable_cdp 2
struct xen_sysctl_psr_cat_op {
uint32_t cmd; /* IN: XEN_SYSCTL_PSR_CAT_* */
uint32_t target; /* IN */
@@ -704,6 +706,9 @@ 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_CDP_DISABLED 0
+ #define XEN_SYSCTL_PSR_CDP_ENABLED 1
+ uint8_t cdp_enabled; /* OUT: CDP status */
} l3_info;
} u;
};
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/5] x86: add domctl cmd to set/get CDP code/data CBM
2015-09-02 8:27 [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling He Chen
2015-09-02 8:27 ` [PATCH 1/5] x86: detect Intel CDP feature He Chen
2015-09-02 8:27 ` [PATCH 2/5] x86: Support enable/disable CDP dynamically and get CDP status He Chen
@ 2015-09-02 8:28 ` He Chen
2015-09-02 11:59 ` Andrew Cooper
2015-09-02 8:28 ` [PATCH 4/5] tools: add tools support for Intel CDP He Chen
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: He Chen @ 2015-09-02 8:28 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, 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 | 33 +++++++++-
xen/arch/x86/psr.c | 142 ++++++++++++++++++++++++++++++++------------
xen/include/asm-x86/psr.h | 12 +++-
xen/include/public/domctl.h | 4 ++
4 files changed, 150 insertions(+), 41 deletions(-)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index bf62a88..c2d771e 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1167,12 +1167,41 @@ 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 26596dd..8e92d24 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -285,14 +285,20 @@ 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 )
+ *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
+ else if ( type == PSR_CBM_TYPE_L3_CODE )
+ *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
+ else
+ *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
return 0;
}
@@ -365,10 +371,51 @@ 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 exist_same_cos(struct psr_cat_cbm *map, int cos_max,
+ uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
+{
+ 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 -EOVERFLOW;
+}
+
+int psr_set_l3_cbm(struct domain *d, unsigned int socket,
+ uint64_t cbm, enum cbm_type type)
{
- unsigned int old_cos, cos;
- struct psr_cat_cbm *map, *found = NULL;
+ unsigned int old_cos, cos_max;
+ int cos, ret, need_write = 1;
+ uint64_t cbm_data, cbm_code;
+ struct psr_cat_cbm *map;
struct psr_cat_socket_info *info = get_cat_socket_info(socket);
if ( IS_ERR(info) )
@@ -377,53 +424,74 @@ int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
if ( !psr_check_cbm(info->cbm_len, cbm) )
return -EINVAL;
+ cos_max = info->cos_max;
+ if ( info->cdp_enabled )
+ cos_max = cos_max / 2;
+
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:
+ cbm_code = cbm;
+ cbm_data = cbm;
break;
- }
+ case PSR_CBM_TYPE_L3_CODE:
+ cbm_code = cbm;
+ cbm_data = map[old_cos].u.cdp.data;
+ break;
+ case PSR_CBM_TYPE_L3_DATA:
+ cbm_code = map[old_cos].u.cdp.code;
+ cbm_data = cbm;
+ break;
+ default:
+ return -EINVAL;
}
- /* If old cos is referred only by the domain, then use it. */
- if ( !found && map[old_cos].ref == 1 )
- found = map + old_cos;
-
- if ( !found )
+ spin_lock(&info->cbm_lock);
+ cos = exist_same_cos(map, cos_max, cbm_code, cbm_data, info->cdp_enabled);
+ if ( cos >= 0 )
{
- spin_unlock(&info->cbm_lock);
- return -EOVERFLOW;
+ if ( unlikely(cos == old_cos) )
+ {
+ spin_unlock(&info->cbm_lock);
+ return 0;
+ }
}
-
- cos = found - map;
- if ( found->u.cbm != cbm )
+ else
{
- int ret = write_l3_cbm(socket, cos, cbm, 0, 0);
-
- if ( ret )
+ cos = pick_avail_cos(map, cos_max, old_cos);
+ if ( cos < 0 )
{
spin_unlock(&info->cbm_lock);
- return ret;
+ return -EOVERFLOW;
+ }
+
+ /* 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 2b79a92..3af4ac2 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, uint8_t *cdp_enabled);
-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] 17+ messages in thread
* [PATCH 4/5] tools: add tools support for Intel CDP
2015-09-02 8:27 [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling He Chen
` (2 preceding siblings ...)
2015-09-02 8:28 ` [PATCH 3/5] x86: add domctl cmd to set/get CDP code/data CBM He Chen
@ 2015-09-02 8:28 ` He Chen
2015-09-02 13:32 ` Wei Liu
2015-09-02 8:28 ` [PATCH 5/5] docs: add document to introduce CDP command He Chen
2015-09-02 12:08 ` [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling Andrew Cooper
5 siblings, 1 reply; 17+ messages in thread
From: He Chen @ 2015-09-02 8:28 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, keir
This is the xc/xl changes to support Intel Code/Data Prioritization.
Two new xl commands are introduced to enable/disable CDP dynamically,
and 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 | 10 ++++--
tools/libxc/xc_psr.c | 42 +++++++++++++++++++++-
tools/libxl/libxl.h | 12 +++++++
tools/libxl/libxl_psr.c | 64 +++++++++++++++++++++++++++++++++-
tools/libxl/libxl_types.idl | 3 ++
tools/libxl/xl.h | 4 +++
tools/libxl/xl_cmdimpl.c | 81 +++++++++++++++++++++++++++++++++++++------
tools/libxl/xl_cmdtable.c | 15 ++++++++
8 files changed, 217 insertions(+), 14 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index de3c0ad..665e6bd 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2798,7 +2798,10 @@ 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 +2827,10 @@ 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,
+ uint8_t *cdp_enabled);
+int xc_psr_cat_enable_cdp(xc_interface *xch);
+int xc_psr_cat_disable_cdp(xc_interface *xch);
#endif
#endif /* XENCTRL_H */
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index d8b3a51..d4ff6f6 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,
+ uint8_t *cdp_enabled)
{
int rc;
DECLARE_SYSCTL;
@@ -320,11 +333,38 @@ 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.cdp_enabled;
}
return rc;
}
+int xc_psr_cat_enable_cdp(xc_interface *xch)
+{
+ int rc;
+ DECLARE_SYSCTL;
+
+ sysctl.cmd = XEN_SYSCTL_psr_cat_op;
+ sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_enable_cdp;
+
+ rc = do_sysctl(xch, &sysctl);
+
+ return rc;
+}
+
+int xc_psr_cat_disable_cdp(xc_interface *xch)
+{
+ int rc;
+ DECLARE_SYSCTL;
+
+ sysctl.cmd = XEN_SYSCTL_psr_cat_op;
+ sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_disable_cdp;
+
+ rc = do_sysctl(xch, &sysctl);
+
+ return rc;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5f9047c..68c14fb 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,11 @@ 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
+int libxl_psr_cat_enable_cdp(libxl_ctx *ctx);
+int libxl_psr_cat_disable_cdp(libxl_ctx *ctx);
+#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..4dae5e5 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,6 +305,22 @@ 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) {
+ if (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;
+ goto out;
+ }
+ }
+
libxl_for_each_set_bit(socketid, *target_map) {
if (socketid >= nr_sockets)
break;
@@ -352,7 +369,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);
@@ -376,6 +393,51 @@ void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr)
free(list);
}
+static void libxl__psr_cdp_log_err_msg(libxl__gc *gc, int err)
+{
+ char *msg;
+
+ switch (err) {
+ case ENODEV:
+ msg = "CDP is not supported in this system";
+ break;
+
+ default:
+ libxl__psr_log_err_msg(gc, err);
+ return;
+ }
+
+ LOGE(ERROR, "%s", msg);
+}
+
+int libxl_psr_cat_enable_cdp(libxl_ctx *ctx)
+{
+ GC_INIT(ctx);
+ int rc = 0;
+
+ if (xc_psr_cat_enable_cdp(ctx->xch)) {
+ libxl__psr_cdp_log_err_msg(gc, errno);
+ rc = ERROR_FAIL;
+ }
+
+ GC_FREE;
+ return rc;
+}
+
+int libxl_psr_cat_disable_cdp(libxl_ctx *ctx)
+{
+ GC_INIT(ctx);
+
+ int rc = 0;
+ if (xc_psr_cat_disable_cdp(ctx->xch)) {
+ libxl__psr_cdp_log_err_msg(gc, errno);
+ rc = ERROR_FAIL;
+ }
+
+ GC_FREE;
+ return rc;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ef346e7..afed626 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", uint8),
])
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 13bccba..e8bb774 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -121,6 +121,10 @@ 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
+int main_psr_cat_cdp_enable(int argc, char **argv);
+int main_psr_cat_cdp_disable(int argc, char **argv);
+#endif
void help(const char *command);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index ebbb9a5..825d707 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8366,6 +8366,15 @@ int main_psr_cmt_show(int argc, char **argv)
#endif
#ifdef LIBXL_HAVE_PSR_CAT
+static void psr_cat_print_cdp_status(uint8_t status)
+{
+ if (status == 0)
+ printf("%-16s: Disabled\n", "CDP Status");
+ else
+ printf("%-16s: Enabled\n", "CDP Status");
+}
+
+
static int psr_cat_hwinfo(void)
{
int rc;
@@ -8390,6 +8399,7 @@ static int psr_cat_hwinfo(void)
}
printf("%-16s: %u\n", "Socket ID", socketid);
printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
+ psr_cat_print_cdp_status(info->cdp_enabled);
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 +8411,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,
+ uint8_t cdp_enabled)
{
char *domain_name;
uint64_t cbm;
@@ -8410,20 +8421,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,
+ uint8_t 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 +8453,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 +8477,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 +8509,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 +8519,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 +8541,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 +8620,32 @@ int main_psr_hwinfo(int argc, char **argv)
return ret;
}
+#ifdef LIBXL_HAVE_PSR_CDP
+int main_psr_cat_cdp_enable(int argc, char **argv)
+{
+ int ret;
+
+ ret = libxl_psr_cat_enable_cdp(ctx);
+ if (ret)
+ return ret;
+ printf("CDP is enabled\n");
+
+ return 0;
+}
+
+int main_psr_cat_cdp_disable(int argc, char **argv)
+{
+ int ret;
+
+ ret = libxl_psr_cat_disable_cdp(ctx);
+ if (ret)
+ return ret;
+ printf("CDP is disabled\n");
+
+ return 0;
+}
+#endif
+
#endif
/*
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 0071f12..2065195 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,19 @@ struct cmd_spec cmd_table[] = {
},
#endif
+
+#ifdef LIBXL_HAVE_PSR_CDP
+ { "psr-cat-cdp-enable",
+ &main_psr_cat_cdp_enable, 0, 1,
+ "Enable Code/Data Prioritization",
+ "",
+ },
+ { "psr-cat-cdp-disable",
+ &main_psr_cat_cdp_disable, 0, 1,
+ "Disable Code/Data Prioritization",
+ "",
+ },
+#endif
};
int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/5] docs: add document to introduce CDP command
2015-09-02 8:27 [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling He Chen
` (3 preceding siblings ...)
2015-09-02 8:28 ` [PATCH 4/5] tools: add tools support for Intel CDP He Chen
@ 2015-09-02 8:28 ` He Chen
2015-09-02 13:32 ` Wei Liu
2015-09-02 12:08 ` [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling Andrew Cooper
5 siblings, 1 reply; 17+ messages in thread
From: He Chen @ 2015-09-02 8:28 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
He Chen, ian.jackson, jbeulich, 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 | 22 +++++++++++++++++++
docs/misc/xl-psr.markdown | 56 ++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 73 insertions(+), 5 deletions(-)
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index f22c3f3..3d7bde6 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,12 +1551,28 @@ 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>]
Show CAT settings for a certain domain or all domains.
+=item B<psr-cat-cdp-enable>
+
+Enable Code/Data Prioritization.
+
+=item B<psr-cat-cdp-disable>
+
+Disable Code/Data Prioritization.
+
=back
=head1 TO BE DOCUMENTED
diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
index 3545912..a3728a9 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,48 @@ 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 on.
+
+Further more, if enabling/disabling CDP dynamically on runtime, all domains
+are reset to COS[0] with fully access to L3 cache before enabling or disabling
+CDP.
+
+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 +150,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).
@@ -127,6 +165,14 @@ Per domain CBM settings can be shown by:
`xl psr-cat-show`
+To enable CDP on the platform:
+
+`xl psr-cat-cdp-enable`
+
+To disable CDP on the platform:
+
+`xl psr-cat-cdp-disable`
+
## Reference
[1] Intel SDM
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] x86: detect Intel CDP feature
2015-09-02 8:27 ` [PATCH 1/5] x86: detect Intel CDP feature He Chen
@ 2015-09-02 11:12 ` Andrew Cooper
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-09-02 11:12 UTC (permalink / raw)
To: He Chen, xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, ian.jackson, jbeulich, keir
On 02/09/15 09:27, He Chen wrote:
> Detect Intel Code/Data Prioritization(CDP) feature and store cpuid
> information for later use. CDP feature is based on CAT, note that all
> sockets in the platform must have CDP either enabled or disabled, not
> a mix. cdp_socket_avail saves CDP capability of every socket so that
> we can determine if CDP is supported in the platform.
>
> Signed-off-by: He Chen <he.chen@linux.intel.com>
> ---
> xen/arch/x86/psr.c | 13 ++++++++++++-
> xen/include/asm-x86/psr.h | 4 ++++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index c0daa2e..b357816 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -43,6 +43,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_avail;
>
> static unsigned int __initdata opt_psr;
> static unsigned int __initdata opt_rmid_max = 255;
> @@ -498,6 +499,13 @@ static void cat_cpu_init(void)
> 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 )
> + {
> + set_bit(socket, cdp_socket_avail);
> + printk(XENLOG_INFO "CDP: available on socket %u\n", socket);
> + }
> +
Extraneous newline.
> }
>
> static void cat_cpu_fini(unsigned int cpu)
> @@ -523,6 +531,8 @@ static void __init psr_cat_free(void)
> cat_socket_enable = NULL;
> xfree(cat_socket_info);
> cat_socket_info = NULL;
> + xfree(cdp_socket_avail);
> + cdp_socket_avail = NULL;
> }
>
> static void __init init_psr_cat(void)
> @@ -535,8 +545,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_avail = 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_avail )
> psr_cat_free();
> }
>
> diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
> index 081750f..a6b83df 100644
> --- a/xen/include/asm-x86/psr.h
> +++ b/xen/include/asm-x86/psr.h
> @@ -27,6 +27,10 @@
> /* L3 Monitoring Features */
> #define PSR_CMT_L3_OCCUPANCY 0x1
>
> +/* CDP Capability */
> +#define PSR_CAT_CDP_CAPABILITY 0x4
Please use (1u << 2) so it is more obvious which bit is in use.
With these two bits things fixed, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] x86: Support enable/disable CDP dynamically and get CDP status
2015-09-02 8:27 ` [PATCH 2/5] x86: Support enable/disable CDP dynamically and get CDP status He Chen
@ 2015-09-02 11:39 ` Andrew Cooper
2015-09-02 14:07 ` Jan Beulich
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2015-09-02 11:39 UTC (permalink / raw)
To: He Chen, xen-devel
Cc: keir, ian.campbell, stefano.stabellini, ian.jackson, jbeulich, wei.liu2
On 02/09/15 09:27, He Chen wrote:
> cdp_enabled is added to CAT socket info to indicate CDP is on or off on
> the socket and struct psr_cat_cbm is extended to support CDP operation.
> IA32_L3_QOS_CFG is a new MSR to enable/disable L3 CDP, when enable or
> disable CDP, all domains will reset to COS0 with fully access all L3
> cache.
>
> Signed-off-by: He Chen <he.chen@linux.intel.com>
> ---
> xen/arch/x86/psr.c | 164 ++++++++++++++++++++++++++++++++++++++++----
> xen/arch/x86/sysctl.c | 9 ++-
> xen/include/asm-x86/psr.h | 10 ++-
> xen/include/public/sysctl.h | 5 ++
> 4 files changed, 174 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index b357816..26596dd 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -17,13 +17,20 @@
> #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)
>
> struct psr_cat_cbm {
> - uint64_t cbm;
> + union {
> + uint64_t cbm;
> + struct {
> + uint64_t code;
> + uint64_t data;
> + }cdp;
> + }u;
Spaces after } please.
> unsigned int ref;
> };
>
> @@ -32,6 +39,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 {
> @@ -263,7 +271,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, uint8_t *cdp_enabled)
bool_t *cdp_enabled which...
> {
> struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>
> @@ -272,6 +280,7 @@ int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
>
> *cbm_len = info->cbm_len;
> *cos_max = info->cos_max;
> + *cdp_enabled = (uint8_t)info->cdp_enabled;
... avoids this typecast.
>
> return 0;
> }
> @@ -283,7 +292,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;
> }
> @@ -314,19 +323,33 @@ 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_mode;
This should either be plain "cdp" which would be fine, or
"cdp_enabled". The name "_mode" here is misleading.
> };
>
> 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_mode == 0 )
> + wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos), info->cbm_code);
> + else
> + {
> + wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos*2+1), info->cbm_code);
> + wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos*2), info->cbm_data);
Please introduce
MSR_IA32_PSR_L3_MASK_CBM_CODE()
MSR_IA32_PSR_L3_MASK_CBM_DATA()
to abstract away the x * 2 + 1 calculation.
> + }
> }
>
> -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_mode)
> {
> - struct cos_cbm_info info = { .cos = cos, .cbm = cbm };
> + struct cos_cbm_info info =
> + { .cos = cos,
Newline here please, so
{
.cos
> + .cbm_code = cbm_code,
> + .cbm_data = cbm_data,
> + .cdp_mode = cdp_mode,
> + };
>
> if ( socket == cpu_to_socket(smp_processor_id()) )
> do_write_l3_cbm(&info);
> @@ -364,7 +387,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) )
> {
> @@ -388,16 +411,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++;
> @@ -491,7 +514,7 @@ 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);
>
> @@ -556,6 +579,123 @@ static int psr_cpu_prepare(unsigned int cpu)
> return cat_cpu_prepare(cpu);
> }
>
> +static void do_write_l3_config(void *data)
> +{
> + uint64_t val;
> + uint64_t *mask = data;
> +
> + rdmsrl(PSR_L3_QOS_CFG, val);
> + wrmsrl(PSR_L3_QOS_CFG, val | *mask);
> +}
> +
> +static int config_socket_l3_cdp(unsigned int socket, uint64_t cdp_mask)
> +{
> + unsigned int cpu;
> + uint64_t mask = cdp_mask;
> +
> + if ( socket == cpu_to_socket(smp_processor_id()) )
> + do_write_l3_config(&mask);
> + else
> + {
> + cpu = get_socket_cpu(socket);
> + if ( cpu >= nr_cpu_ids )
> + return -ENOTSOCK;
> + on_selected_cpus(cpumask_of(cpu), do_write_l3_config, &mask, 1);
> + }
> + return 0;
> +}
> +
> +int psr_cat_enable_cdp(void)
> +{
> + int i, ret;
> + struct psr_cat_socket_info *info;
> + unsigned int socket;
> + struct domain *d;
> +
> + for_each_set_bit(socket, cat_socket_enable, nr_sockets)
> + {
> + ret = config_socket_l3_cdp(socket, 1 << PSR_L3_QOS_CDP_ENABLE_BIT);
> + if ( ret )
> + return ret;
> +
> + info = cat_socket_info + socket;
> + if (info->cdp_enabled)
Style. spaces inside brakces.
> + return 0;
> +
> + /* Cut half of cos_max when CDP enabled */
> + info->cos_max = info->cos_max / 2;
> +
> + spin_lock(&info->cbm_lock);
> +
> + /* Reset COS0 code CBM & data CBM for all domain */
> + 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;
> +
> + for ( i = 0; i <= info->cos_max; i++ )
> + info->cos_to_cbm[0].ref = 0;
> +
> + /* Only write mask1 since mask0 is always all 1 by CAT. */
> + ret = write_l3_cbm(socket, 1, info->cos_to_cbm[0].u.cdp.code, 0, 0);
> + if ( ret )
> + {
> + spin_unlock(&info->cbm_lock);
> + return ret;
> + }
> +
> + /* Reset all domain to COS0 */
> + for_each_domain( d )
> + {
> + d->arch.psr_cos_ids[socket] = 0;
> + info->cos_to_cbm[0].ref++;
This is a long running operation and must not be done synchronously like
this. Unfortunately, it is not easy to make restartable.
I think it would be perfectly reasonable to have cdp as a boot time
switch only, and have no ability to change it at runtime. I don't see a
reasonable case to change it dynamically at runtime; users will either
want to use it, or not.
Making this a boot-time choice (i.e. psr=cat,cdp) removes all of this
re-juggling logic, and simplifies things greatly.
Thoughts?
> + }
> +
> + spin_unlock(&info->cbm_lock);
> + info->cdp_enabled = 1;
> + }
> +
> + return 0;
> +}
> +
> +int psr_cat_disable_cdp(void)
> +{
> + int i, ret;
> + struct psr_cat_socket_info *info;
> + unsigned int socket;
> + struct domain *d;
> +
> + for_each_set_bit(socket, cat_socket_enable, nr_sockets)
> + {
> + ret = config_socket_l3_cdp(socket, 0 << PSR_L3_QOS_CDP_ENABLE_BIT);
> + if ( ret )
> + return ret;
> +
> + info = cat_socket_info + socket;
> + if( !info->cdp_enabled )
> + return 0;
> +
> + /* Restore CAT cos_max when CDP disabled */
> + info->cos_max = info->cos_max * 2 + 1;
> + spin_lock(&info->cbm_lock);
> +
> + /* Reset all CBMs and set fully open COS0 for all domains */
> + info->cos_to_cbm[0].u.cbm = (1ull << info->cbm_len) - 1;
> + for ( i = 0; i <= info->cos_max; i++ )
> + info->cos_to_cbm[i].ref = 0;
> +
> + /* Reset all domains to COS0 */
> + for_each_domain( d )
> + {
> + d->arch.psr_cos_ids[socket] = 0;
> + info->cos_to_cbm[0].ref++;
> + }
> +
> + spin_unlock(&info->cbm_lock);
> + info->cdp_enabled = 0;
> + }
> +
> + return 0;
> +}
> +
> static void psr_cpu_init(void)
> {
> if ( cat_socket_info )
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index f36b52f..51b03a4 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -177,12 +177,19 @@ 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.cdp_enabled);
>
> if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) )
> ret = -EFAULT;
>
> break;
> + case XEN_SYSCTL_PSR_CAT_enable_cdp:
> + ret = psr_cat_enable_cdp();
> + break;
> + case XEN_SYSCTL_PSR_CAT_disable_cdp:
> + ret = psr_cat_disable_cdp();
> + break;
I realise you are copying the existing (incorrect) style, but please
introduce a blank line between break statements and the subsequent
case/default label. It makes the switch statement far easier to read.
> default:
> ret = -EOPNOTSUPP;
> break;
> diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
> index a6b83df..2b79a92 100644
> --- a/xen/include/asm-x86/psr.h
> +++ b/xen/include/asm-x86/psr.h
> @@ -30,6 +30,11 @@
> /* CDP Capability */
> #define PSR_CAT_CDP_CAPABILITY 0x4
>
> +/* L3 QoS Config MSR Address */
> +#define PSR_L3_QOS_CFG 0xc81
> +
> +/* L3 CDP Enable bit*/
> +#define PSR_L3_QOS_CDP_ENABLE_BIT 0x0
>
> struct psr_cmt_l3 {
> unsigned int features;
> @@ -56,13 +61,16 @@ 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, uint8_t *cdp_enabled);
> 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_domain_init(struct domain *d);
> void psr_domain_free(struct domain *d);
>
> +int psr_cat_enable_cdp(void);
> +int psr_cat_disable_cdp(void);
> +
> #endif /* __ASM_PSR_H__ */
>
> /*
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 58c9be2..8b97137 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -697,6 +697,8 @@ 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_enable_cdp 1
> +#define XEN_SYSCTL_PSR_CAT_disable_cdp 2
> struct xen_sysctl_psr_cat_op {
> uint32_t cmd; /* IN: XEN_SYSCTL_PSR_CAT_* */
> uint32_t target; /* IN */
> @@ -704,6 +706,9 @@ 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_CDP_DISABLED 0
> + #define XEN_SYSCTL_PSR_CDP_ENABLED 1
> + uint8_t cdp_enabled; /* OUT: CDP status */
Rather than using a whole 8 bits for a boolean, and leaving alignment
fun for the next person to edit this structure, may I recommend
#define XEN_SYSCTL_PSR_CAT_L3_CDP (1u << 0)
uint32_t flags;
Instead. (Also assumes that in the future we might get CDP at the L4 cache)
~Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] x86: add domctl cmd to set/get CDP code/data CBM
2015-09-02 8:28 ` [PATCH 3/5] x86: add domctl cmd to set/get CDP code/data CBM He Chen
@ 2015-09-02 11:59 ` Andrew Cooper
2015-09-06 7:15 ` He Chen
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2015-09-02 11:59 UTC (permalink / raw)
To: He Chen, xen-devel
Cc: keir, ian.campbell, stefano.stabellini, ian.jackson, jbeulich, wei.liu2
On 02/09/15 09:28, 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 | 33 +++++++++-
> xen/arch/x86/psr.c | 142 ++++++++++++++++++++++++++++++++------------
> xen/include/asm-x86/psr.h | 12 +++-
> xen/include/public/domctl.h | 4 ++
> 4 files changed, 150 insertions(+), 41 deletions(-)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index bf62a88..c2d771e 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1167,12 +1167,41 @@ 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 26596dd..8e92d24 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -285,14 +285,20 @@ 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 )
> + *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
> + else if ( type == PSR_CBM_TYPE_L3_CODE )
> + *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
> + else
> + *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
>
You should check whether cdp is enabled, and reject incorrect update
types. -EXDEV is probably an appropriate error.
It is not valid to get/set PSR_CBM_TYPE_L3 if cdp is enabled, just as it
is not valid to get/set PSR_CBM_TYPE_L3_{CODE,DATA} if cdp is not enabled.
> return 0;
> }
> @@ -365,10 +371,51 @@ 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 exist_same_cos(struct psr_cat_cbm *map, int cos_max,
"exists_same_cos" is a strange way of phrasing this in english. May I
suggest "find_cos" instead.
> + uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
> +{
> + int cos;
cos and cos_max is sourced from unsigned data higher in the callchain.
They must remain unsigned here as well.
> +
> + 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 -EOVERFLOW;
ENOENT surely, or use EOVERFLOW consistently.
> +}
> +
> +int psr_set_l3_cbm(struct domain *d, unsigned int socket,
> + uint64_t cbm, enum cbm_type type)
> {
> - unsigned int old_cos, cos;
> - struct psr_cat_cbm *map, *found = NULL;
> + unsigned int old_cos, cos_max;
> + int cos, ret, need_write = 1;
need_write should be bool_t.
> + uint64_t cbm_data, cbm_code;
> + struct psr_cat_cbm *map;
> struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>
> if ( IS_ERR(info) )
> @@ -377,53 +424,74 @@ int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
> if ( !psr_check_cbm(info->cbm_len, cbm) )
> return -EINVAL;
>
> + cos_max = info->cos_max;
> + if ( info->cdp_enabled )
> + cos_max = cos_max / 2;
> +
> 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)
Style - spaces inside braces.
> {
> - /* 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:
As with the get side, you need to check cdp and reject an incorrect type.
> + cbm_code = cbm;
> + cbm_data = cbm;
> break;
> - }
As with other switch statements, please have a newline between break;
and case/default:
> + case PSR_CBM_TYPE_L3_CODE:
> + cbm_code = cbm;
> + cbm_data = map[old_cos].u.cdp.data;
> + break;
> + case PSR_CBM_TYPE_L3_DATA:
> + cbm_code = map[old_cos].u.cdp.code;
> + cbm_data = cbm;
> + break;
> + default:
> + return -EINVAL;
> }
>
> - /* If old cos is referred only by the domain, then use it. */
> - if ( !found && map[old_cos].ref == 1 )
> - found = map + old_cos;
> -
> - if ( !found )
> + spin_lock(&info->cbm_lock);
> + cos = exist_same_cos(map, cos_max, cbm_code, cbm_data, info->cdp_enabled);
> + if ( cos >= 0 )
> {
> - spin_unlock(&info->cbm_lock);
> - return -EOVERFLOW;
> + if ( unlikely(cos == old_cos) )
> + {
> + spin_unlock(&info->cbm_lock);
> + return 0;
> + }
> }
> -
> - cos = found - map;
> - if ( found->u.cbm != cbm )
> + else
> {
> - int ret = write_l3_cbm(socket, cos, cbm, 0, 0);
> -
> - if ( ret )
> + cos = pick_avail_cos(map, cos_max, old_cos);
> + if ( cos < 0 )
> {
> spin_unlock(&info->cbm_lock);
> - return ret;
> + return -EOVERFLOW;
Please don't override the error return from pick_avail_cos(). i.e.
"return cos;" here.
~Andrew
> + }
> +
> + /* 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 2b79a92..3af4ac2 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, uint8_t *cdp_enabled);
> -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 */
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling
2015-09-02 8:27 [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling He Chen
` (4 preceding siblings ...)
2015-09-02 8:28 ` [PATCH 5/5] docs: add document to introduce CDP command He Chen
@ 2015-09-02 12:08 ` Andrew Cooper
2015-09-06 1:18 ` Chao Peng
2015-09-06 6:49 ` He Chen
5 siblings, 2 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-09-02 12:08 UTC (permalink / raw)
To: He Chen, xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, ian.jackson, jbeulich, keir
On 02/09/15 09:27, He Chen wrote:
> Hi all,
>
> 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, including
> extending CBM operation functions and introducing new commands to enable/disable
> CDP dynamically. 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.
I have taken a look at patches 1-3. For the most part, it looks good.
The main point I have is on patch 2, as to whether it is sensible to
permit enabling/disabling cdp at runtime. I suggest that it is not
sensible, and should be a command line parameter instead.
If this is agreed as ok going forwards, patches 3 through 5 should
become rather more simple.
~Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] tools: add tools support for Intel CDP
2015-09-02 8:28 ` [PATCH 4/5] tools: add tools support for Intel CDP He Chen
@ 2015-09-02 13:32 ` Wei Liu
0 siblings, 0 replies; 17+ messages in thread
From: Wei Liu @ 2015-09-02 13: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 02, 2015 at 04:28:01PM +0800, He Chen wrote:
> This is the xc/xl changes to support Intel Code/Data Prioritization.
> Two new xl commands are introduced to enable/disable CDP dynamically,
> and 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 | 10 ++++--
> tools/libxc/xc_psr.c | 42 +++++++++++++++++++++-
> tools/libxl/libxl.h | 12 +++++++
> tools/libxl/libxl_psr.c | 64 +++++++++++++++++++++++++++++++++-
> tools/libxl/libxl_types.idl | 3 ++
> tools/libxl/xl.h | 4 +++
> tools/libxl/xl_cmdimpl.c | 81 +++++++++++++++++++++++++++++++++++++------
> tools/libxl/xl_cmdtable.c | 15 ++++++++
> 8 files changed, 217 insertions(+), 14 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index de3c0ad..665e6bd 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2798,7 +2798,10 @@ 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,
> +
Stray blank line.
> };
> typedef enum xc_psr_cat_type xc_psr_cat_type;
>
> @@ -2824,7 +2827,10 @@ 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,
> + uint8_t *cdp_enabled);
Why does it need to be uint8_t? The name suggests bool should be used.
> +int xc_psr_cat_enable_cdp(xc_interface *xch);
> +int xc_psr_cat_disable_cdp(xc_interface *xch);
> #endif
>
> #endif /* XENCTRL_H */
> diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
> index d8b3a51..d4ff6f6 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,
> }
>
[...]
> @@ -304,6 +305,22 @@ 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) {
> + if (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
> + type == LIBXL_PSR_CBM_TYPE_L3_DATA)
You can merge this if statement to parent.
if (!info->cdp_enabled &&
(type == _CODE || type == _DATA))
> + {
> + LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
> + rc = EINVAL;
> + goto out;
> + }
> + }
> +
> libxl_for_each_set_bit(socketid, *target_map) {
> if (socketid >= nr_sockets)
> break;
Need to free memory info points to at the end.
> @@ -352,7 +369,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);
> @@ -376,6 +393,51 @@ void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr)
> free(list);
> }
>
[...]
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index 13bccba..e8bb774 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -121,6 +121,10 @@ 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
> +int main_psr_cat_cdp_enable(int argc, char **argv);
> +int main_psr_cat_cdp_disable(int argc, char **argv);
> +#endif
>
> void help(const char *command);
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index ebbb9a5..825d707 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -8366,6 +8366,15 @@ int main_psr_cmt_show(int argc, char **argv)
> #endif
>
> #ifdef LIBXL_HAVE_PSR_CAT
> +static void psr_cat_print_cdp_status(uint8_t status)
> +{
> + if (status == 0)
> + printf("%-16s: Disabled\n", "CDP Status");
> + else
> + printf("%-16s: Enabled\n", "CDP Status");
> +}
> +
> +
Extraneous blank line.
In fact I don't think this a function is necessary. There is nothing
wrong with embedding a if statement in psr_cat_hwinfo.
> static int psr_cat_hwinfo(void)
> {
> int rc;
> @@ -8390,6 +8399,7 @@ static int psr_cat_hwinfo(void)
> }
> printf("%-16s: %u\n", "Socket ID", socketid);
> printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
> + psr_cat_print_cdp_status(info->cdp_enabled);
> 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 +8411,8 @@ out:
> return rc;
> }
>
[...]
> @@ -8585,6 +8620,32 @@ int main_psr_hwinfo(int argc, char **argv)
> return ret;
> }
>
> +#ifdef LIBXL_HAVE_PSR_CDP
> +int main_psr_cat_cdp_enable(int argc, char **argv)
> +{
> + int ret;
> +
> + ret = libxl_psr_cat_enable_cdp(ctx);
> + if (ret)
> + return ret;
Please log the error / failure.
> + printf("CDP is enabled\n");
> +
> + return 0;
> +}
> +
> +int main_psr_cat_cdp_disable(int argc, char **argv)
> +{
> + int ret;
> +
> + ret = libxl_psr_cat_disable_cdp(ctx);
> + if (ret)
> + return ret;
Ditto.
Wei.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] docs: add document to introduce CDP command
2015-09-02 8:28 ` [PATCH 5/5] docs: add document to introduce CDP command He Chen
@ 2015-09-02 13:32 ` Wei Liu
0 siblings, 0 replies; 17+ messages in thread
From: Wei Liu @ 2015-09-02 13: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 02, 2015 at 04:28:02PM +0800, He Chen wrote:
[...]
> +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
Line wrap. Join next line here.
> +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),
Space before '('.
> +which means the number of available COS will reduce to half when CDP on.
> +
when CDP is on.
I went over this document to get an idea how this feature is supposed to
work and pointed out the obvious defects.
It would better if a native English speaker can go over this.
Wei.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] x86: Support enable/disable CDP dynamically and get CDP status
2015-09-02 11:39 ` Andrew Cooper
@ 2015-09-02 14:07 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2015-09-02 14:07 UTC (permalink / raw)
To: Andrew Cooper, He Chen
Cc: wei.liu2, keir, ian.campbell, stefano.stabellini, ian.jackson, xen-devel
>>> On 02.09.15 at 13:39, <andrew.cooper3@citrix.com> wrote:
> On 02/09/15 09:27, He Chen wrote:
>> + /* Reset all domain to COS0 */
>> + for_each_domain( d )
>> + {
>> + d->arch.psr_cos_ids[socket] = 0;
>> + info->cos_to_cbm[0].ref++;
>
> This is a long running operation and must not be done synchronously like
> this. Unfortunately, it is not easy to make restartable.
>
> I think it would be perfectly reasonable to have cdp as a boot time
> switch only, and have no ability to change it at runtime. I don't see a
> reasonable case to change it dynamically at runtime; users will either
> want to use it, or not.
>
> Making this a boot-time choice (i.e. psr=cat,cdp) removes all of this
> re-juggling logic, and simplifies things greatly.
>
> Thoughts?
FWIW I agree.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling
2015-09-02 12:08 ` [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling Andrew Cooper
@ 2015-09-06 1:18 ` Chao Peng
2015-09-06 6:49 ` He Chen
1 sibling, 0 replies; 17+ messages in thread
From: Chao Peng @ 2015-09-06 1:18 UTC (permalink / raw)
To: Andrew Cooper
Cc: wei.liu2, ian.campbell, stefano.stabellini, He Chen, ian.jackson,
jbeulich, xen-devel, keir
On Wed, Sep 02, 2015 at 01:08:33PM +0100, Andrew Cooper wrote:
> On 02/09/15 09:27, He Chen wrote:
> > Hi all,
> >
> > 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, including
> > extending CBM operation functions and introducing new commands to enable/disable
> > CDP dynamically. 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.
>
> I have taken a look at patches 1-3. For the most part, it looks good.
>
> The main point I have is on patch 2, as to whether it is sensible to
> permit enabling/disabling cdp at runtime. I suggest that it is not
> sensible, and should be a command line parameter instead.
>
> If this is agreed as ok going forwards, patches 3 through 5 should
> become rather more simple.
I guess it would be OK, and a helpful change.
Let's see the next version.
Chao
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling
2015-09-02 12:08 ` [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling Andrew Cooper
2015-09-06 1:18 ` Chao Peng
@ 2015-09-06 6:49 ` He Chen
1 sibling, 0 replies; 17+ messages in thread
From: He Chen @ 2015-09-06 6:49 UTC (permalink / raw)
To: Andrew Cooper
Cc: wei.liu2, ian.campbell, stefano.stabellini, ian.jackson,
jbeulich, xen-devel, keir
On Wed, Sep 02, 2015 at 01:08:33PM +0100, Andrew Cooper wrote:
> On 02/09/15 09:27, He Chen wrote:
> > Hi all,
> >
> > 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, including
> > extending CBM operation functions and introducing new commands to enable/disable
> > CDP dynamically. 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.
>
> I have taken a look at patches 1-3. For the most part, it looks good.
>
> The main point I have is on patch 2, as to whether it is sensible to
> permit enabling/disabling cdp at runtime. I suggest that it is not
> sensible, and should be a command line parameter instead.
>
> If this is agreed as ok going forwards, patches 3 through 5 should
> become rather more simple.
>
> ~Andrew
Thanks for your patient review and valuable suggestions.
About permitting enabling/disabling CDP at runtime, I agree with you to
use command line parameter instead, it really makes code simple and
reliable.
For caution's sake, hardware support confingure CDP dynamically at any
point during normal system operation according to Intel SDM (see section
17.16.2), and that is why I wrote patch 2.
Anyway, since there is few cases to change CDP at runtime, I think it is
better to make this a boot-time parameter. I would resend v2 patch soon
and thanks again~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] x86: add domctl cmd to set/get CDP code/data CBM
2015-09-02 11:59 ` Andrew Cooper
@ 2015-09-06 7:15 ` He Chen
2015-09-06 16:29 ` Andrew Cooper
0 siblings, 1 reply; 17+ messages in thread
From: He Chen @ 2015-09-06 7:15 UTC (permalink / raw)
To: Andrew Cooper
Cc: wei.liu2, ian.campbell, stefano.stabellini, ian.jackson,
jbeulich, xen-devel, keir
On Wed, Sep 02, 2015 at 12:59:07PM +0100, Andrew Cooper wrote:
> On 02/09/15 09:28, 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 | 33 +++++++++-
> > xen/arch/x86/psr.c | 142 ++++++++++++++++++++++++++++++++------------
> > xen/include/asm-x86/psr.h | 12 +++-
> > xen/include/public/domctl.h | 4 ++
> > 4 files changed, 150 insertions(+), 41 deletions(-)
> >
> > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> > index 26596dd..8e92d24 100644
> > --- a/xen/arch/x86/psr.c
> > +++ b/xen/arch/x86/psr.c
> > +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 -EOVERFLOW;
>
> ENOENT surely, or use EOVERFLOW consistently.
>
I am not sure I got your point here. pick_avail_cos is to get an unused
COS, if succeed, it returns a positive number which means COS, but
when fail, it should return a negative number to indicate an error.
As far as I know, ENOENT is 2 and EOVERFLOW is 75, if I return ENOENT
directly, the function which call pick_avail_cos could not tell the
value is a valid COS or an error number.
Would you mind explaining in more detail for me and thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] x86: add domctl cmd to set/get CDP code/data CBM
2015-09-06 7:15 ` He Chen
@ 2015-09-06 16:29 ` Andrew Cooper
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-09-06 16:29 UTC (permalink / raw)
To: xen-devel, wei.liu2, ian.campbell, stefano.stabellini,
ian.jackson, jbeulich, keir
On 06/09/15 08:15, He Chen wrote:
> On Wed, Sep 02, 2015 at 12:59:07PM +0100, Andrew Cooper wrote:
>> On 02/09/15 09:28, 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 | 33 +++++++++-
>>> xen/arch/x86/psr.c | 142 ++++++++++++++++++++++++++++++++------------
>>> xen/include/asm-x86/psr.h | 12 +++-
>>> xen/include/public/domctl.h | 4 ++
>>> 4 files changed, 150 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
>>> index 26596dd..8e92d24 100644
>>> --- a/xen/arch/x86/psr.c
>>> +++ b/xen/arch/x86/psr.c
>>> +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 -EOVERFLOW;
>> ENOENT surely, or use EOVERFLOW consistently.
>>
> I am not sure I got your point here. pick_avail_cos is to get an unused
> COS, if succeed, it returns a positive number which means COS, but
> when fail, it should return a negative number to indicate an error.
>
> As far as I know, ENOENT is 2 and EOVERFLOW is 75, if I return ENOENT
> directly, the function which call pick_avail_cos could not tell the
> value is a valid COS or an error number.
>
> Would you mind explaining in more detail for me and thanks.
You are mixing at matching use of ENOENT and EOVERFLOW for the same kind
of failure from different subroutines. Please be consistent (and negative).
~Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-09-06 16:29 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-02 8:27 [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling He Chen
2015-09-02 8:27 ` [PATCH 1/5] x86: detect Intel CDP feature He Chen
2015-09-02 11:12 ` Andrew Cooper
2015-09-02 8:27 ` [PATCH 2/5] x86: Support enable/disable CDP dynamically and get CDP status He Chen
2015-09-02 11:39 ` Andrew Cooper
2015-09-02 14:07 ` Jan Beulich
2015-09-02 8:28 ` [PATCH 3/5] x86: add domctl cmd to set/get CDP code/data CBM He Chen
2015-09-02 11:59 ` Andrew Cooper
2015-09-06 7:15 ` He Chen
2015-09-06 16:29 ` Andrew Cooper
2015-09-02 8:28 ` [PATCH 4/5] tools: add tools support for Intel CDP He Chen
2015-09-02 13:32 ` Wei Liu
2015-09-02 8:28 ` [PATCH 5/5] docs: add document to introduce CDP command He Chen
2015-09-02 13:32 ` Wei Liu
2015-09-02 12:08 ` [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling Andrew Cooper
2015-09-06 1:18 ` Chao Peng
2015-09-06 6:49 ` He Chen
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.