On 27/11/2024 14:24, Carlo Nonato wrote:
>
>
> Hi Michal,
>
> On Wed, Nov 27, 2024 at 11:48 AM Michal Orzel <michal.or...@amd.com> wrote:
>> On 19/11/2024 15:13, Carlo Nonato wrote:
>>>
>>>
>>> Last Level Cache (LLC) coloring allows to partition the cache in smaller
>>> chunks called cache colors.
>>>
>>> Since not all architectures can actually implement it, add a
>>> HAS_LLC_COLORING
>>> Kconfig option.
>>> LLC_COLORS_ORDER Kconfig option has a range maximum of 10 (2^10 = 1024)
>>> because that's the number of colors that fit in a 4 KiB page when integers
>>> are 4 bytes long.
>>>
>>> LLC colors are a property of the domain, so struct domain has to be
>>> extended.
>>>
>>> Based on original work from: Luca Miccio <lucmic...@gmail.com>
>>>
>>> Signed-off-by: Carlo Nonato <carlo.non...@minervasys.tech>
>>> Signed-off-by: Marco Solieri <marco.soli...@minervasys.tech>
[...]
>>> +### llc-nr-ways (arm64)
>>> +> `= <integer>`
>>> +
>>> +> Default: `Obtained from hardware`
>>> +
>>> +Specify the number of ways of the Last Level Cache. This option is
>>> available
>>> +only when `CONFIG_LLC_COLORING` is enabled. LLC size and number of ways
>>> are used
>>> +to find the number of supported cache colors. By default the value is
>>> +automatically computed by probing the hardware, but in case of specific
>>> needs,
>>> +it can be manually set. Those include failing probing and debugging/testing
>>> +purposes so that it's possible to emulate platforms with different number
>>> of
>>> +supported colors. If set, also "llc-size" must be set, otherwise the
>>> default
>>> +will be used. Note that using both options implies "llc-coloring=on".
>> I can understand this decision, but ...
>>
>> [...]
>>
>>> + if ( llc_size && llc_nr_ways )
>>> + {
>>> + llc_coloring_enabled = true;
>>> + way_size = llc_size / llc_nr_ways;
>>> + }
>>> + else if ( !llc_coloring_enabled )
>>> + return;
>> the above code does not seem to be right. When debugging or in general it is
>> useful to have on the cmdline:
>> llc-size=1M llc-nr-ways=16 llc-coloring=on
>> and be able to disable it by just switching between on/off in llc-coloring=.
>> However, with your solution,
>> even if I specify llc-coloring=off, it will be enabled because I specified
>> both llc-size and llc-nr-ways.
>> I think llc-coloring= should have a precedence.
>
> How do you differentiate from
> llc-size=1M llc-nr-ways=16 llc-coloring=off
> where llc coloring is disabled, and
> llc-size=1M llc-nr-ways=16
> where llc coloring is enabled? I mean, in both situations llc_coloring_enabled
> is going to be set to false.
I was thinking about the following:
diff --git a/xen/common/llc-coloring.c b/xen/common/llc-coloring.c
index 29b75e0e0d6a..b6684a6cf736 100644
--- a/xen/common/llc-coloring.c
+++ b/xen/common/llc-coloring.c
@@ -11,7 +11,12 @@
#define NR_LLC_COLORS (1U << CONFIG_LLC_COLORS_ORDER)
-static bool __ro_after_init llc_coloring_enabled;
+/*
+ * -1: not specified (disabled unless llc-size and llc-nr-ways present)
+ * 0: explicitly disabled through cmdline
+ * 1: explicitly enabled through cmdline
+ */
+static int8_t __ro_after_init llc_coloring_enabled = -1;
boolean_param("llc-coloring", llc_coloring_enabled);
static unsigned int __initdata llc_size;
@@ -48,7 +53,7 @@ void __init llc_coloring_init(void)
{
unsigned int way_size;
- if ( llc_size && llc_nr_ways )
+ if ( (llc_coloring_enabled < 0) && (llc_size && llc_nr_ways) )
{
llc_coloring_enabled = true;
way_size = llc_size / llc_nr_ways;
~Michal
