* [v2 3/3] tools & docs: add L2 CAT support in tools and docs.
@ 2016-09-22 2:17 Yi Sun
2016-09-22 10:09 ` Ian Jackson
0 siblings, 1 reply; 4+ messages in thread
From: Yi Sun @ 2016-09-22 2:17 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, he.chen, andrew.cooper3, ian.jackson, Yi Sun, jbeulich,
chao.p.peng
This patch is the xl/xc changes to support Intel L2 CAT
(Cache Allocation Technology).
The new level option is introduced to original CAT setting
command in order to set CBM for specified level CAT.
- 'xl psr-hwinfo' is updated to show both L3 CAT and L2 CAT
info.
- 'xl psr-cat-cbm-set' is updated to set cache capacity
bitmasks(CBM) for a domain according to input cache level.
- 'xl psr-cat-show' is updated to show CBM of a domain
according to input cache level.
Examples:
root@:~$ xl psr-hwinfo --cat
Cache Allocation Technology (CAT): L2
Socket ID : 0
Maximum COS : 3
CBM length : 8
Default CBM : 0xff
root@:~$ xl psr-cat-cbm-set -l2 1 0x7f
root@:~$ xl psr-cat-show -l2 1
Socket ID : 0
Default CBM : 0xff
ID NAME CBM
1 ubuntu14 0x7f
Signed-off-by: He Chen <he.chen@linux.intel.com>
Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
---
Changed since v1:
* xl_cmdimpl.c
- Add prompt message to input cache level.
* xc_psr.c
- Adjust codes to make it easier to read.
---
docs/man/xl.pod.1.in | 25 +++++-
docs/misc/xl-psr.markdown | 10 ++-
tools/libxc/include/xenctrl.h | 7 +-
tools/libxc/xc_psr.c | 47 +++++++---
tools/libxl/libxl.h | 2 +
tools/libxl/libxl_psr.c | 50 ++++++++++-
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl_cmdimpl.c | 195 +++++++++++++++++++++++++++++++++++-------
tools/libxl/xl_cmdtable.c | 4 +-
9 files changed, 287 insertions(+), 54 deletions(-)
diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index a2be541..f7887ac 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -1696,6 +1696,9 @@ occupancy monitoring share the same set of underlying monitoring service. Once
a domain is attached to the monitoring service, monitoring data can be shown
for any of these monitoring types.
+There is no cache monitoring and memory bandwidth monitoring on L2 cache so
+far.
+
=over 4
=item B<psr-cmt-attach> [I<domain-id>]
@@ -1720,7 +1723,7 @@ monitor types are:
Intel Broadwell and later server platforms offer capabilities to configure and
make use of the Cache Allocation Technology (CAT) mechanisms, which enable more
-cache resources (i.e. L3 cache) to be made available for high priority
+cache resources (i.e. L3/L2 cache) to be made available for high priority
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.
@@ -1730,7 +1733,7 @@ Intel Broadwell and later server platforms also offer Code/Data Prioritization
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.
+and data) are mutually exclusive. There is no CDP support on L2 so far.
=over 4
@@ -1747,6 +1750,11 @@ B<OPTIONS>
Specify the socket to process, otherwise all sockets are processed.
+=item B<-l LEVEL>, B<--level=LEVEL>
+
+Specify the cache level to process, otherwise the last level cache is
+processed.
+
=item B<-c>, B<--code>
Set code CBM when CDP is enabled.
@@ -1757,10 +1765,21 @@ Set data CBM when CDP is enabled.
=back
-=item B<psr-cat-show> [I<domain-id>]
+=item B<psr-cat-show> [I<OPTIONS>] [I<domain-id>]
Show CAT settings for a certain domain or all domains.
+B<OPTIONS>
+
+=over 4
+
+=item B<-l LEVEL>, B<--level=LEVEL>
+
+Specify the cache level to process, otherwise the last level cache is
+processed.
+
+=back
+
=back
=head1 IGNORED FOR COMPATIBILITY WITH XM
diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
index c3c1e8e..bd2b6bd 100644
--- a/docs/misc/xl-psr.markdown
+++ b/docs/misc/xl-psr.markdown
@@ -70,7 +70,7 @@ total-mem-bandwidth instead of cache-occupancy). E.g. after a `xl psr-cmt-attach
Cache Allocation Technology (CAT) is a new feature available on Intel
Broadwell and later server platforms that allows an OS or Hypervisor/VMM to
-partition cache allocation (i.e. L3 cache) based on application priority or
+partition cache allocation (i.e. L3/L2 cache) based on application priority or
Class of Service (COS). Each COS is configured using capacity bitmasks (CBM)
which represent cache capacity and indicate the degree of overlap and
isolation between classes. System cache resource is divided into numbers of
@@ -119,13 +119,19 @@ 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.
+In different systems, the different cache level is supported, e.g. L3 cache or
+L2 cache. Per cache level cbm can be specified with the `--level LEVEL` option.
+
Setting the CBM may not be successful if insufficient COS is available. In
such case unused COS(es) may be freed by setting CBM of all related domains to
its default value(all-ones).
Per domain CBM settings can be shown by:
-`xl psr-cat-show`
+`xl psr-cat-show [OPTIONS] <domid>`
+
+In different systems, the different cache level is supported, e.g. L3 cache or
+L2 cache. Per cache level cbm can be specified with the `--level LEVEL` option.
## Code and Data Prioritization (CDP)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 560ce7b..d70e8a8 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2585,6 +2585,7 @@ enum xc_psr_cat_type {
XC_PSR_CAT_L3_CBM = 1,
XC_PSR_CAT_L3_CBM_CODE = 2,
XC_PSR_CAT_L3_CBM_DATA = 3,
+ XC_PSR_CAT_L2_CBM = 4,
};
typedef enum xc_psr_cat_type xc_psr_cat_type;
@@ -2609,9 +2610,9 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
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,
- bool *cdp_enabled);
+int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, int lvl,
+ uint32_t *cos_max, uint32_t *cbm_len,
+ bool *cdp_enabled);
int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps);
int xc_get_cpu_featureset(xc_interface *xch, uint32_t index,
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 43b3286..758f2fc 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -266,6 +266,9 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
case XC_PSR_CAT_L3_CBM_DATA:
cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA;
break;
+ case XC_PSR_CAT_L2_CBM:
+ cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L2_CBM;
+ break;
default:
errno = EINVAL;
return -1;
@@ -299,6 +302,9 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
case XC_PSR_CAT_L3_CBM_DATA:
cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA;
break;
+ case XC_PSR_CAT_L2_CBM:
+ cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L2_CBM;
+ break;
default:
errno = EINVAL;
return -1;
@@ -317,24 +323,41 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
return rc;
}
-int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
- uint32_t *cos_max, uint32_t *cbm_len,
- bool *cdp_enabled)
+#define L3_CAT 3
+#define L2_CAT 2
+int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, int lvl,
+ uint32_t *cos_max, uint32_t *cbm_len,
+ bool *cdp_enabled)
{
- int rc;
+ int rc = -1;
DECLARE_SYSCTL;
sysctl.cmd = XEN_SYSCTL_psr_cat_op;
- sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
sysctl.u.psr_cat_op.target = socket;
- rc = xc_sysctl(xch, &sysctl);
- if ( !rc )
- {
- *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;
+ switch ( lvl ) {
+ case L2_CAT:
+ sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l2_info;
+ rc = xc_sysctl(xch, &sysctl);
+ if ( !rc )
+ {
+ *cos_max = sysctl.u.psr_cat_op.u.l2_info.cos_max;
+ *cbm_len = sysctl.u.psr_cat_op.u.l2_info.cbm_len;
+ }
+ break;
+ case L3_CAT:
+ sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
+ rc = xc_sysctl(xch, &sysctl);
+ if ( !rc )
+ {
+ *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;
+ }
+ break;
+ default:
+ return rc;
}
return rc;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 7cfa540..b154267 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -2147,6 +2147,8 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
*/
int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
int *nr);
+int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
+ int *nr, int lvl);
void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
#endif
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 99733f6..861d5a8 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -379,8 +379,54 @@ 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].cdp_enabled)) {
+ if (xc_psr_cat_get_info(ctx->xch, socketid, 3, &ptr[i].cos_max,
+ &ptr[i].cbm_len, &ptr[i].cdp_enabled)) {
+ libxl__psr_cat_log_err_msg(gc, errno);
+ rc = ERROR_FAIL;
+ free(ptr);
+ goto out;
+ }
+ i++;
+ }
+
+ *info = ptr;
+ *nr = i;
+out:
+ libxl_bitmap_dispose(&socketmap);
+ GC_FREE;
+ return rc;
+}
+
+int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
+ int *nr, int lvl)
+{
+ GC_INIT(ctx);
+ int rc;
+ int i = 0, socketid, nr_sockets;
+ libxl_bitmap socketmap;
+ libxl_psr_cat_info *ptr;
+
+ libxl_bitmap_init(&socketmap);
+
+ rc = libxl__count_physical_sockets(gc, &nr_sockets);
+ if (rc) {
+ LOGE(ERROR, "failed to get system socket count");
+ goto out;
+ }
+
+ libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets);
+ rc = libxl_get_online_socketmap(ctx, &socketmap);
+ if (rc < 0) {
+ LOGE(ERROR, "failed to get available sockets");
+ goto out;
+ }
+
+ ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_cat_info));
+
+ libxl_for_each_set_bit(socketid, socketmap) {
+ ptr[i].id = socketid;
+ if (xc_psr_cat_get_info(ctx->xch, socketid, lvl, &ptr[i].cos_max,
+ &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 98bfc3a..4503a8b 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -890,6 +890,7 @@ libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
(1, "L3_CBM"),
(2, "L3_CBM_CODE"),
(3, "L3_CBM_DATA"),
+ (4, "L2_CBM"),
])
libxl_psr_cat_info = Struct("psr_cat_info", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 62237d0..cc31906 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -9273,18 +9273,124 @@ int main_psr_cmt_show(int argc, char **argv)
#endif
#ifdef LIBXL_HAVE_PSR_CAT
-static int psr_cat_hwinfo(void)
+static void psr_cat_print_one_domain_cbm_type(uint32_t domid, uint32_t socketid,
+ libxl_psr_cbm_type type)
+{
+ uint64_t cbm;
+
+ if (!libxl_psr_cat_get_cbm(ctx, domid, type, socketid, &cbm))
+ printf("%#16"PRIx64, cbm);
+ else
+ printf("%16s", "error");
+}
+
+static int psr_l2_cat_hwinfo(void)
+{
+ int rc;
+ int i, nr;
+ libxl_psr_cat_info *info;
+
+ printf("Cache Allocation Technology (CAT): L2\n");
+
+ rc = libxl_psr_cat_get_info(ctx, &info, &nr, 2);
+ if (rc) {
+ fprintf(stderr, "Failed to get l2 cat info\n");
+ return rc;
+ }
+
+ for (i = 0; i < nr; i++) {
+ /* There is no CMT on L2 cache so far. */
+ printf("%-16s: %u\n", "Socket ID", info[i].id);
+ 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",
+ (1ull << info[i].cbm_len) - 1);
+ }
+
+ libxl_psr_cat_info_list_free(info, nr);
+ return rc;
+}
+
+static void psr_l2_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid)
+{
+ char *domain_name;
+
+ domain_name = libxl_domid_to_name(ctx, domid);
+ printf("%5d%25s", domid, domain_name);
+ free(domain_name);
+
+ psr_cat_print_one_domain_cbm_type(domid, socketid,
+ LIBXL_PSR_CBM_TYPE_L2_CBM);
+
+ printf("\n");
+}
+
+static int psr_l2_cat_print_domain_cbm(uint32_t domid, uint32_t socketid)
+{
+ int i, nr_domains;
+ libxl_dominfo *list;
+
+ if (domid != INVALID_DOMID) {
+ psr_l2_cat_print_one_domain_cbm(domid, socketid);
+ return 0;
+ }
+
+ if (!(list = libxl_list_domain(ctx, &nr_domains))) {
+ fprintf(stderr, "Failed to get domain list for cbm display\n");
+ return -1;
+ }
+
+ for (i = 0; i < nr_domains; i++)
+ psr_l2_cat_print_one_domain_cbm(list[i].domid, socketid);
+ libxl_dominfo_list_free(list, nr_domains);
+
+ return 0;
+}
+
+static int psr_l2_cat_print_socket(uint32_t domid, libxl_psr_cat_info *info)
+{
+ printf("%-16s: %u\n", "Socket ID", info->id);
+ printf("%-16s: %#llx\n", "Default CBM", (1ull << info->cbm_len) - 1);
+ printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
+
+ return psr_l2_cat_print_domain_cbm(domid, info->id);
+}
+
+static int psr_l2_cat_show(uint32_t domid)
+{
+ int i, nr;
+ int rc;
+ libxl_psr_cat_info *info;
+
+ rc = libxl_psr_cat_get_info(ctx, &info, &nr, 2);
+ if (rc) {
+ fprintf(stderr, "Failed to get l2 cat info\n");
+ return rc;
+ }
+
+ for (i = 0; i < nr; i++) {
+ rc = psr_l2_cat_print_socket(domid, info + i);
+ if (rc)
+ goto out;
+ }
+
+out:
+ libxl_psr_cat_info_list_free(info, nr);
+ return rc;
+}
+
+static int psr_l3_cat_hwinfo(void)
{
int rc;
int i, nr;
uint32_t l3_cache_size;
libxl_psr_cat_info *info;
- printf("Cache Allocation Technology (CAT):\n");
+ printf("Cache Allocation Technology (CAT): L3\n");
- rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr);
+ rc = libxl_psr_cat_get_info(ctx, &info, &nr, 3);
if (rc) {
- fprintf(stderr, "Failed to get cat info\n");
+ fprintf(stderr, "Failed to get l3 cat info\n");
return rc;
}
@@ -9310,17 +9416,6 @@ out:
return rc;
}
-static void psr_cat_print_one_domain_cbm_type(uint32_t domid, uint32_t socketid,
- libxl_psr_cbm_type type)
-{
- 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)
{
@@ -9395,9 +9490,9 @@ static int psr_cat_show(uint32_t domid)
int rc;
libxl_psr_cat_info *info;
- rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr);
+ rc = libxl_psr_cat_get_info(ctx, &info, &nr, 3);
if (rc) {
- fprintf(stderr, "Failed to get cat info\n");
+ fprintf(stderr, "Failed to get l3 cat info\n");
return rc;
}
@@ -9424,18 +9519,20 @@ int main_psr_cat_cbm_set(int argc, char **argv)
libxl_string_list socket_list;
unsigned long start, end;
int i, j, len;
+ int lvl = 0;
static struct option opts[] = {
{"socket", 1, 0, 's'},
{"data", 0, 0, 'd'},
{"code", 0, 0, 'c'},
+ {"level", 1, 0, 'l'},
COMMON_LONG_OPTS
};
libxl_socket_bitmap_alloc(ctx, &target_map, 0);
libxl_bitmap_set_none(&target_map);
- SWITCH_FOREACH_OPT(opt, "s:cd", opts, "psr-cat-cbm-set", 2) {
+ SWITCH_FOREACH_OPT(opt, "s:l:cd", opts, "psr-cat-cbm-set", 2) {
case 's':
trim(isspace, optarg, &value);
split_string_into_string_list(value, ",", &socket_list);
@@ -9455,17 +9552,32 @@ int main_psr_cat_cbm_set(int argc, char **argv)
case 'c':
opt_code = 1;
break;
+ case 'l':
+ lvl = atoi(optarg);
+ 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;
+ if (lvl == 2) {
+ if (opt_data || opt_code) {
+ fprintf(stderr, "L2 CAT does not support CDP yet.\n");
+ return -1;
+ }
+ type = LIBXL_PSR_CBM_TYPE_L2_CBM;
+ } else if (lvl == 3) {
+ 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;
+ }
} else {
- type = LIBXL_PSR_CBM_TYPE_L3_CBM;
+ fprintf(stderr, "'xl %s' requires to input cache level -l%d or -l%d.\n",
+ "psr-cat-cbm-set", 2, 3);
+ return -1;
}
if (libxl_bitmap_is_empty(&target_map))
@@ -9487,11 +9599,20 @@ int main_psr_cat_cbm_set(int argc, char **argv)
int main_psr_cat_show(int argc, char **argv)
{
- int opt;
+ int opt = 0;
uint32_t domid;
+ int lvl = 0;
+ int rc;
- SWITCH_FOREACH_OPT(opt, "", NULL, "psr-cat-show", 0) {
- /* No options */
+ static struct option opts[] = {
+ {"level", 1, 0, 'l'},
+ COMMON_LONG_OPTS
+ };
+
+ SWITCH_FOREACH_OPT(opt, "l:", opts, "psr-cat-show", 0) {
+ case 'l':
+ lvl = atoi(optarg);
+ break;
}
if (optind >= argc)
@@ -9503,7 +9624,15 @@ int main_psr_cat_show(int argc, char **argv)
return 2;
}
- return psr_cat_show(domid);
+ if (3 == lvl)
+ rc = psr_cat_show(domid);
+ else if (2 == lvl)
+ rc = psr_l2_cat_show(domid);
+ else
+ fprintf(stderr, "'xl %s' requires to input cache level -l%d or -l%d.\n",
+ "psr-cat-show", 2, 3);
+
+ return rc;
}
int main_psr_hwinfo(int argc, char **argv)
@@ -9529,7 +9658,11 @@ int main_psr_hwinfo(int argc, char **argv)
ret = psr_cmt_hwinfo();
if (!ret && (all || cat))
- ret = psr_cat_hwinfo();
+ ret = psr_l3_cat_hwinfo();
+
+ /* L2 CAT is independent with CMT and L3 CAT */
+ if (all || cat)
+ ret = psr_l2_cat_hwinfo();
return ret;
}
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 78786fe..5a2676e 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -549,13 +549,15 @@ 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"
+ "-l <level> Specify the cache level to process, otherwise L3 cache is 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,
"Show Cache Allocation Technology information",
- "<Domain>",
+ "[options] <Domain>",
+ "-l <level> Specify the cache level to process, otherwise L3 cache is processed\n"
},
#endif
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [v2 3/3] tools & docs: add L2 CAT support in tools and docs.
2016-09-22 2:17 [v2 3/3] tools & docs: add L2 CAT support in tools and docs Yi Sun
@ 2016-09-22 10:09 ` Ian Jackson
2016-09-23 7:24 ` Yi Sun
0 siblings, 1 reply; 4+ messages in thread
From: Ian Jackson @ 2016-09-22 10:09 UTC (permalink / raw)
To: Yi Sun
Cc: wei.liu2, he.chen, andrew.cooper3, ian.jackson, jbeulich,
chao.p.peng, xen-devel
Yi Sun writes ("[v2 3/3] tools & docs: add L2 CAT support in tools and docs."):
> This patch is the xl/xc changes to support Intel L2 CAT
> (Cache Allocation Technology).
>
> The new level option is introduced to original CAT setting
> command in order to set CBM for specified level CAT.
Thanks for this. I have some comments about the libxl API and xl.
You'll see that I'm somewhat confused. Please help enlighten me :-).
> +#define L3_CAT 3
> +#define L2_CAT 2
> +int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, int lvl,
> + uint32_t *cos_max, uint32_t *cbm_len,
> + bool *cdp_enabled)
I find these defines rather odd. You have chosen to use an integer
"level" to specify which cache to affect. It probably wouldn't be
desirable to decouple these integer values from the names, but the
names are just integers with a funny hat on.
If these names are really useful they should be in the IDL.
> -int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
> - uint32_t *cos_max, uint32_t *cbm_len,
> - bool *cdp_enabled);
> +int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, int lvl,
> + uint32_t *cos_max, uint32_t *cbm_len,
> + bool *cdp_enabled);
This needs a new HAVE #define.
> diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> index 99733f6..861d5a8 100644
> --- a/tools/libxl/libxl_psr.c
> +++ b/tools/libxl/libxl_psr.c
..
> +static int psr_l2_cat_hwinfo(void)
> +{
> + int rc;
> + int i, nr;
> + libxl_psr_cat_info *info;
...
> + for (i = 0; i < nr; i++) {
> + /* There is no CMT on L2 cache so far. */
> + printf("%-16s: %u\n", "Socket ID", info[i].id);
> + 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",
> + (1ull << info[i].cbm_len) - 1);
I find this code confusing.
I don't understand why libxl needs to know about the properties of L2
vs L3 CAT. If it does need to know these properties then probably the
IDL needs to know about them too, but the IDL is completely agnostic.
Perhaps libxl ought probably to be "thinner", and simply present whats
in the IDL ?
For example:
> + if (lvl == 2) {
> + if (opt_data || opt_code) {
> + fprintf(stderr, "L2 CAT does not support CDP yet.\n");
> + return -1;
This should perhaps be done by calling libxl and getting a
notimplemented error ?
> + } else if (lvl == 3) {
> + 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;
> + }
Is this inability to do -c and -d really contingent on the cache
level ?
> } else {
> - type = LIBXL_PSR_CBM_TYPE_L3_CBM;
> + fprintf(stderr, "'xl %s' requires to input cache level -l%d or -l%d.\n",
> + "psr-cat-cbm-set", 2, 3);
I think you have made the old calling pattern break, here: that is, if
you pass no -l, it takes this branch and bombs out.
Ie, a non-backwards-compatible change to the xl command line
interface. I'm afraid that's not allowed.
> @@ -9503,7 +9624,15 @@ int main_psr_cat_show(int argc, char **argv)
> return 2;
> }
>
> - return psr_cat_show(domid);
> + if (3 == lvl)
> + rc = psr_cat_show(domid);
> + else if (2 == lvl)
> + rc = psr_l2_cat_show(domid);
This code is confusing to me. Surely psr_cat_show should be changed
to take a level argument ? Or if psr_cat_show can only show L3, why
does it not have l3 in its name ?
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index 78786fe..5a2676e 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
...
> + "-l <level> Specify the cache level to process, otherwise L3 cache is processed\n"
Maybe I have misread the option parsing but is this actually true that
it defaults to L3 ? (See above.)
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [v2 3/3] tools & docs: add L2 CAT support in tools and docs.
2016-09-22 10:09 ` Ian Jackson
@ 2016-09-23 7:24 ` Yi Sun
2016-09-28 1:54 ` Yi Sun
0 siblings, 1 reply; 4+ messages in thread
From: Yi Sun @ 2016-09-23 7:24 UTC (permalink / raw)
To: Ian Jackson
Cc: wei.liu2, he.chen, andrew.cooper3, jbeulich, chao.p.peng, xen-devel
On 16-09-22 11:09:31, Ian Jackson wrote:
> Yi Sun writes ("[v2 3/3] tools & docs: add L2 CAT support in tools and docs."):
> > This patch is the xl/xc changes to support Intel L2 CAT
> > (Cache Allocation Technology).
> >
> > The new level option is introduced to original CAT setting
> > command in order to set CBM for specified level CAT.
>
> Thanks for this. I have some comments about the libxl API and xl.
>
> You'll see that I'm somewhat confused. Please help enlighten me :-).
>
>
Thank you very much for reviewing my patches and provide your comments!
Sorry to make you confused. I will try best to explain my intention ;-).
> > +#define L3_CAT 3
> > +#define L2_CAT 2
> > +int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, int lvl,
> > + uint32_t *cos_max, uint32_t *cbm_len,
> > + bool *cdp_enabled)
>
> I find these defines rather odd. You have chosen to use an integer
> "level" to specify which cache to affect. It probably wouldn't be
> desirable to decouple these integer values from the names, but the
> names are just integers with a funny hat on.
>
> If these names are really useful they should be in the IDL.
>
>
Thanks! I will remove the macros and just use integer.
> > -int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
> > - uint32_t *cos_max, uint32_t *cbm_len,
> > - bool *cdp_enabled);
> > +int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, int lvl,
> > + uint32_t *cos_max, uint32_t *cbm_len,
> > + bool *cdp_enabled);
>
> This needs a new HAVE #define.
>
>
Thanks! I will add one more HAVE #define in xl.h.
> > diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> > index 99733f6..861d5a8 100644
> > --- a/tools/libxl/libxl_psr.c
> > +++ b/tools/libxl/libxl_psr.c
> ..
> > +static int psr_l2_cat_hwinfo(void)
> > +{
> > + int rc;
> > + int i, nr;
> > + libxl_psr_cat_info *info;
> ...
> > + for (i = 0; i < nr; i++) {
> > + /* There is no CMT on L2 cache so far. */
> > + printf("%-16s: %u\n", "Socket ID", info[i].id);
> > + 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",
> > + (1ull << info[i].cbm_len) - 1);
>
> I find this code confusing.
>
> I don't understand why libxl needs to know about the properties of L2
> vs L3 CAT. If it does need to know these properties then probably the
> IDL needs to know about them too, but the IDL is completely agnostic.
>
psr_l2_cat_hwinfo() is implemented to get L2 CAT HW info back. Because this
HW info is almost same as L3 CAT, I reuse the libxl_psr_cat_info defined
in libxl_types.idl. If you think this is not good enough, I think I may add
'lvl' in libxl_psr_cat_info to express the level to handle?
> Perhaps libxl ought probably to be "thinner", and simply present whats
> in the IDL ?
>
> For example:
>
> > + if (lvl == 2) {
> > + if (opt_data || opt_code) {
> > + fprintf(stderr, "L2 CAT does not support CDP yet.\n");
> > + return -1;
>
> This should perhaps be done by calling libxl and getting a
> notimplemented error ?
>
I think your intention is to put more feature details into libxl_psr.c then
the developer of other app will be easy to implement the app, right?
My concerns are below:
1. The application needs user to input the lvl option to know which lvl to
operate anyway. Of course, we can use L3 as default option to be backward
compatible. But to get/set L2, we still need user to input level. That means
the application knows the level concept.
2. From the functional view, print what information out and how to operate
should be decided by application but not the library.
So, I think it is acceptable to let xl know some L2 and L3 details and operate
differently on different levels.
If my thought is not right, please correct me. Thank you!
> > + } else if (lvl == 3) {
> > + 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;
> > + }
>
> Is this inability to do -c and -d really contingent on the cache
> level ?
>
Because '-c'(code) and '-d'(data) are used for CDP and L2 CAT does not
support CDP, they are handled only at L3 level now.
> > } else {
> > - type = LIBXL_PSR_CBM_TYPE_L3_CBM;
> > + fprintf(stderr, "'xl %s' requires to input cache level -l%d or -l%d.\n",
> > + "psr-cat-cbm-set", 2, 3);
>
> I think you have made the old calling pattern break, here: that is, if
> you pass no -l, it takes this branch and bombs out.
>
> Ie, a non-backwards-compatible change to the xl command line
> interface. I'm afraid that's not allowed.
>
Thank you! I will remove this and handle L3 by default because L3 is the only
choice in old interface.
> > @@ -9503,7 +9624,15 @@ int main_psr_cat_show(int argc, char **argv)
> > return 2;
> > }
> >
> > - return psr_cat_show(domid);
> > + if (3 == lvl)
> > + rc = psr_cat_show(domid);
> > + else if (2 == lvl)
> > + rc = psr_l2_cat_show(domid);
>
> This code is confusing to me. Surely psr_cat_show should be changed
> to take a level argument ? Or if psr_cat_show can only show L3, why
> does it not have l3 in its name ?
>
Sorry for the confusion.
psr_cat_show() was implemented for L3 CAT before. To support L2, I implement
psr_l2_cat_show(). But I don't want to modify the old function so
psr_cat_show is remained.
If you think this is not good, I can change psr_cat_show to psr_l3_cat_show
to make it clearer.
> > diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> > index 78786fe..5a2676e 100644
> > --- a/tools/libxl/xl_cmdtable.c
> > +++ b/tools/libxl/xl_cmdtable.c
> ...
> > + "-l <level> Specify the cache level to process, otherwise L3 cache is processed\n"
>
> Maybe I have misread the option parsing but is this actually true that
> it defaults to L3 ? (See above.)
>
Sorry for this. I missed to modify this sentence.
If L3 is the default behavior as above comment, this sentence will be kept.
>
> Thanks,
> Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [v2 3/3] tools & docs: add L2 CAT support in tools and docs.
2016-09-23 7:24 ` Yi Sun
@ 2016-09-28 1:54 ` Yi Sun
0 siblings, 0 replies; 4+ messages in thread
From: Yi Sun @ 2016-09-28 1:54 UTC (permalink / raw)
To: Ian Jackson
Cc: wei.liu2, he.chen, andrew.cooper3, jbeulich, chao.p.peng, xen-devel
Hi, Ian,
Any comments? That would be very appreciated. Thanks!
BRs,
Sun Yi
On 16-09-23 15:24:57, Yi Sun wrote:
> On 16-09-22 11:09:31, Ian Jackson wrote:
> > Yi Sun writes ("[v2 3/3] tools & docs: add L2 CAT support in tools and docs."):
> > > This patch is the xl/xc changes to support Intel L2 CAT
> > > (Cache Allocation Technology).
> > >
> > > The new level option is introduced to original CAT setting
> > > command in order to set CBM for specified level CAT.
> >
> > Thanks for this. I have some comments about the libxl API and xl.
> >
> > You'll see that I'm somewhat confused. Please help enlighten me :-).
> >
> >
> Thank you very much for reviewing my patches and provide your comments!
>
> Sorry to make you confused. I will try best to explain my intention ;-).
>
> > > +#define L3_CAT 3
> > > +#define L2_CAT 2
> > > +int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, int lvl,
> > > + uint32_t *cos_max, uint32_t *cbm_len,
> > > + bool *cdp_enabled)
> >
> > I find these defines rather odd. You have chosen to use an integer
> > "level" to specify which cache to affect. It probably wouldn't be
> > desirable to decouple these integer values from the names, but the
> > names are just integers with a funny hat on.
> >
> > If these names are really useful they should be in the IDL.
> >
> >
> Thanks! I will remove the macros and just use integer.
>
> > > -int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
> > > - uint32_t *cos_max, uint32_t *cbm_len,
> > > - bool *cdp_enabled);
> > > +int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, int lvl,
> > > + uint32_t *cos_max, uint32_t *cbm_len,
> > > + bool *cdp_enabled);
> >
> > This needs a new HAVE #define.
> >
> >
> Thanks! I will add one more HAVE #define in xl.h.
>
> > > diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> > > index 99733f6..861d5a8 100644
> > > --- a/tools/libxl/libxl_psr.c
> > > +++ b/tools/libxl/libxl_psr.c
> > ..
> > > +static int psr_l2_cat_hwinfo(void)
> > > +{
> > > + int rc;
> > > + int i, nr;
> > > + libxl_psr_cat_info *info;
> > ...
> > > + for (i = 0; i < nr; i++) {
> > > + /* There is no CMT on L2 cache so far. */
> > > + printf("%-16s: %u\n", "Socket ID", info[i].id);
> > > + 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",
> > > + (1ull << info[i].cbm_len) - 1);
> >
> > I find this code confusing.
> >
> > I don't understand why libxl needs to know about the properties of L2
> > vs L3 CAT. If it does need to know these properties then probably the
> > IDL needs to know about them too, but the IDL is completely agnostic.
> >
> psr_l2_cat_hwinfo() is implemented to get L2 CAT HW info back. Because this
> HW info is almost same as L3 CAT, I reuse the libxl_psr_cat_info defined
> in libxl_types.idl. If you think this is not good enough, I think I may add
> 'lvl' in libxl_psr_cat_info to express the level to handle?
>
> > Perhaps libxl ought probably to be "thinner", and simply present whats
> > in the IDL ?
> >
> > For example:
> >
> > > + if (lvl == 2) {
> > > + if (opt_data || opt_code) {
> > > + fprintf(stderr, "L2 CAT does not support CDP yet.\n");
> > > + return -1;
> >
> > This should perhaps be done by calling libxl and getting a
> > notimplemented error ?
> >
> I think your intention is to put more feature details into libxl_psr.c then
> the developer of other app will be easy to implement the app, right?
>
> My concerns are below:
> 1. The application needs user to input the lvl option to know which lvl to
> operate anyway. Of course, we can use L3 as default option to be backward
> compatible. But to get/set L2, we still need user to input level. That means
> the application knows the level concept.
> 2. From the functional view, print what information out and how to operate
> should be decided by application but not the library.
>
> So, I think it is acceptable to let xl know some L2 and L3 details and operate
> differently on different levels.
>
> If my thought is not right, please correct me. Thank you!
>
> > > + } else if (lvl == 3) {
> > > + 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;
> > > + }
> >
> > Is this inability to do -c and -d really contingent on the cache
> > level ?
> >
> Because '-c'(code) and '-d'(data) are used for CDP and L2 CAT does not
> support CDP, they are handled only at L3 level now.
>
> > > } else {
> > > - type = LIBXL_PSR_CBM_TYPE_L3_CBM;
> > > + fprintf(stderr, "'xl %s' requires to input cache level -l%d or -l%d.\n",
> > > + "psr-cat-cbm-set", 2, 3);
> >
> > I think you have made the old calling pattern break, here: that is, if
> > you pass no -l, it takes this branch and bombs out.
> >
> > Ie, a non-backwards-compatible change to the xl command line
> > interface. I'm afraid that's not allowed.
> >
> Thank you! I will remove this and handle L3 by default because L3 is the only
> choice in old interface.
>
> > > @@ -9503,7 +9624,15 @@ int main_psr_cat_show(int argc, char **argv)
> > > return 2;
> > > }
> > >
> > > - return psr_cat_show(domid);
> > > + if (3 == lvl)
> > > + rc = psr_cat_show(domid);
> > > + else if (2 == lvl)
> > > + rc = psr_l2_cat_show(domid);
> >
> > This code is confusing to me. Surely psr_cat_show should be changed
> > to take a level argument ? Or if psr_cat_show can only show L3, why
> > does it not have l3 in its name ?
> >
> Sorry for the confusion.
>
> psr_cat_show() was implemented for L3 CAT before. To support L2, I implement
> psr_l2_cat_show(). But I don't want to modify the old function so
> psr_cat_show is remained.
>
> If you think this is not good, I can change psr_cat_show to psr_l3_cat_show
> to make it clearer.
>
> > > diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> > > index 78786fe..5a2676e 100644
> > > --- a/tools/libxl/xl_cmdtable.c
> > > +++ b/tools/libxl/xl_cmdtable.c
> > ...
> > > + "-l <level> Specify the cache level to process, otherwise L3 cache is processed\n"
> >
> > Maybe I have misread the option parsing but is this actually true that
> > it defaults to L3 ? (See above.)
> >
> Sorry for this. I missed to modify this sentence.
>
> If L3 is the default behavior as above comment, this sentence will be kept.
>
> >
> > Thanks,
> > Ian.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-28 1:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-22 2:17 [v2 3/3] tools & docs: add L2 CAT support in tools and docs Yi Sun
2016-09-22 10:09 ` Ian Jackson
2016-09-23 7:24 ` Yi Sun
2016-09-28 1:54 ` Yi Sun
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.