Re: [PATCH 07/12] riscv: dts: allwinner: Add Allwinner D1 Nezha devicetree

[Samuel Holland]

Hi Conor,

On 8/19/22 5:10 PM, Conor.Dooley@xxxxxxxxxxxxx wrote:
> Finally got around to giving this a go with the fix for loading
> modules which is mostly what was blocking me before..
> 
> On 15/08/2022 06:08, Samuel Holland wrote:
>> "D1 Nezha" is Allwinner's first-party development board for the D1 SoC.
>> It was shipped with 512M, 1G, or 2G of DDR3. It supports onboard audio,
> 
> I am really not keen on the way you have things, with the memory
> nodes removed from the device tree. I know your preferred flow
> for booting these things might be to pass the dtb up from U-Boot,
> but I think the devicetree in the kernel should be usable in a
> standalone manner, even if that is the barest-minimum memory
> config.

That is simply not possible to guarantee. As an obvious example, consider the
MangoPi MQ-Pro board with socketed DRAM:

https://twitter.com/mangopi_sbc/status/1516225559214583808

But focusing on the /memory node misses the bigger picture. The DTB is passed
through _all_ of the firmware stages, and gets patched by every one of them:

 - SPL/boot0 adds the /memory node with the detected DRAM size.
   If the in-tree DTS has a "minimum memory config" (which for a
   board with socketed DRAM means the smallest possible die), I
   guarantee people will use it and complain about missing DRAM.

 - The SBI implementation reserves memory for itself and any
   possible secure partitions. Right now, booting happens to work
   without the reserved-memory node because the SBI implementation
   is loaded at the beginning of RAM, and Linux ignores RAM below
   the kernel load address.

   However, memory-constrained devices (e.g. D1s) will need to get
   those 2 MiB back by loading the kernel at the start of DRAM and
   SBI at the end of DRAM. Then the reserved-memory node becomes
   quite important.

   It also adds nodes for CPU idle states, since the available
   states and their latencies depend on the SBI implementation.

   It also reserves devices used by it or by a secure partition.
   And it is responsible for extracting data (e.g. MAC addresses)
   from "secure" eFuses which the OS may not have access to.

 - U-Boot adds other information, like boot arguments, the address
   of the initramfs and framebuffer, etc. These are less of a
   concern because of course U-Boot can patch these in to a DTB
   loaded from disk, but they are relevant if you want to load a
   DTB from a later bootloader like GRUB.

If you load a DTB from disk, you lose all of the changes made by the earlier
firmware stages. On ARM, U-Boot tries to work around this by copying a few
specific bits of information from the firmware DTB to the DTB loaded from disk.
But this misses the point that the SBI implementation can modify *any* part of
the DTB. (So in practice U-Boot on ARM already loses CPU idle states and
reserved memory nodes that were added by the PSCI implementation.)

As an extreme example, consider paravirtualization, where only a small subset of
DRAM and peripherals may be made available to any one OS partition.

Fundamentally, I reserve the right to make arbitrary changes to the DTB in the
SBI implementation, and thus I cannot condone using the DTBs generated from the
Linux source tree for any purpose other than validation.

Regards,
Samuel