diff mbox series

[kvm-unit-tests,v2,16/24] arm/arm64: Share memregions

Message ID 20240126142324.66674-42-andrew.jones@linux.dev (mailing list archive)
State New, archived
Headers show
Series Introduce RISC-V | expand

Commit Message

Andrew Jones Jan. 26, 2024, 2:23 p.m. UTC
arm/arm64 (and to a small extent powerpc) have memory regions which
get built from hardware descriptions (DT/ACPI/EFI) and then used to
build page tables. Move memregions to common code, tweaking the API
a bit at the same time, e.g. change 'mem_region' to 'memregions'.
The biggest change is there is now a default number of memory regions
which, if too small, should be overridden at setup time with a new
init function, memregions_init().

Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Acked-by: Thomas Huth <thuth@redhat.com>
---
 arm/Makefile.common |  1 +
 arm/selftest.c      |  3 +-
 lib/arm/asm/setup.h | 14 -------
 lib/arm/mmu.c       |  1 +
 lib/arm/setup.c     | 93 ++++++++++-----------------------------------
 lib/memregions.c    | 82 +++++++++++++++++++++++++++++++++++++++
 lib/memregions.h    | 29 ++++++++++++++
 7 files changed, 136 insertions(+), 87 deletions(-)
 create mode 100644 lib/memregions.c
 create mode 100644 lib/memregions.h

Comments

Eric Auger Feb. 1, 2024, 12:03 p.m. UTC | #1
Hi Drew,

