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

[Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>]

Hi Anthony,

On Wed, Nov 20, 2024 at 5:25 PM Anthony PERARD
<anthony.perard@xxxxxxxxxx> wrote:
>
> 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?

I don't remember anymore, but I think it should be uint32_t also for the
parameter.

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

You're right.

>> > +            goto out;
> > +        }
>
> Thanks,
>
> --
>
> Anthony Perard | Vates XCP-ng Developer
>
> XCP-ng & Xen Orchestra - Vates solutions
> web: https://vates.tech

(answering the other mail)

On Wed, Nov 20, 2024 at 5:48 PM Anthony PERARD
<anthony.perard@xxxxxxxxxx> wrote:
>
> On Tue, Nov 19, 2024 at 03:13:23PM +0100, Carlo Nonato wrote:
> >  tools/libs/light/libxl_types.idl |  1 +
>
> An other thing I've completely forgot, with this IDL changed there will
> be a need to regen some other files in the repo, namely:
>     tools/golang/xenlight/helpers.gen.go
>     tools/golang/xenlight/types.gen.go
>
> Running this following command should be enough, even if you don't have
> golang installed.
>
>     make -C tools/golang/xenlight helpers.gen.go types.gen.go
>
> If you send an update of the patch series, it would be nice to include
> the regenerated files.

Yes, will do that.

> Cheers,
>
> --
>
> Anthony Perard | Vates XCP-ng Developer
>
> XCP-ng & Xen Orchestra - Vates solutions
>
> web: https://vates.tech

 Thanks.

- Carlo