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.coop...@citrix.com>
Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
> ---
> CC: Anthony PERARD <anthony.per...@vates.tech>
> CC: Michal Orzel <michal.or...@amd.com>
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Julien Grall <jul...@xen.org>
> CC: Roger Pau Monné <roger....@citrix.com>
> CC: Stefano Stabellini <sstabell...@kernel.org>
> CC: Carlo Nonato <carlo.non...@minervasys.tech>
> CC: Marco Solieri <marco.soli...@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
>
