RE: [PATCH 04/37] xen: introduce an arch helper for default dma zone status

From: Wei Chen <Wei.Chen@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"julien@xen.org" <julien@xen.org>,
	Bertrand Marquis <Bertrand.Marquis@arm.com>
Subject: RE: [PATCH 04/37] xen: introduce an arch helper for default dma zone status
Date: Fri, 24 Sep 2021 01:50:26 +0000	[thread overview]
Message-ID: <DB9PR08MB6857017D740D8408A2C30CB39EA49@DB9PR08MB6857.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2109231652340.17979@sstabellini-ThinkPad-T480s>

Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@kernel.org>
> Sent: 2021年9月24日 7:56
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: xen-devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org;
> Bertrand Marquis <Bertrand.Marquis@arm.com>
> Subject: Re: [PATCH 04/37] xen: introduce an arch helper for default dma
> zone status
> 
> On Thu, 23 Sep 2021, Wei Chen wrote:
> > In current code, when Xen is running in a multiple nodes NUMA
> > system, it will set dma_bitsize in end_boot_allocator to reserve
> > some low address memory for DMA.
> >
> > There are some x86 implications in current implementation. Becuase
>                                     ^ the                    ^Because
> 
> > on x86, memory starts from 0. On a multiple nodes NUMA system, if
> > a single node contains the majority or all of the DMA memory. x86
>                                                               ^,
> 
> > prefer to give out memory from non-local allocations rather than
> > exhausting the DMA memory ranges. Hence x86 use dma_bitsize to set
> > aside some largely arbitrary amount memory for DMA memory ranges.
>                                      ^ of memory
> 
> > The allocations from these memory ranges would happen only after
> > exhausting all other nodes' memory.
> >
> > But the implications are not shared across all architectures. For
> > example, Arm doesn't have these implications. So in this patch, we
> > introduce an arch_have_default_dmazone helper for arch to determine
> > that it need to set dma_bitsize for reserve DMA allocations or not.
>           ^ needs
> 

I will fix above typos in next version.

> >
> > Signed-off-by: Wei Chen <wei.chen@arm.com>
> > ---
> >  xen/arch/x86/numa.c        | 5 +++++
> >  xen/common/page_alloc.c    | 2 +-
> >  xen/include/asm-arm/numa.h | 5 +++++
> >  xen/include/asm-x86/numa.h | 1 +
> >  4 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
> > index ce79ee44ce..1fabbe8281 100644
> > --- a/xen/arch/x86/numa.c
> > +++ b/xen/arch/x86/numa.c
> > @@ -371,6 +371,11 @@ unsigned int __init arch_get_dma_bitsize(void)
> >                   + PAGE_SHIFT, 32);
> >  }
> >
> > +unsigned int arch_have_default_dmazone(void)
> 
> Can this function return bool?
> Also, can it be a static inline?
> 

Yes, bool would be better. I will place a static inline in asm/numa.h.
Because arm will have another static inline implementation.

> 
> > +{
> > +    return ( num_online_nodes() > 1 ) ? 1 : 0;
> > +}
> > +
> >  static void dump_numa(unsigned char key)
> >  {
> >      s_time_t now = NOW();
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> > index 5801358b4b..80916205e5 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -1889,7 +1889,7 @@ void __init end_boot_allocator(void)
> >      }
> >      nr_bootmem_regions = 0;
> >
> > -    if ( !dma_bitsize && (num_online_nodes() > 1) )
> > +    if ( !dma_bitsize && arch_have_default_dmazone() )
> >          dma_bitsize = arch_get_dma_bitsize();
> >
> >      printk("Domain heap initialised");
> > diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> > index 31a6de4e23..9d5739542d 100644
> > --- a/xen/include/asm-arm/numa.h
> > +++ b/xen/include/asm-arm/numa.h
> > @@ -25,6 +25,11 @@ extern mfn_t first_valid_mfn;
> >  #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
> >  #define __node_distance(a, b) (20)
> >
> > +static inline unsigned int arch_have_default_dmazone(void)
> > +{
> > +    return 0;
> > +}
> > +
> >  #endif /* __ARCH_ARM_NUMA_H */
> >  /*
> >   * Local variables:
> > diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
> > index 3cf26c2def..8060cbf3f4 100644
> > --- a/xen/include/asm-x86/numa.h
> > +++ b/xen/include/asm-x86/numa.h
> > @@ -78,5 +78,6 @@ extern int valid_numa_range(u64 start, u64 end,
> nodeid_t node);
> >  void srat_parse_regions(u64 addr);
> >  extern u8 __node_distance(nodeid_t a, nodeid_t b);
> >  unsigned int arch_get_dma_bitsize(void);
> > +unsigned int arch_have_default_dmazone(void);
> >
> >  #endif
> > --
> > 2.25.1
> >