[RFC] Refactor UEFI capsule generation #1527
Conversation
Fix indentation in task definitions (use spaces instead of tabs). Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
Rename variable holding Capsule install dir in the final rootfs, so it's more generic TEGRA_UEFI_CAPSULE_INSTALL_DIR and set a weak default value, so it can be adjusted if needed. Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
Drop bup-payload image type from TEGRA_INITRAMFS_FSTYPES, as it's planned to generate BUP image inside UEFI capsule generation recipes. Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
Drop bup-payload image type CONVERSION_CMD, as it's planned to generate BUP image inside UEFI capsule generation recipes, as suggested in [1]. [1] OE4T#1492 Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
Make capsule generation function BUP-file agnostic. Both BUP file path
and capsule file name are supplied via params.
Also rename s/sign/generate/g, as this name fits better ("generation" of
capsule).
Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
Drop BUP-specific recipes and move all logic to UEFI capsule generation recipe, as suggested in [1] [1] OE4T#1492 Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
|
Thanks @igoropaniuk for the PR. I will check the changes and get back to you. |
|
@ichergui not really, I've verified only that cap files (with the same size as without these changes) are generated and deployed. I can test update mechanism if needed, but I haven't touched anything in the way how that capsule updates are processed in runtime |
|
@igoropaniuk Please do |
There was a problem hiding this comment.
Looks like a good start on this, thanks!
A couple of additional cleanups: the tegra-bup bbclass can be removed with these changes, and the bup_dependency() function reference could just be replaced with a dependency on the do_image_complete task for the initrd if the build is using a separate initrd image.
Also, the oe_make_bup_payload() function could be moved out of image_types_tegra.bbclass and directly into the do_compile function of the capsules recipe, as this would be the only user of it.
|
|
||
| # Generate UEFI capsules | ||
| generate_uefi_capsule ${WORKDIR}/bup-payload/bl_only_payload tegra-bl.cap | ||
| generate_uefi_capsule ${WORKDIR}/bup-payload/kernel_only_payload tegra-kernel.cap |
There was a problem hiding this comment.
This should be conditional on the existence of the kernel-only payload. It's technically possible to remove the kernel and DTB partitions from a storage layout, in which case no such payload would get generated.
There was a problem hiding this comment.
My understanding is that the kernel-only payload as a capsule doesn't work to start with. If this is correct, it might make sense to omit it altogether.
That said, in the old days of using bups, it was often convenient to install the combined payload that had everything, and for reasons I've never investigated we have lost that with capsules. In light of removing the kernel payload, it could be interesting to see a combined payload return if possible.
| do_compile[depends] += "${@bup_dependency(d)} ${TEGRA_UEFI_CAPSULE_SIGNING_EXTRA_DEPS}" | ||
| addtask compile before do_build |
There was a problem hiding this comment.
Is this needed? do_compile should already be in the standard list of tasks for every recipe.
|
|
||
| GUID:tegra194 ?= "be3f5d68-7654-4ed2-838c-2a2faf901a78" | ||
| GUID:tegra234 ?= "bf0d4599-20d4-414e-b2c5-3595b1cda402" | ||
|
|
||
| do_compile() { | ||
| sign_uefi_capsules | ||
|
|
||
| if [ ! -z "${INITRAMFS_IMAGE}" ]; then |
There was a problem hiding this comment.
Everything below here should be run regardless of whether INITRAMFS_IMAGE is defined or not. Instead, the parameter passed to oe_make_bup_payload will differ, I think, depending on whether we have a bare kernel/kernel with bundled initramfs or a kernel with separate initrd. Either case would still be a cboot file, but the base file name would be different.
There was a problem hiding this comment.
Yes, already working on fix, thanks
|
@Lexmark-chad It would be good for you to review this as well, if you can |
| @@ -16,9 +16,11 @@ UEFI_CAPSULE_TRUSTED_PUBLIC_CERT ?= "${PYTHON_BASETOOLS}/Pkcs7Sign/TestRoot.pub. | |||
|
|
|||
| # Override this function to have a secure signing server | |||
| # perform the capsule signing. | |||
| sign_uefi_capsules() { | |||
| generate_uefi_capsule() { | |||
There was a problem hiding this comment.
Structurally I like what has been done, but it is going to introduce breaking changes for anyone who has overridden sign_uefi_capsules. The effect of renaming the class function is to not invoke someone's overridden signed implementation leaving capsules signed with default keys. Capsule updates will fail.
There was a problem hiding this comment.
Well, it won't work even if I keep the old name.
Current implementation relies on BUP images deployed to DEPLOY_DIR, and in the PR I've dropped that step. For backward compatibility we probably will need to keep BUP images in DEPLOY_DIR
There was a problem hiding this comment.
There are several things that make this a breaking change: the class function name change, class function is parameterized, and the location of BUP images. All good changes, but breaking changes none-the-less. Chalk that up to maybe not the best first pass implementation. Just deploying the BUP images to DEPLOY_DIR won't solve anything because the overridden class function won't get invoked. It could be decided that maybe not many have even adopted an external signing server yet for generating & signing the capsules (it just went last year) in which case the PR could be merged. Not sure the best way to try to communicate the change other than announcing on the gitter channel so people don't get caught by surprise.
|
@igoropaniuk Any update about this PR ? |
|
@ichergui I'm currently traveling, will get back to it when I'm back (in a week) |
Refactor UEFI capsule generation. Drop BUP-specific recipes and move all logic to UEFI capsule
generation recipe, as suggested in [1]
[1] #1492