| Message ID | 20250111183134.22040-1-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled | expand |
Hi Marek The issue is known and already discussed here: https://lists.yoctoproject.org/g/yocto/message/64428. It probably hits many users and a fix would be more than welcome. A more general discussion about issues related to the kernel-fitimage.bbclass is here: https://bugzilla.yoctoproject.org/show_bug.cgi?id=12912. Am Sa., 11. Jan. 2025 um 19:32 Uhr schrieb Marek Vasut via lists.openembedded.org <marex=denx.de@lists.openembedded.org>: > In case both UBOOT_SIGN_ENABLE and UBOOT_ENV are enabled and > kernel-fitimage.bbclass is in use to generate signed kernel > fitImage, there is a circular dependency between uboot-sign > and kernel-fitimage bbclasses . The loop looks like this: > > kernel-fitimage.bbclass: > - do_populate_sysroot depends on do_assemble_fitimage > - do_assemble_fitimage depends on virtual/bootloader:do_populate_sysroot > - virtual/bootloader:do_populate_sysroot depends on > virtual/bootloader:do_install > => The virtual/bootloader:do_install installs and the > virtual/bootloader:do_populate_sysroot places into > sysroot an U-Boot environment script embedded into > kernel fitImage during do_assemble_fitimage run . > > uboot-sign.bbclass: > - DEPENDS on KERNEL_PN, which is really virtual/kernel. More accurately > - do_deploy depends on do_uboot_assemble_fitimage > - do_install depends on do_uboot_assemble_fitimage > - do_uboot_assemble_fitimage depends on > virtual/kernel:do_populate_sysroot > => do_install depends on virtual/kernel:do_populate_sysroot > > => virtual/bootloader:do_install depends on > virtual/kernel:do_populate_sysroot > virtual/kernel:do_populate_sysroot depends on > virtual/bootloader:do_install > > Attempt to resolve the loop by making u-boot do_compile() task already > deploy the > U-Boot environment script used by kernel-fitimage.bbclass > do_assemble_fitimage() > into STAGING_DIR_HOST , and then by making kernel-fitimage.bbclass depend > on > virtual/bootloader:do_compile instead of > virtual/bootloader:do_populate_sysroot . > > Make sure it is only uboot-sign.bbclass do_uboot_assemble_fitimage() task > that > depends on KERNEL_PN:do_populate_sysroot . > > Fixes: 5e12dc911d0c ("u-boot: Rework signing to remove interdependencies") > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Richard Purdie <richard.purdie@linuxfoundation.org> > Cc: Sean Anderson <sean.anderson@seco.com> > --- > meta/classes-recipe/kernel-fitimage.bbclass | 2 +- > meta/classes-recipe/uboot-sign.bbclass | 3 +-- > meta/recipes-bsp/u-boot/u-boot.inc | 6 ++++++ > 3 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/meta/classes-recipe/kernel-fitimage.bbclass > b/meta/classes-recipe/kernel-fitimage.bbclass > index 67c98adb23..81245446a2 100644 > --- a/meta/classes-recipe/kernel-fitimage.bbclass > +++ b/meta/classes-recipe/kernel-fitimage.bbclass > @@ -42,7 +42,7 @@ python __anonymous () { > > ubootenv = d.getVar('UBOOT_ENV') > if ubootenv: > - d.appendVarFlag('do_assemble_fitimage', 'depends', ' > virtual/bootloader:do_populate_sysroot') > + d.appendVarFlag('do_assemble_fitimage', 'depends', ' > virtual/bootloader:do_compile') > This is definitely one of the problematic lines of code. See also https://lists.yoctoproject.org/g/yocto/message/64428. However, I guess even if you write the env script from the do_compile task to the sysroot directory, the dependency here should be on do_populate_sysroot so that the env file is properly cached by the sstate cache. But unfortunately this is not the only problem the fitimage-kernel.bbclass has when it comes to building from sstate-cache with empty TMPDIR. This is also broken in other places, which means that this patch theoretically makes the situation with the sstate even worse. But since this doesn't work anyway and the patch fixes the circular dependencies issue, the majority of users would probably benefit from this patch. And to fix the problem properly, you would probably have to rewrite the fitimage generation, as discussed on https://bugzilla.yoctoproject.org/show_bug.cgi?id=12912. Regards, Adrian > > #check if there are any dtb providers > providerdtb = d.getVar("PREFERRED_PROVIDER_virtual/dtb") > diff --git a/meta/classes-recipe/uboot-sign.bbclass > b/meta/classes-recipe/uboot-sign.bbclass > index a17be745ce..e1a37b4343 100644 > --- a/meta/classes-recipe/uboot-sign.bbclass > +++ b/meta/classes-recipe/uboot-sign.bbclass > @@ -96,8 +96,6 @@ python() { > sign = d.getVar('UBOOT_SIGN_ENABLE') == '1' > if d.getVar('UBOOT_FITIMAGE_ENABLE') == '1' or sign: > d.appendVar('DEPENDS', " u-boot-tools-native dtc-native") > - if sign: > - d.appendVar('DEPENDS', " " + d.getVar('KERNEL_PN')) > } > > concat_dtb() { > @@ -350,6 +348,7 @@ uboot_assemble_fitimage_helper() { > fi > } > > +do_uboot_assemble_fitimage[depends] = "${@(d.getVar('KERNEL_PN') + > ':do_populate_sysroot') if d.getVar('UBOOT_SIGN_ENABLE') == 1 else ''}" > do_uboot_assemble_fitimage() { > if [ "${UBOOT_SIGN_ENABLE}" = "1" ] ; then > cp "${STAGING_DIR_HOST}/sysroot-only/fitImage" > "${B}/fitImage-linux" > diff --git a/meta/recipes-bsp/u-boot/u-boot.inc > b/meta/recipes-bsp/u-boot/u-boot.inc > index 3270c22e8d..61f98ec5b5 100644 > --- a/meta/recipes-bsp/u-boot/u-boot.inc > +++ b/meta/recipes-bsp/u-boot/u-boot.inc > @@ -72,6 +72,12 @@ do_compile () { > then > ${UBOOT_MKIMAGE} -C none -A ${UBOOT_ARCH} -T script -d > ${UNPACKDIR}/${UBOOT_ENV_SRC} ${B}/${UBOOT_ENV_BINARY} > fi > + > + if [ -n "${UBOOT_ENV}" ] > + then > + mkdir -p ${STAGING_DIR_HOST}/boot/ > + install -m 644 ${B}/${UBOOT_ENV_BINARY} > ${STAGING_DIR_HOST}/boot/${UBOOT_ENV_IMAGE} > + fi > } > > uboot_compile_config () { > -- > 2.45.2 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#209671): > https://lists.openembedded.org/g/openembedded-core/message/209671 > Mute This Topic: https://lists.openembedded.org/mt/110557812/4454582 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > adrian.freihofer@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
On 1/13/25 12:10 PM, Adrian Freihofer wrote: > Hi Marek Hi, > The issue is known and already discussed here: > https://lists.yoctoproject.org/g/yocto/message/64428. It probably hits many > users and a fix would be more than welcome. Indeed. > A more general discussion about issues related to the > kernel-fitimage.bbclass is here: > https://bugzilla.yoctoproject.org/show_bug.cgi?id=12912. [...] >> diff --git a/meta/classes-recipe/kernel-fitimage.bbclass >> b/meta/classes-recipe/kernel-fitimage.bbclass >> index 67c98adb23..81245446a2 100644 >> --- a/meta/classes-recipe/kernel-fitimage.bbclass >> +++ b/meta/classes-recipe/kernel-fitimage.bbclass >> @@ -42,7 +42,7 @@ python __anonymous () { >> >> ubootenv = d.getVar('UBOOT_ENV') >> if ubootenv: >> - d.appendVarFlag('do_assemble_fitimage', 'depends', ' >> virtual/bootloader:do_populate_sysroot') >> + d.appendVarFlag('do_assemble_fitimage', 'depends', ' >> virtual/bootloader:do_compile') >> > This is definitely one of the problematic lines of code. See also > https://lists.yoctoproject.org/g/yocto/message/64428. > > However, I guess even if you write the env script from the do_compile task > to the sysroot directory, the dependency here should be on > do_populate_sysroot so that the env file is properly cached by the sstate > cache. Right, that is one of the things I am not sure about in this patch -- how to stage the mkimage'd env blob into sstate cache early. The other alternative would be, if the kernel-fitimage-bbclass could somehow gain access to the UBOOT_ENV source file in the virtual/bootloader recipe (is that even possible?) , it would be able to run its own mkimage invocation to convert the UBOOT_ENV source file into the env blob, but that is not awesome either. And one more alternative would be to split the env blob generation into separate u-boot-env_1.0.bb recipe , that would seem like the best way to me, but that would break existing layers which bbappend u-boot_%.bb . > But unfortunately this is not the only problem the fitimage-kernel.bbclass > has when it comes to building from sstate-cache with empty TMPDIR. This is > also broken in other places, which means that this patch theoretically > makes the situation with the sstate even worse. But since this doesn't work > anyway and the patch fixes the circular dependencies issue, the majority of > users would probably benefit from this patch. And to fix the problem > properly, you would probably have to rewrite the fitimage generation, as > discussed on https://bugzilla.yoctoproject.org/show_bug.cgi?id=12912. I am in full agreement with this statement, I had that fitimage bbclass rewrite plan on my todo list for years, haven't gotten to it yet though, sorry. I also think the LTS releases will need some minimal fix.
diff --git a/meta/classes-recipe/kernel-fitimage.bbclass b/meta/classes-recipe/kernel-fitimage.bbclass index 67c98adb23..81245446a2 100644 --- a/meta/classes-recipe/kernel-fitimage.bbclass +++ b/meta/classes-recipe/kernel-fitimage.bbclass @@ -42,7 +42,7 @@ python __anonymous () { ubootenv = d.getVar('UBOOT_ENV') if ubootenv: - d.appendVarFlag('do_assemble_fitimage', 'depends', ' virtual/bootloader:do_populate_sysroot') + d.appendVarFlag('do_assemble_fitimage', 'depends', ' virtual/bootloader:do_compile') #check if there are any dtb providers providerdtb = d.getVar("PREFERRED_PROVIDER_virtual/dtb") diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass index a17be745ce..e1a37b4343 100644 --- a/meta/classes-recipe/uboot-sign.bbclass +++ b/meta/classes-recipe/uboot-sign.bbclass @@ -96,8 +96,6 @@ python() { sign = d.getVar('UBOOT_SIGN_ENABLE') == '1' if d.getVar('UBOOT_FITIMAGE_ENABLE') == '1' or sign: d.appendVar('DEPENDS', " u-boot-tools-native dtc-native") - if sign: - d.appendVar('DEPENDS', " " + d.getVar('KERNEL_PN')) } concat_dtb() { @@ -350,6 +348,7 @@ uboot_assemble_fitimage_helper() { fi } +do_uboot_assemble_fitimage[depends] = "${@(d.getVar('KERNEL_PN') + ':do_populate_sysroot') if d.getVar('UBOOT_SIGN_ENABLE') == 1 else ''}" do_uboot_assemble_fitimage() { if [ "${UBOOT_SIGN_ENABLE}" = "1" ] ; then cp "${STAGING_DIR_HOST}/sysroot-only/fitImage" "${B}/fitImage-linux" diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-boot/u-boot.inc index 3270c22e8d..61f98ec5b5 100644 --- a/meta/recipes-bsp/u-boot/u-boot.inc +++ b/meta/recipes-bsp/u-boot/u-boot.inc @@ -72,6 +72,12 @@ do_compile () { then ${UBOOT_MKIMAGE} -C none -A ${UBOOT_ARCH} -T script -d ${UNPACKDIR}/${UBOOT_ENV_SRC} ${B}/${UBOOT_ENV_BINARY} fi + + if [ -n "${UBOOT_ENV}" ] + then + mkdir -p ${STAGING_DIR_HOST}/boot/ + install -m 644 ${B}/${UBOOT_ENV_BINARY} ${STAGING_DIR_HOST}/boot/${UBOOT_ENV_IMAGE} + fi } uboot_compile_config () {
In case both UBOOT_SIGN_ENABLE and UBOOT_ENV are enabled and kernel-fitimage.bbclass is in use to generate signed kernel fitImage, there is a circular dependency between uboot-sign and kernel-fitimage bbclasses . The loop looks like this: kernel-fitimage.bbclass: - do_populate_sysroot depends on do_assemble_fitimage - do_assemble_fitimage depends on virtual/bootloader:do_populate_sysroot - virtual/bootloader:do_populate_sysroot depends on virtual/bootloader:do_install => The virtual/bootloader:do_install installs and the virtual/bootloader:do_populate_sysroot places into sysroot an U-Boot environment script embedded into kernel fitImage during do_assemble_fitimage run . uboot-sign.bbclass: - DEPENDS on KERNEL_PN, which is really virtual/kernel. More accurately - do_deploy depends on do_uboot_assemble_fitimage - do_install depends on do_uboot_assemble_fitimage - do_uboot_assemble_fitimage depends on virtual/kernel:do_populate_sysroot => do_install depends on virtual/kernel:do_populate_sysroot => virtual/bootloader:do_install depends on virtual/kernel:do_populate_sysroot virtual/kernel:do_populate_sysroot depends on virtual/bootloader:do_install Attempt to resolve the loop by making u-boot do_compile() task already deploy the U-Boot environment script used by kernel-fitimage.bbclass do_assemble_fitimage() into STAGING_DIR_HOST , and then by making kernel-fitimage.bbclass depend on virtual/bootloader:do_compile instead of virtual/bootloader:do_populate_sysroot . Make sure it is only uboot-sign.bbclass do_uboot_assemble_fitimage() task that depends on KERNEL_PN:do_populate_sysroot . Fixes: 5e12dc911d0c ("u-boot: Rework signing to remove interdependencies") Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Richard Purdie <richard.purdie@linuxfoundation.org> Cc: Sean Anderson <sean.anderson@seco.com> --- meta/classes-recipe/kernel-fitimage.bbclass | 2 +- meta/classes-recipe/uboot-sign.bbclass | 3 +-- meta/recipes-bsp/u-boot/u-boot.inc | 6 ++++++ 3 files changed, 8 insertions(+), 3 deletions(-)