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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 17/09/15 10:35, He Chen wrote:
> @@ -2798,7 +2798,9 @@ 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,
>  };

No need for the explicit assignments here.  The exact values are not
interesting and guaranteed to be as currently assigned.

>  typedef enum xc_psr_cat_type xc_psr_cat_type;
>
> @@ -306,7 +318,8 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
>  }
>  
>  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,
> +                           bool *cdp_enabled)
>  {
>      int rc;
>      DECLARE_SYSCTL;
> @@ -320,6 +333,8 @@ int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
>      {
>          *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;

!!(sysctl.u.psr_cat_op.u.l3_info.flags & XEN_SYSCTL_PSR_CAT_L3_CDP);

To turn it into a proper boolean, rather than a just a non-zero integer.

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index ebbb9a5..8128f54 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -8390,6 +8390,10 @@ static int psr_cat_hwinfo(void)
>          }
>          printf("%-16s: %u\n", "Socket ID", socketid);
>          printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
> +        if (info->cdp_enabled)
> +            printf("%-16s: Enabled\n", "CDP Status");
> +        else
> +            printf("%-16s: Disabled\n", "CDP Status");

printf("%-16s: %sabled\n", "CDP Status", info->cdp_enabled ? "En" : "Dis");

is rather shorter, if you prefer.

>          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 +8405,8 @@ out:
>      return rc;
>  }
>  
> -static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid)
> +static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid,
> +                                         bool cdp_enabled)
>  {
>      char *domain_name;
>      uint64_t cbm;
> @@ -8410,20 +8415,29 @@ static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid)
>      printf("%5d%25s", domid, domain_name);
>      free(domain_name);
>  
> -    if (!libxl_psr_cat_get_cbm(ctx, domid, LIBXL_PSR_CBM_TYPE_L3_CBM,
> -                               socketid, &cbm))
> -         printf("%#16"PRIx64, cbm);
> -
> +    if (!cdp_enabled) {
> +        if (!libxl_psr_cat_get_cbm(ctx, domid, LIBXL_PSR_CBM_TYPE_L3_CBM,
> +                                   socketid, &cbm))
> +            printf("%#16"PRIx64, cbm);

Mixing # and an explicit width of the integer will clip two nibbles of
the integer.

In this situation, the correct formatting is "0x%016"PRIx64, or
"%#018"PRIx64

> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index 0071f12..13e7aa1 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -543,6 +543,8 @@ struct cmd_spec cmd_table[] = {
>        "Set cache capacity bitmasks(CBM) for a domain",
>        "[options] <Domain> <CBM>",
>        "-s <socket>       Specify the socket to process, otherwise all sockets are processed\n"
> +      "-c                Set code CBM if CDP is supported\n"
> +      "-d                Set data CBM if CDP is supported\n"
>      },
>      { "psr-cat-show",
>        &main_psr_cat_show, 0, 1,
> @@ -551,6 +553,7 @@ struct cmd_spec cmd_table[] = {
>      },
>  
>  #endif
> +

Spurious whitespace change.

~Andrew