From: Mykola Kvach <mykola_kvach@epam.com>

get_llc_way_size() currently scans CLIDR_EL1 Ctype fields from the
highest level downwards and stops at the first unified cache it finds.

However, CLIDR_EL1 describes the cache hierarchy from Ctype1 upwards.
Arm ARM DDI 0487J.a, D19.2.27 says that once software has seen a
Ctype value of 0b000 while reading from Ctype1 upwards, no caches
manageable by the architected set/way maintenance instructions exist at
further-out levels, and the higher Ctype fields must be ignored.

The current reverse scan can therefore select a unified cache level from
a Ctype field above the first no-cache level. Such a field is not part of
the architecturally described CLIDR/CCSIDR cache hierarchy and should not
be used for selecting the CCSIDR level.

Scan Ctype fields from L1 upwards, stop at the first no-cache level, and
keep the outermost unified cache observed before that point.

This preserves the result for regular cache hierarchies, while avoiding
selection of an architecturally ignored Ctype field.

Fixes: f4985fce6f0b ("xen/arm: add initial support for LLC coloring on arm64")
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
This patch follows the xen-devel discussion:
https://lists.xenproject.org/archives/html/xen-devel/2026-01/msg00345.html

In that thread, Michal noted that the reverse scan was a simplification
rather than an intentional requirement, and that changing the
implementation would be fine.

Testing performed:
- standalone synthetic CLIDR tests covered both regular and pathological
  Ctype sequences and showed that the forward scan ignores unified cache
  levels above the first Ctype == 0b000 while the reverse scan can pick
  them
- Renesas H3ULCB booted with llc-coloring=on
---
 xen/arch/arm/llc-coloring.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c
index 6f78817c57..3783f4c824 100644
--- a/xen/arch/arm/llc-coloring.c
+++ b/xen/arch/arm/llc-coloring.c
@@ -22,21 +22,33 @@ unsigned int __init get_llc_way_size(void)
     register_t id_aa64mmfr2_el1 = READ_SYSREG(ID_AA64MMFR2_EL1);
     uint32_t ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT;
     uint32_t ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK;
-    unsigned int n, line_size, num_sets;
-
-    for ( n = CLIDR_CTYPEn_LEVELS; n != 0; n-- )
+    unsigned int n, line_size, num_sets, llc_level = 0;
+
+    /*
+     * CLIDR_EL1 Ctype fields are interpreted from Ctype1 upwards. Once a
+     * no-cache level is seen, higher Ctype fields are architecturally ignored
+     * for the CLIDR/CCSIDR set/way manageable cache hierarchy.
+     *
+     * Keep the outermost unified cache before that point.
+     */
+    for ( n = 1; n <= CLIDR_CTYPEn_LEVELS; n++ )
     {
         uint8_t ctype_n = (clidr_el1 >> CLIDR_CTYPEn_SHIFT(n)) &
                            CLIDR_CTYPEn_MASK;
 
+        if ( ctype_n == 0b000 )
+            break;
+
         /* Unified cache (see Arm ARM DDI 0487J.a D19.2.27) */
         if ( ctype_n == 0b100 )
-            break;
+            llc_level = n;
     }
 
-    if ( n == 0 )
+    if ( !llc_level )
         return 0;
 
+    n = llc_level;
+
     WRITE_SYSREG((n - 1) << CSSELR_LEVEL_SHIFT, CSSELR_EL1);
     isb();
 
-- 
2.43.0

