Message-ID: <tencent_DF3D7C2E8248DA188E3FB8DF31FE21478D08@qq.com>
Date: Wed, 21 Feb 2024 22:28:08 +0800
From: Yangyu Chen <cyy@...self.name>
To: Conor Dooley <conor@...nel.org>
Cc: linux-riscv@...ts.infradead.org, Paul Walmsley
 <paul.walmsley@...ive.com>,  Palmer Dabbelt <palmer@...belt.com>, Albert Ou
 <aou@...s.berkeley.edu>, linux-kernel@...r.kernel.org, Masahiro Yamada
 <masahiroy@...nel.org>, Alexandre Ghiti <alex@...ti.fr>, Rob Herring
 <robh+dt@...nel.org>,  devicetree@...r.kernel.org
Subject: Re: [RFC PATCH 0/1] riscv: dts: Allow BUILTIN_DTB for all socs

On Wed, 2024-02-21 at 11:30 +0000, Conor Dooley wrote:
> Hey,
> 
> On Wed, Feb 21, 2024 at 03:01:53AM +0800, Yangyu Chen wrote:
> > The BUILTIN_DTB kernel feature on RISC-V only works on K210 SoC
> > only. This
> > patch moved this configuration to entire riscv.
> 
> To be honest, I would rather delete BUILTIN_DTB (and the
> configurations
> that depend on it) than expand its usefulness.
> 

I agree it’s useless for most platforms because we need to start SBI
before kernel on RISC-V except NOMMU M-Mode Linux and SBI also need a
DT to work. However, it has been there for M-Mode K210 and it is set by
default for XIP kernel. So there might eventually be another patch to
support some new soc that will do this like this patch.

> > Although BUILTIN_DTB is not a good choice for most platforms, it is
> > likely
> > to be a debug feature when some bootloader will always override
> > something
> > like the memory node in the device tree to adjust the memory size
> > from SPD
> > or configuration resistor, which makes it hard to do some
> > debugging.
> 
> My inclination here is to say "fix your bootloader" and if that's not
> possible, chainload a bootloader that allows you control over
> modifications to your devicetree.
> 

Chainload a bootloader like S-Mode U-Boot on some platforms is hard due
to some drivers like pcie controller does not come to the mainline repo
of the bootloader, and some bootloader source repos provided by the
vendor may require specific versions of the compiler to work, which
makes users not easy to do some kernel debugging if change DT is
needed. The simplest way to do this I can imagine is to write a simple
bootloader by myself link the kernel binary and the dtb I want to it
and replace the a1 register point to the dtb address before jumping to
the kernel. However, kernel has this feature, why should I do it
manually rather than provide a more generic patch for everyone with
this need to use?

> > As an
> > example, some platforms with numa like sg2042 only support sv39
> > will fail
> > to boot when there is no ZONE_HIGHMEM patch with 128G memory. If we
> > want
> > a kernel without this patch to boot, we need to write the memory
> > nodes 
> > in the DT manually.
> 
> If, as Alex suggests, there's a way to gain support some more memory
> in
> sv39, we should do so - but it is worth mentioning that highmem is on
> the
> removal list for the kernel, so mainline support for that is highly
> unlikely.
> 

Yes. But I’m debugging some mm performance issues on the sg2042 kernel.
Specifically, it’s about the IPI latency when doing rfence on
sfence.vma or fence.i. I would like to reduce the memory size and allow
the mainline kernel to boot and test without taking some out-of-tree
kernel patches. If I remove some DIMM modules from the board to reduce
the memory size, it will also lose some memory channels and even leave
some numa nodes with zero memory, and the compatible DIMM module is
hard to find.

> > Also, changing DT on some platforms is not easy. For Milk-V
> > Pioneer, the
> > boot procedure is ZSBL -> OpenSBI -> LinuxBoot -> Linux. If DT gets
> > changed, OpenSBI or LinuxBoot may refuse to boot. And there is some
> > bug on
> > LinuxBoot now which does not consume --dtb argument on kexec and
> > always
> > uses DT from memory.
> 
> I don't use Linuxboot, but let me try to understand. Linuxboot uses
> kexec
> to boot the main Linux kernel, but the dtb you want to use is not
> used, and
> instead the one that Linuxboot itself was booted with is used?
> 
> It sounds like Linuxboot has a --dtb argumet that is meant to be used
> to
> set the dtb for the next stage, but that argument is being ignored?
> 

Yes. That’s correct.

> That sounds like a pretty serious issue with Linuxboot which should
> be
> fixed - what am I missing?
> 


Sure, that should be fixed in the LinuxBoot. However, I think not every
kernel developer should fix some complex bootloader like LinuxBoot
which is built upon the linux kernel with a huge initrd rootfs and runs
some userspace tools to support the boot process. If something is hard
to control, skip it, and doing some override for debugging will be a
better choice.

> > So I would like to do debugging on DT using
> > BUILTIN_DTB, which makes it very simple,
> 
> > I can even install the kernel in
> > the distro's way and provide a kernel package for other users to
> > test.
> 
> I'm not sure what you mean by this, other distros manage to create
> kernel packages without using builtin dtbs.
> 

I mean I can provide a distro package like Debian .deb and distribute
it to other users to test without changing their dtb from the entire
boot process. Because changing the DT from the entire boot process
might prevent their vendor-provided OpenSBI or LinuxBoot from working.
Some vendor kernels may be developed out-of-tree and do not use the dt-
binding from mainline. Even for very basic CLINT and PLIC dt bindings.
It is only for testing, not for the production environment.

> Thanks,
> Conor.

I want this feature to allow more people to participate in debugging
some kernel issues without taking a huge amount of time to deal with
bootloader issues about changing the DT. I think it will be good for
our under-development RISC-V community. Imagine we hardly change the
ACPI table for x86 machines but we sometimes change the DT for
ARM/RISC-V board, right? Also, some SoCs that run M-Mode NOMMU Linux
may need it in the future like K210 for XIP without a prior bootloader.

Thanks,
Yangyu Chen.

Powered by blists - more mailing lists