devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array.
@ 2023-12-04  4:13 Oreoluwa Babatunde
  2023-12-04 18:54 ` Oreoluwa Babatunde
  0 siblings, 1 reply; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-04  4:13 UTC (permalink / raw)
  To: catalin.marinas, will, robh+dt, frowand.list
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel,
	Oreoluwa Babatunde

The reserved_mem array is used to store the data of the different
reserved memory regions specified in the DT of a device.
The array stores information such as the name, node, starting address,
and size of a reserved memory region.

The array is currently statically allocated with a size of
MAX_RESERVED_REGIONS(64). This means that any system that specifies a
number of reserved memory regions greater than MAX_RESERVED_REGIONS(64)
will not have enough space to store the information for all the regions.

Therefore, this series extends the use of a static array for
reserved_mem, and introduces a dynamically allocated array using
memblock_alloc() based on the number of reserved memory regions
specified in the DT.

Memory gotten from memblock_alloc() is only writable after paging_init()
is called, but the reserved memory regions need to be reserved before
then so that the system does not create page table mappings for them.

Reserved memory regions can be divided into 2 groups.
i) Statically-placed reserved memory regions
i.e. regions defined in the DT using the @reg property.
ii) Dynamically-placed reserved memory regions.
i.e. regions specified in the DT using the @alloc_ranges
    and @size properties.

It is possible to call memblock_reserve() and memblock_mark_nomap() on
the statically-placed reserved memory regions and not need to save them
to the array until after paging_init(), but this is not possible for the
dynamically-placed reserved memory because the starting address of these
regions need to be stored somewhere after they are allocated.

Therefore, this series achieves the allocation and population of the
reserved_mem array in two steps:

1. Before paging_init()
   Before paging_init() is called, iterate through the reserved_mem
   nodes in the DT and do the following:
   - Allocate memory for dynamically-placed reserved memory regions and
     store their starting address in the static allocated reserved_mem
     array.
   - Call memblock_reserve() and memblock_mark_nomap() on all the
     reserved memory regions as needed.
   - Count the total number of reserved_mem nodes in the DT.

2. After paging_init()
   After paging_init() is called:
   - Allocate new memory for the reserved_mem array based on the number
     of reserved memory nodes in the DT.
   - Transfer all the information that was stored in the static array
     into the new array.
   - Store the rest of the reserved_mem regions in the new array.
     i.e. the statically-placed regions.

The static array is no longer needed after this point, but there is
currently no obvious way to free the memory. Therefore, the size of the
initial static array is now defined using a config option.
Because the array is used only before paging_init() to store the
dynamically-placed reserved memory regions, the required size can vary
from device to device. Therefore, scaling it can help get some memory
savings.

A possible solution to freeing the memory for the static array will be
to mark it as __initdata. This will automatically free the memory once
the init process is done running.
The reason why this is not pursued in this series is because of
the possibility of a use-after-free.
If the dynamic allocation of the reserved_mem array fails, then future
accesses of the reserved_mem array will still be referencing the static
array. When the init process ends and the memory is freed up, any
further attempts to use the reserved_mem array will result in a
use-after-free.

Note:

- The limitation to this approach is that there is still a limit of
  64 for dynamically reserved memory regions.
- Upon further review, the series might need to be split up/duplicated
  for other archs.


Oreoluwa Babatunde (6):
  of: reserved_mem: Change the order that reserved_mem regions are
    stored
  of: reserved_mem: Swicth call to unflatten_device_tree() to after
    paging_init()
  of: resevred_mem: Delay allocation of memory for dynamic regions
  of: reserved_mem: Add code to use unflattened DT for reserved_mem
    nodes
  of: reserved_mem: Add code to dynamically allocate reserved_mem array
  of: reserved_mem: Make MAX_RESERVED_REGIONS a config option

 arch/loongarch/kernel/setup.c      |   2 +-
 arch/mips/kernel/setup.c           |   3 +-
 arch/nios2/kernel/setup.c          |   4 +-
 arch/openrisc/kernel/setup.c       |   4 +-
 arch/powerpc/kernel/setup-common.c |   3 +
 arch/sh/kernel/setup.c             |   5 +-
 arch/um/kernel/dtb.c               |   1 -
 arch/um/kernel/um_arch.c           |   2 +
 arch/xtensa/kernel/setup.c         |   4 +-
 drivers/of/Kconfig                 |  13 +++
 drivers/of/fdt.c                   |  39 +++++--
 drivers/of/of_private.h            |   6 +-
 drivers/of/of_reserved_mem.c       | 175 +++++++++++++++++++++--------
 include/linux/of_reserved_mem.h    |   8 +-
 kernel/dma/coherent.c              |   4 +-
 kernel/dma/contiguous.c            |   8 +-
 kernel/dma/swiotlb.c               |  10 +-
 17 files changed, 205 insertions(+), 86 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array.
@ 2023-12-04 18:54 Oreoluwa Babatunde
  2023-12-04 18:54 ` [RFC PATCH v2 1/6] of: reserved_mem: Change the order that reserved_mem regions are stored Oreoluwa Babatunde
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-04 18:54 UTC (permalink / raw)
  To: catalin.marinas, will, robh+dt, frowand.list, dinguyen,
	chenhuacai, tsbogend, jonas, stefan.kristiansson, shorne, mpe,
	ysato, dalias, glaubitz, richard, anton.ivanov, johannes, chris,
	jcmvbkbc
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel,
	Oreoluwa Babatunde

The reserved_mem array is used to store the data of the different
reserved memory regions specified in the DT of a device.
The array stores information such as the name, node, starting address,
and size of a reserved memory region.

The array is currently statically allocated with a size of
MAX_RESERVED_REGIONS(64). This means that any system that specifies a
number of reserved memory regions greater than MAX_RESERVED_REGIONS(64)
will not have enough space to store the information for all the regions.

Therefore, this series extends the use of a static array for
reserved_mem, and introduces a dynamically allocated array using
memblock_alloc() based on the number of reserved memory regions
specified in the DT.

Memory gotten from memblock_alloc() is only writable after paging_init()
is called, but the reserved memory regions need to be reserved before
then so that the system does not create page table mappings for them.

Reserved memory regions can be divided into 2 groups.
i) Statically-placed reserved memory regions
i.e. regions defined in the DT using the @reg property.
ii) Dynamically-placed reserved memory regions.
i.e. regions specified in the DT using the @alloc_ranges
    and @size properties.

It is possible to call memblock_reserve() and memblock_mark_nomap() on
the statically-placed reserved memory regions and not need to save them
to the array until after paging_init(), but this is not possible for the
dynamically-placed reserved memory because the starting address of these
regions need to be stored somewhere after they are allocated.

Therefore, this series achieves the allocation and population of the
reserved_mem array in two steps:

1. Before paging_init()
   Before paging_init() is called, iterate through the reserved_mem
   nodes in the DT and do the following:
   - Allocate memory for dynamically-placed reserved memory regions and
     store their starting address in the static allocated reserved_mem
     array.
   - Call memblock_reserve() and memblock_mark_nomap() on all the
     reserved memory regions as needed.
   - Count the total number of reserved_mem nodes in the DT.

2. After paging_init()
   After paging_init() is called:
   - Allocate new memory for the reserved_mem array based on the number
     of reserved memory nodes in the DT.
   - Transfer all the information that was stored in the static array
     into the new array.
   - Store the rest of the reserved_mem regions in the new array.
     i.e. the statically-placed regions.

The static array is no longer needed after this point, but there is
currently no obvious way to free the memory. Therefore, the size of the
initial static array is now defined using a config option.
Because the array is used only before paging_init() to store the
dynamically-placed reserved memory regions, the required size can vary
from device to device. Therefore, scaling it can help get some memory
savings.

A possible solution to freeing the memory for the static array will be
to mark it as __initdata. This will automatically free the memory once
the init process is done running.
The reason why this is not pursued in this series is because of
the possibility of a use-after-free.
If the dynamic allocation of the reserved_mem array fails, then future
accesses of the reserved_mem array will still be referencing the static
array. When the init process ends and the memory is freed up, any
further attempts to use the reserved_mem array will result in a
use-after-free.

Note:

- The limitation to this approach is that there is still a limit of
  64 for dynamically reserved memory regions.
- Upon further review, the series might need to be split up/duplicated
  for other archs.


