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@xxxxxxxxxx> >> >> 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@xxxxxxxxxx> >> Signed-off-by: Zhen Lei <thunder.leizhen@xxxxxxxxxx> >> --- >> 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 > . >