Re: [patches] [PATCH] RISC-V: Support built-in dtb

From: Palmer Dabbelt
Date: Thu Dec 21 2017 - 15:33:10 EST

On Wed, 20 Dec 2017 01:14:31 PST (-0800), zongbox@xxxxxxxxx wrote:
Build the dtb into the kernel image.
If the DTB is given via bootloader, the external DTB is adopted first.

Signed-off-by: Zong Li <zongbox@xxxxxxxxx>
arch/riscv/Kconfig | 4 ++++
arch/riscv/Makefile | 9 +++++++++
arch/riscv/boot/Makefile | 17 +++++++++++++++++
arch/riscv/boot/dts/Makefile | 11 +++++++++++
arch/riscv/kernel/setup.c | 2 +-
5 files changed, 42 insertions(+), 1 deletion(-)
create mode 100644 arch/riscv/boot/Makefile
create mode 100644 arch/riscv/boot/dts/Makefile

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 2e15e85..831cbb9 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -189,6 +189,10 @@ config RISCV_ISA_C
config RISCV_ISA_A
def_bool y

+ string "Builtin DTB"
+ default ""

It looks like most other architectures handle this via something like

bool "Embed DTB in kernel image"
Embeds a device tree binary in the kernel image. The bootloader's device tree will override the kernel's if the bootloader provides a device tree.
string "Built in DTB"
Set the name of the DTB to embed (leave blank to pick one
automatically based on kernel configuration).

which matches what Linux does for other things (CMDLINE_BOOL/CMDLINE and BLK_DEV_INITRD/INITRAMFS_SOURCE are the two I can think of). I don't see any reason to be different here.

Additionally: I'm not sure I like the idea of having a bultin DTB. We're using device tree to ensure the kernel is portable between different RISC-V systems, and building one into the kernel seems to defeat that purpose. Since even BBL can provide a device tree to Linux I don't think there's any reason to build one in -- hopefully real systems will use a proper bootloader, and that will have support for device trees as well.

I've added Arnd and Olof, in case they have a bit more perspective here. If I'm reading this correctly, there isn't an arm or arm64 option to do this. There is an option to built in many DTBs, which makes a lot more sense to me as it doesn't tie the kernel to any one particular implementation. We'd need a mechanism for picking the DTB that Linux should use. We've kicked around the idea of:

* Having the bootloader always provide a DTB.
* Taking a hash of that DTB when booting Linux.
* Using that hash to look up DTBs that are built in to Linux.

This would allow us to support replacing broken DTBs if they escape into the wild, but would still allow us to have a portable kernel.

What is this Kconfig entry meant to be used for?

menu "Kernel type"
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 6719dd3..4c5c9f8 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -57,6 +57,12 @@ ifeq ($(CONFIG_CMODEL_MEDANY),y)
KBUILD_CFLAGS += -mcmodel=medany


Maybe I've lost my mind, but I can't find anything that actually uses CONFIG_BUILTIN_DTB outside of arch-specific code.

# GCC versions that support the "-mstrict-align" option default to allowing
# unaligned accesses. While unaligned accesses are explicitly allowed in the
# RISC-V ISA, they're emulated by machine mode traps on all extant
@@ -69,4 +75,7 @@ core-y += arch/riscv/kernel/ arch/riscv/mm/

libs-y += arch/riscv/lib/

+boot := arch/riscv/boot
+core-$(BUILTIN_DTB) += $(boot)/dts/
all: vmlinux
diff --git a/arch/riscv/boot/Makefile b/arch/riscv/boot/Makefile
new file mode 100644
index 0000000..003d697
--- /dev/null
+++ b/arch/riscv/boot/Makefile
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+targets := Image Image.gz
+$(obj)/Image: vmlinux FORCE
+ $(call if_changed,objcopy)
+$(obj)/Image.gz: $(obj)/Image FORCE
+ $(call if_changed,gzip)
+install: $(obj)/Image
+ $(CONFIG_SHELL) $(srctree)/$(src)/ $(KERNELRELEASE) \
+ $(obj)/Image "$(INSTALL_PATH)"
+zinstall: $(obj)/Image.gz
+ $(CONFIG_SHELL) $(srctree)/$(src)/ $(KERNELRELEASE) \
+ $(obj)/Image.gz "$(INSTALL_PATH)"
diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
new file mode 100644
index 0000000..b65d070
--- /dev/null
+++ b/arch/riscv/boot/dts/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+BUILTIN_DTB := $(patsubst "%",%,$(CONFIG_RISCV_BUILTIN_DTB)).dtb.o
+clean-files := *.dtb *.dtb.S
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index e59a28c..3c89f3d 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -149,7 +149,7 @@ asmlinkage void __init setup_vm(void)

void __init sbi_save(unsigned int hartid, void *dtb)
- early_init_dt_scan(__va(dtb));
+ early_init_dt_scan(dtb ? __va(dtb) : __dtb_start);