The current implementation processes the reserved memory regions in two
stages which are done with two separate functions within the
early_init_fdt_scan_reserved_mem() function.

Within the two stages of processing, the reserved memory regions are
broken up into two groups which are processed differently:
i) Statically-placed reserved memory regions
i.e. regions defined with a static start address and size using the
     "reg" property in the DT.
ii) Dynamically-placed reserved memory regions.
i.e. regions defined by specifying a range of addresses where they can
     be placed in memory using the "alloc_ranges" and "size" properties
     in the DT.

Stage 1: fdt_scan_reserved_mem()
This stage of the reserved memory processing is used to scan through the
reserved memory nodes defined in the devicetree and do the following on
each of the nodes:

1) If the node represents a statically-placed reserved memory region,
   i.e. it is defined using the "reg" property:
   - Call memblock_reserve() or memblock_mark_nomap() as needed.
   - Add the information for the reserved region to the reserved_mem
     array.
     eg: fdt_reserved_mem_save_node(node, name, base, size);

2) If the node represents a dynamically-placed reserved memory region,
   i.e. it is defined using "alloc-ranges" and "size" properties:
   - Add the information for the region to the reserved_mem array with
     the starting address and size set to 0.
     eg: fdt_reserved_mem_save_node(node, name, 0, 0);

Stage 2: fdt_init_reserved_mem()
This stage of the reserved memory processing is used to iterate through
the reserved_mem array which was populated in stage 1 and do the
following on each of the entries:

1) If the entry represents a statically-placed reserved memory region:
   - Call the region specific init function.
2) If the entry represents a dynamically-placed reserved memory region:
   - Call __reserved_mem_alloc_size() which is used to allocate memory
     for the region using memblock_phys_alloc_range(), and call
     memblock_mark_nomap() on the allocated region if the region is
     specified as a no-map region.
   - Call the region specific init function.

On architectures such as arm64, the dynamic allocation of the
reserved_mem array needs to be done after the page tables have been
setup because memblock allocated memory is not writable until then. This
means that the reserved_mem array will not be available to store any
reserved memory information until after the page tables have been setup.

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 reserved_mem array until later. This is because all the
information we need is present in the devicetree.
Dynamically-placed reserved memory regions on the other hand get
assigned a start address only at runtime, and since memblock_reserve()
and memblock_mark_nomap() need to be called before the memory mappings
are created, the allocation needs to happen before the page tables are
setup.

To make it easier to handle dynamically-placed reserved memory regions
before the page tables are setup, this patch makes changes to the steps
above to process the reserved memory regions in the following ways:

Step 1: fdt_scan_reserved_mem()
This stage of the reserved memory processing is used to scan through the
reserved memory nodes defined in the devicetree and do the following on
each of the nodes:

1) If the node represents a statically-placed reserved memory region,
   i.e. it is defined using the "reg" property:
   - Call memblock_reserve() or memblock_mark_nomap() as needed.

2) If the node represents a dynamically-placed reserved memory region,
   i.e. it is defined using "alloc-ranges" and "size" properties:
   - Call __reserved_mem_alloc_size() which will:
     i) Allocate memory for the reserved memory region.
     ii) Call memblock_mark_nomap() as needed.
     Note: There is no need to explicitly call memblock_reserve() here
     because it is already called by memblock when the memory for the
     region is being allocated.
     iii) Save the information for the region in the reserved_mem array.

Step 2: fdt_init_reserved_mem()
This stage of the reserved memory processing is used to:

1) Add the information for the statically-placed reserved memory into
   the reserved_mem array.

2) Iterate through all the entries in the array and call the region
   specific init function for each of them.

fdt_init_reserved_mem() is also now called from within the
unflatten_device_tree() function so that this step happens after the
page tables have been setup.

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

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index a8a04f27915b..527e6bc1c096 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -532,8 +532,6 @@ void __init early_init_fdt_scan_reserved_mem(void)
 			break;
 		memblock_reserve(base, size);
 	}
-
-	fdt_init_reserved_mem();
 }
 
 /**
@@ -1259,6 +1257,9 @@ void __init unflatten_device_tree(void)
 	of_alias_scan(early_init_dt_alloc_memory_arch);
 
 	unittest_unflatten_overlay_base();
+
+	/* initialize the reserved memory regions */
+	fdt_init_reserved_mem();
 }
 
 /**
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 94fc0aa07af9..d889b8c17ca3 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -9,6 +9,7 @@
  */
 
 #define FDT_ALIGN_SIZE 8
+#define MAX_RESERVED_REGIONS    64
 
 /**
  * struct alias_prop - Alias property in 'aliases' node
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 46e1c3fbc769..78ffc2635f11 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -27,7 +27,6 @@
 
 #include "of_private.h"
 
-#define MAX_RESERVED_REGIONS	64
 static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
 static int reserved_mem_count;
 
@@ -106,7 +105,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);
@@ -134,10 +132,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;
 }
@@ -165,12 +159,69 @@ static int __init __reserved_mem_check_root(unsigned long node)
 	return 0;
 }
 
+/**
+ * fdt_scan_reserved_mem_reg_nodes() - Store info for the "reg" defined
+ * reserved memory regions.
+ *
+ * This function is used to scan through the DT and store the
+ * information for the reserved memory regions that are defined using
+ * the "reg" property. The region node number, name, base address, and
+ * size are all stored in the reserved_mem array by calling the
+ * fdt_reserved_mem_save_node() function.
+ */
+static 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_info("Reserved memory: No reserved-memory node in the DT\n");
+		return;
+	}
+
+	if (__reserved_mem_check_root(node)) {
+		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);
+	}
+}
+
+static int __init __reserved_mem_alloc_size(unsigned long node, const char *uname);
+
 /*
  * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
  */
 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");
@@ -192,8 +243,24 @@ 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))
-			fdt_reserved_mem_save_node(child, uname, 0, 0);
+		/*
+		 * Save the nodes for the dynamically-placed regions
+		 * into an array which will be used for allocation right
+		 * after all the statically-placed regions are reserved
+		 * or marked as no-map. This is done to avoid dynamically
+		 * allocating from one of the statically-placed regions.
+		 */
+		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;
 }
@@ -253,8 +320,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)
+static 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;
@@ -333,10 +399,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;
 }
 
@@ -431,6 +494,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();
 
@@ -442,30 +507,23 @@ void __init fdt_init_reserved_mem(void)
 
 		nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
 
-		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");
 		}
 	}
 }
-- 
2.34.1

Hi Oreoluwa,

On Tue, May 28, 2024 at 03:36:47PM -0700, Oreoluwa Babatunde wrote:
> The current implementation processes the reserved memory regions in two
> stages which are done with two separate functions within the
> early_init_fdt_scan_reserved_mem() function.
> 
> Within the two stages of processing, the reserved memory regions are
> broken up into two groups which are processed differently:
> i) Statically-placed reserved memory regions
> i.e. regions defined with a static start address and size using the
>      "reg" property in the DT.
> ii) Dynamically-placed reserved memory regions.
> i.e. regions defined by specifying a range of addresses where they can
>      be placed in memory using the "alloc_ranges" and "size" properties
>      in the DT.
> 
> Stage 1: fdt_scan_reserved_mem()
> This stage of the reserved memory processing is used to scan through the
> reserved memory nodes defined in the devicetree and do the following on
> each of the nodes:
> 
> 1) If the node represents a statically-placed reserved memory region,
>    i.e. it is defined using the "reg" property:
>    - Call memblock_reserve() or memblock_mark_nomap() as needed.
>    - Add the information for the reserved region to the reserved_mem
>      array.
>      eg: fdt_reserved_mem_save_node(node, name, base, size);
> 
> 2) If the node represents a dynamically-placed reserved memory region,
>    i.e. it is defined using "alloc-ranges" and "size" properties:
>    - Add the information for the region to the reserved_mem array with
>      the starting address and size set to 0.
>      eg: fdt_reserved_mem_save_node(node, name, 0, 0);
> 
> Stage 2: fdt_init_reserved_mem()
> This stage of the reserved memory processing is used to iterate through
> the reserved_mem array which was populated in stage 1 and do the
> following on each of the entries:
> 
> 1) If the entry represents a statically-placed reserved memory region:
>    - Call the region specific init function.
> 2) If the entry represents a dynamically-placed reserved memory region:
>    - Call __reserved_mem_alloc_size() which is used to allocate memory
>      for the region using memblock_phys_alloc_range(), and call
>      memblock_mark_nomap() on the allocated region if the region is
>      specified as a no-map region.
>    - Call the region specific init function.
> 
> On architectures such as arm64, the dynamic allocation of the
> reserved_mem array needs to be done after the page tables have been
> setup because memblock allocated memory is not writable until then. This
> means that the reserved_mem array will not be available to store any
> reserved memory information until after the page tables have been setup.
> 
> 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 reserved_mem array until later. This is because all the
> information we need is present in the devicetree.
> Dynamically-placed reserved memory regions on the other hand get
> assigned a start address only at runtime, and since memblock_reserve()
> and memblock_mark_nomap() need to be called before the memory mappings
> are created, the allocation needs to happen before the page tables are
> setup.
> 
> To make it easier to handle dynamically-placed reserved memory regions
> before the page tables are setup, this patch makes changes to the steps
> above to process the reserved memory regions in the following ways:
> 
> Step 1: fdt_scan_reserved_mem()
> This stage of the reserved memory processing is used to scan through the
> reserved memory nodes defined in the devicetree and do the following on
> each of the nodes:
> 
> 1) If the node represents a statically-placed reserved memory region,
>    i.e. it is defined using the "reg" property:
>    - Call memblock_reserve() or memblock_mark_nomap() as needed.
> 
> 2) If the node represents a dynamically-placed reserved memory region,
>    i.e. it is defined using "alloc-ranges" and "size" properties:
>    - Call __reserved_mem_alloc_size() which will:
>      i) Allocate memory for the reserved memory region.
>      ii) Call memblock_mark_nomap() as needed.
>      Note: There is no need to explicitly call memblock_reserve() here
>      because it is already called by memblock when the memory for the
>      region is being allocated.
>      iii) Save the information for the region in the reserved_mem array.
> 
> Step 2: fdt_init_reserved_mem()
> This stage of the reserved memory processing is used to:
> 
> 1) Add the information for the statically-placed reserved memory into
>    the reserved_mem array.
> 
> 2) Iterate through all the entries in the array and call the region
>    specific init function for each of them.
> 
> fdt_init_reserved_mem() is also now called from within the
> unflatten_device_tree() function so that this step happens after the
> page tables have been setup.
> 
> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>

