Message ID | 1489662495-5375-25-git-send-email-yi.y.sun@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thu, Mar 16, 2017 at 07:08:14PM +0800, Yi Sun wrote: > This patch implements the xl/xc changes to support set CBM > for L2 CAT. > > The new level option is introduced to original CAT setting > command in order to set CBM for specified level CAT. > - 'xl psr-cat-set' is updated to set cache capacity bitmasks(CBM) > for a domain according to input cache level. > > root@:~$ xl psr-cat-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> > Acked-by: Wei Liu <wei.liu2@citrix.com> > --- > v9: > - handle the case to set both CODE and DATA for CDP at same time. > For such case, user does not input '-c' or '-d' to set CDP cbm. > - move xl_cmdimpl.c changes into xl/xl_psr.c. > - move xl_cmdtable.c changes into xl/xl_cmdtable.c. Right. Since you've changed how this patch works, it'd be better to drop my ack, so that I know it requires my review. > > rc = libxl__count_physical_sockets(gc, &nr_sockets); > if (rc) { > @@ -331,10 +332,43 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid, > break; > > xc_type = libxl__psr_cbm_type_to_libxc_psr_cat_type(type); > - if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type, > - socketid, cbm)) { > - libxl__psr_cat_log_err_msg(gc, errno); > - rc = ERROR_FAIL; > + > + if (xc_type == XC_PSR_CAT_L3_CBM) { > + if (xc_psr_cat_get_info(ctx->xch, socketid, 3, &cat_info.cos_max, > + &cat_info.cbm_len, &cat_info.cdp_enabled)) { > + libxl__psr_cat_log_err_msg(gc, errno); > + rc = ERROR_FAIL; > + goto out; > + } > + } > + > + /* > + * User can set both CDP CODE and DATA at same time when the '-c' or > + * '-d' is not input. In such case, the input type is > + * LIBXL_PSR_CBM_TYPE_L3_CBM. So, we need check if cdp_enabled is true. > + * If it is true, we need set both CODE and DATA. "When user sets both CDP CODE and DATA at the same time, the input type is..." No need to mention xl options here. Please document xl manpage instead. The rest looks sensible. Wei.
On 17-03-28 15:04:03, Wei Liu wrote: > On Thu, Mar 16, 2017 at 07:08:14PM +0800, Yi Sun wrote: > > This patch implements the xl/xc changes to support set CBM > > for L2 CAT. > > > > The new level option is introduced to original CAT setting > > command in order to set CBM for specified level CAT. > > - 'xl psr-cat-set' is updated to set cache capacity bitmasks(CBM) > > for a domain according to input cache level. > > > > root@:~$ xl psr-cat-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> > > Acked-by: Wei Liu <wei.liu2@citrix.com> > > --- > > v9: > > - handle the case to set both CODE and DATA for CDP at same time. > > For such case, user does not input '-c' or '-d' to set CDP cbm. > > - move xl_cmdimpl.c changes into xl/xl_psr.c. > > - move xl_cmdtable.c changes into xl/xl_cmdtable.c. > > Right. Since you've changed how this patch works, it'd be better to drop > my ack, so that I know it requires my review. > Sorry for that. I will remember this. > > > > rc = libxl__count_physical_sockets(gc, &nr_sockets); > > if (rc) { > > @@ -331,10 +332,43 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid, > > break; > > > > xc_type = libxl__psr_cbm_type_to_libxc_psr_cat_type(type); > > - if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type, > > - socketid, cbm)) { > > - libxl__psr_cat_log_err_msg(gc, errno); > > - rc = ERROR_FAIL; > > + > > + if (xc_type == XC_PSR_CAT_L3_CBM) { > > + if (xc_psr_cat_get_info(ctx->xch, socketid, 3, &cat_info.cos_max, > > + &cat_info.cbm_len, &cat_info.cdp_enabled)) { > > + libxl__psr_cat_log_err_msg(gc, errno); > > + rc = ERROR_FAIL; > > + goto out; > > + } > > + } > > + > > + /* > > + * User can set both CDP CODE and DATA at same time when the '-c' or > > + * '-d' is not input. In such case, the input type is > > + * LIBXL_PSR_CBM_TYPE_L3_CBM. So, we need check if cdp_enabled is true. > > + * If it is true, we need set both CODE and DATA. > > "When user sets both CDP CODE and DATA at the same time, the input type > is..." > > No need to mention xl options here. Please document xl manpage instead. > > The rest looks sensible. > Thanks a lot for your review and comments! > Wei.
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c index 04f5927..039b920 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; diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c index f55ba1e..15e7e59 100644 --- a/tools/libxl/libxl_psr.c +++ b/tools/libxl/libxl_psr.c @@ -317,6 +317,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 cat_info; rc = libxl__count_physical_sockets(gc, &nr_sockets); if (rc) { @@ -331,10 +332,43 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid, break; xc_type = libxl__psr_cbm_type_to_libxc_psr_cat_type(type); - if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type, - socketid, cbm)) { - libxl__psr_cat_log_err_msg(gc, errno); - rc = ERROR_FAIL; + + if (xc_type == XC_PSR_CAT_L3_CBM) { + if (xc_psr_cat_get_info(ctx->xch, socketid, 3, &cat_info.cos_max, + &cat_info.cbm_len, &cat_info.cdp_enabled)) { + libxl__psr_cat_log_err_msg(gc, errno); + rc = ERROR_FAIL; + goto out; + } + } + + /* + * User can set both CDP CODE and DATA at same time when the '-c' or + * '-d' is not input. In such case, the input type is + * LIBXL_PSR_CBM_TYPE_L3_CBM. So, we need check if cdp_enabled is true. + * If it is true, we need set both CODE and DATA. + */ + if (xc_type == XC_PSR_CAT_L3_CBM && cat_info.cdp_enabled) { + xc_type = XC_PSR_CAT_L3_CBM_CODE; + if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type, + socketid, cbm)) { + libxl__psr_cat_log_err_msg(gc, errno); + rc = ERROR_FAIL; + } + + xc_type = XC_PSR_CAT_L3_CBM_DATA; + if (rc != ERROR_FAIL && + xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type, + socketid, cbm)) { + libxl__psr_cat_log_err_msg(gc, errno); + rc = ERROR_FAIL; + } + } else { + if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type, + socketid, cbm)) { + libxl__psr_cat_log_err_msg(gc, errno); + rc = ERROR_FAIL; + } } } diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c index ab7ad60..d332e1a 100644 --- a/tools/xl/xl_cmdtable.c +++ b/tools/xl/xl_cmdtable.c @@ -545,11 +545,12 @@ struct cmd_spec cmd_table[] = { }, #endif #ifdef LIBXL_HAVE_PSR_CAT - { "psr-cat-cbm-set", + { "psr-cat-set", &main_psr_cat_cbm_set, 0, 1, "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" }, diff --git a/tools/xl/xl_psr.c b/tools/xl/xl_psr.c index 575f4a0..7309d4f 100644 --- a/tools/xl/xl_psr.c +++ b/tools/xl/xl_psr.c @@ -490,19 +490,21 @@ int main_psr_cat_cbm_set(int argc, char **argv) char *value; libxl_string_list socket_list; unsigned long start, end; - int i, j, len; + unsigned int i, j, len; + unsigned int lvl = 3; 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-set", 2) { case 's': trim(isspace, optarg, &value); split_string_into_string_list(value, ",", &socket_list); @@ -522,24 +524,35 @@ 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) + 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 EXIT_FAILURE; + } 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, "Input lvl %d is wrong\n", lvl); + return EXIT_FAILURE; } if (libxl_bitmap_is_empty(&target_map)) libxl_bitmap_set_any(&target_map); if (argc != optind + 2) { - help("psr-cat-cbm-set"); + help("psr-cat-set"); return 2; }