Re: [PATCH v6 07/15] xen/arm: add support for cache coloring configuration via device-tree

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.01.2024 18:18, Carlo Nonato wrote:
> @@ -950,6 +951,11 @@ void __init create_domUs(void)
>  #endif
>          }
>  
> +        dt_property_read_string(node, "llc-colors", &llc_colors_str);
> +        if ( !llc_coloring_enabled && llc_colors_str)
> +            printk(XENLOG_WARNING
> +                   "'llc-colors' found, but LLC coloring is disabled\n");

Why's this just a warning, when ...

> @@ -960,6 +966,11 @@ void __init create_domUs(void)
>              panic("Error creating domain %s (rc = %ld)\n",
>                    dt_node_name(node), PTR_ERR(d));
>  
> +        if ( llc_coloring_enabled &&
> +             (rc = domain_set_llc_colors_from_str(d, llc_colors_str)) )
> +            panic("Error initializing LLC coloring for domain %s (rc = 
> %d)\n",
> +                  dt_node_name(node), rc);

... this results in panic()?

> --- a/xen/common/llc-coloring.c
> +++ b/xen/common/llc-coloring.c
> @@ -254,6 +254,29 @@ int domain_set_llc_colors_domctl(struct domain *d,
>      return domain_check_colors(d);
>  }
>  
> +int domain_set_llc_colors_from_str(struct domain *d, const char *str)

__init ?

> +{
> +    int err;
> +    unsigned int *colors;
> +
> +    if ( !str )
> +        return domain_set_default_colors(d);
> +
> +    colors = alloc_colors(max_nr_colors);
> +    if ( !colors )
> +        return -ENOMEM;
> +
> +    err = parse_color_config(str, colors, max_nr_colors, &d->num_llc_colors);
> +    if ( err )
> +    {
> +        printk(XENLOG_ERR "Error parsing LLC color configuration.");

Nit: No full stop at the end of log messages please.

> +        return err;
> +    }
> +    d->llc_colors = colors;
> +
> +    return domain_check_colors(d);

Same ordering issue as in the earlier patch, I think.

Jan