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

[He Chen <he.chen@xxxxxxxxxxxxxxx>]

On Wed, Sep 09, 2015 at 03:32:11PM +0800, Chao Peng wrote:
> 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@xxxxxxxxxxxxxxx>
> > ---
> > 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.
> 

My carelessness. I would remove the redundant code lines.

> >  /* 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

Surely.

> > +        } 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;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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