2017-12-20 09:40:03

by Zong Li

[permalink] [raw]
Subject: [PATCH] RISC-V: Support built-in dtb

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 <[email protected]>
---
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


2017-12-21 20:33:04

by Palmer Dabbelt

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

On Wed, 20 Dec 2017 01:14:31 PST (-0800), [email protected] 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 <[email protected]>
> ---
> 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);
> }
>
> /*

2017-12-21 20:43:24

by Arnd Bergmann

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

On Thu, Dec 21, 2017 at 9:32 PM, Palmer Dabbelt <[email protected]> wrote:
> On Wed, 20 Dec 2017 01:14:31 PST (-0800), [email protected] 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

2017-12-21 20:48:09

by Palmer Dabbelt

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

On Thu, 21 Dec 2017 12:43:20 PST (-0800), Arnd Bergmann wrote:
> On Thu, Dec 21, 2017 at 9:32 PM, Palmer Dabbelt <[email protected]> wrote:
>> On Wed, 20 Dec 2017 01:14:31 PST (-0800), [email protected] 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.