Re: [OE-core][PATCH] Revert "uboot-sign: fix U-Boot binary with public key"

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2024-12-09 at 14:40 -0600, Ryan Eatmon wrote:
> 
> 
> On 12/8/2024 4:22 PM, Richard Purdie wrote:
> > On Fri, 2024-12-06 at 15:09 -0600, Ryan Eatmon via lists.openembedded.org wrote:
> > > This reverts commit 0d14e99aa18ee38293df63d585fafc270a4538be.
> > > 
> > > The patch removed logic required for correct handling of
> > > UBOOT_SUFFIX=img or UBOOT_SUFFIX=rom.  We need to find a better way to
> > > handle the fix for [YOCTO #15649].
> > > 
> > > Signed-off-by:  Ryan Eatmon <reatmon@ti.com>
> > > ---
> > >   meta/classes-recipe/uboot-sign.bbclass | 8 +++++++-
> > >   1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
> > > index 7ee73b872a..a17be745ce 100644
> > > --- a/meta/classes-recipe/uboot-sign.bbclass
> > > +++ b/meta/classes-recipe/uboot-sign.bbclass
> > > @@ -122,7 +122,13 @@ concat_dtb() {
> > >   	# If we're not using a signed u-boot fit, concatenate SPL w/o DTB & U-Boot DTB
> > >   	# with public key (otherwise U-Boot will be packaged by uboot_fitimage_assemble)
> > >   	if [ "${SPL_SIGN_ENABLE}" != "1" ] ; then
> > > -		if [ -e "${UBOOT_NODTB_BINARY}" -a -e "${UBOOT_DTB_BINARY}" ]; then
> > > +		if [ "x${UBOOT_SUFFIX}" = "ximg" -o "x${UBOOT_SUFFIX}" = "xrom" ] && \
> > > +			[ -e "${UBOOT_DTB_BINARY}" ]; then
> > > +			oe_runmake EXT_DTB="${UBOOT_DTB_SIGNED}" ${UBOOT_MAKE_TARGET}
> > > +			if [ -n "${binary}" ]; then
> > > +				cp ${binary} ${UBOOT_BINARYNAME}-${type}.${UBOOT_SUFFIX}
> > > +			fi
> > > +		elif [ -e "${UBOOT_NODTB_BINARY}" -a -e "${UBOOT_DTB_BINARY}" ]; then
> > >   			if [ -n "${binary}" ]; then
> > >   				cat ${UBOOT_NODTB_BINARY} ${UBOOT_DTB_SIGNED} | tee ${binary} > \
> > >   					${UBOOT_BINARYNAME}-${type}.${UBOOT_SUFFIX}
> > > 
> > 
> > I'm in two minds about whether to take this or not. The code is clearly
> > needed for some platforms however the selftests don't cover it and I
> > doubt it is documented either :/
> > 
> > If I take this, can we add in some better testing please?
> 
> The initial commit that starts the img ball rolling came from 2016:
> 
> https://git.openembedded.org/openembedded-core/commit/meta/classes/uboot-sign.bbclass?id=4afee787e455ce1d4c002cd5c003182f1fc50028
> 
> The logic has morphed over time, but there is where it started.  So it's 
> been in there for a number of years.
> 
> I'm willing to take a stab at the selftest.  Is there any documentation 
> to help with that broadly means/entails?  I'll also talk to Denys in our 
> call tomorrow if he knows and can point me in a good direction for this.

You can run a subset of selftest with "oe-selftest -r uboot" wich would
run the test cases in meta/lib/oeqa/selftest/cases/uboot.py. You can
narrow it to a specific class of tests within that file or a specific
test too, e.g.: "oe-selftest -r uboot.UBootTest.test_boot_uboot".

The aim is to have test cases for key workflows we need to ensure work.

There is some information in the manual:
https://docs.yoctoproject.org/test-manual/index.html
but it probably doesn't go into the level of detail you're looking for
about writing individual tests. That is something we've wanted to aim
to add but we're probably not there yet.

Cheers,

Richard