Skip to content
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

tdx-signed.bbclass: fix issue when including IMX HAB config file #16

Merged
merged 1 commit into from Mar 28, 2024

Conversation

sergioprado
Copy link
Collaborator

'imx-generic-bsp' is not populated in 'OVERRIDES' when parsing this class, so the 'OVERRIDES' variable cannot be used in this context.

The reason for this is that NXP BSP adds several machine overrides to a custom variable called 'MACHINEOVERRIDES_EXTENDER', and this variable is parsed by a class in 'meta-freescale' called
'machine-overrides-extender.bbclass'.

This class registers a function called 'machine_overrides_extender_handler' that will add 'MACHINEOVERRIDES_EXTENDER' to 'MACHINEOVERRIDES', that goes later to 'OVERRIDES'.

But this function is registered to run only AFTER the parsing is done. So that means there are a few overrides from NXP that will only be populated into 'OVERRIDES' in the end of the parsing.

Fix this by using 'MACHINEOVERRIDES_EXTENDER' instead of 'OVERRIDES'.

Fixes #15

'imx-generic-bsp' is not populated in 'OVERRIDES' when parsing this
class, so the 'OVERRIDES' variable cannot be used in this context.

The reason for this is that NXP BSP adds several machine overrides to a
custom variable called 'MACHINEOVERRIDES_EXTENDER', and this variable is
parsed by a class in 'meta-freescale' called
'machine-overrides-extender.bbclass'.

This class registers a function called 'machine_overrides_extender_handler'
that will add 'MACHINEOVERRIDES_EXTENDER' to 'MACHINEOVERRIDES', that goes
later to 'OVERRIDES'.

But this function is registered to run only AFTER the parsing is done.
So that means there are a few overrides from NXP that will only be
populated into 'OVERRIDES' in the end of the parsing.

Fix this by using 'MACHINEOVERRIDES_EXTENDER' instead of 'OVERRIDES'.

Fixes toradex#15

Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
@rborn-tx
Copy link
Collaborator

rborn-tx commented Mar 27, 2024

@sergioprado Shouldn't we care about this other occurrence (marked with ^^^):

./meta-toradex-security/classes/tdx-signed.bbclass:8:require ${@ 'tdx-signed-imx-hab.inc' if 'imx-generic-bsp' in d.getVar('OVERRIDES').split(':') else ''}  (SOLVED)
./meta-toradex-security/recipes-bsp/u-boot/u-boot-fit-signature.inc:5:require ${@ 'u-boot-fit-signature-nxp.inc' if 'imx-generic-bsp' in d.getVar('OVERRIDES').split(':') else ''}
                                                                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  included by:
  ./meta-toradex-security/recipes-bsp/u-boot/u-boot-secure-boot.inc:4:require ${@oe.utils.conditional('UBOOT_SIGN_ENABLE', '1', 'u-boot-fit-signature.inc', '', d)}
    included by:
    ./meta-toradex-security/recipes-bsp/u-boot/u-boot_%.bbappend:1:require ${@ 'u-boot-secure-boot.inc' if 'tdx-signed' in d.getVar('OVERRIDES').split(':') else ''}
    ./meta-toradex-security/recipes-bsp/u-boot/u-boot-toradex-ti_%.bbappend:1:require ${@ 'u-boot-secure-boot.inc' if 'tdx-signed' in d.getVar('OVERRIDES').split(':') else ''}
    ./meta-toradex-security/recipes-bsp/u-boot/u-boot-toradex_%.bbappend:1:require ${@ 'u-boot-secure-boot.inc' if 'tdx-signed' in d.getVar('OVERRIDES').split(':') else ''}

@sergioprado
Copy link
Collaborator Author

@sergioprado Shouldn't we care about this other occurrence (marked with ^^^):

./meta-toradex-security/classes/tdx-signed.bbclass:8:require ${@ 'tdx-signed-imx-hab.inc' if 'imx-generic-bsp' in d.getVar('OVERRIDES').split(':') else ''}  (SOLVED)
./meta-toradex-security/recipes-bsp/u-boot/u-boot-fit-signature.inc:5:require ${@ 'u-boot-fit-signature-nxp.inc' if 'imx-generic-bsp' in d.getVar('OVERRIDES').split(':') else ''}
                                                                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  included by:
  ./meta-toradex-security/recipes-bsp/u-boot/u-boot-secure-boot.inc:4:require ${@oe.utils.conditional('UBOOT_SIGN_ENABLE', '1', 'u-boot-fit-signature.inc', '', d)}
    included by:
    ./meta-toradex-security/recipes-bsp/u-boot/u-boot_%.bbappend:1:require ${@ 'u-boot-secure-boot.inc' if 'tdx-signed' in d.getVar('OVERRIDES').split(':') else ''}
    ./meta-toradex-security/recipes-bsp/u-boot/u-boot-toradex-ti_%.bbappend:1:require ${@ 'u-boot-secure-boot.inc' if 'tdx-signed' in d.getVar('OVERRIDES').split(':') else ''}
    ./meta-toradex-security/recipes-bsp/u-boot/u-boot-toradex_%.bbappend:1:require ${@ 'u-boot-secure-boot.inc' if 'tdx-signed' in d.getVar('OVERRIDES').split(':') else ''}

@rborn-tx no need to change this one.

The mentioned NXP class and related functions are executed in the end of parsing the common metadata, but before parsing the recipe metadata. So when it comes to parsing the U-Boot recipe itself, the OVERRIDES variable has the correct value.

I confirmed that by adding the following line just before the require directive:

MYOVERRIDES := "111:${OVERRIDES}:222"

I can see the OVERRIDES is correct:

$ bitbake virtual/bootloader -e | grep ^MYOVERRIDES=
MYOVERRIDES="111:linux:aarch64:pn-defaultpkgname:aarch64:armv8a:use-nxp-bsp:imx-generic-bsp:imx-nxp-bsp:imxdrm:imxvpu:imxgpu:imxgpu2d:imxgpu3d:imxvulkan:mx8-generic-bsp:mx8-nxp-bsp:mx8m-generic-bsp:mx8m-nxp-bsp:mx8mp-generic-bsp:mx8mp-nxp-bsp:use-head-next:verdin-imx8mp:tdx-xwayland:tdx:tdx-signed:tdx-signed-dmverity:class-target:libc-glibc:forcevariable:222"

I also added a line in u-boot-fit-signature-nxp.inc to fail the processing and make sure the file is being included, and I could confirm that by trying to process the U-Boot recipe:

$ bitbake virtual/bootloader
[...]
ERROR: ParseError at /home/sprado/projects/toradex/bsp/build-verdin-imx8mp/../layers/meta-toradex-security/recipes-bsp/u-boot/u-boot-fit-signature-nxp.inc:1: unparsed line: 'fail here!'

Copy link
Collaborator

@rborn-tx rborn-tx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@jsrc27 jsrc27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing to add from my side.

@rborn-tx rborn-tx merged commit d4ed7e1 into toradex:kirkstone-6.x.y Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TDX_IMX_HAB_ENABLE not being set by tdx-signed
3 participants