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

[Michal Orzel <michal.orzel@xxxxxxx>]

On 05/02/2024 10:39, Jan Beulich wrote:
> 
> 
> On 03.02.2024 12:43, Carlo Nonato wrote:
>> On Thu, Feb 1, 2024 at 3:19 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>> 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 ...
>>
>> This informs the user that this configuration will be ignored, but the DomU 
>> can
>> be constructed anyway...
> 
> Yet that's a violation of the principle that Julien had outlined when
> discussing whether to panic() in such cases. The property indicates to
> me that the domain ought to be run with coloring enabled, i.e. not much
> different from ...
> 
>>>> @@ -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()?
>>
>> ... while here we can't continue because there's some error in the
>> configuration and the DomU can't be constructed. Domains must have a valid
>> coloring configuration.
> 
> ... the request not being possible to fulfill here.
+1
If the user requests a certain functionality which cannot be fulfilled, we 
shall panic.
Take a look at e.g. sve, static-shmem, vpl011.

~Michal