[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 3/3] tools & docs: add L2 CAT support in tools and docs.



On 16-09-30 17:29:58, Konrad Rzeszutek Wilk wrote:
> On Thu, Aug 25, 2016 at 01:22:45PM +0800, Yi Sun wrote:
> > This patch is the xl/xc changes to support Intel L2 CAT
> > (Cache Allocation Technology).
> > 
> > The new level option is introduced to original CAT setting
> > command in order to set CBM for specified level CAT.
> > - 'xl psr-hwinfo' is updated to show both L3 CAT and L2 CAT
> >   info.
> > - 'xl psr-cat-cbm-set' is updated to set cache capacity
> >   bitmasks(CBM) for a domain according to input cache level.
> > - 'xl psr-cat-show' is updated to show CBM of a domain
> >   according to input cache level.
> > 
> > Examples:
> > root@:~$ xl psr-hwinfo --cat
> > Cache Allocation Technology (CAT): L2
> > Socket ID       : 0
> > Maximum COS     : 3
> > CBM length      : 8
> > Default CBM     : 0xff
> > 
> > root@:~$ xl psr-cat-cbm-set -l2 1 0x7f
> > 
> > root@:~$ xl psr-cat-show -l2 1
> > Socket ID       : 0
> > Default CBM     : 0xff
> >    ID                     NAME             CBM
> >     1                 ubuntu14            0x7f
> > 
> > Signed-off-by: He Chen <he.chen@xxxxxxxxxxxxxxx>
> > Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> > ---
> >  docs/man/xl.pod.1.in          |  25 +++++-
> >  docs/misc/xl-psr.markdown     |  10 ++-
> >  tools/libxc/include/xenctrl.h |   7 +-
> >  tools/libxc/xc_psr.c          |  28 +++++--
> >  tools/libxl/libxl.h           |   2 +
> >  tools/libxl/libxl_psr.c       |  50 ++++++++++-
> >  tools/libxl/libxl_types.idl   |   1 +
> >  tools/libxl/xl_cmdimpl.c      | 191 
> > +++++++++++++++++++++++++++++++++++-------
> >  tools/libxl/xl_cmdtable.c     |   4 +-
> >  9 files changed, 268 insertions(+), 50 deletions(-)
> > 
> > diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
> > index 1adf322..47bdfc6 100644
> > --- a/docs/man/xl.pod.1.in
> > +++ b/docs/man/xl.pod.1.in
> > @@ -1660,6 +1660,9 @@ occupancy monitoring share the same set of underlying 
> > monitoring service. Once
> >  a domain is attached to the monitoring service, monitoring data can be 
> > shown
> >  for any of these monitoring types.
> >  
> > +There is no cache monitoring and memory bandwidth monitoring on L2 cache so
> > +far.
> > +
> >  =over 4
> >  
> >  =item B<psr-cmt-attach> [I<domain-id>]
> > @@ -1684,7 +1687,7 @@ monitor types are:
> >  
> >  Intel Broadwell and later server platforms offer capabilities to configure 
> > and
> >  make use of the Cache Allocation Technology (CAT) mechanisms, which enable 
> > more
> > -cache resources (i.e. L3 cache) to be made available for high priority
> > +cache resources (i.e. L3/L2 cache) to be made available for high priority
> >  applications. In the Xen implementation, CAT is used to control cache 
> > allocation
> >  on VM basis. To enforce cache on a specific domain, just set capacity 
> > bitmasks
> >  (CBM) for the domain.
> > @@ -1694,7 +1697,7 @@ Intel Broadwell and later server platforms also offer 
> > Code/Data Prioritization
> >  applications. CDP is used on a per VM basis in the Xen implementation. To
> >  specify code or data CBM for the domain, CDP feature must be enabled and 
> > CBM
> >  type options need to be specified when setting CBM, and the type options 
> > (code
> > -and data) are mutually exclusive.
> > +and data) are mutually exclusive. There is no CDP support on L2 so far.
> >  
> >  =over 4
> >  
> > @@ -1711,6 +1714,11 @@ B<OPTIONS>
> >  
> >  Specify the socket to process, otherwise all sockets are processed.
> >  
> > +=item B<-l LEVEL>, B<--level=LEVEL>
> > +
> > +Specify the cache level to process, otherwise the last level cache is
> 
> s/last level cache/last level cache (L3)/
> 
> Because you may have L4 at some point (weren't there some CPUs with L4?)
> 
Thank you!

> > +processed.
> > +
> >  =item B<-c>, B<--code>
> >  
> >  Set code CBM when CDP is enabled.
> > @@ -1721,10 +1729,21 @@ Set data CBM when CDP is enabled.
> >  
> >  =back
> >  
> > -=item B<psr-cat-show> [I<domain-id>]
> > +=item B<psr-cat-show> [I<OPTIONS>] [I<domain-id>]
> >  
> >  Show CAT settings for a certain domain or all domains.
> >  
> > +B<OPTIONS>
> > +
> > +=over 4
> > +
> > +=item B<-l LEVEL>, B<--level=LEVEL>
> > +
> > +Specify the cache level to process, otherwise the last level cache is
> 
> Again, s/last level/last level (L3)//

Thank you!

> > +processed.
> > +
> > +=back
> > +
> >  =back
> >  
> >  =head1 IGNORED FOR COMPATIBILITY WITH XM
> > diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
> > index c3c1e8e..bd2b6bd 100644
> > --- a/docs/misc/xl-psr.markdown
> > +++ b/docs/misc/xl-psr.markdown
> > @@ -70,7 +70,7 @@ total-mem-bandwidth instead of cache-occupancy). E.g. 
> > after a `xl psr-cmt-attach
> >  
> >  Cache Allocation Technology (CAT) is a new feature available on Intel
> >  Broadwell and later server platforms that allows an OS or Hypervisor/VMM to
> > -partition cache allocation (i.e. L3 cache) based on application priority or
> > +partition cache allocation (i.e. L3/L2 cache) based on application 
> > priority or
> >  Class of Service (COS). Each COS is configured using capacity bitmasks 
> > (CBM)
> >  which represent cache capacity and indicate the degree of overlap and
> >  isolation between classes. System cache resource is divided into numbers of
> > @@ -119,13 +119,19 @@ A cbm is valid only when:
> >  In a multi-socket system, the same cbm will be set on each socket by 
> > default.
> >  Per socket cbm can be specified with the `--socket SOCKET` option.
> >  
> > +In different systems, the different cache level is supported, e.g. L3 
> > cache or
> > +L2 cache. Per cache level cbm can be specified with the `--level LEVEL` 
> > option.
> > +
> >  Setting the CBM may not be successful if insufficient COS is available. In
> >  such case unused COS(es) may be freed by setting CBM of all related 
> > domains to
> >  its default value(all-ones).
> >  
> >  Per domain CBM settings can be shown by:
> >  
> > -`xl psr-cat-show`
> > +`xl psr-cat-show [OPTIONS] <domid>`
> > +
> > +In different systems, the different cache level is supported, e.g. L3 
> > cache or
> > +L2 cache. Per cache level cbm can be specified with the `--level LEVEL` 
> > option.
> >  
> >  ## Code and Data Prioritization (CDP)
> >  
> > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> > index 560ce7b..d70e8a8 100644
> > --- a/tools/libxc/include/xenctrl.h
> > +++ b/tools/libxc/include/xenctrl.h
> > @@ -2585,6 +2585,7 @@ enum xc_psr_cat_type {
> >      XC_PSR_CAT_L3_CBM      = 1,
> >      XC_PSR_CAT_L3_CBM_CODE = 2,
> >      XC_PSR_CAT_L3_CBM_DATA = 3,
> > +    XC_PSR_CAT_L2_CBM      = 4,
> >  };
> >  typedef enum xc_psr_cat_type xc_psr_cat_type;
> >  
> > @@ -2609,9 +2610,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, 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..618dbcc 100644
> > --- a/tools/libxc/xc_psr.c
> > +++ b/tools/libxc/xc_psr.c
> > @@ -266,6 +266,9 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, 
> > uint32_t domid,
> >      case XC_PSR_CAT_L3_CBM_DATA:
> >          cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA;
> >          break;
> > +    case XC_PSR_CAT_L2_CBM:
> > +        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L2_CBM;
> > +        break;
> >      default:
> >          errno = EINVAL;
> >          return -1;
> > @@ -299,6 +302,9 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, 
> > uint32_t domid,
> >      case XC_PSR_CAT_L3_CBM_DATA:
> >          cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA;
> >          break;
> > +    case XC_PSR_CAT_L2_CBM:
> > +        cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L2_CBM;
> > +        break;
> >      default:
> >          errno = EINVAL;
> >          return -1;
> > @@ -317,23 +323,29 @@ 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, int lvl,
> > +                        uint32_t *cos_max, uint32_t *cbm_len,
> > +                        bool *cdp_enabled)
> >  {
> > -    int rc;
> > +    int rc = -1;
> >      DECLARE_SYSCTL;
> >  
> > +    if ( lvl == 2 )
> > +        sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l2_info;
> > +    else if ( lvl == 3 )
> > +        sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
> > +    else
> > +        return rc;
> 
> Perhaps:
> 
> else {
>       errno = EOPNOTSUPP;
>       return rc;
> }
> 
> ?
> 
Ok, thanks!

> > +
> >      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 &
> > +        *cos_max = sysctl.u.psr_cat_op.u.info.cos_max;
> > +        *cbm_len = sysctl.u.psr_cat_op.u.info.cbm_len;
> > +        *cdp_enabled = sysctl.u.psr_cat_op.u.info.flags &
> >                         XEN_SYSCTL_PSR_CAT_L3_CDP;
> >      }
> >  
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index e4c25c4..1cdf621 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -2141,6 +2141,8 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t 
> > domid,
> >   */
> >  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, int lvl);
> >  void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
> >  #endif
> >  
> > diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> > index 99733f6..861d5a8 100644
> > --- a/tools/libxl/libxl_psr.c
> > +++ b/tools/libxl/libxl_psr.c
> > @@ -379,8 +379,54 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, 
> > libxl_psr_cat_info **info,
> >  
> >      libxl_for_each_set_bit(socketid, socketmap) {
> >          ptr[i].id = socketid;
> > -        if (xc_psr_cat_get_l3_info(ctx->xch, socketid, &ptr[i].cos_max,
> > -                                   &ptr[i].cbm_len, &ptr[i].cdp_enabled)) {
> > +        if (xc_psr_cat_get_info(ctx->xch, socketid, 3, &ptr[i].cos_max,
> > +                                &ptr[i].cbm_len, &ptr[i].cdp_enabled)) {
> > +            libxl__psr_cat_log_err_msg(gc, errno);
> > +            rc = ERROR_FAIL;
> > +            free(ptr);
> > +            goto out;
> > +        }
> > +        i++;
> > +    }
> > +
> > +    *info = ptr;
> > +    *nr = i;
> > +out:
> > +    libxl_bitmap_dispose(&socketmap);
> > +    GC_FREE;
> > +    return rc;
> > +}
> > +
> > +int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
> > +                           int *nr, int lvl)
> > +{
> > +    GC_INIT(ctx);
> > +    int rc;
> > +    int i = 0, socketid, nr_sockets;
> > +    libxl_bitmap socketmap;
> > +    libxl_psr_cat_info *ptr;
> > +
> > +    libxl_bitmap_init(&socketmap);
> > +
> > +    rc = libxl__count_physical_sockets(gc, &nr_sockets);
> > +    if (rc) {
> > +        LOGE(ERROR, "failed to get system socket count");
> > +        goto out;
> > +    }
> > +
> > +    libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets);
> > +    rc = libxl_get_online_socketmap(ctx, &socketmap);
> > +    if (rc < 0) {
> > +        LOGE(ERROR, "failed to get available sockets");
> > +        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)) {
> 
> This function looks very close to  libxl_psr_cat_get_l3_info. Any chance
> that you can one function doing it?
> 
Yes, per my original design, libxl_psr_cat_get_l3_info will be replaced by this
function. But considering to be backward compatible, I keep this interface in
libxenlight.so.

> >              libxl__psr_cat_log_err_msg(gc, errno);
> >              rc = ERROR_FAIL;
> >              free(ptr);
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 98bfc3a..4503a8b 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -890,6 +890,7 @@ libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
> >      (1, "L3_CBM"),
> >      (2, "L3_CBM_CODE"),
> >      (3, "L3_CBM_DATA"),
> > +    (4, "L2_CBM"),
> >      ])
> >  
> >  libxl_psr_cat_info = Struct("psr_cat_info", [
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 7540fb1..4a83caf 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -9273,18 +9273,124 @@ int main_psr_cmt_show(int argc, char **argv)
> >  #endif
> >  
> >  #ifdef LIBXL_HAVE_PSR_CAT
> > -static int psr_cat_hwinfo(void)
> > +static void psr_cat_print_one_domain_cbm_type(uint32_t domid, uint32_t 
> > socketid,
> > +                                              libxl_psr_cbm_type type)
> > +{
> > +    uint64_t cbm;
> > +
> > +    if (!libxl_psr_cat_get_cbm(ctx, domid, type, socketid, &cbm))
> > +        printf("%#16"PRIx64, cbm);
> > +    else
> > +        printf("%16s", "error");
> > +}
> > +
> > +static int psr_l2_cat_hwinfo(void)
> > +{
> > +    int rc;
> > +    int i, nr;
> 
> How about unsigned int?

Yes, thanks!

> > +    libxl_psr_cat_info *info;
> > +
> > +    printf("Cache Allocation Technology (CAT): L2\n");
> > +
> > +    rc = libxl_psr_cat_get_info(ctx, &info, &nr, 2);
> > +    if (rc) {
> > +        fprintf(stderr, "Failed to get l2 cat info\n");
> > +        return rc;
> > +    }
> > +
> > +    for (i = 0; i < nr; i++) {
> > +        /* There is no CMT on L2 cache so far. */
> > +        printf("%-16s: %u\n", "Socket ID", info[i].id);
> > +        printf("%-16s: %u\n", "Maximum COS", info[i].cos_max);
> > +        printf("%-16s: %u\n", "CBM length", info[i].cbm_len);
> > +        printf("%-16s: %#llx\n", "Default CBM",
> > +               (1ull << info[i].cbm_len) - 1);
> > +    }
> > +
> > +    libxl_psr_cat_info_list_free(info, nr);
> > +    return rc;
> > +}
> > +
> > +static void psr_l2_cat_print_one_domain_cbm(uint32_t domid, uint32_t 
> > socketid)
> > +{
> > +    char *domain_name;
> > +
> > +    domain_name = libxl_domid_to_name(ctx, domid);
> > +    printf("%5d%25s", domid, domain_name);
> > +    free(domain_name);
> > +
> > +    psr_cat_print_one_domain_cbm_type(domid, socketid,
> > +                                      LIBXL_PSR_CBM_TYPE_L2_CBM);
> > +
> > +    printf("\n");
> > +}
> > +
> > +static int psr_l2_cat_print_domain_cbm(uint32_t domid, uint32_t socketid)
> > +{
> > +    int i, nr_domains;
> 
> unsigned int?

Yes, thanks!

> > +    libxl_dominfo *list;
> > +
> > +    if (domid != INVALID_DOMID) {
> > +        psr_l2_cat_print_one_domain_cbm(domid, socketid);
> > +        return 0;
> > +    }
> > +
> > +    if (!(list = libxl_list_domain(ctx, &nr_domains))) {
> > +        fprintf(stderr, "Failed to get domain list for cbm display\n");
> > +        return -1;
> 
> -1 ? Not ERROR_FAIL?
> 
Just follow the old codes. ERROR_FAIL should be good. Thanks!

> > +    }
> > +
> > +    for (i = 0; i < nr_domains; i++)
> > +        psr_l2_cat_print_one_domain_cbm(list[i].domid, socketid);
> > +    libxl_dominfo_list_free(list, nr_domains);
> > +
> > +    return 0;
> > +}
> > +
> > +static int psr_l2_cat_print_socket(uint32_t domid, libxl_psr_cat_info 
> > *info)
> > +{
> > +    printf("%-16s: %u\n", "Socket ID", info->id);
> > +    printf("%-16s: %#llx\n", "Default CBM", (1ull << info->cbm_len) - 1);
> > +    printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
> > +
> > +    return psr_l2_cat_print_domain_cbm(domid, info->id);
> > +}
> > +
> > +static int psr_l2_cat_show(uint32_t domid)
> > +{
> > +    int i, nr;
> 
> unsigned int?

Yes, thanks!

> > +    int rc;
> > +    libxl_psr_cat_info *info;
> > +
> > +    rc = libxl_psr_cat_get_info(ctx, &info, &nr, 2);
> > +    if (rc) {
> > +        fprintf(stderr, "Failed to get l2 cat info\n");
> > +        return rc;
> > +    }
> > +
> > +    for (i = 0; i < nr; i++) {
> > +        rc = psr_l2_cat_print_socket(domid, info + i);
> > +        if (rc)
> > +            goto out;
> > +    }
> > +
> > +out:
> > +    libxl_psr_cat_info_list_free(info, nr);
> > +    return rc;
> > +}
> > +
> > +static int psr_l3_cat_hwinfo(void)
> >  {
> >      int rc;
> >      int i, nr;
> >      uint32_t l3_cache_size;
> >      libxl_psr_cat_info *info;
> >  
> > -    printf("Cache Allocation Technology (CAT):\n");
> > +    printf("Cache Allocation Technology (CAT): L3\n");
> >  
> > -    rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr);
> > +    rc = libxl_psr_cat_get_info(ctx, &info, &nr, 3);
> >      if (rc) {
> > -        fprintf(stderr, "Failed to get cat info\n");
> > +        fprintf(stderr, "Failed to get l3 cat info\n");
> >          return rc;
> >      }
> >  
> > @@ -9310,17 +9416,6 @@ out:
> >      return rc;
> >  }
> >  
> > -static void psr_cat_print_one_domain_cbm_type(uint32_t domid, uint32_t 
> > socketid,
> > -                                              libxl_psr_cbm_type type)
> > -{
> > -    uint64_t cbm;
> > -
> > -    if (!libxl_psr_cat_get_cbm(ctx, domid, type, socketid, &cbm))
> > -        printf("%#16"PRIx64, cbm);
> > -    else
> > -        printf("%16s", "error");
> > -}
> > -
> >  static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid,
> >                                           bool cdp_enabled)
> >  {
> > @@ -9395,9 +9490,9 @@ static int psr_cat_show(uint32_t domid)
> >      int rc;
> >      libxl_psr_cat_info *info;
> >  
> > -    rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr);
> > +    rc = libxl_psr_cat_get_info(ctx, &info, &nr, 3);
> >      if (rc) {
> > -        fprintf(stderr, "Failed to get cat info\n");
> > +        fprintf(stderr, "Failed to get l3 cat info\n");
> >          return rc;
> >      }
> >  
> > @@ -9424,18 +9519,20 @@ int main_psr_cat_cbm_set(int argc, char **argv)
> >      libxl_string_list socket_list;
> >      unsigned long start, end;
> >      int i, j, len;
> > +    int lvl = 3;
> 
> unsigned int?
> 
Yes, thanks!

> >  
> >      static struct option opts[] = {
> >          {"socket", 1, 0, 's'},
> >          {"data", 0, 0, 'd'},
> >          {"code", 0, 0, 'c'},
> > +        {"level", 1, 0, 'l'},
> >          COMMON_LONG_OPTS
> >      };
> >  
> >      libxl_socket_bitmap_alloc(ctx, &target_map, 0);
> >      libxl_bitmap_set_none(&target_map);
> >  
> > -    SWITCH_FOREACH_OPT(opt, "s:cd", opts, "psr-cat-cbm-set", 2) {
> > +    SWITCH_FOREACH_OPT(opt, "s:l:cd", opts, "psr-cat-cbm-set", 2) {
> >      case 's':
> >          trim(isspace, optarg, &value);
> >          split_string_into_string_list(value, ",", &socket_list);
> > @@ -9455,17 +9552,31 @@ int main_psr_cat_cbm_set(int argc, char **argv)
> >      case 'c':
> >          opt_code = 1;
> >          break;
> > +    case 'l':
> > +        lvl = atoi(optarg);
> > +        break;
> >      }
> >  
> > -    if (opt_data && opt_code) {
> > -        fprintf(stderr, "Cannot handle -c and -d at the same time\n");
> > -        return -1;
> > -    } else if (opt_data) {
> > -        type = LIBXL_PSR_CBM_TYPE_L3_CBM_DATA;
> > -    } else if (opt_code) {
> > -        type = LIBXL_PSR_CBM_TYPE_L3_CBM_CODE;
> > +    if (lvl == 2) {
> > +        if (opt_data || opt_code) {
> > +            fprintf(stderr, "L2 CAT does not support CDP yet.\n");
> > +            return -1;
> 
> ERROR_FAIL?
> 
Should be good. Thanks!

> > +        }
> > +        type = LIBXL_PSR_CBM_TYPE_L2_CBM;
> > +    } else if (lvl == 3) {
> > +        if (opt_data && opt_code) {
> > +            fprintf(stderr, "Cannot handle -c and -d at the same time\n");
> > +            return -1;
> > +        } else if (opt_data) {
> > +            type = LIBXL_PSR_CBM_TYPE_L3_CBM_DATA;
> > +        } else if (opt_code) {
> > +            type = LIBXL_PSR_CBM_TYPE_L3_CBM_CODE;
> > +        } else {
> > +            type = LIBXL_PSR_CBM_TYPE_L3_CBM;
> > +        }
> >      } else {
> > -        type = LIBXL_PSR_CBM_TYPE_L3_CBM;
> > +        fprintf(stderr, "Invalid cache level.\n");
> > +        return -1;
> 
> ERROR_FAIL?

Thanks!

> >      }
> >  
> >      if (libxl_bitmap_is_empty(&target_map))
> > @@ -9487,11 +9598,20 @@ int main_psr_cat_cbm_set(int argc, char **argv)
> >  
> >  int main_psr_cat_show(int argc, char **argv)
> >  {
> > -    int opt;
> > +    int opt = 0;
> >      uint32_t domid;
> > +    int lvl = 3;
> 
> unsigned int?

Thanks!

> > +    int rc;
> >  
> > -    SWITCH_FOREACH_OPT(opt, "", NULL, "psr-cat-show", 0) {
> > -        /* No options */
> > +    static struct option opts[] = {
> > +        {"level", 1, 0, 'l'},
> > +        COMMON_LONG_OPTS
> > +    };
> > +
> > +    SWITCH_FOREACH_OPT(opt, "l:", opts, "psr-cat-show", 1) {
> > +    case 'l':
> > +        lvl = atoi(optarg);
> > +        break;
> >      }
> >  
> >      if (optind >= argc)
> > @@ -9503,7 +9623,12 @@ int main_psr_cat_show(int argc, char **argv)
> >          return 2;
> >      }
> >  
> > -    return psr_cat_show(domid);
> > +    if (3 == lvl)
> > +        rc = psr_cat_show(domid);
> > +    else if (2 == lvl)
> 
> Could you swap that around? lvl == 2 or such?
> 
That is a coding habit. To put value before variable can help to find code
error by compilation, like '2 = lvl', a '=' is missed.

If this is not the style of Xen, I can change it. Thanks!

> > +        rc = psr_l2_cat_show(domid);
> > +
> > +    return rc;
> >  }
> >  
> >  int main_psr_hwinfo(int argc, char **argv)
> > @@ -9529,7 +9654,11 @@ int main_psr_hwinfo(int argc, char **argv)
> >          ret = psr_cmt_hwinfo();
> >  
> >      if (!ret && (all || cat))
> > -        ret = psr_cat_hwinfo();
> > +        ret = psr_l3_cat_hwinfo();
> > +
> > +    /* L2 CAT is independent with CMT and L3 CAT */
> 
> s/with/off/?

Thank you!

> > +    if (all || cat)
> > +        ret = psr_l2_cat_hwinfo();
> >  
> >      return ret;
> >  }
> > diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> > index 85c1e0f..bb3ce69 100644
> > --- a/tools/libxl/xl_cmdtable.c
> > +++ b/tools/libxl/xl_cmdtable.c
> > @@ -549,13 +549,15 @@ 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"
> > +      "-l <level>        Specify the cache level to process, otherwise L3 
> > cache is 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,
> >        "Show Cache Allocation Technology information",
> > -      "<Domain>",
> > +      "[options] <Domain>",
> > +      "-l <level>        Specify the cache level to process, otherwise L3 
> > cache is processed\n"
> >      },
> >  
> >  #endif
> > -- 
> > 1.9.1
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > https://lists.xen.org/xen-devel

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.