Hi Marek,

On 11/24/25 8:40 PM, Marek Vasut wrote:
Introduce two new parameters to be used with mkimage -f auto to bundle
TEE image into fitImage, using auto-generated fitImage. Add -z to specify
TEE file name and -Z to specify TEE load and entry point address. This is
meant to be used with systems which boot all of TEE, Linux and its DT from
a single fitImage, all booted by U-Boot.

Example invocation:
"
$ mkimage -E -A arm -C none -e 0xc0008000 -a 0xc0008000 -f auto \
           -d arch/arm/boot/zImage \
           -b arch/arm/boot/dts/st/stm32mp135f-dhcor-dhsbc.dtb \
           -z ../optee_os/out/arm-plat-stm32mp1/core/tee-raw.bin \
          -Z 0xde000000 \
           /path/to/output/fitImage
"

Documentation update and test are also included, the test validates
both positive and negative test cases, where fitImage does not include
TEE and does include TEE blobs.

Signed-off-by: Marek Vasut <[email protected]>
---
Cc: "Carlos López" <[email protected]>
Cc: Aristo Chen <[email protected]>
Cc: Ilias Apalodimas <[email protected]>
Cc: Julien Masson <[email protected]>
Cc: Mattijs Korpershoek <[email protected]>
Cc: Mayuresh Chitale <[email protected]>
Cc: Paul HENRYS <[email protected]>
Cc: Quentin Schulz <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Simon Glass <[email protected]>
Cc: Tom Rini <[email protected]>
Cc: Wolfgang Wallner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
V2: - Update manpage to list --tee-* options instead of --optee-*
     - Update comments in pytest test to reflect the TEE support
---
  doc/mkimage.1                         | 12 +++++
  include/image.h                       |  1 +
  test/py/tests/test_fit_auto_signed.py | 65 ++++++++++++++++++++++++---
  tools/fit_image.c                     | 55 ++++++++++++++++++++++-
  tools/imagetool.h                     |  2 +
  tools/mkimage.c                       | 17 ++++++-
  6 files changed, 142 insertions(+), 10 deletions(-)

diff --git a/doc/mkimage.1 b/doc/mkimage.1
index c705218d345..29df21440e7 100644
--- a/doc/mkimage.1
+++ b/doc/mkimage.1
@@ -251,6 +251,18 @@ Append TFA BL31 file to the image.
  .B \-\-tfa-bl31-addr
  Set TFA BL31 file load and entry point address.
  .
+.TP
+.B \-z
+.TQ
+.B \-\-tee-file
+Append TEE file to the image.

Please specify which format is supported here if only select ones are.

+.
+.TP
+.B \-Z
+.TQ
+.B \-\-tee-addr
+Set TEE file load and entry point address, in hexadecimal.

s/address/addresses/ ?

[...]
@@ -239,3 +251,42 @@ def test_fit_auto_signed(ubman):
      generate_and_check_fit_image(' -fauto-conf' + b_args + s_args + " " + 
fit_file,
                                   scfgs=True, bl31present=True,
                                   key_name=key_name, sign_algo=sign_algo)
+
+    # Run the same tests as 1/2/3 above, but this time with TEE
+    # options -z tee.bin -Z 0x56780000 to cover both mkimage with
+    # and without TEE use cases.
+    b_args = " -d" + kernel_file + " -b" + dt1_file + " -b" + dt2_file + " -z" + 
tee_file + " -Z 0x56780000"
+

General remark but using f-string may improve readability:

b_args = f'-d {kernel_file} -b {dt1_file} -b {dt2_file} -z {tee_file} -Z 0x56780000'

[...]

@@ -473,10 +504,20 @@ static void fit_write_configs(struct image_tool_params 
*params, char *fdt)
                len = strlen(str);
                fdt_property_string(fdt, typename, str);