I am seeing a warning when booting aspeed_g5_defconfig in QEMU that I
bisected to this change in -next as commit a46cccb0ee2d ("of:
reserved_mem: Restruture how the reserved memory regions are
processed").

$ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- aspeed_g5_defconfig zImage aspeed/aspeed-bmc-opp-romulus.dtb

$ qemu-system-arm \
    -display none \
    -nodefaults \
    -no-reboot \
    -machine romulus-bmc \
    -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-opp-romulus.dtb \
    -kernel arch/arm/boot/zImage \
    -initrd rootfs.cpio \
    -m 512m \
    -serial mon:stdio
...
[    0.000000] Linux version 6.10.0-rc1-00007-ga46cccb0ee2d (nathan@thelio-3990X) (arm-linux-gnueabi-gcc (GCC) 13.2.0, GNU ld (GNU Binutils) 2.41) #1 SMP Mon Jun 10 14:20:06 MST 2024
...
[    0.623194] [drm] Initialized aspeed-gfx-drm 1.0.0 20180319 for 1e6e6000.display on minor 0
[    0.636348] ------------[ cut here ]------------
[    0.636613] WARNING: CPU: 0 PID: 1 at mm/memory.c:2789 __apply_to_page_range+0x360/0x380
[    0.637601] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.10.0-rc1-00007-ga46cccb0ee2d #1
[    0.638001] Hardware name: Generic DT based system
[    0.638298] Call trace:
[    0.638743]  unwind_backtrace from show_stack+0x18/0x1c
[    0.639163]  show_stack from dump_stack_lvl+0x54/0x68
[    0.639374]  dump_stack_lvl from __warn+0x74/0x114
[    0.639590]  __warn from warn_slowpath_fmt+0x13c/0x1c0
[    0.639855]  warn_slowpath_fmt from __apply_to_page_range+0x360/0x380
[    0.640110]  __apply_to_page_range from apply_to_page_range+0x24/0x2c
[    0.640356]  apply_to_page_range from __alloc_from_contiguous+0xc4/0x158
[    0.640619]  __alloc_from_contiguous from cma_allocator_alloc+0x3c/0x44
[    0.640870]  cma_allocator_alloc from arch_dma_alloc+0x128/0x2b4
[    0.641100]  arch_dma_alloc from dma_alloc_attrs+0x90/0x150
[    0.641318]  dma_alloc_attrs from drm_gem_dma_create+0xa4/0x13c
[    0.641553]  drm_gem_dma_create from drm_gem_dma_create_with_handle+0x24/0xac
[    0.641833]  drm_gem_dma_create_with_handle from drm_gem_dma_dumb_create+0x44/0x50
[    0.642122]  drm_gem_dma_dumb_create from drm_client_framebuffer_create+0x9c/0x164
[    0.642410]  drm_client_framebuffer_create from drm_fbdev_dma_helper_fb_probe+0x84/0x23c
[    0.642724]  drm_fbdev_dma_helper_fb_probe from __drm_fb_helper_initial_config_and_unlock+0x2e4/0x4f8
[    0.643076]  __drm_fb_helper_initial_config_and_unlock from drm_fbdev_dma_client_hotplug+0x74/0xb8
[    0.643416]  drm_fbdev_dma_client_hotplug from drm_client_register+0x5c/0x98
[    0.643696]  drm_client_register from aspeed_gfx_probe+0x278/0x3c0
[    0.643935]  aspeed_gfx_probe from platform_probe+0x60/0xb8
[    0.644192]  platform_probe from really_probe+0xd4/0x3b4
[    0.644434]  really_probe from __driver_probe_device+0x90/0x1dc
[    0.644697]  __driver_probe_device from driver_probe_device+0x38/0xd0
[    0.644978]  driver_probe_device from __driver_attach+0x118/0x1dc
[    0.645246]  __driver_attach from bus_for_each_dev+0x84/0xd4
[    0.645498]  bus_for_each_dev from bus_add_driver+0xec/0x1f0
[    0.645755]  bus_add_driver from driver_register+0x84/0x11c
[    0.645999]  driver_register from do_one_initcall+0x84/0x1c8
[    0.646351]  do_one_initcall from kernel_init_freeable+0x1a4/0x230
[    0.646624]  kernel_init_freeable from kernel_init+0x1c/0x138
[    0.646885]  kernel_init from ret_from_fork+0x14/0x28
[    0.647154] Exception stack(0x9f015fb0 to 0x9f015ff8)
[    0.647535] 5fa0:                                     00000000 00000000 00000000 00000000
[    0.647895] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.648237] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    0.648730] ---[ end trace 0000000000000000 ]---
[    0.653499] aspeed_gfx 1e6e6000.display: [drm] fb0: aspeed-gfx-drmd frame buffer device
...

The rootfs is from [1] if you should need it (arm-rootfs.cpio.zst,
decompress with zstd first). I am more than happy to provide any
additional information or test patches as necessary.

[1]: https://github.com/ClangBuiltLinux/boot-utils/releases

Cheers,
Nathan

# bad: [d35b2284e966c0bef3e2182a5c5ea02177dd32e4] Add linux-next specific files for 20240607
# good: [8a92980606e3585d72d510a03b59906e96755b8a] Merge tag 'scsi-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi
git bisect start 'd35b2284e966c0bef3e2182a5c5ea02177dd32e4' '8a92980606e3585d72d510a03b59906e96755b8a'
# good: [faef37a085e57f29479f853624948cdc7df6e366] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git
git bisect good faef37a085e57f29479f853624948cdc7df6e366
# good: [822946749b5f02d9f49dde4b4cb8f2535c247ce5] Merge branch 'drm-xe-next' of https://gitlab.freedesktop.org/drm/xe/kernel
git bisect good 822946749b5f02d9f49dde4b4cb8f2535c247ce5
# bad: [9ffebdc86bc23fdf0622eb0c38d395c2b99b7f32] Merge branch 'next' of git://git.kernel.org/pub/scm/virt/kvm/kvm.git
git bisect bad 9ffebdc86bc23fdf0622eb0c38d395c2b99b7f32
# bad: [9a85e49be89a9150fd2ec9964f48013a00c261d1] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
git bisect bad 9a85e49be89a9150fd2ec9964f48013a00c261d1
# good: [bd8c07372f9f6b8e496b7f1f0dcc287dd81c0706] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux
git bisect good bd8c07372f9f6b8e496b7f1f0dcc287dd81c0706
# good: [a93b9dea60d7c46dca8de8d5180f3ceb3696ab8a] Merge branch 'apparmor-next' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
git bisect good a93b9dea60d7c46dca8de8d5180f3ceb3696ab8a
# good: [c1b93986dfb2a31b0528fe929d574843801089f5] spi: pxa2xx: Remove hard coded number of chip select pins
git bisect good c1b93986dfb2a31b0528fe929d574843801089f5
# good: [76a621455f7be3d9c963c8a1a27b234d27d14f16] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git
git bisect good 76a621455f7be3d9c963c8a1a27b234d27d14f16
# bad: [16227bf21dff0905bfa303f8cbd9a2f7ca51f883] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
git bisect bad 16227bf21dff0905bfa303f8cbd9a2f7ca51f883
# good: [77b023ba4f45ce2bf26edb095b4a81829c9e621b] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G2UL CSI-2 block
git bisect good 77b023ba4f45ce2bf26edb095b4a81829c9e621b
# bad: [b8ffe4a11001660522ccc5b2cc892cf8aa03a62e] of: reserved_mem: Add code to dynamically allocate reserved_mem array
git bisect bad b8ffe4a11001660522ccc5b2cc892cf8aa03a62e
# bad: [a46cccb0ee2d62206f6db5e3b85fdd9e2760ee1a] of: reserved_mem: Restruture how the reserved memory regions are processed
git bisect bad a46cccb0ee2d62206f6db5e3b85fdd9e2760ee1a
# good: [83138f8fb798627531be3b5627af4a6008a7bbd6] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G2UL CRU block
git bisect good 83138f8fb798627531be3b5627af4a6008a7bbd6
# first bad commit: [a46cccb0ee2d62206f6db5e3b85fdd9e2760ee1a] of: reserved_mem: Restruture how the reserved memory regions are processed