On 04-May-26 11:19, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@epam.com>
> 
> get_llc_way_size() currently scans CLIDR_EL1 Ctype fields from the
> highest level downwards and stops at the first unified cache it finds.
> 
> However, CLIDR_EL1 describes the cache hierarchy from Ctype1 upwards.
> Arm ARM DDI 0487J.a, D19.2.27 says that once software has seen a
> Ctype value of 0b000 while reading from Ctype1 upwards, no caches
> manageable by the architected set/way maintenance instructions exist at
> further-out levels, and the higher Ctype fields must be ignored.
> 
> The current reverse scan can therefore select a unified cache level from
> a Ctype field above the first no-cache level. Such a field is not part of
> the architecturally described CLIDR/CCSIDR cache hierarchy and should not
> be used for selecting the CCSIDR level.
> 
> Scan Ctype fields from L1 upwards, stop at the first no-cache level, and
> keep the outermost unified cache observed before that point.
> 
> This preserves the result for regular cache hierarchies, while avoiding
> selection of an architecturally ignored Ctype field.
> 
> Fixes: f4985fce6f0b ("xen/arm: add initial support for LLC coloring on arm64")
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> This patch follows the xen-devel discussion:
> https://lists.xenproject.org/archives/html/xen-devel/2026-01/msg00345.html
> 
> In that thread, Michal noted that the reverse scan was a simplification
> rather than an intentional requirement, and that changing the
> implementation would be fine.
> 
> Testing performed:
> - standalone synthetic CLIDR tests covered both regular and pathological
>   Ctype sequences and showed that the forward scan ignores unified cache
>   levels above the first Ctype == 0b000 while the reverse scan can pick
>   them
> - Renesas H3ULCB booted with llc-coloring=on
> ---
>  xen/arch/arm/llc-coloring.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c
> index 6f78817c57..3783f4c824 100644
> --- a/xen/arch/arm/llc-coloring.c
> +++ b/xen/arch/arm/llc-coloring.c
> @@ -22,21 +22,33 @@ unsigned int __init get_llc_way_size(void)
>      register_t id_aa64mmfr2_el1 = READ_SYSREG(ID_AA64MMFR2_EL1);
>      uint32_t ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT;
>      uint32_t ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK;
> -    unsigned int n, line_size, num_sets;
> -
> -    for ( n = CLIDR_CTYPEn_LEVELS; n != 0; n-- )
> +    unsigned int n, line_size, num_sets, llc_level = 0;
> +
> +    /*
> +     * CLIDR_EL1 Ctype fields are interpreted from Ctype1 upwards. Once a
> +     * no-cache level is seen, higher Ctype fields are architecturally ignored
> +     * for the CLIDR/CCSIDR set/way manageable cache hierarchy.
> +     *
> +     * Keep the outermost unified cache before that point.
> +     */
> +    for ( n = 1; n <= CLIDR_CTYPEn_LEVELS; n++ )
>      {
>          uint8_t ctype_n = (clidr_el1 >> CLIDR_CTYPEn_SHIFT(n)) &
>                             CLIDR_CTYPEn_MASK;
>  
> +        if ( ctype_n == 0b000 )
> +            break;
> +
>          /* Unified cache (see Arm ARM DDI 0487J.a D19.2.27) */
>          if ( ctype_n == 0b100 )
> -            break;
> +            llc_level = n;
>      }
>  
> -    if ( n == 0 )
> +    if ( !llc_level )
>          return 0;
>  
> +    n = llc_level;
After a loop, n does not carry any meaning, so I find this assignment a bit odd
and confusing to read. Just use llc_level below. With that:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Hi Michal,

Thank you for the feedback.

