Re: [PATCH] xen/arm: scan CLIDR Ctype fields upwards when probing LLC

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

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@xxxxxxxxx> wrote:
>
> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
>
> 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@xxxxxxxx>
> ---
> 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
>