| Message ID | 20241118063233.698679-1-jamin_lin@aspeedtech.com |
|---|---|
| Headers | show |
| Series | uboot-sign: support ATF and TFE ITS generation | expand |
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. 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. Cheers, Ross > On 18 Nov 2024, at 06:32, Jamin Lin via lists.openembedded.org <jamin_lin=aspeedtech.com@lists.openembedded.org> 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/20241118062113.269253-1-jamin_lin@aspeedtech.com/ > > 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 (#207203): https://lists.openembedded.org/g/openembedded-core/message/207203 > Mute This Topic: https://lists.openembedded.org/mt/109640118/6875888 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ross.burton@arm.com] > -=-=-=-=-=-=-=-=-=-=-=- >
Hi Ross > Subject: Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS > generation > > 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. > > 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. > Thanks for your review and suggestion. I agree to create a new bbclass to generate the ITS with users arbitrary inputs. Could you please give me the following comments? 1. What is the name of this bbclass? 2. This new bbclass should be placed in which directory and meta layer? 3. My briefly proposal of this new bbclass as following, could you please give me any suggestion? create_its() { cat << EOF >> ${UBOOT_ITS} ${USER_FIRMWARE} { description = ${USER_DESCRIPTION}; data = /incbin/("${UBOOT_FIT_SSP_IMAGE}"); type = ${USER_FIRMWARE}; arch = "${USER_ARCH}"; os = "${USER_OS}"; load = <${USER_LOADADDRESS}>; entry = <${USER_ENTRYPOINT}>; compression = ${USER_COMPRESS}; EOF if [ "${SPL_SIGN_ENABLE}" = "1" ] ; then cat << EOF >> ${UBOOT_ITS} signature { algo = "${UBOOT_FIT_HASH_ALG},${UBOOT_FIT_SIGN_ALG}"; key-name-hint = "${SPL_SIGN_KEYNAME}"; }; EOF fi cat << EOF >> ${UBOOT_ITS} }; EOF } Then, users set “USER_XXX” variables to create their own ITS file for u-boot/kernel FIT image generation. Or can you give me any suggestion about your goal and ITS improvement? Thanks-Jamin > Cheers, > Ross > > > > On 18 Nov 2024, at 06:32, Jamin Lin via lists.openembedded.org > <jamin_lin=aspeedtech.com@lists.openembedded.org> 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/20241118062113.2 > > 69253-1-jamin_lin@aspeedtech.com/ > > > > 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 > > > > > > > >
Hi Jamin, On 2 Dec 2024, at 05:39, Jamin Lin <jamin_lin@aspeedtech.com> wrote: > create_its() { > cat << EOF >> ${UBOOT_ITS} > ${USER_FIRMWARE} { > description = ${USER_DESCRIPTION}; > data = /incbin/("${UBOOT_FIT_SSP_IMAGE}"); > type = ${USER_FIRMWARE}; > arch = "${USER_ARCH}"; > os = "${USER_OS}"; > load = <${USER_LOADADDRESS}>; > entry = <${USER_ENTRYPOINT}>; > compression = ${USER_COMPRESS}; > EOF > if [ "${SPL_SIGN_ENABLE}" = "1" ] ; then > cat << EOF >> ${UBOOT_ITS} > signature { > algo = "${UBOOT_FIT_HASH_ALG},${UBOOT_FIT_SIGN_ALG}"; > key-name-hint = "${SPL_SIGN_KEYNAME}"; > }; > EOF > fi > cat << EOF >> ${UBOOT_ITS} > }; > EOF > } > > Then, users set “USER_XXX” variables to create their own ITS file for u-boot/kernel FIT image generation. > Or can you give me any suggestion about your goal and ITS improvement? I was actually wondering if it would be sensible to have the user of the class write the .its from scratch. It could support variable expansion to allow the use of variables that are already defined in the machine configuration, but I could see someone wanting more flexibility in your design. Important note: I’ve not used fitimages so I might not be right, this is just an observation from seeing increasing amounts of customisation needed on the existing fit generation. Ross
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? 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 > > <jamin_lin=aspeedtech.com@lists.openembedded.org> 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/20241118062113.269253-1-jamin_lin@aspeedtech.com/ > > > > 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 (#207991): > https://lists.openembedded.org/g/openembedded-core/message/207991 > Mute This Topic: https://lists.openembedded.org/mt/109640118/4454582 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: > https://lists.openembedded.org/g/openembedded-core/unsub [ > adrian.freihofer@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
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 > > > <jamin_lin=aspeedtech.com@lists.openembedded.org> 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/20241118062113 > > > .269253-1-jamin_lin@aspeedtech.com/ > > > > > > 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 (#207991): > > https://lists.openembedded.org/g/openembedded-core/message/207991 > > Mute This Topic: https://lists.openembedded.org/mt/109640118/4454582 > > Group Owner: openembedded-core+owner@lists.openembedded.org > > Unsubscribe: > > https://lists.openembedded.org/g/openembedded-core/unsub [ > > adrian.freihofer@gmail.com] > > -=-=-=-=-=-=-=-=-=-=-=- > >
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
> > > > <jamin_lin=aspeedtech.com@lists.openembedded.org> 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 .269253-1-jamin_lin@aspeedtech.com/
> > > >
> > > > 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
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
Hi Adrian, Ross I resend v5 patch series here, https://patchwork.yoctoproject.org/project/oe-core/cover/20241211023515.2108415-1-jamin_lin@aspeedtech.com/ Thanks-Jamin > Subject: RE: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS > generation >