On 1/26/24 15:23, Andrew Jones wrote:
> arm/arm64 (and to a small extent powerpc) have memory regions which
> get built from hardware descriptions (DT/ACPI/EFI) and then used to
> build page tables. Move memregions to common code, tweaking the API
> a bit at the same time, e.g. change 'mem_region' to 'memregions'.
> The biggest change is there is now a default number of memory regions
> which, if too small, should be overridden at setup time with a new
> init function, memregions_init().
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> Acked-by: Thomas Huth <thuth@redhat.com>
> ---
>  arm/Makefile.common |  1 +
>  arm/selftest.c      |  3 +-
>  lib/arm/asm/setup.h | 14 -------
>  lib/arm/mmu.c       |  1 +
>  lib/arm/setup.c     | 93 ++++++++++-----------------------------------
>  lib/memregions.c    | 82 +++++++++++++++++++++++++++++++++++++++
>  lib/memregions.h    | 29 ++++++++++++++
>  7 files changed, 136 insertions(+), 87 deletions(-)
>  create mode 100644 lib/memregions.c
>  create mode 100644 lib/memregions.h
>
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index dc92a7433350..4dfd570fa59e 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -42,6 +42,7 @@ cflatobjs += lib/alloc_page.o
>  cflatobjs += lib/vmalloc.o
>  cflatobjs += lib/alloc.o
>  cflatobjs += lib/devicetree.o
> +cflatobjs += lib/memregions.o
>  cflatobjs += lib/migrate.o
>  cflatobjs += lib/on-cpus.o
>  cflatobjs += lib/pci.o
> diff --git a/arm/selftest.c b/arm/selftest.c
> index 9f459ed3d571..007d2309d01c 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -8,6 +8,7 @@
>  #include <libcflat.h>
>  #include <util.h>
>  #include <devicetree.h>
> +#include <memregions.h>
>  #include <vmalloc.h>
>  #include <asm/setup.h>
>  #include <asm/ptrace.h>
> @@ -90,7 +91,7 @@ static bool check_pabt_init(void)
>  			highest_end = PAGE_ALIGN(r->end);
>  	}
>  
> -	if (mem_region_get_flags(highest_end) != MR_F_UNKNOWN)
> +	if (memregions_get_flags(highest_end) != MR_F_UNKNOWN)
>  		return false;
>  
>  	vaddr = (unsigned long)vmap(highest_end, PAGE_SIZE);
> diff --git a/lib/arm/asm/setup.h b/lib/arm/asm/setup.h
> index 060691165a20..9f8ef82efb90 100644
> --- a/lib/arm/asm/setup.h
> +++ b/lib/arm/asm/setup.h
> @@ -13,22 +13,8 @@
>  extern u64 cpus[NR_CPUS];	/* per-cpu IDs (MPIDRs) */
>  extern int nr_cpus;
>  
> -#define MR_F_IO			(1U << 0)
> -#define MR_F_CODE		(1U << 1)
> -#define MR_F_RESERVED		(1U << 2)
> -#define MR_F_UNKNOWN		(1U << 31)
> -
> -struct mem_region {
> -	phys_addr_t start;
> -	phys_addr_t end;
> -	unsigned int flags;
> -};
> -extern struct mem_region *mem_regions;
>  extern phys_addr_t __phys_offset, __phys_end;
>  
> -extern struct mem_region *mem_region_find(phys_addr_t paddr);
> -extern unsigned int mem_region_get_flags(phys_addr_t paddr);
> -
>  #define PHYS_OFFSET		(__phys_offset)
>  #define PHYS_END		(__phys_end)
>  
> diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
> index b16517a3200d..eb5e82a95f06 100644
> --- a/lib/arm/mmu.c
> +++ b/lib/arm/mmu.c
> @@ -6,6 +6,7 @@
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
>  #include <cpumask.h>
> +#include <memregions.h>
>  #include <asm/setup.h>
>  #include <asm/thread_info.h>
>  #include <asm/mmu.h>
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index b6fc453e5b31..0382cbdaf5a1 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -13,6 +13,7 @@
>  #include <libcflat.h>
>  #include <libfdt/libfdt.h>
>  #include <devicetree.h>
> +#include <memregions.h>
>  #include <alloc.h>
>  #include <alloc_phys.h>
>  #include <alloc_page.h>
> @@ -31,7 +32,7 @@
>  
>  #define MAX_DT_MEM_REGIONS	16
>  #define NR_EXTRA_MEM_REGIONS	64
> -#define NR_INITIAL_MEM_REGIONS	(MAX_DT_MEM_REGIONS + NR_EXTRA_MEM_REGIONS)
> +#define NR_MEM_REGIONS		(MAX_DT_MEM_REGIONS + NR_EXTRA_MEM_REGIONS)
>  
>  extern unsigned long _text, _etext, _data, _edata;
>  
> @@ -41,8 +42,7 @@ u32 initrd_size;
>  u64 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (u64)~0 };
>  int nr_cpus;
>  
> -static struct mem_region __initial_mem_regions[NR_INITIAL_MEM_REGIONS + 1];
> -struct mem_region *mem_regions = __initial_mem_regions;
> +static struct mem_region arm_mem_regions[NR_MEM_REGIONS + 1];
>  phys_addr_t __phys_offset = (phys_addr_t)-1, __phys_end = 0;
>  
>  extern void exceptions_init(void);
> @@ -114,68 +114,14 @@ static void cpu_init(void)
>  	set_cpu_online(0, true);
>  }
>  
> -static void mem_region_add(struct mem_region *r)
> +static void arm_memregions_add_assumed(void)
>  {
> -	struct mem_region *r_next = mem_regions;
> -	int i = 0;
> -
> -	for (; r_next->end; ++r_next, ++i)
> -		;
> -	assert(i < NR_INITIAL_MEM_REGIONS);
> -
> -	*r_next = *r;
> -}
> -
> -static void mem_regions_add_dt_regions(void)
> -{
> -	struct dt_pbus_reg regs[MAX_DT_MEM_REGIONS];
> -	int nr_regs, i;
> -
> -	nr_regs = dt_get_memory_params(regs, MAX_DT_MEM_REGIONS);
> -	assert(nr_regs > 0);
> -
> -	for (i = 0; i < nr_regs; ++i) {
> -		mem_region_add(&(struct mem_region){
> -			.start = regs[i].addr,
> -			.end = regs[i].addr + regs[i].size,
> -		});
> -	}
> -}
> -
> -struct mem_region *mem_region_find(phys_addr_t paddr)
> -{
> -	struct mem_region *r;
> -
> -	for (r = mem_regions; r->end; ++r)
> -		if (paddr >= r->start && paddr < r->end)
> -			return r;
> -	return NULL;
> -}
> -
> -unsigned int mem_region_get_flags(phys_addr_t paddr)
> -{
> -	struct mem_region *r = mem_region_find(paddr);
> -	return r ? r->flags : MR_F_UNKNOWN;
> -}
> -
> -static void mem_regions_add_assumed(void)
> -{
> -	phys_addr_t code_end = (phys_addr_t)(unsigned long)&_etext;
> -	struct mem_region *r;
> -
> -	r = mem_region_find(code_end - 1);
> -	assert(r);
> +	struct mem_region *code, *data;
>  
>  	/* Split the region with the code into two regions; code and data */
> -	mem_region_add(&(struct mem_region){
> -		.start = code_end,
> -		.end = r->end,
> -	});
> -	*r = (struct mem_region){
> -		.start = r->start,
> -		.end = code_end,
> -		.flags = MR_F_CODE,
> -	};
> +	memregions_split((unsigned long)&_etext, &code, &data);
> +	assert(code);
> +	code->flags |= MR_F_CODE;
I think this would deserve to be split into several patches, esp. this
change in the implementation of

mem_regions_add_assumed and the init changes. At the moment this is pretty difficult to review

Eric

>  
>  	/*
>  	 * mach-virt I/O regions:
> @@ -183,10 +129,10 @@ static void mem_regions_add_assumed(void)
>  	 *   - 512M at 256G (arm64, arm uses highmem=off)
>  	 *   - 512G at 512G (arm64, arm uses highmem=off)
>  	 */
> -	mem_region_add(&(struct mem_region){ 0, (1ul << 30), MR_F_IO });
> +	memregions_add(&(struct mem_region){ 0, (1ul << 30), MR_F_IO });
>  #ifdef __aarch64__
> -	mem_region_add(&(struct mem_region){ (1ul << 38), (1ul << 38) | (1ul << 29), MR_F_IO });
> -	mem_region_add(&(struct mem_region){ (1ul << 39), (1ul << 40), MR_F_IO });
> +	memregions_add(&(struct mem_region){ (1ul << 38), (1ul << 38) | (1ul << 29), MR_F_IO });
> +	memregions_add(&(struct mem_region){ (1ul << 39), (1ul << 40), MR_F_IO });
>  #endif
>  }
>  
> @@ -197,7 +143,7 @@ static void mem_init(phys_addr_t freemem_start)
>  		.start = (phys_addr_t)-1,
>  	};
>  
> -	freemem = mem_region_find(freemem_start);
> +	freemem = memregions_find(freemem_start);
>  	assert(freemem && !(freemem->flags & (MR_F_IO | MR_F_CODE)));
>  
>  	for (r = mem_regions; r->end; ++r) {
> @@ -212,9 +158,9 @@ static void mem_init(phys_addr_t freemem_start)
>  	mem.end &= PHYS_MASK;
>  
>  	/* Check for holes */
> -	r = mem_region_find(mem.start);
> +	r = memregions_find(mem.start);
>  	while (r && r->end != mem.end)
> -		r = mem_region_find(r->end);
> +		r = memregions_find(r->end);
>  	assert(r);
>  
>  	/* Ensure our selected freemem range is somewhere in our full range */
> @@ -263,8 +209,9 @@ void setup(const void *fdt, phys_addr_t freemem_start)
>  		freemem += initrd_size;
>  	}
>  
> -	mem_regions_add_dt_regions();
> -	mem_regions_add_assumed();
> +	memregions_init(arm_mem_regions, NR_MEM_REGIONS);
> +	memregions_add_dt_regions(MAX_DT_MEM_REGIONS);
> +	arm_memregions_add_assumed();
>  	mem_init(PAGE_ALIGN((unsigned long)freemem));
>  
>  	psci_set_conduit();
> @@ -371,7 +318,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
>  				assert(edata <= r.end);
>  				r.flags = MR_F_CODE;
>  				r.end = data;
> -				mem_region_add(&r);
> +				memregions_add(&r);
>  				r.start = data;
>  				r.end = tmp;
>  				r.flags = 0;
> @@ -393,7 +340,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
>  			if (r.end > __phys_end)
>  				__phys_end = r.end;
>  		}
> -		mem_region_add(&r);
> +		memregions_add(&r);
>  	}
>  	if (fdt) {
>  		/* Move the FDT to the base of free memory */
> @@ -439,6 +386,8 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
>  
>  	exceptions_init();
>  
> +	memregions_init(arm_mem_regions, NR_MEM_REGIONS);
> +
>  	status = efi_mem_init(efi_bootinfo);
>  	if (status != EFI_SUCCESS) {
>  		printf("Failed to initialize memory: ");
> diff --git a/lib/memregions.c b/lib/memregions.c
> new file mode 100644
> index 000000000000..96de86b27333
> --- /dev/null
> +++ b/lib/memregions.c
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <libcflat.h>
> +#include <devicetree.h>
> +#include <memregions.h>
> +
> +static struct mem_region __initial_mem_regions[NR_INITIAL_MEM_REGIONS + 1];
> +static size_t nr_regions = NR_INITIAL_MEM_REGIONS;
> +
> +struct mem_region *mem_regions = __initial_mem_regions;
> +
> +void memregions_init(struct mem_region regions[], size_t nr)
> +{
> +	mem_regions = regions;
> +	nr_regions = nr;
> +}
> +
> +struct mem_region *memregions_add(struct mem_region *r)
> +{
> +	struct mem_region *r_next = mem_regions;
> +	int i = 0;
> +
> +	for (; r_next->end; ++r_next, ++i)
> +		;
> +	assert(i < nr_regions);
> +
> +	*r_next = *r;
> +
> +	return r_next;
> +}
> +
> +struct mem_region *memregions_find(phys_addr_t paddr)
> +{
> +	struct mem_region *r;
> +
> +	for (r = mem_regions; r->end; ++r)
> +		if (paddr >= r->start && paddr < r->end)
> +			return r;
> +	return NULL;
> +}
> +
> +uint32_t memregions_get_flags(phys_addr_t paddr)
> +{
> +	struct mem_region *r = memregions_find(paddr);
> +
> +	return r ? r->flags : MR_F_UNKNOWN;
> +}
> +
> +void memregions_split(phys_addr_t addr, struct mem_region **r1, struct mem_region **r2)
> +{
> +	*r1 = memregions_find(addr);
> +	assert(*r1);
> +
> +	if ((*r1)->start == addr) {
> +		*r2 = *r1;
> +		*r1 = NULL;
> +		return;
> +	}
> +
> +	*r2 = memregions_add(&(struct mem_region){
> +		.start = addr,
> +		.end = (*r1)->end,
> +		.flags = (*r1)->flags,
> +	});
> +
> +	(*r1)->end = addr;
> +}
> +
> +void memregions_add_dt_regions(size_t max_nr)
> +{
> +	struct dt_pbus_reg regs[max_nr];
> +	int nr_regs, i;
> +
> +	nr_regs = dt_get_memory_params(regs, max_nr);
> +	assert(nr_regs > 0);
> +
> +	for (i = 0; i < nr_regs; ++i) {
> +		memregions_add(&(struct mem_region){
> +			.start = regs[i].addr,
> +			.end = regs[i].addr + regs[i].size,
> +		});
> +	}
> +}
> diff --git a/lib/memregions.h b/lib/memregions.h
> new file mode 100644
> index 000000000000..9a8e33182fe5
> --- /dev/null
> +++ b/lib/memregions.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _MEMREGIONS_H_
> +#define _MEMREGIONS_H_
> +#include <libcflat.h>
> +#include <bitops.h>
> +
> +#define NR_INITIAL_MEM_REGIONS		8
> +
> +#define MR_F_IO				BIT(0)
> +#define MR_F_CODE			BIT(1)
> +#define MR_F_RESERVED			BIT(2)
> +#define MR_F_UNKNOWN			BIT(31)
> +
> +struct mem_region {
> +	phys_addr_t start;
> +	phys_addr_t end;
> +	uint32_t flags;
> +};
> +
> +extern struct mem_region *mem_regions;
> +
> +void memregions_init(struct mem_region regions[], size_t nr);
> +struct mem_region *memregions_add(struct mem_region *r);
> +struct mem_region *memregions_find(phys_addr_t paddr);
> +uint32_t memregions_get_flags(phys_addr_t paddr);
> +void memregions_split(phys_addr_t addr, struct mem_region **r1, struct mem_region **r2);
> +void memregions_add_dt_regions(size_t max_nr);
> +
> +#endif /* _MEMREGIONS_H_ */
Andrew Jones Feb. 1, 2024, 2:21 p.m. UTC | #2
On Thu, Feb 01, 2024 at 01:03:54PM +0100, Eric Auger wrote:
> Hi Drew,
> 
> On 1/26/24 15:23, Andrew Jones wrote:
...
> > -static void mem_regions_add_assumed(void)
> > -{
> > -	phys_addr_t code_end = (phys_addr_t)(unsigned long)&_etext;
> > -	struct mem_region *r;
> > -
> > -	r = mem_region_find(code_end - 1);
> > -	assert(r);
> > +	struct mem_region *code, *data;
> >  
> >  	/* Split the region with the code into two regions; code and data */
> > -	mem_region_add(&(struct mem_region){
> > -		.start = code_end,
> > -		.end = r->end,
> > -	});
> > -	*r = (struct mem_region){
> > -		.start = r->start,
> > -		.end = code_end,
> > -		.flags = MR_F_CODE,
> > -	};
> > +	memregions_split((unsigned long)&_etext, &code, &data);
> > +	assert(code);
> > +	code->flags |= MR_F_CODE;
> I think this would deserve to be split into several patches, esp. this
> change in the implementation of
> 
> mem_regions_add_assumed and the init changes. At the moment this is pretty difficult to review
>

Darn, you called me out on this one :-) I had a feeling I should split out
the introduction of memregions_split(), since it was sneaking a bit more
into the patch than just code motion as advertised, but then I hoped I
get away with putting a bit more burden on the reviewer instead. If you
haven't already convinced yourself that the new function is equivalent to
the old code, then I'll respin with the splitting and also create a new
patch for the 'mem_region' to 'memregions' rename while at it (so there
will be three patches instead of one). But, if you're already good with
it, then I'll leave it as is, since patch splitting is a pain...

Thanks,
drew
Eric Auger Feb. 1, 2024, 5:46 p.m. UTC | #3
On 2/1/24 15:21, Andrew Jones wrote:
> On Thu, Feb 01, 2024 at 01:03:54PM +0100, Eric Auger wrote:
>> Hi Drew,
>>
>> On 1/26/24 15:23, Andrew Jones wrote:
> ...
>>> -static void mem_regions_add_assumed(void)
>>> -{
>>> -	phys_addr_t code_end = (phys_addr_t)(unsigned long)&_etext;
>>> -	struct mem_region *r;
>>> -
>>> -	r = mem_region_find(code_end - 1);
>>> -	assert(r);
>>> +	struct mem_region *code, *data;
>>>  
>>>  	/* Split the region with the code into two regions; code and data */
>>> -	mem_region_add(&(struct mem_region){
>>> -		.start = code_end,
>>> -		.end = r->end,
>>> -	});
>>> -	*r = (struct mem_region){
>>> -		.start = r->start,
>>> -		.end = code_end,
>>> -		.flags = MR_F_CODE,
>>> -	};
>>> +	memregions_split((unsigned long)&_etext, &code, &data);
>>> +	assert(code);
>>> +	code->flags |= MR_F_CODE;
>> I think this would deserve to be split into several patches, esp. this
>> change in the implementation of
>>
>> mem_regions_add_assumed and the init changes. At the moment this is pretty difficult to review
>>
> Darn, you called me out on this one :-) I had a feeling I should split out
> the introduction of memregions_split(), since it was sneaking a bit more
> into the patch than just code motion as advertised, but then I hoped I
> get away with putting a bit more burden on the reviewer instead. If you
> haven't already convinced yourself that the new function is equivalent to
> the old code, then I'll respin with the splitting and also create a new
> patch for the 'mem_region' to 'memregions' rename while at it (so there
> will be three patches instead of one). But, if you're already good with
> it, then I'll leave it as is, since patch splitting is a pain...
frankly I would prefer you split. But maybe somebody smarter than me
will be able to review as is, maybe just wait a little bit until you
respin ;-)

Eric
>
> Thanks,
> drew
>
diff mbox series

Patch

diff --git a/arm/Makefile.common b/arm/Makefile.common
index dc92a7433350..4dfd570fa59e 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -42,6 +42,7 @@  cflatobjs += lib/alloc_page.o
 cflatobjs += lib/vmalloc.o
 cflatobjs += lib/alloc.o
 cflatobjs += lib/devicetree.o
+cflatobjs += lib/memregions.o
 cflatobjs += lib/migrate.o
 cflatobjs += lib/on-cpus.o
 cflatobjs += lib/pci.o
diff --git a/arm/selftest.c b/arm/selftest.c
index 9f459ed3d571..007d2309d01c 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -8,6 +8,7 @@ 
 #include <libcflat.h>
 #include <util.h>
 #include <devicetree.h>
+#include <memregions.h>
 #include <vmalloc.h>
 #include <asm/setup.h>
 #include <asm/ptrace.h>
@@ -90,7 +91,7 @@  static bool check_pabt_init(void)
 			highest_end = PAGE_ALIGN(r->end);
 	}
 
