Hi Ross, Adrian
Sorry reply you late.
I successfully added users snippet ITS in the variable and my changes as
following.
1. remove hook function uboot_fitimage_user_image
2. replace "UBOOT_FIT_USER_IMAGE" with "UBOOT_FIT_USER_SETTINGS" variable
# User specific settings
UBOOT_FIT_USER_SETTINGS ?= ""
# Unit name containing a list of users additional binaries to be loaded.
# It is a comma-separated list of strings.
UBOOT_FIT_CONF_USER_LOADABLES ?= ''
Create a ITS file for the U-boot FIT, for use when
# we want to sign it so that the SPL can verify it
uboot_fitimage_assemble() {
---
if [ -n "${UBOOT_FIT_USER_SETTINGS}" ] ; then
echo -e "${UBOOT_FIT_USER_SETTINGS}" >> ${UBOOT_ITS}
fi
if [ -n "${UBOOT_FIT_CONF_USER_LOADABLES}" ] ; then
conf_loadables="${conf_loadables}${UBOOT_FIT_CONF_USER_LOADABLES}"
fi
---
3. Set UBOOT_FIT_CONF_USER_LOADABLES to load users image
UBOOT_FIT_CONF_USER_LOADABLES = ' \"sspfw\", \"tspfw\" '
4. Users add their own snippet ITS into UBOOT_FIT_USER_SETTINGS variable.
Ex:
UBOOT_FIT_SSP_ITS = '\
sspfw {\n\
description = \"SSP Firmware\";\n\
data = /incbin/(\"${UBOOT_FIT_SSP_IMAGE}\");\n\
type = \"firmware\";\n\
arch = \"${UBOOT_FIT_SSP_ARCH}\";\n\
os = \"${UBOOT_FIT_SSP_OS}\";\n\
load = <${UBOOT_FIT_SSP_LOADADDRESS}>;\n\
entry = <${UBOOT_FIT_SSP_ENTRYPOINT}>;\n\
compression = \"none\";\n\
};\n\
'
UBOOT_FIT_TSP_ITS = '\
tspfw {\n\
description = \"TSP Firmware\";\n\
data = /incbin/(\"${UBOOT_FIT_TSP_IMAGE}\");\n\
type = \"firmware\";\n\
arch = \"${UBOOT_FIT_TSP_ARCH}\";\n\
os = \"${UBOOT_FIT_TSP_OS}\";\n\
load = <${UBOOT_FIT_TSP_LOADADDRESS}>;\n\
entry = <${UBOOT_FIT_TSP_ENTRYPOINT}>;\n\
compression = \"none\";\n\
};\n\
'
UBOOT_FIT_USER_SETTINGS = " ${UBOOT_FIT_SSP_ITS} ${UBOOT_FIT_TSP_ITS} "
Then, I got the following generated ITS.
```
/dts-v1/;
/ {
description = "U-Boot fitImage for Phosphor OpenBMC (Phosphor OpenBMC
Project Reference Distro)/v2023.10+git/ast2700-default";
#address-cells = <1>;
images {
uboot {
description = "U-Boot image";
data = /incbin/("u-boot-nodtb.bin");
type = "standalone";
os = "u-boot";
arch = "";
compression = "none";
load = <0x80000000>;
entry = <0x80000000>;
};
fdt {
description = "U-Boot FDT";
data = /incbin/("u-boot.dtb");
type = "flat_dt";
arch = "";
compression = "none";
};
tee {
description = "Trusted Execution Environment";
data =
/incbin/("/home/jamin_lin/openbmc-ast2700/1206/build-ast2700/tmp/deploy/images/ast2700-default/optee/tee-raw.bin");
type = "tee";
arch = "";
os = "tee";
load = <0xb0080000>;
entry = <0xb0080000>;
compression = "none";
};
atf {
description = "ARM Trusted Firmware";
data =
/incbin/("/home/jamin_lin/openbmc-ast2700/1206/build-ast2700/tmp/deploy/images/ast2700-default/bl31.bin");
type = "firmware";
arch = "";
os = "arm-trusted-firmware";
load = <0xb0000000>;
entry = <0xb0000000>;
compression = "none";
};
sspfw {
description = "SSP Firmware";
data =
/incbin/("/home/jamin_lin/openbmc-ast2700/1206/build-ast2700/tmp/deploy/images/ast2700-default/ast2700-ssp.bin");
type = "firmware";
arch = "arm";
os = "zephyr";
load = <0xb2000000>;
entry = <0xb2000000>;
compression = "none";
};
tspfw {
description = "TSP Firmware";
data =
/incbin/("/home/jamin_lin/openbmc-ast2700/1206/build-ast2700/tmp/deploy/images/ast2700-default/ast2700-tsp.bin");
type = "firmware";
arch = "arm";
os = "zephyr";
load = <0xb4000000>;
entry = <0xb4000000>;
compression = "none";
};
};
configurations {
default = "conf";
conf {
description = "Boot with signed U-Boot FIT";
loadables = "atf", "tee", "uboot" ,"sspfw" ,"tspfw";
fdt = "fdt";
};
};
};
```
If you agree, this new design, I will resend v4 patch.
Do you have any concern or could you please give me any suggestion?
Thanks-Jamin
> Subject: Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS
> generation
>
> Hi Adrian, Ross
>
> Thanks for your input.
> Sorry, I am working on another task and will update you soon.
> The following are my briefly answer about uboot_fitimage_user_image
> function.
>
> > Subject: Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE
> > ITS generation
> >
> > Hi Jamin, hi Ross
> >
> > On Thu, 2024-11-28 at 13:51 +0000, Ross Burton via
> > lists.openembedded.org
> > wrote:
> > > Hi Jamin,
> > >
> > > Thanks for the patches.
> > >
> > > However, I’m getting more convinced that as our FIT generation is
> > > spread between uboot-sign and kernel-fitimage, maybe we should just
> > > create a new class that is dedicated to creating a FIT image from
> > > arbitrary inputs, so in this case you’d have dependencies on tf-
> > > a/uboot/kernel and write a dts that describes the layout. I’m
> > > unconvinced that the complexity of these classes is sustainable and
> > > a truly generic class might be a more maintainable alternative.
> >
> > I apologize for digressing a bit now. But the complexity which
> > requires a redesign of the ftiImage generation code only becomes clear
> > when you look at the kernel and u-boot fitImage handling together.
> >
> > I think that the generation of its files, as done by kernel-
> > fitimage.bbclass and uboot-sign.bbclass, is not generally wrong.
> > Writing an its file is not something that a user can do quickly. It
> > requires quite a lot of know-how, which is taken away from the user by
> > the existing code. The code is also relatively well tested, which I
> > can't imagine it can be achieved with handwritten its files.
> > Standardizing how its files should be written looks helpful to me in
> > general.
> >
> > Handling the task dependencies is not easy either. Many artifacts and
> > keys must be provided by different recipes before the assembly of
> > fitImages can take place. Simply leaving it up to the user certainly doesn't
> make it any easier.
> >
> > But I agree, it ended up at an unmaintainable state. The main issue is
> > that the generator code in the kernel-fitimage.bbclass currently gets
> > executed from the kernel build folder and from the u-boot build folder
> > and that the u-boot and the kernel recipes have many inter task
> > dependencies. It does not properly work with the sstate-cache. For
> > example building with empty temp means rebuilding from scratch at
> > least when also an initramfs is involved. I tried to solve this with
> > this series here
> https://patchwork.yoctoproject.org/project/oe-core/list/?series=27108.
> > It is also possible to end up with circular task dependencies by
> > setting the
> > UBOOT_* variables to SIGNED images, for example, and packing a
> > boot.txt file into the fitImage.
> >
> > The idea that seems most promising to me to solve these problems (
> > sstate, maintainability, complexity, task dependencies) is to
> > completely remove the fitimage code from the kernel and the u-boot
> > recipes and create a linux-fitimage and an uboot-fitimage recipe that
> > take the artifacts from the linux and the u-boot recipes and simply
> > put them into a fit container. The fitimage_assemble and
> uboot_fitimage_assemble functions would be reusable.
> > All the rest would probably be rewritten from scratch.
> > Some use cases in which, for example, the kernel Makefile (which can
> > run from the kernel build tree only not from a setscene folder
> > structure) generates the fitImage should simply be omitted in favor of
> > simplification.
> >
> > Having for example a linux-yocto recipe which does not support
> > building fitImages at all and a second recipe linux-yocto-fitimage
> > which depends on the linux-yocto recipe would allow several improvements:
> > - The kernel artifacts can be forwarded via sysroots from linux-yocto
> > to the linux-yocto-fitimage. Currently it goes via deploy.
> > - The kernel-yocto-fitImage class could also provide a kernel package.
> > Currently this kernel artifact is only available from the deploy folder.
> > - Standard sstate tasks (sysroot and deploy) can be used for building
> > the kernel and for building the fitImage. Doing all these steps in the
> > contecxt of one recipe leads to unusual sstated tasks which causes some
> issues.
> > - I hope the code would be much simpler. But I'm not set sure. I want
> > to try this but did not find the time so far.
> >
> > >
> > > What’s your opinion on this? The fact that you had to add custom
> > > hooks in 2/3 seems to indicate that a more inherently flexible class
> > > would be the right approach.
> >
> > I think the two new functions uboot_fitimage_atf and
> > uboot_fitimage_tee look reasonable and are in line with the existing code.
> >
> > But the uboot_fitimage_user_image hook looks a little unusual. What's
> > the use case for that? Would it be possible to pass the its snippet as
> > a variable? Or even support the use case with a standard snippet?
>
> The reason is ASPEED want to add our images into u-boot FIT image which are
> TSP and SSP.
> Both images are ASPEED only and they are not generic images such as TEE and
> ATF.
> Therefore, I added this hook function to make users add their ITS for specific
> need.
> Our ITS looks like as following for ASPEED latest SOCs(AST2700)
>
> Thanks -Jamin
>
> /dts-v1/;
>
> / {
> description = "U-Boot fitImage for Phosphor OpenBMC (Phosphor
> OpenBMC Project Reference Distro)/v2023.10+git/ast2700-default";
> #address-cells = <1>;
>
> images {
> uboot {
> description = "U-Boot image";
> data = /incbin/("u-boot-nodtb.bin");
> type = "standalone";
> os = "u-boot";
> arch = "";
> compression = "none";
> load = <0x80000000>;
> entry = <0x80000000>;
> };
> fdt {
> description = "U-Boot FDT";
> data = /incbin/("u-boot.dtb");
> type = "flat_dt";
> arch = "";
> compression = "none";
> };
> tee {
> description = "Trusted Execution Environment";
> data = /incbin/("tee-raw.bin");
> type = "tee";
> arch = "";
> os = "tee";
> load = <0xb0080000>;
> entry = <0xb0080000>;
> compression = "none";
> };
> atf {
> description = "ARM Trusted Firmware";
> data = /incbin/("bl31.bin");
> type = "firmware";
> arch = "";
> os = "arm-trusted-firmware";
> load = <0xb0000000>;
> entry = <0xb0000000>;
> compression = "none";
> };
> sspfw {
> description = "SSP Firmware";
> data = /incbin/("ast2700-ssp.bin");
> type = "firmware";
> arch = "arm";
> os = "zephyr";
> load = <0xb2000000>;
> entry = <0xb2000000>;
> compression = "none";
> };
> tspfw {
> description = "TSP Firmware";
> data = /incbin/("ast2700-tsp.bin");
> type = "firmware";
> arch = "arm";
> os = "zephyr";
> load = <0xb4000000>;
> entry = <0xb4000000>;
> compression = "none";
> };
> };
>
> configurations {
> default = "conf";
> conf {
> description = "Boot with signed U-Boot FIT";
> loadables = "atf", "tee", "uboot" ,"sspfw" ,"tspfw";
> fdt = "fdt";
> };
> };
> };
>
>
> >
> > All in all, I think these patches do not make it worse than it already
> > is. Solving the issues with the fitImage is a different topic.
> >
> > Regards,
> > Adrian
> >
> >
> >
> >
> > >
> > > Cheers,
> > > Ross
> > >
> > >
> > > > On 18 Nov 2024, at 06:32, Jamin Lin via lists.openembedded.org
> > > > <[email protected]> wrote:
> > > >
> > > > v0:
> > > > 1. add variable to set load address and entrypoint.
> > > > 2. Fix to install nonexistent dtb file.
> > > > 3. support to verify signed FIT
> > > > 4. support to load optee-os and TFA images
> > > > v1:
> > > > Fix patch for code review
> > > > v2:
> > > > created a series patch
> > > > v3:
> > > > add cover letter
> > > > v4:
> > > > Add oe-self testcases for reviewer suggestion
> > > > Add documentation for reviewer suggestion.
> > > > Link:
> > > > https://patchwork.yoctoproject.org/project/docs/patch/202411180621
> > > > 13 [email protected]/
> > > >
> > > > The following patches had been applied from v0 changes.
> > > > a. Fix to install nonexistent dtb file
> > > > b. support to verify signed FIT
> > > > c. add variable to set load address and entrypoint.
> > > >
> > > > Jamin Lin (3):
> > > > uboot-sign: support to create TEE and ATF image tree source
> > > > uboot-sign: support to create users specific image tree source
> > > > oe-selftest: fitimage: add testcases to test ATF and TEE
> > > >
> > > > meta/classes-recipe/uboot-sign.bbclass | 100 +++++++-
> > > > meta/lib/oeqa/selftest/cases/fitimage.py | 288
> > > > +++++++++++++++++++++++
> > > > 2 files changed, 387 insertions(+), 1 deletion(-)
> > > >
> > > > --
> > > > 2.25.1
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#208520):
https://lists.openembedded.org/g/openembedded-core/message/208520
Mute This Topic: https://lists.openembedded.org/mt/109640118/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
