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....@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_...@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 (#208403): https://lists.openembedded.org/g/openembedded-core/message/208403 Mute This Topic: https://lists.openembedded.org/mt/109640118/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
