Re: [XEN PATCH v3 3/3] xen/mm: add declaration for first_valid_mfn

[Julien Grall <julien@xxxxxxx>]

Hi  George,

On 14/12/2023 14:15, George Dunlap wrote:
> But I do think that it's fair to ask Julien to think about a suitable
> wording, since the comment is in a sense to remind him (or other ARM
> maintainers) what's needed, and since the eventual solution will be
> something to do with the ARM code and architecture anyway.
The comment is for anyone using !NUMA (i.e. all architectures but x86) 
:). What about the following (this is Nicola's patch with the comments 
reworked):
diff --git a/xen/arch/arm/include/asm/numa.h 
b/xen/arch/arm/include/asm/numa.h
index e2bee2bd8223..4bf7c304ea3c 100644
--- a/xen/arch/arm/include/asm/numa.h
+++ b/xen/arch/arm/include/asm/numa.h
@@ -2,8 +2,9 @@
 #define __ARCH_ARM_NUMA_H

 #include <xen/mm.h>
+#include <xen/types.h>

-typedef u8 nodeid_t;
+typedef uint8_t nodeid_t;

 #ifndef CONFIG_NUMA

@@ -11,12 +12,6 @@ typedef u8 nodeid_t;
 #define cpu_to_node(cpu) 0
 #define node_to_cpumask(node)   (cpu_online_map)

-/*
- * TODO: make first_valid_mfn static when NUMA is supported on Arm, this
- * is required because the dummy helpers are using it.
- */
-extern mfn_t first_valid_mfn;
-
 /* XXX: implement NUMA support */
 #define node_spanned_pages(nid) (max_page - mfn_x(first_valid_mfn))
 #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
diff --git a/xen/arch/ppc/include/asm/numa.h 
b/xen/arch/ppc/include/asm/numa.h
index 7fdf66c3da74..888de2dbd1eb 100644
--- a/xen/arch/ppc/include/asm/numa.h
+++ b/xen/arch/ppc/include/asm/numa.h
@@ -1,8 +1,8 @@
 #ifndef __ASM_PPC_NUMA_H__
 #define __ASM_PPC_NUMA_H__

-#include <xen/types.h>
 #include <xen/mm.h>
+#include <xen/types.h>

 typedef uint8_t nodeid_t;

@@ -10,12 +10,6 @@ typedef uint8_t nodeid_t;
 #define cpu_to_node(cpu) 0
 #define node_to_cpumask(node)   (cpu_online_map)

-/*
- * TODO: make first_valid_mfn static when NUMA is supported on PPC, this
- * is required because the dummy helpers are using it.
- */
-extern mfn_t first_valid_mfn;
-
 /* XXX: implement NUMA support */
 #define node_spanned_pages(nid) (max_page - mfn_x(first_valid_mfn))
 #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 9b5df74fddab..d874525916ea 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -255,8 +255,10 @@ static PAGE_LIST_HEAD(page_broken_list);
  */

 /*
- * first_valid_mfn is exported because it is use in ARM specific NUMA
- * helpers. See comment in arch/arm/include/asm/numa.h.
+ * first_valid_mfn is exported because it is used when !CONFIG_NUMA.
+ *
+ * TODO: Consider if we can conditionally export first_valid_mfn based
+ * on whether NUMA is selected.
  */
 mfn_t first_valid_mfn = INVALID_MFN_INITIALIZER;

diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 3d9b2d05a5c8..a13a9a46ced7 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -118,6 +118,8 @@ int destroy_xen_mappings(unsigned long s, unsigned 
long e);
 /* Retrieve the MFN mapped by VA in Xen virtual address space. */
 mfn_t xen_map_to_mfn(unsigned long va);

+extern mfn_t first_valid_mfn;
+
 /*
  * Create only non-leaf page table entries for the
  * page range in Xen virtual address space.

Cheers,

--
Julien Grall