-	if (mem_region_get_flags(highest_end) != MR_F_UNKNOWN)
+	if (memregions_get_flags(highest_end) != MR_F_UNKNOWN)
 		return false;
 
 	vaddr = (unsigned long)vmap(highest_end, PAGE_SIZE);
diff --git a/lib/arm/asm/setup.h b/lib/arm/asm/setup.h
index 060691165a20..9f8ef82efb90 100644
--- a/lib/arm/asm/setup.h
+++ b/lib/arm/asm/setup.h
@@ -13,22 +13,8 @@ 
 extern u64 cpus[NR_CPUS];	/* per-cpu IDs (MPIDRs) */
 extern int nr_cpus;
 
-#define MR_F_IO			(1U << 0)
-#define MR_F_CODE		(1U << 1)
-#define MR_F_RESERVED		(1U << 2)
-#define MR_F_UNKNOWN		(1U << 31)
-
-struct mem_region {
-	phys_addr_t start;
-	phys_addr_t end;
-	unsigned int flags;
-};
-extern struct mem_region *mem_regions;
 extern phys_addr_t __phys_offset, __phys_end;
 
-extern struct mem_region *mem_region_find(phys_addr_t paddr);
-extern unsigned int mem_region_get_flags(phys_addr_t paddr);
-
 #define PHYS_OFFSET		(__phys_offset)
 #define PHYS_END		(__phys_end)
 
diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
index b16517a3200d..eb5e82a95f06 100644
--- a/lib/arm/mmu.c
+++ b/lib/arm/mmu.c
@@ -6,6 +6,7 @@ 
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 #include <cpumask.h>
+#include <memregions.h>
 #include <asm/setup.h>
 #include <asm/thread_info.h>
 #include <asm/mmu.h>
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index b6fc453e5b31..0382cbdaf5a1 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -13,6 +13,7 @@ 
 #include <libcflat.h>
 #include <libfdt/libfdt.h>
 #include <devicetree.h>
+#include <memregions.h>
 #include <alloc.h>
 #include <alloc_phys.h>
 #include <alloc_page.h>
@@ -31,7 +32,7 @@ 
 
 #define MAX_DT_MEM_REGIONS	16
 #define NR_EXTRA_MEM_REGIONS	64
-#define NR_INITIAL_MEM_REGIONS	(MAX_DT_MEM_REGIONS + NR_EXTRA_MEM_REGIONS)
+#define NR_MEM_REGIONS		(MAX_DT_MEM_REGIONS + NR_EXTRA_MEM_REGIONS)
 
 extern unsigned long _text, _etext, _data, _edata;
 
@@ -41,8 +42,7 @@  u32 initrd_size;
 u64 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (u64)~0 };
 int nr_cpus;
 