On Mon, Jun 10, 2024 at 02:34:03PM -0700, Nathan Chancellor wrote:
> On Tue, May 28, 2024 at 03:36:47PM -0700, Oreoluwa Babatunde wrote:

> > fdt_init_reserved_mem() is also now called from within the
> > unflatten_device_tree() function so that this step happens after the
> > page tables have been setup.

> > Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>

> I am seeing a warning when booting aspeed_g5_defconfig in QEMU that I
> bisected to this change in -next as commit a46cccb0ee2d ("of:
> reserved_mem: Restruture how the reserved memory regions are
> processed").

I'm also seeing issues in -next which I bisected to this commit, on the
original Raspberry Pi the cpufreq driver fails to come up and I see
(potentially separate?) backtraces:

[    0.100390] ------------[ cut here ]------------
[    0.100476] WARNING: CPU: 0 PID: 1 at mm/memory.c:2835 __apply_to_page_range+0xd4/0x2c8
[    0.100637] Modules linked in:
[    0.100665] CPU: 0 PID: 1 Comm: swapper Not tainted 6.10.0-rc2-next-20240607 #1
[    0.100692] Hardware name: BCM2835
[    0.100705] Call trace: 
[    0.100727]  unwind_backtrace from show_stack+0x18/0x1c
[    0.100790]  show_stack from dump_stack_lvl+0x38/0x48
[    0.100833]  dump_stack_lvl from __warn+0x8c/0xf4
[    0.100888]  __warn from warn_slowpath_fmt+0x80/0xbc
[    0.100933]  warn_slowpath_fmt from __apply_to_page_range+0xd4/0x2c8
[    0.100983]  __apply_to_page_range from apply_to_page_range+0x20/0x28
[    0.101027]  apply_to_page_range from __dma_remap+0x58/0x88
[    0.101071]  __dma_remap from __alloc_from_contiguous+0x6c/0xa8
[    0.101106]  __alloc_from_contiguous from atomic_pool_init+0x9c/0x1c4
[    0.101169]  atomic_pool_init from do_one_initcall+0x68/0x158
[    0.101223]  do_one_initcall from kernel_init_freeable+0x1ac/0x1f0
[    0.101267]  kernel_init_freeable from kernel_init+0x1c/0x140
[    0.101309]  kernel_init from ret_from_fork+0x14/0x28
[    0.101344] Exception stack(0xdc80dfb0 to 0xdc80dff8)
[    0.101369] dfa0:                                     00000000 00000000 00000000 00000000
[    0.101393] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.101414] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    0.101428] ---[ end trace 0000000000000000 ]---

Full boot log at:

   https://lava.sirena.org.uk/scheduler/job/374962

You can see the report of cpufreq not being loaded in the log.

NFS boots also fail, apparently due to slowness bringing up a Debian
userspace which may well be due to cpufreq isues:

