|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4/5] tools: add tools support for Intel CDP
On Wed, Sep 02, 2015 at 04:28:01PM +0800, He Chen wrote:
> This is the xc/xl changes to support Intel Code/Data Prioritization.
> Two new xl commands are introduced to enable/disable CDP dynamically,
> and CAT xl commands to set/get CBMs are extended to support CDP.
>
> Signed-off-by: He Chen <he.chen@xxxxxxxxxxxxxxx>
> ---
> tools/libxc/include/xenctrl.h | 10 ++++--
> tools/libxc/xc_psr.c | 42 +++++++++++++++++++++-
> tools/libxl/libxl.h | 12 +++++++
> tools/libxl/libxl_psr.c | 64 +++++++++++++++++++++++++++++++++-
> tools/libxl/libxl_types.idl | 3 ++
> tools/libxl/xl.h | 4 +++
> tools/libxl/xl_cmdimpl.c | 81
> +++++++++++++++++++++++++++++++++++++------
> tools/libxl/xl_cmdtable.c | 15 ++++++++
> 8 files changed, 217 insertions(+), 14 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index de3c0ad..665e6bd 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2798,7 +2798,10 @@ enum xc_psr_cmt_type {
> typedef enum xc_psr_cmt_type xc_psr_cmt_type;
>
> enum xc_psr_cat_type {
> - XC_PSR_CAT_L3_CBM = 1,
> + XC_PSR_CAT_L3_CBM = 1,
> + XC_PSR_CAT_L3_CODE = 2,
> + XC_PSR_CAT_L3_DATA = 3,
> +
Stray blank line.
> };
> typedef enum xc_psr_cat_type xc_psr_cat_type;
>
> @@ -2824,7 +2827,10 @@ int xc_psr_cat_get_domain_data(xc_interface *xch,
> uint32_t domid,
> xc_psr_cat_type type, uint32_t target,
> uint64_t *data);
> int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
> - uint32_t *cos_max, uint32_t *cbm_len);
> + uint32_t *cos_max, uint32_t *cbm_len,
> + uint8_t *cdp_enabled);
Why does it need to be uint8_t? The name suggests bool should be used.
> +int xc_psr_cat_enable_cdp(xc_interface *xch);
> +int xc_psr_cat_disable_cdp(xc_interface *xch);
> #endif
>
> #endif /* XENCTRL_H */
> diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
> index d8b3a51..d4ff6f6 100644
> --- a/tools/libxc/xc_psr.c
> +++ b/tools/libxc/xc_psr.c
> @@ -260,6 +260,12 @@ int xc_psr_cat_set_domain_data(xc_interface *xch,
> uint32_t domid,
> case XC_PSR_CAT_L3_CBM:
> cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM;
> break;
> + case XC_PSR_CAT_L3_CODE:
> + cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE;
> + break;
> + case XC_PSR_CAT_L3_DATA:
> + cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA;
> + break;
> default:
> errno = EINVAL;
> return -1;
> @@ -287,6 +293,12 @@ int xc_psr_cat_get_domain_data(xc_interface *xch,
> uint32_t domid,
> case XC_PSR_CAT_L3_CBM:
> cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM;
> break;
> + case XC_PSR_CAT_L3_CODE:
> + cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE;
> + break;
> + case XC_PSR_CAT_L3_DATA:
> + cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA;
> + break;
> default:
> errno = EINVAL;
> return -1;
> @@ -306,7 +318,8 @@ int xc_psr_cat_get_domain_data(xc_interface *xch,
> uint32_t domid,
> }
>
[...]
> @@ -304,6 +305,22 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> goto out;
> }
>
> + rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
> + if (rc) {
> + LOGE(ERROR, "Failed to get cat info");
> + goto out;
> + }
> +
> + if (!info->cdp_enabled) {
> + if (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
> + type == LIBXL_PSR_CBM_TYPE_L3_DATA)
You can merge this if statement to parent.
if (!info->cdp_enabled &&
(type == _CODE || type == _DATA))
> + {
> + LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
> + rc = EINVAL;
> + goto out;
> + }
> + }
> +
> libxl_for_each_set_bit(socketid, *target_map) {
> if (socketid >= nr_sockets)
> break;
Need to free memory info points to at the end.
> @@ -352,7 +369,7 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx,
> libxl_psr_cat_info **info,
>
> for (i = 0; i < nr_sockets; i++) {
> if (xc_psr_cat_get_l3_info(ctx->xch, i, &ptr[i].cos_max,
> - &ptr[i].cbm_len)) {
> + &ptr[i].cbm_len, &ptr[i].cdp_enabled)) {
> libxl__psr_cat_log_err_msg(gc, errno);
> rc = ERROR_FAIL;
> free(ptr);
> @@ -376,6 +393,51 @@ void libxl_psr_cat_info_list_free(libxl_psr_cat_info
> *list, int nr)
> free(list);
> }
>
[...]
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index 13bccba..e8bb774 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -121,6 +121,10 @@ int main_psr_cmt_show(int argc, char **argv);
> int main_psr_cat_cbm_set(int argc, char **argv);
> int main_psr_cat_show(int argc, char **argv);
> #endif
> +#ifdef LIBXL_HAVE_PSR_CDP
> +int main_psr_cat_cdp_enable(int argc, char **argv);
> +int main_psr_cat_cdp_disable(int argc, char **argv);
> +#endif
>
> void help(const char *command);
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index ebbb9a5..825d707 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -8366,6 +8366,15 @@ int main_psr_cmt_show(int argc, char **argv)
> #endif
>
> #ifdef LIBXL_HAVE_PSR_CAT
> +static void psr_cat_print_cdp_status(uint8_t status)
> +{
> + if (status == 0)
> + printf("%-16s: Disabled\n", "CDP Status");
> + else
> + printf("%-16s: Enabled\n", "CDP Status");
> +}
> +
> +
Extraneous blank line.
In fact I don't think this a function is necessary. There is nothing
wrong with embedding a if statement in psr_cat_hwinfo.
> static int psr_cat_hwinfo(void)
> {
> int rc;
> @@ -8390,6 +8399,7 @@ static int psr_cat_hwinfo(void)
> }
> printf("%-16s: %u\n", "Socket ID", socketid);
> printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
> + psr_cat_print_cdp_status(info->cdp_enabled);
> printf("%-16s: %u\n", "Maximum COS", info->cos_max);
> printf("%-16s: %u\n", "CBM length", info->cbm_len);
> printf("%-16s: %#llx\n", "Default CBM",
> @@ -8401,7 +8411,8 @@ out:
> return rc;
> }
>
[...]
> @@ -8585,6 +8620,32 @@ int main_psr_hwinfo(int argc, char **argv)
> return ret;
> }
>
> +#ifdef LIBXL_HAVE_PSR_CDP
> +int main_psr_cat_cdp_enable(int argc, char **argv)
> +{
> + int ret;
> +
> + ret = libxl_psr_cat_enable_cdp(ctx);
> + if (ret)
> + return ret;
Please log the error / failure.
> + printf("CDP is enabled\n");
> +
> + return 0;
> +}
> +
> +int main_psr_cat_cdp_disable(int argc, char **argv)
> +{
> + int ret;
> +
> + ret = libxl_psr_cat_disable_cdp(ctx);
> + if (ret)
> + return ret;
Ditto.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |