Re: [XEN RFC PATCH 18/40] xen/arm: Keep memory nodes in dtb for NUMA when boot from EFI

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 27 Aug 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 27/08/2021 00:24, Stefano Stabellini wrote:
> > On Wed, 11 Aug 2021, Wei Chen wrote:
> > > EFI can get memory map from EFI system table. But EFI system
> > > table doesn't contain memory NUMA information, EFI depends on
> > > ACPI SRAT or device tree memory node to parse memory blocks'
> > > NUMA mapping.
> > > 
> > > But in current code, when Xen is booting from EFI, it will
> > > delete all memory nodes in device tree. So in UEFI + DTB
> > > boot, we don't have numa-node-id for memory blocks any more.
> > > 
> > > So in this patch, we will keep memory nodes in device tree for
> > > NUMA code to parse memory numa-node-id later.
> > > 
> > > As a side effect, if we still parse boot memory information in
> > > early_scan_node, bootmem.info will calculate memory ranges in
> > > memory nodes twice. So we have to prvent early_scan_node to
> > > parse memory nodes in EFI boot.
> > > 
> > > As EFI APIs only can be used in Arm64, so we introduced a wrapper
> > > in header file to prevent #ifdef CONFIG_ARM_64/32 in code block.
> > > 
> > > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > > ---
> > >   xen/arch/arm/bootfdt.c      |  8 +++++++-
> > >   xen/arch/arm/efi/efi-boot.h | 25 -------------------------
> > >   xen/include/asm-arm/setup.h |  6 ++++++
> > >   3 files changed, 13 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> > > index 476e32e0f5..7df149dbca 100644
> > > --- a/xen/arch/arm/bootfdt.c
> > > +++ b/xen/arch/arm/bootfdt.c
> > > @@ -11,6 +11,7 @@
> > >   #include <xen/lib.h>
> > >   #include <xen/kernel.h>
> > >   #include <xen/init.h>
> > > +#include <xen/efi.h>
> > >   #include <xen/device_tree.h>
> > >   #include <xen/libfdt/libfdt.h>
> > >   #include <xen/sort.h>
> > > @@ -335,7 +336,12 @@ static int __init early_scan_node(const void *fdt,
> > >   {
> > >       int rc = 0;
> > >   -    if ( device_tree_node_matches(fdt, node, "memory") )
> > > +    /*
> > > +     * If system boot from EFI, bootinfo.mem has been set by EFI,
> > > +     * so we don't need to parse memory node from DTB.
> > > +     */
> > > +    if ( device_tree_node_matches(fdt, node, "memory") &&
> > > +         !arch_efi_enabled(EFI_BOOT) )
> > >           rc = process_memory_node(fdt, node, name, depth,
> > >                                    address_cells, size_cells,
> > > &bootinfo.mem);
> > >       else if ( depth == 1 && !dt_node_cmp(name, "reserved-memory") )
> > 
> > 
> > If we are going to use the device tree info for the numa nodes (and
> > related memory) does it make sense to still rely on the EFI tables for
> > the memory map?
> 
> Yes. AFAIK, when booting using EFI, the Device-Tree may not contain all the
> reserved regions. Furthermore, we are still too early to know whether we boot
> using ACPI and DT.
> 
> > 
> > I wonder if we should just use device tree for memory and ignore EFI
> > instead. Do you know what Linux does in this regard?
> I looked at Linux when I first reviewed this patch because I was wondering
> what happens if the DT and UEFI map disagrees.
> 
> Linux and Xen are the same after this patch:
>   1) The memory map is coming from UEFI map
>   2) NUMA ID is coming from the DT
> 
> The commit that introduced the change in Linux is:
[...]

Thanks both you and Wei for the investigation :-)