On Mon, Jun 10, 2024 at 10:47:18PM +0100, Mark Brown wrote:
> On Mon, Jun 10, 2024 at 02:34:03PM -0700, Nathan Chancellor wrote:
> > On Tue, May 28, 2024 at 03:36:47PM -0700, Oreoluwa Babatunde wrote:
> 
> > > fdt_init_reserved_mem() is also now called from within the
> > > unflatten_device_tree() function so that this step happens after the
> > > page tables have been setup.
> 
> > > Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
> 
> > I am seeing a warning when booting aspeed_g5_defconfig in QEMU that I
> > bisected to this change in -next as commit a46cccb0ee2d ("of:
> > reserved_mem: Restruture how the reserved memory regions are
> > processed").
> 
> I'm also seeing issues in -next which I bisected to this commit, on the
> original Raspberry Pi the cpufreq driver fails to come up and I see
> (potentially separate?) backtraces:

This is still in -next and breaking boot as reported above.

On 6/25/2024 2:25 PM, Mark Brown wrote:
>> I'm also seeing issues in -next which I bisected to this commit, on the
>> original Raspberry Pi the cpufreq driver fails to come up and I see
>> (potentially separate?) backtraces:
> This is still in -next and breaking boot as reported above.
Hi Mark,

I uploaded a fix about a week ago here:
https://lore.kernel.org/all/20240620001027.2326275-1-quic_obabatun@quicinc.com/

It is still pending review and expected to be merged if everything
is okay.

You can use this and see if it works for you. If it does, you can
also add a 'Tested-by:' tag to help speed things up.

Thank you!
Oreoluwa

On 6/10/2024 2:47 PM, Mark Brown wrote:
> On Mon, Jun 10, 2024 at 02:34:03PM -0700, Nathan Chancellor wrote:
>> On Tue, May 28, 2024 at 03:36:47PM -0700, Oreoluwa Babatunde wrote:
>>> fdt_init_reserved_mem() is also now called from within the
>>> unflatten_device_tree() function so that this step happens after the
>>> page tables have been setup.
>>> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
>> I am seeing a warning when booting aspeed_g5_defconfig in QEMU that I
>> bisected to this change in -next as commit a46cccb0ee2d ("of:
>> reserved_mem: Restruture how the reserved memory regions are
>> processed").
> I'm also seeing issues in -next which I bisected to this commit, on the
> original Raspberry Pi the cpufreq driver fails to come up and I see
> (potentially separate?) backtraces:
>
> [    0.100390] ------------[ cut here ]------------
> [    0.100476] WARNING: CPU: 0 PID: 1 at mm/memory.c:2835 __apply_to_page_range+0xd4/0x2c8
> [    0.100637] Modules linked in:
> [    0.100665] CPU: 0 PID: 1 Comm: swapper Not tainted 6.10.0-rc2-next-20240607 #1
> [    0.100692] Hardware name: BCM2835
> [    0.100705] Call trace: 
> [    0.100727]  unwind_backtrace from show_stack+0x18/0x1c
> [    0.100790]  show_stack from dump_stack_lvl+0x38/0x48
> [    0.100833]  dump_stack_lvl from __warn+0x8c/0xf4
> [    0.100888]  __warn from warn_slowpath_fmt+0x80/0xbc
> [    0.100933]  warn_slowpath_fmt from __apply_to_page_range+0xd4/0x2c8
> [    0.100983]  __apply_to_page_range from apply_to_page_range+0x20/0x28
> [    0.101027]  apply_to_page_range from __dma_remap+0x58/0x88
> [    0.101071]  __dma_remap from __alloc_from_contiguous+0x6c/0xa8
> [    0.101106]  __alloc_from_contiguous from atomic_pool_init+0x9c/0x1c4
> [    0.101169]  atomic_pool_init from do_one_initcall+0x68/0x158
> [    0.101223]  do_one_initcall from kernel_init_freeable+0x1ac/0x1f0
> [    0.101267]  kernel_init_freeable from kernel_init+0x1c/0x140
> [    0.101309]  kernel_init from ret_from_fork+0x14/0x28
> [    0.101344] Exception stack(0xdc80dfb0 to 0xdc80dff8)
> [    0.101369] dfa0:                                     00000000 00000000 00000000 00000000
> [    0.101393] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    0.101414] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [    0.101428] ---[ end trace 0000000000000000 ]---
>
> Full boot log at:
>
>    https://lava.sirena.org.uk/scheduler/job/374962
>
> You can see the report of cpufreq not being loaded in the log.
>
> NFS boots also fail, apparently due to slowness bringing up a Debian
> userspace which may well be due to cpufreq isues:
Hi Mark & Nathan,

Taking a look at this now and will provide a fix soon if
needed.

At first glance, it looks like there are a couple of WARN_ON*
function calls in __apply_to_page_range(). Please could
you run faddr2line and tell me which of the WARN_ON*
cases we are hitting?

Thank you!

Oreoluwa

On Thu, Jun 13, 2024 at 09:05:18AM -0700, Oreoluwa Babatunde wrote:
> 
> On 6/10/2024 2:47 PM, Mark Brown wrote:
> > On Mon, Jun 10, 2024 at 02:34:03PM -0700, Nathan Chancellor wrote:
> >> On Tue, May 28, 2024 at 03:36:47PM -0700, Oreoluwa Babatunde wrote:
> >>> fdt_init_reserved_mem() is also now called from within the
> >>> unflatten_device_tree() function so that this step happens after the
> >>> page tables have been setup.
> >>> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
> >> I am seeing a warning when booting aspeed_g5_defconfig in QEMU that I
> >> bisected to this change in -next as commit a46cccb0ee2d ("of:
> >> reserved_mem: Restruture how the reserved memory regions are
> >> processed").
> > I'm also seeing issues in -next which I bisected to this commit, on the
> > original Raspberry Pi the cpufreq driver fails to come up and I see
> > (potentially separate?) backtraces:
> >
> > [    0.100390] ------------[ cut here ]------------
> > [    0.100476] WARNING: CPU: 0 PID: 1 at mm/memory.c:2835 __apply_to_page_range+0xd4/0x2c8
> > [    0.100637] Modules linked in:
> > [    0.100665] CPU: 0 PID: 1 Comm: swapper Not tainted 6.10.0-rc2-next-20240607 #1
> > [    0.100692] Hardware name: BCM2835
> > [    0.100705] Call trace: 
> > [    0.100727]  unwind_backtrace from show_stack+0x18/0x1c
> > [    0.100790]  show_stack from dump_stack_lvl+0x38/0x48
> > [    0.100833]  dump_stack_lvl from __warn+0x8c/0xf4
> > [    0.100888]  __warn from warn_slowpath_fmt+0x80/0xbc
> > [    0.100933]  warn_slowpath_fmt from __apply_to_page_range+0xd4/0x2c8
> > [    0.100983]  __apply_to_page_range from apply_to_page_range+0x20/0x28
> > [    0.101027]  apply_to_page_range from __dma_remap+0x58/0x88
> > [    0.101071]  __dma_remap from __alloc_from_contiguous+0x6c/0xa8
> > [    0.101106]  __alloc_from_contiguous from atomic_pool_init+0x9c/0x1c4
> > [    0.101169]  atomic_pool_init from do_one_initcall+0x68/0x158
> > [    0.101223]  do_one_initcall from kernel_init_freeable+0x1ac/0x1f0
> > [    0.101267]  kernel_init_freeable from kernel_init+0x1c/0x140
> > [    0.101309]  kernel_init from ret_from_fork+0x14/0x28
> > [    0.101344] Exception stack(0xdc80dfb0 to 0xdc80dff8)
> > [    0.101369] dfa0:                                     00000000 00000000 00000000 00000000
> > [    0.101393] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > [    0.101414] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > [    0.101428] ---[ end trace 0000000000000000 ]---
> >
> > Full boot log at:
> >
> >    https://lava.sirena.org.uk/scheduler/job/374962
> >
> > You can see the report of cpufreq not being loaded in the log.
> >
> > NFS boots also fail, apparently due to slowness bringing up a Debian
> > userspace which may well be due to cpufreq isues:
> Hi Mark & Nathan,
> 
> Taking a look at this now and will provide a fix soon if
> needed.
> 
> At first glance, it looks like there are a couple of WARN_ON*
> function calls in __apply_to_page_range(). Please could
> you run faddr2line and tell me which of the WARN_ON*
> cases we are hitting?

That shouldn't be needed, right? The line is in the WARNING: mm/memory.c:2835
which, in next-20240607, is: if (WARN_ON_ONCE(pmd_leaf(*pmd))).

Thanks,
Conor.

On 13/06/2024 5:17 pm, Conor Dooley wrote:
> On Thu, Jun 13, 2024 at 09:05:18AM -0700, Oreoluwa Babatunde wrote:
>>
>> On 6/10/2024 2:47 PM, Mark Brown wrote:
>>> On Mon, Jun 10, 2024 at 02:34:03PM -0700, Nathan Chancellor wrote:
>>>> On Tue, May 28, 2024 at 03:36:47PM -0700, Oreoluwa Babatunde wrote:
>>>>> fdt_init_reserved_mem() is also now called from within the
>>>>> unflatten_device_tree() function so that this step happens after the
>>>>> page tables have been setup.
>>>>> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
>>>> I am seeing a warning when booting aspeed_g5_defconfig in QEMU that I
>>>> bisected to this change in -next as commit a46cccb0ee2d ("of:
>>>> reserved_mem: Restruture how the reserved memory regions are
>>>> processed").
>>> I'm also seeing issues in -next which I bisected to this commit, on the
>>> original Raspberry Pi the cpufreq driver fails to come up and I see
>>> (potentially separate?) backtraces:
>>>
>>> [    0.100390] ------------[ cut here ]------------
>>> [    0.100476] WARNING: CPU: 0 PID: 1 at mm/memory.c:2835 __apply_to_page_range+0xd4/0x2c8
>>> [    0.100637] Modules linked in:
>>> [    0.100665] CPU: 0 PID: 1 Comm: swapper Not tainted 6.10.0-rc2-next-20240607 #1
>>> [    0.100692] Hardware name: BCM2835
>>> [    0.100705] Call trace:
>>> [    0.100727]  unwind_backtrace from show_stack+0x18/0x1c
>>> [    0.100790]  show_stack from dump_stack_lvl+0x38/0x48
>>> [    0.100833]  dump_stack_lvl from __warn+0x8c/0xf4
>>> [    0.100888]  __warn from warn_slowpath_fmt+0x80/0xbc
>>> [    0.100933]  warn_slowpath_fmt from __apply_to_page_range+0xd4/0x2c8
>>> [    0.100983]  __apply_to_page_range from apply_to_page_range+0x20/0x28
>>> [    0.101027]  apply_to_page_range from __dma_remap+0x58/0x88
>>> [    0.101071]  __dma_remap from __alloc_from_contiguous+0x6c/0xa8
>>> [    0.101106]  __alloc_from_contiguous from atomic_pool_init+0x9c/0x1c4
>>> [    0.101169]  atomic_pool_init from do_one_initcall+0x68/0x158
>>> [    0.101223]  do_one_initcall from kernel_init_freeable+0x1ac/0x1f0
>>> [    0.101267]  kernel_init_freeable from kernel_init+0x1c/0x140
>>> [    0.101309]  kernel_init from ret_from_fork+0x14/0x28
>>> [    0.101344] Exception stack(0xdc80dfb0 to 0xdc80dff8)
>>> [    0.101369] dfa0:                                     00000000 00000000 00000000 00000000
>>> [    0.101393] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>> [    0.101414] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>>> [    0.101428] ---[ end trace 0000000000000000 ]---
>>>
>>> Full boot log at:
>>>
>>>     https://lava.sirena.org.uk/scheduler/job/374962
>>>
>>> You can see the report of cpufreq not being loaded in the log.
>>>
>>> NFS boots also fail, apparently due to slowness bringing up a Debian
>>> userspace which may well be due to cpufreq isues:
>> Hi Mark & Nathan,
>>
>> Taking a look at this now and will provide a fix soon if
>> needed.
>>
>> At first glance, it looks like there are a couple of WARN_ON*
>> function calls in __apply_to_page_range(). Please could
>> you run faddr2line and tell me which of the WARN_ON*
>> cases we are hitting?
> 
> That shouldn't be needed, right? The line is in the WARNING: mm/memory.c:2835
> which, in next-20240607, is: if (WARN_ON_ONCE(pmd_leaf(*pmd))).

Indeed, and the overall implication there would seem to be that, because 
the dynamic CMA region wasn't allocated and reserved before we created 
the pagetables, we thus haven't created the pagetables in a shape which 
can accommodate chopping it out again later. Note that on arm64 at 
least, this is liable to be hidden by other options like rodata_full and 
debug_pagealloc - see can_set_direct_map().

Thanks,
Robin.

After all the reserved memory regions have been added to the
reserved_mem array, a region specific initialization function is called
on each of reserved memory regions in a loop to initialize them.

With recent changes made to allow the reserved_mem array be dynamically
allocated, the cma reserved memory regions are not initialized until
after the page tables are setup. This causes the warning seen in the
dump stack below:

	WARNING: CPU: 0 PID: 1 at mm/memory.c:2789 __apply_to_page_range+0x360/0x380
	CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.10.0-rc1-00007-ga46cccb0ee2d #1
	Hardware name: Generic DT based system
	Call trace:
	unwind_backtrace from show_stack+0x18/0x1c
	show_stack from dump_stack_lvl+0x54/0x68
	dump_stack_lvl from __warn+0x74/0x114
	__warn from warn_slowpath_fmt+0x13c/0x1c0
	warn_slowpath_fmt from __apply_to_page_range+0x360/0x380
	__apply_to_page_range from apply_to_page_range+0x24/0x2c
	apply_to_page_range from __alloc_from_contiguous+0xc4/0x158
	__alloc_from_contiguous from cma_allocator_alloc+0x3c/0x44
	cma_allocator_alloc from arch_dma_alloc+0x128/0x2b4
	arch_dma_alloc from dma_alloc_attrs+0x90/0x150
	dma_alloc_attrs from drm_gem_dma_create+0xa4/0x13c
	drm_gem_dma_create from drm_gem_dma_create_with_handle+0x24/0xac
	drm_gem_dma_create_with_handle from drm_gem_dma_dumb_create+0x44/0x50
	drm_gem_dma_dumb_create from drm_client_framebuffer_create+0x9c/0x164
	drm_client_framebuffer_create from drm_fbdev_dma_helper_fb_probe+0x84/0x23c
	drm_fbdev_dma_helper_fb_probe from __drm_fb_helper_initial_config_and_unlock+0x2e4/0x4f8
	__drm_fb_helper_initial_config_and_unlock from drm_fbdev_dma_client_hotplug+0x74/0xb8
	drm_fbdev_dma_client_hotplug from drm_client_register+0x5c/0x98
	drm_client_register from aspeed_gfx_probe+0x278/0x3c0
	aspeed_gfx_probe from platform_probe+0x60/0xb8
	platform_probe from really_probe+0xd4/0x3b4
	really_probe from __driver_probe_device+0x90/0x1dc
	__driver_probe_device from driver_probe_device+0x38/0xd0
	driver_probe_device from __driver_attach+0x118/0x1dc
	__driver_attach from bus_for_each_dev+0x84/0xd4
	bus_for_each_dev from bus_add_driver+0xec/0x1f0
	bus_add_driver from driver_register+0x84/0x11c
	driver_register from do_one_initcall+0x84/0x1c8
	do_one_initcall from kernel_init_freeable+0x1a4/0x230
	kernel_init_freeable from kernel_init+0x1c/0x138
	kernel_init from ret_from_fork+0x14/0x28
	Exception stack(0x9f015fb0 to 0x9f015ff8)
	5fa0:                                     00000000 00000000 00000000 00000000
	5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
	5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
	---[ end trace 0000000000000000 ]---
	aspeed_gfx 1e6e6000.display: [drm] fb0: aspeed-gfx-drmd frame buffer device

Hence, restructure the code to initialize the regions as soon as each
of them are added to the reserved_mem array.

Fixes: a46cccb0ee2d ("of: reserved_mem: Restruture how the reserved memory regions are processed")
Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
---
 drivers/of/fdt.c             |  2 +-
 drivers/of/of_private.h      |  2 +-
 drivers/of/of_reserved_mem.c | 77 ++++++++++++++++++++----------------
 3 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 9cde2abd2fc0..ea2dff0478c7 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1239,7 +1239,7 @@ void __init unflatten_device_tree(void)
 	unittest_unflatten_overlay_base();
 
 	/* initialize the reserved memory regions */
-	of_init_reserved_mem();
+	of_scan_reserved_mem_reg_nodes();
 }
 
 /**
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 01b33c4b1e9f..7412aed903df 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -181,7 +181,7 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node *
 #endif
 
 int fdt_scan_reserved_mem(void);
-void of_init_reserved_mem(void);
+void of_scan_reserved_mem_reg_nodes(void);
 
 bool of_fdt_device_is_available(const void *blob, unsigned long node);
 
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index eb54490a0a11..0b1eb34b891e 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -97,6 +97,8 @@ static void __init alloc_reserved_mem_array(void)
 	reserved_mem = new_array;
 }
 
+static void __init of_init_reserved_mem_node(struct reserved_mem *rmem);
+
 /*
  * of_reserved_mem_save_node() - save fdt node for second pass initialization
  */
@@ -115,6 +117,12 @@ static void __init of_reserved_mem_save_node(struct device_node *node, const cha
 	rmem->base = base;
 	rmem->size = size;
 
+	/*
+	 * Run the region specific initialization function for the rmem
+	 * node.
+	 */
+	of_init_reserved_mem_node(rmem);
+
 	reserved_mem_count++;
 	return;
 }
@@ -201,6 +209,8 @@ static int __init __fdt_reserved_mem_check_root(unsigned long node)
 	return 0;
 }
 
+static void __init __rmem_check_for_overlap(void);
+
 /**
  * of_scan_reserved_mem_reg_nodes() - Store info for the "reg" defined
  * reserved memory regions.
@@ -211,7 +221,7 @@ static int __init __fdt_reserved_mem_check_root(unsigned long node)
  * size are all stored in the reserved_mem array by calling the
  * of_reserved_mem_save_node() function.
  */
-static void __init of_scan_reserved_mem_reg_nodes(void)
+void __init of_scan_reserved_mem_reg_nodes(void)
 {
 	struct device_node *node, *child;
 	phys_addr_t base, size;
@@ -222,6 +232,13 @@ static void __init of_scan_reserved_mem_reg_nodes(void)
 		return;
 	}
 
+	/*
+	 * Before moving forward, allocate the exact size needed for the
+	 * reserved_mem array and copy all previously saved contents
+	 * into the new array if successful.
+	 */
+	alloc_reserved_mem_array();
+
 	for_each_child_of_node(node, child) {
 		int ret = 0;
 		const char *uname;
@@ -246,6 +263,8 @@ static void __init of_scan_reserved_mem_reg_nodes(void)
 		if (size)
 			of_reserved_mem_save_node(child, uname, base, size);
 	}
+	/* check for overlapping reserved regions */
+	__rmem_check_for_overlap();
 }
 
 static int __init __reserved_mem_alloc_size(unsigned long node, const char *uname);
