All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/3] detect and initialize CDP (Code/Data Prioritization) feature
@ 2015-10-13  8:53 He Chen
  2015-10-13  8:53 ` [PATCH v7 1/3] x86: Support enable CDP by boot parameter and add get CDP status He Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: He Chen @ 2015-10-13  8:53 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, jbeulich, chao.p.peng, keir

Changes in v7:
- x86:
  * amend function find_cos and pick_avail_cos to ignore reference count
    for COS0
  * write CDP data mask (mask1) before turning CDP on
  * check CDP status before clear enabled bit in function cat_cpu_fini
  * code style
- tools & docs:
  * LIBXL_PSR_CBM_TYPE_L3_CODE (DATA) =>
    LIBXL_PSR_CBM_TYPE_L3_CBM_CODE (DATA)
  * refactor psr_cat_print_one_domain and add a helper to print different
    type CBM
  * fix docs wrap

Changes in v6:
- x86:
  * remove unnecessary parameter in cdp_is_enabled function
  * remove variable need_write and restruct code in psr_set_l3_cbm
- tools & docs:
  * separate CBM headings in the output of xl psr-cat-show
  * revert the numbers of SDM chapter in xl-psr.markdown
  * XC_PSR_CAT_L3_CODE (DATA) => XC_PSR_CAT_L3_CBM_CODE (DATA)
  * correct error message of passing -c and -d at the same time

Changes in v5:
- x86: address Andrew's and Jan's coments.
- tools: refine options parsing in psr-cat-cbm-set and invalidate passing -c
  and -d at the same time
- merge tools and docs patches.
- code style

Changes in v4:
- x86:
  * remove union member name in struct `psr_cat_cbm` (suggested by Jan)
  * fix log info of CAT & CDP (suggested by Chao & Jan)
  * add a helper `cdp_is_enabled` to tell the status of CDP and CDP initialize
    failed is considered (Jan's comment)
  * XEN_SYSCTL_INTERFACE_VERSION 0x0000000C -> 0x0000000D (suggested by Jan)
  * refine CBM type check logic in get/set CBM function (suggested by Jan)
  * loop optimization in function `find_cos` (suggested by Jan)
- tools: address Chao's comments.
- docs: address Chao's comments.
- code style

Changes in v3:
- x86: remove redundant CDP field in cat_socket_enable (suggested by Chao)
- tools: simplify CBM setting function in tools (suggested by Jan)
- docs: add boot parameter description (suggested by Chao & Ian)
- code style

Changes in v2:
- x86: enable CDP by boot parameter instead of enabling/disabling CDP at
       runtime (suggested by Andrew)
- 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 v7 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 (3):
  x86: Support enable CDP by boot parameter and add get CDP status
  x86: add domctl cmd to set/get CDP code/data CBM
  tools & docs: add tools and docs support for Intel CDP

 docs/man/xl.pod.1                   |  15 +++
 docs/misc/xen-command-line.markdown |  11 +-
 docs/misc/xl-psr.markdown           |  53 ++++++++
 tools/libxc/include/xenctrl.h       |   7 +-
 tools/libxc/xc_psr.c                |  17 ++-
 tools/libxl/libxl.h                 |   7 ++
 tools/libxl/libxl_psr.c             |   5 +-
 tools/libxl/libxl_types.idl         |   3 +
 tools/libxl/xl_cmdimpl.c            |  66 ++++++++--
 tools/libxl/xl_cmdtable.c           |   2 +
 xen/arch/x86/domctl.c               |  32 ++++-
 xen/arch/x86/psr.c                  | 232 +++++++++++++++++++++++++++++-------
 xen/arch/x86/sysctl.c               |   5 +-
 xen/include/asm-x86/msr-index.h     |   3 +
 xen/include/asm-x86/psr.h           |  20 +++-
 xen/include/public/domctl.h         |   4 +
 xen/include/public/sysctl.h         |   4 +-
 17 files changed, 414 insertions(+), 72 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v7 1/3] x86: Support enable CDP by boot parameter and add get CDP status
  2015-10-13  8:53 [PATCH v7 0/3] detect and initialize CDP (Code/Data Prioritization) feature He Chen
@ 2015-10-13  8:53 ` He Chen
  2015-10-14  2:02   ` Chao Peng
  2015-10-13  8:53 ` [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM He Chen
  2015-10-13  8:53 ` [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP He Chen
  2 siblings, 1 reply; 22+ messages in thread
From: He Chen @ 2015-10-13  8:53 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, jbeulich, chao.p.peng, keir

Add boot parameter `psr=cdp` to enable CDP at boot time.
Intel Code/Data Prioritization (CDP) feature is based on CAT. 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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v7:
* write CDP data mask (mask1) before turning CDP on
* check CDP status before clear enabled bit in function cat_cpu_fini

Changes in v6:
* remove unnecessary parameter in cdp_is_enabled

Changes in v5:
* remove unnecessary u in psr_cat_cbm structure
* revert write_l3_cbm and put the modification to next patch
* remove duplicate PSR_CAT_FLAG_L3_CDP
---
 docs/misc/xen-command-line.markdown | 11 ++++++--
 xen/arch/x86/psr.c                  | 51 ++++++++++++++++++++++++++++++++++---
 xen/arch/x86/sysctl.c               |  5 ++--
 xen/include/asm-x86/msr-index.h     |  3 +++
 xen/include/asm-x86/psr.h           |  8 +++++-
 xen/include/public/sysctl.h         |  4 ++-
 6 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index a565c1b..416e559 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1174,9 +1174,9 @@ This option can be specified more than once (up to 8 times at present).
 > `= <integer>`
 
 ### psr (Intel)
-> `= List of ( cmt:<boolean> | rmid_max:<integer> | cat:<boolean> | cos_max:<integer> )`
+> `= List of ( cmt:<boolean> | rmid_max:<integer> | cat:<boolean> | cos_max:<integer> | cdp:<boolean> )`
 
-> Default: `psr=cmt:0,rmid_max:255,cat:0,cos_max:255`
+> Default: `psr=cmt:0,rmid_max:255,cat:0,cos_max:255,cdp:0`
 
 Platform Shared Resource(PSR) Services.  Intel Haswell and later server
 platforms offer information about the sharing of resources.
@@ -1206,6 +1206,13 @@ The following resources are available:
   the cache allocation.
   * `cat` instructs Xen to enable/disable Cache Allocation Technology.
   * `cos_max` indicates the max value for COS ID.
+* Code and Data Prioritization Technology (Broadwell and later). Information
+  regarding the code cache and the data cache allocation. CDP is based on CAT.
+  * `cdp` instructs Xen to enable/disable Code and Data Prioritization. Note
+    that `cos_max` of CDP is a little different from `cos_max` of CAT. With
+    CDP, one COS will corespond two CBMs other than one with CAT, due to the
+    sum of CBMs is fixed, that means actual `cos_max` in use will automatically
+    reduce to half when CDP is enabled.
 
 ### reboot
 > `= t[riple] | k[bd] | a[cpi] | p[ci] | P[ower] | e[fi] | n[o] [, [w]arm | [c]old]`
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index c0daa2e..5ddeb2b 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -21,9 +21,16 @@
 
 #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;
+        };
+    };
     unsigned int ref;
 };
 
@@ -43,6 +50,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 +102,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);
@@ -261,8 +270,13 @@ static struct psr_cat_socket_info *get_cat_socket_info(unsigned int socket)
     return cat_socket_info + socket;
 }
 
+static inline bool_t cdp_is_enabled(unsigned int socket)
+{
+    return cdp_socket_enable && test_bit(socket, cdp_socket_enable);
+}
+
 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 +286,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 ( cdp_is_enabled(socket) )
+        *flags |= XEN_SYSCTL_PSR_CAT_L3_CDP;
+
     return 0;
 }
 
@@ -470,6 +488,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 )
@@ -495,8 +514,27 @@ static void cat_cpu_init(void)
         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) &&
+             cdp_socket_enable && !test_bit(socket, cdp_socket_enable) )
+        {
+            info->cos_to_cbm[0].code = (1ull << info->cbm_len) - 1;
+            info->cos_to_cbm[0].data = (1ull << info->cbm_len) - 1;
+
+            /* We only write mask1 since mask0 is always all ones by default. */
+            wrmsrl(MSR_IA32_PSR_L3_MASK(1), (1ull << info->cbm_len) - 1);
+
+            rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
+            wrmsrl(MSR_IA32_PSR_L3_QOS_CFG, val | (1 << PSR_L3_QOS_CDP_ENABLE_BIT));
+
+            /* Cut half of cos_max when CDP is enabled. */
+            info->cos_max >>= 1;
+
+            set_bit(socket, cdp_socket_enable);
+        }
+        printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u, CDP:%s\n",
+               socket, info->cos_max, info->cbm_len,
+               cdp_is_enabled(socket) ? "on" : "off");
     }
 }
 
@@ -513,6 +551,10 @@ static void cat_cpu_fini(unsigned int cpu)
             xfree(info->cos_to_cbm);
             info->cos_to_cbm = NULL;
         }
+
+        if ( cdp_is_enabled(socket) )
+            clear_bit(socket, cdp_socket_enable);
+
         clear_bit(socket, cat_socket_enable);
     }
 }
@@ -535,6 +577,7 @@ 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 )
         psr_cat_free();
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 38b5dcb..e54ddac 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -178,12 +178,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..65c1d02 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_CODE(n)	(0x00000c90 + (n) * 2 + 1)
+#define MSR_IA32_PSR_L3_MASK_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..5faeaaf 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -27,6 +27,12 @@
 /* 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
+
 struct psr_cmt_l3 {
     unsigned int features;
     unsigned int upscaling_factor;
@@ -52,7 +58,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 0cacacc..96680eb 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -36,7 +36,7 @@
 #include "physdev.h"
 #include "tmem.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000C
+#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000D
 
 /*
  * Read console content from Xen buffer ring.
@@ -705,6 +705,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] 22+ messages in thread

* [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-13  8:53 [PATCH v7 0/3] detect and initialize CDP (Code/Data Prioritization) feature He Chen
  2015-10-13  8:53 ` [PATCH v7 1/3] x86: Support enable CDP by boot parameter and add get CDP status He Chen
@ 2015-10-13  8:53 ` He Chen
  2015-10-13 15:38   ` Jan Beulich
  2015-10-15 14:57   ` Olaf Hering
  2015-10-13  8:53 ` [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP He Chen
  2 siblings, 2 replies; 22+ messages in thread
From: He Chen @ 2015-10-13  8:53 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, jbeulich, chao.p.peng, keir

CDP extends CAT and provides the capacity to control L3 code & data
cache. With CDP, one COS corresponds to two CMBs(code & data). cbm_type
is added to distinguish different CBM operations. 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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v7:
* amend function find_cos and pick_avail_cos to ignore reference count
  for COS0
* code style

Changes in v6:
* remove variable need_write and restruct code in psr_set_l3_cbm
* remove redundant type == PSR_CBM_TYPE_L3 in psr_get_l3_cbm

Changes in v5:
* replace -EINVAL with -ENXIO when setting code/data CBM on CDP
  disabled.
---
 xen/arch/x86/domctl.c       |  32 +++++++-
 xen/arch/x86/psr.c          | 181 ++++++++++++++++++++++++++++++++++----------
 xen/include/asm-x86/psr.h   |  12 ++-
 xen/include/public/domctl.h |   4 +
 4 files changed, 183 insertions(+), 46 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index f8a559c..0f6fdb9 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1168,12 +1168,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 5ddeb2b..0083115 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -293,14 +293,40 @@ 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);
+    bool_t cdp_enabled = cdp_is_enabled(socket);
 
     if ( IS_ERR(info) )
         return PTR_ERR(info);
 
-    *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].cbm;
+    switch ( type )
+    {
+    case PSR_CBM_TYPE_L3:
+        if ( cdp_enabled )
+            return -EXDEV;
+        *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].cbm;
+        break;
+
+    case PSR_CBM_TYPE_L3_CODE:
+        if ( !cdp_enabled )
+            *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].cbm;
+        else
+            *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].code;
+        break;
+
+    case PSR_CBM_TYPE_L3_DATA:
+        if ( !cdp_enabled )
+            *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].cbm;
+        else
+            *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].data;
+        break;
+
+    default:
+        ASSERT_UNREACHABLE();
+    }
 
     return 0;
 }
@@ -331,19 +357,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_CODE(info->cos), info->cbm_code);
+        wrmsrl(MSR_IA32_PSR_L3_MASK_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);
@@ -359,10 +400,48 @@ static int write_l3_cbm(unsigned int socket, unsigned int cos, uint64_t cbm)
     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, unsigned int cos_max,
+                    uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
+{
+    unsigned int cos;
+
+    for ( cos = 0; cos <= cos_max; cos++ )
+    {
+        if ( (map[cos].ref || cos == 0) &&
+             ((!cdp_enabled && map[cos].cbm == cbm_code) ||
+              (cdp_enabled && map[cos].code == cbm_code &&
+                              map[cos].data == cbm_data)) )
+            return cos;
+    }
+
+    return -ENOENT;
+}
+
+static int pick_avail_cos(struct psr_cat_cbm *map, unsigned int cos_max,
+                          unsigned int old_cos)
+{
+    unsigned int cos;
+
+    /* If old cos is referred only by the domain, then use it. */
+    if ( map[old_cos].ref == 1 && old_cos != 0 )
+        return old_cos;
+
+    /* 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;
-    struct psr_cat_cbm *map, *found = NULL;
+    unsigned int old_cos, cos_max;
+    int cos, ret;
+    uint64_t cbm_data, cbm_code;
+    bool_t cdp_enabled = cdp_is_enabled(socket);
+    struct psr_cat_cbm *map;
     struct psr_cat_socket_info *info = get_cat_socket_info(socket);
 
     if ( IS_ERR(info) )
@@ -371,53 +450,71 @@ int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
     if ( !psr_check_cbm(info->cbm_len, cbm) )
         return -EINVAL;
 
+    if ( !cdp_enabled && (type == PSR_CBM_TYPE_L3_CODE ||
+                          type == PSR_CBM_TYPE_L3_DATA) )
+        return -ENXIO;
+
+    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].cbm == cbm )
-        {
-            if ( unlikely(cos == old_cos) )
-            {
-                ASSERT(cos == 0 || map[cos].ref != 0);
-                spin_unlock(&info->cbm_lock);
-                return 0;
-            }
-            found = map + cos;
-            break;
-        }
-    }
+    case PSR_CBM_TYPE_L3:
+        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:
+        cbm_code = cbm;
+        cbm_data = map[old_cos].data;
+        break;
 
-    if ( !found )
-    {
-        spin_unlock(&info->cbm_lock);
-        return -EOVERFLOW;
+    case PSR_CBM_TYPE_L3_DATA:
+        cbm_code = map[old_cos].code;
+        cbm_data = cbm;
+        break;
+
+    default:
+        ASSERT_UNREACHABLE();
     }
 
-    cos = found - map;
-    if ( found->cbm != cbm )
+    spin_lock(&info->cbm_lock);
+    cos = find_cos(map, cos_max, cbm_code, cbm_data, cdp_enabled);
+    if ( cos >= 0 )
     {
-        int ret = write_l3_cbm(socket, cos, cbm);
-
-        if ( ret )
+        if ( 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 ( (cdp_enabled &&
+             (map[cos].code != cbm_code || map[cos].data != cbm_data)) ||
+             (!cdp_enabled && map[cos].cbm != cbm_code) )
+        {
+            ret = write_l3_cbm(socket, cos, cbm_code, cbm_data, cdp_enabled);
+            if ( ret )
+            {
+                spin_unlock(&info->cbm_lock);
+                return ret;
+            }
+            map[cos].code = cbm_code;
+            map[cos].data = cbm_data;
         }
-        found->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 5faeaaf..57f47e9 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -46,6 +46,12 @@ struct psr_cmt {
     struct psr_cmt_l3 l3;
 };
 
+enum cbm_type {
+    PSR_CBM_TYPE_L3,
+    PSR_CBM_TYPE_L3_CODE,
+    PSR_CBM_TYPE_L3_DATA,
+};
+
 extern struct psr_cmt *psr_cmt;
 
 static inline bool_t psr_cmt_enabled(void)
@@ -59,8 +65,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 ae241f2..86cd0ab 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1057,6 +1057,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] 22+ messages in thread

* [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP
  2015-10-13  8:53 [PATCH v7 0/3] detect and initialize CDP (Code/Data Prioritization) feature He Chen
  2015-10-13  8:53 ` [PATCH v7 1/3] x86: Support enable CDP by boot parameter and add get CDP status He Chen
  2015-10-13  8:53 ` [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM He Chen
@ 2015-10-13  8:53 ` He Chen
  2015-10-13 16:23   ` Ian Campbell
                     ` (3 more replies)
  2 siblings, 4 replies; 22+ messages in thread
From: He Chen @ 2015-10-13  8:53 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	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.
Add new CDP options with CAT commands in xl interface man page.
Add description of CDP in xl-psr.markdown.

Signed-off-by: He Chen <he.chen@linux.intel.com>
---
Changes in v7:
* LIBXL_PSR_CBM_TYPE_L3_CODE (DATA) =>
  LIBXL_PSR_CBM_TYPE_L3_CBM_CODE (DATA)
* refactor psr_cat_print_one_domain and add a helper to print different
  type CBM
* fix docs wrap

Changes in v6:
* separate CBM headings in the output of xl psr-cat-show
* revert the numbers of SDM chapter in xl-psr.markdown
* XC_PSR_CAT_L3_CODE (DATA) => XC_PSR_CAT_L3_CBM_CODE (DATA)
* correct error message of passing -c and -d at the same time

Changes in v5:
* merge tools and docs patches
* replace EINVAL with ENXIO in libxl__psr_cat_log_err_msg
* revise options parsing in psr-cat-cbm-set and invalidate passing -c
  and -d at the same time
* refine CDP status output codes in psr_cat_hwinfo
* adjust CBM output format in command xl psr-cat-show
* docs revision

Example of new output format for command xl psr-cat-show:

*** CAT-only ***

Socket ID       : 0
L3 Cache        : 56320KB
Default CBM     : 0xfffff
   ID                     NAME             CBM
    0                 Domain-0         0xfffff
    1               centos.hvm         0xfffff

*** CDP enabled ***

Socket ID       : 0
L3 Cache        : 56320KB
Default CBM     : 0xfffff
   ID                     NAME      CBM (code)     CBM (data)
    0                 Domain-0         0xfffff        0xfffff
    1               centos.hvm         0xfffff        0xfffff
---
 docs/man/xl.pod.1             | 15 ++++++++++
 docs/misc/xl-psr.markdown     | 53 ++++++++++++++++++++++++++++++++++
 tools/libxc/include/xenctrl.h |  7 +++--
 tools/libxc/xc_psr.c          | 17 ++++++++++-
 tools/libxl/libxl.h           |  7 +++++
 tools/libxl/libxl_psr.c       |  5 +++-
 tools/libxl/libxl_types.idl   |  3 ++
 tools/libxl/xl_cmdimpl.c      | 66 +++++++++++++++++++++++++++++++++++--------
 tools/libxl/xl_cmdtable.c     |  2 ++
 9 files changed, 159 insertions(+), 16 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index d0cd612..4279c7c 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1518,6 +1518,13 @@ 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 allocations, which support specifying code or data cache for
+applications. CDP is used on a per 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, and the type options (code
+and data) are mutually exclusive.
+
 =over 4
 
 =item B<psr-cat-cbm-set> [I<OPTIONS>] I<domain-id> I<cbm>
@@ -1533,6 +1540,14 @@ B<OPTIONS>
 
 Specify the socket to process, otherwise all sockets are processed.
 
+=item B<-c>, B<--code>
+
+Set code CBM when CDP is enabled.
+
+=item B<-d>, B<--data>
+
+Set data CBM when CDP is 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 c32e25c..9b08ee3 100644
--- a/docs/misc/xl-psr.markdown
+++ b/docs/misc/xl-psr.markdown
@@ -127,6 +127,59 @@ Per domain CBM settings can be shown by:
 
 `xl psr-cat-show`
 
+## Code and Data Prioritization (CDP)
+
+Code and 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 can be enabled by adding `psr=cdp` to Xen command line.
+
+When CDP is enabled,
+
+ * the CAT masks are re-mapped into interleaved pairs of masks 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 the OS or Hypervisor to partition cache allocation in a more
+fine-grained. Code cache and data cache can be specified independently.
+With CDP enabled, one COS corresponds to two CBMs (code CBM & data CBM),
+since the sum of CBMs is fixed, that means the number of available COSes
+will reduce by half when CDP is on.
+
+For more detailed information please refer to Intel SDM chapter
+"Platform Shared Resource Control: Cache Allocation Technology".
+
+The xl interfaces are the same with that of CAT. The difference is that
+CBM type can be passed as option to set code CBM or data CBM.
+
+When CDP is enabled, `-c` or `--code` option is available to set code CBM
+for the domain.
+
+When CDP is enabled, `-d` or `--data` option is available to set data CBM
+for the domain.
+
+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. Passing both `-c` and `-d`
+options is invalid.
+
+Example:
+
+Setting code CBM for a domain:
+`xl psr-cat-cbm-set -c <domid> <cbm>`
+
+Setting data CBM for a domain:
+`xl psr-cat-cbm-set -d <domid> <cbm>`
+
+Setting the same code and data CBM for a domain:
+`xl psr-cat-cbm-set <domid> <cbm>`
+
 ## Reference
 
 [1] Intel SDM
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 3bfa00b..2fec1fb 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2827,7 +2827,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_CBM_CODE = 2,
+    XC_PSR_CAT_L3_CBM_DATA = 3,
 };
 typedef enum xc_psr_cat_type xc_psr_cat_type;
 
@@ -2853,7 +2855,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..43b3286 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_CBM_CODE:
+        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE;
+        break;
+    case XC_PSR_CAT_L3_CBM_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_CBM_CODE:
+        cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE;
+        break;
+    case XC_PSR_CAT_L3_CBM_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 615b1de..168fedd 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -803,6 +803,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 and Data Prioritization feature is supported.
+ */
+#define LIBXL_HAVE_PSR_CDP 1
 #endif
 
 /*
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 000d748..3d0dc61 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -87,6 +87,9 @@ static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
     case EEXIST:
         msg = "The same CBM is already set to this domain";
         break;
+    case ENXIO:
+        msg = "Unable to set code or data CBM when CDP is disabled";
+        break;
 
     default:
         libxl__psr_log_err_msg(gc, err);
@@ -363,7 +366,7 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
     libxl_for_each_set_bit(socketid, socketmap) {
         ptr[i].id = socketid;
         if (xc_psr_cat_get_l3_info(ctx->xch, socketid, &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 d6ef9a2..4d78f86 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -797,10 +797,13 @@ libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
 libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
     (0, "UNKNOWN"),
     (1, "L3_CBM"),
+    (2, "L3_CBM_CODE"),
+    (3, "L3_CBM_DATA"),
     ])
 
 libxl_psr_cat_info = Struct("psr_cat_info", [
     ("id", uint32),
     ("cos_max", uint32),
     ("cbm_len", uint32),
+    ("cdp_enabled", bool),
     ])
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 365798b..8db0132 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8434,6 +8434,8 @@ static int psr_cat_hwinfo(void)
         }
         printf("%-16s: %u\n", "Socket ID", info[i].id);
         printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
+        printf("%-16s: %s\n", "CDP Status",
+               info->cdp_enabled ? "Enabled" : "Disabled");
         printf("%-16s: %u\n", "Maximum COS", info[i].cos_max);
         printf("%-16s: %u\n", "CBM length", info[i].cbm_len);
         printf("%-16s: %#llx\n", "Default CBM",
@@ -8445,29 +8447,46 @@ 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_type(uint32_t domid, uint32_t socketid,
+                                              libxl_psr_cbm_type type)
 {
-    char *domain_name;
     uint64_t cbm;
+        if (!libxl_psr_cat_get_cbm(ctx, domid, type, socketid, &cbm))
+            printf("%#16"PRIx64, cbm);
+        else
+            printf("%16s", "error");
+}
+
+static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid,
+                                         bool cdp_enabled)
+{
+    char *domain_name;
 
     domain_name = libxl_domid_to_name(ctx, domid);
     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) {
+        psr_cat_print_one_domain_cbm_type(domid, socketid,
+                                          LIBXL_PSR_CBM_TYPE_L3_CBM);
+    } else {
+        psr_cat_print_one_domain_cbm_type(domid, socketid,
+                                          LIBXL_PSR_CBM_TYPE_L3_CBM_CODE);
+        psr_cat_print_one_domain_cbm_type(domid, socketid,
+                                          LIBXL_PSR_CBM_TYPE_L3_CBM_DATA);
+    }
 
     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;
     }
 
@@ -8477,7 +8496,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;
@@ -8498,9 +8517,12 @@ static int psr_cat_print_socket(uint32_t domid, libxl_psr_cat_info *info)
     printf("%-16s: %u\n", "Socket ID", info->id);
     printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
     printf("%-16s: %#llx\n", "Default CBM", (1ull << info->cbm_len) - 1);
-    printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
+    if (info->cdp_enabled)
+        printf("%5s%25s%16s%16s\n", "ID", "NAME", "CBM (code)", "CBM (data)");
+    else
+        printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
 
-    return psr_cat_print_domain_cbm(domid, info->id);
+    return psr_cat_print_domain_cbm(domid, info->id, info->cdp_enabled);
 }
 
 static int psr_cat_show(uint32_t domid)
@@ -8529,9 +8551,10 @@ out:
 int main_psr_cat_cbm_set(int argc, char **argv)
 {
     uint32_t domid;
-    libxl_psr_cbm_type type = LIBXL_PSR_CBM_TYPE_L3_CBM;
+    libxl_psr_cbm_type type;
     uint64_t cbm;
     int ret, opt = 0;
+    int opt_data = 0, opt_code = 0;
     libxl_bitmap target_map;
     char *value;
     libxl_string_list socket_list;
@@ -8540,13 +8563,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);
@@ -8560,6 +8585,23 @@ int main_psr_cat_cbm_set(int argc, char **argv)
         libxl_string_list_dispose(&socket_list);
         free(value);
         break;
+    case 'd':
+        opt_data = 1;
+        break;
+    case 'c':
+        opt_code = 1;
+        break;
+    }
+
+    if (opt_data && opt_code) {
+        fprintf(stderr, "Cannot handle -c and -d at the same time\n");
+        return -1;
+    } else if (opt_data) {
+        type = LIBXL_PSR_CBM_TYPE_L3_CBM_DATA;
+    } else if (opt_code) {
+        type = LIBXL_PSR_CBM_TYPE_L3_CBM_CODE;
+    } else {
+        type = LIBXL_PSR_CBM_TYPE_L3_CBM;
     }
 
     if (libxl_bitmap_is_empty(&target_map))
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 0071f12..fdc1ac6 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,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-13  8:53 ` [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM He Chen
@ 2015-10-13 15:38   ` Jan Beulich
  2015-10-14  2:07     ` Chao Peng
  2015-10-15 14:57   ` Olaf Hering
  1 sibling, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2015-10-13 15:38 UTC (permalink / raw)
  To: He Chen
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, xen-devel, chao.p.peng, keir

>>> On 13.10.15 at 10:53, <he.chen@linux.intel.com> wrote:
> @@ -331,19 +357,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;
>  };

The bool_t field really fits well in the gap between the first and second
one; I'll take the liberty to adjust this when committing (unless Chao
comes forward with further comments; otherwise I'm kind of hoping
for a Reviewed-by-him).

Jan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP
  2015-10-13  8:53 ` [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP He Chen
@ 2015-10-13 16:23   ` Ian Campbell
  2015-10-14  2:38   ` Chao Peng
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 22+ messages in thread
From: Ian Campbell @ 2015-10-13 16:23 UTC (permalink / raw)
  To: He Chen, xen-devel
  Cc: wei.liu2, stefano.stabellini, andrew.cooper3, ian.jackson,
	jbeulich, chao.p.peng, keir

On Tue, 2015-10-13 at 16:53 +0800, He Chen wrote:
> @@ -8445,29 +8447,46 @@ 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_type(uint32_t domid, uint32_t
> socketid,
> +                                              libxl_psr_cbm_type type)
>  {
> -    char *domain_name;
>      uint64_t cbm;
> +        if (!libxl_psr_cat_get_cbm(ctx, domid, type, socketid, &cbm))
> +            printf("%#16"PRIx64, cbm);
> +        else
> +            printf("%16s", "error");

Coding style is 4 space indentation, here you appear to have 8 for the
first level and 4 for the second level of indentation.

With that fixed:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 1/3] x86: Support enable CDP by boot parameter and add get CDP status
  2015-10-13  8:53 ` [PATCH v7 1/3] x86: Support enable CDP by boot parameter and add get CDP status He Chen
@ 2015-10-14  2:02   ` Chao Peng
  0 siblings, 0 replies; 22+ messages in thread
From: Chao Peng @ 2015-10-14  2:02 UTC (permalink / raw)
  To: He Chen
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, jbeulich, xen-devel, keir

On Tue, Oct 13, 2015 at 04:53:44PM +0800, He Chen wrote:
> Add boot parameter `psr=cdp` to enable CDP at boot time.
> Intel Code/Data Prioritization (CDP) feature is based on CAT. 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>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changes in v7:
> * write CDP data mask (mask1) before turning CDP on
> * check CDP status before clear enabled bit in function cat_cpu_fini

Reviewed-by: Chao Peng <chao.p.peng@linux.intel.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-13 15:38   ` Jan Beulich
@ 2015-10-14  2:07     ` Chao Peng
  0 siblings, 0 replies; 22+ messages in thread
From: Chao Peng @ 2015-10-14  2:07 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	He Chen, ian.jackson, xen-devel, keir

On Tue, Oct 13, 2015 at 09:38:58AM -0600, Jan Beulich wrote:
> >>> On 13.10.15 at 10:53, <he.chen@linux.intel.com> wrote:
> > @@ -331,19 +357,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;
> >  };
> 
> The bool_t field really fits well in the gap between the first and second
> one; I'll take the liberty to adjust this when committing (unless Chao
> comes forward with further comments; otherwise I'm kind of hoping
> for a Reviewed-by-him).

Correct, it can be a optimization and I don't see any problem. So please feel
free to adjust it. Thereby:

Reviewed-by: Chao Peng <chao.p.peng@linux.intel.com>

> 
> Jan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP
  2015-10-13  8:53 ` [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP He Chen
  2015-10-13 16:23   ` Ian Campbell
@ 2015-10-14  2:38   ` Chao Peng
  2015-10-14  8:52     ` Ian Campbell
  2015-10-14  9:08   ` Dario Faggioli
  2015-10-14 10:52   ` Jan Beulich
  3 siblings, 1 reply; 22+ messages in thread
From: Chao Peng @ 2015-10-14  2:38 UTC (permalink / raw)
  To: He Chen
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, jbeulich, xen-devel, keir

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 365798b..8db0132 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -8434,6 +8434,8 @@ static int psr_cat_hwinfo(void)
>          }
>          printf("%-16s: %u\n", "Socket ID", info[i].id);
>          printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
> +        printf("%-16s: %s\n", "CDP Status",
> +               info->cdp_enabled ? "Enabled" : "Disabled");

Sorry I didn't notice this before but I guess 'info->cdp_enabled' here
really would be 'info[i].cdp_enabled' as CDP status is per-socket.
Current code will always print the data for the first socket in each
iteration, which is undesirable.

With this and the indention issue proposed by Ian being fixed:

Reviewed-by: Chao Peng <chao.p.peng@linux.intel.com>

>          printf("%-16s: %u\n", "Maximum COS", info[i].cos_max);
>          printf("%-16s: %u\n", "CBM length", info[i].cbm_len);
>          printf("%-16s: %#llx\n", "Default CBM",
> @@ -8445,29 +8447,46 @@ out:
>      return rc;
>  }

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP
  2015-10-14  2:38   ` Chao Peng
@ 2015-10-14  8:52     ` Ian Campbell
  0 siblings, 0 replies; 22+ messages in thread
From: Ian Campbell @ 2015-10-14  8:52 UTC (permalink / raw)
  To: Chao Peng, He Chen
  Cc: wei.liu2, keir, stefano.stabellini, andrew.cooper3, ian.jackson,
	jbeulich, xen-devel

On Wed, 2015-10-14 at 10:38 +0800, Chao Peng wrote:
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 365798b..8db0132 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -8434,6 +8434,8 @@ static int psr_cat_hwinfo(void)
> >          }
> >          printf("%-16s: %u\n", "Socket ID", info[i].id);
> >          printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
> > +        printf("%-16s: %s\n", "CDP Status",
> > +               info->cdp_enabled ? "Enabled" : "Disabled");
> 
> Sorry I didn't notice this before but I guess 'info->cdp_enabled' here
> really would be 'info[i].cdp_enabled' as CDP status is per-socket.
> Current code will always print the data for the first socket in each
> iteration, which is undesirable.

I think you are correct, well spotted.


> With this and the indention issue proposed by Ian being fixed:
> 
> Reviewed-by: Chao Peng <chao.p.peng@linux.intel.com>
> 
> >          printf("%-16s: %u\n", "Maximum COS", info[i].cos_max);
> >          printf("%-16s: %u\n", "CBM length", info[i].cbm_len);
> >          printf("%-16s: %#llx\n", "Default CBM",
> > @@ -8445,29 +8447,46 @@ out:
> >      return rc;
> >  }

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP
  2015-10-13  8:53 ` [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP He Chen
  2015-10-13 16:23   ` Ian Campbell
  2015-10-14  2:38   ` Chao Peng
@ 2015-10-14  9:08   ` Dario Faggioli
  2015-10-14  9:20     ` Ian Campbell
  2015-10-14 10:52   ` Jan Beulich
  3 siblings, 1 reply; 22+ messages in thread
From: Dario Faggioli @ 2015-10-14  9:08 UTC (permalink / raw)
  To: He Chen, xen-devel
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, jbeulich, chao.p.peng, keir


[-- Attachment #1.1: Type: text/plain, Size: 1741 bytes --]

On Tue, 2015-10-13 at 16:53 +0800, He Chen wrote:
> diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
> index c32e25c..9b08ee3 100644
> --- a/docs/misc/xl-psr.markdown
> +++ b/docs/misc/xl-psr.markdown
> @@ -127,6 +127,59 @@ Per domain CBM settings can be shown by:
>  
>  `xl psr-cat-show`
>  
> +## Code and Data Prioritization (CDP)
> +
> +Code and 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 can be enabled by adding `psr=cdp` to Xen command line.
> +
> +When CDP is enabled,
> +
> + * the CAT masks are re-mapped into interleaved pairs of masks 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 the OS or Hypervisor to partition cache allocation in a
> more
> +fine-grained.
>
"In a more fine-grained" "way"? Or "manner"?

(I'm not a native speaker, though, so if that's not the case, sorry for
the noise.)

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP
  2015-10-14  9:08   ` Dario Faggioli
@ 2015-10-14  9:20     ` Ian Campbell
  0 siblings, 0 replies; 22+ messages in thread
From: Ian Campbell @ 2015-10-14  9:20 UTC (permalink / raw)
  To: Dario Faggioli, He Chen, xen-devel
  Cc: wei.liu2, stefano.stabellini, andrew.cooper3, ian.jackson,
	jbeulich, chao.p.peng, keir

On Wed, 2015-10-14 at 11:08 +0200, Dario Faggioli wrote:
> On Tue, 2015-10-13 at 16:53 +0800, He Chen wrote:
> > diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
> > index c32e25c..9b08ee3 100644
> > --- a/docs/misc/xl-psr.markdown
> > +++ b/docs/misc/xl-psr.markdown
> > @@ -127,6 +127,59 @@ Per domain CBM settings can be shown by:
> >  
> >  `xl psr-cat-show`
> >  
> > +## Code and Data Prioritization (CDP)
> > +
> > +Code and 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 can be enabled by adding `psr=cdp` to Xen command line.
> > +
> > +When CDP is enabled,
> > +
> > + * the CAT masks are re-mapped into interleaved pairs of masks 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 the OS or Hypervisor to partition cache allocation in a
> > more
> > +fine-grained.
> > 
> "In a more fine-grained" "way"? Or "manner"?
> 
> (I'm not a native speaker, though, so if that's not the case, sorry for
> the noise.)

You are right, there is a word missing. "manner" would be my interpretation
of what was intended.

Ian.


> 
> Regards,
> Dario

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP
  2015-10-13  8:53 ` [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP He Chen
                     ` (2 preceding siblings ...)
  2015-10-14  9:08   ` Dario Faggioli
@ 2015-10-14 10:52   ` Jan Beulich
  3 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2015-10-14 10:52 UTC (permalink / raw)
  To: ian.campbell, wei.liu2, ian.jackson, stefano.stabellini
  Cc: andrew.cooper3, He Chen, keir, xen-devel, chao.p.peng

>>> On 13.10.15 at 10:53, <he.chen@linux.intel.com> 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.
> Add new CDP options with CAT commands in xl interface man page.
> Add description of CDP in xl-psr.markdown.
> 
> Signed-off-by: He Chen <he.chen@linux.intel.com>

Tools maintainers, JFTR - I committed patches 1 and 2 of this series,
and will drop this tools side patch from my queue, assuming one of
you is going to take care of it.

Jan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-13  8:53 ` [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM He Chen
  2015-10-13 15:38   ` Jan Beulich
@ 2015-10-15 14:57   ` Olaf Hering
  2015-10-15 15:10     ` Wei Liu
  1 sibling, 1 reply; 22+ messages in thread
From: Olaf Hering @ 2015-10-15 14:57 UTC (permalink / raw)
  To: He Chen
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, jbeulich, chao.p.peng, xen-devel, keir

On Tue, Oct 13, He Chen wrote:

> +int psr_set_l3_cbm(struct domain *d, unsigned int socket,
> +                   uint64_t cbm, enum cbm_type type)
>  {

> +    uint64_t cbm_data, cbm_code;

> +    switch ( type )
>      {

> +    case PSR_CBM_TYPE_L3:
> +        cbm_code = cbm;
> +        break;

> +    case PSR_CBM_TYPE_L3_CODE:
> +        cbm_code = cbm;
> +        break;

> +    case PSR_CBM_TYPE_L3_DATA:
> +        cbm_code = map[old_cos].code;
> +        break;
> +
> +    default:
> +        ASSERT_UNREACHABLE();

> +    cos = find_cos(map, cos_max, cbm_code, cbm_data, cdp_enabled);

gcc5 does not like ASSERT_UNREACHABLE(), whatever is in staging fails:

[ 2144s] psr.c: In function 'psr_set_l3_cbm':
[ 2144s] psr.c:381:25: error: 'cbm_code' may be used uninitialized in this function [-Werror=maybe-uninitialized]
[ 2144s]      struct cos_cbm_info info =
[ 2144s]                          ^
[ 2144s] psr.c:442:24: note: 'cbm_code' was declared here
[ 2144s]      uint64_t cbm_data, cbm_code;
[ 2144s]                         ^
[ 2144s] psr.c:513:27: error: 'cbm_data' may be used uninitialized in this function [-Werror=maybe-uninitialized]
[ 2144s]              map[cos].data = cbm_data;
[ 2144s]                            ^
[ 2145s] cc1: all warnings being treated as errors
[ 2145s] /home/abuild/rpmbuild/BUILD/xen-4.7.31795/non-dbg/xen/Rules.mk:174: recipe for target 'psr.o' failed
[ 2145s] make[4]: *** [psr.o] Error 1

Havent checked if there is already a fix in-flight.

Olaf

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-15 14:57   ` Olaf Hering
@ 2015-10-15 15:10     ` Wei Liu
  2015-10-15 15:22       ` Olaf Hering
  2015-10-15 15:47       ` Jan Beulich
  0 siblings, 2 replies; 22+ messages in thread
From: Wei Liu @ 2015-10-15 15:10 UTC (permalink / raw)
  To: Olaf Hering
  Cc: wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	He Chen, ian.jackson, jbeulich, chao.p.peng, xen-devel, keir

On Thu, Oct 15, 2015 at 04:57:16PM +0200, Olaf Hering wrote:
> On Tue, Oct 13, He Chen wrote:
> 
> > +int psr_set_l3_cbm(struct domain *d, unsigned int socket,
> > +                   uint64_t cbm, enum cbm_type type)
> >  {
> 
> > +    uint64_t cbm_data, cbm_code;
> 
> > +    switch ( type )
> >      {
> 
> > +    case PSR_CBM_TYPE_L3:
> > +        cbm_code = cbm;
> > +        break;
> 
> > +    case PSR_CBM_TYPE_L3_CODE:
> > +        cbm_code = cbm;
> > +        break;
> 
> > +    case PSR_CBM_TYPE_L3_DATA:
> > +        cbm_code = map[old_cos].code;
> > +        break;
> > +
> > +    default:
> > +        ASSERT_UNREACHABLE();
> 
> > +    cos = find_cos(map, cos_max, cbm_code, cbm_data, cdp_enabled);
> 
> gcc5 does not like ASSERT_UNREACHABLE(), whatever is in staging fails:
> 
> [ 2144s] psr.c: In function 'psr_set_l3_cbm':
> [ 2144s] psr.c:381:25: error: 'cbm_code' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> [ 2144s]      struct cos_cbm_info info =
> [ 2144s]                          ^
> [ 2144s] psr.c:442:24: note: 'cbm_code' was declared here
> [ 2144s]      uint64_t cbm_data, cbm_code;
> [ 2144s]                         ^
> [ 2144s] psr.c:513:27: error: 'cbm_data' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> [ 2144s]              map[cos].data = cbm_data;
> [ 2144s]                            ^
> [ 2145s] cc1: all warnings being treated as errors
> [ 2145s] /home/abuild/rpmbuild/BUILD/xen-4.7.31795/non-dbg/xen/Rules.mk:174: recipe for target 'psr.o' failed
> [ 2145s] make[4]: *** [psr.o] Error 1
> 

Is it because in non-debug build ASSERT_UNREACHABLE is nop?

> Havent checked if there is already a fix in-flight.
> 
> Olaf

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-15 15:10     ` Wei Liu
@ 2015-10-15 15:22       ` Olaf Hering
  2015-10-15 15:26         ` Wei Liu
  2015-10-15 15:47       ` Jan Beulich
  1 sibling, 1 reply; 22+ messages in thread
From: Olaf Hering @ 2015-10-15 15:22 UTC (permalink / raw)
  To: Wei Liu
  Cc: keir, ian.campbell, stefano.stabellini, andrew.cooper3, He Chen,
	ian.jackson, jbeulich, chao.p.peng, xen-devel

On Thu, Oct 15, Wei Liu wrote:

> On Thu, Oct 15, 2015 at 04:57:16PM +0200, Olaf Hering wrote:
> > gcc5 does not like ASSERT_UNREACHABLE(), whatever is in staging fails:
> Is it because in non-debug build ASSERT_UNREACHABLE is nop?

That might explain it.
Does the staging build test catch this bug anyway?

Olaf

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-15 15:22       ` Olaf Hering
@ 2015-10-15 15:26         ` Wei Liu
  2015-10-15 15:30           ` Wei Liu
  2015-10-16  9:10           ` Olaf Hering
  0 siblings, 2 replies; 22+ messages in thread
From: Wei Liu @ 2015-10-15 15:26 UTC (permalink / raw)
  To: Olaf Hering
  Cc: Wei Liu, ian.campbell, stefano.stabellini, andrew.cooper3,
	He Chen, ian.jackson, jbeulich, chao.p.peng, xen-devel, keir

On Thu, Oct 15, 2015 at 05:22:39PM +0200, Olaf Hering wrote:
> On Thu, Oct 15, Wei Liu wrote:
> 
> > On Thu, Oct 15, 2015 at 04:57:16PM +0200, Olaf Hering wrote:
> > > gcc5 does not like ASSERT_UNREACHABLE(), whatever is in staging fails:
> > Is it because in non-debug build ASSERT_UNREACHABLE is nop?
> 
> That might explain it.
> Does the staging build test catch this bug anyway?
> 

No, because 1) staging is not using GCC5, 2) staging has debug=y.

Wei.

> Olaf

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-15 15:26         ` Wei Liu
@ 2015-10-15 15:30           ` Wei Liu
  2015-10-16  9:10           ` Olaf Hering
  1 sibling, 0 replies; 22+ messages in thread
From: Wei Liu @ 2015-10-15 15:30 UTC (permalink / raw)
  To: Olaf Hering
  Cc: Wei Liu, ian.campbell, stefano.stabellini, andrew.cooper3,
	He Chen, ian.jackson, jbeulich, chao.p.peng, xen-devel, keir

On Thu, Oct 15, 2015 at 04:26:47PM +0100, Wei Liu wrote:
> On Thu, Oct 15, 2015 at 05:22:39PM +0200, Olaf Hering wrote:
> > On Thu, Oct 15, Wei Liu wrote:
> > 
> > > On Thu, Oct 15, 2015 at 04:57:16PM +0200, Olaf Hering wrote:
> > > > gcc5 does not like ASSERT_UNREACHABLE(), whatever is in staging fails:
> > > Is it because in non-debug build ASSERT_UNREACHABLE is nop?
> > 
> > That might explain it.
> > Does the staging build test catch this bug anyway?
> > 
> 
> No, because 1) staging is not using GCC5, 2) staging has debug=y.
> 
> Wei.
> 

OK. I can confirm staging fails with non-debug build with the same
symptom with gcc 4.9 in Debian Jessie.

Wei.

> > Olaf

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-15 15:10     ` Wei Liu
  2015-10-15 15:22       ` Olaf Hering
@ 2015-10-15 15:47       ` Jan Beulich
  2015-10-16  8:24         ` He Chen
  1 sibling, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2015-10-15 15:47 UTC (permalink / raw)
  To: Olaf Hering, wei.liu2, He Chen
  Cc: keir, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, xen-devel, chao.p.peng

>>> On 15.10.15 at 17:10, <wei.liu2@citrix.com> wrote:
> On Thu, Oct 15, 2015 at 04:57:16PM +0200, Olaf Hering wrote:
>> On Tue, Oct 13, He Chen wrote:
>> 
>> > +int psr_set_l3_cbm(struct domain *d, unsigned int socket,
>> > +                   uint64_t cbm, enum cbm_type type)
>> >  {
>> 
>> > +    uint64_t cbm_data, cbm_code;
>> 
>> > +    switch ( type )
>> >      {
>> 
>> > +    case PSR_CBM_TYPE_L3:
>> > +        cbm_code = cbm;
>> > +        break;
>> 
>> > +    case PSR_CBM_TYPE_L3_CODE:
>> > +        cbm_code = cbm;
>> > +        break;
>> 
>> > +    case PSR_CBM_TYPE_L3_DATA:
>> > +        cbm_code = map[old_cos].code;
>> > +        break;
>> > +
>> > +    default:
>> > +        ASSERT_UNREACHABLE();
>> 
>> > +    cos = find_cos(map, cos_max, cbm_code, cbm_data, cdp_enabled);
>> 
>> gcc5 does not like ASSERT_UNREACHABLE(), whatever is in staging fails:
>> 
>> [ 2144s] psr.c: In function 'psr_set_l3_cbm':
>> [ 2144s] psr.c:381:25: error: 'cbm_code' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> [ 2144s]      struct cos_cbm_info info =
>> [ 2144s]                          ^
>> [ 2144s] psr.c:442:24: note: 'cbm_code' was declared here
>> [ 2144s]      uint64_t cbm_data, cbm_code;
>> [ 2144s]                         ^
>> [ 2144s] psr.c:513:27: error: 'cbm_data' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> [ 2144s]              map[cos].data = cbm_data;
>> [ 2144s]                            ^
>> [ 2145s] cc1: all warnings being treated as errors
>> [ 2145s] /home/abuild/rpmbuild/BUILD/xen-4.7.31795/non-dbg/xen/Rules.mk:174: recipe for target 'psr.o' failed
>> [ 2145s] make[4]: *** [psr.o] Error 1
>> 
> 
> Is it because in non-debug build ASSERT_UNREACHABLE is nop?

Ah, yes, in cases like this it should always be followed by return
(or whatever else is suitable). Sorry for not having spotted this
during review.

Jan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-15 15:47       ` Jan Beulich
@ 2015-10-16  8:24         ` He Chen
  2015-10-16  8:32           ` Jan Beulich
  0 siblings, 1 reply; 22+ messages in thread
From: He Chen @ 2015-10-16  8:24 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Olaf Hering, keir, ian.campbell, stefano.stabellini,
	andrew.cooper3, ian.jackson, chao.p.peng, xen-devel, wei.liu2

On Thu, Oct 15, 2015 at 09:47:37AM -0600, Jan Beulich wrote:
> 
> Ah, yes, in cases like this it should always be followed by return
> (or whatever else is suitable). Sorry for not having spotted this
> during review.
> 
Sorry for this bug. Is it proper to fix this bug by just adding a
return after ASSERT_UNREACHABLE? Or do some changes in
ASSERT_UNREACHABLE?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-16  8:24         ` He Chen
@ 2015-10-16  8:32           ` Jan Beulich
  0 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2015-10-16  8:32 UTC (permalink / raw)
  To: He Chen
  Cc: Olaf Hering, wei.liu2, ian.campbell, stefano.stabellini,
	andrew.cooper3, ian.jackson, xen-devel, chao.p.peng, keir

>>> On 16.10.15 at 10:24, <he.chen@linux.intel.com> wrote:
> On Thu, Oct 15, 2015 at 09:47:37AM -0600, Jan Beulich wrote:
>> 
>> Ah, yes, in cases like this it should always be followed by return
>> (or whatever else is suitable). Sorry for not having spotted this
>> during review.
>> 
> Sorry for this bug. Is it proper to fix this bug by just adding a
> return after ASSERT_UNREACHABLE? Or do some changes in
> ASSERT_UNREACHABLE?

As said (still visible above) you should return. You should definitely
not modify ASSERT_UNREACHABLE(), since what to do after it
depends on the circumstances. (We might consider requiring that
action to be the macro's sole argument, but I guess there may be
cases where that being blank is valid, and then would pointlessly
require some dummy argument to be passed. But certainly nothing
for you to worry about here.)

Jan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM
  2015-10-15 15:26         ` Wei Liu
  2015-10-15 15:30           ` Wei Liu
@ 2015-10-16  9:10           ` Olaf Hering
  1 sibling, 0 replies; 22+ messages in thread
From: Olaf Hering @ 2015-10-16  9:10 UTC (permalink / raw)
  To: Wei Liu
  Cc: keir, ian.campbell, He Chen, andrew.cooper3, stefano.stabellini,
	ian.jackson, jbeulich, xen-devel, chao.p.peng

On Thu, Oct 15, Wei Liu wrote:

> No, because 1) staging is not using GCC5, 2) staging has debug=y.

Are there enough resources to address #2?
Build once with debug=y and once without?

Olaf

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-10-16  9:11 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-13  8:53 [PATCH v7 0/3] detect and initialize CDP (Code/Data Prioritization) feature He Chen
2015-10-13  8:53 ` [PATCH v7 1/3] x86: Support enable CDP by boot parameter and add get CDP status He Chen
2015-10-14  2:02   ` Chao Peng
2015-10-13  8:53 ` [PATCH v7 2/3] x86: add domctl cmd to set/get CDP code/data CBM He Chen
2015-10-13 15:38   ` Jan Beulich
2015-10-14  2:07     ` Chao Peng
2015-10-15 14:57   ` Olaf Hering
2015-10-15 15:10     ` Wei Liu
2015-10-15 15:22       ` Olaf Hering
2015-10-15 15:26         ` Wei Liu
2015-10-15 15:30           ` Wei Liu
2015-10-16  9:10           ` Olaf Hering
2015-10-15 15:47       ` Jan Beulich
2015-10-16  8:24         ` He Chen
2015-10-16  8:32           ` Jan Beulich
2015-10-13  8:53 ` [PATCH v7 3/3] tools & docs: add tools and docs support for Intel CDP He Chen
2015-10-13 16:23   ` Ian Campbell
2015-10-14  2:38   ` Chao Peng
2015-10-14  8:52     ` Ian Campbell
2015-10-14  9:08   ` Dario Faggioli
2015-10-14  9:20     ` Ian Campbell
2015-10-14 10:52   ` Jan Beulich

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.