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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.05.2024 18:55, Carlo Nonato wrote:
> @@ -266,6 +267,47 @@ int domain_set_llc_colors(struct domain *d,
>      return 0;
>  }
>  
> +int __init domain_set_llc_colors_from_str(struct domain *d, const char *str)
> +{
> +    int err;
> +    unsigned int *colors, num_colors;
> +
> +    if ( !str )
> +    {
> +        domain_set_default_colors(d);
> +        return 0;
> +    }
> +
> +    colors = xmalloc_array(unsigned int, max_nr_colors);
> +    if ( !colors )
> +        return -ENOMEM;
> +
> +    err = parse_color_config(str, colors, max_nr_colors, &num_colors);
> +    if ( err )
> +    {
> +        printk(XENLOG_ERR "Error parsing LLC color configuration");
> +        xfree(colors);
> +        return err;
> +    }
> +
> +    if ( !check_colors(colors, num_colors) )
> +    {
> +        printk(XENLOG_ERR "Bad LLC color config for %pd\n", d);
> +        xfree(colors);
> +        return -EINVAL;
> +    }
> +
> +    /* Adjust the size cause it was initially set to max_nr_colors */
> +    colors = xrealloc_array(colors, num_colors);
> +    if ( !colors )
> +        return -ENOMEM;

In the error case you're leaking the (too large) array. IMO you want this to
be best effort anyway - try to shrink, but if shrinking fails use what you
already have:

    d->llc_colors = xrealloc_array(colors, num_colors);
    if ( !d->llc_colors )
        d->llc_colors = colors;

Jan