RE: [XEN RFC PATCH 28/40] xen/x86: decouple nodes_cover_memory with E820 map

2021-08-31 Thread Wei Chen
Hi Stefano,

> -Original Message-
> From: Stefano Stabellini 
> Sent: 2021年8月31日 9:08
> To: Wei Chen 
> Cc: xen-devel@lists.xenproject.org; sstabell...@kernel.org; jul...@xen.org;
> jbeul...@suse.com; Bertrand Marquis 
> Subject: Re: [XEN RFC PATCH 28/40] xen/x86: decouple nodes_cover_memory
> with E820 map
> 
> On Wed, 11 Aug 2021, Wei Chen wrote:
> > We will reuse nodes_cover_memory for Arm to check its bootmem
> > info. So we introduce two arch helpers to get memory map's
> > entry number and specified entry's range:
> > arch_get_memory_bank_number
> > arch_get_memory_bank_range
> >
> > Depends above two helpers, we make nodes_cover_memory become
> > architecture independent.
> 
> You might want to note that the only change from an x86 perspective is
> the additional checks:
> 
>   !start || !end
> 

Thanks, I will add it.

> 
> > Signed-off-by: Wei Chen 
> > ---
> >  xen/arch/x86/numa.c| 18 ++
> >  xen/arch/x86/srat.c|  8 +++-
> >  xen/include/xen/numa.h |  4 
> >  3 files changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
> > index 6908738305..8b43be4aa7 100644
> > --- a/xen/arch/x86/numa.c
> > +++ b/xen/arch/x86/numa.c
> > @@ -128,6 +128,24 @@ unsigned int __init arch_get_dma_bitsize(void)
> >   + PAGE_SHIFT, 32);
> >  }
> >
> > +uint32_t __init arch_meminfo_get_nr_bank(void)
> > +{
> > +   return e820.nr_map;
> > +}
> > +
> > +int __init arch_meminfo_get_ram_bank_range(int bank,
> > +   unsigned long long *start, unsigned long long *end)
> > +{
> > +   if (e820.map[bank].type != E820_RAM || !start || !end) {
> > +   return -1;
> > +   }
> > +
> > +   *start = e820.map[bank].addr;
> > +   *end = e820.map[bank].addr + e820.map[bank].size;
> > +
> > +   return 0;
> > +}
> > +
> >  static void dump_numa(unsigned char key)
> >  {
> >  s_time_t now = NOW();
> > diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
> > index 6d68b8a614..2298353846 100644
> > --- a/xen/arch/x86/srat.c
> > +++ b/xen/arch/x86/srat.c
> > @@ -316,18 +316,16 @@ acpi_numa_memory_affinity_init(const struct
> acpi_srat_mem_affinity *ma)
> >  static int __init nodes_cover_memory(void)
> >  {
> > int i;
> > +   uint32_t nr_banks = arch_meminfo_get_nr_bank();
> >
> > -   for (i = 0; i < e820.nr_map; i++) {
> > +   for (i = 0; i < nr_banks; i++) {
> > int j, found;
> > unsigned long long start, end;
> >
> > -   if (e820.map[i].type != E820_RAM) {
> > +   if (arch_meminfo_get_ram_bank_range(i, , )) {
> > continue;
> > }
> >
> > -   start = e820.map[i].addr;
> > -   end = e820.map[i].addr + e820.map[i].size;
> > -
> > do {
> > found = 0;
> > for_each_node_mask(j, memory_nodes_parsed)
> > diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
> > index 0475823b13..6d18059bcd 100644
> > --- a/xen/include/xen/numa.h
> > +++ b/xen/include/xen/numa.h
> > @@ -89,6 +89,10 @@ static inline void clear_node_cpumask(int cpu)
> > cpumask_clear_cpu(cpu, _to_cpumask[cpu_to_node(cpu)]);
> >  }
> >
> > +extern uint32_t arch_meminfo_get_nr_bank(void);
> > +extern int arch_meminfo_get_ram_bank_range(int bank,
> > +unsigned long long *start, unsigned long long *end);
> > +
> >  #endif /* CONFIG_NUMA */
> >
> >  #endif /* _XEN_NUMA_H */
> > --
> > 2.25.1
> >


Re: [XEN RFC PATCH 28/40] xen/x86: decouple nodes_cover_memory with E820 map

2021-08-30 Thread Stefano Stabellini
On Wed, 11 Aug 2021, Wei Chen wrote:
> We will reuse nodes_cover_memory for Arm to check its bootmem
> info. So we introduce two arch helpers to get memory map's
> entry number and specified entry's range:
> arch_get_memory_bank_number
> arch_get_memory_bank_range
> 
> Depends above two helpers, we make nodes_cover_memory become
> architecture independent.

You might want to note that the only change from an x86 perspective is
the additional checks:

  !start || !end


> Signed-off-by: Wei Chen 
> ---
>  xen/arch/x86/numa.c| 18 ++
>  xen/arch/x86/srat.c|  8 +++-
>  xen/include/xen/numa.h |  4 
>  3 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
> index 6908738305..8b43be4aa7 100644
> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -128,6 +128,24 @@ unsigned int __init arch_get_dma_bitsize(void)
>   + PAGE_SHIFT, 32);
>  }
>  
> +uint32_t __init arch_meminfo_get_nr_bank(void)
> +{
> + return e820.nr_map;
> +}
> +
> +int __init arch_meminfo_get_ram_bank_range(int bank,
> + unsigned long long *start, unsigned long long *end)
> +{
> + if (e820.map[bank].type != E820_RAM || !start || !end) {
> + return -1;
> + }
> +
> + *start = e820.map[bank].addr;
> + *end = e820.map[bank].addr + e820.map[bank].size;
> +
> + return 0;
> +}
> +
>  static void dump_numa(unsigned char key)
>  {
>  s_time_t now = NOW();
> diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
> index 6d68b8a614..2298353846 100644
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -316,18 +316,16 @@ acpi_numa_memory_affinity_init(const struct 
> acpi_srat_mem_affinity *ma)
>  static int __init nodes_cover_memory(void)
>  {
>   int i;
> + uint32_t nr_banks = arch_meminfo_get_nr_bank();
>  
> - for (i = 0; i < e820.nr_map; i++) {
> + for (i = 0; i < nr_banks; i++) {
>   int j, found;
>   unsigned long long start, end;
>  
> - if (e820.map[i].type != E820_RAM) {
> + if (arch_meminfo_get_ram_bank_range(i, , )) {
>   continue;
>   }
>  
> - start = e820.map[i].addr;
> - end = e820.map[i].addr + e820.map[i].size;
> -
>   do {
>   found = 0;
>   for_each_node_mask(j, memory_nodes_parsed)
> diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
> index 0475823b13..6d18059bcd 100644
> --- a/xen/include/xen/numa.h
> +++ b/xen/include/xen/numa.h
> @@ -89,6 +89,10 @@ static inline void clear_node_cpumask(int cpu)
>   cpumask_clear_cpu(cpu, _to_cpumask[cpu_to_node(cpu)]);
>  }
>  
> +extern uint32_t arch_meminfo_get_nr_bank(void);
> +extern int arch_meminfo_get_ram_bank_range(int bank,
> +unsigned long long *start, unsigned long long *end);
> +
>  #endif /* CONFIG_NUMA */
>  
>  #endif /* _XEN_NUMA_H */
> -- 
> 2.25.1
>