Re: [PATCH v7 04/14] xen/arm: add Dom0 cache coloring support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.03.2024 16:04, Carlo Nonato wrote:
> On Tue, Mar 19, 2024 at 4:30 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 15.03.2024 11:58, Carlo Nonato wrote:
>>> --- a/docs/misc/xen-command-line.pandoc
>>> +++ b/docs/misc/xen-command-line.pandoc
>>> @@ -963,6 +963,15 @@ Controls for the dom0 IOMMU setup.
>>>
>>>  Specify a list of IO ports to be excluded from dom0 access.
>>>
>>> +### dom0-llc-colors
>>> +> `= List of [ <integer> | <integer>-<integer> ]`
>>> +
>>> +> Default: `All available LLC colors`
>>> +
>>> +Specify dom0 LLC color configuration. This option is available only when
>>> +`CONFIG_LLC_COLORING` is enabled. If the parameter is not set, all 
>>> available
>>> +colors are used.
>>
>> My reservation towards this being a top-level option remains.
> 
> How can I turn this into a lower-level option? Moving it into "dom0=" doesn't
> seem possible to me. How can I express a list (llc-colors) inside another list
> (dom0)? dom0=llc-colors=0-3,12-15,other-param=... How can I stop parsing
> before reaching other-param?

For example by using a different separator:

dom0=llc-colors=0-3+12-15,other-param=...

>>> @@ -91,6 +164,61 @@ void cf_check domain_dump_llc_colors(const struct 
>>> domain *d)
>>>      print_colors(d->llc_colors, d->num_llc_colors);
>>>  }
>>>
>>> +static int domain_set_default_colors(struct domain *d)
>>> +{
>>> +    unsigned int *colors = xmalloc_array(unsigned int, max_nr_colors);
>>> +    unsigned int i;
>>> +
>>> +    if ( !colors )
>>> +        return -ENOMEM;
>>> +
>>> +    printk(XENLOG_WARNING
>>> +           "LLC color config not found for %pd, using all colors\n", d);
>>> +
>>> +    for ( i = 0; i < max_nr_colors; i++ )
>>> +        colors[i] = i;
>>> +
>>> +    d->llc_colors = colors;
>>> +    d->num_llc_colors = max_nr_colors;
>>> +
>>> +    return 0;
>>> +}
>>
>> If this function is expected to actually come into play, wouldn't it
>> make sense to set up such an array just once, and re-use it wherever
>> necessary?
> 
> Then how to distinguish when to free it in domain_destroy() and when not to do
> it?

By checking against that one special array instance.

Jan