Oreoluwa Babatunde (6):
  of: reserved_mem: Change the order that reserved_mem regions are
    stored
  of: reserved_mem: Swicth call to unflatten_device_tree() to after
    paging_init()
  of: resevred_mem: Delay allocation of memory for dynamic regions
  of: reserved_mem: Add code to use unflattened DT for reserved_mem
    nodes
  of: reserved_mem: Add code to dynamically allocate reserved_mem array
  of: reserved_mem: Make MAX_RESERVED_REGIONS a config option

 arch/loongarch/kernel/setup.c      |   2 +-
 arch/mips/kernel/setup.c           |   3 +-
 arch/nios2/kernel/setup.c          |   4 +-
 arch/openrisc/kernel/setup.c       |   4 +-
 arch/powerpc/kernel/setup-common.c |   3 +
 arch/sh/kernel/setup.c             |   5 +-
 arch/um/kernel/dtb.c               |   1 -
 arch/um/kernel/um_arch.c           |   2 +
 arch/xtensa/kernel/setup.c         |   4 +-
 drivers/of/Kconfig                 |  13 +++
 drivers/of/fdt.c                   |  39 +++++--
 drivers/of/of_private.h            |   6 +-
 drivers/of/of_reserved_mem.c       | 175 +++++++++++++++++++++--------
 include/linux/of_reserved_mem.h    |   8 +-
 kernel/dma/coherent.c              |   4 +-
 kernel/dma/contiguous.c            |   8 +-
 kernel/dma/swiotlb.c               |  10 +-
 17 files changed, 205 insertions(+), 86 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH v2 1/6] of: reserved_mem: Change the order that reserved_mem regions are stored
  2023-12-04 18:54 [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Oreoluwa Babatunde
@ 2023-12-04 18:54 ` Oreoluwa Babatunde
  2023-12-04 18:54 ` [RFC PATCH v2 2/6] of: reserved_mem: Swicth call to unflatten_device_tree() to after paging_init() Oreoluwa Babatunde
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-04 18:54 UTC (permalink / raw)
  To: catalin.marinas, will, robh+dt, frowand.list, dinguyen,
	chenhuacai, tsbogend, jonas, stefan.kristiansson, shorne, mpe,
	ysato, dalias, glaubitz, richard, anton.ivanov, johannes, chris,
	jcmvbkbc
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel,
	Oreoluwa Babatunde

The dynamic allocation of the reserved_mem array needs to be done after
paging_init() is called because memory allocated using memblock_alloc()
is not writeable before that.

Statically-placed reserved memory regions can wait until after
paging_init() to be stored in the array because the starting address and
size information is already specified in the DT.
Dynamically-placed regions on the other hand does not have its starting
address specified in the DT because it is allocated at runtime. Hence,
the starting address of these regions need to be stored somewhere.
The allocation of dynamically-placed regions cannot be done after
paging_init() either to avoid getting regions that are already added to
the page table mappings.

Hence, change the code to:
1. Before paging_init(), allocate and store information for the
   dynamically-placed reserved memory regions.
2. After paging_init(), store the information for the statically
   placed reserved memory regions in the array.

All the regions are also reserved or marked as nomap as needed before
paging_init().

Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
---
 drivers/of/fdt.c                | 63 ++++++++++++++++++++++++++++-----
 drivers/of/of_reserved_mem.c    | 53 ++++++++++++---------------
 include/linux/of_fdt.h          |  1 +
 include/linux/of_reserved_mem.h |  6 ++++
 4 files changed, 84 insertions(+), 39 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index bf502ba8da95..34223c249914 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -504,7 +504,6 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
 	phys_addr_t base, size;
 	int len;
 	const __be32 *prop;
-	int first = 1;
 	bool nomap;
 
 	prop = of_get_flat_dt_prop(node, "reg", &len);
@@ -532,10 +531,6 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
 			       uname, &base, (unsigned long)(size / SZ_1M));
 
 		len -= t_len;
-		if (first) {
-			fdt_reserved_mem_save_node(node, uname, base, size);
-			first = 0;
-		}
 	}
 	return 0;
 }
@@ -564,7 +559,56 @@ static int __init __reserved_mem_check_root(unsigned long node)
 }
 
 /*
- * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
+ * Save the reserved_mem reg nodes in the reserved_mem array
+ */
+void __init fdt_scan_reserved_mem_reg_nodes(void)
+
+{
+	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
+	const void *fdt = initial_boot_params;
+	phys_addr_t base, size;
+	const __be32 *prop;
+	int node, child;
+	int len;
+
+	node = fdt_path_offset(fdt, "/reserved-memory");
+	if (node < 0) {
+		pr_err("Reserved memory: Did not find reserved-memory node\n");
+		return;
+	}
+
+	if (__reserved_mem_check_root(node) != 0) {
+		pr_err("Reserved memory: unsupported node format, ignoring\n");
+		return;
+	}
+
+	fdt_for_each_subnode(child, fdt, node) {
+		const char *uname;
+
+		prop = of_get_flat_dt_prop(child, "reg", &len);
+		if (!prop)
+			continue;
+
+		if (!of_fdt_device_is_available(fdt, child))
+			continue;
+
+		uname = fdt_get_name(fdt, child, NULL);
+		if (len && len % t_len != 0) {
+			pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
+			       uname);
+			continue;
+		}
+
+		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
+		size = dt_mem_next_cell(dt_root_size_cells, &prop);
+
+		if (size)
+			fdt_reserved_mem_save_node(child, uname, base, size);
+	}
+}
+
+/*
+ * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory.
  */
 static int __init fdt_scan_reserved_mem(void)
 {
@@ -591,7 +635,7 @@ static int __init fdt_scan_reserved_mem(void)
 
 		err = __reserved_mem_reserve_reg(child, uname);
 		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
-			fdt_reserved_mem_save_node(child, uname, 0, 0);
+			__reserved_mem_alloc_size(child, uname);
 	}
 	return 0;
 }
@@ -645,8 +689,6 @@ void __init early_init_fdt_scan_reserved_mem(void)
 			break;
 		memblock_reserve(base, size);
 	}
-
-	fdt_init_reserved_mem();
 }
 
 /**
@@ -1335,6 +1377,9 @@ void __init unflatten_device_tree(void)
 	of_alias_scan(early_init_dt_alloc_memory_arch);
 
 	unittest_unflatten_overlay_base();
+
+	/*Initialize the reserved_mem regions*/
+	fdt_init_reserved_mem();
 }
 
 /**
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 7ec94cfcbddb..8bd8bbc3aaec 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -132,8 +132,7 @@ static int __init __reserved_mem_alloc_in_range(phys_addr_t size,
  * __reserved_mem_alloc_size() - allocate reserved memory described by
  *	'size', 'alignment'  and 'alloc-ranges' properties.
  */
-static int __init __reserved_mem_alloc_size(unsigned long node,
-	const char *uname, phys_addr_t *res_base, phys_addr_t *res_size)
+int __init __reserved_mem_alloc_size(unsigned long node, const char *uname)
 {
 	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
 	phys_addr_t start = 0, end = 0;
@@ -212,10 +211,7 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
 		       uname, (unsigned long)(size / SZ_1M));
 		return -ENOMEM;
 	}
-
-	*res_base = base;
-	*res_size = size;
-
+	fdt_reserved_mem_save_node(node, uname, base, size);
 	return 0;
 }
 
