On Tue, 2026-09-08 at 17:12 -0300, Ricardo Salveti wrote:
> Hi,
>
> On Mon, Sep 7, 2026 at 1:28 PM <[email protected]> wrote:
> >
> > On Sat, 2026-09-05 at 22:44 +0000, Ricardo Salveti via
> > lists.openembedded.org wrote:
> > > concat_spl_dtb() rebuilds the SPL as a plain concatenation of the
> > > nodtb
> > > image and the signed device tree. U-Boot only assembles it that
> > > way
> > > with
> > > SPL_SEPARATE_BSS; otherwise BSS is part of the image and it pads
> > > between
> > > the two, so that clearing BSS cannot overwrite the appended
> > > device
> > > tree.
> > > Platforms loading the SPL as one contiguous image, such as
> > > Qualcomm
> > > SoCs,
> > > need that layout: without the padding the device tree ends up
> > > inside
> > > BSS
> > > and spl_early_init() finds none, before the console is up.
> > >
> > > Reassemble at the offset U-Boot used, taken from its own image as
> > > SPL_WITH_DTB_BINARY, and fall back to the current behaviour when
> > > it
> > > did
> > > not pad.
> > >
> > > AI-Generated: Uses Claude Code
> > > Signed-off-by: Ricardo Salveti <[email protected]>
> > > ---
> > > meta/classes-recipe/uboot-sign.bbclass | 20 +++++++++++++++++++-
> > > 1 file changed, 19 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/meta/classes-recipe/uboot-sign.bbclass
> > > b/meta/classes-
> > > recipe/uboot-sign.bbclass
> > > index 236d19925e..4743b86dd4 100644
> > > --- a/meta/classes-recipe/uboot-sign.bbclass
> > > +++ b/meta/classes-recipe/uboot-sign.bbclass
> > > @@ -57,6 +57,8 @@ SPL_DTB_SYMLINK ?= "u-boot-spl-${MACHINE}.dtb"
> > > SPL_NODTB_IMAGE ?= "u-boot-spl-nodtb-${MACHINE}-${PV}-${PR}.bin"
> > > SPL_NODTB_BINARY ?= "u-boot-spl-nodtb.bin"
> > > SPL_NODTB_SYMLINK ?= "u-boot-spl-nodtb-${MACHINE}.bin"
> > > +# The SPL image as assembled by U-Boot, before signing.
> > > +SPL_WITH_DTB_BINARY ?= "u-boot-spl-dtb.bin"
> >
> > This looks like a MACHINE specific variable. Should it default to
> > empty
> > string and be assigned only in machine conf files which need it?
> > And
> > then...
>
> The file itself exists for every configuration with an SPL device
> tree, so while it depends on the configuration used by the machine,
> it
> is generic and widely used.
To be honest, I did not look into all these details. I'm trying to
understand if this new variable can have a negative impact on other
MACHINEs. Usually the review is easy if the code looks like:
NEW_VARIABLE ?= ""
...
if [ -n "${NEW_VARIABLE}" ] ; then
new opt-in feature...
fi
Sure, there's no logic or rule that says new features can always be
implemented that easily and well isolated. But we also need to prevent
from commits that potentially have a negative impact on existing
boards.
If I correctly understand your response, I do not understand that you
are 100% sure the this is 100% always right and there is no risk for
other MACHINEs. Or am I misunderstanding?
>
> From u-boot sources (depends on OF_REAL and OF_SEPARATE /
> OF_HOSTFILE,
> and this class already depends on OF_SEPARATE for signing support):
> https://github.com/u-boot/u-boot/blob/main/scripts/Makefile.xpl#L323
>
> > > # U-Boot fitImage description
> > > UBOOT_FIT_DESC ?= "U-Boot fitImage for
> > > ${DISTRO_NAME}/${PV}/${MACHINE}"
> > > @@ -249,7 +251,23 @@ deploy_dtb() {
> > >
> > > concat_spl_dtb() {
> > > if [ -e "${SPL_DIR}/${SPL_NODTB_BINARY}" ] && [ -e
> > > "${SPL_DIR}/${SPL_DTB_BINARY}" ] ; then
> > > - cat ${SPL_DIR}/${SPL_NODTB_BINARY}
> > > ${SPL_DIR}/${SPL_DTB_SIGNED} > "${SPL_BINARY}"
> > > + # U-Boot pads behind BSS before appending the
> > > device
> > > tree
> > > + # ($(SPL_BIN)-pad.bin in scripts/Makefile.xpl);
> > > keep
> > > that offset.
> > > + nodtb_size=$(stat -Lc %s
> > > ${SPL_DIR}/${SPL_NODTB_BINARY})
> > > + dtb_offset=${nodtb_size}
> > > +
> > > + if [ -e "${SPL_DIR}/${SPL_WITH_DTB_BINARY}" ]; then
> >
> > ... this -e could fail fatal if the variable is non-empty string
> > and
> > the file does not exist rather than fail silently?
> >
> > Further, would it be less risky if all the special handling of
> > SPL_WITH_DTB_BINARY would be in the if block?
>
> A board that does not define OF_SEPARATE would cause issues even
> before this function is executed, but we can separate into another if
> block if preferred, something like:
>
> + if [ -n "${SPL_WITH_DTB_BINARY}" ] ; then
> + if [ ! -e "${SPL_DIR}/${SPL_WITH_DTB_BINARY}"
> ] ; then
I think we should not write -e in bitbake code. There are many reasons
why a file can be there by accident (e.g. previous build with different
configuration) or not there by accident (e.g. a bug that the file is
just not built). In general it's more robust if the code looks like:
if [ -n "${NEW_VARIABLE}" ] ; then
test -e "${SPL_DIR}/${SPL_WITH_DTB_BINARY}" || bbfatal "file not
there".
...
fi
Regards,
Adrian
> + bbfatal "${SPL_WITH_DTB_BINARY} not
> found in ${SPL_DIR}. Set SPL_WITH_DTB_BINARY to the SPL image U-Boot
> assembled, or to an empty string if it builds none."
> + fi
> +
> + # U-Boot pads behind BSS before appending the
> device tree
> + # ($(SPL_BIN)-pad.bin in
> scripts/Makefile.xpl); keep that offset.
> + image_size=$(stat -Lc %s
> ${SPL_DIR}/${SPL_WITH_DTB_BINARY})
> + dtb_size=$(stat -Lc %s
> ${SPL_DIR}/${SPL_DTB_BINARY})
> + offset=$(expr ${image_size} - ${dtb_size})
> +
> + if [ ${offset} -gt ${nodtb_size} ] ; then
> + dtb_offset=${offset}
> + fi
> + fi
> +
> + if [ ${dtb_offset} -gt ${nodtb_size} ] ; then
> + cp ${SPL_DIR}/${SPL_NODTB_BINARY}
> "${SPL_BINARY}"
> + truncate -s ${dtb_offset} "${SPL_BINARY}"
> + cat ${SPL_DIR}/${SPL_DTB_SIGNED} >>
> "${SPL_BINARY}"
> + else
> + cat ${SPL_DIR}/${SPL_NODTB_BINARY}
> ${SPL_DIR}/${SPL_DTB_SIGNED} > "${SPL_BINARY}"
> + fi
>
> Let me know what you think and I can send a v2.
>
> Thanks,
>
> Ricardo
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#245492):
https://lists.openembedded.org/g/openembedded-core/message/245492
Mute This Topic: https://lists.openembedded.org/mt/121108517/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
