Re: [PATCH v4 04/11] xen: extend domctl interface for cache coloring

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.01.2023 16:47, Carlo Nonato wrote:
> @@ -275,6 +276,19 @@ unsigned int *dom0_llc_colors(unsigned int *num_colors)
>      return colors;
>  }
>  
> +unsigned int *llc_colors_from_guest(struct xen_domctl_createdomain *config)

const struct ...?

> +{
> +    unsigned int *colors;
> +
> +    if ( !config->num_llc_colors )
> +        return NULL;
> +
> +    colors = alloc_colors(config->num_llc_colors);

Error handling needs to occur here; the panic() in alloc_colors() needs
to go away.

> @@ -434,7 +436,15 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>              rover = dom;
>          }
>  
> -        d = domain_create(dom, &op->u.createdomain, false);
> +        if ( llc_coloring_enabled )
> +        {
> +            llc_colors = llc_colors_from_guest(&op->u.createdomain);
> +            num_llc_colors = op->u.createdomain.num_llc_colors;

I think you would better avoid setting num_llc_colors to non-zero if
you got back NULL from the function. It's at best confusing.

> @@ -92,6 +92,10 @@ struct xen_domctl_createdomain {
>      /* CPU pool to use; specify 0 or a specific existing pool */
>      uint32_t cpupool_id;
>  
> +    /* IN LLC coloring parameters */
> +    uint32_t num_llc_colors;
> +    XEN_GUEST_HANDLE(uint32) llc_colors;

Despite your earlier replies I continue to be unconvinced that this
is information which needs to be available right at domain_create.
Without that you'd also get away without the sufficiently odd
domain_create_llc_colored(). (Odd because: Think of two or three
more extended features appearing, all of which want a special cased
domain_create().)

Jan