@@ -310,6 +306,8 @@ void __init fdt_init_reserved_mem(void)
 {
 	int i;
 
+	fdt_scan_reserved_mem_reg_nodes();
+
 	/* check for overlapping reserved regions */
 	__rmem_check_for_overlap();
 
@@ -328,30 +326,25 @@ void __init fdt_init_reserved_mem(void)
 		if (prop)
 			rmem->phandle = of_read_number(prop, len/4);
 
-		if (rmem->size == 0)
-			err = __reserved_mem_alloc_size(node, rmem->name,
-						 &rmem->base, &rmem->size);
-		if (err == 0) {
-			err = __reserved_mem_init_node(rmem);
-			if (err != 0 && err != -ENOENT) {
-				pr_info("node %s compatible matching fail\n",
-					rmem->name);
-				if (nomap)
-					memblock_clear_nomap(rmem->base, rmem->size);
-				else
-					memblock_phys_free(rmem->base,
-							   rmem->size);
-			} else {
-				phys_addr_t end = rmem->base + rmem->size - 1;
-				bool reusable =
-					(of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
-
-				pr_info("%pa..%pa (%lu KiB) %s %s %s\n",
-					&rmem->base, &end, (unsigned long)(rmem->size / SZ_1K),
-					nomap ? "nomap" : "map",
-					reusable ? "reusable" : "non-reusable",
-					rmem->name ? rmem->name : "unknown");
-			}
+		err = __reserved_mem_init_node(rmem);
+		if (err != 0 && err != -ENOENT) {
+			pr_info("node %s compatible matching fail\n",
+				rmem->name);
+			if (nomap)
+				memblock_clear_nomap(rmem->base, rmem->size);
+			else
+				memblock_phys_free(rmem->base,
+						   rmem->size);
+		} else {
+			phys_addr_t end = rmem->base + rmem->size - 1;
+			bool reusable =
+				(of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
+
+			pr_info("%pa..%pa (%lu KiB) %s %s %s\n",
+				&rmem->base, &end, (unsigned long)(rmem->size / SZ_1K),
+				nomap ? "nomap" : "map",
+				reusable ? "reusable" : "non-reusable",
+				rmem->name ? rmem->name : "unknown");
 		}
 	}
 }
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index d69ad5bb1eb1..7b2a5d93d719 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -73,6 +73,7 @@ extern int early_init_dt_scan_root(void);
 extern bool early_init_dt_scan(void *params);
 extern bool early_init_dt_verify(void *params);
 extern void early_init_dt_scan_nodes(void);
+extern void fdt_scan_reserved_mem_reg_nodes(void);
 
 extern const char *of_flat_dt_get_machine_name(void);
 extern const void *of_flat_dt_match_machine(const void *default_match,
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index 4de2a24cadc9..dc13bcd04b12 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -38,6 +38,7 @@ int of_reserved_mem_device_init_by_name(struct device *dev,
 					struct device_node *np,
 					const char *name);
 void of_reserved_mem_device_release(struct device *dev);
+int __reserved_mem_alloc_size(unsigned long node, const char *uname);
 
 struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
 #else
@@ -60,6 +61,11 @@ static inline int of_reserved_mem_device_init_by_name(struct device *dev,
 
 static inline void of_reserved_mem_device_release(struct device *pdev) { }
 
+static inline int __reserved_mem_alloc_size(unsigned long node, const char *uname)
+{
+	return -ENOSYS;
+}
+
 static inline struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
 {
 	return NULL;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v2 2/6] of: reserved_mem: Swicth call to unflatten_device_tree() to after paging_init()
  2023-12-04 18:54 [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Oreoluwa Babatunde
  2023-12-04 18:54 ` [RFC PATCH v2 1/6] of: reserved_mem: Change the order that reserved_mem regions are stored Oreoluwa Babatunde
@ 2023-12-04 18:54 ` Oreoluwa Babatunde
  2023-12-06 21:31   ` Rob Herring
  2023-12-04 18:54 ` [RFC PATCH v2 3/6] of: resevred_mem: Delay allocation of memory for dynamic regions Oreoluwa Babatunde
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-04 18:54 UTC (permalink / raw)
  To: catalin.marinas, will, robh+dt, frowand.list, dinguyen,
	chenhuacai, tsbogend, jonas, stefan.kristiansson, shorne, mpe,
	ysato, dalias, glaubitz, richard, anton.ivanov, johannes, chris,
	jcmvbkbc
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel,
	Oreoluwa Babatunde

Switch call to unflatten_device_tree() to after paging_init() on other
archs to follow new order in which the reserved_mem regions are
processed.

Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
---
 arch/loongarch/kernel/setup.c      | 2 +-
 arch/mips/kernel/setup.c           | 3 ++-
 arch/nios2/kernel/setup.c          | 4 ++--
 arch/openrisc/kernel/setup.c       | 4 ++--
 arch/powerpc/kernel/setup-common.c | 3 +++
 arch/sh/kernel/setup.c             | 5 ++---
 arch/um/kernel/dtb.c               | 1 -
 arch/um/kernel/um_arch.c           | 2 ++
 arch/xtensa/kernel/setup.c         | 4 +++-
 9 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index d183a745fb85..a01051b0f9e0 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -366,7 +366,6 @@ void __init platform_init(void)
 	acpi_gbl_use_default_register_widths = false;
 	acpi_boot_table_init();
 #endif
-	unflatten_and_copy_device_tree();
 
 #ifdef CONFIG_NUMA
 	init_numa_memory();
@@ -626,6 +625,7 @@ void __init setup_arch(char **cmdline_p)
 
 	paging_init();
 
+	unflatten_and_copy_device_tree();
 #ifdef CONFIG_KASAN
 	kasan_init();
 #endif
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 2d2ca024bd47..d3b6c86a8037 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -667,7 +667,6 @@ static void __init arch_mem_init(char **cmdline_p)
 	mips_reserve_vmcore();
 
 	mips_parse_crashkernel();
-	device_tree_init();
 
 	/*
 	 * In order to reduce the possibility of kernel panic when failed to
@@ -798,6 +797,8 @@ void __init setup_arch(char **cmdline_p)
 	cpu_cache_init();
 	paging_init();
 
+	device_tree_init();
+
 	memblock_dump_all();
 
 	setup_rng_seed();
diff --git a/arch/nios2/kernel/setup.c b/arch/nios2/kernel/setup.c
index da122a5fa43b..6f1a4232b8f0 100644
--- a/arch/nios2/kernel/setup.c
+++ b/arch/nios2/kernel/setup.c
@@ -169,8 +169,6 @@ void __init setup_arch(char **cmdline_p)
 	early_init_fdt_reserve_self();
 	early_init_fdt_scan_reserved_mem();
 
-	unflatten_and_copy_device_tree();
-
 	setup_cpuinfo();
 
 	copy_exception_handler(cpuinfo.exception_addr);
@@ -189,4 +187,6 @@ void __init setup_arch(char **cmdline_p)
 	 * get kmalloc into gear
 	 */
 	paging_init();
+
+	unflatten_and_copy_device_tree();
 }
diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
index 9cf7fb60441f..fcda33bdca19 100644
--- a/arch/openrisc/kernel/setup.c
+++ b/arch/openrisc/kernel/setup.c
@@ -255,8 +255,6 @@ void calibrate_delay(void)
 
 void __init setup_arch(char **cmdline_p)
 {
-	unflatten_and_copy_device_tree();
-
 	setup_cpuinfo();
 
 #ifdef CONFIG_SMP
@@ -284,6 +282,8 @@ void __init setup_arch(char **cmdline_p)
 	/* paging_init() sets up the MMU and marks all pages as reserved */
 	paging_init();
 
+	unflatten_and_copy_device_tree();
+
 	*cmdline_p = boot_command_line;
 
 	printk(KERN_INFO "OpenRISC Linux -- http://openrisc.io\n");
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 9b142b9d5187..58da58d02652 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -986,6 +986,9 @@ void __init setup_arch(char **cmdline_p)
 
 	paging_init();
 
+	/* Unflatten the device-tree passed by prom_init or kexec */
+	unflatten_device_tree();
+
 	/* Initialize the MMU context management stuff. */
 	mmu_context_init();
 
diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c
index 3d80515298d2..2553696af21b 100644
--- a/arch/sh/kernel/setup.c
+++ b/arch/sh/kernel/setup.c
@@ -322,6 +322,8 @@ void __init setup_arch(char **cmdline_p)
 	/* Let earlyprintk output early console messages */
 	sh_early_platform_driver_probe("earlyprintk", 1, 1);
 
+	paging_init();
+
 #ifdef CONFIG_OF_EARLY_FLATTREE
 #ifdef CONFIG_USE_BUILTIN_DTB
 	unflatten_and_copy_device_tree();
@@ -329,9 +331,6 @@ void __init setup_arch(char **cmdline_p)
 	unflatten_device_tree();
 #endif
 #endif
-
-	paging_init();
-
 	/* Perform the machine specific initialisation */
 	if (likely(sh_mv.mv_setup))
 		sh_mv.mv_setup(cmdline_p);
diff --git a/arch/um/kernel/dtb.c b/arch/um/kernel/dtb.c
index 484141b06938..04b0ada3b929 100644
--- a/arch/um/kernel/dtb.c
+++ b/arch/um/kernel/dtb.c
@@ -26,7 +26,6 @@ void uml_dtb_init(void)
 	}
 
 	early_init_fdt_scan_reserved_mem();
-	unflatten_device_tree();
 }
 
 static int __init uml_dtb_setup(char *line, int *add)
diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index b1bfed0c8528..fe6ecaa12ef2 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -7,6 +7,7 @@
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/mm.h>
+#include <linux/of_fdt.h>
 #include <linux/ctype.h>
 #include <linux/module.h>
 #include <linux/panic_notifier.h>
@@ -421,6 +422,7 @@ void __init setup_arch(char **cmdline_p)
 	read_initrd();
 
 	paging_init();
+	unflatten_device_tree();
 	strscpy(boot_command_line, command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = command_line;
 	setup_hostinfo(host_info, sizeof host_info);
diff --git a/arch/xtensa/kernel/setup.c b/arch/xtensa/kernel/setup.c
index bdec4a773af0..d20c56b4182e 100644
--- a/arch/xtensa/kernel/setup.c
+++ b/arch/xtensa/kernel/setup.c
@@ -355,13 +355,15 @@ void __init setup_arch(char **cmdline_p)
 	parse_early_param();
 	bootmem_init();
 	kasan_init();
-	unflatten_and_copy_device_tree();
 
 #ifdef CONFIG_SMP
 	smp_init_cpus();
 #endif
 
 	paging_init();
+
+	unflatten_and_copy_device_tree();
+
 	zones_init();
 
 #ifdef CONFIG_VT
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v2 3/6] of: resevred_mem: Delay allocation of memory for dynamic regions
  2023-12-04 18:54 [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Oreoluwa Babatunde
  2023-12-04 18:54 ` [RFC PATCH v2 1/6] of: reserved_mem: Change the order that reserved_mem regions are stored Oreoluwa Babatunde
  2023-12-04 18:54 ` [RFC PATCH v2 2/6] of: reserved_mem: Swicth call to unflatten_device_tree() to after paging_init() Oreoluwa Babatunde
@ 2023-12-04 18:54 ` Oreoluwa Babatunde
  2023-12-04 18:54 ` [RFC PATCH v2 4/6] of: reserved_mem: Add code to use unflattened DT for reserved_mem nodes Oreoluwa Babatunde
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-04 18:54 UTC (permalink / raw)
  To: catalin.marinas, will, robh+dt, frowand.list, dinguyen,
	chenhuacai, tsbogend, jonas, stefan.kristiansson, shorne, mpe,
	ysato, dalias, glaubitz, richard, anton.ivanov, johannes, chris,
	jcmvbkbc
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel,
	Oreoluwa Babatunde

Defer the allocation of memory for the dynamically-placed reserved
memory regions until after the statically-placed reserved memory
regions have been reserved or marked as nomap in memblock.
This is to avoid allocating from regions that will later turn out to
have been reserved by other nodes.

Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
---
 drivers/of/fdt.c             | 18 ++++++++++++++++--
 drivers/of/of_private.h      |  1 +
 drivers/of/of_reserved_mem.c |  1 -
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 34223c249914..46adce306bbd 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -613,6 +613,8 @@ void __init fdt_scan_reserved_mem_reg_nodes(void)
 static int __init fdt_scan_reserved_mem(void)
 {
 	int node, child;
+	int dynamic_nodes_cnt = 0;
+	int dynamic_nodes[MAX_RESERVED_REGIONS];
 	const void *fdt = initial_boot_params;
 
 	node = fdt_path_offset(fdt, "/reserved-memory");
@@ -634,8 +636,20 @@ static int __init fdt_scan_reserved_mem(void)
 		uname = fdt_get_name(fdt, child, NULL);
 
 		err = __reserved_mem_reserve_reg(child, uname);
-		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
-			__reserved_mem_alloc_size(child, uname);
+
+		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) {
+			dynamic_nodes[dynamic_nodes_cnt] = child;
+			dynamic_nodes_cnt++;
+		}
+	}
+
+	for (int i = 0; i < dynamic_nodes_cnt; i++) {
+		const char *uname;
+
+		child = dynamic_nodes[i];
+		uname = fdt_get_name(fdt, child, NULL);
+
+		__reserved_mem_alloc_size(child, uname);
 	}
 	return 0;
 }
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index f38397c7b582..57694a704b00 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -36,6 +36,7 @@ struct alias_prop {
 #endif
 
 #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
+#define MAX_RESERVED_REGIONS    64
 
 extern struct mutex of_mutex;
 extern raw_spinlock_t devtree_lock;
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 8bd8bbc3aaec..d62f1956024c 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -26,7 +26,6 @@
 
 #include "of_private.h"
 
-#define MAX_RESERVED_REGIONS	64
 static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
 static int reserved_mem_count;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v2 4/6] of: reserved_mem: Add code to use unflattened DT for reserved_mem nodes
  2023-12-04 18:54 [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Oreoluwa Babatunde
                   ` (2 preceding siblings ...)
  2023-12-04 18:54 ` [RFC PATCH v2 3/6] of: resevred_mem: Delay allocation of memory for dynamic regions Oreoluwa Babatunde
@ 2023-12-04 18:54 ` Oreoluwa Babatunde
  2023-12-04 18:54 ` [RFC PATCH v2 5/6] of: reserved_mem: Add code to dynamically allocate reserved_mem array Oreoluwa Babatunde
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-04 18:54 UTC (permalink / raw)
  To: catalin.marinas, will, robh+dt, frowand.list, dinguyen,
	chenhuacai, tsbogend, jonas, stefan.kristiansson, shorne, mpe,
	ysato, dalias, glaubitz, richard, anton.ivanov, johannes, chris,
	jcmvbkbc
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel,
	Oreoluwa Babatunde

The unflattened devicetree APIs are available to be used right after
paging_init() runs. Therefore, use the unflattened devicetree APIs to
process the reserved memory regions from this point.

Using the unflattened devicetree APIs is more efficient than using the
flattened devicetree APIs.

Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
---
 drivers/of/fdt.c                | 51 +--------------------
 drivers/of/of_private.h         |  4 +-
 drivers/of/of_reserved_mem.c    | 79 +++++++++++++++++++++++++--------
 include/linux/of_fdt.h          |  1 -
 include/linux/of_reserved_mem.h |  2 +-
 kernel/dma/coherent.c           |  4 +-
 kernel/dma/contiguous.c         |  8 ++--
 kernel/dma/swiotlb.c            | 10 ++---
 8 files changed, 76 insertions(+), 83 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 46adce306bbd..12769dd53c34 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -558,55 +558,6 @@ static int __init __reserved_mem_check_root(unsigned long node)
 	return 0;
 }
 
-/*
- * Save the reserved_mem reg nodes in the reserved_mem array
- */
-void __init fdt_scan_reserved_mem_reg_nodes(void)
-
-{
-	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
-	const void *fdt = initial_boot_params;
-	phys_addr_t base, size;
-	const __be32 *prop;
-	int node, child;
-	int len;
-
-	node = fdt_path_offset(fdt, "/reserved-memory");
-	if (node < 0) {
-		pr_err("Reserved memory: Did not find reserved-memory node\n");
-		return;
-	}
-
-	if (__reserved_mem_check_root(node) != 0) {
-		pr_err("Reserved memory: unsupported node format, ignoring\n");
-		return;
-	}
-
-	fdt_for_each_subnode(child, fdt, node) {
-		const char *uname;
-
-		prop = of_get_flat_dt_prop(child, "reg", &len);
-		if (!prop)
-			continue;
-
-		if (!of_fdt_device_is_available(fdt, child))
-			continue;
-
-		uname = fdt_get_name(fdt, child, NULL);
-		if (len && len % t_len != 0) {
-			pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
-			       uname);
-			continue;
-		}
-
-		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
-		size = dt_mem_next_cell(dt_root_size_cells, &prop);
-
-		if (size)
-			fdt_reserved_mem_save_node(child, uname, base, size);
-	}
-}
-
 /*
  * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory.
  */
@@ -1393,7 +1344,7 @@ void __init unflatten_device_tree(void)
 	unittest_unflatten_overlay_base();
 
 	/*Initialize the reserved_mem regions*/
-	fdt_init_reserved_mem();
+	init_reserved_mem();
 }
 
 /**
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 57694a704b00..575e2b4119e0 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -176,8 +176,8 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node *
 }
 #endif
 
-void fdt_init_reserved_mem(void);
-void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
+void init_reserved_mem(void);
+void dt_reserved_mem_save_node(struct device_node *node, const char *uname,
 			       phys_addr_t base, phys_addr_t size);
 
 #endif /* _LINUX_OF_PRIVATE_H */
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index d62f1956024c..2ef9edcb8c93 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -55,9 +55,9 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
 }
 
 /*
- * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
+ * dt_reserved_mem_save_node() - save dt node for second pass initialization
  */