@@ -526,44 +545,32 @@ static void __init __rmem_check_for_overlap(void)
 }
 
 /**
- * of_init_reserved_mem() - allocate and init all saved reserved memory regions
+ * of_init_reserved_mem_node() - Initialize a saved reserved memory region.
  */
-void __init of_init_reserved_mem(void)
+static void __init of_init_reserved_mem_node(struct reserved_mem *rmem)
 {
-	int i;
-
-	alloc_reserved_mem_array();
-
-	of_scan_reserved_mem_reg_nodes();
+	int err;
+	bool nomap;
+	struct device_node *node = rmem->dev_node;
 
-	/* check for overlapping reserved regions */
-	__rmem_check_for_overlap();
+	nomap = of_property_present(node, "no-map");
 
-	for (i = 0; i < reserved_mem_count; i++) {
-		struct reserved_mem *rmem = &reserved_mem[i];
-		struct device_node *node = rmem->dev_node;
-		int err = 0;
-		bool nomap;
-
-		nomap = of_property_present(node, "no-map");
-
-		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_property_present(node, "reusable");
-
-			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_property_present(node, "reusable");
+
+		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");
 	}
 }
 
-- 
2.34.1

Hi Oreoluwa,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on next-20240617]
[cannot apply to linus/master v6.10-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Oreoluwa-Babatunde/of-reserved_mem-Restructure-code-to-call-reserved-mem-init-functions-earlier/20240618-033815
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20240617193357.3929092-1-quic_obabatun%40quicinc.com
patch subject: [PATCH] of: reserved_mem: Restructure code to call reserved mem init functions earlier
config: i386-buildonly-randconfig-001-20240618 (https://download.01.org/0day-ci/archive/20240618/202406181626.126X1Nbz-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240618/202406181626.126X1Nbz-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406181626.126X1Nbz-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/of/of_reserved_mem.c:551: warning: Function parameter or struct member 'rmem' not described in 'of_init_reserved_mem_node'


vim +551 drivers/of/of_reserved_mem.c

ae1add247bf8c2 Mitchel Humpherys  2015-09-15  546  
3f0c8206644836 Marek Szyprowski   2014-02-28  547  /**
cb40a192099698 Oreoluwa Babatunde 2024-06-17  548   * of_init_reserved_mem_node() - Initialize a saved reserved memory region.
3f0c8206644836 Marek Szyprowski   2014-02-28  549   */
cb40a192099698 Oreoluwa Babatunde 2024-06-17  550  static void __init of_init_reserved_mem_node(struct reserved_mem *rmem)
3f0c8206644836 Marek Szyprowski   2014-02-28 @551  {
cb40a192099698 Oreoluwa Babatunde 2024-06-17  552  	int err;
6f1188b4ac7577 Yue Hu             2020-07-30  553  	bool nomap;
cb40a192099698 Oreoluwa Babatunde 2024-06-17  554  	struct device_node *node = rmem->dev_node;
3f0c8206644836 Marek Szyprowski   2014-02-28  555  
59d2c3cbcb5a47 Oreoluwa Babatunde 2024-05-28  556  	nomap = of_property_present(node, "no-map");
9dcfee01930e6c Marek Szyprowski   2014-07-14  557  
d0b8ed47e83a22 pierre Kuo         2019-02-19  558  	err = __reserved_mem_init_node(rmem);
d0b8ed47e83a22 pierre Kuo         2019-02-19  559  	if (err != 0 && err != -ENOENT) {
a46cccb0ee2d62 Oreoluwa Babatunde 2024-05-28  560  		pr_info("node %s compatible matching fail\n", rmem->name);
d0b8ed47e83a22 pierre Kuo         2019-02-19  561  		if (nomap)
7b25995f5319ad Dong Aisheng       2021-06-11  562  			memblock_clear_nomap(rmem->base, rmem->size);
3c6867a12a224d Dong Aisheng       2021-06-11  563  		else
a46cccb0ee2d62 Oreoluwa Babatunde 2024-05-28  564  			memblock_phys_free(rmem->base, rmem->size);
aeb9267eb6b1df Martin Liu         2023-02-10  565  	} else {
aeb9267eb6b1df Martin Liu         2023-02-10  566  		phys_addr_t end = rmem->base + rmem->size - 1;
59d2c3cbcb5a47 Oreoluwa Babatunde 2024-05-28  567  		bool reusable = of_property_present(node, "reusable");
aeb9267eb6b1df Martin Liu         2023-02-10  568  
6ee7afbabcee4d Geert Uytterhoeven 2023-02-16  569  		pr_info("%pa..%pa (%lu KiB) %s %s %s\n",
aeb9267eb6b1df Martin Liu         2023-02-10  570  			&rmem->base, &end, (unsigned long)(rmem->size / SZ_1K),
aeb9267eb6b1df Martin Liu         2023-02-10  571  			nomap ? "nomap" : "map",
aeb9267eb6b1df Martin Liu         2023-02-10  572  			reusable ? "reusable" : "non-reusable",
aeb9267eb6b1df Martin Liu         2023-02-10  573  			rmem->name ? rmem->name : "unknown");
d0b8ed47e83a22 pierre Kuo         2019-02-19  574  	}
d0b8ed47e83a22 pierre Kuo         2019-02-19  575  }
9dcfee01930e6c Marek Szyprowski   2014-07-14  576  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

After all the reserved memory regions have been added to the
reserved_mem array, a region specific initialization function is called
on each of reserved memory regions in a loop to initialize them.

With recent changes made to allow the reserved_mem array be dynamically
allocated, the cma reserved memory regions are not initialized until
after the page tables are setup. This causes the warning seen in the
dump stack below:

	WARNING: CPU: 0 PID: 1 at mm/memory.c:2789 __apply_to_page_range+0x360/0x380
	CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.10.0-rc1-00007-ga46cccb0ee2d #1
	Hardware name: Generic DT based system
	Call trace:
	unwind_backtrace from show_stack+0x18/0x1c
	show_stack from dump_stack_lvl+0x54/0x68
	dump_stack_lvl from __warn+0x74/0x114
	__warn from warn_slowpath_fmt+0x13c/0x1c0
	warn_slowpath_fmt from __apply_to_page_range+0x360/0x380
	__apply_to_page_range from apply_to_page_range+0x24/0x2c
	apply_to_page_range from __alloc_from_contiguous+0xc4/0x158
	__alloc_from_contiguous from cma_allocator_alloc+0x3c/0x44
	cma_allocator_alloc from arch_dma_alloc+0x128/0x2b4
	arch_dma_alloc from dma_alloc_attrs+0x90/0x150
	dma_alloc_attrs from drm_gem_dma_create+0xa4/0x13c
	drm_gem_dma_create from drm_gem_dma_create_with_handle+0x24/0xac
	drm_gem_dma_create_with_handle from drm_gem_dma_dumb_create+0x44/0x50
	drm_gem_dma_dumb_create from drm_client_framebuffer_create+0x9c/0x164
	drm_client_framebuffer_create from drm_fbdev_dma_helper_fb_probe+0x84/0x23c
	drm_fbdev_dma_helper_fb_probe from __drm_fb_helper_initial_config_and_unlock+0x2e4/0x4f8
	__drm_fb_helper_initial_config_and_unlock from drm_fbdev_dma_client_hotplug+0x74/0xb8
	drm_fbdev_dma_client_hotplug from drm_client_register+0x5c/0x98
	drm_client_register from aspeed_gfx_probe+0x278/0x3c0
	aspeed_gfx_probe from platform_probe+0x60/0xb8
	platform_probe from really_probe+0xd4/0x3b4
	really_probe from __driver_probe_device+0x90/0x1dc
	__driver_probe_device from driver_probe_device+0x38/0xd0
	driver_probe_device from __driver_attach+0x118/0x1dc
	__driver_attach from bus_for_each_dev+0x84/0xd4
	bus_for_each_dev from bus_add_driver+0xec/0x1f0
	bus_add_driver from driver_register+0x84/0x11c
	driver_register from do_one_initcall+0x84/0x1c8
	do_one_initcall from kernel_init_freeable+0x1a4/0x230
	kernel_init_freeable from kernel_init+0x1c/0x138
	kernel_init from ret_from_fork+0x14/0x28
	Exception stack(0x9f015fb0 to 0x9f015ff8)
	5fa0:                                     00000000 00000000 00000000 00000000
	5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
	5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
	---[ end trace 0000000000000000 ]---
	aspeed_gfx 1e6e6000.display: [drm] fb0: aspeed-gfx-drmd frame buffer device

Hence, restructure the code to initialize the regions as soon as each
of them are added to the reserved_mem array.

Fixes: a46cccb0ee2d ("of: reserved_mem: Restruture how the reserved memory regions are processed")
Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
---
v2:
- Fix kernel-doc for of_init_reserved_mem_node() in response to the
  below warning from v1:
  https://lore.kernel.org/all/202406181626.126X1Nbz-lkp@intel.com/

v1:
  https://lore.kernel.org/all/20240617193357.3929092-1-quic_obabatun@quicinc.com/

 drivers/of/fdt.c             |  2 +-
 drivers/of/of_private.h      |  2 +-
 drivers/of/of_reserved_mem.c | 83 +++++++++++++++++++++---------------
 3 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 9cde2abd2fc0..ea2dff0478c7 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1239,7 +1239,7 @@ void __init unflatten_device_tree(void)
 	unittest_unflatten_overlay_base();
 
 	/* initialize the reserved memory regions */
-	of_init_reserved_mem();
+	of_scan_reserved_mem_reg_nodes();
 }
 
 /**
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 01b33c4b1e9f..7412aed903df 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -181,7 +181,7 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node *
 #endif
 
 int fdt_scan_reserved_mem(void);
-void of_init_reserved_mem(void);
+void of_scan_reserved_mem_reg_nodes(void);
 
 bool of_fdt_device_is_available(const void *blob, unsigned long node);
 
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index eb54490a0a11..b31001728866 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -97,6 +97,8 @@ static void __init alloc_reserved_mem_array(void)
 	reserved_mem = new_array;
 }
 
+static void __init of_init_reserved_mem_node(struct reserved_mem *rmem);
+
 /*
  * of_reserved_mem_save_node() - save fdt node for second pass initialization
  */
@@ -115,6 +117,12 @@ static void __init of_reserved_mem_save_node(struct device_node *node, const cha
 	rmem->base = base;
 	rmem->size = size;
 
+	/*
+	 * Run the region specific initialization function for the rmem
+	 * node.
+	 */
+	of_init_reserved_mem_node(rmem);
+
 	reserved_mem_count++;
 	return;
 }
@@ -201,6 +209,8 @@ static int __init __fdt_reserved_mem_check_root(unsigned long node)
 	return 0;
 }
 
