lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 21 Oct 2021 09:14:26 +0800
From:   "Leizhen (ThunderTown)" <thunder.leizhen@...wei.com>
To:     Rob Herring <robh@...nel.org>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        <x86@...nel.org>, "H . Peter Anvin" <hpa@...or.com>,
        <linux-kernel@...r.kernel.org>, Dave Young <dyoung@...hat.com>,
        Baoquan He <bhe@...hat.com>, Vivek Goyal <vgoyal@...hat.com>,
        Eric Biederman <ebiederm@...ssion.com>,
        <kexec@...ts.infradead.org>,
        Catalin Marinas <catalin.marinas@....com>,
        "Will Deacon" <will@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        "Frank Rowand" <frowand.list@...il.com>,
        <devicetree@...r.kernel.org>, "Jonathan Corbet" <corbet@....net>,
        <linux-doc@...r.kernel.org>, Randy Dunlap <rdunlap@...radead.org>,
        Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        Feng Zhou <zhoufeng.zf@...edance.com>,
        Kefeng Wang <wangkefeng.wang@...wei.com>
Subject: Re: [PATCH v15 09/10] of: fdt: Add memory for devices by DT property
 "linux,usable-memory-range"



On 2021/10/20 22:19, Rob Herring wrote:
> On Wed, Oct 20, 2021 at 10:03:16AM +0800, Zhen Lei wrote:
>> From: Chen Zhou <chenzhou10@...wei.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@...wei.com>
>> Signed-off-by: Zhen Lei <thunder.leizhen@...wei.com>
>> ---
>>  drivers/of/fdt.c | 47 ++++++++++++++++++++++++++++++++++++-----------
>>  1 file changed, 36 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 4546572af24bbf1..cf59c847b2c28a5 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -969,8 +969,16 @@ static void __init early_init_dt_check_for_elfcorehdr(unsigned long node)
>>  		 elfcorehdr_addr, elfcorehdr_size);
>>  }
>>  
>> -static phys_addr_t cap_mem_addr;
>> -static phys_addr_t cap_mem_size;
>> +/*
>> + * 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
>> +
>> +static struct memblock_region cap_mem_regions[MAX_USABLE_RANGES];
>>  
>>  /**
>>   * early_init_dt_check_for_usable_mem_range - Decode usable memory range
>> @@ -979,20 +987,30 @@ static phys_addr_t cap_mem_size;
>>   */
>>  static void __init early_init_dt_check_for_usable_mem_range(unsigned long node)
>>  {
>> -	const __be32 *prop;
>> -	int len;
>> +	const __be32 *prop, *endp;
>> +	int len, nr = 0;
>> +	struct memblock_region *rgn = &cap_mem_regions[0];
>>  
>>  	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->base = dt_mem_next_cell(dt_root_addr_cells, &prop);
>> +		rgn->size = dt_mem_next_cell(dt_root_size_cells, &prop);
>> +
>> +		pr_debug("cap_mem_regions[%d]: base=%pa, size=%pa\n",
>> +			 nr, &rgn->base, &rgn->size);
>> +
>> +		if (++nr >= MAX_USABLE_RANGES)
>> +			break;
>> +
>> +		rgn++;
>> +	}
>>  
>> -	pr_debug("cap_mem_start=%pa cap_mem_size=%pa\n", &cap_mem_addr,
>> -		 &cap_mem_size);
>>  }
>>  
>>  #ifdef CONFIG_SERIAL_EARLYCON
>> @@ -1265,7 +1283,8 @@ bool __init early_init_dt_verify(void *params)
>>  
>>  void __init early_init_dt_scan_nodes(void)
>>  {
>> -	int rc = 0;
>> +	int i, rc = 0;
>> +	struct memblock_region *rgn = &cap_mem_regions[0];
>>  
>>  	/* Initialize {size,address}-cells info */
>>  	of_scan_flat_dt(early_init_dt_scan_root, NULL);
>> @@ -1279,7 +1298,13 @@ void __init early_init_dt_scan_nodes(void)
>>  	of_scan_flat_dt(early_init_dt_scan_memory, NULL);
>>  
>>  	/* Handle linux,usable-memory-range property */
>> -	memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
>> +	memblock_cap_memory_range(rgn->base, rgn->size);
>> +	for (i = 1; i < MAX_USABLE_RANGES; i++) {
>> +		rgn++;
> 
> Just use rgn[i].

OK.

> 
>> +
>> +		if (rgn->size)
> 
> This check can be in the 'for' conditions check.

Yes, this node is added by kexec tool, it is impossible that
the first range is zero and the second range is not zero.

> 
>> +			memblock_add(rgn->base, rgn->size);
>> +	}
> 
> 
> There's not really any point in doing all this in 2 steps. I'm 
> assuming this needs to be handled after scanning the memory nodes, so 
> can you refactor this moving early_init_dt_check_for_usable_mem_range 
> out of early_init_dt_scan_chosen() and call it here. You'll have to get 
> the offset for /chosen twice or save the offset.

This's a good suggestion. I'll refactor it. Thanks.

Zhen Lei

> 
> Rob
> .
> 

Powered by blists - more mailing lists