[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] xen/arm: Move early mapping operations to new function
Hi,
On 12/02/2025 09:19, Luca Fancellu wrote:
Currently start_xen() is performing some early operations using
the boot page tables that are configured during early asm boot,
before setting up the runtime page tables that might also use
the cache coloring feature.
On an MPU system the cache coloring feature is not applicable,
also before using any early mapping function, the MPU data
structure needs to be initialised.
Another thing that is redundant is mapping the DTB twice, since
cache coloring is not applicable.
Because of the above reason, isolate the early mapping code into
a function called 'setup_early_mappings' that is defined under
the MMU module, an MPU system will need to implement such function
to perform the operation needed for early mapping that will be
in part different from an MMU system.
Moved the 'HAS_LLC_COLORING' Kconfig symbol into the MMU one,
selected only when system is Arm64 and not numa.
Now setup_pagetables() is called only in the mmu/setup.c module,
limit its visibility and remove it from mm.h header.
Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
---
xen/arch/arm/Kconfig | 2 +-
xen/arch/arm/include/asm/mm.h | 4 ++--
xen/arch/arm/mmu/setup.c | 35 ++++++++++++++++++++++++++++++++++-
xen/arch/arm/setup.c | 30 +-----------------------------
4 files changed, 38 insertions(+), 33 deletions(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index a26d3e11827c..91f838a32bc6 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 && ARM64
select HAS_PMAP
select HAS_VMAP
select HAS_PASSTHROUGH
diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index a0d8e5afe977..d4b6daa85aa0 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -203,8 +203,8 @@ extern unsigned long frametable_base_pdx;
#define PDX_GROUP_SHIFT SECOND_SHIFT
-/* Boot-time pagetable setup */
-extern void setup_pagetables(void);
+/* Setup early mappings */
+extern void setup_early_mappings(paddr_t fdt_paddr);
/* Map FDT in boot pagetable */
extern void *early_fdt_map(paddr_t fdt_paddr);
/* Remove early mappings */
diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
index 30afe9778194..f7862d0bafd3 100644
--- a/xen/arch/arm/mmu/setup.c
+++ b/xen/arch/arm/mmu/setup.c
@@ -354,7 +354,7 @@ static void __init create_llc_coloring_mappings(void)
* Boot-time pagetable setup.
* Changes here may need matching changes in head.S
*/
-void __init setup_pagetables(void)
+static void __init setup_pagetables(void)
{
uint64_t ttbr;
lpae_t pte, *p;
@@ -469,6 +469,39 @@ void __init setup_pagetables(void)
xen_pt_enforce_wnx();
}
+void __init setup_early_mappings(paddr_t fdt_paddr)
+{
+ const char *cmdline;
+ struct bootmodule *xen_bootmodule;
+
+ device_tree_flattened = early_fdt_map(fdt_paddr);
+ if ( !device_tree_flattened )
+ panic("Invalid device tree blob at physical address %#"PRIpaddr".\n"
+ "The DTB must be 8-byte aligned and must not exceed 2 MB in
size.\n\n"
+ "Please check your bootloader.\n",
+ fdt_paddr);
+
+ /* Register Xen's load address as a boot module. */
+ xen_bootmodule = add_boot_module(BOOTMOD_XEN,
+ virt_to_maddr(_start),
+ (paddr_t)(uintptr_t)(_end - _start), false);
+ BUG_ON(!xen_bootmodule);
Don't you need the code above for the MPU?
+
+ cmdline = boot_fdt_cmdline(device_tree_flattened);
+ printk("Command line: %s\n", cmdline);
+ cmdline_parse(cmdline);
I find confusing this is part of early mappings. Shouldn't this be part
of common code?
+
+ llc_coloring_init();
+
+ /*
+ * Page tables must be setup after LLC coloring initialization because
+ * coloring info are required in order to create colored mappings
+ */
+ setup_pagetables();
+ /* Device-tree was mapped in boot page tables, remap it in the new tables
*/
+ device_tree_flattened = early_fdt_map(fdt_paddr);
+}
+
void *__init arch_vmap_virt_end(void)
{
return (void *)(VMAP_VIRT_START + VMAP_VIRT_SIZE);
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index c1f2d1b89d43..b2f34ba2a873 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -12,7 +12,6 @@
#include <xen/device_tree.h>
#include <xen/domain_page.h>
#include <xen/grant_table.h>
-#include <xen/llc-coloring.h>
#include <xen/types.h>
#include <xen/string.h>
#include <xen/serial.h>
@@ -300,8 +299,6 @@ size_t __read_mostly dcache_line_bytes;
void asmlinkage __init start_xen(unsigned long fdt_paddr)
{
size_t fdt_size;
- const char *cmdline;
- struct bootmodule *xen_bootmodule;
struct domain *d;
int rc, i;
@@ -315,35 +312,10 @@ void asmlinkage __init start_xen(unsigned long fdt_paddr)
smp_clear_cpu_maps();
- device_tree_flattened = early_fdt_map(fdt_paddr);
- if ( !device_tree_flattened )
- panic("Invalid device tree blob at physical address %#lx.\n"
- "The DTB must be 8-byte aligned and must not exceed 2 MB in
size.\n\n"
- "Please check your bootloader.\n",
- fdt_paddr);
I understand why you don't need to call early_fdt_map() twice. But I am
a bit surprised why the two calls are moved to the MMU code. I think it
would be better if one of the call is kept here and early_fdt_map() is
implemented for the MPU.
-
- /* Register Xen's load address as a boot module. */
- xen_bootmodule = add_boot_module(BOOTMOD_XEN,
- virt_to_maddr(_start),
- (paddr_t)(uintptr_t)(_end - _start), false);
- BUG_ON(!xen_bootmodule);
+ setup_early_mappings(fdt_paddr);
fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
This function will parse the memory banks. This is required by ...
- cmdline = boot_fdt_cmdline(device_tree_flattened);
- printk("Command line: %s\n", cmdline);
- cmdline_parse(cmdline);
-
- llc_coloring_init();
... llc_coloring_init(). Yet you move it early. So I think it means the
cache coloring code would stop working.
> -> - /*
- * Page tables must be setup after LLC coloring initialization because
- * coloring info are required in order to create colored mappings
- */
- setup_pagetables();
- /* Device-tree was mapped in boot page tables, remap it in the new tables
*/
- device_tree_flattened = early_fdt_map(fdt_paddr);
-
setup_mm();
vm_init();
Cheers,
--
Julien Grall
|
