diff mbox series

[1/7] MIPS: init: Drop boot_mem_map

Message ID 20190808075013.4852-2-jiaxun.yang@flygoat.com (mailing list archive)
State Superseded
Headers show
Series [1/7] MIPS: init: Drop boot_mem_map | expand

Commit Message

Jiaxun Yang Aug. 8, 2019, 7:50 a.m. UTC
boot_mem_map was introduced very early and cannot handle memory maps
with nid. Nowadays, memblock can exactly replace boot_mem_map.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/include/asm/bootinfo.h |  15 --
 arch/mips/kernel/setup.c         | 352 ++++++++-----------------------
 arch/mips/mm/init.c              |  51 +----
 3 files changed, 91 insertions(+), 327 deletions(-)

Comments

Serge Semin Aug. 14, 2019, 11:54 a.m. UTC | #1
Hello Jiaxun,

On Thu, Aug 08, 2019 at 03:50:07PM +0800, Jiaxun Yang wrote:
> boot_mem_map was introduced very early and cannot handle memory maps
> with nid. Nowadays, memblock can exactly replace boot_mem_map.
> 

Seeing how much changes the patch introduces, the message doesn't explains
the way the replacement and cleanup is performed. I suggest to extend the
commit message with more descriptive text of what the patch does, what is
removed and why.
As an alternative for better readability the patch could be split up
into a series of smaller commits.

> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  arch/mips/include/asm/bootinfo.h |  15 --
>  arch/mips/kernel/setup.c         | 352 ++++++++-----------------------
>  arch/mips/mm/init.c              |  51 +----
>  3 files changed, 91 insertions(+), 327 deletions(-)
> 
> diff --git a/arch/mips/include/asm/bootinfo.h b/arch/mips/include/asm/bootinfo.h
> index 235bc2f52113..f711ccf7bace 100644
> --- a/arch/mips/include/asm/bootinfo.h
> +++ b/arch/mips/include/asm/bootinfo.h
> @@ -94,21 +94,6 @@ extern unsigned long mips_machtype;
>  #define BOOT_MEM_INIT_RAM	4
>  #define BOOT_MEM_NOMAP		5
>  
> -/*
> - * A memory map that's built upon what was determined
> - * or specified on the command line.
> - */
> -struct boot_mem_map {
> -	int nr_map;
> -	struct boot_mem_map_entry {
> -		phys_addr_t addr;	/* start of memory segment */
> -		phys_addr_t size;	/* size of memory segment */
> -		long type;		/* type of memory segment */
> -	} map[BOOT_MEM_MAP_MAX];
> -};
> -
> -extern struct boot_mem_map boot_mem_map;
> -
>  extern void add_memory_region(phys_addr_t start, phys_addr_t size, long type);
>  extern void detect_memory_region(phys_addr_t start, phys_addr_t sz_min,  phys_addr_t sz_max);
>  
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index ab349d2381c3..ceef8240f171 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -63,8 +63,6 @@ unsigned long mips_machtype __read_mostly = MACH_UNKNOWN;
>  
>  EXPORT_SYMBOL(mips_machtype);
>  
> -struct boot_mem_map boot_mem_map;
> -
>  static char __initdata command_line[COMMAND_LINE_SIZE];
>  char __initdata arcs_cmdline[COMMAND_LINE_SIZE];
>  
> @@ -92,8 +90,10 @@ EXPORT_SYMBOL(ARCH_PFN_OFFSET);
>  
>  void __init add_memory_region(phys_addr_t start, phys_addr_t size, long type)
>  {
> -	int x = boot_mem_map.nr_map;
> -	int i;
> +	/*
> +	 * Note: This function only exists for historical reason,
> +	 * new code should use memblock_add or memblock_add_node instead.
> +	 */
>  

Since now we discourage to use this method to add the memory regions, what about
printing an info/warning regarding the function being deprecated, for instance
by using pr_*_once()? In addition, since you are going to mark this function
as deprecated, it's better to remove it' usage at least from the generic MIPS code:
arch/mips/kernel/setup.c
arch/mips/kernel/prom.c

On the other hand we can have a good use of this method. Since platforms are using
it to declare the whole memory space, we could perform the regions sanity checks right
here at add-time. For instance this concerns PHYS_OFFSET/PFN_OFFSET checks, HIGHMEM-related
test and so on. We could also perform the {max,min}_low_pfn, max_pfn, high{start,end}_pfn,
calculations right here. In this case the corresponding bootmem_init() code can be just
removed.

What do you think? Paul, your opinion?

>  	/*
>  	 * If the region reaches the top of the physical address space, adjust
> @@ -108,38 +108,20 @@ void __init add_memory_region(phys_addr_t start, phys_addr_t size, long type)
>  		return;
>  	}
>  
> -	/*
> -	 * Try to merge with existing entry, if any.
> -	 */
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> -		struct boot_mem_map_entry *entry = boot_mem_map.map + i;
> -		unsigned long top;
> -
> -		if (entry->type != type)
> -			continue;
> -
> -		if (start + size < entry->addr)
> -			continue;			/* no overlap */
> +	memblock_add(start, size);
> +	/* Reserve any memory except the ordinary RAM ranges. */
> +	switch (type) {
> +	case BOOT_MEM_RAM:
> +		break;
>  
> -		if (entry->addr + entry->size < start)
> -			continue;			/* no overlap */
> +	case BOOT_MEM_NOMAP: /* Discard the range from the system. */
> +		memblock_remove(start, size);
> +		break;
>  
> -		top = max(entry->addr + entry->size, start + size);
> -		entry->addr = min(entry->addr, start);
> -		entry->size = top - entry->addr;
> -
> -		return;
> +	default: /* Reserve the rest of the memory types at boot time */
> +		memblock_reserve(start, size);
> +		break;
>  	}
> -
> -	if (boot_mem_map.nr_map == BOOT_MEM_MAP_MAX) {
> -		pr_err("Ooops! Too many entries in the memory map!\n");
> -		return;
> -	}
> -
> -	boot_mem_map.map[x].addr = start;
> -	boot_mem_map.map[x].size = size;
> -	boot_mem_map.map[x].type = type;
> -	boot_mem_map.nr_map++;
>  }
>  
>  void __init detect_memory_region(phys_addr_t start, phys_addr_t sz_min, phys_addr_t sz_max)
> @@ -161,70 +143,6 @@ void __init detect_memory_region(phys_addr_t start, phys_addr_t sz_min, phys_add
>  	add_memory_region(start, size, BOOT_MEM_RAM);
>  }
>  
> -static bool __init __maybe_unused memory_region_available(phys_addr_t start,
> -							  phys_addr_t size)
> -{
> -	int i;
> -	bool in_ram = false, free = true;
> -
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> -		phys_addr_t start_, end_;
> -
> -		start_ = boot_mem_map.map[i].addr;
> -		end_ = boot_mem_map.map[i].addr + boot_mem_map.map[i].size;
> -
> -		switch (boot_mem_map.map[i].type) {
> -		case BOOT_MEM_RAM:
> -			if (start >= start_ && start + size <= end_)
> -				in_ram = true;
> -			break;
> -		case BOOT_MEM_RESERVED:
> -		case BOOT_MEM_NOMAP:
> -			if ((start >= start_ && start < end_) ||
> -			    (start < start_ && start + size >= start_))
> -				free = false;
> -			break;
> -		default:
> -			continue;
> -		}
> -	}
> -
> -	return in_ram && free;
> -}
> -
> -static void __init print_memory_map(void)
> -{
> -	int i;
> -	const int field = 2 * sizeof(unsigned long);
> -
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> -		printk(KERN_INFO " memory: %0*Lx @ %0*Lx ",
> -		       field, (unsigned long long) boot_mem_map.map[i].size,
> -		       field, (unsigned long long) boot_mem_map.map[i].addr);
> -
> -		switch (boot_mem_map.map[i].type) {
> -		case BOOT_MEM_RAM:
> -			printk(KERN_CONT "(usable)\n");
> -			break;
> -		case BOOT_MEM_INIT_RAM:
> -			printk(KERN_CONT "(usable after init)\n");
> -			break;
> -		case BOOT_MEM_ROM_DATA:
> -			printk(KERN_CONT "(ROM data)\n");
> -			break;
> -		case BOOT_MEM_RESERVED:
> -			printk(KERN_CONT "(reserved)\n");
> -			break;
> -		case BOOT_MEM_NOMAP:
> -			printk(KERN_CONT "(nomap)\n");
> -			break;
> -		default:
> -			printk(KERN_CONT "type %lu\n", boot_mem_map.map[i].type);
> -			break;
> -		}
> -	}
> -}
> -
>  /*
>   * Manage initrd
>   */
> @@ -376,8 +294,11 @@ static void __init bootmem_init(void)
>  
>  static void __init bootmem_init(void)
>  {
> -	phys_addr_t ramstart = PHYS_ADDR_MAX;
> -	int i;
> +	struct memblock_region *mem;
> +	phys_addr_t ramstart, ramend;
> +
> +	ramstart = memblock_start_of_DRAM();
> +	ramend = memblock_end_of_DRAM();
>  
>  	/*
>  	 * Sanity check any INITRD first. We don't take it into account
> @@ -398,115 +319,61 @@ static void __init bootmem_init(void)
>  	min_low_pfn = ~0UL;
>  	max_low_pfn = 0;

This initialization is pointless, since max_low_pfn is static and will be zero anyway,
while min_low_pfn will be reinitialized in the next few lines of code.

>  
> -	/* Find the highest and lowest page frame numbers we have available. */
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> -		unsigned long start, end;
> -
> -		if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
> -			continue;
> +#ifdef CONFIG_MIPS_AUTO_PFN_OFFSET
> +	min_low_pfn = PFN_UP(ramstart);
> +	ARCH_PFN_OFFSET = PFN_UP(ramstart);
> +#else
> +	/*
> +	 * Reserve any memory between the start of RAM and PHYS_OFFSET
> +	 */
> +	if (PFN_UP(ramstart) > PHYS_OFFSET)
> +		memblock_reserve(PHYS_OFFSET, PFN_UP(ramstart) - PHYS_OFFSET);

PFN_UP gives you pfn, while PHYS_OFFSET is actual address offset. This is completely
wrong. The former condition was: "(ramstart > PHYS_OFFSET)".
As I see it, this condition could be combined with the next one.

In addition, you are reserving something, which isn't going to be in memory anyway.
Is this correct?

>  
> -		start = PFN_UP(boot_mem_map.map[i].addr);
> -		end = PFN_DOWN(boot_mem_map.map[i].addr
> -				+ boot_mem_map.map[i].size);
> +	if (PFN_UP(ramstart) > ARCH_PFN_OFFSET) {
> +		pr_info("Wasting %lu bytes for tracking %lu unused pages\n",
> +			(unsigned long)((PFN_UP(ramstart) - ARCH_PFN_OFFSET) * sizeof(struct page)),
> +			(unsigned long)(PFN_UP(ramstart) - ARCH_PFN_OFFSET));
> +	} else if (ARCH_PFN_OFFSET - PFN_UP(ramstart) > 0UL) {
> +		pr_info("%lu free pages won't be used\n",
> +			(unsigned long)(ARCH_PFN_OFFSET - PFN_UP(ramstart)));
> +	}
> +	min_low_pfn = ARCH_PFN_OFFSET;
> +#endif
>  
> -		ramstart = min(ramstart, boot_mem_map.map[i].addr);
> +	max_pfn = PFN_DOWN(ramend);

So the next loop is only used to calculate the max_low_pfn and min_low_pfn is
always set to ARCH_PFN_OFFSET, right?
Then as far as I could track the min_low_pfn usage, it isn't utilized that much
in the rest of the system. According to the following changes, you don't use it
after this place either. In this case saving any value in it might be just pointless.
Before your patch, the variable was used to save the lowest low-memory pfn boundary,
which then would be used to set the low memory limit. See the loop:
"Install all valid RAM ranges to the memblock memory region." But since you
removed it, there is no point in the code above. We might need to somehow change
the memoblock lower limit here as well.

> +	for_each_memblock(memory, mem) {
> +		unsigned long start = memblock_region_memory_base_pfn(mem);
> +		unsigned long end = memblock_region_memory_end_pfn(mem);
>  
> -#ifndef CONFIG_HIGHMEM
>  		/*
>  		 * Skip highmem here so we get an accurate max_low_pfn if low
>  		 * memory stops short of high memory.
>  		 * If the region overlaps HIGHMEM_START, end is clipped so
>  		 * max_pfn excludes the highmem portion.
>  		 */
> +		if (memblock_is_nomap(mem))
> +			continue;

Sorry, I don't see a reason why do we have to skip the nomap regions here.
Could you explain?

>  		if (start >= PFN_DOWN(HIGHMEM_START))
>  			continue;
>  		if (end > PFN_DOWN(HIGHMEM_START))
>  			end = PFN_DOWN(HIGHMEM_START);
> -#endif
> -
>  		if (end > max_low_pfn)
>  			max_low_pfn = end;
> -		if (start < min_low_pfn)
> -			min_low_pfn = start;
>  	}
>  
> -	if (min_low_pfn >= max_low_pfn)
> -		panic("Incorrect memory mapping !!!");

Hmm, removing this sanity check doesn't seem right. What if a platform code
haven't added any memory region?

> -
> -#ifdef CONFIG_MIPS_AUTO_PFN_OFFSET
> -	ARCH_PFN_OFFSET = PFN_UP(ramstart);
> -#else
> -	/*
> -	 * Reserve any memory between the start of RAM and PHYS_OFFSET
> -	 */
> -	if (ramstart > PHYS_OFFSET) {
> -		add_memory_region(PHYS_OFFSET, ramstart - PHYS_OFFSET,
> -				  BOOT_MEM_RESERVED);
> -		memblock_reserve(PHYS_OFFSET, ramstart - PHYS_OFFSET);
> -	}
> -
> -	if (min_low_pfn > ARCH_PFN_OFFSET) {
> -		pr_info("Wasting %lu bytes for tracking %lu unused pages\n",
> -			(min_low_pfn - ARCH_PFN_OFFSET) * sizeof(struct page),
> -			min_low_pfn - ARCH_PFN_OFFSET);
> -	} else if (ARCH_PFN_OFFSET - min_low_pfn > 0UL) {
> -		pr_info("%lu free pages won't be used\n",
> -			ARCH_PFN_OFFSET - min_low_pfn);
> +#ifdef CONFIG_HIGHMEM
> +	if (max_pfn > PFN_DOWN(HIGHMEM_START)) {
> +		highstart_pfn = max_low_pfn;
> +		highend_pfn = max_pfn;
>  	}
> -	min_low_pfn = ARCH_PFN_OFFSET;
>  #endif

Are you sure, that 'max_low_pfn' always ends up at the 'highstart_pfn'?
I am not. Even if 'max_pfn' is greater than HIGHMEM_START, since the memory
space may have wholes, the max_low_pfn might be initialized with something
smaller than HIGHMEM_START.

One more problem I see here. What happens if CONFIG_HIGHMEM is disabled and
max_pfn exceeds HIGHMEM_START? We'll end up with unreachable memory...

>  
>  	/*
> -	 * Determine low and high memory ranges
> +	 * In any case the added to the memblock memory regions
> +	 * (highmem/lowmem, available/reserved, etc) are considered
> +	 * as present, so inform sparsemem about them.
>  	 */
> -	max_pfn = max_low_pfn;
> -	if (max_low_pfn > PFN_DOWN(HIGHMEM_START)) {
> -#ifdef CONFIG_HIGHMEM
> -		highstart_pfn = PFN_DOWN(HIGHMEM_START);
> -		highend_pfn = max_low_pfn;
> -#endif
> -		max_low_pfn = PFN_DOWN(HIGHMEM_START);
> -	}
> -
> -	/* Install all valid RAM ranges to the memblock memory region */
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> -		unsigned long start, end;
> -
> -		start = PFN_UP(boot_mem_map.map[i].addr);
> -		end = PFN_DOWN(boot_mem_map.map[i].addr
> -				+ boot_mem_map.map[i].size);
> -
> -		if (start < min_low_pfn)
> -			start = min_low_pfn;
> -#ifndef CONFIG_HIGHMEM
> -		/* Ignore highmem regions if highmem is unsupported */
> -		if (end > max_low_pfn)
> -			end = max_low_pfn;
> -#endif

Here we used to set the memory limits if platform code defined something
invalid. Like unreachable memory or a memory less than PHYS_OFFSET.
Since this code is going to be removed, the system might end up with
incorrect memory layout.

> -		if (end <= start)
> -			continue;
> -
> -		memblock_add_node(PFN_PHYS(start), PFN_PHYS(end - start), 0);
> -
> -		/* Reserve any memory except the ordinary RAM ranges. */
> -		switch (boot_mem_map.map[i].type) {
> -		case BOOT_MEM_RAM:
> -			break;
> -		case BOOT_MEM_NOMAP: /* Discard the range from the system. */
> -			memblock_remove(PFN_PHYS(start), PFN_PHYS(end - start));
> -			continue;
> -		default: /* Reserve the rest of the memory types at boot time */
> -			memblock_reserve(PFN_PHYS(start), PFN_PHYS(end - start));
> -			break;
> -		}
> -
> -		/*
> -		 * In any case the added to the memblock memory regions
> -		 * (highmem/lowmem, available/reserved, etc) are considered
> -		 * as present, so inform sparsemem about them.
> -		 */
> -		memory_present(0, start, end);
> -	}
> +	memblocks_present();
>  
>  	/*
>  	 * Reserve initrd memory if needed.
> @@ -528,8 +395,9 @@ static int __init early_parse_mem(char *p)
>  	 * size.
>  	 */
>  	if (usermem == 0) {
> -		boot_mem_map.nr_map = 0;
>  		usermem = 1;
> +		memblock_remove(memblock_start_of_DRAM(),
> +			memblock_end_of_DRAM() - memblock_start_of_DRAM());
>  	}
>  	start = 0;
>  	size = memparse(p, &p);
> @@ -586,14 +454,13 @@ early_param("memmap", early_parse_memmap);
>  unsigned long setup_elfcorehdr, setup_elfcorehdr_size;
>  static int __init early_parse_elfcorehdr(char *p)
>  {
> -	int i;
> +	struct memblock_region *mem;
>  
>  	setup_elfcorehdr = memparse(p, &p);
>  
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> -		unsigned long start = boot_mem_map.map[i].addr;
> -		unsigned long end = (boot_mem_map.map[i].addr +
> -				     boot_mem_map.map[i].size);
> +	 for_each_memblock(memory, mem) {
> +		unsigned long start = mem->base;
> +		unsigned long end = mem->end;
>  		if (setup_elfcorehdr >= start && setup_elfcorehdr < end) {
>  			/*
>  			 * Reserve from the elf core header to the end of
> @@ -613,47 +480,20 @@ static int __init early_parse_elfcorehdr(char *p)
>  early_param("elfcorehdr", early_parse_elfcorehdr);
>  #endif
>  
> -static void __init arch_mem_addpart(phys_addr_t mem, phys_addr_t end, int type)
> -{
> -	phys_addr_t size;
> -	int i;
> -
> -	size = end - mem;
> -	if (!size)
> -		return;
> -
> -	/* Make sure it is in the boot_mem_map */
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> -		if (mem >= boot_mem_map.map[i].addr &&
> -		    mem < (boot_mem_map.map[i].addr +
> -			   boot_mem_map.map[i].size))
> -			return;
> -	}
> -	add_memory_region(mem, size, type);
> -}
> -
>  #ifdef CONFIG_KEXEC
> -static inline unsigned long long get_total_mem(void)
> -{
> -	unsigned long long total;
> -
> -	total = max_pfn - min_low_pfn;
> -	return total << PAGE_SHIFT;
> -}
> -
>  static void __init mips_parse_crashkernel(void)
>  {
>  	unsigned long long total_mem;
>  	unsigned long long crash_size, crash_base;
>  	int ret;
>  
> -	total_mem = get_total_mem();
> +	total_mem = memblock_phys_mem_size();
>  	ret = parse_crashkernel(boot_command_line, total_mem,
>  				&crash_size, &crash_base);
>  	if (ret != 0 || crash_size <= 0)
>  		return;
>  
> -	if (!memory_region_available(crash_base, crash_size)) {
> +	if (!memblock_find_in_range(crash_base, crash_base + crash_size, crash_size, 0)) {
>  		pr_warn("Invalid memory region reserved for crash kernel\n");
>  		return;
>  	}
> @@ -686,6 +526,17 @@ static void __init request_crashkernel(struct resource *res)
>  }
>  #endif /* !defined(CONFIG_KEXEC)  */
>  
> +static void __init check_kernel_sections_mem(void)
> +{
> +	phys_addr_t start = PFN_DOWN(__pa_symbol(&_text)) << PAGE_SHIFT;
> +	phys_addr_t size = (PFN_UP(__pa_symbol(&_end)) - start) << PAGE_SHIFT;
> +
> +	if (!memblock_is_region_memory(start, size)) {
> +		pr_info("Kernel sections are not in the memory maps\n");
> +		memblock_add(start, size);
> +	}
> +}
> +
>  #define USE_PROM_CMDLINE	IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_BOOTLOADER)
>  #define USE_DTB_CMDLINE		IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_DTB)
>  #define EXTEND_WITH_PROM	IS_ENABLED(CONFIG_MIPS_CMDLINE_DTB_EXTEND)
> @@ -731,25 +582,6 @@ static void __init arch_mem_init(char **cmdline_p)
>  	plat_mem_setup();
>  	memblock_set_bottom_up(true);
>  
> -	/*
> -	 * Make sure all kernel memory is in the maps.  The "UP" and
> -	 * "DOWN" are opposite for initdata since if it crosses over
> -	 * into another memory section you don't want that to be
> -	 * freed when the initdata is freed.
> -	 */
> -	arch_mem_addpart(PFN_DOWN(__pa_symbol(&_text)) << PAGE_SHIFT,
> -			 PFN_UP(__pa_symbol(&_edata)) << PAGE_SHIFT,
> -			 BOOT_MEM_RAM);
> -	arch_mem_addpart(PFN_UP(__pa_symbol(&__init_begin)) << PAGE_SHIFT,
> -			 PFN_DOWN(__pa_symbol(&__init_end)) << PAGE_SHIFT,
> -			 BOOT_MEM_INIT_RAM);
> -	arch_mem_addpart(PFN_DOWN(__pa_symbol(&__bss_start)) << PAGE_SHIFT,
> -			 PFN_UP(__pa_symbol(&__bss_stop)) << PAGE_SHIFT,
> -			 BOOT_MEM_RAM);
> -

I see you replaced this code with check_kernel_sections_mem(). Neat.
You could also replace "<< PAGE_SHIFT" with PFN_PHYS() macro in the function.

> -	pr_info("Determined physical RAM map:\n");
> -	print_memory_map();

I'll miss you "print_memory_map()"... =) Nothing is going to print a detected
physical memory layout at boot time. Someone might disagree, but I found this
very useful...

> -
>  #if defined(CONFIG_CMDLINE_BOOL) && defined(CONFIG_CMDLINE_OVERRIDE)
>  	strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
>  #else
> @@ -783,14 +615,17 @@ static void __init arch_mem_init(char **cmdline_p)
>  
>  	parse_early_param();
>  
> -	if (usermem) {
> -		pr_info("User-defined physical RAM map:\n");
> -		print_memory_map();
> -	}
> +	if (usermem)
> +		pr_info("User-defined physical RAM map overwrite\n");
> +
> +	check_kernel_sections_mem();
>  
>  	early_init_fdt_reserve_self();
>  	early_init_fdt_scan_reserved_mem();
>  
> +#ifndef CONFIG_NUMA
> +	memblock_set_node(0, PHYS_ADDR_MAX, &memblock.memory, 0);
> +#endif
>  	bootmem_init();
>  
>  	/*
> @@ -835,7 +670,7 @@ static void __init arch_mem_init(char **cmdline_p)
>  
>  static void __init resource_init(void)
>  {
> -	int i;
> +	struct memblock_region *region;
>  
>  	if (UNCAC_BASE != IO_BASE)
>  		return;
> @@ -847,16 +682,10 @@ static void __init resource_init(void)
>  	bss_resource.start = __pa_symbol(&__bss_start);
>  	bss_resource.end = __pa_symbol(&__bss_stop) - 1;
>  
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> +	for_each_memblock(memory, region) {
> +		phys_addr_t start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
> +		phys_addr_t end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;

I haven't seen __pfn_to_phys() macro usage in the MIPS code. Instead the upper-case
versions (PFN_PHYS()/PHYS_PFN()) are utilized. It might be better to follow the
MIPS-platform way. They are identical at this moment though. Paul?

>  		struct resource *res;
> -		unsigned long start, end;
> -
> -		start = boot_mem_map.map[i].addr;
> -		end = boot_mem_map.map[i].addr + boot_mem_map.map[i].size - 1;
> -		if (start >= HIGHMEM_START)
> -			continue;
> -		if (end >= HIGHMEM_START)
> -			end = HIGHMEM_START - 1;
>  
>  		res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
>  		if (!res)
> @@ -865,20 +694,7 @@ static void __init resource_init(void)
>  
>  		res->start = start;
>  		res->end = end;
> -		res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> -
> -		switch (boot_mem_map.map[i].type) {
> -		case BOOT_MEM_RAM:
> -		case BOOT_MEM_INIT_RAM:
> -		case BOOT_MEM_ROM_DATA:
> -			res->name = "System RAM";

Is that right to also discard the "System RAM" resource name?

> -			res->flags |= IORESOURCE_SYSRAM;
> -			break;
> -		case BOOT_MEM_RESERVED:
> -		case BOOT_MEM_NOMAP:
> -		default:
> -			res->name = "reserved";
> -		}
> +		res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
>  
>  		request_resource(&iomem_resource, res);
>  
> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
> index 8a038b30d3c4..2bb8ebf0d2d5 100644
> --- a/arch/mips/mm/init.c
> +++ b/arch/mips/mm/init.c
> @@ -271,25 +271,17 @@ void __init fixrange_init(unsigned long start, unsigned long end,
>  
>  unsigned __weak platform_maar_init(unsigned num_pairs)
>  {
> -	struct maar_config cfg[BOOT_MEM_MAP_MAX];
> -	unsigned i, num_configured, num_cfg = 0;
> -
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> -		switch (boot_mem_map.map[i].type) {
> -		case BOOT_MEM_RAM:
> -		case BOOT_MEM_INIT_RAM:
> -			break;
> -		default:
> -			continue;
> -		}
> +	struct maar_config cfg[32];
> +	unsigned int num_configured, num_cfg = 0;
> +	struct memblock_region *mem;
>  
> +	for_each_memblock(memory, mem) {
>  		/* Round lower up */
> -		cfg[num_cfg].lower = boot_mem_map.map[i].addr;
> +		cfg[num_cfg].lower = __pfn_to_phys(memblock_region_memory_base_pfn(mem));

The same thing with PFN_PHYS()/PHYS_PFN().

>  		cfg[num_cfg].lower = (cfg[num_cfg].lower + 0xffff) & ~0xffff;
>  
>  		/* Round upper down */
> -		cfg[num_cfg].upper = boot_mem_map.map[i].addr +
> -					boot_mem_map.map[i].size;
> +		cfg[num_cfg].upper = __pfn_to_phys(memblock_region_memory_end_pfn(mem));

PFN_PHYS()/PHYS_PFN() again.

>  		cfg[num_cfg].upper = (cfg[num_cfg].upper & ~0xffff) - 1;
>  
>  		cfg[num_cfg].attrs = MIPS_MAAR_S;
> @@ -382,33 +374,6 @@ void maar_init(void)
>  }
>  
>  #ifndef CONFIG_NEED_MULTIPLE_NODES
> -int page_is_ram(unsigned long pagenr)
> -{
> -	int i;
> -
> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
> -		unsigned long addr, end;
> -
> -		switch (boot_mem_map.map[i].type) {
> -		case BOOT_MEM_RAM:
> -		case BOOT_MEM_INIT_RAM:
> -			break;
> -		default:
> -			/* not usable memory */
> -			continue;
> -		}
> -
> -		addr = PFN_UP(boot_mem_map.map[i].addr);
> -		end = PFN_DOWN(boot_mem_map.map[i].addr +
> -			       boot_mem_map.map[i].size);
> -
> -		if (pagenr >= addr && pagenr < end)
> -			return 1;
> -	}
> -
> -	return 0;
> -}
> -
>  void __init paging_init(void)
>  {
>  	unsigned long max_zone_pfns[MAX_NR_ZONES];
> @@ -452,9 +417,7 @@ static inline void mem_init_free_highmem(void)
>  		return;
>  
>  	for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
> -		struct page *page = pfn_to_page(tmp);
> -
> -		if (!page_is_ram(tmp))
> +		if (!memblock_is_memory(PFN_PHYS(tmp)))

See. You've used PFN_PHYS() here instead of __pfn_to_phys().

-Sergey

>  			SetPageReserved(page);
>  		else
>  			free_highmem_page(page);
> -- 
> 2.22.0
>
Jiaxun Yang Aug. 14, 2019, 1:40 p.m. UTC | #2
On 2019/8/14 下午7:54, Serge Semin wrote:
> Hello Jiaxun,
>
> On Thu, Aug 08, 2019 at 03:50:07PM +0800, Jiaxun Yang wrote:
>> boot_mem_map was introduced very early and cannot handle memory maps
>> with nid. Nowadays, memblock can exactly replace boot_mem_map.
>>
> Seeing how much changes the patch introduces, the message doesn't explains
> the way the replacement and cleanup is performed. I suggest to extend the
> commit message with more descriptive text of what the patch does, what is
> removed and why.

Hi Serge.

I'm going to split maar part from this patch. But I think it's fine to 
keep remaining part together. As it's almost impossible to keep code 
runnable without putting them together.

> As an alternative for better readability the patch could be split up
> into a series of smaller commits.
>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>   arch/mips/include/asm/bootinfo.h |  15 --
>>   arch/mips/kernel/setup.c         | 352 ++++++++-----------------------
>>   arch/mips/mm/init.c              |  51 +----
>>   3 files changed, 91 insertions(+), 327 deletions(-)
>>
>> diff --git a/arch/mips/include/asm/bootinfo.h b/arch/mips/include/asm/bootinfo.h
>> index 235bc2f52113..f711ccf7bace 100644
>> --- a/arch/mips/include/asm/bootinfo.h
>> +++ b/arch/mips/include/asm/bootinfo.h
>> @@ -94,21 +94,6 @@ extern unsigned long mips_machtype;
>>   #define BOOT_MEM_INIT_RAM	4
>>   #define BOOT_MEM_NOMAP		5
>>   
>> -/*
>> - * A memory map that's built upon what was determined
>> - * or specified on the command line.
>> - */
>> -struct boot_mem_map {
>> -	int nr_map;
>> -	struct boot_mem_map_entry {
>> -		phys_addr_t addr;	/* start of memory segment */
>> -		phys_addr_t size;	/* size of memory segment */
>> -		long type;		/* type of memory segment */
>> -	} map[BOOT_MEM_MAP_MAX];
>> -};
>> -
>> -extern struct boot_mem_map boot_mem_map;
>> -
>>   extern void add_memory_region(phys_addr_t start, phys_addr_t size, long type);
>>   extern void detect_memory_region(phys_addr_t start, phys_addr_t sz_min,  phys_addr_t sz_max);
>>   
>> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
>> index ab349d2381c3..ceef8240f171 100644
>> --- a/arch/mips/kernel/setup.c
>> +++ b/arch/mips/kernel/setup.c
>> @@ -63,8 +63,6 @@ unsigned long mips_machtype __read_mostly = MACH_UNKNOWN;
>>   
>>   EXPORT_SYMBOL(mips_machtype);
>>   
>> -struct boot_mem_map boot_mem_map;
>> -
>>   static char __initdata command_line[COMMAND_LINE_SIZE];
>>   char __initdata arcs_cmdline[COMMAND_LINE_SIZE];
>>   
>> @@ -92,8 +90,10 @@ EXPORT_SYMBOL(ARCH_PFN_OFFSET);
>>   
>>   void __init add_memory_region(phys_addr_t start, phys_addr_t size, long type)
>>   {
>> -	int x = boot_mem_map.nr_map;
>> -	int i;
>> +	/*
>> +	 * Note: This function only exists for historical reason,
>> +	 * new code should use memblock_add or memblock_add_node instead.
>> +	 */
>>   
> Since now we discourage to use this method to add the memory regions, what about
> printing an info/warning regarding the function being deprecated, for instance
> by using pr_*_once()? In addition, since you are going to mark this function
> as deprecated, it's better to remove it' usage at least from the generic MIPS code:
> arch/mips/kernel/setup.c
> arch/mips/kernel/prom.c
>
> On the other hand we can have a good use of this method. Since platforms are using
> it to declare the whole memory space, we could perform the regions sanity checks right
> here at add-time. For instance this concerns PHYS_OFFSET/PFN_OFFSET checks, HIGHMEM-related
> test and so on. We could also perform the {max,min}_low_pfn, max_pfn, high{start,end}_pfn,
> calculations right here. In this case the corresponding bootmem_init() code can be just
> removed.

As my universal target is to simplify the NUMA memory initialization, 
without add "nid" to this function, I don't think it is a good idea to 
keep it.

But the conversion from this function to memblock is not that urgent, 
probably not worthy to give a warning.

>
> What do you think? Paul, your opinion?
>
>>   	/*
>>   	 * If the region reaches the top of the physical address space, adjust
>> @@ -108,38 +108,20 @@ void __init add_memory_region(phys_addr_t start, phys_addr_t size, long type)
>>   		return;
>>   	}
>>   
>> -	/*
>> -	 * Try to merge with existing entry, if any.
>> -	 */
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> -		struct boot_mem_map_entry *entry = boot_mem_map.map + i;
>> -		unsigned long top;
>> -
>> -		if (entry->type != type)
>> -			continue;
>> -
>> -		if (start + size < entry->addr)
>> -			continue;			/* no overlap */
>> +	memblock_add(start, size);
>> +	/* Reserve any memory except the ordinary RAM ranges. */
>> +	switch (type) {
>> +	case BOOT_MEM_RAM:
>> +		break;
>>   
>> -		if (entry->addr + entry->size < start)
>> -			continue;			/* no overlap */
>> +	case BOOT_MEM_NOMAP: /* Discard the range from the system. */
>> +		memblock_remove(start, size);
>> +		break;
>>   
>> -		top = max(entry->addr + entry->size, start + size);
>> -		entry->addr = min(entry->addr, start);
>> -		entry->size = top - entry->addr;
>> -
>> -		return;
>> +	default: /* Reserve the rest of the memory types at boot time */
>> +		memblock_reserve(start, size);
>> +		break;
>>   	}
>> -
>> -	if (boot_mem_map.nr_map == BOOT_MEM_MAP_MAX) {
>> -		pr_err("Ooops! Too many entries in the memory map!\n");
>> -		return;
>> -	}
>> -
>> -	boot_mem_map.map[x].addr = start;
>> -	boot_mem_map.map[x].size = size;
>> -	boot_mem_map.map[x].type = type;
>> -	boot_mem_map.nr_map++;
>>   }
>>   
>>   void __init detect_memory_region(phys_addr_t start, phys_addr_t sz_min, phys_addr_t sz_max)
>> @@ -161,70 +143,6 @@ void __init detect_memory_region(phys_addr_t start, phys_addr_t sz_min, phys_add
>>   	add_memory_region(start, size, BOOT_MEM_RAM);
>>   }
>>   
>> -static bool __init __maybe_unused memory_region_available(phys_addr_t start,
>> -							  phys_addr_t size)
>> -{
>> -	int i;
>> -	bool in_ram = false, free = true;
>> -
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> -		phys_addr_t start_, end_;
>> -
>> -		start_ = boot_mem_map.map[i].addr;
>> -		end_ = boot_mem_map.map[i].addr + boot_mem_map.map[i].size;
>> -
>> -		switch (boot_mem_map.map[i].type) {
>> -		case BOOT_MEM_RAM:
>> -			if (start >= start_ && start + size <= end_)
>> -				in_ram = true;
>> -			break;
>> -		case BOOT_MEM_RESERVED:
>> -		case BOOT_MEM_NOMAP:
>> -			if ((start >= start_ && start < end_) ||
>> -			    (start < start_ && start + size >= start_))
>> -				free = false;
>> -			break;
>> -		default:
>> -			continue;
>> -		}
>> -	}
>> -
>> -	return in_ram && free;
>> -}
>> -
>> -static void __init print_memory_map(void)
>> -{
>> -	int i;
>> -	const int field = 2 * sizeof(unsigned long);
>> -
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> -		printk(KERN_INFO " memory: %0*Lx @ %0*Lx ",
>> -		       field, (unsigned long long) boot_mem_map.map[i].size,
>> -		       field, (unsigned long long) boot_mem_map.map[i].addr);
>> -
>> -		switch (boot_mem_map.map[i].type) {
>> -		case BOOT_MEM_RAM:
>> -			printk(KERN_CONT "(usable)\n");
>> -			break;
>> -		case BOOT_MEM_INIT_RAM:
>> -			printk(KERN_CONT "(usable after init)\n");
>> -			break;
>> -		case BOOT_MEM_ROM_DATA:
>> -			printk(KERN_CONT "(ROM data)\n");
>> -			break;
>> -		case BOOT_MEM_RESERVED:
>> -			printk(KERN_CONT "(reserved)\n");
>> -			break;
>> -		case BOOT_MEM_NOMAP:
>> -			printk(KERN_CONT "(nomap)\n");
>> -			break;
>> -		default:
>> -			printk(KERN_CONT "type %lu\n", boot_mem_map.map[i].type);
>> -			break;
>> -		}
>> -	}
>> -}
>> -
>>   /*
>>    * Manage initrd
>>    */
>> @@ -376,8 +294,11 @@ static void __init bootmem_init(void)
>>   
>>   static void __init bootmem_init(void)
>>   {
>> -	phys_addr_t ramstart = PHYS_ADDR_MAX;
>> -	int i;
>> +	struct memblock_region *mem;
>> +	phys_addr_t ramstart, ramend;
>> +
>> +	ramstart = memblock_start_of_DRAM();	
>> +	ramend = memblock_end_of_DRAM();
>>   
>>   	/*
>>   	 * Sanity check any INITRD first. We don't take it into account
>> @@ -398,115 +319,61 @@ static void __init bootmem_init(void)
>>   	min_low_pfn = ~0UL;
>>   	max_low_pfn = 0;
> This initialization is pointless, since max_low_pfn is static and will be zero anyway,
> while min_low_pfn will be reinitialized in the next few lines of code.
I'm going to remove it, nobody knows why it exist.
>
>>   
>> -	/* Find the highest and lowest page frame numbers we have available. */
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> -		unsigned long start, end;
>> -
>> -		if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
>> -			continue;
>> +#ifdef CONFIG_MIPS_AUTO_PFN_OFFSET
>> +	min_low_pfn = PFN_UP(ramstart);
>> +	ARCH_PFN_OFFSET = PFN_UP(ramstart);
>> +#else
>> +	/*
>> +	 * Reserve any memory between the start of RAM and PHYS_OFFSET
>> +	 */
>> +	if (PFN_UP(ramstart) > PHYS_OFFSET)
>> +		memblock_reserve(PHYS_OFFSET, PFN_UP(ramstart) - PHYS_OFFSET);
> PFN_UP gives you pfn, while PHYS_OFFSET is actual address offset. This is completely
> wrong. The former condition was: "(ramstart > PHYS_OFFSET)".
> As I see it, this condition could be combined with the next one.
My fault, will fix it.
>
> In addition, you are reserving something, which isn't going to be in memory anyway.
> Is this correct?
Yes, for memblock it should be safe to reserve it. Or probably removing 
it is better?
>
>>   
>> -		start = PFN_UP(boot_mem_map.map[i].addr);
>> -		end = PFN_DOWN(boot_mem_map.map[i].addr
>> -				+ boot_mem_map.map[i].size);
>> +	if (PFN_UP(ramstart) > ARCH_PFN_OFFSET) {
>> +		pr_info("Wasting %lu bytes for tracking %lu unused pages\n",
>> +			(unsigned long)((PFN_UP(ramstart) - ARCH_PFN_OFFSET) * sizeof(struct page)),
>> +			(unsigned long)(PFN_UP(ramstart) - ARCH_PFN_OFFSET));
>> +	} else if (ARCH_PFN_OFFSET - PFN_UP(ramstart) > 0UL) {
>> +		pr_info("%lu free pages won't be used\n",
>> +			(unsigned long)(ARCH_PFN_OFFSET - PFN_UP(ramstart)));
>> +	}
>> +	min_low_pfn = ARCH_PFN_OFFSET;
>> +#endif
>>   
>> -		ramstart = min(ramstart, boot_mem_map.map[i].addr);
>> +	max_pfn = PFN_DOWN(ramend);
> So the next loop is only used to calculate the max_low_pfn and min_low_pfn is
> always set to ARCH_PFN_OFFSET, right?
> Then as far as I could track the min_low_pfn usage, it isn't utilized that much
> in the rest of the system. According to the following changes, you don't use it
> after this place either. In this case saving any value in it might be just pointless.
> Before your patch, the variable was used to save the lowest low-memory pfn boundary,
> which then would be used to set the low memory limit. See the loop:
> "Install all valid RAM ranges to the memblock memory region." But since you
> removed it, there is no point in the code above. We might need to somehow change
> the memoblock lower limit here as well.
Got it, thanks.
>
>> +	for_each_memblock(memory, mem) {
>> +		unsigned long start = memblock_region_memory_base_pfn(mem);
>> +		unsigned long end = memblock_region_memory_end_pfn(mem);
>>   
>> -#ifndef CONFIG_HIGHMEM
>>   		/*
>>   		 * Skip highmem here so we get an accurate max_low_pfn if low
>>   		 * memory stops short of high memory.
>>   		 * If the region overlaps HIGHMEM_START, end is clipped so
>>   		 * max_pfn excludes the highmem portion.
>>   		 */
>> +		if (memblock_is_nomap(mem))
>> +			continue;
> Sorry, I don't see a reason why do we have to skip the nomap regions here.
> Could you explain?

As the origin code have:

         if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
             continue;

>
>>   		if (start >= PFN_DOWN(HIGHMEM_START))
>>   			continue;
>>   		if (end > PFN_DOWN(HIGHMEM_START))
>>   			end = PFN_DOWN(HIGHMEM_START);
>> -#endif
>> -
>>   		if (end > max_low_pfn)
>>   			max_low_pfn = end;
>> -		if (start < min_low_pfn)
>> -			min_low_pfn = start;
>>   	}
>>   
>> -	if (min_low_pfn >= max_low_pfn)
>> -		panic("Incorrect memory mapping !!!");
> Hmm, removing this sanity check doesn't seem right. What if a platform code
> haven't added any memory region?

This won't happen unless platform didn't add any low mem.

>
>> -
>> -#ifdef CONFIG_MIPS_AUTO_PFN_OFFSET
>> -	ARCH_PFN_OFFSET = PFN_UP(ramstart);
>> -#else
>> -	/*
>> -	 * Reserve any memory between the start of RAM and PHYS_OFFSET
>> -	 */
>> -	if (ramstart > PHYS_OFFSET) {
>> -		add_memory_region(PHYS_OFFSET, ramstart - PHYS_OFFSET,
>> -				  BOOT_MEM_RESERVED);
>> -		memblock_reserve(PHYS_OFFSET, ramstart - PHYS_OFFSET);
>> -	}
>> -
>> -	if (min_low_pfn > ARCH_PFN_OFFSET) {
>> -		pr_info("Wasting %lu bytes for tracking %lu unused pages\n",
>> -			(min_low_pfn - ARCH_PFN_OFFSET) * sizeof(struct page),
>> -			min_low_pfn - ARCH_PFN_OFFSET);
>> -	} else if (ARCH_PFN_OFFSET - min_low_pfn > 0UL) {
>> -		pr_info("%lu free pages won't be used\n",
>> -			ARCH_PFN_OFFSET - min_low_pfn);
>> +#ifdef CONFIG_HIGHMEM
>> +	if (max_pfn > PFN_DOWN(HIGHMEM_START)) {
>> +		highstart_pfn = max_low_pfn;
>> +		highend_pfn = max_pfn;
>>   	}
>> -	min_low_pfn = ARCH_PFN_OFFSET;
>>   #endif
> Are you sure, that 'max_low_pfn' always ends up at the 'highstart_pfn'?
> I am not. Even if 'max_pfn' is greater than HIGHMEM_START, since the memory
> space may have wholes, the max_low_pfn might be initialized with something
> smaller than HIGHMEM_START.

In mm/memory.c it mentions  max_low_pfn and highstart_pfn must be the same

But it should be a x86-only requirement.

Should I fix it?

> One more problem I see here. What happens if CONFIG_HIGHMEM is disabled and
> max_pfn exceeds HIGHMEM_START? We'll end up with unreachable memory...
I'm going to set max_pfn = HIGHMEM_START if max_pfn > HIGGMEM_START 
while HIGHMEM disabled.

>>   
>>   	/*
>> -	 * Determine low and high memory ranges
>> +	 * In any case the added to the memblock memory regions
>> +	 * (highmem/lowmem, available/reserved, etc) are considered
>> +	 * as present, so inform sparsemem about them.
>>   	 */
>> -	max_pfn = max_low_pfn;
>> -	if (max_low_pfn > PFN_DOWN(HIGHMEM_START)) {
>> -#ifdef CONFIG_HIGHMEM
>> -		highstart_pfn = PFN_DOWN(HIGHMEM_START);
>> -		highend_pfn = max_low_pfn;
>> -#endif
>> -		max_low_pfn = PFN_DOWN(HIGHMEM_START);
>> -	}
>> -
>> -	/* Install all valid RAM ranges to the memblock memory region */
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> -		unsigned long start, end;
>> -
>> -		start = PFN_UP(boot_mem_map.map[i].addr);
>> -		end = PFN_DOWN(boot_mem_map.map[i].addr
>> -				+ boot_mem_map.map[i].size);
>> -
>> -		if (start < min_low_pfn)
>> -			start = min_low_pfn;
>> -#ifndef CONFIG_HIGHMEM
>> -		/* Ignore highmem regions if highmem is unsupported */
>> -		if (end > max_low_pfn)
>> -			end = max_low_pfn;
>> -#endif
> Here we used to set the memory limits if platform code defined something
> invalid. Like unreachable memory or a memory less than PHYS_OFFSET.
> Since this code is going to be removed, the system might end up with
> incorrect memory layout.
Memblock won't allow something like this happen, never.
>
>> -		if (end <= start)
>> -			continue;
>> -
>> -		memblock_add_node(PFN_PHYS(start), PFN_PHYS(end - start), 0);
>> -
>> -		/* Reserve any memory except the ordinary RAM ranges. */
>> -		switch (boot_mem_map.map[i].type) {
>> -		case BOOT_MEM_RAM:
>> -			break;
>> -		case BOOT_MEM_NOMAP: /* Discard the range from the system. */
>> -			memblock_remove(PFN_PHYS(start), PFN_PHYS(end - start));
>> -			continue;
>> -		default: /* Reserve the rest of the memory types at boot time */
>> -			memblock_reserve(PFN_PHYS(start), PFN_PHYS(end - start));
>> -			break;
>> -		}
>> -
>> -		/*
>> -		 * In any case the added to the memblock memory regions
>> -		 * (highmem/lowmem, available/reserved, etc) are considered
>> -		 * as present, so inform sparsemem about them.
>> -		 */
>> -		memory_present(0, start, end);
>> -	}
>> +	memblocks_present();
>>   
>>   	/*
>>   	 * Reserve initrd memory if needed.
>> @@ -528,8 +395,9 @@ static int __init early_parse_mem(char *p)
>>   	 * size.
>>   	 */
>>   	if (usermem == 0) {
>> -		boot_mem_map.nr_map = 0;
>>   		usermem = 1;
>> +		memblock_remove(memblock_start_of_DRAM(),
>> +			memblock_end_of_DRAM() - memblock_start_of_DRAM());
>>   	}
>>   	start = 0;
>>   	size = memparse(p, &p);
>> @@ -586,14 +454,13 @@ early_param("memmap", early_parse_memmap);
>>   unsigned long setup_elfcorehdr, setup_elfcorehdr_size;
>>   static int __init early_parse_elfcorehdr(char *p)
>>   {
>> -	int i;
>> +	struct memblock_region *mem;
>>   
>>   	setup_elfcorehdr = memparse(p, &p);
>>   
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> -		unsigned long start = boot_mem_map.map[i].addr;
>> -		unsigned long end = (boot_mem_map.map[i].addr +
>> -				     boot_mem_map.map[i].size);
>> +	 for_each_memblock(memory, mem) {
>> +		unsigned long start = mem->base;
>> +		unsigned long end = mem->end;
>>   		if (setup_elfcorehdr >= start && setup_elfcorehdr < end) {
>>   			/*
>>   			 * Reserve from the elf core header to the end of
>> @@ -613,47 +480,20 @@ static int __init early_parse_elfcorehdr(char *p)
>>   early_param("elfcorehdr", early_parse_elfcorehdr);
>>   #endif
>>   
>> -static void __init arch_mem_addpart(phys_addr_t mem, phys_addr_t end, int type)
>> -{
>> -	phys_addr_t size;
>> -	int i;
>> -
>> -	size = end - mem;
>> -	if (!size)
>> -		return;
>> -
>> -	/* Make sure it is in the boot_mem_map */
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> -		if (mem >= boot_mem_map.map[i].addr &&
>> -		    mem < (boot_mem_map.map[i].addr +
>> -			   boot_mem_map.map[i].size))
>> -			return;
>> -	}
>> -	add_memory_region(mem, size, type);
>> -}
>> -
>>   #ifdef CONFIG_KEXEC
>> -static inline unsigned long long get_total_mem(void)
>> -{
>> -	unsigned long long total;
>> -
>> -	total = max_pfn - min_low_pfn;
>> -	return total << PAGE_SHIFT;
>> -}
>> -
>>   static void __init mips_parse_crashkernel(void)
>>   {
>>   	unsigned long long total_mem;
>>   	unsigned long long crash_size, crash_base;
>>   	int ret;
>>   
>> -	total_mem = get_total_mem();
>> +	total_mem = memblock_phys_mem_size();
>>   	ret = parse_crashkernel(boot_command_line, total_mem,
>>   				&crash_size, &crash_base);
>>   	if (ret != 0 || crash_size <= 0)
>>   		return;
>>   
>> -	if (!memory_region_available(crash_base, crash_size)) {
>> +	if (!memblock_find_in_range(crash_base, crash_base + crash_size, crash_size, 0)) {
>>   		pr_warn("Invalid memory region reserved for crash kernel\n");
>>   		return;
>>   	}
>> @@ -686,6 +526,17 @@ static void __init request_crashkernel(struct resource *res)
>>   }
>>   #endif /* !defined(CONFIG_KEXEC)  */
>>   
>> +static void __init check_kernel_sections_mem(void)
>> +{
>> +	phys_addr_t start = PFN_DOWN(__pa_symbol(&_text)) << PAGE_SHIFT;
>> +	phys_addr_t size = (PFN_UP(__pa_symbol(&_end)) - start) << PAGE_SHIFT;
>> +
>> +	if (!memblock_is_region_memory(start, size)) {
>> +		pr_info("Kernel sections are not in the memory maps\n");
>> +		memblock_add(start, size);
>> +	}
>> +}
>> +
>>   #define USE_PROM_CMDLINE	IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_BOOTLOADER)
>>   #define USE_DTB_CMDLINE		IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_DTB)
>>   #define EXTEND_WITH_PROM	IS_ENABLED(CONFIG_MIPS_CMDLINE_DTB_EXTEND)
>> @@ -731,25 +582,6 @@ static void __init arch_mem_init(char **cmdline_p)
>>   	plat_mem_setup();
>>   	memblock_set_bottom_up(true);
>>   
>> -	/*
>> -	 * Make sure all kernel memory is in the maps.  The "UP" and
>> -	 * "DOWN" are opposite for initdata since if it crosses over
>> -	 * into another memory section you don't want that to be
>> -	 * freed when the initdata is freed.
>> -	 */
>> -	arch_mem_addpart(PFN_DOWN(__pa_symbol(&_text)) << PAGE_SHIFT,
>> -			 PFN_UP(__pa_symbol(&_edata)) << PAGE_SHIFT,
>> -			 BOOT_MEM_RAM);
>> -	arch_mem_addpart(PFN_UP(__pa_symbol(&__init_begin)) << PAGE_SHIFT,
>> -			 PFN_DOWN(__pa_symbol(&__init_end)) << PAGE_SHIFT,
>> -			 BOOT_MEM_INIT_RAM);
>> -	arch_mem_addpart(PFN_DOWN(__pa_symbol(&__bss_start)) << PAGE_SHIFT,
>> -			 PFN_UP(__pa_symbol(&__bss_stop)) << PAGE_SHIFT,
>> -			 BOOT_MEM_RAM);
>> -
> I see you replaced this code with check_kernel_sections_mem(). Neat.
> You could also replace "<< PAGE_SHIFT" with PFN_PHYS() macro in the function.
>
>> -	pr_info("Determined physical RAM map:\n");
>> -	print_memory_map();
> I'll miss you "print_memory_map()"... =) Nothing is going to print a detected
> physical memory layout at boot time. Someone might disagree, but I found this
> very useful...
memblock will also print it.
>
>> -
>>   #if defined(CONFIG_CMDLINE_BOOL) && defined(CONFIG_CMDLINE_OVERRIDE)
>>   	strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
>>   #else
>> @@ -783,14 +615,17 @@ static void __init arch_mem_init(char **cmdline_p)
>>   
>>   	parse_early_param();
>>   
>> -	if (usermem) {
>> -		pr_info("User-defined physical RAM map:\n");
>> -		print_memory_map();
>> -	}
>> +	if (usermem)
>> +		pr_info("User-defined physical RAM map overwrite\n");
>> +
>> +	check_kernel_sections_mem();
>>   
>>   	early_init_fdt_reserve_self();
>>   	early_init_fdt_scan_reserved_mem();
>>   
>> +#ifndef CONFIG_NUMA
>> +	memblock_set_node(0, PHYS_ADDR_MAX, &memblock.memory, 0);
>> +#endif
>>   	bootmem_init();
>>   
>>   	/*
>> @@ -835,7 +670,7 @@ static void __init arch_mem_init(char **cmdline_p)
>>   
>>   static void __init resource_init(void)
>>   {
>> -	int i;
>> +	struct memblock_region *region;
>>   
>>   	if (UNCAC_BASE != IO_BASE)
>>   		return;
>> @@ -847,16 +682,10 @@ static void __init resource_init(void)
>>   	bss_resource.start = __pa_symbol(&__bss_start);
>>   	bss_resource.end = __pa_symbol(&__bss_stop) - 1;
>>   
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> +	for_each_memblock(memory, region) {
>> +		phys_addr_t start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
>> +		phys_addr_t end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;
> I haven't seen __pfn_to_phys() macro usage in the MIPS code. Instead the upper-case
> versions (PFN_PHYS()/PHYS_PFN()) are utilized. It might be better to follow the
> MIPS-platform way. They are identical at this moment though. Paul?
I just copied them from ARM side. I'm going to unify them.
>>   		struct resource *res;
>> -		unsigned long start, end;
>> -
>> -		start = boot_mem_map.map[i].addr;
>> -		end = boot_mem_map.map[i].addr + boot_mem_map.map[i].size - 1;
>> -		if (start >= HIGHMEM_START)
>> -			continue;
>> -		if (end >= HIGHMEM_START)
>> -			end = HIGHMEM_START - 1;
>>   
>>   		res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
>>   		if (!res)
>> @@ -865,20 +694,7 @@ static void __init resource_init(void)
>>   
>>   		res->start = start;
>>   		res->end = end;
>> -		res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
>> -
>> -		switch (boot_mem_map.map[i].type) {
>> -		case BOOT_MEM_RAM:
>> -		case BOOT_MEM_INIT_RAM:
>> -		case BOOT_MEM_ROM_DATA:
>> -			res->name = "System RAM";
> Is that right to also discard the "System RAM" resource name?
>
>> -			res->flags |= IORESOURCE_SYSRAM;
>> -			break;
>> -		case BOOT_MEM_RESERVED:
>> -		case BOOT_MEM_NOMAP:
>> -		default:
>> -			res->name = "reserved";
>> -		}
>> +		res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
>>   
>>   		request_resource(&iomem_resource, res);
>>   
>> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
>> index 8a038b30d3c4..2bb8ebf0d2d5 100644
>> --- a/arch/mips/mm/init.c
>> +++ b/arch/mips/mm/init.c
>> @@ -271,25 +271,17 @@ void __init fixrange_init(unsigned long start, unsigned long end,
>>   
>>   unsigned __weak platform_maar_init(unsigned num_pairs)
>>   {
>> -	struct maar_config cfg[BOOT_MEM_MAP_MAX];
>> -	unsigned i, num_configured, num_cfg = 0;
>> -
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> -		switch (boot_mem_map.map[i].type) {
>> -		case BOOT_MEM_RAM:
>> -		case BOOT_MEM_INIT_RAM:
>> -			break;
>> -		default:
>> -			continue;
>> -		}
>> +	struct maar_config cfg[32];
>> +	unsigned int num_configured, num_cfg = 0;
>> +	struct memblock_region *mem;
>>   
>> +	for_each_memblock(memory, mem) {
>>   		/* Round lower up */
>> -		cfg[num_cfg].lower = boot_mem_map.map[i].addr;
>> +		cfg[num_cfg].lower = __pfn_to_phys(memblock_region_memory_base_pfn(mem));
> The same thing with PFN_PHYS()/PHYS_PFN().
>
>>   		cfg[num_cfg].lower = (cfg[num_cfg].lower + 0xffff) & ~0xffff;
>>   
>>   		/* Round upper down */
>> -		cfg[num_cfg].upper = boot_mem_map.map[i].addr +
>> -					boot_mem_map.map[i].size;
>> +		cfg[num_cfg].upper = __pfn_to_phys(memblock_region_memory_end_pfn(mem));
> PFN_PHYS()/PHYS_PFN() again.
>
>>   		cfg[num_cfg].upper = (cfg[num_cfg].upper & ~0xffff) - 1;
>>   
>>   		cfg[num_cfg].attrs = MIPS_MAAR_S;
>> @@ -382,33 +374,6 @@ void maar_init(void)
>>   }
>>   
>>   #ifndef CONFIG_NEED_MULTIPLE_NODES
>> -int page_is_ram(unsigned long pagenr)
>> -{
>> -	int i;
>> -
>> -	for (i = 0; i < boot_mem_map.nr_map; i++) {
>> -		unsigned long addr, end;
>> -
>> -		switch (boot_mem_map.map[i].type) {
>> -		case BOOT_MEM_RAM:
>> -		case BOOT_MEM_INIT_RAM:
>> -			break;
>> -		default:
>> -			/* not usable memory */
>> -			continue;
>> -		}
>> -
>> -		addr = PFN_UP(boot_mem_map.map[i].addr);
>> -		end = PFN_DOWN(boot_mem_map.map[i].addr +
>> -			       boot_mem_map.map[i].size);
>> -
>> -		if (pagenr >= addr && pagenr < end)
>> -			return 1;
>> -	}
>> -
>> -	return 0;
>> -}
>> -
>>   void __init paging_init(void)
>>   {
>>   	unsigned long max_zone_pfns[MAX_NR_ZONES];
>> @@ -452,9 +417,7 @@ static inline void mem_init_free_highmem(void)
>>   		return;
>>   
>>   	for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
>> -		struct page *page = pfn_to_page(tmp);
>> -
>> -		if (!page_is_ram(tmp))
>> +		if (!memblock_is_memory(PFN_PHYS(tmp)))
> See. You've used PFN_PHYS() here instead of __pfn_to_phys().

Because that was written by myself : -)

Thanks for pointing out these issues.

> -Sergey
>
>>   			SetPageReserved(page);
>>   		else
>>   			free_highmem_page(page);
>> -- 
>> 2.22.0
>>
---

Jiaxun
diff mbox series

Patch

diff --git a/arch/mips/include/asm/bootinfo.h b/arch/mips/include/asm/bootinfo.h
index 235bc2f52113..f711ccf7bace 100644
--- a/arch/mips/include/asm/bootinfo.h
+++ b/arch/mips/include/asm/bootinfo.h
@@ -94,21 +94,6 @@  extern unsigned long mips_machtype;
 #define BOOT_MEM_INIT_RAM	4
 #define BOOT_MEM_NOMAP		5
 
-/*
- * A memory map that's built upon what was determined
- * or specified on the command line.
- */
-struct boot_mem_map {
-	int nr_map;
-	struct boot_mem_map_entry {
-		phys_addr_t addr;	/* start of memory segment */
-		phys_addr_t size;	/* size of memory segment */
-		long type;		/* type of memory segment */
-	} map[BOOT_MEM_MAP_MAX];
-};
-
-extern struct boot_mem_map boot_mem_map;
-
 extern void add_memory_region(phys_addr_t start, phys_addr_t size, long type);
 extern void detect_memory_region(phys_addr_t start, phys_addr_t sz_min,  phys_addr_t sz_max);
 
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index ab349d2381c3..ceef8240f171 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -63,8 +63,6 @@  unsigned long mips_machtype __read_mostly = MACH_UNKNOWN;
 
 EXPORT_SYMBOL(mips_machtype);
 
-struct boot_mem_map boot_mem_map;
-
 static char __initdata command_line[COMMAND_LINE_SIZE];
 char __initdata arcs_cmdline[COMMAND_LINE_SIZE];
 
@@ -92,8 +90,10 @@  EXPORT_SYMBOL(ARCH_PFN_OFFSET);
 
 void __init add_memory_region(phys_addr_t start, phys_addr_t size, long type)
 {
-	int x = boot_mem_map.nr_map;
-	int i;
+	/*
+	 * Note: This function only exists for historical reason,
+	 * new code should use memblock_add or memblock_add_node instead.
+	 */
 
 	/*
 	 * If the region reaches the top of the physical address space, adjust
@@ -108,38 +108,20 @@  void __init add_memory_region(phys_addr_t start, phys_addr_t size, long type)
 		return;
 	}
 
-	/*
-	 * Try to merge with existing entry, if any.
-	 */
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		struct boot_mem_map_entry *entry = boot_mem_map.map + i;
-		unsigned long top;
-
-		if (entry->type != type)
-			continue;
-
-		if (start + size < entry->addr)
-			continue;			/* no overlap */
+	memblock_add(start, size);
+	/* Reserve any memory except the ordinary RAM ranges. */
+	switch (type) {
+	case BOOT_MEM_RAM:
+		break;
 
-		if (entry->addr + entry->size < start)
-			continue;			/* no overlap */
+	case BOOT_MEM_NOMAP: /* Discard the range from the system. */
+		memblock_remove(start, size);
+		break;
 
-		top = max(entry->addr + entry->size, start + size);
-		entry->addr = min(entry->addr, start);
-		entry->size = top - entry->addr;
-
-		return;
+	default: /* Reserve the rest of the memory types at boot time */
+		memblock_reserve(start, size);
+		break;
 	}
-
-	if (boot_mem_map.nr_map == BOOT_MEM_MAP_MAX) {
-		pr_err("Ooops! Too many entries in the memory map!\n");
-		return;
-	}
-
-	boot_mem_map.map[x].addr = start;
-	boot_mem_map.map[x].size = size;
-	boot_mem_map.map[x].type = type;
-	boot_mem_map.nr_map++;
 }
 
 void __init detect_memory_region(phys_addr_t start, phys_addr_t sz_min, phys_addr_t sz_max)
@@ -161,70 +143,6 @@  void __init detect_memory_region(phys_addr_t start, phys_addr_t sz_min, phys_add
 	add_memory_region(start, size, BOOT_MEM_RAM);
 }
 
-static bool __init __maybe_unused memory_region_available(phys_addr_t start,
-							  phys_addr_t size)
-{
-	int i;
-	bool in_ram = false, free = true;
-
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		phys_addr_t start_, end_;
-
-		start_ = boot_mem_map.map[i].addr;
-		end_ = boot_mem_map.map[i].addr + boot_mem_map.map[i].size;
-
-		switch (boot_mem_map.map[i].type) {
-		case BOOT_MEM_RAM:
-			if (start >= start_ && start + size <= end_)
-				in_ram = true;
-			break;
-		case BOOT_MEM_RESERVED:
-		case BOOT_MEM_NOMAP:
-			if ((start >= start_ && start < end_) ||
-			    (start < start_ && start + size >= start_))
-				free = false;
-			break;
-		default:
-			continue;
-		}
-	}
-
-	return in_ram && free;
-}
-
-static void __init print_memory_map(void)
-{
-	int i;
-	const int field = 2 * sizeof(unsigned long);
-
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		printk(KERN_INFO " memory: %0*Lx @ %0*Lx ",
-		       field, (unsigned long long) boot_mem_map.map[i].size,
-		       field, (unsigned long long) boot_mem_map.map[i].addr);
-
-		switch (boot_mem_map.map[i].type) {
-		case BOOT_MEM_RAM:
-			printk(KERN_CONT "(usable)\n");
-			break;
-		case BOOT_MEM_INIT_RAM:
-			printk(KERN_CONT "(usable after init)\n");
-			break;
-		case BOOT_MEM_ROM_DATA:
-			printk(KERN_CONT "(ROM data)\n");
-			break;
-		case BOOT_MEM_RESERVED:
-			printk(KERN_CONT "(reserved)\n");
-			break;
-		case BOOT_MEM_NOMAP:
-			printk(KERN_CONT "(nomap)\n");
-			break;
-		default:
-			printk(KERN_CONT "type %lu\n", boot_mem_map.map[i].type);
-			break;
-		}
-	}
-}
-
 /*
  * Manage initrd
  */
