2023-12-01 18:46:13

by Masahiro Yamada

[permalink] [raw]
Subject: Question about BUILTIN_DTB support in RISCV

Hi.

I have a question about CONFIG_BUILTIN_DTB for riscv.

Please see this commit history.


[1]
2d2682512f0faf4d09a696184bf3c0bb6838baca
added built-in DTB support, attempting
to include multiple DTB into vmlinux
by using SOC_BUILTIN_DTB_DECLARE() macro.


[2]
d5805af9fe9ffe4a9d975e9bc39496f57a161076
pointed out that choosing the correct DTB
is impossible. It fell back to the single
built-in DTB support, like other architectures.

[3]
0ddd7eaffa644baa78e247bbd220ab7195b1eed6
added BUILTIN_DTB support for sifive and microchip,
while apparently multiple DTBs are embedded into vmlinux.



So, how does it work?


With
CONFIG_ARCH_MICROCHIP_POLARFIRE=y
CONFIG_ARCH_SIFIVE=y
CONFIG_BUILTIN_DTB=y

7 DTB files are embedded in vmlinux.


masahiro@zoe:~/ref/linux(master)$ riscv64-linux-gnu-nm -n vmlinux |
grep -A15 dtb_start
ffffffff82112620 D __dtb_start
ffffffff82115a2c D __dtb_mpfs_icicle_kit_end
ffffffff82115a40 D __dtb_mpfs_m100pfsevp_begin
ffffffff82118b3b D __dtb_mpfs_m100pfsevp_end
ffffffff82118b40 D __dtb_mpfs_polarberry_begin
ffffffff8211b9c2 D __dtb_mpfs_polarberry_end
ffffffff8211b9e0 D __dtb_mpfs_sev_kit_begin
ffffffff8211e7bb D __dtb_mpfs_sev_kit_end
ffffffff8211e7c0 D __dtb_mpfs_tysom_m_begin
ffffffff8212162e D __dtb_mpfs_tysom_m_end
ffffffff82121640 D __dtb_hifive_unleashed_a00_begin
ffffffff821236af D __dtb_hifive_unleashed_a00_end
ffffffff821236c0 D __dtb_hifive_unmatched_a00_begin
ffffffff8212621b D __dtb_hifive_unmatched_a00_end
ffffffff82126220 D __dtb_end


In my understanding, the first one
(mpfs-icicle-kit.dtb) is always used.

You cannot use the other 6 DTBs.
Am I missing something?



arch/riscv64/boot/dts/canaan/Makefile
is correct because only one DTB is embedded
if CONFIG_ARCH_CANAAN_K210_DTB_SOURCE contains
a single word.



--
Best Regards
Masahiro Yamada


2023-12-02 11:46:24

by Conor Dooley

[permalink] [raw]
Subject: Re: Question about BUILTIN_DTB support in RISCV

On Sat, Dec 02, 2023 at 03:45:00AM +0900, Masahiro Yamada wrote:
> Hi.
>
> I have a question about CONFIG_BUILTIN_DTB for riscv.
>
> Please see this commit history.
>
>
> [1]
> 2d2682512f0faf4d09a696184bf3c0bb6838baca
> added built-in DTB support, attempting
> to include multiple DTB into vmlinux
> by using SOC_BUILTIN_DTB_DECLARE() macro.
>
>
> [2]
> d5805af9fe9ffe4a9d975e9bc39496f57a161076
> pointed out that choosing the correct DTB
> is impossible. It fell back to the single
> built-in DTB support, like other architectures.
>
> [3]
> 0ddd7eaffa644baa78e247bbd220ab7195b1eed6
> added BUILTIN_DTB support for sifive and microchip,
> while apparently multiple DTBs are embedded into vmlinux.

The thread for this one is interesting:
https://lore.kernel.org/all/[email protected]/

None of the other !canaan platforms actually support this, so it is only
for PolarFire and the SiFive boards that this is enabled. It sounds,
from your comments below, that this does not even work. If it doesn't, I
would be inclined to just delete it.

If XIP is the user, we want to deprecate XIP anyway, so pointing out
that this has not worked properly in a long time for PolarFire and
?ever? for the SiFive unmatched is another nail in the coffin for that.

My vote would be for reverting [3] and removing support for XIP :)

> So, how does it work?
>
>
> With
> CONFIG_ARCH_MICROCHIP_POLARFIRE=y
> CONFIG_ARCH_SIFIVE=y
> CONFIG_BUILTIN_DTB=y
>
> 7 DTB files are embedded in vmlinux.
>
>
> masahiro@zoe:~/ref/linux(master)$ riscv64-linux-gnu-nm -n vmlinux |
> grep -A15 dtb_start
> ffffffff82112620 D __dtb_start
> ffffffff82115a2c D __dtb_mpfs_icicle_kit_end
> ffffffff82115a40 D __dtb_mpfs_m100pfsevp_begin
> ffffffff82118b3b D __dtb_mpfs_m100pfsevp_end
> ffffffff82118b40 D __dtb_mpfs_polarberry_begin
> ffffffff8211b9c2 D __dtb_mpfs_polarberry_end
> ffffffff8211b9e0 D __dtb_mpfs_sev_kit_begin
> ffffffff8211e7bb D __dtb_mpfs_sev_kit_end
> ffffffff8211e7c0 D __dtb_mpfs_tysom_m_begin
> ffffffff8212162e D __dtb_mpfs_tysom_m_end
> ffffffff82121640 D __dtb_hifive_unleashed_a00_begin
> ffffffff821236af D __dtb_hifive_unleashed_a00_end
> ffffffff821236c0 D __dtb_hifive_unmatched_a00_begin
> ffffffff8212621b D __dtb_hifive_unmatched_a00_end
> ffffffff82126220 D __dtb_end
>
>
> In my understanding, the first one
> (mpfs-icicle-kit.dtb) is always used.
>
> You cannot use the other 6 DTBs.
> Am I missing something?

Alex? I gave a cursory test and it does in fact behave like this.


Attachments:
(No filename) (2.53 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-20 16:47:41

by Yangyu Chen

[permalink] [raw]
Subject: Re: Question about BUILTIN_DTB support in RISCV

Same problem here. I review the code in the current mainline kernel.
On arch/riscv/kernel/head.S:297:

```asm
#ifdef CONFIG_BUILTIN_DTB
la a0, __dtb_start
XIP_FIXUP_OFFSET a0
#else
mv a0, a1
#endif /* CONFIG_BUILTIN_DTB */
call setup_vm
```

Then, function `void __init setup_vm(uintptr_t dtb_pa)` will take this
dtb address from a0, and consume the dtb_pa as the only one dtb to set
dtb_early_va and dtb_early_pa. It will never change. Then, the
`parse_dtb` function which will be called from start_kernel-
>setup_arch->parse_dtb will only use dtb_early_va.

I think there is something that might need to be changed. As
CONFIG_BUILTIN_DTB depends on NONPORTABLE, we might need a config like
BUILTIN_DTB_NAME / BUILTIN_DTB_SOURCE to specify what dtb to be linked
to the kernel as loongarch / sh / xtensa does. Currently, we have only
ARCH_CANAAN_K210_DTB_SOURCE to select when building the K210 kernel.
Things to do is to make this configuration more general and let it be
set on all riscv rather than k210 only currently.

Thanks,
Yangyu Chen