On Fri, May 22, 2026 at 9:41 AM Orzel, Michal <michal.orzel@amd.com> wrote:
>
>
>
> On 04-May-26 11:19, Mykola Kvach wrote:
> > From: Mykola Kvach <mykola_kvach@epam.com>
> >
> > get_llc_way_size() currently scans CLIDR_EL1 Ctype fields from the
> > highest level downwards and stops at the first unified cache it finds.
> >
> > However, CLIDR_EL1 describes the cache hierarchy from Ctype1 upwards.
> > Arm ARM DDI 0487J.a, D19.2.27 says that once software has seen a
> > Ctype value of 0b000 while reading from Ctype1 upwards, no caches
> > manageable by the architected set/way maintenance instructions exist at
> > further-out levels, and the higher Ctype fields must be ignored.
> >
> > The current reverse scan can therefore select a unified cache level from
> > a Ctype field above the first no-cache level. Such a field is not part of
> > the architecturally described CLIDR/CCSIDR cache hierarchy and should not
> > be used for selecting the CCSIDR level.
> >
> > Scan Ctype fields from L1 upwards, stop at the first no-cache level, and
> > keep the outermost unified cache observed before that point.
> >
> > This preserves the result for regular cache hierarchies, while avoiding
> > selection of an architecturally ignored Ctype field.
> >
> > Fixes: f4985fce6f0b ("xen/arm: add initial support for LLC coloring on arm64")
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> > This patch follows the xen-devel discussion:
> > https://lists.xenproject.org/archives/html/xen-devel/2026-01/msg00345.html
> >
> > In that thread, Michal noted that the reverse scan was a simplification
> > rather than an intentional requirement, and that changing the
> > implementation would be fine.
> >
> > Testing performed:
> > - standalone synthetic CLIDR tests covered both regular and pathological
> >   Ctype sequences and showed that the forward scan ignores unified cache
> >   levels above the first Ctype == 0b000 while the reverse scan can pick
> >   them
> > - Renesas H3ULCB booted with llc-coloring=on
> > ---
> >  xen/arch/arm/llc-coloring.c | 22 +++++++++++++++++-----
> >  1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c
> > index 6f78817c57..3783f4c824 100644
> > --- a/xen/arch/arm/llc-coloring.c
> > +++ b/xen/arch/arm/llc-coloring.c
> > @@ -22,21 +22,33 @@ unsigned int __init get_llc_way_size(void)
> >      register_t id_aa64mmfr2_el1 = READ_SYSREG(ID_AA64MMFR2_EL1);
> >      uint32_t ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT;
> >      uint32_t ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK;
> > -    unsigned int n, line_size, num_sets;
> > -
> > -    for ( n = CLIDR_CTYPEn_LEVELS; n != 0; n-- )
> > +    unsigned int n, line_size, num_sets, llc_level = 0;
> > +
> > +    /*
> > +     * CLIDR_EL1 Ctype fields are interpreted from Ctype1 upwards. Once a
> > +     * no-cache level is seen, higher Ctype fields are architecturally ignored
> > +     * for the CLIDR/CCSIDR set/way manageable cache hierarchy.
> > +     *
> > +     * Keep the outermost unified cache before that point.
> > +     */
> > +    for ( n = 1; n <= CLIDR_CTYPEn_LEVELS; n++ )
> >      {
> >          uint8_t ctype_n = (clidr_el1 >> CLIDR_CTYPEn_SHIFT(n)) &
> >                             CLIDR_CTYPEn_MASK;
> >
> > +        if ( ctype_n == 0b000 )
> > +            break;
> > +
> >          /* Unified cache (see Arm ARM DDI 0487J.a D19.2.27) */
> >          if ( ctype_n == 0b100 )
> > -            break;
> > +            llc_level = n;
> >      }
> >
> > -    if ( n == 0 )
> > +    if ( !llc_level )
> >          return 0;
> >
> > +    n = llc_level;
> After a loop, n does not carry any meaning, so I find this assignment a bit odd
> and confusing to read. Just use llc_level below. With that:

Ack.

Best regards,
Mykola

Hi,

Adding Luca, Carlo and Marco for visibility, as this patch touches code
based on Luca's original work and signed off by Carlo and Marco.

The patch already has Luca Fancellu's Reviewed-by. Please let me know if
you have any concerns.

Best regards,
Mykola

