Re: [Xen-devel] [PATCH v4 21/24] tools: L2 CAT: support get HW info for L2 CAT.

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Dec 14, 2016 at 12:08:01PM +0800, Yi Sun wrote:
> This patch implements xl/xc changes to support get HW info
> for L2 CAT.
> 
> 'xl psr-hwinfo' is updated to show both L3 CAT and L2 CAT
> info.
> 
> Example(on machine which only supports L2 CAT):
> Cache Monitoring Technology (CMT):
> Enabled         : 0
> Cache Allocation Technology (CAT): L3
> libxl: error: libxl_psr.c:100:libxl__psr_cat_log_err_msg: CAT is not enabled 
> on the socket: No such file or directory
> Failed to get l3 cat info

What is this?

> Cache Allocation Technology (CAT): L2
> Socket ID       : 0
> Maximum COS     : 3
> CBM length      : 8
> Default CBM     : 0xff
> 
> Signed-off-by: He Chen <he.chen@xxxxxxxxxxxxxxx>
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> ---
>  tools/libxc/include/xenctrl.h |  6 +++---
>  tools/libxc/xc_psr.c          | 40 +++++++++++++++++++++++-----------
>  tools/libxl/libxl.h           | 11 ++++++++--
>  tools/libxl/libxl_psr.c       |  8 +++----
>  tools/libxl/xl_cmdimpl.c      | 50 
> +++++++++++++++++++++++++++++++++++--------
>  5 files changed, 85 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 2c83544..5b7df22 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2626,9 +2626,9 @@ 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_l3_info(xc_interface *xch, uint32_t socket,
> -                           uint32_t *cos_max, uint32_t *cbm_len,
> -                           bool *cdp_enabled);
> +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_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 43b3286..7af17d9 100644
> --- a/tools/libxc/xc_psr.c
> +++ b/tools/libxc/xc_psr.c
> @@ -317,24 +317,40 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, 
> uint32_t domid,
>      return rc;
>  }
>  
> -int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
> -                           uint32_t *cos_max, uint32_t *cbm_len,
> -                           bool *cdp_enabled)
> +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 rc;
> +    int rc = -1;
>      DECLARE_SYSCTL;
>  
>      sysctl.cmd = XEN_SYSCTL_psr_cat_op;
> -    sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
>      sysctl.u.psr_cat_op.target = socket;
>  
> -    rc = xc_sysctl(xch, &sysctl);
> -    if ( !rc )
> -    {
> -        *cos_max = sysctl.u.psr_cat_op.u.l3_info.cos_max;
> -        *cbm_len = sysctl.u.psr_cat_op.u.l3_info.cbm_len;
> -        *cdp_enabled = sysctl.u.psr_cat_op.u.l3_info.flags &
> -                       XEN_SYSCTL_PSR_CAT_L3_CDP;
> +    switch ( lvl ) {
> +    case 2:
> +        sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l2_info;
> +        rc = xc_sysctl(xch, &sysctl);
> +        if ( !rc )
> +        {
> +            *cos_max = sysctl.u.psr_cat_op.u.l2_info.cos_max;
> +            *cbm_len = sysctl.u.psr_cat_op.u.l2_info.cbm_len;
> +            *cdp_enabled = false;
> +        }
> +        break;
> +    case 3:
> +        sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
> +        rc = xc_sysctl(xch, &sysctl);
> +        if ( !rc )
> +        {
> +            *cos_max = sysctl.u.psr_cat_op.u.l3_info.cos_max;
> +            *cbm_len = sysctl.u.psr_cat_op.u.l3_info.cbm_len;
> +            *cdp_enabled = sysctl.u.psr_cat_op.u.l3_info.flags &
> +                           XEN_SYSCTL_PSR_CAT_L3_CDP;
> +        }
> +        break;
> +    default:
> +        errno = EOPNOTSUPP;
> +        return rc;

There is no need for "return rc".

>      }
>  
>      return rc;
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index acbf476..9233b0f 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -904,6 +904,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
> const libxl_mac *src);
>   * If this is defined, the Code and Data Prioritization feature is supported.
>   */
>  #define LIBXL_HAVE_PSR_CDP 1
> +
> +/*
> + * LIBXL_HAVE_PSR_L2_CAT
> + *
> + * If this is defined, the L2 Cache Allocation Technology feature is 
> supported.
> + */
> +#define LIBXL_HAVE_PSR_L2_CAT 1
>  #endif
>  
>  /*
> @@ -2159,8 +2166,8 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t 
> domid,
>   * On success, the function returns an array of elements in 'info',
>   * and the length in 'nr'.
>   */
> -int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
> -                              int *nr);
> +int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
> +                           int *nr, unsigned int lvl);

You can't delete old public functions. They will basically stay there
forever.

Please refactor the code to support both the new functions and the old.
In this particular case, it would be easy to implement _get_l3_info with
_get_info.

Wei.

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