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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 14 Dec 2023, Julien Grall wrote:
> 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):

The below looks good to me.



> 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
>