Skip to contentSkip to conversation

Fix Random Kernel panic from when fail to reserve memory

PATCH Superseded Lucas Tanure 3 participants 5 messages Apr 10, 2023

Subsystems (3)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover Fix Random Kernel panic from when fail to reserve memory

I am trying to fix an issue where the kernel panics randomly on my Vim3 board. The problem happens when the ARM Trusted Firmware memory is not removed from the available ram.

This happens because my u-boot provides /memreserve/, but it doesn't flag it as nomap. And the kernel scan of /reserved-memory node can't map as nomap as the region is already reserved.

The idea now is to scan /memreserve/ after /reserved-memory node, and /memreserve/ will skip regions already reserved by /reserved-memory node.

V1 Patch series: https://lore.kernel.org/all/[email protected]/

Previous Threads:
#regzbot link: https://lore.kernel.org/linux-arm-kernel/[email protected]/#regzbot
#regzbot link: https://lore.kernel.org/all/CAJX_Q+1Tjc+-TjZ6JW9X0NxEdFe=82a9626yL63j7uVD4LpxEA@mail.gmail.com/

Change from V1:
- Instead of allowing to mark nomap a region already reserved, give
precedence to /reserved-memory node scan.

Lucas Tanure (1):
of: fdt: Scan /memreserve/ last

 drivers/of/fdt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Signature · 1 line
2.40.0
#1 lore raw

PATCH v2 1/1 of: fdt: Scan /memreserve/ last

Change the order of scanning /memreserve/ and /reserved-memory node. /reserved-memory node should go first, as it has a more updated description of the memory regions and it can apply flags, like nomap. Also, /memreserve/ should avoid reserving regions described in /reserved-memory node.

---
 drivers/of/fdt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/of/fdt.c +7 -2
index d1a68b6d03b3..26e608d8025d 100644
@@ -635,16 +635,21 @@ void __init early_init_fdt_scan_reserved_mem(void)
 	if (!initial_boot_params)
 		return;
 
+	fdt_scan_reserved_mem();
+	fdt_reserve_elfcorehdr();
+
 	/* Process header /memreserve/ fields */
 	for (n = 0; ; n++) {
 		fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
 		if (!size)
 			break;
+		if (memblock_overlaps_region(&memblock.memory, base, size) &&
+		    memblock_is_region_reserved(base, size))
+			break;
+
 		memblock_reserve(base, size);
 	}
 
-	fdt_scan_reserved_mem();
-	fdt_reserve_elfcorehdr();
 	fdt_init_reserved_mem();
 }
 
Signature · 1 line
2.40.0
#2 lore raw

Re: [PATCH v2 1/1] of: fdt: Scan /memreserve/ last

Quoted 6 lines · Lucas Tanure

Change the order of scanning /memreserve/ and /reserved-memory node. /reserved-memory node should go first, as it has a more updated description of the memory regions and it can apply flags, like nomap. Also, /memreserve/ should avoid reserving regions described in /reserved-memory node.

Like I said on v1, I think doing this has a high chance of causing regressions on other platforms. It should probably not go to stable for some time after a kernel release.

Quoted 13 lines
---
 drivers/of/fdt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/of/fdt.c +0 -0
index d1a68b6d03b3..26e608d8025d 100644
@@ -635,16 +635,21 @@ void __init early_init_fdt_scan_reserved_mem(void)
        if (!initial_boot_params)
                return;
 

drivers/of/fdt.c void __init early_init_fdt_scan_reserved_mem(void) · line 647

Quoted 10 lines
drivers/of/fdt.c +5 -0
+       fdt_scan_reserved_mem();
+       fdt_reserve_elfcorehdr();
+
        /* Process header /memreserve/ fields */
        for (n = 0; ; n++) {
                fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
                if (!size)
                        break;
+               if (memblock_overlaps_region(&memblock.memory, base, size) &&
+                   memblock_is_region_reserved(base, size))

Just to make sure, a partial overlap will still get reserved?

drivers/of/fdt.c void __init early_init_fdt_scan_reserved_mem(void) · line 648

Quoted 1 line
drivers/of/fdt.c +1 -0
+                       break;

Shouldn't we continue to the next entry rather than stopping.

Quoted 12 lines
drivers/of/fdt.c +1 -3
+
                memblock_reserve(base, size);
        }
 
-       fdt_scan_reserved_mem();
-       fdt_reserve_elfcorehdr();
        fdt_init_reserved_mem();
 }
 
--

2.40.0

#3 lore raw

Re: [PATCH v2 1/1] of: fdt: Scan /memreserve/ last

Quoted 26 lines · Rob Herring
Quoted 6 lines · Lucas Tanure

Change the order of scanning /memreserve/ and /reserved-memory node. /reserved-memory node should go first, as it has a more updated description of the memory regions and it can apply flags, like nomap. Also, /memreserve/ should avoid reserving regions described in /reserved-memory node.

Like I said on v1, I think doing this has a high chance of causing regressions on other platforms. It should probably not go to stable for some time after a kernel release.

Quoted 13 lines
---
 drivers/of/fdt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/of/fdt.c +0 -0
index d1a68b6d03b3..26e608d8025d 100644
@@ -635,16 +635,21 @@ void __init early_init_fdt_scan_reserved_mem(void)
        if (!initial_boot_params)
                return;
 
Quoted 12 lines

drivers/of/fdt.c void __init early_init_fdt_scan_reserved_mem(void) · line 647

Quoted 10 lines
drivers/of/fdt.c +5 -0
+       fdt_scan_reserved_mem();
+       fdt_reserve_elfcorehdr();
+
        /* Process header /memreserve/ fields */
        for (n = 0; ; n++) {
                fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
                if (!size)
                        break;
+               if (memblock_overlaps_region(&memblock.memory, base, size) &&
+                   memblock_is_region_reserved(base, size))

Just to make sure, a partial overlap will still get reserved?

A partial overlap will get reserved if not already reserved by the /reserved-memory node.

Quoted 4 lines

drivers/of/fdt.c void __init early_init_fdt_scan_reserved_mem(void) · line 648

Quoted 2 lines
drivers/of/fdt.c +1 -0
+                       break;

Shouldn't we continue to the next entry rather than stopping.

Yes, my mistake; I will send v3.

Quoted 13 lines
Quoted 13 lines
drivers/of/fdt.c +1 -3
+
                memblock_reserve(base, size);
        }
 
-       fdt_scan_reserved_mem();
-       fdt_reserve_elfcorehdr();
        fdt_init_reserved_mem();
 }
 
--

2.40.0

#4 lore raw

Re: [PATCH v2 1/1] of: fdt: Scan /memreserve/ last

In reply to Lucas Tanure
Quoted 20 lines · Lucas Tanure

Change the order of scanning /memreserve/ and /reserved-memory node. /reserved-memory node should go first, as it has a more updated description of the memory regions and it can apply flags, like nomap. Also, /memreserve/ should avoid reserving regions described in /reserved-memory node.

---
 drivers/of/fdt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/of/fdt.c +1 -0
index d1a68b6d03b3..26e608d8025d 100644
@@ -635,16 +635,21 @@ void __init early_init_fdt_scan_reserved_mem(void)
 	if (!initial_boot_params)
 		return;
 
+	fdt_scan_reserved_mem();

drivers/of/fdt.c void __init early_init_fdt_scan_reserved_mem(void) · line 648

Quoted 10 lines
drivers/of/fdt.c +5 -0
+	fdt_reserve_elfcorehdr();
+
 	/* Process header /memreserve/ fields */
 	for (n = 0; ; n++) {
 		fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
 		if (!size)
 			break;
+		if (memblock_overlaps_region(&memblock.memory, base, size) &&
+		    memblock_is_region_reserved(base, size))
+			break;

I don't think this is really needed, it's ok to reserve the same ranges multiple times.
Both checks are not cheap, so it'll be better to just reserve everything both from performance and simplicity points of view.

Quoted 12 lines
drivers/of/fdt.c +1 -2
+
 		memblock_reserve(base, size);
 	}
 
-	fdt_scan_reserved_mem();
-	fdt_reserve_elfcorehdr();
 	fdt_init_reserved_mem();
 }
 
Signature · 1 line
2.40.0
Signature · 2 lines
Sincerely yours,
Mike.