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

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Sent: 2021年8月27日 7:25
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx;
> jbeulich@xxxxxxxx; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [XEN RFC PATCH 18/40] xen/arm: Keep memory nodes in dtb for
> NUMA when boot from EFI
> 
> 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?
> 
> I wonder if we should just use device tree for memory and ignore EFI
> instead. Do you know what Linux does in this regard?

We don't use device tree for memory map on EFI boot. We just reply on
device tree to provide memory NUMA node info. Because EFI system table
doesn't contain this kind of data.

I have a quick look into Linux. Linux efi stub has a update_fdt function in:
drivers/firmware/efi/libstub/fdt.c. In this function, efi stub only delete
reserve memory nodes. Usable memory nodes will not be touched in this function.

Before Linux efi_init, early_init_dt_scan will be invoked. This means, if the
efi stub doesn't remove the normal memory nodes, these nodes will be found and
added to memblock by early_init_dt_add_memory_arch();

Later, in efi_init, sytemtable->memory_description will also be parsed,
the memory block will be added to memblock by early_init_dt_add_memory_arch.
So the duplicated memory nodes will be merged/ignored in memblock_add.

In Linux NUMA init, if ACPI is off, the NUMA info will be parsed from DT.

So I think in EFI boot, we use system table to parse memory, but use
DT to parse NUMA info. It doesn't seem particularly strange : )

Cheers,
Wei Chen