-
Notifications
You must be signed in to change notification settings - Fork 229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for building capsules #1285
Conversation
#!/bin/bash | ||
|
||
printf "\x07\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00" > /tmp/var_tmp.bin | ||
dd if=/tmp/var_tmp.bin of=/sys/firmware/efi/efivars/OsIndications-8be4df61-93ca-11d2-aa0d-00e098032b8c bs=12;sync |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The destination path has to be different for the AGX Xavier, which doesn't support writing variables in the efivars sysfs.
It might be better to move this script to the setup-nv-boot-control
recipe and have it leverage the logic in the existing script there (maybe with a bit of refactoring) to determine how to modify the EFI variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See what you think of the refactoring. I initially wanted to make a .func
file for both compat spec logic and for setting the uefi var, but they were intertwined just enough that I left it together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first glance it looks good - just the sort of thing I had in mind.. We should probably use the full path to the .func
file in the source
line, though. I'll review in more detail when I get a chance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Full path has been taken care of. Not sure why, but the auto-builder / checklayer broke after I forced push. The commits all look correct so not sure what happened. I am still chasing down an issue during my testing. Invoking the script to set OsIndications
works the first time through, but not the second. I am tracking that down now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK... I should have looked at the existing logic a little closer (and the comment). I'll have to rework that logic as it looks like the design intent was to only write TegraPlatformSpec
and TegraPlatformCompatSpec
the first time - presumably because those should be static values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why, but the auto-builder / checklayer broke after I forced push. The commits all look correct so not sure what happened.
Force-pushes (and some other unusual events) can confuse the autobuilder's PR handler. Fixing that is on my to-do list, but in the meantime, I re-queued the job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic should be fixed now. Tested A->B and B->A on Orin AGX Devkit. Will test Xavier AGX Devkit next.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested A->B and B->A on Xavier AGX Devkit as well and everything looks OK. I think this is gtg pending any additional feedback.
Thanks @Lexmark-chad, this is a good start.
Probably new recipes/bbclass/whatever for now. At some point we might be able to combine the BUP and capsule generation logic, together, eliminating the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The basic structure LGTM. See in-line for some minor changes needed.
@madisongh , thanks for all the feedback. I am close on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@dwalkes could you take a look at this also? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @Lexmark-chad for taking this on. I tried today with jetson-xavier-nx-devkit-emmc
, adding to the build with:
IMAGE_INSTALL:append = " tegra-uefi-capsules"
Then after booting up with the resulting build, copied the tegra-bl.cap file to the expected location for UEFI:
cp /opt/nvidia/UpdateCapsule/tegra-bl.cap /opt/nvidia/esp/EFI/UpdateCapsule/TEGRA_BL.Cap
Then ran:
oe4t-set-uefi-OSIndications
Verified the value was set:
root@jetson-xavier-nx-devkit-emmc:~# efivar -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-OsIndications
GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c
Name: "OsIndications"
Attributes:
Non-Volatile
Boot Service Access
Runtime Service Access
Value:
00000000 04 00 00 00 00 00 00 00 |........ |
root@jetson-xavier-nx-devkit-emmc:~# efivar -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-OsIndications
GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c
Name: "OsIndications"
Attributes:
Non-Volatile
Boot Service Access
Runtime Service Access
Value:
00000000 04 00 00 00 00 00 00 00 |........ |
Then rebooted
After reboot I got
root@jetson-xavier-nx-devkit-emmc:~# nvbootctrl dump-slots-info
Current version: 35.3.1
Capsule update status: 0
Current bootloader slot: A
Active bootloader slot: A
num_slots: 2
slot: 0, status: normal
slot: 1, status: normal
I didn't have a serial port connected yet, this would be the next step to debug this. Wanted to make sure I'm not missing something. @Lexmark-chad have you tried on jetson-xavier-nx-devkit-emmc
yet or only on AGX Xavier?
I've tried it on both Orin & Xavier AGX. I'll take a peek at the Xavier NX Devkit. |
@dwalkes, I got the same result. I'll have to get someone here to rig me up a serial debug cable so I can see what is going on during boot. |
This is exactly what happened before 64a3404 but I see that is in the kirkstone branch. I actually only tested it on master with https://matrix.to/#/!YBfWVpJwNVtkmqVCPS:gitter.im/$S8dzzJNfmhMzAt5cp4ikgNjJtez98R68O_39q3oAG0M?via=gitter.im&via=matrix.org&via=3dvisionlabs.com I believe. I will try to re-test the NVIDIA built capsule update on kirkstone tonight. |
I've verified I can successfully complete an update with this TEGRA_BL.Cap generated with the Jetpack 5.1.1 release and instructions here and the rest of the image matching the kirkstone build on this branch. So it appears to be some difference in capsule content which causes the issue although I'm not sure what that would be. |
When I used NVIDIA's tooling with the |
I finally got a serial cable hooked up. There wasn't anything that stood out as a hint of what is going on so I instrumented the tooling to output the exact command used to generate the bup payloads. The main difference I saw was that L4T tooling uses |
I suggest |
Just changing
|
I think this means the check here is failing, and turning on that info print might be helpful in figuring out why. I suspect it's some issue with tnspec match. Here's the working TegraBL.cap on the left linked above (from 5.1.1) and the one from the tegra build on the right. |
I'm getting closer. The tnspecs are different between meta-tegra builds and l4t tooling which causes the code in FwPackageGetImageIndex to flow through differently. I've added debug all throughout that function and it is clearly following a different code path. The tnspecs for
while l4t's is:
It's the
Looking at that code now to understand design intent. |
While it was true that the |
Hmm. The TEGRA_BUPGEN_SPECS setting for jetson-xavier-nx-devkit-emmc is incorrect. I must have missed that when converting us over to using the compatibility specs. We should be building just the -100 and -301 entries, just like L4T does.
Are we setting up the compat spec wrong for the Xavier NXes, too? |
That looks correct: |
I mean in this function: meta-tegra/recipes-bsp/tools/setup-nv-boot-control/setup-nv-boot-control.sh.in Lines 61 to 69 in 3c429b6
|
I modified
|
False alarm. Not everything rebuilt after I changed the machine conf to make |
Thanks @Lexmark-chad , kicked off a build, will retry in the morning. |
Confirmed working on |
Issue logged. |
@Lexmark-chad could you make sure you've added a |
Sorry, forgot about that... Will take care of that shortly. |
into edk2-firmware-tegra-35.3.1.inc for sharing with other recipes who need just that info. Signed-off-by: Chad McQuillen <[email protected]>
publish the python tools from edk2 source which is required for generating uefi capsules. Signed-off-by: Chad McQuillen <[email protected]>
Signed-off-by: Chad McQuillen <[email protected]>
creating uefi capsules from the bootloader and kernel bup payloads. Signed-off-by: Chad McQuillen <[email protected]>
from setup-nv-boot-control.sh into uefi_common.func.in for other scripts that will need to set uefi variables. - Changed interface to set_ufi_var to require guid and added an optional parameter to control 'write once' behavior for static variables. Signed-off-by: Chad McQuillen <[email protected]>
Signed-off-by: Chad McQuillen <[email protected]>
to match L4T's specs. Signed-off-by: Chad McQuillen <[email protected]>
Consider this just a starting point to spur some discussion on how best to implement.
Should the capsules be included in the rootfs by default or simply be deployed to
${DEPLOYDIR}
? I initially went ahead and included them in the rootfs. That follows the pattern of howswupdate
works (which is what we use) integra-test-distro
wherebytegra-bootloader-update
andbl_update_payload
are packaged into the rootfs and then swupdate mounts the just programmed inactive partition to drive the bootloader upddate. Having said that, I believe when we implementeddm-verity
to create a full chain of trust from the bootloaders to the rootfs it created a cycle of dependencies in the build. To break the cycle we pulled the bootloader payload out of the rootfs and packaged it directly into the .swu. For uefi the capsules eventually have to end up in/opt/nvidia/esp/EFI/UpdateCapsule/
. Maybe that means that the capsules should simply be deployed to the${DEPLOYDIR}
and it is up to the update mechanism to pull them out of the update specific packaging and copy them to the appropriate directory.Is a new recipe really required? I created a separate recipe instead of piling on top of
tegra-bup-payload
. By doing so I needed to move some logic to a .bbclass to avoid having to repeat it in the new recipe and creating a new point of maintenance.Requesting a capsule update is obscure so I created and a helper script to set
OSIndications
.I like choosing good names for things so if there are any suggestions to improved naming on anything I am open to suggestions.