diff mbox series

[v16,10/11] of: fdt: Add memory for devices by DT property "linux,usable-memory-range"

Message ID 20211123124646.1995-11-thunder.leizhen@huawei.com
State Changes Requested, archived
Headers show
Series support reserving crashkernel above 4G on arm64 kdump | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Leizhen (ThunderTown) Nov. 23, 2021, 12:46 p.m. UTC 
From: Chen Zhou <chenzhou10@huawei.com>

When reserving crashkernel in high memory, some low memory is reserved
for crash dump kernel devices and never mapped by the first kernel.
This memory range is advertised to crash dump kernel via DT property
under /chosen,
        linux,usable-memory-range = <BASE1 SIZE1 [BASE2 SIZE2]>

We reused the DT property linux,usable-memory-range and made the low
memory region as the second range "BASE2 SIZE2", which keeps compatibility
with existing user-space and older kdump kernels.

Crash dump kernel reads this property at boot time and call memblock_add()
to add the low memory region after memblock_cap_memory_range() has been
called.

Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/of/fdt.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

Comments

Rob Herring (Arm) Nov. 30, 2021, 10:16 p.m. UTC | #1 
On Tue, Nov 23, 2021 at 08:46:45PM +0800, Zhen Lei wrote:
> From: Chen Zhou <chenzhou10@huawei.com>
> 
> When reserving crashkernel in high memory, some low memory is reserved
> for crash dump kernel devices and never mapped by the first kernel.
> This memory range is advertised to crash dump kernel via DT property
> under /chosen,
>         linux,usable-memory-range = <BASE1 SIZE1 [BASE2 SIZE2]>
> 
> We reused the DT property linux,usable-memory-range and made the low
> memory region as the second range "BASE2 SIZE2", which keeps compatibility
> with existing user-space and older kdump kernels.
> 
> Crash dump kernel reads this property at boot time and call memblock_add()
> to add the low memory region after memblock_cap_memory_range() has been
> called.
> 
> Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/of/fdt.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 37b477a51175359..1ea2a0b1657e3a9 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -967,6 +967,15 @@ static void __init early_init_dt_check_for_elfcorehdr(unsigned long node)
>  
>  static unsigned long chosen_node_offset = -FDT_ERR_NOTFOUND;
>  
> +/*
> + * The main usage of linux,usable-memory-range is for crash dump kernel.
> + * Originally, the number of usable-memory regions is one. Now there may
> + * be two regions, low region and high region.
> + * To make compatibility with existing user-space and older kdump, the low
> + * region is always the last range of linux,usable-memory-range if exist.
> + */
> +#define MAX_USABLE_RANGES		2
> +
>  /**
>   * early_init_dt_check_for_usable_mem_range - Decode usable memory range
>   * location from flat tree
> @@ -974,10 +983,9 @@ static unsigned long chosen_node_offset = -FDT_ERR_NOTFOUND;
>   */
>  static void __init early_init_dt_check_for_usable_mem_range(unsigned long node)
>  {
> -	const __be32 *prop;
> -	int len;
> -	phys_addr_t cap_mem_addr;
> -	phys_addr_t cap_mem_size;
> +	struct memblock_region rgn[MAX_USABLE_RANGES] = {0};
> +	const __be32 *prop, *endp;
> +	int len, i = 0;
>  
>  	if ((long)node < 0)
>  		return;
> @@ -985,16 +993,24 @@ static void __init early_init_dt_check_for_usable_mem_range(unsigned long node)
>  	pr_debug("Looking for usable-memory-range property... ");
>  
>  	prop = of_get_flat_dt_prop(node, "linux,usable-memory-range", &len);
> -	if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
> +	if (!prop)

if (!prop || (len % (dt_root_addr_cells + dt_root_size_cells)))

>  		return;
>  
> -	cap_mem_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
> -	cap_mem_size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +	endp = prop + (len / sizeof(__be32));
> +	while ((endp - prop) >= (dt_root_addr_cells + dt_root_size_cells)) {

for (i = 0; i < MAX_USABLE_RANGES, prop < endp; i++) {

> +		rgn[i].base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> +		rgn[i].size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> +		pr_debug("cap_mem_regions[%d]: base=%pa, size=%pa\n",
> +			 i, &rgn[i].base, &rgn[i].size);
>  
> -	pr_debug("cap_mem_start=%pa cap_mem_size=%pa\n", &cap_mem_addr,
> -		 &cap_mem_size);
> +		if (++i >= MAX_USABLE_RANGES)
> +			break;

And drop this if.

> +	}
>  
> -	memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
> +	memblock_cap_memory_range(rgn[0].base, rgn[0].size);
> +	for (i = 1; i < MAX_USABLE_RANGES && rgn[i].size; i++)

s/ &&/,/

> +		memblock_add(rgn[i].base, rgn[i].size);
>  }
>  
>  #ifdef CONFIG_SERIAL_EARLYCON
> -- 
> 2.25.1
> 
>
Leizhen (ThunderTown) Dec. 1, 2021, 2:55 a.m. UTC | #2 
On 2021/12/1 6:16, Rob Herring wrote:
> On Tue, Nov 23, 2021 at 08:46:45PM +0800, Zhen Lei wrote:
>> From: Chen Zhou <chenzhou10@huawei.com>
>>
>> When reserving crashkernel in high memory, some low memory is reserved
>> for crash dump kernel devices and never mapped by the first kernel.
>> This memory range is advertised to crash dump kernel via DT property
>> under /chosen,
>>         linux,usable-memory-range = <BASE1 SIZE1 [BASE2 SIZE2]>
>>
>> We reused the DT property linux,usable-memory-range and made the low
>> memory region as the second range "BASE2 SIZE2", which keeps compatibility
>> with existing user-space and older kdump kernels.
>>
>> Crash dump kernel reads this property at boot time and call memblock_add()
>> to add the low memory region after memblock_cap_memory_range() has been
>> called.
>>
>> Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  drivers/of/fdt.c | 36 ++++++++++++++++++++++++++----------
>>  1 file changed, 26 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 37b477a51175359..1ea2a0b1657e3a9 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -967,6 +967,15 @@ static void __init early_init_dt_check_for_elfcorehdr(unsigned long node)
>>  
>>  static unsigned long chosen_node_offset = -FDT_ERR_NOTFOUND;
>>  
>> +/*
>> + * The main usage of linux,usable-memory-range is for crash dump kernel.
>> + * Originally, the number of usable-memory regions is one. Now there may
>> + * be two regions, low region and high region.
>> + * To make compatibility with existing user-space and older kdump, the low
>> + * region is always the last range of linux,usable-memory-range if exist.
>> + */
>> +#define MAX_USABLE_RANGES		2
>> +
>>  /**
>>   * early_init_dt_check_for_usable_mem_range - Decode usable memory range
>>   * location from flat tree
>> @@ -974,10 +983,9 @@ static unsigned long chosen_node_offset = -FDT_ERR_NOTFOUND;
>>   */
>>  static void __init early_init_dt_check_for_usable_mem_range(unsigned long node)
>>  {
>> -	const __be32 *prop;
>> -	int len;
>> -	phys_addr_t cap_mem_addr;
>> -	phys_addr_t cap_mem_size;
>> +	struct memblock_region rgn[MAX_USABLE_RANGES] = {0};
>> +	const __be32 *prop, *endp;
>> +	int len, i = 0;
>>  
>>  	if ((long)node < 0)
>>  		return;
>> @@ -985,16 +993,24 @@ static void __init early_init_dt_check_for_usable_mem_range(unsigned long node)
>>  	pr_debug("Looking for usable-memory-range property... ");
>>  
>>  	prop = of_get_flat_dt_prop(node, "linux,usable-memory-range", &len);
>> -	if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
>> +	if (!prop)
> 
> if (!prop || (len % (dt_root_addr_cells + dt_root_size_cells)))

OK.

> 
>>  		return;
>>  
>> -	cap_mem_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
>> -	cap_mem_size = dt_mem_next_cell(dt_root_size_cells, &prop);
>> +	endp = prop + (len / sizeof(__be32));
>> +	while ((endp - prop) >= (dt_root_addr_cells + dt_root_size_cells)) {
> 
> for (i = 0; i < MAX_USABLE_RANGES, prop < endp; i++) {
> 
>> +		rgn[i].base = dt_mem_next_cell(dt_root_addr_cells, &prop);
>> +		rgn[i].size = dt_mem_next_cell(dt_root_size_cells, &prop);
>> +
>> +		pr_debug("cap_mem_regions[%d]: base=%pa, size=%pa\n",
>> +			 i, &rgn[i].base, &rgn[i].size);
>>  
>> -	pr_debug("cap_mem_start=%pa cap_mem_size=%pa\n", &cap_mem_addr,
>> -		 &cap_mem_size);
>> +		if (++i >= MAX_USABLE_RANGES)
>> +			break;
> 
> And drop this if.

OK.

> 
>> +	}
>>  
>> -	memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
>> +	memblock_cap_memory_range(rgn[0].base, rgn[0].size);
>> +	for (i = 1; i < MAX_USABLE_RANGES && rgn[i].size; i++)
> 
> s/ &&/,/

Hi Rob:

The comma operator may not be suitable for logical judgment. The logical judgment
before commas (,) is ignored.

Here's my test：

C code：
int main()
{
        int i, j;

        printf("&&:\n");
        for (i = 0, j = 0; i < 2 && j < 3; i++, j++)
                printf("i=%d, j=%d\n", i, j);

        printf("\ncomma:\n");
        for (i = 0, j = 0; i < 2, j < 3; i++, j++)	//(i < 2） before comma is ignored
                printf("i=%d, j=%d\n", i, j);

        return 0;
}

Output：
&&:
i=0, j=0
i=1, j=1

comma:
i=0, j=0
i=1, j=1
i=2, j=2


> 
>> +		memblock_add(rgn[i].base, rgn[i].size);
>>  }
>>  
>>  #ifdef CONFIG_SERIAL_EARLYCON
>> -- 
>> 2.25.1
>>
>>
> 
> .
>
Leizhen (ThunderTown) Dec. 9, 2021, 1:59 a.m. UTC | #3 
On 2021/12/1 10:55, Leizhen (ThunderTown) wrote:
>>> +	}
>>>  
>>> -	memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
>>> +	memblock_cap_memory_range(rgn[0].base, rgn[0].size);
>>> +	for (i = 1; i < MAX_USABLE_RANGES && rgn[i].size; i++)
>> s/ &&/,/

Hi Rob:
  I want to keep "&&" unchanged, do you mind? I'm going to post an
updated version tomorrow, hopefully the last.

> Hi Rob:
> 
> The comma operator may not be suitable for logical judgment. The logical judgment
> before commas (,) is ignored.
> 
> Here's my test：
> 
> C code：
> int main()
> {
>         int i, j;
> 
>         printf("&&:\n");
>         for (i = 0, j = 0; i < 2 && j < 3; i++, j++)
>                 printf("i=%d, j=%d\n", i, j);
> 
>         printf("\ncomma:\n");
>         for (i = 0, j = 0; i < 2, j < 3; i++, j++)	//(i < 2） before comma is ignored
>                 printf("i=%d, j=%d\n", i, j);
> 
>         return 0;
> }
> 
> Output：
> &&:
> i=0, j=0
> i=1, j=1
> 
> comma:
> i=0, j=0
> i=1, j=1
> i=2, j=2
> 
>
Rob Herring (Arm) Dec. 9, 2021, 5:40 p.m. UTC | #4 
On Wed, Dec 8, 2021 at 7:59 PM Leizhen (ThunderTown)
<thunder.leizhen@huawei.com> wrote:
>
>
>
> On 2021/12/1 10:55, Leizhen (ThunderTown) wrote:
> >>> +   }
> >>>
> >>> -   memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
> >>> +   memblock_cap_memory_range(rgn[0].base, rgn[0].size);
> >>> +   for (i = 1; i < MAX_USABLE_RANGES && rgn[i].size; i++)
> >> s/ &&/,/
>
> Hi Rob:
>   I want to keep "&&" unchanged, do you mind? I'm going to post an
> updated version tomorrow, hopefully the last.

