Hi Michal, > On 17 Feb 2025, at 12:55, Orzel, Michal <[email protected]> wrote: > > > > 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().
Ok, you mean asserting that mem ( bootinfo_get_mem() ) is not empty? > The reason is ... > >> >> Signed-off-by: Luca Fancellu <[email protected]> >> --- >> 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? Yeah I get your point, do you think we should just go with the assert or maybe add a comment on top of llc_coloring_init() to say it needs to be called after boot_fdt_info and boot_fdt_cmdline in order to work? Also because the assert in get_xen_paddr (llc-coloring.c) won’t be compiled on a setup not having cache coloring Cheers, Luca > > ~Michal >
