Patrick Williams | 92b42cb | 2022-09-03 06:53:57 -0500 | [diff] [blame^] | 1 | From 7bfeda1c9224270af97adf799ce0b5a4292bceb6 Mon Sep 17 00:00:00 2001 |
| 2 | From: Andre Przywara <andre.przywara@arm.com> |
| 3 | Date: Tue, 17 May 2022 11:14:10 +0100 |
| 4 | Subject: [PATCH] of/fdt: Ignore disabled memory nodes |
| 5 | |
| 6 | When we boot a machine using a devicetree, the generic DT code goes |
| 7 | through all nodes with a 'device_type = "memory"' property, and collects |
| 8 | all memory banks mentioned there. However it does not check for the |
| 9 | status property, so any nodes which are explicitly "disabled" will still |
| 10 | be added as a memblock. |
| 11 | This ends up badly for QEMU, when booting with secure firmware on |
| 12 | arm/arm64 machines, because QEMU adds a node describing secure-only |
| 13 | memory: |
| 14 | =================== |
| 15 | secram@e000000 { |
| 16 | secure-status = "okay"; |
| 17 | status = "disabled"; |
| 18 | reg = <0x00 0xe000000 0x00 0x1000000>; |
| 19 | device_type = "memory"; |
| 20 | }; |
| 21 | =================== |
| 22 | |
| 23 | The kernel will eventually use that memory block (which is located below |
| 24 | the main DRAM bank), but accesses to that will be answered with an |
| 25 | SError: |
| 26 | =================== |
| 27 | [ 0.000000] Internal error: synchronous external abort: 96000050 [#1] PREEMPT SMP |
| 28 | [ 0.000000] Modules linked in: |
| 29 | [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.18.0-rc6-00014-g10c8acb8b679 #524 |
| 30 | [ 0.000000] Hardware name: linux,dummy-virt (DT) |
| 31 | [ 0.000000] pstate: 200000c5 (nzCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) |
| 32 | [ 0.000000] pc : new_slab+0x190/0x340 |
| 33 | [ 0.000000] lr : new_slab+0x184/0x340 |
| 34 | [ 0.000000] sp : ffff80000a4b3d10 |
| 35 | .... |
| 36 | ================== |
| 37 | The actual crash location and call stack will be somewhat random, and |
| 38 | depend on the specific allocation of that physical memory range. |
| 39 | |
| 40 | As the DT spec[1] explicitly mentions standard properties, add a simple |
| 41 | check to skip over disabled memory nodes, so that we only use memory |
| 42 | that is meant for non-secure code to use. |
| 43 | |
| 44 | That fixes booting a QEMU arm64 VM with EL3 enabled ("secure=on"), when |
| 45 | not using UEFI. In this case the QEMU generated DT will be handed on |
| 46 | to the kernel, which will see the secram node. |
| 47 | This issue is reproducible when using TF-A together with U-Boot as |
| 48 | firmware, then booting with the "booti" command. |
| 49 | |
| 50 | When using U-Boot as an UEFI provider, the code there [2] explicitly |
| 51 | filters for disabled nodes when generating the UEFI memory map, so we |
| 52 | are safe. |
| 53 | EDK/2 only reads the first bank of the first DT memory node [3] to learn |
| 54 | about memory, so we got lucky there. |
| 55 | |
| 56 | [1] https://github.com/devicetree-org/devicetree-specification/blob/main/source/chapter3-devicenodes.rst#memory-node (after the table) |
| 57 | [2] https://source.denx.de/u-boot/u-boot/-/blob/master/lib/fdtdec.c#L1061-1063 |
| 58 | [3] https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/PrePi/FdtParser.c |
| 59 | |
| 60 | Reported-by: Ross Burton <ross.burton@arm.com> |
| 61 | Signed-off-by: Andre Przywara <andre.przywara@arm.com> |
| 62 | |
| 63 | Upstream-Status: Submitted [https://lore.kernel.org/linux-arm-kernel/20220517101410.3493781-1-andre.przywara@arm.com/T/#u] |
| 64 | Signed-off-by: Ross Burton <ross.burton@arm.com> |
| 65 | |
| 66 | --- |
| 67 | drivers/of/fdt.c | 3 +++ |
| 68 | 1 file changed, 3 insertions(+) |
| 69 | |
| 70 | diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c |
| 71 | index 59a7a9ee58ef..5439c899fe04 100644 |
| 72 | --- a/drivers/of/fdt.c |
| 73 | +++ b/drivers/of/fdt.c |
| 74 | @@ -1102,6 +1102,9 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname, |
| 75 | if (type == NULL || strcmp(type, "memory") != 0) |
| 76 | return 0; |
| 77 | |
| 78 | + if (!of_fdt_device_is_available(initial_boot_params, node)) |
| 79 | + return 0; |
| 80 | + |
| 81 | reg = of_get_flat_dt_prop(node, "linux,usable-memory", &l); |
| 82 | if (reg == NULL) |
| 83 | reg = of_get_flat_dt_prop(node, "reg", &l); |
| 84 | -- |
| 85 | 2.25.1 |