of: populate of_root node if bootloader doesn't

Call this function unconditionally so that we can populate an empty DTB on platforms that don't boot with a firmware provided or builtin DTB. When ACPI is in use, unflatten_device_tree() ignores the 'initial_boot_params' pointer so the live DT on those systems won't be whatever that's pointing to. Similarly, when kexec copies the DT data the previous kernel to the new one on ACPI systems, of_kexec_alloc_and_setup_fdt() will ignore the live DT (the empty root one) and copy the 'initial_boot_params' data.

Cc Rob Herring <[email protected]>

Cc Frank Rowand <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Mark Rutland <[email protected]>

Cc <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 arch/arm64/kernel/setup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Quoted 16 lines · Stephen Boyd

Call this function unconditionally so that we can populate an empty DTB on platforms that don't boot with a firmware provided or builtin DTB. When ACPI is in use, unflatten_device_tree() ignores the 'initial_boot_params' pointer so the live DT on those systems won't be whatever that's pointing to. Similarly, when kexec copies the DT data the previous kernel to the new one on ACPI systems, of_kexec_alloc_and_setup_fdt() will ignore the live DT (the empty root one) and copy the 'initial_boot_params' data.

Cc Rob Herring <[email protected]>

Cc Frank Rowand <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Mark Rutland <[email protected]>

Cc <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

Quoted 3 lines

---
 arch/arm64/kernel/setup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Catalin, Will, Can I get an ack on this so I can take the series via the DT tree.

Quoted 21 lines · Rob Herring

Quoted 19 lines · Stephen Boyd

Call this function unconditionally so that we can populate an empty DTB on platforms that don't boot with a firmware provided or builtin DTB. When ACPI is in use, unflatten_device_tree() ignores the 'initial_boot_params' pointer so the live DT on those systems won't be whatever that's pointing to. Similarly, when kexec copies the DT data the previous kernel to the new one on ACPI systems, of_kexec_alloc_and_setup_fdt() will ignore the live DT (the empty root one) and copy the 'initial_boot_params' data.

Cc Rob Herring <[email protected]>

Cc Frank Rowand <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Mark Rutland <[email protected]>

Cc <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 arch/arm64/kernel/setup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Quoted 2 lines

Catalin, Will, Can I get an ack on this so I can take the series via the DT tree.

Mark had strong pretty strong objections to this in version one:

https://lore.kernel.org/all/ZaZtbU9hre3YhZam@FVFF77S0Q05N/

and this patch looks the same now as it did then. Did something else change?

Quoted 23 lines · Will Deacon

Quoted 21 lines · Rob Herring

Quoted 19 lines · Stephen Boyd

Call this function unconditionally so that we can populate an empty DTB on platforms that don't boot with a firmware provided or builtin DTB. When ACPI is in use, unflatten_device_tree() ignores the 'initial_boot_params' pointer so the live DT on those systems won't be whatever that's pointing to. Similarly, when kexec copies the DT data the previous kernel to the new one on ACPI systems, of_kexec_alloc_and_setup_fdt() will ignore the live DT (the empty root one) and copy the 'initial_boot_params' data.

Cc Rob Herring <[email protected]>

Cc Frank Rowand <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Mark Rutland <[email protected]>

Cc <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 arch/arm64/kernel/setup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Quoted 4 lines

Quoted 2 lines

Catalin, Will, Can I get an ack on this so I can take the series via the DT tree.

Mark had strong pretty strong objections to this in version one:

Yes, I had concerns with it as well.

Quoted 2 lines

https://lore.kernel.org/all/ZaZtbU9hre3YhZam@FVFF77S0Q05N/

Quoted 2 lines

and this patch looks the same now as it did then. Did something else change?

Yes, that version unflattened the bootloader passed DT. Now within unflatten_devicetree(), the bootloader DT is ignored if ACPI is enabled and we unflatten an empty tree. That will prevent the kernel getting 2 h/w descriptions if/when a platform does such a thing. Also, kexec still uses the bootloader provided DT as before.

Quoted 33 lines · Rob Herring

Quoted 23 lines · Will Deacon

Quoted 21 lines · Rob Herring

Quoted 19 lines · Stephen Boyd

Call this function unconditionally so that we can populate an empty DTB on platforms that don't boot with a firmware provided or builtin DTB. When ACPI is in use, unflatten_device_tree() ignores the 'initial_boot_params' pointer so the live DT on those systems won't be whatever that's pointing to. Similarly, when kexec copies the DT data the previous kernel to the new one on ACPI systems, of_kexec_alloc_and_setup_fdt() will ignore the live DT (the empty root one) and copy the 'initial_boot_params' data.

Cc Rob Herring <[email protected]>

Cc Frank Rowand <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Mark Rutland <[email protected]>

Cc <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 arch/arm64/kernel/setup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Quoted 4 lines

Quoted 2 lines

Catalin, Will, Can I get an ack on this so I can take the series via the DT tree.

Mark had strong pretty strong objections to this in version one:

Yes, I had concerns with it as well.

Quoted 2 lines

https://lore.kernel.org/all/ZaZtbU9hre3YhZam@FVFF77S0Q05N/

Quoted 8 lines

Quoted 2 lines

and this patch looks the same now as it did then. Did something else change?

