Re: [PATCH v3 3/5] x86/boot: Get the max address from SRAT

[Masayoshi Mizuma <msys.mizuma@gmail.com>]

On Thu, Sep 05, 2019 at 09:51:34PM +0800, Baoquan He wrote:
> On 08/30/19 at 05:47pm, Masayoshi Mizuma wrote:
> > From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> > 
> > Get the max address from SRAT and write it into boot_params->max_addr.
> > 
> > Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> > ---
> >  arch/x86/boot/compressed/acpi.c | 24 ++++++++++++++++++------
> >  1 file changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
> > index 908a1bfab..ba2bc5ab9 100644
> > --- a/arch/x86/boot/compressed/acpi.c
> > +++ b/arch/x86/boot/compressed/acpi.c
> > @@ -362,16 +362,24 @@ static unsigned long get_acpi_srat_table(void)
> >  	return 0;
> >  }
> >  
> > -static void subtable_parse(struct acpi_subtable_header *sub_table, int *num)
> > +static void subtable_parse(struct acpi_subtable_header *sub_table, int *num,
> > +		unsigned long *max_addr)
> >  {
> >  	struct acpi_srat_mem_affinity *ma;
> > +	unsigned long addr;
> >  
> >  	ma = (struct acpi_srat_mem_affinity *)sub_table;
> >  
> > -	if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && ma->length) {
> > -		immovable_mem[*num].start = ma->base_address;
> > -		immovable_mem[*num].size = ma->length;
> > -		(*num)++;
> > +	if (ma->length) {
> > +		if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) {
> > +			addr = ma->base_address + ma->length;
> > +			if (addr > *max_addr)
> > +				*max_addr = addr;
> 
> Can we return max_addr or only pass out the max_addr, then let the
> max_addr compared and got outside of subtable_parse()? This can keep
> subtable_parse() really only doing parsing work.

Sounds great! I'll change subtable_parse() to return max_addr.

Thanks,
Masa

> 
> Personal opinion, see what maintainers and other reviewers will say.
> 
> Thanks
> Baoquan
> 
> > +		} else {
> > +			immovable_mem[*num].start = ma->base_address;
> > +			immovable_mem[*num].size = ma->length;
> > +			(*num)++;
> > +		}
> >  	}
> >  }
> >  
> > @@ -391,6 +399,7 @@ int count_immovable_mem_regions(void)
> >  	struct acpi_subtable_header *sub_table;
> >  	struct acpi_table_header *table_header;
> >  	char arg[MAX_ACPI_ARG_LENGTH];
> > +	unsigned long max_addr = 0;
> >  	int num = 0;
> >  
> >  	if (cmdline_find_option("acpi", arg, sizeof(arg)) == 3 &&
> > @@ -409,7 +418,7 @@ int count_immovable_mem_regions(void)
> >  		sub_table = (struct acpi_subtable_header *)table;
> >  		if (sub_table->type == ACPI_SRAT_TYPE_MEMORY_AFFINITY) {
> >  
> > -			subtable_parse(sub_table, &num);
> > +			subtable_parse(sub_table, &num, &max_addr);
> >  
> >  			if (num >= MAX_NUMNODES*2) {
> >  				debug_putstr("Too many immovable memory regions, aborting.\n");
> > @@ -418,6 +427,9 @@ int count_immovable_mem_regions(void)
> >  		}
> >  		table += sub_table->length;
> >  	}
> > +
> > +	boot_params->max_addr = max_addr;
> > +
> >  	return num;
> >  }
> >  #endif /* CONFIG_RANDOMIZE_BASE && CONFIG_MEMORY_HOTREMOVE */
> > -- 
> > 2.18.1
> >