-static struct mem_region __initial_mem_regions[NR_INITIAL_MEM_REGIONS + 1];
-struct mem_region *mem_regions = __initial_mem_regions;
+static struct mem_region arm_mem_regions[NR_MEM_REGIONS + 1];
 phys_addr_t __phys_offset = (phys_addr_t)-1, __phys_end = 0;
 
 extern void exceptions_init(void);
@@ -114,68 +114,14 @@  static void cpu_init(void)
 	set_cpu_online(0, true);
 }
 
-static void mem_region_add(struct mem_region *r)
+static void arm_memregions_add_assumed(void)
 {
-	struct mem_region *r_next = mem_regions;
-	int i = 0;
-
-	for (; r_next->end; ++r_next, ++i)
-		;
-	assert(i < NR_INITIAL_MEM_REGIONS);
-
-	*r_next = *r;
-}
-
-static void mem_regions_add_dt_regions(void)
-{
-	struct dt_pbus_reg regs[MAX_DT_MEM_REGIONS];
-	int nr_regs, i;
-
-	nr_regs = dt_get_memory_params(regs, MAX_DT_MEM_REGIONS);
-	assert(nr_regs > 0);
-
-	for (i = 0; i < nr_regs; ++i) {
-		mem_region_add(&(struct mem_region){
-			.start = regs[i].addr,
-			.end = regs[i].addr + regs[i].size,
-		});
-	}
-}
-
-struct mem_region *mem_region_find(phys_addr_t paddr)
-{
-	struct mem_region *r;
-
-	for (r = mem_regions; r->end; ++r)
-		if (paddr >= r->start && paddr < r->end)
-			return r;
-	return NULL;
-}
-
-unsigned int mem_region_get_flags(phys_addr_t paddr)
-{
-	struct mem_region *r = mem_region_find(paddr);
-	return r ? r->flags : MR_F_UNKNOWN;
-}
-
-static void mem_regions_add_assumed(void)
-{
-	phys_addr_t code_end = (phys_addr_t)(unsigned long)&_etext;
-	struct mem_region *r;
-
-	r = mem_region_find(code_end - 1);
-	assert(r);
+	struct mem_region *code, *data;
 
 	/* Split the region with the code into two regions; code and data */
-	mem_region_add(&(struct mem_region){
-		.start = code_end,
-		.end = r->end,
-	});
-	*r = (struct mem_region){
-		.start = r->start,
-		.end = code_end,
-		.flags = MR_F_CODE,
-	};
+	memregions_split((unsigned long)&_etext, &code, &data);
+	assert(code);
+	code->flags |= MR_F_CODE;
 
 	/*
 	 * mach-virt I/O regions:
@@ -183,10 +129,10 @@  static void mem_regions_add_assumed(void)
 	 *   - 512M at 256G (arm64, arm uses highmem=off)
 	 *   - 512G at 512G (arm64, arm uses highmem=off)
 	 */
-	mem_region_add(&(struct mem_region){ 0, (1ul << 30), MR_F_IO });
+	memregions_add(&(struct mem_region){ 0, (1ul << 30), MR_F_IO });
 #ifdef __aarch64__
-	mem_region_add(&(struct mem_region){ (1ul << 38), (1ul << 38) | (1ul << 29), MR_F_IO });
-	mem_region_add(&(struct mem_region){ (1ul << 39), (1ul << 40), MR_F_IO });
+	memregions_add(&(struct mem_region){ (1ul << 38), (1ul << 38) | (1ul << 29), MR_F_IO });
+	memregions_add(&(struct mem_region){ (1ul << 39), (1ul << 40), MR_F_IO });
 #endif
 }
 
