Hi,

On 13/12/2022 01:41, Stefano Stabellini wrote:
diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
index fdbf68aadcaa..e7a80fecec14 100644
--- a/xen/arch/arm/include/asm/setup.h
+++ b/xen/arch/arm/include/asm/setup.h
@@ -168,6 +168,17 @@ int map_range_to_domain(const struct dt_device_node *dev,
extern const char __ro_after_init_start[], __ro_after_init_end[]; +extern DEFINE_BOOT_PAGE_TABLE(boot_pgtable);
+
+#ifdef CONFIG_ARM_64
+extern DEFINE_BOOT_PAGE_TABLE(boot_first_id);
+#endif
+extern DEFINE_BOOT_PAGE_TABLE(boot_second_id);
+extern DEFINE_BOOT_PAGE_TABLE(boot_third_id);

This is more a matter of taste but I would either:
- define extern all BOOT_PAGE_TABLEs here both ARM64 and ARM32 with
   #ifdefs

A grep of BOOT_PAGE_TABLE shows that they are all defined in setup.h.

- or define all the ARM64 only BOOT_PAGE_TABLE in arm64/mm.h and all the
   ARM32 only BOOT_PAGE_TABLE in arm32/mm.h >
Right now we have a mix, as we have boot_first_id with a #ifdef here
and we have xen_pgtable in arm64/mm.h
We are talking about two distinct set of page-tables. One is used at runtime (i.e. xen_pgtable) and the other are for boot/smp-bring up.

So adding the boot_* in setup.h is correct. As I wrote earlier, setup.h would need a split. But this is not something I really want to handle here...


Also we are missing boot_second and boot_third. We might as well be
consistent and declare them all?

My plan is really to kill boot_second and boot_third. So I don't really want to export them right now (even temporarily).

In any case, I don't think such change belongs in this patch (it is already complex enough).

+/* Find where Xen will be residing at runtime and return an PT entry */
+lpae_t pte_of_xenaddr(vaddr_t);
+
  #endif
  /*
   * Local variables:
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 0cf7ad4f0e8c..39e0d9e03c9c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -93,7 +93,7 @@ DEFINE_BOOT_PAGE_TABLE(boot_third);
#ifdef CONFIG_ARM_64
  #define HYP_PT_ROOT_LEVEL 0
-static DEFINE_PAGE_TABLE(xen_pgtable);
+DEFINE_PAGE_TABLE(xen_pgtable);
  static DEFINE_PAGE_TABLE(xen_first);
  #define THIS_CPU_PGTABLE xen_pgtable
  #else
@@ -388,7 +388,7 @@ void flush_page_to_ram(unsigned long mfn, bool sync_icache)
          invalidate_icache();
  }
-static inline lpae_t pte_of_xenaddr(vaddr_t va)
+lpae_t pte_of_xenaddr(vaddr_t va)
  {
      paddr_t ma = va + phys_offset;
@@ -495,6 +495,8 @@ void __init setup_pagetables(unsigned long boot_phys_offset) phys_offset = boot_phys_offset; + arch_setup_page_tables();
+
  #ifdef CONFIG_ARM_64
      pte = pte_of_xenaddr((uintptr_t)xen_first);
      pte.pt.table = 1;
--
2.38.1


Cheers,

--
Julien Grall

Reply via email to