Skip to contentSkip to conversation

Intel Code and Data Prioritization (CDP) feature enabling

PATCH Superseded He Chen 3 participants 12 messages Sep 14, 2015

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

He Chen

author
#0 lore raw

v3 cover Intel Code and Data Prioritization (CDP) feature enabling

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 patchset has been tested on Intel Broadwell server platform.

To make this patchset better, any comment or suggestion is welcomed, I would really appreciate it.

Thanks

He Chen (4):
x86: Support enable CDP by boot parameter and add get CDP status
x86: add domctl cmd to set/get CDP code/data CBM
tools: add tools support for Intel CDP
docs: add document to introduce CDP command

 docs/man/xl.pod.1                   |  14 +++
 docs/misc/xen-command-line.markdown |   8 +-
 docs/misc/xl-psr.markdown           |  42 ++++++-
 tools/libxc/include/xenctrl.h       |   7 +-
 tools/libxc/xc_psr.c                |  17 ++-
 tools/libxl/libxl.h                 |   1 +
 tools/libxl/libxl_psr.c             |  20 ++-
 tools/libxl/libxl_types.idl         |   3 +
 tools/libxl/xl_cmdimpl.c            |  49 ++++++--
 tools/libxl/xl_cmdtable.c           |   3 +
 xen/arch/x86/domctl.c               |  32 ++++-
 xen/arch/x86/psr.c                  | 240 +++++++++++++++++++++++++++++-------
 xen/arch/x86/sysctl.c               |   5 +-
 xen/include/asm-x86/msr-index.h     |   3 +
 xen/include/asm-x86/psr.h           |  23 +++-
 xen/include/public/domctl.h         |   4 +
 xen/include/public/sysctl.h         |   2 +
 17 files changed, 399 insertions(+), 74 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v3 1/4 x86: Support enable CDP by boot parameter and add get CDP status

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.

---
 docs/misc/xen-command-line.markdown |  8 +++-
 xen/arch/x86/psr.c                  | 84 ++++++++++++++++++++++++++++++-------
 xen/arch/x86/sysctl.c               |  5 ++-
 xen/include/asm-x86/msr-index.h     |  3 ++
 xen/include/asm-x86/psr.h           | 11 ++++-
 xen/include/public/sysctl.h         |  2 +
 6 files changed, 93 insertions(+), 20 deletions(-)
docs/misc/xen-command-line.markdown +6 -2
index a2e427c..8ca1d7f 100644
@@ -1165,9 +1165,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.
@@ -1197,6 +1197,10 @@ 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.
+
 
 ### reboot
 > `= t[riple] | k[bd] | a[cpi] | p[ci] | P[ower] | e[fi] | n[o] [, [w]arm | [c]old]`
xen/arch/x86/psr.c +69 -15

191 lines not on this page · raw message

xen/arch/x86/sysctl.c +3 -2
index f36b52f..e42c5e7 100644
@@ -177,12 +177,13 @@ long arch_do_sysctl(
         case XEN_SYSCTL_PSR_CAT_get_l3_info:
             ret = psr_get_cat_l3_info(sysctl->u.psr_cat_op.target,
                                       &sysctl->u.psr_cat_op.u.l3_info.cbm_len,
-                                      &sysctl->u.psr_cat_op.u.l3_info.cos_max);
+                                      &sysctl->u.psr_cat_op.u.l3_info.cos_max,
+                                      &sysctl->u.psr_cat_op.u.l3_info.flags);
 
             if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) )
                 ret = -EFAULT;
-
             break;
+
         default:
             ret = -EOPNOTSUPP;
             break;
xen/include/asm-x86/msr-index.h +3 -0
index e9c4723..9149b20 100644
@@ -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))
xen/include/asm-x86/psr.h +10 -1
index 081750f..422b053 100644
@@ -27,6 +27,15 @@
 /* L3 Monitoring Features */
 #define PSR_CMT_L3_OCCUPANCY           0x1
 
+/* CDP Capability */
+#define PSR_CAT_CDP_CAPABILITY       (1u << 2)
+
+/* L3 CDP Enable bit*/
+#define PSR_L3_QOS_CDP_ENABLE_BIT       0x0
+
+/* L3 CDP flag bit */
+#define PSR_CAT_FLAG_L3_CDP       (1u << 0)
+
 struct psr_cmt_l3 {
     unsigned int features;
     unsigned int upscaling_factor;
@@ -52,7 +61,7 @@ void psr_free_rmid(struct domain *d);
 void psr_ctxt_switch_to(struct domain *d);
 
 int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
-                        uint32_t *cos_max);
+                        uint32_t *cos_max, uint32_t *flags);
 int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm);
 int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm);
 
