On 22.12.2023 14:07, Oleksii wrote: > On Fri, 2023-12-22 at 09:22 +0100, Jan Beulich wrote: >> On 21.12.2023 20:09, Julien Grall wrote: >>> On 20/12/2023 14:08, Oleksii Kurochko wrote: >>>> <asm/numa.h> is common through some archs so it is moved >>>> to asm-generic. >>>> >>>> Signed-off-by: Oleksii Kurochko <[email protected]> >>>> Reviewed-by: Michal Orzel <[email protected]> >>>> Acked-by: Jan Beulich <[email protected]> >>>> Acked-by: Shawn Anastasio <[email protected]> >>> >>> I think this patch will need to be rebased on top of the lastest >>> staging >>> as this should clash with 51ffb3311895. >> >> No, and I'd like to withdraw my ack here. In this case a stub header >> isn't >> the right choice imo - the !NUMA case should be handled in common >> code. I >> would have submitted the patch I have, if only the first_valid_mfn >> patch >> hadn't been committed already (which I now need to re-base over). > I haven't seen your patch yet (waiting for it), but I assume that > <asm/numa.h> will still be necessary and remain the same across Arm, > PPC, and RISC-V. > > What am I missing?
asm/numa.h will be necessary for an arch only if it actually has a way to have CONFIG_NUMA enabled. Below, for your reference, the patch in the not-yet-rebased form. As you can see, Arm's (and PPC's) header goes away. If and when they choose to support NUMA, they may need to regain one; whether at that point we can sort how an asm-generic/ form of the header might sensibly look like remains to be seen. Jan NUMA: no need for asm/numa.h when !NUMA There's no point in every architecture carrying the same stubs for the case when NUMA isn't enabled (or even supported). Move all of that to xen/numa.h; replace explicit uses of asm/numa.h in common code. Make inclusion of asm/numa.h dependent upon NUMA=y. Address the TODO regarding first_valid_mfn by making the variable static when NUMA=y, thus also addressing a Misrs C:2012 rule 8.4 concern. Drop the not really applicable "implement NUMA support" comment - in a !NUMA section this simply makes no sense. Signed-off-by: Jan Beulich <[email protected]> --- This is an alternative proposal to https://lists.xen.org/archives/html/xen-devel/2023-12/msg01586.html and its earlier forms. --- unstable.orig/xen/arch/arm/include/asm/numa.h +++ /dev/null @@ -1,37 +0,0 @@ -#ifndef __ARCH_ARM_NUMA_H -#define __ARCH_ARM_NUMA_H - -#include <xen/mm.h> - -typedef u8 nodeid_t; - -#ifndef CONFIG_NUMA - -/* 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 - * 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) - -#endif - -#define arch_want_default_dmazone() (false) - -#endif /* __ARCH_ARM_NUMA_H */ -/* - * Local variables: - * mode: C - * c-file-style: "BSD" - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ --- unstable.orig/xen/arch/arm/smpboot.c +++ unstable/xen/arch/arm/smpboot.c @@ -42,7 +42,7 @@ integer_param("maxcpus", max_cpus); /* CPU logical map: map xen cpuid to an MPIDR */ register_t __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = MPIDR_INVALID }; -/* Fake one node for now. See also asm/numa.h */ +/* Fake one node for now. See also xen/numa.h */ nodemask_t __read_mostly node_online_map = { { [0] = 1UL } }; /* Xen stack for bringing up the first CPU. */ --- unstable.orig/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__ */ --- unstable.orig/xen/common/page_alloc.c +++ unstable/xen/common/page_alloc.c @@ -140,7 +140,6 @@ #include <xen/vm_event.h> #include <asm/flushtlb.h> -#include <asm/numa.h> #include <asm/page.h> #include <public/sysctl.h> @@ -256,10 +255,14 @@ static PAGE_LIST_HEAD(page_broken_list); * BOOT-TIME ALLOCATOR */ +#ifndef CONFIG_NUMA /* * first_valid_mfn is exported because it is use in ARM specific NUMA * helpers. See comment in arch/arm/include/asm/numa.h. */ +#else +static +#endif mfn_t first_valid_mfn = INVALID_MFN_INITIALIZER; struct bootmem_region { --- unstable.orig/xen/common/sysctl.c +++ unstable/xen/common/sysctl.c @@ -22,8 +22,8 @@ #include <asm/current.h> #include <xen/hypercall.h> #include <public/sysctl.h> -#include <asm/numa.h> #include <xen/nodemask.h> +#include <xen/numa.h> #include <xsm/xsm.h> #include <xen/pmstat.h> #include <xen/livepatch.h> --- unstable.orig/xen/include/xen/domain.h +++ unstable/xen/include/xen/domain.h @@ -2,6 +2,7 @@ #ifndef __XEN_DOMAIN_H__ #define __XEN_DOMAIN_H__ +#include <xen/numa.h> #include <xen/types.h> #include <public/xen.h> @@ -12,7 +13,6 @@ struct guest_area { }; #include <asm/domain.h> -#include <asm/numa.h> typedef union { struct vcpu_guest_context *nat; --- unstable.orig/xen/include/xen/numa.h +++ unstable/xen/include/xen/numa.h @@ -2,7 +2,13 @@ #define _XEN_NUMA_H #include <xen/mm-frame.h> + +#ifdef CONFIG_NUMA +#include <xen/pdx.h> #include <asm/numa.h> +#else +typedef uint8_t nodeid_t; +#endif #define NUMA_NO_NODE 0xFF #define NUMA_NO_DISTANCE 0xFF @@ -108,6 +114,18 @@ extern void numa_set_processor_nodes_par #else +/* Fake one node for now. See also node_online_map. */ +#define cpu_to_node(cpu) 0 +#define node_to_cpumask(node) cpu_online_map + +#define arch_want_default_dmazone() false + +extern mfn_t first_valid_mfn; + +#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 + static inline nodeid_t mfn_to_nid(mfn_t mfn) { return 0; --- unstable.orig/xen/include/xen/pci.h +++ unstable/xen/include/xen/pci.h @@ -11,10 +11,10 @@ #include <xen/list.h> #include <xen/spinlock.h> #include <xen/irq.h> +#include <xen/numa.h> #include <xen/pci_regs.h> #include <xen/pfn.h> #include <asm/device.h> -#include <asm/numa.h> /* * The PCI interface treats multi-function devices as independent