Yes, that version unflattened the bootloader passed DT. Now within unflatten_devicetree(), the bootloader DT is ignored if ACPI is enabled and we unflatten an empty tree. That will prevent the kernel getting 2 h/w descriptions if/when a platform does such a thing. Also, kexec still uses the bootloader provided DT as before.

That avoids the main instance of my concern, and means that this'll boot without issue, but IIUC this opens the door to dynamically instantiating DT devices atop an ACPI base system, which I think in general is something that's liable to cause more problems than it solves.

I understand that's desireable for the selftests, though I still don't believe it's strictly necessary -- there are plenty of other things that only work if the kernel is booted in a specific configuration.

Putting the selftests aside, why do we need to do this? Is there any other reason to enable this?

Quoted 7 lines

Quoted 6 lines

Yes, that version unflattened the bootloader passed DT. Now within unflatten_devicetree(), the bootloader DT is ignored if ACPI is enabled and we unflatten an empty tree. That will prevent the kernel getting 2 h/w descriptions if/when a platform does such a thing. Also, kexec still uses the bootloader provided DT as before.

Quoted 8 lines

That avoids the main instance of my concern, and means that this'll boot without issue, but IIUC this opens the door to dynamically instantiating DT devices atop an ACPI base system, which I think in general is something that's liable to cause more problems than it solves.

I understand that's desireable for the selftests, though I still don't believe it's strictly necessary -- there are plenty of other things that only work if the kernel is booted in a specific configuration.

Why add to the test matrix if we don't have to?

Quoted 2 lines

Putting the selftests aside, why do we need to do this? Is there any other reason to enable this?

See my Plumbers talk...

Or in short, there's 3 main usecases:

- PCI FPGA card with devices instantiated in it
- SoCs which expose their peripherals via a PCI endpoint.
- Injecting test devices with QEMU (testing, but not what this series does. Jonathan Cameron's usecase)

In all cases, drivers already exist for the devices, and they often only support DT. DT overlays is the natural solution for this, and there's now kernel support for it (dynamically generating PCI DT nodes when they don't exist). The intent is to do the same thing on ACPI systems.

I don't see another solution other than 'go away, you're crazy'. There's ACPI overlays, but that's only a debug feature. Also, that would encourage more of the DT bindings in ACPI which I find worse than this mixture. There's swnodes, but that's just board files and platform_data 2.0.

I share the concerns with mixing, but I don't see a better solution. The scope of what's possible is contained enough to avoid issues.

Call this function unconditionally so that we can populate an empty DTB on platforms that don't boot with a firmware provided or builtin DTB. When ACPI is in use, unflatten_device_tree() ignores the 'initial_boot_params' pointer so the live DT on those systems won't be whatever that's pointing to. Similarly, when kexec copies the DT data the previous kernel to the new one on ACPI systems, of_kexec_alloc_and_setup_fdt() will ignore the live DT (the empty root one) and copy the 'initial_boot_params' data.

Cc Rob Herring <[email protected]>

Cc Frank Rowand <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Mark Rutland <[email protected]>

Cc <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 arch/arm64/kernel/setup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

arch/arm64/kernel/setup.c +1 -2

index 42c690bb2d60..0d210720d47d 100644
@@ -351,8 +351,7 @@ void __init __no_sanitize_address setup_arch(char **cmdline_p)
 	/* Parse the ACPI tables for possible boot-time configuration */
 	acpi_boot_table_init();
 
-	if (acpi_disabled)
-		unflatten_device_tree();
+	unflatten_device_tree();
 
 	bootmem_init();

Arch maintainers, please ack/review patches.

This is a resend of a series from Frank last year[1]. I worked in Rob's review comments to unconditionally call unflatten_device_tree() and fixup/audit calls to of_have_populated_dt() so that behavior doesn't change.

I need this series so I can add DT based tests in the clk framework. Either I can merge it through the clk tree once everyone is happy, or Rob can merge it through the DT tree and provide some branch so I can base clk patches on it.

Changes from v3 (https://lore.kernel.org/r/[email protected]):
* Made OF_UNITTEST depend on OF_EARLY_FLATREE
* Made OF_EARLY_FLATREE depend on absence of arches that don't call unflatten_device_tree()
* Added of_ prefix to dtb_ prefixed KUnit tests
* Picked up tags

Changes from v2 (https://lore.kernel.org/r/[email protected]):
* Reorder patches to have OF changes largely first
* No longer modify initial_boot_params if ACPI=y
* Put arm64 patch back to v1

Changes from v1 (https://lore.kernel.org/r/[email protected]):
* x86 patch included
* arm64 knocks out initial dtb if acpi is in use
* keep Kconfig hidden but def_bool enabled otherwise

Changes from Frank's series[1]:
* Add a DTB loaded kunit test
* Make of_have_populated_dt() return false if the DTB isn't from the bootloader
* Architecture calls made unconditional so that a root node is always made

Frank Rowand (2):
of: Create of_root if no dtb provided by firmware
of: unittest: treat missing of_root as error instead of fixing up

Stephen Boyd (5):
of: Always unflatten in unflatten_and_copy_device_tree()
um: Unconditionally call unflatten_device_tree() x86/of: Unconditionally call unflatten_and_copy_device_tree()
arm64: Unconditionally call unflatten_device_tree()
of: Add KUnit test to confirm DTB is loaded