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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 11 Dec 2023, Nicola Vetrini wrote:
> Such declaration is needed because a compatible declaration
> is not visible in xen/common/page_alloc.c, where the variable
> is defined. That variable can't yet be static because of the lack of
> support from ARM and PPC for NUMA.
> 
> On the occasion, use drop a use of u8.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
> Having this declaration essentially sidesteps the current impossibility
> of having a static variable, as described in the comments in
> ARM and PCC's asm/numa.h.
> 
> Changes in v3:
> - Drop redundant declarations of first_valid_mfn in asm/numa.h for Arm and 
> PPC.
> ---
>  xen/arch/arm/include/asm/numa.h | 8 ++++----
>  xen/arch/ppc/include/asm/numa.h | 7 +++----
>  xen/include/xen/mm.h            | 2 ++
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/numa.h b/xen/arch/arm/include/asm/numa.h
> index e2bee2bd8223..44b689f67db8 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>

Why the addition of types.h? It doesn't seem to be necessary. Also on
PPC below doesn't seem to be necessary.

Everything else looks fine.



> -typedef u8 nodeid_t;
> +typedef uint8_t nodeid_t;
>  
>  #ifndef CONFIG_NUMA
>  
> @@ -12,10 +13,9 @@ typedef u8 nodeid_t;
>  #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.
> + * TODO: define here first_valid_mfn as 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))
> diff --git a/xen/arch/ppc/include/asm/numa.h b/xen/arch/ppc/include/asm/numa.h
> index 7fdf66c3da74..152305ebe446 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;
>  
> @@ -11,10 +11,9 @@ typedef uint8_t nodeid_t;
>  #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.
> + * TODO: define here first_valid_mfn as 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))
> 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.
> -- 
> 2.34.1
>