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@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Carlo Nonato <carlo.nonato@minervasys.tech>
CC: Marco Solieri <marco.solieri@minervasys.tech>
---
 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

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@citrix.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> CC: Anthony PERARD <anthony.perard@vates.tech>
> CC: Michal Orzel <michal.orzel@amd.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Julien Grall <julien@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Carlo Nonato <carlo.nonato@minervasys.tech>
> CC: Marco Solieri <marco.solieri@minervasys.tech>
> ---
>  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
>