Re: [PATCH v4 02/11] tools: binman: extend ti_secure_rom to handle op-tee

Hi Bryan,

On 2026-08-04T21:55:02, Bryan Brattlof <[email protected]> wrote:
> tools: binman: extend ti_secure_rom to handle op-tee

>
> Add an optional 'tee' entry to the x509 certificate template to include
> optee during the second (tispl) boot phase for the AM62L.

The commit message is terse for a change that adds a new component
slot to the ROM cert format. Please expand it to explain why the
existing dm-data slot cannot be reused, which SoCs need this, and the
meaning of the compType/bootCore values chosen.

>
> Add an optional 'tee' entry to the x509 certificate template to include
> optee during the second (tispl) boot phase for the AM62L.
>
> Tested-by: Anshul Dalal <[email protected]>
> Signed-off-by: Bryan Brattlof <[email protected]>
>
> tools/binman/btool/openssl.py                      | 11 ++++---
>  tools/binman/etype/ti_secure_rom.py                | 34 +++++++++++++++++++++-
>  tools/binman/etype/x509_cert.py                    |  4 +++
>  .../binman/test/vendor/ti_secure_rom_combined.dts  |  1 +
>  4 files changed, 45 insertions(+), 5 deletions(-)

> diff --git a/tools/binman/etype/ti_secure_rom.py b/tools/binman/etype/ti_secure_rom.py
> @@ -80,6 +82,11 @@ class Entry_ti_secure_rom(Entry_x509_cert):
>              self.dm_data = fdt_util.GetBool(self._node, 'dm-data', False)
>              if self.dm_data:
>                  self.load_addr_dm_data = fdt_util.GetInt(self._node, 'load-dm-data', 0x00000000)
> +
> +        self.tee = fdt_util.GetBool(self._node, 'content-tee', False)
> +        if self.tee:
> +            self.load_addr_tee = fdt_util.GetInt(self._node, 'load-tee', 0x00000000)
> +

This block sits outside the 'if self.combined:' guard even though tee
is only consumed by CombinedGetCertificate() - please move it inside
for consistency with sysfw_inner_cert and dm_data.

Second, the enable-trigger is inconsistent: sysfw-inner-cert and
dm-data each have a dedicated boolean property in the DT (see the test
dts), whereas here you overload the presence of 'content-tee' via
GetBool(). Please add a matching 'tee;' boolean so the DT schema
follows the same pattern, and drop the 'content-tee' bool test.

> diff --git a/tools/binman/etype/ti_secure_rom.py b/tools/binman/etype/ti_secure_rom.py
> @@ -219,7 +226,32 @@ compSize = INTEGER:{imagesize_dm_data}
> +            self.tee_ext_boot_block = f"""[tee]
> +compType = INTEGER:17
> +bootCore = INTEGER:16
> +compOpts = INTEGER:0
> +destAddr = FORMAT:HEX,OCT:{self.load_addr_tee:08x}
> +compSize = INTEGER:{imagesize_tee}
> +shaType  = OID:{self.sha_type}
> +shaValue = FORMAT:HEX,OCT:{hashval_tee}"""

Just to check - compType 17 and bootCore 16 are the same values used
for dm-data - is that really correct for OP-TEE, which typically runs
on the secure A53 rather than a DM core? A brief note in the commit
message (or a pointer to the TI ROM spec) would help future readers.

> diff --git a/tools/binman/test/vendor/ti_secure_rom_combined.dts b/tools/binman/test/vendor/ti_secure_rom_combined.dts
> @@ -14,6 +14,7 @@
>                       content-sysfw-data = <&unsecure_binary>;
>                       content-sysfw-inner-cert = <&unsecure_binary>;
>                       content-dm-data = <&unsecure_binary>;
> +                     content-tee = <&unsecure_binary>;
>                       combined;
>                       sysfw-inner-cert;
>                       dm-data;

If you add a dedicated 'tee;' boolean per the comment above, please
also add it here so the new branch is exercised by
testPackTiSecureROMCombined. Worth adding an explicit 'load-tee'
property too, to cover the non-default load-address path.

Regards,
Simon