Re: [PATCH v9 24/25] tools: L2 CAT: support set cbm for L2 CAT.

[Yi Sun <yi.y.sun@linux.intel.com>]

On 17-03-28 15:04:03, Wei Liu wrote:
> On Thu, Mar 16, 2017 at 07:08:14PM +0800, Yi Sun wrote:
> > This patch implements the xl/xc changes to support set CBM
> > for L2 CAT.
> > 
> > The new level option is introduced to original CAT setting
> > command in order to set CBM for specified level CAT.
> > - 'xl psr-cat-set' is updated to set cache capacity bitmasks(CBM)
> >   for a domain according to input cache level.
> > 
> > root@:~$ xl psr-cat-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@linux.intel.com>
> > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > v9:
> >     - handle the case to set both CODE and DATA for CDP at same time.
> >       For such case, user does not input '-c' or '-d' to set CDP cbm.
> >     - move xl_cmdimpl.c changes into xl/xl_psr.c.
> >     - move xl_cmdtable.c changes into xl/xl_cmdtable.c.
> 
> Right. Since you've changed how this patch works, it'd be better to drop
> my ack, so that I know it requires my review.
> 
Sorry for that. I will remember this.

> >  
> >      rc = libxl__count_physical_sockets(gc, &nr_sockets);
> >      if (rc) {
> > @@ -331,10 +332,43 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> >              break;
> >  
> >          xc_type = libxl__psr_cbm_type_to_libxc_psr_cat_type(type);
> > -        if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type,
> > -                                       socketid, cbm)) {
> > -            libxl__psr_cat_log_err_msg(gc, errno);
> > -            rc = ERROR_FAIL;
> > +
> > +        if (xc_type == XC_PSR_CAT_L3_CBM) {
> > +            if (xc_psr_cat_get_info(ctx->xch, socketid, 3, &cat_info.cos_max,
> > +                                    &cat_info.cbm_len, &cat_info.cdp_enabled)) {
> > +                libxl__psr_cat_log_err_msg(gc, errno);
> > +                rc = ERROR_FAIL;
> > +                goto out;
> > +            }
> > +        }
> > +
> > +        /*
> > +         * User can set both CDP CODE and DATA at same time when the '-c' or
> > +         * '-d' is not input. In such case, the input type is
> > +         * LIBXL_PSR_CBM_TYPE_L3_CBM. So, we need check if cdp_enabled is true.
> > +         * If it is true, we need set both CODE and DATA.
> 
> "When user sets both CDP CODE and DATA at the same time, the input type
> is..."
> 
> No need to mention xl options here.  Please document xl manpage instead.
> 
> The rest looks sensible.
> 
Thanks a lot for your review and comments!

> Wei.

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