Re: [PATCH v6 5/9] xen/asm-generic: introduce stub header numa.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.12.2023 15:08, Oleksii Kurochko wrote:
> <asm/numa.h> is common through some archs so it is moved
> to asm-generic.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> Acked-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>

I'd like to withdraw my ack here. As said elsewhere (and see the
respective patch), I don't think a generic header is wanted or
needed here (nor in similar other cases). !NUMA logic ought to live
in xen/numa.h.

Jan

> ---
> Changes in V6:
>  - Rebase only.
> ---
> Changes in V5:
>   - Added Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>   - Updated the comment around first_valid_mfn. ( Arm -> GENERIC )
>   - Added Acked-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> ---
> Changes in V4:
>  - Updated guards name: *ARCH_GENERIC* -> *ASM_GENERIC*.
>  - Moved inclusion of xen/mm-frame.h under "#ifndef CONFIG_NUMA".
>  - Added Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>.
> ---
> Changes in V3:
>  - Remove old header inclusion in asm-generic numa.h and include
>    <xen/mm-frame.h> and <xen/stdint.h>
>  - Drop Arm and PPC's numa.h and use asm-generic version instead.
> ---
> Changes in V2:
>       - update the commit message.
>       - change u8 to uint8_t.
>       - add ifnded CONFIG_NUMA.
> ---
>  xen/arch/arm/include/asm/Makefile             |  1 +
>  xen/arch/ppc/include/asm/Makefile             |  1 +
>  xen/arch/ppc/include/asm/numa.h               | 26 -------------------
>  .../asm => include/asm-generic}/numa.h        | 16 +++++++-----
>  4 files changed, 12 insertions(+), 32 deletions(-)
>  delete mode 100644 xen/arch/ppc/include/asm/numa.h
>  rename xen/{arch/arm/include/asm => include/asm-generic}/numa.h (67%)
> 
> diff --git a/xen/arch/arm/include/asm/Makefile 
> b/xen/arch/arm/include/asm/Makefile
> index 8221429c2c..0c855a798a 100644
> --- a/xen/arch/arm/include/asm/Makefile
> +++ b/xen/arch/arm/include/asm/Makefile
> @@ -2,6 +2,7 @@
>  generic-y += altp2m.h
>  generic-y += hardirq.h
>  generic-y += iocap.h
> +generic-y += numa.h
>  generic-y += paging.h
>  generic-y += percpu.h
>  generic-y += random.h
> diff --git a/xen/arch/ppc/include/asm/Makefile 
> b/xen/arch/ppc/include/asm/Makefile
> index a8e848d4d0..f09c5ea8a1 100644
> --- a/xen/arch/ppc/include/asm/Makefile
> +++ b/xen/arch/ppc/include/asm/Makefile
> @@ -4,6 +4,7 @@ generic-y += div64.h
>  generic-y += hardirq.h
>  generic-y += hypercall.h
>  generic-y += iocap.h
> +generic-y += numa.h
>  generic-y += paging.h
>  generic-y += percpu.h
>  generic-y += random.h
> diff --git a/xen/arch/ppc/include/asm/numa.h b/xen/arch/ppc/include/asm/numa.h
> deleted file mode 100644
> index 7fdf66c3da..0000000000
> --- a/xen/arch/ppc/include/asm/numa.h
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -#ifndef __ASM_PPC_NUMA_H__
> -#define __ASM_PPC_NUMA_H__
> -
> -#include <xen/types.h>
> -#include <xen/mm.h>
> -
> -typedef uint8_t nodeid_t;
> -
> -/* Fake one node for now. See also node_online_map. */
> -#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))
> -#define __node_distance(a, b) (20)
> -
> -#define arch_want_default_dmazone() (false)
> -
> -#endif /* __ASM_PPC_NUMA_H__ */
> diff --git a/xen/arch/arm/include/asm/numa.h b/xen/include/asm-generic/numa.h
> similarity index 67%
> rename from xen/arch/arm/include/asm/numa.h
> rename to xen/include/asm-generic/numa.h
> index e2bee2bd82..7f95a77e89 100644
> --- a/xen/arch/arm/include/asm/numa.h
> +++ b/xen/include/asm-generic/numa.h
> @@ -1,18 +1,21 @@
> -#ifndef __ARCH_ARM_NUMA_H
> -#define __ARCH_ARM_NUMA_H
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_GENERIC_NUMA_H
> +#define __ASM_GENERIC_NUMA_H
>  
> -#include <xen/mm.h>
> +#include <xen/stdint.h>
>  
> -typedef u8 nodeid_t;
> +typedef uint8_t nodeid_t;
>  
>  #ifndef CONFIG_NUMA
>  
> +#include <xen/mm-frame.h>
> +
>  /* Fake one node for now. See also node_online_map. */
>  #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
> + * TODO: make first_valid_mfn static when NUMA is supported on GENERIC, this
>   * is required because the dummy helpers are using it.
>   */
>  extern mfn_t first_valid_mfn;
> @@ -26,7 +29,8 @@ extern mfn_t first_valid_mfn;
>  
>  #define arch_want_default_dmazone() (false)
>  
> -#endif /* __ARCH_ARM_NUMA_H */
> +#endif /* __ASM_GENERIC_NUMA_H */
> +
>  /*
>   * Local variables:
>   * mode: C