Re: [PATCH v10 06/12] tools: add support for cache coloring configuration

["Anthony PERARD" <anthony.perard@xxxxxxxxxx>]

Hi Carlo,

On Tue, Nov 19, 2024 at 03:13:23PM +0100, Carlo Nonato wrote:
> diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
> index e3538ec0ba..4ed339e6e4 100644
> --- a/tools/libs/ctrl/xc_domain.c
> +++ b/tools/libs/ctrl/xc_domain.c
> @@ -2195,6 +2195,41 @@ int xc_domain_soft_reset(xc_interface *xch,
>      domctl.domain = domid;
>      return do_domctl(xch, &domctl);
>  }
> +
> +int xc_domain_set_llc_colors(xc_interface *xch, uint32_t domid,
> +                             const unsigned int *llc_colors,

The hypercall seems to take a buffer of `uint32_t` for `llc_colors`, but
the parameter of the function is `unsigned int` instead, is there any
reason for them to be different?

> +                             unsigned int num_llc_colors)
> +{
> +    struct xen_domctl domctl = {};
> +    DECLARE_HYPERCALL_BUFFER(uint32_t, local);
> +    int ret = -1;
> +
> +    if ( num_llc_colors )
> +    {
> +        size_t bytes = sizeof(uint32_t) * num_llc_colors;
> +
> +        local = xc_hypercall_buffer_alloc(xch, local, bytes);
> +        if ( local == NULL )
> +        {
> +            PERROR("Could not allocate LLC colors for set_llc_colors");
> +            ret = -ENOMEM;

I think we are supposed to return -1 in case of error, see:
    https://elixir.bootlin.com/xen/v4.19.0/source/tools/include/xenctrl.h#L101

And there's nothing else to do, xc_hypercall_buffer_alloc() should
already have set `errno`, and PERROR() preserves it.

> +            goto out;
> +        }

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech