Re: [PATCH v2 3/4] tools: add tools support for Intel CDP

[Chao Peng <chao.p.peng@linux.intel.com>]

On Wed, Sep 09, 2015 at 01:16:47PM +0800, He Chen wrote:
> This is the xl/xc changes to support Intel Code/Data Prioritization.
> CAT xl commands to set/get CBMs are extended to support CDP.
> 
> Signed-off-by: He Chen <he.chen@linux.intel.com>
> ---
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 5f9047c..e4eb4df 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -796,6 +796,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
>   * If this is defined, the Cache Allocation Technology feature is supported.
>   */
>  #define LIBXL_HAVE_PSR_CAT 1
> +
> +/*
> + * LIBXL_HAVE_PSR_CDP
> + *
> + * If this is defined, the Code/Data Prioritization feature is supported.
> + */
> +#define LIBXL_HAVE_PSR_CDP 1
>  #endif
>  
>  /*
> @@ -1729,6 +1736,9 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
>  void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
>  #endif
>  
> +#ifdef LIBXL_HAVE_PSR_CDP
> +#endif
> +

Why this? There are several of them in this patch.

>  /* misc */
>  
>  /* Each of these sets or clears the flag according to whether the
> diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> index 3378239..26939a5 100644
> --- a/tools/libxl/libxl_psr.c
> +++ b/tools/libxl/libxl_psr.c
> @@ -297,6 +297,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 *info;
>  
>      rc = libxl__count_physical_sockets(gc, &nr_sockets);
>      if (rc) {
> @@ -304,14 +305,41 @@ 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 &&
> +       (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
> +        type == LIBXL_PSR_CBM_TYPE_L3_DATA))
> +    {
> +            LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
> +            rc = EINVAL;
> +            free(info);
> +            goto out;
> +    }
> +
>      libxl_for_each_set_bit(socketid, *target_map) {
>          if (socketid >= nr_sockets)
>              break;
> -        if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
> +        if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
> +        {
> +            if(xc_psr_cat_set_domain_data(ctx->xch, domid,
> +               LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
> +               xc_psr_cat_set_domain_data(ctx->xch, domid,
> +               LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
> +            {
> +                libxl__psr_cat_log_err_msg(gc, errno);
> +                rc = ERROR_FAIL;
> +            }

Will you merge the two if's?

Chao
> +        } else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
>              libxl__psr_cat_log_err_msg(gc, errno);
>              rc = ERROR_FAIL;
>          }
>      }
> +    free(info);
>  
>  out:
>      GC_FREE;