xen/include/public/sysctl.h +2 -0
index 58c9be2..4102cd2 100644
@@ -704,6 +704,8 @@ struct xen_sysctl_psr_cat_op {
         struct {
             uint32_t cbm_len;   /* OUT: CBM length */
             uint32_t cos_max;   /* OUT: Maximum COS */
+            #define XEN_SYSCTL_PSR_CAT_L3_CDP       (1u << 0)
+            uint32_t flags;     /* OUT: CAT flags */
         } l3_info;
     } u;
 };
Signature · 1 line
1.9.1
author
#4 lore raw

PATCH v3 2/4 x86: add domctl cmd to set/get CDP code/data CBM

In reply to He Chen

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.

---
 xen/arch/x86/domctl.c       |  32 ++++++++-
 xen/arch/x86/psr.c          | 166 ++++++++++++++++++++++++++++++++++----------
 xen/include/asm-x86/psr.h   |  12 +++-
 xen/include/public/domctl.h |   4 ++
 4 files changed, 174 insertions(+), 40 deletions(-)
xen/arch/x86/domctl.c +30 -2
index bf62a88..734fddb 100644
@@ -1167,12 +1167,40 @@ long arch_do_domctl(
         {
         case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
             ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
-                                 domctl->u.psr_cat_op.data);
+                                 domctl->u.psr_cat_op.data,
+                                 PSR_CBM_TYPE_L3);
+            break;
+
+        case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE:
+            ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
+                                 domctl->u.psr_cat_op.data,
+                                 PSR_CBM_TYPE_L3_CODE);
+            break;
+
+        case XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA:
+            ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
+                                 domctl->u.psr_cat_op.data,
+                                 PSR_CBM_TYPE_L3_DATA);
             break;
 
         case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
             ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
-                                 &domctl->u.psr_cat_op.data);
+                                 &domctl->u.psr_cat_op.data,
+                                 PSR_CBM_TYPE_L3);
+            copyback = 1;
+            break;
+
+        case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE:
+            ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
+                                 &domctl->u.psr_cat_op.data,
+                                 PSR_CBM_TYPE_L3_CODE);
+            copyback = 1;
+            break;
+
+        case XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA:
+            ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
+                                 &domctl->u.psr_cat_op.data,
+                                 PSR_CBM_TYPE_L3_DATA);
             copyback = 1;
             break;
 
xen/arch/x86/psr.c +130 -36

207 lines not on this page · raw message

xen/include/asm-x86/psr.h +10 -2
index 422b053..c0b3f66 100644
@@ -49,6 +49,12 @@ struct psr_cmt {
     struct psr_cmt_l3 l3;
 };
 
+enum cbm_type {
+    PSR_CBM_TYPE_L3         = 1,
+    PSR_CBM_TYPE_L3_CODE    = 2,
+    PSR_CBM_TYPE_L3_DATA    = 3,
+};
+
 extern struct psr_cmt *psr_cmt;
 
 static inline bool_t psr_cmt_enabled(void)
@@ -62,8 +68,10 @@ void psr_ctxt_switch_to(struct domain *d);
 
 int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
                         uint32_t *cos_max, uint32_t *flags);
-int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm);
-int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm);
+int psr_get_l3_cbm(struct domain *d, unsigned int socket,
+                   uint64_t *cbm, enum cbm_type type);
+int psr_set_l3_cbm(struct domain *d, unsigned int socket,
+                   uint64_t cbm, enum cbm_type type);
 
 int psr_domain_init(struct domain *d);
 void psr_domain_free(struct domain *d);
xen/include/public/domctl.h +4 -0
index 675f021..f438cae 100644
@@ -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 */
Signature · 1 line
1.9.1
author
#7 lore raw

PATCH v3 3/4 tools: add tools support for Intel CDP

In reply to He Chen

This is the xl/xc changes to support Intel Code/Data Prioritization. CAT xl commands to set/get CBMs are extended to support CDP.

---
 tools/libxc/include/xenctrl.h |  7 +++++--
 tools/libxc/xc_psr.c          | 17 ++++++++++++++-
 tools/libxl/libxl.h           |  1 +
 tools/libxl/libxl_psr.c       | 20 +++++++++++++++++-
 tools/libxl/libxl_types.idl   |  3 +++
 tools/libxl/xl_cmdimpl.c      | 49 ++++++++++++++++++++++++++++++++++---------
 tools/libxl/xl_cmdtable.c     |  3 +++
 7 files changed, 86 insertions(+), 14 deletions(-)