@@ -197,7 +143,7 @@  static void mem_init(phys_addr_t freemem_start)
 		.start = (phys_addr_t)-1,
 	};
 
-	freemem = mem_region_find(freemem_start);
+	freemem = memregions_find(freemem_start);
 	assert(freemem && !(freemem->flags & (MR_F_IO | MR_F_CODE)));
 
 	for (r = mem_regions; r->end; ++r) {
@@ -212,9 +158,9 @@  static void mem_init(phys_addr_t freemem_start)
 	mem.end &= PHYS_MASK;
 
 	/* Check for holes */
-	r = mem_region_find(mem.start);
+	r = memregions_find(mem.start);
 	while (r && r->end != mem.end)
-		r = mem_region_find(r->end);
+		r = memregions_find(r->end);
 	assert(r);
 
 	/* Ensure our selected freemem range is somewhere in our full range */
@@ -263,8 +209,9 @@  void setup(const void *fdt, phys_addr_t freemem_start)
 		freemem += initrd_size;
 	}
 
-	mem_regions_add_dt_regions();
-	mem_regions_add_assumed();
+	memregions_init(arm_mem_regions, NR_MEM_REGIONS);
+	memregions_add_dt_regions(MAX_DT_MEM_REGIONS);
+	arm_memregions_add_assumed();
 	mem_init(PAGE_ALIGN((unsigned long)freemem));
 
 	psci_set_conduit();
