[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Julien Grall <julien@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, "jbeulich@xxxxxxxx" <jbeulich@xxxxxxxx>
  • From: Wei Chen <Wei.Chen@xxxxxxx>
  • Date: Fri, 20 Aug 2021 02:18:18 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=w7ow2CG31+YTKKpEYNAbnCqZcngS2cScXnjBUlDZiQo=; b=mA+xqeWR0fN5hNMBvQnKpLPxHroKcFCuXqAVNdNv9or7t7p559hWg3lTY7diJA9BBt0953l2B6/QyotyNvYoiy913be4eHdsYu1r1p80jDMiwwFbU5QGx7gr/2aixoTWLKg98Aeh0dgwQUtP9zKcBVj67SI0cWJ6pxwUVXCCWqdfOOSQBnE52xqZ3LiIPa+B245xK/4kuF8igIRrWEiDBee5kUc5/i5X5gnc6JJFgk5kIZsN9HeLSoZvjuhruZK03ZCNvLt/Fp5bVMMwS4pkK4lVDRHscRZd4XOrISUg8POvWsxZl/ikIwDLtt5/fmDd734FF1Ej5jTBUUDycVjScw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hDPmmo8A0Zk27Q8IlC7JS4PQKU1teJeW9rbQz0cquBKmHy7lqJ7s5IR7MJtlW/FdckumODCAQesSP1iXXYjTfc1ElZS0zrTGUcLV3gbl7v/y+2xy/ydsD7lo6ot5cL3y3HSRVjRBiIFtVhRPXAWzum3U6PI1eEic//or94ZXG4rbjNGDF4znTy7Xe/Mmp4hNJf1WXIk+ABkxsGfs4BXkiwwYNTYzx/dqB99S2pjvn20Izc6hmYfshf+JCPcMAs7mFLfQOI5XeA60praeJPXeOSi6bTFaqfaUhaiaqoNPvE8hRc+XTGCR9tgscqN8hRVaBTuhz1BcKeGlV5eClfsx+w==
  • Authentication-results-original: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Delivery-date: Fri, 20 Aug 2021 02:19:01 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHXjptJE815hXcKO0qqVfBN7cbO2at7I+qAgACRNuA=
  • Thread-topic: [XEN RFC PATCH 18/40] xen/arm: Keep memory nodes in dtb for NUMA when boot from EFI

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 2021年8月20日 1:35
> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> Cc: 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
> 
> Hi Wei,
> 
> On 11/08/2021 11:24, 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
> 
> s/prvent/prevent/
> 

Ok

> > 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,
> 
> "If the system boot". Although, I would suggest to write:
> 
> "If Xen has been booted via UEFI, the memory banks will already be
> populated. So we should skip the parsing."
> 

Yes, that would be better. I will change it in next version.

> > +     * so we don't need to parse memory node from DTB.
> > +     */
> > +    if ( device_tree_node_matches(fdt, node, "memory") &&
> > +         !arch_efi_enabled(EFI_BOOT) )
> 
> arch_efi_enabled() is going to be less expensive than
> device_tree_node_matches(). So I would suggest to re-order the operands.
> 

yes.

> >           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/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> > index c4b6af6029..e4fb5f0d49 100644
> > --- a/xen/include/asm-arm/setup.h
> > +++ b/xen/include/asm-arm/setup.h
> > @@ -123,6 +123,12 @@ void device_tree_get_reg(const __be32 **cell, u32
> address_cells,
> >   u32 device_tree_get_u32(const void *fdt, int node,
> >                           const char *prop_name, u32 dflt);
> >
> > +#if defined(CONFIG_ARM_64)
> > +#define arch_efi_enabled(x) efi_enabled(x)
> > +#else
> > +#define arch_efi_enabled(x) (0)
> > +#endif
> 
> I would prefer if we introduce CONFIG_EFI that would stub efi_enabled
> for architecture not supporting EFI.
> 

Yes, that's a good idea.
I also feel a little awkward with the current arch helpers.

> Cheers,
> 
> --
> Julien Grall

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.