|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 09/15] tools: implement the new libxc get hw info interface
On Tue, Sep 05, 2017 at 05:32:31PM +0800, Yi Sun wrote:
> This patch implements a new libxc get hw info interface and corresponding
> data structures. It also changes libxl_psr.c to call this new interface.
>
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> ---
> v3:
> - rename 'psr_cat/PSR_CAT' to 'psr_alloc/PSR_ALLOC' and remove 'op/OP'
> from name.
> (suggested by Roger Pau Monné)
> - remove 'info' from 'xc_cat_info' and 'xc_mba_info'.
> (suggested by Roger Pau Monné)
> - set errno in 'xc_psr_get_hw_info'.
> (suggested by Roger Pau Monné)
> - remove 'inline'.
> (suggested by Roger Pau Monné)
> - remove 'psr' from 'libxl__psr_feat_type_to_libxc_psr_feat_type' to make
> function name shorter.
> (suggested by Roger Pau Monné)
> - check 'xc_type' in 'libxl_psr_cat_get_info'.
> (suggested by Roger Pau Monné)
> v2:
> - split this patch out from a big patch in v1.
> (suggested by Wei Liu)
> - change 'CAT_INFO' and 'MBA_INFO' to 'CAT' and 'MBA'.
> (suggested by Chao Peng)
> ---
> tools/libxc/include/xenctrl.h | 30 +++++++++++++++++++++++---
> tools/libxc/xc_psr.c | 49
> ++++++++++++++++++++++++++++++++-----------
> tools/libxl/libxl_psr.c | 38 +++++++++++++++++++++++++++++++--
> 3 files changed, 100 insertions(+), 17 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index c7710b8..bbdf8e2 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2458,6 +2458,31 @@ enum xc_psr_cat_type {
> };
> typedef enum xc_psr_cat_type xc_psr_cat_type;
>
> +enum xc_psr_feat_type {
> + XC_PSR_FEAT_UNKNOWN,
You don't seem to have such an unknown type in the libxl layer, any
reason you need it at the libxc layer?
> + XC_PSR_FEAT_CAT_L3,
> + XC_PSR_FEAT_CAT_L2,
> + XC_PSR_FEAT_MBA,
I think you can drop the _FEAT_ from the enum names.
> +};
> +typedef enum xc_psr_feat_type xc_psr_feat_type;
> +
> +struct xc_psr_hw_info {
> + union {
> + struct {
> + uint32_t cos_max;
> + uint32_t cbm_len;
> + bool cdp_enabled;
> + } xc_cat;
No need for the 'xc_cat', just 'cat' please (and 'mba' below).
> +
> + struct {
> + uint32_t cos_max;
> + uint32_t thrtl_max;
> + bool linear;
> + } xc_mba;
> + } u;
> +};
> +typedef struct xc_psr_hw_info xc_psr_hw_info;
> +
> int xc_psr_cmt_attach(xc_interface *xch, uint32_t domid);
> int xc_psr_cmt_detach(xc_interface *xch, uint32_t domid);
> int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
> @@ -2479,9 +2504,8 @@ 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_info(xc_interface *xch, uint32_t socket, unsigned int lvl,
> - uint32_t *cos_max, uint32_t *cbm_len,
> - bool *cdp_enabled);
> +int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
> + xc_psr_feat_type type, xc_psr_hw_info *hw_info);
>
> 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 7e1c0d6..a8a750a 100644
> --- a/tools/libxc/xc_psr.c
> +++ b/tools/libxc/xc_psr.c
> @@ -323,36 +323,61 @@ int xc_psr_cat_get_domain_data(xc_interface *xch,
> uint32_t domid,
> return rc;
> }
>
> -int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, unsigned int lvl,
> - uint32_t *cos_max, uint32_t *cbm_len, bool
> *cdp_enabled)
> +int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
> + xc_psr_feat_type type, xc_psr_hw_info *hw_info)
> {
> int rc = -1;
> DECLARE_SYSCTL;
>
> + if ( !hw_info )
> + {
> + errno = EINVAL;
> + return rc;
> + }
> +
> sysctl.cmd = XEN_SYSCTL_psr_alloc;
> sysctl.u.psr_alloc.target = socket;
>
> - switch ( lvl )
> + switch ( type )
> {
> - case 2:
> + case XC_PSR_FEAT_CAT_L2:
> sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_ALLOC_get_l2_info;
> rc = xc_sysctl(xch, &sysctl);
> if ( !rc )
> {
> - *cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
> - *cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
> - *cdp_enabled = false;
> + hw_info->u.xc_cat.cos_max =
> + sysctl.u.psr_alloc.u.cat_info.cos_max;
> + hw_info->u.xc_cat.cbm_len =
> + sysctl.u.psr_alloc.u.cat_info.cbm_len;
> + hw_info->u.xc_cat.cdp_enabled = false;
> }
> break;
> - case 3:
> + case XC_PSR_FEAT_CAT_L3:
> sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_ALLOC_get_l3_info;
> rc = xc_sysctl(xch, &sysctl);
> if ( !rc )
> {
> - *cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
> - *cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
> - *cdp_enabled = sysctl.u.psr_alloc.u.cat_info.flags &
> - XEN_SYSCTL_PSR_ALLOC_L3_CDP;
> + hw_info->u.xc_cat.cos_max =
> + sysctl.u.psr_alloc.u.cat_info.cos_max;
> + hw_info->u.xc_cat.cbm_len =
> + sysctl.u.psr_alloc.u.cat_info.cbm_len;
> + hw_info->u.xc_cat.cdp_enabled =
> + sysctl.u.psr_alloc.u.cat_info.flags &
> + XEN_SYSCTL_PSR_ALLOC_L3_CDP;
> + }
> + break;
> + case XC_PSR_FEAT_MBA:
> + sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_ALLOC_get_mba_info;
> + rc = xc_sysctl(xch, &sysctl);
> + if ( !rc )
> + {
> + hw_info->u.xc_mba.cos_max =
> + sysctl.u.psr_alloc.u.mba_info.cos_max;
> + hw_info->u.xc_mba.thrtl_max =
> + sysctl.u.psr_alloc.u.mba_info.thrtl_max;
> + hw_info->u.xc_mba.linear =
> + sysctl.u.psr_alloc.u.mba_info.flags &
> + XEN_SYSCTL_PSR_ALLOC_MBA_LINEAR;
> }
Would it help to prevent line breaks to change the indentation above
to:
rc = xc_sysctl(...);
if ( rc )
break;
hw_info->u....
?
> break;
> default:
> diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> index 4a6978e..dd412cc 100644
> --- a/tools/libxl/libxl_psr.c
> +++ b/tools/libxl/libxl_psr.c
> @@ -361,6 +361,27 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
> return rc;
> }
>
> +static xc_psr_feat_type libxl__feat_type_to_libxc_feat_type(
> + libxl_psr_feat_type type, unsigned int lvl)
> +{
> + xc_psr_feat_type xc_type = XC_PSR_FEAT_UNKNOWN;
> +
> + switch (type) {
> + case LIBXL_PSR_FEAT_TYPE_CAT:
> + if (lvl == 3)
> + xc_type = XC_PSR_FEAT_CAT_L3;
> + if (lvl == 2)
> + xc_type = XC_PSR_FEAT_CAT_L2;
> + break;
> + case LIBXL_PSR_FEAT_TYPE_MBA:
> + xc_type = XC_PSR_FEAT_MBA;
> + default:
> + break;
assert?
libxl_psr_feat_type cannot have any other value apart from the two
that you list above.
> + }
> +
> + return xc_type;
> +}
> +
> int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
> unsigned int *nr, unsigned int lvl)
> {
> @@ -369,6 +390,8 @@ int libxl_psr_cat_get_info(libxl_ctx *ctx,
> libxl_psr_cat_info **info,
> int i = 0, socketid, nr_sockets;
> libxl_bitmap socketmap;
> libxl_psr_cat_info *ptr;
> + xc_psr_hw_info hw_info;
> + xc_psr_feat_type xc_type;
>
> libxl_bitmap_init(&socketmap);
>
> @@ -385,16 +408,27 @@ int libxl_psr_cat_get_info(libxl_ctx *ctx,
> libxl_psr_cat_info **info,
> goto out;
> }
>
> + xc_type = libxl__feat_type_to_libxc_feat_type(LIBXL_PSR_FEAT_TYPE_CAT,
> lvl);
> + if (xc_type == XC_PSR_FEAT_UNKNOWN) {
> + LOG(ERROR, "feature type or lvl is wrong");
> + rc = ERROR_FAIL;
> + 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)) {
> + if (xc_psr_get_hw_info(ctx->xch, socketid, xc_type, &hw_info)) {
It might be nice to LOG the errno here, or else you lose it AFAICT.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |