> -----Original Message-----
> From: Stefano Stabellini <sstabell...@kernel.org>
> Sent: 2021年9月24日 9:23
> To: Wei Chen <wei.c...@arm.com>
> Cc: xen-devel@lists.xenproject.org; sstabell...@kernel.org; jul...@xen.org;
> Bertrand Marquis <bertrand.marq...@arm.com>
> Subject: Re: [PATCH 21/37] xen/arm: Keep memory nodes in dtb for NUMA when
> boot from EFI
>
> On Thu, 23 Sep 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 prevent early_scan_node to
> > parse memory nodes in EFI boot.
> >
> > As EFI APIs only can be used in Arm64, so we introduced a stub
> > API for non-EFI supported Arm32. This will prevent
>
> This last sentence is incomplete.
>
> But aside from that, this patch looks good to me.
>
Ah, it truncated by accident. I will fix it.
>
> > Signed-off-by: Wei Chen <wei.c...@arm.com>
> > ---
> > xen/arch/arm/bootfdt.c | 8 +++++++-
> > xen/arch/arm/efi/efi-boot.h | 25 -------------------------
> > xen/include/xen/efi.h | 7 +++++++
> > 3 files changed, 14 insertions(+), 26 deletions(-)
> >
> > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> > index afaa0e249b..6bc5a465ec 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>
> > @@ -370,7 +371,12 @@ static int __init early_scan_node(const void *fdt,
> > {
> > int rc = 0;
> >
> > - if ( device_tree_node_matches(fdt, node, "memory") )
> > + /*
> > + * If Xen has been booted via UEFI, the memory banks will already
> > + * be populated. So we should skip the parsing.
> > + */
> > + if ( !efi_enabled(EFI_BOOT) &&
> > + device_tree_node_matches(fdt, node, "memory"))
> > rc = process_memory_node(fdt, node, name, depth,
> > address_cells, size_cells,
> &bootinfo.mem);
> > else if ( depth == 1 && !dt_node_cmp(name, "reserved-memory") )
> > diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> > index cf9c37153f..d0a9987fa4 100644
> > --- a/xen/arch/arm/efi/efi-boot.h
> > +++ b/xen/arch/arm/efi/efi-boot.h
> > @@ -197,33 +197,8 @@ EFI_STATUS __init
> fdt_add_uefi_nodes(EFI_SYSTEM_TABLE *sys_table,
> > int status;
> > u32 fdt_val32;
> > u64 fdt_val64;
> > - int prev;
> > int num_rsv;
> >
> > - /*
> > - * Delete any memory nodes present. The EFI memory map is the only
> > - * memory description provided to Xen.
> > - */
> > - prev = 0;
> > - for (;;)
> > - {
> > - const char *type;
> > - int len;
> > -
> > - node = fdt_next_node(fdt, prev, NULL);
> > - if ( node < 0 )
> > - break;
> > -
> > - type = fdt_getprop(fdt, node, "device_type", &len);
> > - if ( type && strncmp(type, "memory", len) == 0 )
> > - {
> > - fdt_del_node(fdt, node);
> > - continue;
> > - }
> > -
> > - prev = node;
> > - }
> > -
> > /*
> > * Delete all memory reserve map entries. When booting via UEFI,
> > * kernel will use the UEFI memory map to find reserved regions.
> > diff --git a/xen/include/xen/efi.h b/xen/include/xen/efi.h
> > index 661a48286a..b52a4678e9 100644
> > --- a/xen/include/xen/efi.h
> > +++ b/xen/include/xen/efi.h
> > @@ -47,6 +47,13 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *);
> > int efi_compat_get_info(uint32_t idx, union compat_pf_efi_info *);
> > int efi_compat_runtime_call(struct compat_pf_efi_runtime_call *);
> >
> > +#else
> > +
> > +static inline bool efi_enabled(unsigned int feature)
> > +{
> > + return false;
> > +}
> > +
> > #endif /* CONFIG_EFI*/
> >
> > #endif /* !__ASSEMBLY__ */
> > --
> > 2.25.1
> >
