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

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

[Zong Li]

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@gmail.com>
---
 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
 
+config RISCV_BUILTIN_DTB
+        string "Builtin DTB"
+        default ""
+
 endmenu
 
 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
 endif
 
+ifneq '$(CONFIG_RISCV_BUILTIN_DTB)' '""'
+BUILTIN_DTB := y
+else
+BUILTIN_DTB := n
+endif
+
 # 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)/install.sh $(KERNELRELEASE) \
+	$(obj)/Image System.map "$(INSTALL_PATH)"
+
+zinstall: $(obj)/Image.gz
+	$(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \
+	$(obj)/Image.gz System.map "$(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
+
+ifneq '$(CONFIG_RISCV_BUILTIN_DTB)' '""'
+BUILTIN_DTB := $(patsubst "%",%,$(CONFIG_RISCV_BUILTIN_DTB)).dtb.o
+else
+BUILTIN_DTB :=
+endif
+
+obj-$(CONFIG_OF) += $(BUILTIN_DTB)
+
+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);
 }
 
 /*
-- 
2.7.4

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

[Palmer Dabbelt]

On Wed, 20 Dec 2017 01:14:31 PST (-0800), zongbox@gmail.com 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@gmail.com>
> ---
>  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
>
> +config RISCV_BUILTIN_DTB
> +        string "Builtin DTB"
> +        default ""
> +
>  endmenu

It looks like most other architectures handle this via something like

    config RISCV_BUILTIN_DTB
            bool "Embed DTB in kernel image"
            select BUILTIN_DTB
            help
              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.
    
    config RISCV_BUILTIN_DTB_NAME
            string "Built in DTB"
            depends on RISCV_BUILTIN_DTB
            help
              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
>  endif
>
> +ifneq '$(CONFIG_RISCV_BUILTIN_DTB)' '""'
> +BUILTIN_DTB := y
> +else
> +BUILTIN_DTB := n
> +endif

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)/install.sh $(KERNELRELEASE) \
> +	$(obj)/Image System.map "$(INSTALL_PATH)"
> +
> +zinstall: $(obj)/Image.gz
> +	$(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \
> +	$(obj)/Image.gz System.map "$(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
> +
> +ifneq '$(CONFIG_RISCV_BUILTIN_DTB)' '""'
> +BUILTIN_DTB := $(patsubst "%",%,$(CONFIG_RISCV_BUILTIN_DTB)).dtb.o
> +else
> +BUILTIN_DTB :=
> +endif
> +
> +obj-$(CONFIG_OF) += $(BUILTIN_DTB)
> +
> +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);
>  }
>
>  /*

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

[Arnd Bergmann]

On Thu, Dec 21, 2017 at 9:32 PM, Palmer Dabbelt <palmer@sifive.com> wrote:
> On Wed, 20 Dec 2017 01:14:31 PST (-0800), zongbox@gmail.com wrote:

> 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.

Having an embedded DTB is only necessary for platforms with a legacy bootloader
that doesn't understand DT at all, you should never need that here.

I would suggest to require each bootloader to provide a way to replace the DTB
with one that gets installed alongside the kernel.

        Arnd

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

[Palmer Dabbelt]

On Thu, 21 Dec 2017 12:43:20 PST (-0800), Arnd Bergmann wrote:
> On Thu, Dec 21, 2017 at 9:32 PM, Palmer Dabbelt <palmer@sifive.com> wrote:
>> On Wed, 20 Dec 2017 01:14:31 PST (-0800), zongbox@gmail.com wrote:
>
>> 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.
>
> Having an embedded DTB is only necessary for platforms with a legacy bootloader
> that doesn't understand DT at all, you should never need that here.
>
> I would suggest to require each bootloader to provide a way to replace the DTB
> with one that gets installed alongside the kernel.

Sounds good to me.

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

[Zong Li]

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@gmail.com>
---
 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
 
+config RISCV_BUILTIN_DTB
+        string "Builtin DTB"
+        default ""
+
 endmenu
 
 menu "Kernel type"
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 6719dd3..baed60a 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -57,6 +57,12 @@ ifeq ($(CONFIG_CMODEL_MEDANY),y)
 	KBUILD_CFLAGS += -mcmodel=medany
 endif
 
+ifneq '$(CONFIG_NDS32_BUILTIN_DTB)' '""'
+BUILTIN_DTB := y
+else
+BUILTIN_DTB := n
+endif
+
 # 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)/install.sh $(KERNELRELEASE) \
+	$(obj)/Image System.map "$(INSTALL_PATH)"
+
+zinstall: $(obj)/Image.gz
+	$(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \
+	$(obj)/Image.gz System.map "$(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
+
+ifneq '$(CONFIG_RISCV_BUILTIN_DTB)' '""'
+BUILTIN_DTB := $(patsubst "%",%,$(CONFIG_RISCV_BUILTIN_DTB)).dtb.o
+else
+BUILTIN_DTB :=
+endif
+
+obj-$(CONFIG_OF) += $(BUILTIN_DTB)
+
+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);
 }
 
 /*
-- 
2.7.4