-void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
+void __init dt_reserved_mem_save_node(struct device_node *node, const char *uname,
 				      phys_addr_t base, phys_addr_t size)
 {
 	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
@@ -67,7 +67,7 @@ void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
 		return;
 	}
 
-	rmem->fdt_node = node;
+	rmem->dev_node = node;
 	rmem->name = uname;
 	rmem->base = base;
 	rmem->size = size;
@@ -76,6 +76,54 @@ void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
 	return;
 }
 
+/*
+ * Save the reserved_mem reg nodes in the reserved_mem array
+ */
+static void __init scan_reserved_mem_reg_nodes(void)
+
+{
+	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
+	struct device_node *node, *child;
+	phys_addr_t base, size;
+	const __be32 *prop;
+	int len;
+
+	node = of_find_node_by_path("/reserved-memory");
+	if (node < 0) {
+		pr_err("Reserved memory: Did not find reserved-memory node\n");
+		return;
+	}
+
+	for_each_child_of_node(node, child) {
+		const char *uname;
+		struct reserved_mem *rmem;
+
+		if (!of_device_is_available(child))
+			continue;
+
+		prop = of_get_property(child, "reg", &len);
+		if (!prop) {
+			rmem = of_reserved_mem_lookup(child);
+			if (rmem)
+				rmem->dev_node = child;
+			continue;
+		}
+
+		uname = of_node_full_name(child);
+		if (len && len % t_len != 0) {
+			pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
+			       uname);
+			continue;
+		}
+
+		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
+		size = dt_mem_next_cell(dt_root_size_cells, &prop);
+
+		if (size)
+			dt_reserved_mem_save_node(child, uname, base, size);
+	}
+}
+
 /*
  * __reserved_mem_alloc_in_range() - allocate reserved memory described with
  *	'alloc-ranges'. Choose bottom-up/top-down depending on nearby existing
@@ -210,7 +258,7 @@ int __init __reserved_mem_alloc_size(unsigned long node, const char *uname)
 		       uname, (unsigned long)(size / SZ_1M));
 		return -ENOMEM;
 	}
-	fdt_reserved_mem_save_node(node, uname, base, size);
+	dt_reserved_mem_save_node(NULL, uname, base, size);
 	return 0;
 }
 
@@ -230,7 +278,7 @@ static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
 		reservedmem_of_init_fn initfn = i->data;
 		const char *compat = i->compatible;
 
-		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
+		if (!of_device_is_compatible(rmem->dev_node, compat))
 			continue;
 
 		ret = initfn(rmem);
@@ -263,11 +311,6 @@ static int __init __rmem_cmp(const void *a, const void *b)
 	if (ra->size > rb->size)
 		return 1;
 
-	if (ra->fdt_node < rb->fdt_node)
-		return -1;
-	if (ra->fdt_node > rb->fdt_node)
-		return 1;
-
 	return 0;
 }
 
@@ -299,29 +342,29 @@ static void __init __rmem_check_for_overlap(void)
 }
 
 /**
- * fdt_init_reserved_mem() - allocate and init all saved reserved memory regions
+ * init_reserved_mem() - allocate and init all saved reserved memory regions
  */