tools/libxc/include/xenctrl.h +5 -2
index de3c0ad..54f2069 100644
@@ -2798,7 +2798,9 @@ enum xc_psr_cmt_type {
 typedef enum xc_psr_cmt_type xc_psr_cmt_type;
 
 enum xc_psr_cat_type {
-    XC_PSR_CAT_L3_CBM = 1,
+    XC_PSR_CAT_L3_CBM  = 1,
+    XC_PSR_CAT_L3_CODE = 2,
+    XC_PSR_CAT_L3_DATA = 3,
 };
 typedef enum xc_psr_cat_type xc_psr_cat_type;
 
@@ -2824,7 +2826,8 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
                                xc_psr_cat_type type, uint32_t target,
                                uint64_t *data);
 int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
-                           uint32_t *cos_max, uint32_t *cbm_len);
+                           uint32_t *cos_max, uint32_t *cbm_len,
+                           bool *cdp_enabled);
 #endif
 
 #endif /* XENCTRL_H */
tools/libxc/xc_psr.c +16 -1
index d8b3a51..5bbe950 100644
@@ -260,6 +260,12 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
     case XC_PSR_CAT_L3_CBM:
         cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM;
         break;
+    case XC_PSR_CAT_L3_CODE:
+        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE;
+        break;
+    case XC_PSR_CAT_L3_DATA:
+        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA;
+        break;
     default:
         errno = EINVAL;
         return -1;
@@ -287,6 +293,12 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
     case XC_PSR_CAT_L3_CBM:
         cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM;
         break;
+    case XC_PSR_CAT_L3_CODE:
+        cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE;
+        break;
+    case XC_PSR_CAT_L3_DATA:
+        cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA;
+        break;
     default:
         errno = EINVAL;
         return -1;
@@ -306,7 +318,8 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
 }
 
 int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