Looks like that was obviously correct, so no objections.

Rob

>
> > Hi Rob:
> >
> > The comma operator may not be suitable for logical judgment. The logical judgment
> > before commas (,) is ignored.
> >
> > Here's my test：
> >
> > C code：
> > int main()
> > {
> >         int i, j;
> >
> >         printf("&&:\n");
> >         for (i = 0, j = 0; i < 2 && j < 3; i++, j++)
> >                 printf("i=%d, j=%d\n", i, j);
> >
> >         printf("\ncomma:\n");
> >         for (i = 0, j = 0; i < 2, j < 3; i++, j++)    //(i < 2） before comma is ignored
> >                 printf("i=%d, j=%d\n", i, j);
> >
> >         return 0;
> > }
> >
> > Output：
> > &&:
> > i=0, j=0
> > i=1, j=1
> >
> > comma:
> > i=0, j=0
> > i=1, j=1
> > i=2, j=2
> >
> >
diff mbox series

Patch

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 37b477a51175359..1ea2a0b1657e3a9 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -967,6 +967,15 @@  static void __init early_init_dt_check_for_elfcorehdr(unsigned long node)
 
 static unsigned long chosen_node_offset = -FDT_ERR_NOTFOUND;
 
+/*
+ * The main usage of linux,usable-memory-range is for crash dump kernel.
+ * Originally, the number of usable-memory regions is one. Now there may
+ * be two regions, low region and high region.
+ * To make compatibility with existing user-space and older kdump, the low
+ * region is always the last range of linux,usable-memory-range if exist.
+ */
+#define MAX_USABLE_RANGES		2
+
 /**
  * early_init_dt_check_for_usable_mem_range - Decode usable memory range
  * location from flat tree
@@ -974,10 +983,9 @@  static unsigned long chosen_node_offset = -FDT_ERR_NOTFOUND;
  */
 static void __init early_init_dt_check_for_usable_mem_range(unsigned long node)
 {
-	const __be32 *prop;
-	int len;
-	phys_addr_t cap_mem_addr;
-	phys_addr_t cap_mem_size;
+	struct memblock_region rgn[MAX_USABLE_RANGES] = {0};
+	const __be32 *prop, *endp;
+	int len, i = 0;
 
 	if ((long)node < 0)
 		return;
@@ -985,16 +993,24 @@  static void __init early_init_dt_check_for_usable_mem_range(unsigned long node)
 	pr_debug("Looking for usable-memory-range property... ");
 
 	prop = of_get_flat_dt_prop(node, "linux,usable-memory-range", &len);
-	if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
+	if (!prop)
 		return;
 
-	cap_mem_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
-	cap_mem_size = dt_mem_next_cell(dt_root_size_cells, &prop);
+	endp = prop + (len / sizeof(__be32));
+	while ((endp - prop) >= (dt_root_addr_cells + dt_root_size_cells)) {
+		rgn[i].base = dt_mem_next_cell(dt_root_addr_cells, &prop);
+		rgn[i].size = dt_mem_next_cell(dt_root_size_cells, &prop);
+
+		pr_debug("cap_mem_regions[%d]: base=%pa, size=%pa\n",
+			 i, &rgn[i].base, &rgn[i].size);
 
-	pr_debug("cap_mem_start=%pa cap_mem_size=%pa\n", &cap_mem_addr,
-		 &cap_mem_size);
+		if (++i >= MAX_USABLE_RANGES)
+			break;
+	}
 
-	memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
+	memblock_cap_memory_range(rgn[0].base, rgn[0].size);
+	for (i = 1; i < MAX_USABLE_RANGES && rgn[i].size; i++)
+		memblock_add(rgn[i].base, rgn[i].size);
 }
 
 #ifdef CONFIG_SERIAL_EARLYCON