Re: [Xen-devel] [RFC v2 09/12] tools: implement the new get hw info interface suitable to all psr allocation features.

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Jul 20, 2017 at 04:49:10PM +0800, Yi Sun wrote:
> This patch implements a new get hw info interface suitable for all psr 
> allocation
> features and the whole flow. It also enables MBA support in tools to get MBA
> HW info.
> 
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> ---
>  tools/libxc/include/xenctrl.h |  30 +++++++-
>  tools/libxc/xc_psr.c          |  46 +++++++++----
>  tools/libxl/libxl_psr.c       | 155 
> +++++++++++++++++++++++++++++++++++-------
>  tools/xl/xl_cmdtable.c        |   3 +
>  tools/xl/xl_psr.c             |  45 +++++++++++-
>  5 files changed, 238 insertions(+), 41 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 2248900..0b0ec31 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2460,6 +2460,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    = 0,
> +    XC_PSR_FEAT_CAT_L3     = 1,
> +    XC_PSR_FEAT_CAT_L2     = 2,
> +    XC_PSR_FEAT_MBA        = 3,

Pointless initialisers.

> +};
> +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_info;
> +
> +        struct {
> +            uint32_t cos_max;
> +            uint32_t thrtl_max;
> +            bool     linear;
> +        } xc_mba_info;
> +    } u;
> +};
> +typedef struct xc_psr_hw_info xc_psr_hw_info;
[...]
> index 8319301..43b84b6 100644
> --- a/tools/libxl/libxl_psr.c
> +++ b/tools/libxl/libxl_psr.c
> @@ -361,47 +361,49 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t 
> domid,
>      return rc;
>  }
>  
> +static inline int libxl_psr_hw_info_to_libxl_psr_cat_info(

libxl__ -> two underscores for internal functions.

> +                      libxl_psr_feat_type type, libxl_psr_hw_info *hw_info,
> +                      libxl_psr_cat_info *cat_info)
> +{
> +    if (type != LIBXL_PSR_FEAT_TYPE_CAT_INFO)
> +        return -1;

ERROR_INVAL;

> +
> +static inline int libxc__psr_hw_info_to_libxl_psr_hw_info(
> +                      libxl_psr_feat_type type, xc_psr_hw_info *xc_hw_info,
> +                      libxl_psr_hw_info *xl_hw_info)
> +{
> +    switch (type) {
> +    case LIBXL_PSR_FEAT_TYPE_CAT_INFO:
> +        xl_hw_info->u.cat_info.cos_max = xc_hw_info->u.xc_cat_info.cos_max;
> +        xl_hw_info->u.cat_info.cbm_len = xc_hw_info->u.xc_cat_info.cbm_len;
> +        xl_hw_info->u.cat_info.cdp_enabled =
> +                                        
> xc_hw_info->u.xc_cat_info.cdp_enabled;
> +        break;
> +    case LIBXL_PSR_FEAT_TYPE_MBA_INFO:
> +        xl_hw_info->u.mba_info.cos_max = xc_hw_info->u.xc_mba_info.cos_max;
> +        xl_hw_info->u.mba_info.thrtl_max = 
> xc_hw_info->u.xc_mba_info.thrtl_max;
> +        xl_hw_info->u.mba_info.linear = xc_hw_info->u.xc_mba_info.linear;
> +        break;
> +    default:
> +        return -1;

ERROR_INVAL

> +    }
> +
> +    return 0;
> +}
> +
>  int libxl_psr_get_hw_info(libxl_ctx *ctx, libxl_psr_hw_info **info,
>                            int *nr, libxl_psr_feat_type type, int lvl)
>  {
> -    return EXIT_FAILURE;
> +    GC_INIT(ctx);
> +    int rc;
> +    int i = 0, socketid, nr_sockets;
> +    libxl_bitmap socketmap;
> +    libxl_psr_hw_info *ptr;
> +    xc_psr_feat_type xc_type;
> +    xc_psr_hw_info hw_info;
> +
> +    libxl_bitmap_init(&socketmap);
> +
> +    if ( type == LIBXL_PSR_FEAT_TYPE_CAT_INFO && lvl != 3 && lvl != 2) {

Extraneous space.

>  
>  void libxl_psr_hw_info_list_free(libxl_psr_hw_info *list, int nr)
>  {
> +    int i;

unsigned int

> +
> +    for (i = 0; i < nr; i++)
> +        libxl_psr_hw_info_dispose(&list[i]);
> +    free(list);
>  }
>  
>  /*
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index 2c71a9f..14a02d4 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -524,6 +524,9 @@ struct cmd_spec cmd_table[] = {
>        "[options]",
>        "-m, --cmt       Show Cache Monitoring Technology (CMT) hardware 
> info\n"
>        "-a, --cat       Show Cache Allocation Technology (CAT) hardware 
> info\n"
> +#ifdef LIBXL_HAVE_PSR_MBA
> +      "-b, --mba       Show Memory Bandwidth Allocation (MBA) hardware 
> info\n"
> +#endif

You don't need to test the macro. xl always has all features available.

Same comment applies to the rest of this patch.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel