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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxx>
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> 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.

>  
>      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.

Wei.

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