@@ -371,7 +318,7 @@  static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
 				assert(edata <= r.end);
 				r.flags = MR_F_CODE;
 				r.end = data;
-				mem_region_add(&r);
+				memregions_add(&r);
 				r.start = data;
 				r.end = tmp;
 				r.flags = 0;
@@ -393,7 +340,7 @@  static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
 			if (r.end > __phys_end)
 				__phys_end = r.end;
 		}
-		mem_region_add(&r);
+		memregions_add(&r);
 	}
 	if (fdt) {
 		/* Move the FDT to the base of free memory */
@@ -439,6 +386,8 @@  efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 
 	exceptions_init();
 
+	memregions_init(arm_mem_regions, NR_MEM_REGIONS);
+
 	status = efi_mem_init(efi_bootinfo);
 	if (status != EFI_SUCCESS) {
 		printf("Failed to initialize memory: ");
diff --git a/lib/memregions.c b/lib/memregions.c
new file mode 100644
index 000000000000..96de86b27333
--- /dev/null
+++ b/lib/memregions.c
@@ -0,0 +1,82 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+#include <libcflat.h>
+#include <devicetree.h>
+#include <memregions.h>
+
+static struct mem_region __initial_mem_regions[NR_INITIAL_MEM_REGIONS + 1];
+static size_t nr_regions = NR_INITIAL_MEM_REGIONS;
+
+struct mem_region *mem_regions = __initial_mem_regions;
+
+void memregions_init(struct mem_region regions[], size_t nr)
+{
+	mem_regions = regions;
+	nr_regions = nr;
+}
+
+struct mem_region *memregions_add(struct mem_region *r)
+{
+	struct mem_region *r_next = mem_regions;
+	int i = 0;
+
+	for (; r_next->end; ++r_next, ++i)
+		;
+	assert(i < nr_regions);
+
+	*r_next = *r;
+
+	return r_next;
+}
+
+struct mem_region *memregions_find(phys_addr_t paddr)
+{
+	struct mem_region *r;
+
+	for (r = mem_regions; r->end; ++r)
+		if (paddr >= r->start && paddr < r->end)
+			return r;
+	return NULL;
+}
+
+uint32_t memregions_get_flags(phys_addr_t paddr)
+{
+	struct mem_region *r = memregions_find(paddr);
+
+	return r ? r->flags : MR_F_UNKNOWN;
+}
+
+void memregions_split(phys_addr_t addr, struct mem_region **r1, struct mem_region **r2)
+{
+	*r1 = memregions_find(addr);
+	assert(*r1);
+
+	if ((*r1)->start == addr) {
+		*r2 = *r1;
+		*r1 = NULL;
+		return;
+	}
+
+	*r2 = memregions_add(&(struct mem_region){
+		.start = addr,
+		.end = (*r1)->end,
+		.flags = (*r1)->flags,
+	});
+
+	(*r1)->end = addr;
+}
+
+void memregions_add_dt_regions(size_t max_nr)
+{
+	struct dt_pbus_reg regs[max_nr];
+	int nr_regs, i;
+
+	nr_regs = dt_get_memory_params(regs, max_nr);
+	assert(nr_regs > 0);
+
+	for (i = 0; i < nr_regs; ++i) {
+		memregions_add(&(struct mem_region){
+			.start = regs[i].addr,
+			.end = regs[i].addr + regs[i].size,
+		});
+	}
+}
diff --git a/lib/memregions.h b/lib/memregions.h
new file mode 100644
index 000000000000..9a8e33182fe5
--- /dev/null
+++ b/lib/memregions.h
@@ -0,0 +1,29 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _MEMREGIONS_H_
+#define _MEMREGIONS_H_
+#include <libcflat.h>
+#include <bitops.h>
+
+#define NR_INITIAL_MEM_REGIONS		8
+
+#define MR_F_IO				BIT(0)
+#define MR_F_CODE			BIT(1)
+#define MR_F_RESERVED			BIT(2)
+#define MR_F_UNKNOWN			BIT(31)
+
+struct mem_region {
+	phys_addr_t start;
+	phys_addr_t end;
+	uint32_t flags;
+};
+
+extern struct mem_region *mem_regions;
+
+void memregions_init(struct mem_region regions[], size_t nr);
+struct mem_region *memregions_add(struct mem_region *r);
+struct mem_region *memregions_find(phys_addr_t paddr);
+uint32_t memregions_get_flags(phys_addr_t paddr);
+void memregions_split(phys_addr_t addr, struct mem_region **r1, struct mem_region **r2);
+void memregions_add_dt_regions(size_t max_nr);
+
+#endif /* _MEMREGIONS_H_ */