+static void __init __rmem_check_for_overlap(void);
+
 /**
  * of_scan_reserved_mem_reg_nodes() - Store info for the "reg" defined
  * reserved memory regions.
@@ -211,7 +221,7 @@ static int __init __fdt_reserved_mem_check_root(unsigned long node)
  * size are all stored in the reserved_mem array by calling the
  * of_reserved_mem_save_node() function.
  */
-static void __init of_scan_reserved_mem_reg_nodes(void)
+void __init of_scan_reserved_mem_reg_nodes(void)
 {
 	struct device_node *node, *child;
 	phys_addr_t base, size;
@@ -222,6 +232,13 @@ static void __init of_scan_reserved_mem_reg_nodes(void)
 		return;
 	}
 
+	/*
+	 * Before moving forward, allocate the exact size needed for the
+	 * reserved_mem array and copy all previously saved contents
+	 * into the new array if successful.
+	 */
+	alloc_reserved_mem_array();
+
 	for_each_child_of_node(node, child) {
 		int ret = 0;
 		const char *uname;
@@ -246,6 +263,8 @@ static void __init of_scan_reserved_mem_reg_nodes(void)
 		if (size)
 			of_reserved_mem_save_node(child, uname, base, size);
 	}
+	/* check for overlapping reserved regions */
+	__rmem_check_for_overlap();
 }
 
 static int __init __reserved_mem_alloc_size(unsigned long node, const char *uname);
@@ -526,44 +545,38 @@ static void __init __rmem_check_for_overlap(void)
 }
 
 /**
- * of_init_reserved_mem() - allocate and init all saved reserved memory regions
+ * of_init_reserved_mem_node() - Initialize a saved reserved memory region.
+ * @rmem: reserved_mem object of the memory region to be initialized.
+ *
+ * This function is used to call the region specific initialization
+ * function on the rmem object passed as an argument. The rmem object
+ * will contain the base address, size, node name, and device_node of
+ * the reserved memory region to be initialized.
  */
-void __init of_init_reserved_mem(void)
+static void __init of_init_reserved_mem_node(struct reserved_mem *rmem)
 {
-	int i;
-
-	alloc_reserved_mem_array();
-
-	of_scan_reserved_mem_reg_nodes();
+	int err;
+	bool nomap;
+	struct device_node *node = rmem->dev_node;
 
-	/* check for overlapping reserved regions */
-	__rmem_check_for_overlap();
+	nomap = of_property_present(node, "no-map");
 
-	for (i = 0; i < reserved_mem_count; i++) {
-		struct reserved_mem *rmem = &reserved_mem[i];
-		struct device_node *node = rmem->dev_node;
-		int err = 0;
-		bool nomap;
-
-		nomap = of_property_present(node, "no-map");
-
-		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_property_present(node, "reusable");
-
-			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_property_present(node, "reusable");
+
+		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");
 	}
 }
 
-- 
2.34.1