-                           uint32_t *cos_max, uint32_t *cbm_len)
+                           uint32_t *cos_max, uint32_t *cbm_len,
+                           bool *cdp_enabled)
 {
     int rc;
     DECLARE_SYSCTL;
@@ -320,6 +333,8 @@ int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
     {
         *cos_max = sysctl.u.psr_cat_op.u.l3_info.cos_max;
         *cbm_len = sysctl.u.psr_cat_op.u.l3_info.cbm_len;
+        *cdp_enabled = sysctl.u.psr_cat_op.u.l3_info.flags &
+                       XEN_SYSCTL_PSR_CAT_L3_CDP;
     }
 
     return rc;
tools/libxl/libxl.h +1 -0
index 5f9047c..d824105 100644
@@ -796,6 +796,7 @@ 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
+
 #endif
 
 /*
tools/libxl/libxl_psr.c +19 -1
index 3378239..140af4d 100644
@@ -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 &&
+       (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
+        type == LIBXL_PSR_CBM_TYPE_L3_DATA))
+    {
+            LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
+            rc = EINVAL;
+            free(info);
+            goto out;
+    }
+
     libxl_for_each_set_bit(socketid, *target_map) {
         if (socketid >= nr_sockets)
             break;
@@ -312,6 +329,7 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
             rc = ERROR_FAIL;
         }
     }
+    free(info);
 
 out:
     GC_FREE;
@@ -352,7 +370,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);
tools/libxl/libxl_types.idl +3 -0
index ef346e7..fa017ad 100644
@@ -787,9 +787,12 @@ libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
 libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
     (0, "UNKNOWN"),
     (1, "L3_CBM"),
+    (2, "L3_CODE"),
+    (3, "L3_DATA"),
     ])
 
 libxl_psr_cat_info = Struct("psr_cat_info", [
     ("cos_max", uint32),
     ("cbm_len", uint32),
+    ("cdp_enabled", bool),
     ])
tools/libxl/xl_cmdimpl.c +39 -10

113 lines not on this page · raw message

tools/libxl/xl_cmdtable.c +3 -0
index 0071f12..13e7aa1 100644
@@ -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,7 @@ struct cmd_spec cmd_table[] = {
     },
 
 #endif
+
 };
 
 int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
Signature · 1 line
1.9.1
author
#9 lore raw

PATCH v3 4/4 docs: add document to introduce CDP command

In reply to He Chen

Add new CDP option with CAT commands in xl interface man page. Add description of CDP in xl-psr.markdown.

---
 docs/man/xl.pod.1         | 14 ++++++++++++++
 docs/misc/xl-psr.markdown | 42 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 51 insertions(+), 5 deletions(-)
docs/man/xl.pod.1 +14 -0
index f22c3f3..3801039 100644
@@ -1530,6 +1530,12 @@ applications. In the Xen implementation, CAT is used to control cache allocation
 on VM basis. To enforce cache on a specific domain, just set capacity bitmasks
 (CBM) for the domain.
 
+Intel Broadwell and later server platforms also offer Code/Data Prioritization
+(CDP) for cache allocation, which support specify code or data cache for
+applications. CDP is used on VM basis in the Xen implementation. To specify
+code or data CBM for the domain, CDP feature must be enabled and CBM type
+options need to be specified when setting CBM.
+
 =over 4
 
 =item B<psr-cat-cbm-set> [I<OPTIONS>] I<domain-id> I<cbm>
@@ -1545,6 +1551,14 @@ B<OPTIONS>
 
 Specify the socket to process, otherwise all sockets are processed.
 
+=item B<-c>, B<--code>
+
+Set code CBM with CDP enabled.
+
+=item B<-d>, B<--data>
+
+Set data CBM with CDP enabled.
+
 =back
 
 =item B<psr-cat-show> [I<domain-id>]
docs/misc/xl-psr.markdown +37 -5

67 lines not on this page · raw message

Signature · 1 line
1.9.1
#2 lore raw

Re: [PATCH v3 1/4] x86: Support enable CDP by boot parameter and add get CDP status

In reply to He Chen
Quoted 13 lines · He Chen
docs/misc/xen-command-line.markdown +2 -2
@@ -1165,9 +1165,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.

docs/misc/xen-command-line.markdown The following resources are available: · line 1202

Quoted 6 lines
docs/misc/xen-command-line.markdown +3 -0
@@ -1197,6 +1197,10 @@ 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.

It's better to have some description for cos_max here (e.g. if the meaning is the same for CAT and CDP).

xen/arch/x86/psr.c static void cat_cpu_init(void) · line 499

Quoted 4 lines
xen/arch/x86/psr.c +0 -2
         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);

Then there will be no output in CAT-only mode. Probably not good just to remove it. I guess you can add cdp information to it and then move it to the end of the function.

xen/arch/x86/psr.c static void cat_cpu_init(void) · line 539

Quoted 13 lines
xen/arch/x86/psr.c +13 -0
+
+        if ( (ecx & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
+        {
+            if ( test_bit(socket, cdp_socket_enable) )
+                return;
+
+            rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
+            wrmsrl(MSR_IA32_PSR_L3_QOS_CFG, val | 1 << PSR_L3_QOS_CDP_ENABLE_BIT);
+
+            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;
+
+            /* We only write mask1 since mask0 is always all ones by default */

Missing '.'

xen/arch/x86/psr.c static void cat_cpu_init(void) · line 542

Quoted 3 lines
xen/arch/x86/psr.c +3 -0
+            wrmsrl(MSR_IA32_PSR_L3_MASK(1), (1ull << info->cbm_len) - 1);
+
+            /* Cut half of cos_max when CDP enabled */

Ditto.

Quoted 10 lines
xen/arch/x86/psr.c +6 -0
+            info->cos_max = info->cos_max / 2;
+
+            set_bit(socket, cdp_socket_enable);
+            printk(XENLOG_INFO "CDP: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
+                   socket, info->cos_max, info->cbm_len);
+        }
     }
 }

xen/arch/x86/psr.c static void cat_cpu_fini(unsigned int cpu) · line 561

Quoted 5 lines
xen/arch/x86/psr.c +2 -0
@@ -508,6 +557,8 @@ static void cat_cpu_fini(unsigned int cpu)
     {
         struct psr_cat_socket_info *info = cat_socket_info + socket;
 
+        clear_bit(socket, cdp_socket_enable);
+

It's better to move this below, together with that of cat_socket_enable.

Quoted 11 lines
xen/arch/x86/psr.c +2 -0
         if ( info->cos_to_cbm )
         {
             xfree(info->cos_to_cbm);
@@ -523,6 +574,8 @@ static void __init psr_cat_free(void)
     cat_socket_enable = NULL;
     xfree(cat_socket_info);
     cat_socket_info = NULL;
+    xfree(cdp_socket_enable);
+    cdp_socket_enable = NULL;
 }
 
Reviewed-by on v3 2/4
#5 lore raw

Re: [PATCH v3 2/4] x86: add domctl cmd to set/get CDP code/data CBM

In reply to He Chen
Quoted 2 lines · He Chen

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

                                    ^s
Quoted 1 line

is added to support distinguish different CBM operation. Besides, new

^'support' can be ommitted. ^s

xen/arch/x86/psr.c static int write_l3_cbm(unsigned int socket, unsigned int cos, · line 427

Quoted 11 lines

domctl cmds are introdunced to support set/get CDP CBM. Some CAT functions to operation CBMs are extended to support CDP.

xen/arch/x86/psr.c +6 -0
+
+    /* If old cos is referred only by the domain, then use it. */
+    if ( map[old_cos].ref == 1 )
+        return old_cos;
+
+    /* Find an unused one other than cos0 */

Missing stop.

Quoted 20 lines
xen/arch/x86/psr.c +16 -0
+    for ( cos = 1; cos <= cos_max; cos++ )
+        if ( map[cos].ref == 0 )
+            return cos;
+
+    return -ENOENT;
+}
+
+int psr_set_l3_cbm(struct domain *d, unsigned int socket,
+                   uint64_t cbm, enum cbm_type type)
+{
+    unsigned int old_cos, cos_max;
+    int cos, ret;
+    uint64_t cbm_data, cbm_code;
+    bool_t need_write = 1;
+    bool_t cdp_enabled = test_bit(socket, cdp_socket_enable);
+    struct psr_cat_cbm *map;
     struct psr_cat_socket_info *info = get_cat_socket_info(socket);
 
     if ( IS_ERR(info) )

xen/arch/x86/psr.c int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm) · line 453

Quoted 5 lines
xen/arch/x86/psr.c +2 -0
@@ -381,53 +449,79 @@ 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) )