- if (params->fit_tfa_bl31) {
+               if (params->fit_tfa_bl31 && params->fit_tee) {
+                       snprintf(str, sizeof(str), "%s-1." FIT_TFA_BL31_PROP "-1." 
FIT_TEE_PROP "-1", typename);
+                       str[len] = 0;
+                       len += strlen(FIT_TFA_BL31_PROP "-1") + 1;
+                       str[len] = 0;
+                       len += strlen(FIT_TEE_PROP "-1") + 1;
+               } else if (params->fit_tfa_bl31) {
                        snprintf(str, sizeof(str), "%s-1." FIT_TFA_BL31_PROP 
"-1", typename);
                        str[len] = 0;
                        len += strlen(FIT_TFA_BL31_PROP "-1") + 1;
+               } else if (params->fit_tee) {
+                       snprintf(str, sizeof(str), "%s-1." FIT_TEE_PROP "-1", 
typename);
+                       str[len] = 0;
+                       len += strlen(FIT_TEE_PROP "-1") + 1;

It actually took me a while to figure out that we are reusing len from outside of the if block because the loadables property starts with kernel-1.

I'm wondering if it would make it more readable to do the following

if (params->fit_tfa_bl31) {
snprintf(&str[len + 1], sizeof(str) - (len + 1), FIT_TFA_BL31_PROP "-1");
    len += strlen(&str[len + 1]) + 1;
}

if (params->fit_tee) {
    snprintf(&str[len + 1], sizeof(str) - (len + 1), FIT_TEE_PROP "-1");
    len += strlen(&str[len + 1]) + 1;
}

(NOT TESTED!)

(snprintf apparently adds the trailing NUL character so no need to str[len] = 0 as far as I could tell, c.f. https://cplusplus.com/reference/cstdio/snprintf/ though that is c++ and the manpage of snprintf(3) isn't clear enough).

The issue is that we're now making access to an array and have to deal with possible out-of-bounds, as opposed to snprintf on str which handles this by itself by simply truncating safely. Not sure it's worth it.

                }
fdt_property(fdt, FIT_LOADABLE_PROP, str, len + 1);
@@ -498,10 +539,20 @@ static void fit_write_configs(struct image_tool_params 
*params, char *fdt)
                len = strlen(str);
                fdt_property_string(fdt, typename, str);
- if (params->fit_tfa_bl31) {
+               if (params->fit_tfa_bl31 && params->fit_tee) {
+                       snprintf(str, sizeof(str), "%s-1." FIT_TFA_BL31_PROP "-1." 
FIT_TEE_PROP "-1", typename);
+                       str[len] = 0;
+                       len += strlen(FIT_TFA_BL31_PROP "-1") + 1;
+                       str[len] = 0;
+                       len += strlen(FIT_TEE_PROP "-1") + 1;
+               } else if (params->fit_tfa_bl31) {
                        snprintf(str, sizeof(str), "%s-1." FIT_TFA_BL31_PROP 
"-1", typename);
                        str[len] = 0;
                        len += strlen(FIT_TFA_BL31_PROP "-1") + 1;
+               } else if (params->fit_tee) {
+                       snprintf(str, sizeof(str), "%s-1." FIT_TEE_PROP "-1", 
typename);
+                       str[len] = 0;
+                       len += strlen(FIT_TEE_PROP "-1") + 1;
                }
fdt_property(fdt, FIT_LOADABLE_PROP, str, len + 1);
diff --git a/tools/imagetool.h b/tools/imagetool.h
index 866b8834fd7..d0e7d6d56e3 100644
--- a/tools/imagetool.h
+++ b/tools/imagetool.h
@@ -101,6 +101,8 @@ struct image_tool_params {
        struct image_summary summary;   /* results of signing process */
        char *fit_tfa_bl31;     /* TFA BL31 file to include */
        unsigned int fit_tfa_bl31_addr; /* TFA BL31 load and entry point 
address */
+       char *fit_tee;          /* TEE file to include */
+       unsigned int fit_tee_addr;      /* TEE load and entry point address */
  };
/*
diff --git a/tools/mkimage.c b/tools/mkimage.c
index a800f9507bf..139d1bece2c 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -103,6 +103,8 @@ static void usage(const char *msg)
                "          -s ==> create an image with no data\n"
                "          -y ==> append TFA BL31 file to the image\n"
                "          -Y ==> set TFA BL31 file load and entry point 
address\n"
+               "          -z ==> append TEE file to the image\n"
+               "          -Z ==> set TEE file load and entry point address\n"

s/address/addresses/ ?

Looks ok to me, so

Acked-by: Quentin Schulz <[email protected]>

Thanks!
Quentin

Reply via email to