-void __init fdt_init_reserved_mem(void)
+void __init init_reserved_mem(void)
 {
 	int i;
 
-	fdt_scan_reserved_mem_reg_nodes();
+	scan_reserved_mem_reg_nodes();
 
 	/* check for overlapping reserved regions */
 	__rmem_check_for_overlap();
 
 	for (i = 0; i < reserved_mem_count; i++) {
 		struct reserved_mem *rmem = &reserved_mem[i];
-		unsigned long node = rmem->fdt_node;
+		struct device_node *node = rmem->dev_node;
 		int len;
 		const __be32 *prop;
 		int err = 0;
 		bool nomap;
 
-		nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
-		prop = of_get_flat_dt_prop(node, "phandle", &len);
+		nomap = of_get_property(node, "no-map", NULL) != NULL;
+		prop = of_get_property(node, "phandle", &len);
 		if (!prop)
-			prop = of_get_flat_dt_prop(node, "linux,phandle", &len);
+			prop = of_get_property(node, "linux,phandle", &len);
 		if (prop)
 			rmem->phandle = of_read_number(prop, len/4);
 
@@ -337,7 +380,7 @@ void __init fdt_init_reserved_mem(void)
 		} else {
 			phys_addr_t end = rmem->base + rmem->size - 1;
 			bool reusable =
-				(of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
+				(of_get_property(node, "reusable", NULL)) != NULL;
 
 			pr_info("%pa..%pa (%lu KiB) %s %s %s\n",
 				&rmem->base, &end, (unsigned long)(rmem->size / SZ_1K),
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 7b2a5d93d719..d69ad5bb1eb1 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -73,7 +73,6 @@ extern int early_init_dt_scan_root(void);
 extern bool early_init_dt_scan(void *params);
 extern bool early_init_dt_verify(void *params);
 extern void early_init_dt_scan_nodes(void);
-extern void fdt_scan_reserved_mem_reg_nodes(void);
 
 extern const char *of_flat_dt_get_machine_name(void);
 extern const void *of_flat_dt_match_machine(const void *default_match,
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index dc13bcd04b12..2b205ce6beb9 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -10,7 +10,7 @@ struct reserved_mem_ops;
 
 struct reserved_mem {
 	const char			*name;
-	unsigned long			fdt_node;
+	struct device_node		*dev_node;
 	unsigned long			phandle;
 	const struct reserved_mem_ops	*ops;
 	phys_addr_t			base;
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
index c21abc77c53e..3eba9678dc33 100644
--- a/kernel/dma/coherent.c
+++ b/kernel/dma/coherent.c
@@ -360,9 +360,9 @@ static const struct reserved_mem_ops rmem_dma_ops = {
 
 static int __init rmem_dma_setup(struct reserved_mem *rmem)
 {
-	unsigned long node = rmem->fdt_node;
+	struct device_node *node = rmem->dev_node;
 
-	if (of_get_flat_dt_prop(node, "reusable", NULL))
+	if (of_get_property(node, "reusable", NULL))
 		return -EINVAL;
 
 #ifdef CONFIG_ARM
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index f005c66f378c..b54cf128a9d9 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -462,8 +462,8 @@ static const struct reserved_mem_ops rmem_cma_ops = {
 
 static int __init rmem_cma_setup(struct reserved_mem *rmem)
 {
-	unsigned long node = rmem->fdt_node;
-	bool default_cma = of_get_flat_dt_prop(node, "linux,cma-default", NULL);
+	struct device_node *node = rmem->dev_node;
+	bool default_cma = of_get_property(node, "linux,cma-default", NULL);
 	struct cma *cma;
 	int err;
 
@@ -473,8 +473,8 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)
 		return -EBUSY;
 	}
 
-	if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
-	    of_get_flat_dt_prop(node, "no-map", NULL))
+	if (!of_get_property(node, "reusable", NULL) ||
+	    of_get_property(node, "no-map", NULL))
 		return -EINVAL;
 
 	if (!IS_ALIGNED(rmem->base | rmem->size, CMA_MIN_ALIGNMENT_BYTES)) {
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 33d942615be5..14840a58a6e6 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -1709,12 +1709,12 @@ static const struct reserved_mem_ops rmem_swiotlb_ops = {
 
 static int __init rmem_swiotlb_setup(struct reserved_mem *rmem)
 {
-	unsigned long node = rmem->fdt_node;
+	struct device_node *node = rmem->dev_node;
 
-	if (of_get_flat_dt_prop(node, "reusable", NULL) ||
-	    of_get_flat_dt_prop(node, "linux,cma-default", NULL) ||
-	    of_get_flat_dt_prop(node, "linux,dma-default", NULL) ||
-	    of_get_flat_dt_prop(node, "no-map", NULL))
+	if (of_get_property(node, "reusable", NULL) ||
+	    of_get_property(node, "linux,cma-default", NULL) ||
+	    of_get_property(node, "linux,dma-default", NULL) ||
+	    of_get_property(node, "no-map", NULL))
 		return -EINVAL;
 
 	rmem->ops = &rmem_swiotlb_ops;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v2 5/6] of: reserved_mem: Add code to dynamically allocate reserved_mem array
  2023-12-04 18:54 [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Oreoluwa Babatunde
                   ` (3 preceding siblings ...)
  2023-12-04 18:54 ` [RFC PATCH v2 4/6] of: reserved_mem: Add code to use unflattened DT for reserved_mem nodes Oreoluwa Babatunde
@ 2023-12-04 18:54 ` Oreoluwa Babatunde
  2023-12-04 18:54 ` [RFC PATCH v2 6/6] of: reserved_mem: Make MAX_RESERVED_REGIONS a config option Oreoluwa Babatunde
  2023-12-06 21:35 ` [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Rob Herring
  6 siblings, 0 replies; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-04 18:54 UTC (permalink / raw)
  To: catalin.marinas, will, robh+dt, frowand.list, dinguyen,
	chenhuacai, tsbogend, jonas, stefan.kristiansson, shorne, mpe,
	ysato, dalias, glaubitz, richard, anton.ivanov, johannes, chris,
	jcmvbkbc
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel,
	Oreoluwa Babatunde

The reserved_mem array is statically allocated with a size of
MAX_RESERVED_REGIONS(64). Therefore, if the number of reserved_mem
regions exceeds this size, there will not be enough space to store
all the data.

Therefore, extend the use of the static array by introducing a
dynamically allocated array based on the number of reserved memory
regions specified in the DT.

Before paging_init() runs, the static array is used to store the
dynamically-placed regions.
After paging_init(), memory is dynamically allocated for the
reserved_mem array, and all entries from the static array is copied
over to the new array, and all other statically-placed regions are
added in as well.

Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
---
 drivers/of/fdt.c             | 13 +++++++---
 drivers/of/of_private.h      |  1 +
 drivers/of/of_reserved_mem.c | 48 +++++++++++++++++++++++++++++++++---
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 12769dd53c34..2f1eabbd6869 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -563,8 +563,8 @@ static int __init __reserved_mem_check_root(unsigned long node)
  */
 static int __init fdt_scan_reserved_mem(void)
 {
-	int node, child;
-	int dynamic_nodes_cnt = 0;
+	int node, child, err = 0;
+	int dynamic_nodes_cnt = 0, count = 0;
 	int dynamic_nodes[MAX_RESERVED_REGIONS];
 	const void *fdt = initial_boot_params;
 
@@ -579,7 +579,6 @@ static int __init fdt_scan_reserved_mem(void)
 
 	fdt_for_each_subnode(child, fdt, node) {
 		const char *uname;
-		int err;
 
 		if (!of_fdt_device_is_available(fdt, child))
 			continue;
@@ -587,6 +586,8 @@ static int __init fdt_scan_reserved_mem(void)
 		uname = fdt_get_name(fdt, child, NULL);
 
 		err = __reserved_mem_reserve_reg(child, uname);
+		if (!err)
+			count++;
 
 		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) {
 			dynamic_nodes[dynamic_nodes_cnt] = child;
@@ -600,8 +601,12 @@ static int __init fdt_scan_reserved_mem(void)
 		child = dynamic_nodes[i];
 		uname = fdt_get_name(fdt, child, NULL);
 
-		__reserved_mem_alloc_size(child, uname);
+		err = __reserved_mem_alloc_size(child, uname);
+		if (!err)
+			count++;
 	}
+	update_reserved_mem_max_cnt(count);
+
 	return 0;
 }
 
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 575e2b4119e0..ef56b2ea185c 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -179,5 +179,6 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node *
 void init_reserved_mem(void);
 void dt_reserved_mem_save_node(struct device_node *node, const char *uname,
 			       phys_addr_t base, phys_addr_t size);
+void update_reserved_mem_max_cnt(int max_count);
 
 #endif /* _LINUX_OF_PRIVATE_H */
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 2ef9edcb8c93..01cd6a571dc2 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -26,7 +26,9 @@
 
 #include "of_private.h"
 
-static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
+static struct reserved_mem reserved_mem_array[MAX_RESERVED_REGIONS];
+static struct reserved_mem *reserved_mem = reserved_mem_array;
+static int total_reserved_mem_cnt = MAX_RESERVED_REGIONS;
 static int reserved_mem_count;
 
 static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
@@ -54,6 +56,42 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
 	return err;
 }
 
+void __init update_reserved_mem_max_cnt(int max_count)
+{
+	total_reserved_mem_cnt = max_count;
+}
+
+static int alloc_reserved_mem_array(void)
+{
+	struct reserved_mem *new_array;
+	size_t alloc_size, copy_size, memset_size;
+
+	alloc_size = array_size(total_reserved_mem_cnt, sizeof(*new_array));
+	if (alloc_size == SIZE_MAX)
+		return -1;
+
+	new_array = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
+	if (!new_array)
+		return -ENOMEM;
+
+	copy_size = array_size(reserved_mem_count, sizeof(*new_array));
+	if (copy_size == SIZE_MAX)
+		goto overlow_err;
+
+	memset_size = alloc_size - copy_size;
+
+	memcpy(new_array, reserved_mem, copy_size);
+	memset(new_array + reserved_mem_count, 0, memset_size);
+
+	reserved_mem = new_array;
+	return 0;
+
+overlow_err:
+	memblock_free(new_array, alloc_size);
+	total_reserved_mem_cnt = MAX_RESERVED_REGIONS;
+	return -1;
+}
+
 /*
  * dt_reserved_mem_save_node() - save dt node for second pass initialization
  */
@@ -62,7 +100,7 @@ void __init dt_reserved_mem_save_node(struct device_node *node, const char *unam
 {
 	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
 
-	if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
+	if (reserved_mem_count == total_reserved_mem_cnt) {
 		pr_err("not enough space for all defined regions.\n");
 		return;
 	}
@@ -346,7 +384,11 @@ static void __init __rmem_check_for_overlap(void)
  */
 void __init init_reserved_mem(void)
 {
-	int i;
+	int i, ret;
+
+	ret = alloc_reserved_mem_array();
+	if (ret)
+		pr_err("Failed to allocate memory for reserved_mem array with err: %d", ret);
 
 	scan_reserved_mem_reg_nodes();
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v2 6/6] of: reserved_mem: Make MAX_RESERVED_REGIONS a config option
  2023-12-04 18:54 [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Oreoluwa Babatunde
                   ` (4 preceding siblings ...)
  2023-12-04 18:54 ` [RFC PATCH v2 5/6] of: reserved_mem: Add code to dynamically allocate reserved_mem array Oreoluwa Babatunde
@ 2023-12-04 18:54 ` Oreoluwa Babatunde
  2023-12-06 21:35 ` [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Rob Herring
  6 siblings, 0 replies; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-04 18:54 UTC (permalink / raw)
  To: catalin.marinas, will, robh+dt, frowand.list, dinguyen,
	chenhuacai, tsbogend, jonas, stefan.kristiansson, shorne, mpe,
	ysato, dalias, glaubitz, richard, anton.ivanov, johannes, chris,
	jcmvbkbc
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel,
	Oreoluwa Babatunde

Make the value of MAX_RESERVED_REGIONS a config option which can be
modified based on user requirements.
The reserved_mem_array is required during device bootup to store the
information of the dynamically-placed reserved memory regions.
After paging_init(), this information is transferred to another
array which is dynamically allocated and used to store all the reserved
memory regions.

There is currently no obvious way to free the memory for the static
array after its contents are copied over to the dynamically allocated
array, but since the size required for the reserved_mem_array can vary
from device to device depending on the number of dynamically-placed
reserved memory regions, make the size of the array configurable in an
attempt to save some memory.

Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
---
 drivers/of/Kconfig      | 13 +++++++++++++
 drivers/of/of_private.h |  2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index da9826accb1b..409ce2527461 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -102,4 +102,17 @@ config OF_OVERLAY
 config OF_NUMA
 	bool
 
+config OF_MAX_RESERVED_REGIONS
+	int "OF resvered_mem array size"
+	default "64"
+	range 1 64
+	help
+	   The reserved_mem_array is used to store information about the dynamically
+	   placed reserved memory regions before we are able to allocate the memory
+	   needed to store all the reserved memory regions defined in the DT.
+	   Because the amount of memory needed initially for this array could vary,
+	   make the size of the reserved_mem_array configurable in an attempt to
+	   save some memory when possible.
+	   if unsure, leave as default value.
+
 endif # OF
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index ef56b2ea185c..278038bce0c0 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -36,7 +36,7 @@ struct alias_prop {
 #endif
 
 #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
-#define MAX_RESERVED_REGIONS    64
+#define MAX_RESERVED_REGIONS    CONFIG_OF_MAX_RESERVED_REGIONS
 
 extern struct mutex of_mutex;
 extern raw_spinlock_t devtree_lock;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array.
  2023-12-04  4:13 Oreoluwa Babatunde
@ 2023-12-04 18:54 ` Oreoluwa Babatunde
  0 siblings, 0 replies; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-04 18:54 UTC (permalink / raw)
  To: catalin.marinas, will, robh+dt, frowand.list
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel


On 12/3/2023 8:13 PM, Oreoluwa Babatunde wrote:
> The reserved_mem array is used to store the data of the different
> reserved memory regions specified in the DT of a device.
> The array stores information such as the name, node, starting address,
> and size of a reserved memory region.
>
> The array is currently statically allocated with a size of
> MAX_RESERVED_REGIONS(64). This means that any system that specifies a
> number of reserved memory regions greater than MAX_RESERVED_REGIONS(64)
> will not have enough space to store the information for all the regions.
>
> Therefore, this series extends the use of a static array for
> reserved_mem, and introduces a dynamically allocated array using
> memblock_alloc() based on the number of reserved memory regions
> specified in the DT.
>
> Memory gotten from memblock_alloc() is only writable after paging_init()
> is called, but the reserved memory regions need to be reserved before
> then so that the system does not create page table mappings for them.
>
> Reserved memory regions can be divided into 2 groups.
> i) Statically-placed reserved memory regions
> i.e. regions defined in the DT using the @reg property.
> ii) Dynamically-placed reserved memory regions.
> i.e. regions specified in the DT using the @alloc_ranges
>     and @size properties.
>
> It is possible to call memblock_reserve() and memblock_mark_nomap() on
> the statically-placed reserved memory regions and not need to save them
> to the array until after paging_init(), but this is not possible for the
> dynamically-placed reserved memory because the starting address of these
> regions need to be stored somewhere after they are allocated.
>
> Therefore, this series achieves the allocation and population of the
> reserved_mem array in two steps:
>
> 1. Before paging_init()
>    Before paging_init() is called, iterate through the reserved_mem
>    nodes in the DT and do the following:
>    - Allocate memory for dynamically-placed reserved memory regions and
>      store their starting address in the static allocated reserved_mem
>      array.
>    - Call memblock_reserve() and memblock_mark_nomap() on all the
>      reserved memory regions as needed.
>    - Count the total number of reserved_mem nodes in the DT.
>
> 2. After paging_init()
>    After paging_init() is called:
>    - Allocate new memory for the reserved_mem array based on the number
>      of reserved memory nodes in the DT.
>    - Transfer all the information that was stored in the static array
>      into the new array.
>    - Store the rest of the reserved_mem regions in the new array.
>      i.e. the statically-placed regions.
>
> The static array is no longer needed after this point, but there is
> currently no obvious way to free the memory. Therefore, the size of the
> initial static array is now defined using a config option.
> Because the array is used only before paging_init() to store the
> dynamically-placed reserved memory regions, the required size can vary
> from device to device. Therefore, scaling it can help get some memory
> savings.
>
> A possible solution to freeing the memory for the static array will be
> to mark it as __initdata. This will automatically free the memory once
> the init process is done running.
> The reason why this is not pursued in this series is because of
> the possibility of a use-after-free.
> If the dynamic allocation of the reserved_mem array fails, then future
> accesses of the reserved_mem array will still be referencing the static
> array. When the init process ends and the memory is freed up, any
> further attempts to use the reserved_mem array will result in a
> use-after-free.
>
> Note:
>
> - The limitation to this approach is that there is still a limit of
>   64 for dynamically reserved memory regions.
> - Upon further review, the series might need to be split up/duplicated
>   for other archs.
>
>
> Oreoluwa Babatunde (6):
>   of: reserved_mem: Change the order that reserved_mem regions are
>     stored
>   of: reserved_mem: Swicth call to unflatten_device_tree() to after
>     paging_init()
>   of: resevred_mem: Delay allocation of memory for dynamic regions
>   of: reserved_mem: Add code to use unflattened DT for reserved_mem
>     nodes
>   of: reserved_mem: Add code to dynamically allocate reserved_mem array
>   of: reserved_mem: Make MAX_RESERVED_REGIONS a config option
>
>  arch/loongarch/kernel/setup.c      |   2 +-
>  arch/mips/kernel/setup.c           |   3 +-
>  arch/nios2/kernel/setup.c          |   4 +-
>  arch/openrisc/kernel/setup.c       |   4 +-
>  arch/powerpc/kernel/setup-common.c |   3 +
>  arch/sh/kernel/setup.c             |   5 +-
>  arch/um/kernel/dtb.c               |   1 -
>  arch/um/kernel/um_arch.c           |   2 +
>  arch/xtensa/kernel/setup.c         |   4 +-
>  drivers/of/Kconfig                 |  13 +++
>  drivers/of/fdt.c                   |  39 +++++--
>  drivers/of/of_private.h            |   6 +-
>  drivers/of/of_reserved_mem.c       | 175 +++++++++++++++++++++--------
>  include/linux/of_reserved_mem.h    |   8 +-
>  kernel/dma/coherent.c              |   4 +-
>  kernel/dma/contiguous.c            |   8 +-
>  kernel/dma/swiotlb.c               |  10 +-
>  17 files changed, 205 insertions(+), 86 deletions(-)
re-sending this to include maintainers for the other
archs.

Regards,

Oreoluwa


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v2 2/6] of: reserved_mem: Swicth call to unflatten_device_tree() to after paging_init()
  2023-12-04 18:54 ` [RFC PATCH v2 2/6] of: reserved_mem: Swicth call to unflatten_device_tree() to after paging_init() Oreoluwa Babatunde
@ 2023-12-06 21:31   ` Rob Herring
  2023-12-11  0:51     ` Oreoluwa Babatunde
  0 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2023-12-06 21:31 UTC (permalink / raw)
  To: Oreoluwa Babatunde
  Cc: catalin.marinas, will, frowand.list, dinguyen, chenhuacai,
	tsbogend, jonas, stefan.kristiansson, shorne, mpe, ysato, dalias,
	glaubitz, richard, anton.ivanov, johannes, chris, jcmvbkbc,
	linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel

On Mon, Dec 04, 2023 at 10:54:05AM -0800, Oreoluwa Babatunde wrote:
> Switch call to unflatten_device_tree() to after paging_init() on other
> archs to follow new order in which the reserved_mem regions are
> processed.

You did this so that you could allocate memory for the reserved regions. 
But unflatten_device_tree() can already do allocations using memblock. 
So the same thing should work for you.

I suspect that moving this will break any arch that called an of_* API 
between the original call and the new call location.

Rob

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array.
  2023-12-04 18:54 [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Oreoluwa Babatunde
                   ` (5 preceding siblings ...)
  2023-12-04 18:54 ` [RFC PATCH v2 6/6] of: reserved_mem: Make MAX_RESERVED_REGIONS a config option Oreoluwa Babatunde
@ 2023-12-06 21:35 ` Rob Herring
  2023-12-11  0:42   ` Oreoluwa Babatunde
  6 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2023-12-06 21:35 UTC (permalink / raw)
  To: Oreoluwa Babatunde
  Cc: catalin.marinas, will, frowand.list, dinguyen, chenhuacai,
	tsbogend, jonas, stefan.kristiansson, shorne, mpe, ysato, dalias,
	glaubitz, richard, anton.ivanov, johannes, chris, jcmvbkbc,
	linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel

On Mon, Dec 04, 2023 at 10:54:03AM -0800, Oreoluwa Babatunde wrote:
> The reserved_mem array is used to store the data of the different
> reserved memory regions specified in the DT of a device.
> The array stores information such as the name, node, starting address,
> and size of a reserved memory region.
> 
> The array is currently statically allocated with a size of
> MAX_RESERVED_REGIONS(64). This means that any system that specifies a
> number of reserved memory regions greater than MAX_RESERVED_REGIONS(64)
> will not have enough space to store the information for all the regions.
> 
> Therefore, this series extends the use of a static array for
> reserved_mem, and introduces a dynamically allocated array using
> memblock_alloc() based on the number of reserved memory regions
> specified in the DT.
> 
> Memory gotten from memblock_alloc() is only writable after paging_init()
> is called, but the reserved memory regions need to be reserved before
> then so that the system does not create page table mappings for them.
> 
> Reserved memory regions can be divided into 2 groups.
> i) Statically-placed reserved memory regions
> i.e. regions defined in the DT using the @reg property.
> ii) Dynamically-placed reserved memory regions.
> i.e. regions specified in the DT using the @alloc_ranges
>     and @size properties.
> 
> It is possible to call memblock_reserve() and memblock_mark_nomap() on
> the statically-placed reserved memory regions and not need to save them
> to the array until after paging_init(), but this is not possible for the
> dynamically-placed reserved memory because the starting address of these
> regions need to be stored somewhere after they are allocated.
> 
> Therefore, this series achieves the allocation and population of the
> reserved_mem array in two steps:
> 
> 1. Before paging_init()
>    Before paging_init() is called, iterate through the reserved_mem
>    nodes in the DT and do the following:
>    - Allocate memory for dynamically-placed reserved memory regions and
>      store their starting address in the static allocated reserved_mem
>      array.
>    - Call memblock_reserve() and memblock_mark_nomap() on all the
>      reserved memory regions as needed.
>    - Count the total number of reserved_mem nodes in the DT.
> 
> 2. After paging_init()
>    After paging_init() is called:
>    - Allocate new memory for the reserved_mem array based on the number
>      of reserved memory nodes in the DT.
>    - Transfer all the information that was stored in the static array
>      into the new array.
>    - Store the rest of the reserved_mem regions in the new array.
>      i.e. the statically-placed regions.
> 
> The static array is no longer needed after this point, but there is
> currently no obvious way to free the memory. Therefore, the size of the
> initial static array is now defined using a config option.

A config option is not going to work here.

> Because the array is used only before paging_init() to store the
> dynamically-placed reserved memory regions, the required size can vary
> from device to device. Therefore, scaling it can help get some memory
> savings.
> 
> A possible solution to freeing the memory for the static array will be
> to mark it as __initdata. This will automatically free the memory once
> the init process is done running.
> The reason why this is not pursued in this series is because of
> the possibility of a use-after-free.
> If the dynamic allocation of the reserved_mem array fails, then future
> accesses of the reserved_mem array will still be referencing the static
> array. When the init process ends and the memory is freed up, any
> further attempts to use the reserved_mem array will result in a
> use-after-free.

If memory allocation for the reserved_mem array fails so early in boot, 
you've got much bigger problems. Use __initdata, and just WARN if 
allocation fails and continue on (so hopefully the console is brought 
up and someone can see the WARN).

Rob

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array.
  2023-12-06 21:35 ` [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Rob Herring
@ 2023-12-11  0:42   ` Oreoluwa Babatunde
  0 siblings, 0 replies; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-11  0:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: catalin.marinas, will, frowand.list, dinguyen, chenhuacai,
	tsbogend, jonas, stefan.kristiansson, shorne, mpe, ysato, dalias,
	glaubitz, richard, anton.ivanov, johannes, chris, jcmvbkbc,
	linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel


On 12/6/2023 1:35 PM, Rob Herring wrote:
> On Mon, Dec 04, 2023 at 10:54:03AM -0800, Oreoluwa Babatunde wrote:
>> The reserved_mem array is used to store the data of the different
>> reserved memory regions specified in the DT of a device.
>> The array stores information such as the name, node, starting address,
>> and size of a reserved memory region.
>>
>> The array is currently statically allocated with a size of
>> MAX_RESERVED_REGIONS(64). This means that any system that specifies a
>> number of reserved memory regions greater than MAX_RESERVED_REGIONS(64)
>> will not have enough space to store the information for all the regions.
>>
>> Therefore, this series extends the use of a static array for
>> reserved_mem, and introduces a dynamically allocated array using
>> memblock_alloc() based on the number of reserved memory regions
>> specified in the DT.
>>
>> Memory gotten from memblock_alloc() is only writable after paging_init()
>> is called, but the reserved memory regions need to be reserved before
>> then so that the system does not create page table mappings for them.
>>
>> Reserved memory regions can be divided into 2 groups.
>> i) Statically-placed reserved memory regions
>> i.e. regions defined in the DT using the @reg property.
>> ii) Dynamically-placed reserved memory regions.
>> i.e. regions specified in the DT using the @alloc_ranges
>>     and @size properties.
>>
>> It is possible to call memblock_reserve() and memblock_mark_nomap() on
>> the statically-placed reserved memory regions and not need to save them
>> to the array until after paging_init(), but this is not possible for the
>> dynamically-placed reserved memory because the starting address of these
>> regions need to be stored somewhere after they are allocated.
>>
>> Therefore, this series achieves the allocation and population of the
>> reserved_mem array in two steps:
>>
>> 1. Before paging_init()
>>    Before paging_init() is called, iterate through the reserved_mem
>>    nodes in the DT and do the following:
>>    - Allocate memory for dynamically-placed reserved memory regions and
>>      store their starting address in the static allocated reserved_mem
>>      array.
>>    - Call memblock_reserve() and memblock_mark_nomap() on all the
>>      reserved memory regions as needed.
>>    - Count the total number of reserved_mem nodes in the DT.
>>
>> 2. After paging_init()
>>    After paging_init() is called:
>>    - Allocate new memory for the reserved_mem array based on the number
>>      of reserved memory nodes in the DT.
>>    - Transfer all the information that was stored in the static array
>>      into the new array.
>>    - Store the rest of the reserved_mem regions in the new array.
>>      i.e. the statically-placed regions.
>>
>> The static array is no longer needed after this point, but there is
>> currently no obvious way to free the memory. Therefore, the size of the
>> initial static array is now defined using a config option.
> A config option is not going to work here.
>
>> Because the array is used only before paging_init() to store the
>> dynamically-placed reserved memory regions, the required size can vary
>> from device to device. Therefore, scaling it can help get some memory
>> savings.
>>
>> A possible solution to freeing the memory for the static array will be
>> to mark it as __initdata. This will automatically free the memory once
>> the init process is done running.
>> The reason why this is not pursued in this series is because of
>> the possibility of a use-after-free.
>> If the dynamic allocation of the reserved_mem array fails, then future
>> accesses of the reserved_mem array will still be referencing the static
>> array. When the init process ends and the memory is freed up, any
>> further attempts to use the reserved_mem array will result in a
>> use-after-free.
> If memory allocation for the reserved_mem array fails so early in boot, 
> you've got much bigger problems. Use __initdata, and just WARN if 
> allocation fails and continue on (so hopefully the console is brought 
> up and someone can see the WARN).
>
> Rob

Thanks for pointing that out! I'll move forward with implementing the use
of __initdata to delete the static array.

Regards,
Oreoluwa


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v2 2/6] of: reserved_mem: Swicth call to unflatten_device_tree() to after paging_init()
  2023-12-06 21:31   ` Rob Herring
@ 2023-12-11  0:51     ` Oreoluwa Babatunde
  0 siblings, 0 replies; 13+ messages in thread
From: Oreoluwa Babatunde @ 2023-12-11  0:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: catalin.marinas, will, frowand.list, dinguyen, chenhuacai,
	tsbogend, jonas, stefan.kristiansson, shorne, mpe, ysato, dalias,
	glaubitz, richard, anton.ivanov, johannes, chris, jcmvbkbc,
	linux-arm-kernel, linux-kernel, devicetree, linux-arm-msm, kernel


On 12/6/2023 1:31 PM, Rob Herring wrote:
> On Mon, Dec 04, 2023 at 10:54:05AM -0800, Oreoluwa Babatunde wrote:
>> Switch call to unflatten_device_tree() to after paging_init() on other
>> archs to follow new order in which the reserved_mem regions are
>> processed.
> You did this so that you could allocate memory for the reserved regions. 
> But unflatten_device_tree() can already do allocations using memblock. 
> So the same thing should work for you.
>
> I suspect that moving this will break any arch that called an of_* API 
> between the original call and the new call location.
>
> Rob
Hi Rob,

Thank you for your feedback. You are correct, I see that
unflatten_device_tree() is allocating memory from memblock and writing
to it before paging_init() is called on these other architectures.

This series will still require fdt_init_reserved_mem() to be called
after paging_init() on architectures such as arm64 which require
paging_init() to run before memblock allocated memory is writable.

In light of this, there seems to be no non-architecture-specific
universal calling point to stick in fdt_init_reserved_mem() that works
for all architectures.  Therefore, I propose taking out the call to
fdt_init_reserved_mem() from the unflatten_device_tree() function and
placing it in the setup_arch() function for each architecture.
(similar to the way it was in v1 of this series).

Doing this will allow us to place  fdt_init_reserved_mem() after
paging_init() on architectures that require paging_init() to be called
before memblock allocated memory is writable, and place it before
paging_init() on architectures that does not require paging_init() to be
called before writing to memblock allocated memory.

fdt_init_reserved_mem() can also be called after unflatten_device_tree()
on all architectures to ensure we get the speed benefits of using the
unflattened device tree APIs for the final pass.

Regards,
Oreoluwa

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-12-11  0:52 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-04 18:54 [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Oreoluwa Babatunde
2023-12-04 18:54 ` [RFC PATCH v2 1/6] of: reserved_mem: Change the order that reserved_mem regions are stored Oreoluwa Babatunde
2023-12-04 18:54 ` [RFC PATCH v2 2/6] of: reserved_mem: Swicth call to unflatten_device_tree() to after paging_init() Oreoluwa Babatunde
2023-12-06 21:31   ` Rob Herring
2023-12-11  0:51     ` Oreoluwa Babatunde
2023-12-04 18:54 ` [RFC PATCH v2 3/6] of: resevred_mem: Delay allocation of memory for dynamic regions Oreoluwa Babatunde
2023-12-04 18:54 ` [RFC PATCH v2 4/6] of: reserved_mem: Add code to use unflattened DT for reserved_mem nodes Oreoluwa Babatunde
2023-12-04 18:54 ` [RFC PATCH v2 5/6] of: reserved_mem: Add code to dynamically allocate reserved_mem array Oreoluwa Babatunde
2023-12-04 18:54 ` [RFC PATCH v2 6/6] of: reserved_mem: Make MAX_RESERVED_REGIONS a config option Oreoluwa Babatunde
2023-12-06 21:35 ` [RFC PATCH v2 0/6] Dynamic allocation of reserved_mem array Rob Herring
2023-12-11  0:42   ` Oreoluwa Babatunde
  -- strict thread matches above, loose matches on Subject: below --
2023-12-04  4:13 Oreoluwa Babatunde
2023-12-04 18:54 ` Oreoluwa Babatunde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).