On Wed, Jun 19, 2024 at 05:10:27PM -0700, Oreoluwa Babatunde wrote:
> After all the reserved memory regions have been added to the
> reserved_mem array, a region specific initialization function is called
> on each of reserved memory regions in a loop to initialize them.
> 
> With recent changes made to allow the reserved_mem array be dynamically
> allocated, the cma reserved memory regions are not initialized until
> after the page tables are setup. This causes the warning seen in the
> dump stack below:
> 
> 	WARNING: CPU: 0 PID: 1 at mm/memory.c:2789 __apply_to_page_range+0x360/0x380
> 	CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.10.0-rc1-00007-ga46cccb0ee2d #1
> 	Hardware name: Generic DT based system
> 	Call trace:
> 	unwind_backtrace from show_stack+0x18/0x1c
> 	show_stack from dump_stack_lvl+0x54/0x68
> 	dump_stack_lvl from __warn+0x74/0x114
> 	__warn from warn_slowpath_fmt+0x13c/0x1c0
> 	warn_slowpath_fmt from __apply_to_page_range+0x360/0x380
> 	__apply_to_page_range from apply_to_page_range+0x24/0x2c
> 	apply_to_page_range from __alloc_from_contiguous+0xc4/0x158
> 	__alloc_from_contiguous from cma_allocator_alloc+0x3c/0x44
> 	cma_allocator_alloc from arch_dma_alloc+0x128/0x2b4
> 	arch_dma_alloc from dma_alloc_attrs+0x90/0x150
> 	dma_alloc_attrs from drm_gem_dma_create+0xa4/0x13c
> 	drm_gem_dma_create from drm_gem_dma_create_with_handle+0x24/0xac
> 	drm_gem_dma_create_with_handle from drm_gem_dma_dumb_create+0x44/0x50
> 	drm_gem_dma_dumb_create from drm_client_framebuffer_create+0x9c/0x164
> 	drm_client_framebuffer_create from drm_fbdev_dma_helper_fb_probe+0x84/0x23c
> 	drm_fbdev_dma_helper_fb_probe from __drm_fb_helper_initial_config_and_unlock+0x2e4/0x4f8
> 	__drm_fb_helper_initial_config_and_unlock from drm_fbdev_dma_client_hotplug+0x74/0xb8
> 	drm_fbdev_dma_client_hotplug from drm_client_register+0x5c/0x98
> 	drm_client_register from aspeed_gfx_probe+0x278/0x3c0
> 	aspeed_gfx_probe from platform_probe+0x60/0xb8
> 	platform_probe from really_probe+0xd4/0x3b4
> 	really_probe from __driver_probe_device+0x90/0x1dc
> 	__driver_probe_device from driver_probe_device+0x38/0xd0
> 	driver_probe_device from __driver_attach+0x118/0x1dc
> 	__driver_attach from bus_for_each_dev+0x84/0xd4
> 	bus_for_each_dev from bus_add_driver+0xec/0x1f0
> 	bus_add_driver from driver_register+0x84/0x11c
> 	driver_register from do_one_initcall+0x84/0x1c8
> 	do_one_initcall from kernel_init_freeable+0x1a4/0x230
> 	kernel_init_freeable from kernel_init+0x1c/0x138
> 	kernel_init from ret_from_fork+0x14/0x28
> 	Exception stack(0x9f015fb0 to 0x9f015ff8)
> 	5fa0:                                     00000000 00000000 00000000 00000000
> 	5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 	5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 	---[ end trace 0000000000000000 ]---
> 	aspeed_gfx 1e6e6000.display: [drm] fb0: aspeed-gfx-drmd frame buffer device
> 
> Hence, restructure the code to initialize the regions as soon as each
> of them are added to the reserved_mem array.
> 
> Fixes: a46cccb0ee2d ("of: reserved_mem: Restruture how the reserved memory regions are processed")
> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
> ---
> v2:
> - Fix kernel-doc for of_init_reserved_mem_node() in response to the
>   below warning from v1:
>   https://lore.kernel.org/all/202406181626.126X1Nbz-lkp@intel.com/
> 
> v1:
>   https://lore.kernel.org/all/20240617193357.3929092-1-quic_obabatun@quicinc.com/
> 
>  drivers/of/fdt.c             |  2 +-
>  drivers/of/of_private.h      |  2 +-
>  drivers/of/of_reserved_mem.c | 83 +++++++++++++++++++++---------------
>  3 files changed, 50 insertions(+), 37 deletions(-)

Applied.

In the future, do not send fixes or new versions as replies to previous 
threads. The default behavior of b4 is to apply v6 of the series because 
that is the 'newest' version.

Rob

On 6/26/2024 8:16 AM, Rob Herring wrote:
> On Wed, Jun 19, 2024 at 05:10:27PM -0700, Oreoluwa Babatunde wrote:
>> After all the reserved memory regions have been added to the
>> reserved_mem array, a region specific initialization function is called
>> on each of reserved memory regions in a loop to initialize them.
>>
>> With recent changes made to allow the reserved_mem array be dynamically
>> allocated, the cma reserved memory regions are not initialized until
>> after the page tables are setup. This causes the warning seen in the
>> dump stack below:
>>
>> 	WARNING: CPU: 0 PID: 1 at mm/memory.c:2789 __apply_to_page_range+0x360/0x380
>> 	CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.10.0-rc1-00007-ga46cccb0ee2d #1
>> 	Hardware name: Generic DT based system
>> 	Call trace:
>> 	unwind_backtrace from show_stack+0x18/0x1c
>> 	show_stack from dump_stack_lvl+0x54/0x68
>> 	dump_stack_lvl from __warn+0x74/0x114
>> 	__warn from warn_slowpath_fmt+0x13c/0x1c0
>> 	warn_slowpath_fmt from __apply_to_page_range+0x360/0x380
>> 	__apply_to_page_range from apply_to_page_range+0x24/0x2c
>> 	apply_to_page_range from __alloc_from_contiguous+0xc4/0x158
>> 	__alloc_from_contiguous from cma_allocator_alloc+0x3c/0x44
>> 	cma_allocator_alloc from arch_dma_alloc+0x128/0x2b4
>> 	arch_dma_alloc from dma_alloc_attrs+0x90/0x150
>> 	dma_alloc_attrs from drm_gem_dma_create+0xa4/0x13c
>> 	drm_gem_dma_create from drm_gem_dma_create_with_handle+0x24/0xac
>> 	drm_gem_dma_create_with_handle from drm_gem_dma_dumb_create+0x44/0x50
>> 	drm_gem_dma_dumb_create from drm_client_framebuffer_create+0x9c/0x164
>> 	drm_client_framebuffer_create from drm_fbdev_dma_helper_fb_probe+0x84/0x23c
>> 	drm_fbdev_dma_helper_fb_probe from __drm_fb_helper_initial_config_and_unlock+0x2e4/0x4f8
>> 	__drm_fb_helper_initial_config_and_unlock from drm_fbdev_dma_client_hotplug+0x74/0xb8
>> 	drm_fbdev_dma_client_hotplug from drm_client_register+0x5c/0x98
>> 	drm_client_register from aspeed_gfx_probe+0x278/0x3c0
>> 	aspeed_gfx_probe from platform_probe+0x60/0xb8
>> 	platform_probe from really_probe+0xd4/0x3b4
>> 	really_probe from __driver_probe_device+0x90/0x1dc
>> 	__driver_probe_device from driver_probe_device+0x38/0xd0
>> 	driver_probe_device from __driver_attach+0x118/0x1dc
>> 	__driver_attach from bus_for_each_dev+0x84/0xd4
>> 	bus_for_each_dev from bus_add_driver+0xec/0x1f0
>> 	bus_add_driver from driver_register+0x84/0x11c
>> 	driver_register from do_one_initcall+0x84/0x1c8
>> 	do_one_initcall from kernel_init_freeable+0x1a4/0x230
>> 	kernel_init_freeable from kernel_init+0x1c/0x138
>> 	kernel_init from ret_from_fork+0x14/0x28
>> 	Exception stack(0x9f015fb0 to 0x9f015ff8)
>> 	5fa0:                                     00000000 00000000 00000000 00000000
>> 	5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 	5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>> 	---[ end trace 0000000000000000 ]---
>> 	aspeed_gfx 1e6e6000.display: [drm] fb0: aspeed-gfx-drmd frame buffer device
>>
>> Hence, restructure the code to initialize the regions as soon as each
>> of them are added to the reserved_mem array.
>>
>> Fixes: a46cccb0ee2d ("of: reserved_mem: Restruture how the reserved memory regions are processed")
>> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
>> ---
>> v2:
>> - Fix kernel-doc for of_init_reserved_mem_node() in response to the
>>   below warning from v1:
>>   https://lore.kernel.org/all/202406181626.126X1Nbz-lkp@intel.com/
>>
>> v1:
>>   https://lore.kernel.org/all/20240617193357.3929092-1-quic_obabatun@quicinc.com/
>>
>>  drivers/of/fdt.c             |  2 +-
>>  drivers/of/of_private.h      |  2 +-
>>  drivers/of/of_reserved_mem.c | 83 +++++++++++++++++++++---------------
>>  3 files changed, 50 insertions(+), 37 deletions(-)
> Applied.
>
> In the future, do not send fixes or new versions as replies to previous 
> threads. The default behavior of b4 is to apply v6 of the series because 
> that is the 'newest' version.
>
> Rob
ack.