Nice code can be made by aligning PSR_CBM_TYPE_L3_DATA with PSR_CBM_TYPE_L3_DATA.

xen/arch/x86/psr.c int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm) · line 461

Quoted 10 lines
xen/arch/x86/psr.c +4 -2
+        return -EINVAL;
+
+    cos_max = info->cos_max;
     old_cos = d->arch.psr_cos_ids[socket];
     map = info->cos_to_cbm;
 
-    spin_lock(&info->cbm_lock);
 
-    for ( cos = 0; cos <= info->cos_max; cos++ )
+    switch( type )

Coding style.

xen/arch/x86/psr.c int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm) · line 501

Quoted 9 lines
xen/arch/x86/psr.c +9 -0
+    {
+        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 */

Missing stop.

Overall it looks only some minor issues, with these being fixed, then

Chao

#8 lore raw

Re: [PATCH v3 3/4] tools: add tools support for Intel CDP

In reply to He Chen
Quoted 7 lines · He Chen

This is the xl/xc changes to support Intel Code/Data Prioritization. CAT xl commands to set/get CBMs are extended to support CDP.

---
#define LIBXL_HAVE_PSR_CAT 1
+

? And you still need to broadcast the feature, just like what you have done in v2 (e.g. to define LIBXL_HAVE_PSR_CDP).

Quoted 17 lines

#endif

/*

tools/libxl/libxl_psr.c +1 -0
index 3378239..140af4d 100644
@@ -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;

tools/libxl/libxl_psr.c int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid, · line 315

Quoted 10 lines
tools/libxl/libxl_psr.c +8 -0
     }
 
+    rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
+    if (rc) {
+        LOGE(ERROR, "Failed to get cat info");
+        goto out;
+    }
+
+    if (!info->cdp_enabled &&
+       (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||

Does this align up correctly?

Chao

#10 lore raw

Re: [PATCH v3 4/4] docs: add document to introduce CDP command

In reply to He Chen
Quoted 1 line · He Chen

Add new CDP option with CAT commands in xl interface man page.

^s to

Quoted 13 lines

Add description of CDP in xl-psr.markdown.

---
 docs/man/xl.pod.1         | 14 ++++++++++++++
 docs/misc/xl-psr.markdown | 42 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 51 insertions(+), 5 deletions(-)
docs/man/xl.pod.1 +0 -0
index f22c3f3..3801039 100644

docs/man/xl.pod.1 applications. In the Xen implementation, CAT is used to control cache allocation · line 1534

Quoted 5 lines
docs/man/xl.pod.1 +2 -0
@@ -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

^ing

Quoted 8 lines
docs/man/xl.pod.1 +4 -0
+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>

docs/man/xl.pod.1 B<OPTIONS> · line 1556

Quoted 6 lines
docs/man/xl.pod.1 +3 -0
@@ -1545,6 +1551,14 @@ B<OPTIONS>
 
 Specify the socket to process, otherwise all sockets are processed.
 
+=item B<-c>, B<--code>
+
+Set code CBM with CDP enabled.

^when CDP is enabled.

docs/man/xl.pod.1 B<OPTIONS> · line 1560

Quoted 4 lines
docs/man/xl.pod.1 +4 -0
+
+=item B<-d>, B<--data>
+
+Set data CBM with CDP enabled.

Ditto.

Quoted 29 lines
docs/man/xl.pod.1 +1 -0
+
 =back
 
 =item B<psr-cat-show> [I<domain-id>]
docs/misc/xl-psr.markdown +1 -4
index 3545912..538ea60 100644
@@ -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,42 @@ 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.
 

docs/misc/xl-psr.markdown For example, assuming a system with 8 portions and 3 domains: · line 108

Quoted 10 lines
docs/misc/xl-psr.markdown +10 -0
+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.

The last sentence can be ommited I think. You can however add such sentence:

It can be enabled by adding 'cdp=1' to Xen bootline.

docs/misc/xl-psr.markdown For example, assuming a system with 8 portions and 3 domains: · line 121

Quoted 13 lines
docs/misc/xl-psr.markdown +13 -0
+
+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. With CDP enabled,
+one COS corresponds to two CBMs (code CBM & data CBM), which means the number
+of available COS will reduce to half when CDP is on.

This paragraph can be improved somehow.

Quoted 13 lines
docs/misc/xl-psr.markdown +6 -1
+
+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`
 

docs/misc/xl-psr.markdown A cbm is valid only when: · line 153

Quoted 10 lines
docs/misc/xl-psr.markdown +7 -0
@@ -119,6 +144,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.
+

I guess you can move these staff to the end of description for valid cbm, right before "In a multi-socket system". And indicate they are OPTIONS.

Chao

#11 lore raw
In reply to He Chen
Quoted 31 lines · He Chen

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 patchset has been tested on Intel Broadwell server platform.

Quoted 7 lines

To make this patchset better, any comment or suggestion is welcomed, I would really appreciate it.

Thanks

He Chen (4):
x86: Support enable CDP by boot parameter and add get CDP status

The subject can simply be:
x86: detect and initialize CDP (Code/Data Prioritization) feature

Chao

Quoted 31 lines

x86: add domctl cmd to set/get CDP code/data CBM
tools: add tools support for Intel CDP
docs: add document to introduce CDP command

 docs/man/xl.pod.1                   |  14 +++
 docs/misc/xen-command-line.markdown |   8 +-
 docs/misc/xl-psr.markdown           |  42 ++++++-
 tools/libxc/include/xenctrl.h       |   7 +-
 tools/libxc/xc_psr.c                |  17 ++-
 tools/libxl/libxl.h                 |   1 +
 tools/libxl/libxl_psr.c             |  20 ++-
 tools/libxl/libxl_types.idl         |   3 +
 tools/libxl/xl_cmdimpl.c            |  49 ++++++--
 tools/libxl/xl_cmdtable.c           |   3 +
 xen/arch/x86/domctl.c               |  32 ++++-
 xen/arch/x86/psr.c                  | 240 +++++++++++++++++++++++++++++-------
 xen/arch/x86/sysctl.c               |   5 +-
 xen/include/asm-x86/msr-index.h     |   3 +
 xen/include/asm-x86/psr.h           |  23 +++-
 xen/include/public/domctl.h         |   4 +
 xen/include/public/sysctl.h         |   2 +
 17 files changed, 399 insertions(+), 74 deletions(-)
Signature · 7 lines
1.9.1


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
#3 lore raw

Re: [PATCH v3 1/4] x86: Support enable CDP by boot parameter and add get CDP status

In reply to He Chen
Quoted 8 lines
Quoted 1 line
Quoted 1 line · On 14.09.15 at 05:27, <[email protected]> wrote:
xen/arch/x86/psr.c +1 -0
@@ -21,9 +21,16 @@
 
 #define PSR_CMT        (1<<0)
 #define PSR_CAT        (1<<1)
+#define PSR_CDP        (1<<2)

xen/arch/x86/psr.c line 33

Quoted 10 lines
xen/arch/x86/psr.c +7 -1
 
 struct psr_cat_cbm {
-    uint64_t cbm;
+    union {
+        uint64_t cbm;
+        struct {
+            uint64_t code;
+            uint64_t data;
+        } cdp;
+    } u;

While in the public interfaces use of gcc extensions is not allowed without very special care, please make use of such in internal headers when that helps readability (as well as in the case patch size): No need to name the union member of the outer structure.

xen/arch/x86/psr.c static void cat_cpu_init(void) · line 499

Quoted 12 lines
xen/arch/x86/psr.c +1 -3
@@ -490,13 +519,33 @@ static void cat_cpu_init(void)
         info->cos_to_cbm = temp_cos_to_cbm;
         temp_cos_to_cbm = NULL;
         /* cos=0 is reserved as default cbm(all ones). */
-        info->cos_to_cbm[0].cbm = (1ull << info->cbm_len) - 1;
+        info->cos_to_cbm[0].u.cbm = (1ull << info->cbm_len) - 1;
 
         spin_lock_init(&info->cbm_lock);
 
         set_bit(socket, cat_socket_enable);
-        printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
-               socket, info->cos_max, info->cbm_len);