@@ -376,8 +294,11 @@  static void __init bootmem_init(void)
 
 static void __init bootmem_init(void)
 {
-	phys_addr_t ramstart = PHYS_ADDR_MAX;
-	int i;
+	struct memblock_region *mem;
+	phys_addr_t ramstart, ramend;
+
+	ramstart = memblock_start_of_DRAM();
+	ramend = memblock_end_of_DRAM();
 
 	/*
 	 * Sanity check any INITRD first. We don't take it into account
@@ -398,115 +319,61 @@  static void __init bootmem_init(void)
 	min_low_pfn = ~0UL;
 	max_low_pfn = 0;
 
-	/* Find the highest and lowest page frame numbers we have available. */
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		unsigned long start, end;
-
-		if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
-			continue;
+#ifdef CONFIG_MIPS_AUTO_PFN_OFFSET
+	min_low_pfn = PFN_UP(ramstart);
+	ARCH_PFN_OFFSET = PFN_UP(ramstart);
+#else
+	/*
+	 * Reserve any memory between the start of RAM and PHYS_OFFSET
+	 */
+	if (PFN_UP(ramstart) > PHYS_OFFSET)
+		memblock_reserve(PHYS_OFFSET, PFN_UP(ramstart) - PHYS_OFFSET);
 
-		start = PFN_UP(boot_mem_map.map[i].addr);
-		end = PFN_DOWN(boot_mem_map.map[i].addr
-				+ boot_mem_map.map[i].size);
+	if (PFN_UP(ramstart) > ARCH_PFN_OFFSET) {
+		pr_info("Wasting %lu bytes for tracking %lu unused pages\n",
+			(unsigned long)((PFN_UP(ramstart) - ARCH_PFN_OFFSET) * sizeof(struct page)),
+			(unsigned long)(PFN_UP(ramstart) - ARCH_PFN_OFFSET));
+	} else if (ARCH_PFN_OFFSET - PFN_UP(ramstart) > 0UL) {
+		pr_info("%lu free pages won't be used\n",
+			(unsigned long)(ARCH_PFN_OFFSET - PFN_UP(ramstart)));
+	}
+	min_low_pfn = ARCH_PFN_OFFSET;
+#endif
 
-		ramstart = min(ramstart, boot_mem_map.map[i].addr);
+	max_pfn = PFN_DOWN(ramend);
+	for_each_memblock(memory, mem) {
+		unsigned long start = memblock_region_memory_base_pfn(mem);
+		unsigned long end = memblock_region_memory_end_pfn(mem);
 
-#ifndef CONFIG_HIGHMEM
 		/*
 		 * Skip highmem here so we get an accurate max_low_pfn if low
 		 * memory stops short of high memory.
 		 * If the region overlaps HIGHMEM_START, end is clipped so
 		 * max_pfn excludes the highmem portion.
 		 */
+		if (memblock_is_nomap(mem))
+			continue;
 		if (start >= PFN_DOWN(HIGHMEM_START))
 			continue;
 		if (end > PFN_DOWN(HIGHMEM_START))
 			end = PFN_DOWN(HIGHMEM_START);
-#endif
-
 		if (end > max_low_pfn)
 			max_low_pfn = end;
-		if (start < min_low_pfn)
-			min_low_pfn = start;
 	}
 
-	if (min_low_pfn >= max_low_pfn)
-		panic("Incorrect memory mapping !!!");
-
-#ifdef CONFIG_MIPS_AUTO_PFN_OFFSET
-	ARCH_PFN_OFFSET = PFN_UP(ramstart);
-#else
-	/*
-	 * Reserve any memory between the start of RAM and PHYS_OFFSET
-	 */
-	if (ramstart > PHYS_OFFSET) {
-		add_memory_region(PHYS_OFFSET, ramstart - PHYS_OFFSET,
-				  BOOT_MEM_RESERVED);
-		memblock_reserve(PHYS_OFFSET, ramstart - PHYS_OFFSET);
-	}
-
-	if (min_low_pfn > ARCH_PFN_OFFSET) {
-		pr_info("Wasting %lu bytes for tracking %lu unused pages\n",
-			(min_low_pfn - ARCH_PFN_OFFSET) * sizeof(struct page),
-			min_low_pfn - ARCH_PFN_OFFSET);
-	} else if (ARCH_PFN_OFFSET - min_low_pfn > 0UL) {
-		pr_info("%lu free pages won't be used\n",
-			ARCH_PFN_OFFSET - min_low_pfn);
+#ifdef CONFIG_HIGHMEM
+	if (max_pfn > PFN_DOWN(HIGHMEM_START)) {
+		highstart_pfn = max_low_pfn;
+		highend_pfn = max_pfn;
 	}
-	min_low_pfn = ARCH_PFN_OFFSET;
 #endif
 
 	/*
-	 * Determine low and high memory ranges
+	 * In any case the added to the memblock memory regions
+	 * (highmem/lowmem, available/reserved, etc) are considered
+	 * as present, so inform sparsemem about them.
 	 */
-	max_pfn = max_low_pfn;
-	if (max_low_pfn > PFN_DOWN(HIGHMEM_START)) {
-#ifdef CONFIG_HIGHMEM
-		highstart_pfn = PFN_DOWN(HIGHMEM_START);
-		highend_pfn = max_low_pfn;
-#endif
-		max_low_pfn = PFN_DOWN(HIGHMEM_START);
-	}
-
-	/* Install all valid RAM ranges to the memblock memory region */
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		unsigned long start, end;
-
-		start = PFN_UP(boot_mem_map.map[i].addr);
-		end = PFN_DOWN(boot_mem_map.map[i].addr
-				+ boot_mem_map.map[i].size);
-
-		if (start < min_low_pfn)
-			start = min_low_pfn;
-#ifndef CONFIG_HIGHMEM
-		/* Ignore highmem regions if highmem is unsupported */
-		if (end > max_low_pfn)
-			end = max_low_pfn;
-#endif
-		if (end <= start)
-			continue;
-
-		memblock_add_node(PFN_PHYS(start), PFN_PHYS(end - start), 0);
-
-		/* Reserve any memory except the ordinary RAM ranges. */
-		switch (boot_mem_map.map[i].type) {
-		case BOOT_MEM_RAM:
-			break;
-		case BOOT_MEM_NOMAP: /* Discard the range from the system. */
-			memblock_remove(PFN_PHYS(start), PFN_PHYS(end - start));
-			continue;
-		default: /* Reserve the rest of the memory types at boot time */
-			memblock_reserve(PFN_PHYS(start), PFN_PHYS(end - start));
-			break;
-		}
-
-		/*
-		 * In any case the added to the memblock memory regions
-		 * (highmem/lowmem, available/reserved, etc) are considered
-		 * as present, so inform sparsemem about them.
-		 */
-		memory_present(0, start, end);
-	}
+	memblocks_present();
 
 	/*
 	 * Reserve initrd memory if needed.
@@ -528,8 +395,9 @@  static int __init early_parse_mem(char *p)
 	 * size.
 	 */
 	if (usermem == 0) {
-		boot_mem_map.nr_map = 0;
 		usermem = 1;
+		memblock_remove(memblock_start_of_DRAM(),
+			memblock_end_of_DRAM() - memblock_start_of_DRAM());
 	}
 	start = 0;
 	size = memparse(p, &p);
@@ -586,14 +454,13 @@  early_param("memmap", early_parse_memmap);
 unsigned long setup_elfcorehdr, setup_elfcorehdr_size;
 static int __init early_parse_elfcorehdr(char *p)
 {
-	int i;
+	struct memblock_region *mem;
 
 	setup_elfcorehdr = memparse(p, &p);
 
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		unsigned long start = boot_mem_map.map[i].addr;
-		unsigned long end = (boot_mem_map.map[i].addr +
-				     boot_mem_map.map[i].size);
+	 for_each_memblock(memory, mem) {
+		unsigned long start = mem->base;
+		unsigned long end = mem->end;
 		if (setup_elfcorehdr >= start && setup_elfcorehdr < end) {
 			/*
 			 * Reserve from the elf core header to the end of
@@ -613,47 +480,20 @@  static int __init early_parse_elfcorehdr(char *p)
 early_param("elfcorehdr", early_parse_elfcorehdr);
 #endif
 
-static void __init arch_mem_addpart(phys_addr_t mem, phys_addr_t end, int type)
-{
-	phys_addr_t size;
-	int i;
-
-	size = end - mem;
-	if (!size)
-		return;
-
-	/* Make sure it is in the boot_mem_map */
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		if (mem >= boot_mem_map.map[i].addr &&
-		    mem < (boot_mem_map.map[i].addr +
-			   boot_mem_map.map[i].size))
-			return;
-	}
-	add_memory_region(mem, size, type);
-}
-
 #ifdef CONFIG_KEXEC
-static inline unsigned long long get_total_mem(void)
-{
-	unsigned long long total;
-
-	total = max_pfn - min_low_pfn;
-	return total << PAGE_SHIFT;
-}
-
 static void __init mips_parse_crashkernel(void)
 {
 	unsigned long long total_mem;
 	unsigned long long crash_size, crash_base;
 	int ret;
 
-	total_mem = get_total_mem();
+	total_mem = memblock_phys_mem_size();
 	ret = parse_crashkernel(boot_command_line, total_mem,
 				&crash_size, &crash_base);
 	if (ret != 0 || crash_size <= 0)
 		return;
 
-	if (!memory_region_available(crash_base, crash_size)) {
+	if (!memblock_find_in_range(crash_base, crash_base + crash_size, crash_size, 0)) {
 		pr_warn("Invalid memory region reserved for crash kernel\n");
 		return;
 	}
@@ -686,6 +526,17 @@  static void __init request_crashkernel(struct resource *res)
 }
 #endif /* !defined(CONFIG_KEXEC)  */
 
+static void __init check_kernel_sections_mem(void)
+{
+	phys_addr_t start = PFN_DOWN(__pa_symbol(&_text)) << PAGE_SHIFT;
+	phys_addr_t size = (PFN_UP(__pa_symbol(&_end)) - start) << PAGE_SHIFT;
+
+	if (!memblock_is_region_memory(start, size)) {
+		pr_info("Kernel sections are not in the memory maps\n");
+		memblock_add(start, size);
+	}
+}
+
 #define USE_PROM_CMDLINE	IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_BOOTLOADER)
 #define USE_DTB_CMDLINE		IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_DTB)
 #define EXTEND_WITH_PROM	IS_ENABLED(CONFIG_MIPS_CMDLINE_DTB_EXTEND)
@@ -731,25 +582,6 @@  static void __init arch_mem_init(char **cmdline_p)
 	plat_mem_setup();
 	memblock_set_bottom_up(true);
 
-	/*
-	 * Make sure all kernel memory is in the maps.  The "UP" and
-	 * "DOWN" are opposite for initdata since if it crosses over
-	 * into another memory section you don't want that to be
-	 * freed when the initdata is freed.
-	 */
-	arch_mem_addpart(PFN_DOWN(__pa_symbol(&_text)) << PAGE_SHIFT,
-			 PFN_UP(__pa_symbol(&_edata)) << PAGE_SHIFT,
-			 BOOT_MEM_RAM);
-	arch_mem_addpart(PFN_UP(__pa_symbol(&__init_begin)) << PAGE_SHIFT,
-			 PFN_DOWN(__pa_symbol(&__init_end)) << PAGE_SHIFT,
-			 BOOT_MEM_INIT_RAM);
-	arch_mem_addpart(PFN_DOWN(__pa_symbol(&__bss_start)) << PAGE_SHIFT,
-			 PFN_UP(__pa_symbol(&__bss_stop)) << PAGE_SHIFT,
-			 BOOT_MEM_RAM);
-
-	pr_info("Determined physical RAM map:\n");
-	print_memory_map();
-
 #if defined(CONFIG_CMDLINE_BOOL) && defined(CONFIG_CMDLINE_OVERRIDE)
 	strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
 #else
@@ -783,14 +615,17 @@  static void __init arch_mem_init(char **cmdline_p)
 
 	parse_early_param();
 
-	if (usermem) {
-		pr_info("User-defined physical RAM map:\n");
-		print_memory_map();
-	}
+	if (usermem)
+		pr_info("User-defined physical RAM map overwrite\n");
+
+	check_kernel_sections_mem();
 
 	early_init_fdt_reserve_self();
 	early_init_fdt_scan_reserved_mem();
 
+#ifndef CONFIG_NUMA
+	memblock_set_node(0, PHYS_ADDR_MAX, &memblock.memory, 0);
+#endif
 	bootmem_init();
 
 	/*
@@ -835,7 +670,7 @@  static void __init arch_mem_init(char **cmdline_p)
 
 static void __init resource_init(void)
 {
-	int i;
+	struct memblock_region *region;
 
 	if (UNCAC_BASE != IO_BASE)
 		return;
@@ -847,16 +682,10 @@  static void __init resource_init(void)
 	bss_resource.start = __pa_symbol(&__bss_start);
 	bss_resource.end = __pa_symbol(&__bss_stop) - 1;
 
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
+	for_each_memblock(memory, region) {
+		phys_addr_t start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
+		phys_addr_t end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;
 		struct resource *res;
-		unsigned long start, end;
-
-		start = boot_mem_map.map[i].addr;
-		end = boot_mem_map.map[i].addr + boot_mem_map.map[i].size - 1;
-		if (start >= HIGHMEM_START)
-			continue;
-		if (end >= HIGHMEM_START)
-			end = HIGHMEM_START - 1;
 
 		res = memblock_alloc(sizeof(struct resource), SMP_CACHE_BYTES);
 		if (!res)
@@ -865,20 +694,7 @@  static void __init resource_init(void)
 
 		res->start = start;
 		res->end = end;
-		res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
-
-		switch (boot_mem_map.map[i].type) {
-		case BOOT_MEM_RAM:
-		case BOOT_MEM_INIT_RAM:
-		case BOOT_MEM_ROM_DATA:
-			res->name = "System RAM";
-			res->flags |= IORESOURCE_SYSRAM;
-			break;
-		case BOOT_MEM_RESERVED:
-		case BOOT_MEM_NOMAP:
-		default:
-			res->name = "reserved";
-		}
+		res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
 
 		request_resource(&iomem_resource, res);
 
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 8a038b30d3c4..2bb8ebf0d2d5 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -271,25 +271,17 @@  void __init fixrange_init(unsigned long start, unsigned long end,
 
 unsigned __weak platform_maar_init(unsigned num_pairs)
 {
-	struct maar_config cfg[BOOT_MEM_MAP_MAX];
-	unsigned i, num_configured, num_cfg = 0;
-
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		switch (boot_mem_map.map[i].type) {
-		case BOOT_MEM_RAM:
-		case BOOT_MEM_INIT_RAM:
-			break;
-		default:
-			continue;
-		}
+	struct maar_config cfg[32];
+	unsigned int num_configured, num_cfg = 0;
+	struct memblock_region *mem;
 
+	for_each_memblock(memory, mem) {
 		/* Round lower up */
-		cfg[num_cfg].lower = boot_mem_map.map[i].addr;
+		cfg[num_cfg].lower = __pfn_to_phys(memblock_region_memory_base_pfn(mem));
 		cfg[num_cfg].lower = (cfg[num_cfg].lower + 0xffff) & ~0xffff;
 
 		/* Round upper down */
-		cfg[num_cfg].upper = boot_mem_map.map[i].addr +
-					boot_mem_map.map[i].size;
+		cfg[num_cfg].upper = __pfn_to_phys(memblock_region_memory_end_pfn(mem));
 		cfg[num_cfg].upper = (cfg[num_cfg].upper & ~0xffff) - 1;
 
 		cfg[num_cfg].attrs = MIPS_MAAR_S;
@@ -382,33 +374,6 @@  void maar_init(void)
 }
 
 #ifndef CONFIG_NEED_MULTIPLE_NODES
-int page_is_ram(unsigned long pagenr)
-{
-	int i;
-
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		unsigned long addr, end;
-
-		switch (boot_mem_map.map[i].type) {
-		case BOOT_MEM_RAM:
-		case BOOT_MEM_INIT_RAM:
-			break;
-		default:
-			/* not usable memory */
-			continue;
-		}
-
-		addr = PFN_UP(boot_mem_map.map[i].addr);
-		end = PFN_DOWN(boot_mem_map.map[i].addr +
-			       boot_mem_map.map[i].size);
-
-		if (pagenr >= addr && pagenr < end)
-			return 1;
-	}
-
-	return 0;
-}
-
 void __init paging_init(void)
 {
 	unsigned long max_zone_pfns[MAX_NR_ZONES];
@@ -452,9 +417,7 @@  static inline void mem_init_free_highmem(void)
 		return;
 
 	for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
-		struct page *page = pfn_to_page(tmp);
-
-		if (!page_is_ram(tmp))
+		if (!memblock_is_memory(PFN_PHYS(tmp)))
 			SetPageReserved(page);
 		else
 			free_highmem_page(page);