Hi Julien,
> -----Original Message-----
> From: Julien Grall <jul...@xen.org>
> Sent: 2021年8月26日 16:22
> To: Wei Chen <wei.c...@arm.com>; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis <bertrand.marq...@arm.com>
> Subject: Re: [XEN RFC PATCH 23/40] xen/arm: introduce a helper to parse
> device tree memory node
>
>
>
> On 26/08/2021 07:35, Wei Chen wrote:
> > Hi Julien,
>
> Hi Wei,
>
> >> -----Original Message-----
> >> From: Julien Grall <jul...@xen.org>
> >> Sent: 2021年8月25日 21:49
> >> To: Wei Chen <wei.c...@arm.com>; xen-devel@lists.xenproject.org;
> >> sstabell...@kernel.org; jbeul...@suse.com
> >> Cc: Bertrand Marquis <bertrand.marq...@arm.com>
> >> Subject: Re: [XEN RFC PATCH 23/40] xen/arm: introduce a helper to parse
> >> device tree memory node
> >>
> >> Hi Wei,
> >>
> >> On 11/08/2021 11:24, Wei Chen wrote:
> >>> Memory blocks' NUMA ID information is stored in device tree's
> >>> memory nodes as "numa-node-id". We need a new helper to parse
> >>> and verify this ID from memory nodes.
> >>>
> >>> In order to support memory affinity in later use, the valid
> >>> memory ranges and NUMA ID will be saved to tables.
> >>>
> >>> Signed-off-by: Wei Chen <wei.c...@arm.com>
> >>> ---
> >>> xen/arch/arm/numa_device_tree.c | 130
> ++++++++++++++++++++++++++++++++
> >>> 1 file changed, 130 insertions(+)
> >>>
> >>> diff --git a/xen/arch/arm/numa_device_tree.c
> >> b/xen/arch/arm/numa_device_tree.c
> >>> index 37cc56acf3..bbe081dcd1 100644
> >>> --- a/xen/arch/arm/numa_device_tree.c
> >>> +++ b/xen/arch/arm/numa_device_tree.c
> >>> @@ -20,11 +20,13 @@
> >>> #include <xen/init.h>
> >>> #include <xen/nodemask.h>
> >>> #include <xen/numa.h>
> >>> +#include <xen/libfdt/libfdt.h>
> >>> #include <xen/device_tree.h>
> >>> #include <asm/setup.h>
> >>>
> >>> s8 device_tree_numa = 0;
> >>> static nodemask_t processor_nodes_parsed __initdata;
> >>> +static nodemask_t memory_nodes_parsed __initdata;
> >>>
> >>> static int srat_disabled(void)
> >>> {
> >>> @@ -55,6 +57,79 @@ static int __init
> >> dtb_numa_processor_affinity_init(nodeid_t node)
> >>> return 0;
> >>> }
> >>>
> >>> +/* Callback for parsing of the memory regions affinity */
> >>> +static int __init dtb_numa_memory_affinity_init(nodeid_t node,
> >>> + paddr_t start, paddr_t size)
> >>> +{
> >>
> >> The implementation of this function is quite similar ot the ACPI
> >> version. Can this be abstracted?
> >
> > In my draft, I had tried to merge ACPI and DTB versions in one
> > function. I introduced a number of "if else" to distinguish ACPI
> > from DTB, especially ACPI hotplug. The function seems very messy.
> > Not enough benefits to make up for the mess, so I gave up.
>
> It think you can get away from distinguishing between ACPI and DT in
> that helper:
> * ma->flags & ACPI_SRAT_MEM_HOTPLUGGABLE could be replace by an
> argument indicating whether the region is hotpluggable (this would
> always be false for DT)
> * Access to memblk_hotplug can be stubbed (in the future we may want
> to consider memory hotplug even on Arm).
>
> Do you still have the "if else" version? If so can you post it?
>
I just tried to do that in draft process, because I was not satisfied
with the changes, I haven't saved them as a patch.
I think your suggestions are worth to try again, I will do it
in next version.
> Cheers,
>
> --
> Julien Grall