I fail to see a replacement for this message.

xen/arch/x86/psr.c static void cat_cpu_init(void) · line 531

Quoted 4 lines
xen/arch/x86/psr.c +4 -0
+        if ( (ecx & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
+        {
+            if ( test_bit(socket, cdp_socket_enable) )
+                return;

This will make future changes more cumbersome, especially if one would not require CDP as a prereq. Please fold the two if()s, allowing execution to progress past the outer if() in all cases.

xen/arch/x86/psr.c static void cat_cpu_init(void) · line 534

Quoted 2 lines
xen/arch/x86/psr.c +2 -0
+            rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
+            wrmsrl(MSR_IA32_PSR_L3_QOS_CFG, val | 1 << PSR_L3_QOS_CDP_ENABLE_BIT);

Missing parentheses around the <<.

xen/arch/x86/psr.c static void cat_cpu_init(void) · line 543

Quoted 8 lines
xen/arch/x86/psr.c +8 -0
+            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;
+
+            /* We only write mask1 since mask0 is always all ones by default */
+            wrmsrl(MSR_IA32_PSR_L3_MASK(1), (1ull << info->cbm_len) - 1);
+
+            /* Cut half of cos_max when CDP enabled */
+            info->cos_max = info->cos_max / 2;

Please use /= or >>=.

xen/arch/x86/psr.c static void __init init_psr_cat(void) · line 594

Quoted 9 lines
xen/arch/x86/psr.c +2 -1
@@ -535,8 +588,9 @@ static void __init init_psr_cat(void)
 
     cat_socket_enable = xzalloc_array(unsigned long, BITS_TO_LONGS(nr_sockets));
     cat_socket_info = xzalloc_array(struct psr_cat_socket_info, nr_sockets);
+    cdp_socket_enable = xzalloc_array(unsigned long, BITS_TO_LONGS(nr_sockets));
 
-    if ( !cat_socket_enable || !cat_socket_info )
+    if ( !cat_socket_enable || !cat_socket_info || !cdp_socket_enable )
         psr_cat_free();

No - just disable CDP if only that third allocation fails.

xen/include/asm-x86/msr-index.h line 334

Quoted 10 lines
xen/include/asm-x86/msr-index.h +3 -0
@@ -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))

Please always fully parenthesize uses of macro parameters (in fact the parentheses you have could simply be moved to isolate just n).

xen/include/public/sysctl.h struct xen_sysctl_psr_cat_op { · line 709

Quoted 9 lines
xen/include/public/sysctl.h +2 -0
@@ -704,6 +704,8 @@ struct xen_sysctl_psr_cat_op {
         struct {
             uint32_t cbm_len;   /* OUT: CBM length */
             uint32_t cos_max;   /* OUT: Maximum COS */
+            #define XEN_SYSCTL_PSR_CAT_L3_CDP       (1u << 0)
+            uint32_t flags;     /* OUT: CAT flags */
         } l3_info;

Even if only mildly incompatible, any interface change to sysctl (or domctl) should make sure the respective interface version got bumped after branching the previous release tree.

Jan

#6 lore raw

Re: [PATCH v3 2/4] x86: add domctl cmd to set/get CDP code/data CBM

In reply to He Chen
Quoted 14 lines
Quoted 1 line
Quoted 1 line · On 14.09.15 at 05:27, <[email protected]> wrote:
xen/arch/x86/psr.c +3 -1
@@ -288,14 +288,39 @@ 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 = test_bit(socket, cdp_socket_enable);
 

xen/arch/x86/psr.c int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len, · line 305

Quoted 10 lines
xen/arch/x86/psr.c +6 -1
     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 && cdp_enabled )
+        return -EXDEV;
+
+    if ( (type == PSR_CBM_TYPE_L3_CODE || type == PSR_CBM_TYPE_L3_DATA)
+        && !cdp_enabled )
+        return -EXDEV;

These checks really ought to go into the subsequent switch(). But then I wonder why asking for code or data would be wrong when !cdp_enabled? You could simple return the same mask for all three variants in that case.

Quoted 6 lines
xen/arch/x86/psr.c +6 -0
+    switch ( type )
+    {
+        case PSR_CBM_TYPE_L3:
+            *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
+            break;
+

xen/arch/x86/psr.c int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len, · line 322

Quoted 10 lines
xen/arch/x86/psr.c +10 -0
+        case PSR_CBM_TYPE_L3_CODE:
+            *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
+            break;
+
+        case PSR_CBM_TYPE_L3_DATA:
+            *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
+            break;
+
+        default:
+            return -EINVAL;

Considering that this is a helper function and "type" comes from a caller inside the hypervisor, I'd really like to see an ASSERT_UNREACHABLE() here.

Quoted 15 lines
xen/arch/x86/psr.c +7 -3
@@ -369,10 +394,53 @@ static int write_l3_cbm(unsigned int socket, unsigned int cos,
     return 0;
 }
 
-int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
+static int find_cos(struct psr_cat_cbm *map, int cos_max,
+                    uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
 {
-    unsigned int old_cos, cos;
-    struct psr_cat_cbm *map, *found = NULL;
+    unsigned int cos;
+
+    if ( !cdp_enabled )
+    {
+        for ( cos = 0; cos <= cos_max; cos++ )

xen/arch/x86/psr.c static int write_l3_cbm(unsigned int socket, unsigned int cos, · line 414

Quoted 10 lines
xen/arch/x86/psr.c +10 -0
+            if ( map[cos].ref && map[cos].u.cbm == cbm_code )
+                return cos;
+    }
+    else
+    {
+        for ( cos = 0; cos <= cos_max; cos++ )
+            if ( map[cos].ref && map[cos].u.cdp.code == cbm_code &&
+                 map[cos].u.cdp.data == cbm_data )
+                return cos;
+    }

I think having just a single loop here with a suitable conditional expression inside the if() would both shrink code size and allow the compiler to produce better (smaller) code.

xen/arch/x86/psr.c static int write_l3_cbm(unsigned int socket, unsigned int cos, · line 421

Quoted 3 lines
xen/arch/x86/psr.c +3 -0
+static int pick_avail_cos(struct psr_cat_cbm *map, int cos_max, int old_cos)
+{
+    int cos;

In the function right above "cos" was unsigned int, which I think is the correct type (also for "old_cos" and "cos_max").

xen/arch/x86/psr.c static int write_l3_cbm(unsigned int socket, unsigned int cos, · line 439

Quoted 5 lines
xen/arch/x86/psr.c +5 -0
+int psr_set_l3_cbm(struct domain *d, unsigned int socket,
+                   uint64_t cbm, enum cbm_type type)
+{
+    unsigned int old_cos, cos_max;
+    int cos, ret;

Whereas here I can see why it should be plain int.

Quoted 22 lines
xen/arch/x86/psr.c +6 -7
@@ -381,53 +449,79 @@ 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 -EINVAL;
+
+    cos_max = info->cos_max;
     old_cos = d->arch.psr_cos_ids[socket];
     map = info->cos_to_cbm;
 
-    spin_lock(&info->cbm_lock);
 
-    for ( cos = 0; cos <= info->cos_max; cos++ )
+    switch( type )
     {
-        /* If still not found, then keep unused one. */
-        if ( !found && cos != 0 && map[cos].ref == 0 )
-            found = map + cos;
-        else if ( map[cos].u.cbm == cbm )
-        {

xen/arch/x86/psr.c int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm) · line 465

Quoted 10 lines
xen/arch/x86/psr.c +3 -7
-            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;

Wrong indentation.

Quoted 25 lines
xen/arch/x86/psr.c +11 -9
             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].u.cdp.data;
+            break;
 
-    if ( !found )
-    {
-        spin_unlock(&info->cbm_lock);
-        return -EOVERFLOW;
+        case PSR_CBM_TYPE_L3_DATA:
+            cbm_code = map[old_cos].u.cdp.code;
+            cbm_data = cbm;
+            break;
+
+        default:
+            return -EINVAL;
     }
 

xen/arch/x86/psr.c int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm) · line 486

Quoted 10 lines
xen/arch/x86/psr.c +4 -5
-    cos = found - map;
-    if ( found->u.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, 0, 0);
-
-        if ( ret )
+        if ( unlikely(cos == old_cos) )

Are you sure about the unlikely() here?

Quoted 14 lines
xen/arch/x86/psr.c +11 -1
         {
             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;
+        }

xen/arch/x86/psr.c int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm) · line 509

Quoted 10 lines
xen/arch/x86/psr.c +10 -0
+
+        /* We try to avoid writing MSR */
+        if ( 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;

Please write such without conditional expression (a comparison on the right side of an assignment is quite fine). Also the scope of "need_write" would better be reduced to the minimal required one.

xen/include/asm-x86/psr.h struct psr_cmt { · line 55

Quoted 10 lines
xen/include/asm-x86/psr.h +4 -0
@@ -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,

It doesn't look like you require these values to be 1, 2, and 3 respectively. Please omit explicit values in such cases.

Jan