On Mon, May 4, 2026 at 12:21 PM Mykola Kvach <xakep.amatop@gmail.com> wrote:
>
> From: Mykola Kvach <mykola_kvach@epam.com>
>
> get_llc_way_size() currently scans CLIDR_EL1 Ctype fields from the
> highest level downwards and stops at the first unified cache it finds.
>
> However, CLIDR_EL1 describes the cache hierarchy from Ctype1 upwards.
> Arm ARM DDI 0487J.a, D19.2.27 says that once software has seen a
> Ctype value of 0b000 while reading from Ctype1 upwards, no caches
> manageable by the architected set/way maintenance instructions exist at
> further-out levels, and the higher Ctype fields must be ignored.
>
> The current reverse scan can therefore select a unified cache level from
> a Ctype field above the first no-cache level. Such a field is not part of
> the architecturally described CLIDR/CCSIDR cache hierarchy and should not
> be used for selecting the CCSIDR level.
>
> Scan Ctype fields from L1 upwards, stop at the first no-cache level, and
> keep the outermost unified cache observed before that point.
>
> This preserves the result for regular cache hierarchies, while avoiding
> selection of an architecturally ignored Ctype field.
>
> Fixes: f4985fce6f0b ("xen/arm: add initial support for LLC coloring on arm64")
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> This patch follows the xen-devel discussion:
> https://lists.xenproject.org/archives/html/xen-devel/2026-01/msg00345.html
>
> In that thread, Michal noted that the reverse scan was a simplification
> rather than an intentional requirement, and that changing the
> implementation would be fine.
>
> Testing performed:
> - standalone synthetic CLIDR tests covered both regular and pathological
>   Ctype sequences and showed that the forward scan ignores unified cache
>   levels above the first Ctype == 0b000 while the reverse scan can pick
>   them
> - Renesas H3ULCB booted with llc-coloring=on
> ---
>  xen/arch/arm/llc-coloring.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c
> index 6f78817c57..3783f4c824 100644
> --- a/xen/arch/arm/llc-coloring.c
> +++ b/xen/arch/arm/llc-coloring.c
> @@ -22,21 +22,33 @@ unsigned int __init get_llc_way_size(void)
>      register_t id_aa64mmfr2_el1 = READ_SYSREG(ID_AA64MMFR2_EL1);
>      uint32_t ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT;
>      uint32_t ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK;
> -    unsigned int n, line_size, num_sets;
> -
> -    for ( n = CLIDR_CTYPEn_LEVELS; n != 0; n-- )
> +    unsigned int n, line_size, num_sets, llc_level = 0;
> +
> +    /*
> +     * CLIDR_EL1 Ctype fields are interpreted from Ctype1 upwards. Once a
> +     * no-cache level is seen, higher Ctype fields are architecturally ignored
> +     * for the CLIDR/CCSIDR set/way manageable cache hierarchy.
> +     *
> +     * Keep the outermost unified cache before that point.
> +     */
> +    for ( n = 1; n <= CLIDR_CTYPEn_LEVELS; n++ )
>      {
>          uint8_t ctype_n = (clidr_el1 >> CLIDR_CTYPEn_SHIFT(n)) &
>                             CLIDR_CTYPEn_MASK;
>
> +        if ( ctype_n == 0b000 )
> +            break;
> +
>          /* Unified cache (see Arm ARM DDI 0487J.a D19.2.27) */
>          if ( ctype_n == 0b100 )
> -            break;
> +            llc_level = n;
>      }
>
> -    if ( n == 0 )
> +    if ( !llc_level )
>          return 0;
>
> +    n = llc_level;
> +
>      WRITE_SYSREG((n - 1) << CSSELR_LEVEL_SHIFT, CSSELR_EL1);
>      isb();
>
> --
> 2.43.0
>

Hi Mykola,

> On 4 May 2026, at 10:19, Mykola Kvach <xakep.amatop@gmail.com> wrote:
> 
> From: Mykola Kvach <mykola_kvach@epam.com>
> 
> get_llc_way_size() currently scans CLIDR_EL1 Ctype fields from the
> highest level downwards and stops at the first unified cache it finds.
> 
> However, CLIDR_EL1 describes the cache hierarchy from Ctype1 upwards.
> Arm ARM DDI 0487J.a, D19.2.27 says that once software has seen a
> Ctype value of 0b000 while reading from Ctype1 upwards, no caches
> manageable by the architected set/way maintenance instructions exist at
> further-out levels, and the higher Ctype fields must be ignored.
> 
> The current reverse scan can therefore select a unified cache level from
> a Ctype field above the first no-cache level. Such a field is not part of
> the architecturally described CLIDR/CCSIDR cache hierarchy and should not
> be used for selecting the CCSIDR level.
> 
> Scan Ctype fields from L1 upwards, stop at the first no-cache level, and
> keep the outermost unified cache observed before that point.
> 
> This preserves the result for regular cache hierarchies, while avoiding
> selection of an architecturally ignored Ctype field.
> 
> Fixes: f4985fce6f0b ("xen/arm: add initial support for LLC coloring on arm64")
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> This patch follows the xen-devel discussion:
> https://lists.xenproject.org/archives/html/xen-devel/2026-01/msg00345.html
> 
> In that thread, Michal noted that the reverse scan was a simplification
> rather than an intentional requirement, and that changing the
> implementation would be fine.
> 
> Testing performed:
> - standalone synthetic CLIDR tests covered both regular and pathological
>  Ctype sequences and showed that the forward scan ignores unified cache
>  levels above the first Ctype == 0b000 while the reverse scan can pick
>  them
> - Renesas H3ULCB booted with llc-coloring=on
> ---

The changes looks ok to me

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Cheers,
Luca