| Message ID | 20231009172717.2695854-1-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: pad generated DTB files | expand |
On Mon, Oct 09, 2023 at 08:27:17PM +0300, Dmitry Baryshkov wrote: > On Qualcomm platforms the bootloader populates device tree with some > extra nodes / properties (like memory size, boot time, etc). Usually > default padding is enough for the bootloader. But in some cases the > board will fail to boot if there is not enough padding space. > > Add `--pad 4096' to DTC_FLAGS so that all Qualcomm DTB files get this > extra padding space. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > > This is primarily necessary for SA8155P, but I have the feeling that it > might be better to be enabled on the global scale. By default there should not be any padding at all. This is because the bootloader is responsible to make room for new nodes by calling fdt_open_into() with an adjusted size. This will result in a simple memmove() that shifts the end of the DTB in memory so that the padding can be used for new nodes and properties. If the bootloader doesn't add enough padding then it is broken and should be fixed. Both LK [1] and ABL [2] do or have done this correctly at some point. If more space is needed for some weird new modifications the padding size there should be adjusted. [1]: https://git.codelinaro.org/clo/la/kernel/lk/-/blob/lk.lnx.1.0.r54-rel/platform/msm_shared/dev_tree.c#L2051-2057 [2]: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/blob/uefi.lnx.4.0.r40-rel/QcomModulePkg/Library/BootLib/UpdateDeviceTree.c#L1402-1414 By adding --pad 4096 globally we would waste pointless empty space for every single DTB, which ends up on all systems that use generic kernels with Qualcomm support included. With the ~230 DTBs we have at the moment this would already waste ~1 MiB (~16 MiB -> ~17 MiB total). So please: - If you can, update the bootloader and fix the padding size there. - If this is not possible: Add the padding only for the boards with broken bootloaders with a clear comment that this should be the last resort for devices that are locked down. - Or maybe boot a less broken bootloader inbetween (like U-Boot). :) Thanks, Stephan > > --- > arch/arm64/boot/dts/qcom/Makefile | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > index d6cb840b7050..8e9fa2539265 100644 > --- a/arch/arm64/boot/dts/qcom/Makefile > +++ b/arch/arm64/boot/dts/qcom/Makefile > @@ -1,4 +1,8 @@ > # SPDX-License-Identifier: GPL-2.0 > + > +# pad DT allowing bootloader to populate several extra nodes > +DTC_FLAGS += --pad 4096 > + > dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb > > apq8016-sbc-usb-host-dtbs := apq8016-sbc.dtb apq8016-sbc-usb-host.dtbo > -- > 2.39.2 >
On Tue, 10 Oct 2023 at 10:40, Stephan Gerhold <stephan@gerhold.net> wrote: > > On Mon, Oct 09, 2023 at 08:27:17PM +0300, Dmitry Baryshkov wrote: > > On Qualcomm platforms the bootloader populates device tree with some > > extra nodes / properties (like memory size, boot time, etc). Usually > > default padding is enough for the bootloader. But in some cases the > > board will fail to boot if there is not enough padding space. > > > > Add `--pad 4096' to DTC_FLAGS so that all Qualcomm DTB files get this > > extra padding space. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > > > This is primarily necessary for SA8155P, but I have the feeling that it > > might be better to be enabled on the global scale. > > By default there should not be any padding at all. This is because the > bootloader is responsible to make room for new nodes by calling > fdt_open_into() with an adjusted size. This will result in a simple > memmove() that shifts the end of the DTB in memory so that the padding > can be used for new nodes and properties. > > If the bootloader doesn't add enough padding then it is broken and > should be fixed. Several other platforms use --pad, see microblaze, openrisc and powerpc. In some cases we just can not update the bootloader. > > Both LK [1] and ABL [2] do or have done this correctly at some point. > If more space is needed for some weird new modifications the padding > size there should be adjusted. > > [1]: https://git.codelinaro.org/clo/la/kernel/lk/-/blob/lk.lnx.1.0.r54-rel/platform/msm_shared/dev_tree.c#L2051-2057 > [2]: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/blob/uefi.lnx.4.0.r40-rel/QcomModulePkg/Library/BootLib/UpdateDeviceTree.c#L1402-1414 In this case ABL fails to handle the situation: Cmdline: ignore_loglevel console=tty0 console=ttyMSM0,115200n8 earlycon root=PARTLABEL=rootfs rootwait adbd -- androidboot.verifiedbootstate Error adding node Error carving out UEFI memory: FFFFFFFF Adding --pad 1024 is enough for this board to boot. > > By adding --pad 4096 globally we would waste pointless empty space for > every single DTB, which ends up on all systems that use generic kernels > with Qualcomm support included. With the ~230 DTBs we have at the moment > this would already waste ~1 MiB (~16 MiB -> ~17 MiB total). > > So please: > > - If you can, update the bootloader and fix the padding size there. > > - If this is not possible: Add the padding only for the boards with > broken bootloaders with a clear comment that this should be the last > resort for devices that are locked down. Yep, this sounds like the correct approach in this case. Thank you. > > - Or maybe boot a less broken bootloader inbetween (like U-Boot). :) > > Thanks, > Stephan > > > > > --- > > arch/arm64/boot/dts/qcom/Makefile | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > > index d6cb840b7050..8e9fa2539265 100644 > > --- a/arch/arm64/boot/dts/qcom/Makefile > > +++ b/arch/arm64/boot/dts/qcom/Makefile > > @@ -1,4 +1,8 @@ > > # SPDX-License-Identifier: GPL-2.0 > > + > > +# pad DT allowing bootloader to populate several extra nodes > > +DTC_FLAGS += --pad 4096 > > + > > dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb > > > > apq8016-sbc-usb-host-dtbs := apq8016-sbc.dtb apq8016-sbc-usb-host.dtbo > > -- > > 2.39.2 > > -- With best wishes Dmitry
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile index d6cb840b7050..8e9fa2539265 100644 --- a/arch/arm64/boot/dts/qcom/Makefile +++ b/arch/arm64/boot/dts/qcom/Makefile @@ -1,4 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 + +# pad DT allowing bootloader to populate several extra nodes +DTC_FLAGS += --pad 4096 + dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb apq8016-sbc-usb-host-dtbs := apq8016-sbc.dtb apq8016-sbc-usb-host.dtbo
On Qualcomm platforms the bootloader populates device tree with some extra nodes / properties (like memory size, boot time, etc). Usually default padding is enough for the bootloader. But in some cases the board will fail to boot if there is not enough padding space. Add `--pad 4096' to DTC_FLAGS so that all Qualcomm DTB files get this extra padding space. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- This is primarily necessary for SA8155P, but I have the feeling that it might be better to be enabled on the global scale. --- arch/arm64/boot/dts/qcom/Makefile | 4 ++++ 1 file changed, 4 insertions(+)