2019-08-30 21:49:54

by Masayoshi Mizuma

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

From: Masayoshi Mizuma <[email protected]>

Get the max address from SRAT and write it into boot_params->max_addr.

Signed-off-by: Masayoshi Mizuma <[email protected]>
---
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;
+ } 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


2019-09-05 14:16:03

by Baoquan He

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

On 08/30/19 at 05:47pm, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma <[email protected]>
>
> Get the max address from SRAT and write it into boot_params->max_addr.
>
> Signed-off-by: Masayoshi Mizuma <[email protected]>
> ---
> 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.

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
>

2019-10-29 15:58:15

by Masayoshi Mizuma

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

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 <[email protected]>
> >
> > Get the max address from SRAT and write it into boot_params->max_addr.
> >
> > Signed-off-by: Masayoshi Mizuma <[email protected]>
> > ---
> > 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
> >