Thank you!

On Wed, Jun 19, 2024 at 05:10:27PM -0700, Oreoluwa Babatunde wrote:
> After all the reserved memory regions have been added to the
> reserved_mem array, a region specific initialization function is called
> on each of reserved memory regions in a loop to initialize them.
> 
> With recent changes made to allow the reserved_mem array be dynamically
> allocated, the cma reserved memory regions are not initialized until
> after the page tables are setup. This causes the warning seen in the
> dump stack below:
> 
> 	WARNING: CPU: 0 PID: 1 at mm/memory.c:2789 __apply_to_page_range+0x360/0x380
> 	CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.10.0-rc1-00007-ga46cccb0ee2d #1
> 	Hardware name: Generic DT based system
> 	Call trace:
> 	unwind_backtrace from show_stack+0x18/0x1c
> 	show_stack from dump_stack_lvl+0x54/0x68
> 	dump_stack_lvl from __warn+0x74/0x114
> 	__warn from warn_slowpath_fmt+0x13c/0x1c0
> 	warn_slowpath_fmt from __apply_to_page_range+0x360/0x380
> 	__apply_to_page_range from apply_to_page_range+0x24/0x2c
> 	apply_to_page_range from __alloc_from_contiguous+0xc4/0x158
> 	__alloc_from_contiguous from cma_allocator_alloc+0x3c/0x44
> 	cma_allocator_alloc from arch_dma_alloc+0x128/0x2b4
> 	arch_dma_alloc from dma_alloc_attrs+0x90/0x150
> 	dma_alloc_attrs from drm_gem_dma_create+0xa4/0x13c
> 	drm_gem_dma_create from drm_gem_dma_create_with_handle+0x24/0xac
> 	drm_gem_dma_create_with_handle from drm_gem_dma_dumb_create+0x44/0x50
> 	drm_gem_dma_dumb_create from drm_client_framebuffer_create+0x9c/0x164
> 	drm_client_framebuffer_create from drm_fbdev_dma_helper_fb_probe+0x84/0x23c
> 	drm_fbdev_dma_helper_fb_probe from __drm_fb_helper_initial_config_and_unlock+0x2e4/0x4f8
> 	__drm_fb_helper_initial_config_and_unlock from drm_fbdev_dma_client_hotplug+0x74/0xb8
> 	drm_fbdev_dma_client_hotplug from drm_client_register+0x5c/0x98
> 	drm_client_register from aspeed_gfx_probe+0x278/0x3c0
> 	aspeed_gfx_probe from platform_probe+0x60/0xb8
> 	platform_probe from really_probe+0xd4/0x3b4
> 	really_probe from __driver_probe_device+0x90/0x1dc
> 	__driver_probe_device from driver_probe_device+0x38/0xd0
> 	driver_probe_device from __driver_attach+0x118/0x1dc
> 	__driver_attach from bus_for_each_dev+0x84/0xd4
> 	bus_for_each_dev from bus_add_driver+0xec/0x1f0
> 	bus_add_driver from driver_register+0x84/0x11c
> 	driver_register from do_one_initcall+0x84/0x1c8
> 	do_one_initcall from kernel_init_freeable+0x1a4/0x230
> 	kernel_init_freeable from kernel_init+0x1c/0x138
> 	kernel_init from ret_from_fork+0x14/0x28
> 	Exception stack(0x9f015fb0 to 0x9f015ff8)
> 	5fa0:                                     00000000 00000000 00000000 00000000
> 	5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 	5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 	---[ end trace 0000000000000000 ]---
> 	aspeed_gfx 1e6e6000.display: [drm] fb0: aspeed-gfx-drmd frame buffer device
> 
> Hence, restructure the code to initialize the regions as soon as each
> of them are added to the reserved_mem array.
> 
> Fixes: a46cccb0ee2d ("of: reserved_mem: Restruture how the reserved memory regions are processed")
> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>

This resolves the warning that I see and I see no other warnings
introduced in any of my virtual boot tests.

Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
> v2:
> - Fix kernel-doc for of_init_reserved_mem_node() in response to the
>   below warning from v1:
>   https://lore.kernel.org/all/202406181626.126X1Nbz-lkp@intel.com/
> 
> v1:
>   https://lore.kernel.org/all/20240617193357.3929092-1-quic_obabatun@quicinc.com/
> 
>  drivers/of/fdt.c             |  2 +-
>  drivers/of/of_private.h      |  2 +-
>  drivers/of/of_reserved_mem.c | 83 +++++++++++++++++++++---------------
>  3 files changed, 50 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 9cde2abd2fc0..ea2dff0478c7 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1239,7 +1239,7 @@ void __init unflatten_device_tree(void)
>  	unittest_unflatten_overlay_base();
>  
>  	/* initialize the reserved memory regions */
> -	of_init_reserved_mem();
> +	of_scan_reserved_mem_reg_nodes();
>  }
>  
>  /**
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 01b33c4b1e9f..7412aed903df 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -181,7 +181,7 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node *
>  #endif
>  
>  int fdt_scan_reserved_mem(void);
> -void of_init_reserved_mem(void);
> +void of_scan_reserved_mem_reg_nodes(void);
>  
>  bool of_fdt_device_is_available(const void *blob, unsigned long node);
>  
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index eb54490a0a11..b31001728866 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -97,6 +97,8 @@ static void __init alloc_reserved_mem_array(void)
>  	reserved_mem = new_array;
>  }
>  
> +static void __init of_init_reserved_mem_node(struct reserved_mem *rmem);
> +
>  /*
>   * of_reserved_mem_save_node() - save fdt node for second pass initialization
>   */
> @@ -115,6 +117,12 @@ static void __init of_reserved_mem_save_node(struct device_node *node, const cha
>  	rmem->base = base;
>  	rmem->size = size;
>  
> +	/*
> +	 * Run the region specific initialization function for the rmem
> +	 * node.
> +	 */
> +	of_init_reserved_mem_node(rmem);
> +
>  	reserved_mem_count++;
>  	return;
>  }
> @@ -201,6 +209,8 @@ static int __init __fdt_reserved_mem_check_root(unsigned long node)
>  	return 0;
>  }
>  
> +static void __init __rmem_check_for_overlap(void);
> +
>  /**
>   * of_scan_reserved_mem_reg_nodes() - Store info for the "reg" defined
>   * reserved memory regions.
> @@ -211,7 +221,7 @@ static int __init __fdt_reserved_mem_check_root(unsigned long node)
>   * size are all stored in the reserved_mem array by calling the
>   * of_reserved_mem_save_node() function.
>   */
> -static void __init of_scan_reserved_mem_reg_nodes(void)
> +void __init of_scan_reserved_mem_reg_nodes(void)
>  {
>  	struct device_node *node, *child;
>  	phys_addr_t base, size;
> @@ -222,6 +232,13 @@ static void __init of_scan_reserved_mem_reg_nodes(void)
>  		return;
>  	}
>  
> +	/*
> +	 * Before moving forward, allocate the exact size needed for the
> +	 * reserved_mem array and copy all previously saved contents
> +	 * into the new array if successful.
> +	 */
> +	alloc_reserved_mem_array();
> +
>  	for_each_child_of_node(node, child) {
>  		int ret = 0;
>  		const char *uname;
> @@ -246,6 +263,8 @@ static void __init of_scan_reserved_mem_reg_nodes(void)
>  		if (size)
>  			of_reserved_mem_save_node(child, uname, base, size);
>  	}
> +	/* check for overlapping reserved regions */
> +	__rmem_check_for_overlap();
>  }
>  
>  static int __init __reserved_mem_alloc_size(unsigned long node, const char *uname);
> @@ -526,44 +545,38 @@ static void __init __rmem_check_for_overlap(void)
>  }
>  
>  /**
> - * of_init_reserved_mem() - allocate and init all saved reserved memory regions
> + * of_init_reserved_mem_node() - Initialize a saved reserved memory region.
> + * @rmem: reserved_mem object of the memory region to be initialized.
> + *
> + * This function is used to call the region specific initialization
> + * function on the rmem object passed as an argument. The rmem object
> + * will contain the base address, size, node name, and device_node of
> + * the reserved memory region to be initialized.
>   */
> -void __init of_init_reserved_mem(void)
> +static void __init of_init_reserved_mem_node(struct reserved_mem *rmem)
>  {
> -	int i;
> -
> -	alloc_reserved_mem_array();
> -
> -	of_scan_reserved_mem_reg_nodes();
> +	int err;
> +	bool nomap;
> +	struct device_node *node = rmem->dev_node;
>  
> -	/* check for overlapping reserved regions */
> -	__rmem_check_for_overlap();
> +	nomap = of_property_present(node, "no-map");
>  
> -	for (i = 0; i < reserved_mem_count; i++) {
> -		struct reserved_mem *rmem = &reserved_mem[i];
> -		struct device_node *node = rmem->dev_node;
> -		int err = 0;
> -		bool nomap;
> -
> -		nomap = of_property_present(node, "no-map");
> -
> -		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_property_present(node, "reusable");
> -
> -			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_property_present(node, "reusable");
> +
> +		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");
>  	}
>  }
>  
> -- 
> 2.34.1
>