Re: [PATCH v2 2/2] xen/arm: Restrict Kconfig configuration for LLC coloring

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 17/02/2025 11:27, Luca Fancellu wrote:
> 
> 
> LLC coloring can be used only on MMU system, move the code
> that selects it from ARM_64 to MMU and add the ARM_64
> dependency.
> 
> While there, add a clarification comment in the startup
> code related to the LLC coloring, because boot_fdt_info()
> is required to be called before llc_coloring_init(), because
> it parses the memory banks from the DT, but to discover that
> the developer needs to dig into the function.
Well, if at all such requirement would better be expressed using ASSERT in
get_xen_paddr(). The reason is ...

> 
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
> v2 changes:
>  - dropped part of the v1 code, now this one is simpler, I will
>    discuss better how to design a common boot flow for MPU and
>    implement on another patch.
> 
> ---
> ---
>  xen/arch/arm/Kconfig | 2 +-
>  xen/arch/arm/setup.c | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index a26d3e11827c..ffdff1f0a36c 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -8,7 +8,6 @@ config ARM_64
>         depends on !ARM_32
>         select 64BIT
>         select HAS_FAST_MULTIPLY
> -       select HAS_LLC_COLORING if !NUMA
> 
>  config ARM
>         def_bool y
> @@ -76,6 +75,7 @@ choice
> 
>  config MMU
>         bool "MMU"
> +       select HAS_LLC_COLORING if !NUMA && ARM_64
>         select HAS_PMAP
>         select HAS_VMAP
>         select HAS_PASSTHROUGH
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index c1f2d1b89d43..91fa579e73e5 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -328,6 +328,7 @@ void asmlinkage __init start_xen(unsigned long fdt_paddr)
>                               (paddr_t)(uintptr_t)(_end - _start), false);
>      BUG_ON(!xen_bootmodule);
> 
> +    /* This parses memory banks needed for LLC coloring */
this comment is confusing. It reads as if boot_fdt_info was here only for LLC
coloring. Moreover, if you add such comment here, why not adding a comment above
boot_fdt_cmdline and cmdline_parse which are hard dependency for LLC coloring
code to read LLC cmdline options parsed by llc_coloring_init?

~Michal