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....@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 (#208401): https://lists.openembedded.org/g/openembedded-core/message/208401 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] -=-=-=-=-=-=-=-=-=-=-=-
