Re: [PATCH 4/4] tools/libxl: Remove unconditional XEN_DOMCTL_set_llc_colors hypercall

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 24 Jul 2025, Andrew Cooper wrote:
> Hypercalls are not free; cache colouring is an experimental feature, and
> ignoring an -EOPNOTSUPP is bad form.
> 
> Now that Xen has been fixed to initialise colouring information correctly for
> domains, xc_domain_set_llc_colors() only needs calling for domains with
> explicit configuration.
> 
> Rearrange the logic to avoid the hypercall in the general case, leaving a
> comment explaining why it is performed so early.
> 
> Fixes: 748bd725fbec ("tools: add support for cache coloring configuration")
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>
> CC: Marco Solieri <marco.solieri@xxxxxxxxxxxxxxx>
> ---
>  tools/libs/light/libxl_create.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index 4301f17f901a..4042ae1a8957 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -699,19 +699,21 @@ int libxl__domain_make(libxl__gc *gc, 
> libxl_domain_config *d_config,
>              /* A new domain now exists */
>              *domid = local_domid;
>  
> -            ret = xc_domain_set_llc_colors(ctx->xch, local_domid,
> -                                           b_info->llc_colors,
> -                                           b_info->num_llc_colors);
> -            if (ret < 0) {
> -                if (errno == EOPNOTSUPP) {
> -                    if (b_info->num_llc_colors > 0) {
> +            /*
> +             * If Cache Coloring is wanted for the guest, it must be
> +             * communicated to Xen prior to allocating guest memory.
> +             */
> +            if (b_info->num_llc_colors) {
> +                ret = xc_domain_set_llc_colors(ctx->xch, local_domid,
> +                                               b_info->llc_colors,
> +                                               b_info->num_llc_colors);
> +                if (ret < 0) {
> +                    if (errno == EOPNOTSUPP) {
>                          LOGED(ERROR, local_domid,
> -                            "LLC coloring not enabled in the hypervisor");
> -                        rc = ERROR_FAIL;
> -                        goto out;
> +                              "LLC coloring not enabled in the hypervisor");
> +                    } else {
> +                        LOGED(ERROR, local_domid, "LLC colors allocation 
> failed");
>                      }
> -                } else {
> -                    LOGED(ERROR, local_domid, "LLC colors allocation 
> failed");
>                      rc = ERROR_FAIL;
>                      goto out;
>                  }
> -- 
> 2.39.5
>