[PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[Jamin Lin]

v0:
  1. add variable to set load address and entrypoint.
  2. Fix to install nonexistent dtb file.
  3. support to verify signed FIT
  4. support to load optee-os and TFA images
v1:
  Fix patch for code review
v2:
  created a series patch
v3:
  add cover letter
v4:
  Add oe-self testcases for reviewer suggestion
  Add documentation for reviewer suggestion.
  Link: https://patchwork.yoctoproject.org/project/docs/patch/20241118062113.269253-1-jamin_lin@aspeedtech.com/

  The following patches had been applied from v0 changes.
  a. Fix to install nonexistent dtb file
  b. support to verify signed FIT
  c. add variable to set load address and entrypoint.
 
Jamin Lin (3):
  uboot-sign: support to create TEE and ATF image tree source
  uboot-sign: support to create users specific image tree source
  oe-selftest: fitimage: add testcases to test ATF and TEE

 meta/classes-recipe/uboot-sign.bbclass   | 100 +++++++-
 meta/lib/oeqa/selftest/cases/fitimage.py | 288 +++++++++++++++++++++++
 2 files changed, 387 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v4 1/3] uboot-sign: support to create TEE and ATF image tree source

[Jamin Lin]

Currently, uboot-sign.bbclass only supports to create Image Tree Source(ITS)
for "u-boot" and "flat_dt". However, users may want to support multiple images
such as  ARM Trusted Firmware(ATF), Trusted Execution Environment(TEE) and
users private images for specific application and purpose.

To make this bbclass more flexible and support ATF and TEE, creates new
functions which are "uboot_fitimage_atf" and "uboot_fitimage_tee"
for ATF and TEE ITS file creation, respectively.

Add a variable "UBOOT_FIT_ARM_TRUSTED_FIRMWARE" to
enable ATF ITS generation and it is disable by default.

Add a variable "UBOOT_FIT_TEE" to enable TEE ITS generation
and it is disable by default.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 meta/classes-recipe/uboot-sign.bbclass | 80 +++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
index a17be745ce..f1b3470127 100644
--- a/meta/classes-recipe/uboot-sign.bbclass
+++ b/meta/classes-recipe/uboot-sign.bbclass
@@ -88,6 +88,18 @@ UBOOT_FIT_ADDRESS_CELLS ?= "1"
 # This is only necessary for determining the signing configuration
 KERNEL_PN = "${PREFERRED_PROVIDER_virtual/kernel}"
 
+# ARM Trusted Firmware(ATF) is a reference implementation of secure world
+# software for Arm A-Profile architectures, (Armv8-A and Armv7-A), including
+# an Exception Level 3 (EL3) Secure Monitor.
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE ?= "0"
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE_IMAGE ?= "bl31.bin"
+
+# A Trusted Execution Environment(TEE) is an environment for executing code,
+# in which those executing the code can have high levels of trust in the asset
+# management of that surrounding environment.
+UBOOT_FIT_TEE ?= "0"
+UBOOT_FIT_TEE_IMAGE ?= "tee-raw.bin"
+
 UBOOT_FIT_UBOOT_LOADADDRESS ?= "${UBOOT_LOADADDRESS}"
 UBOOT_FIT_UBOOT_ENTRYPOINT ?= "${UBOOT_ENTRYPOINT}"
 
@@ -234,9 +246,64 @@ do_uboot_generate_rsa_keys() {
 
 addtask uboot_generate_rsa_keys before do_uboot_assemble_fitimage after do_compile
 
+# Create a ITS file for the atf
+uboot_fitimage_atf() {
+	cat << EOF >> ${UBOOT_ITS}
+        atf {
+            description = "ARM Trusted Firmware";
+            data = /incbin/("${UBOOT_FIT_ARM_TRUSTED_FIRMWARE_IMAGE}");
+            type = "firmware";
+            arch = "${UBOOT_ARCH}";
+            os = "arm-trusted-firmware";
+            load = <${UBOOT_FIT_ARM_TRUSTED_FIRMWARE_LOADADDRESS}>;
+            entry = <${UBOOT_FIT_ARM_TRUSTED_FIRMWARE_ENTRYPOINT}>;
+            compression = "none";
+EOF
+	if [ "${SPL_SIGN_ENABLE}" = "1" ] ; then
+		cat << EOF >> ${UBOOT_ITS}
+            signature {
+                algo = "${UBOOT_FIT_HASH_ALG},${UBOOT_FIT_SIGN_ALG}";
+                key-name-hint = "${SPL_SIGN_KEYNAME}";
+            };
+EOF
+	fi
+
+	cat << EOF >> ${UBOOT_ITS}
+        };
+EOF
+}
+
+# Create a ITS file for the tee
+uboot_fitimage_tee() {
+	cat << EOF >> ${UBOOT_ITS}
+        tee {
+            description = "Trusted Execution Environment";
+            data = /incbin/("${UBOOT_FIT_TEE_IMAGE}");
+            type = "tee";
+            arch = "${UBOOT_ARCH}";
+            os = "tee";
+            load = <${UBOOT_FIT_TEE_LOADADDRESS}>;
+            entry = <${UBOOT_FIT_TEE_ENTRYPOINT}>;
+            compression = "none";
+EOF
+	if [ "${SPL_SIGN_ENABLE}" = "1" ] ; then
+		cat << EOF >> ${UBOOT_ITS}
+            signature {
+                algo = "${UBOOT_FIT_HASH_ALG},${UBOOT_FIT_SIGN_ALG}";
+                key-name-hint = "${SPL_SIGN_KEYNAME}";
+            };
+EOF
+	fi
+
+	cat << EOF >> ${UBOOT_ITS}
+        };
+EOF
+}
+
 # Create a ITS file for the U-boot FIT, for use when
 # we want to sign it so that the SPL can verify it
 uboot_fitimage_assemble() {
+	conf_loadables="\"uboot\""
 	rm -f ${UBOOT_ITS} ${UBOOT_FITIMAGE_BINARY}
 
 	# First we create the ITS script
@@ -289,13 +356,24 @@ EOF
 
 	cat << EOF >> ${UBOOT_ITS}
         };
+EOF
+	if [ "${UBOOT_FIT_TEE}" = "1" ] ; then
+		conf_loadables="\"tee\", ${conf_loadables}"
+		uboot_fitimage_tee
+	fi
+
+	if [ "${UBOOT_FIT_ARM_TRUSTED_FIRMWARE}" = "1" ] ; then
+		conf_loadables="\"atf\", ${conf_loadables}"
+		uboot_fitimage_atf
+	fi
+	cat << EOF >> ${UBOOT_ITS}
     };
 
     configurations {
         default = "conf";
         conf {
             description = "Boot with signed U-Boot FIT";
-            loadables = "uboot";
+            loadables = ${conf_loadables};
             fdt = "fdt";
         };
     };
-- 
2.25.1

[PATCH v4 2/3] uboot-sign: support to create users specific image tree source

[Jamin Lin]

Currently, uboot-sign.bbclass only supports to create Image Tree Source(ITS)
for "u-boot" and "flat_dt". However, users may want to support their private
images for specific application and purpose.

To make this bbclass more flexible and support users specific ITS, creates new
"uboot_fitimage_user_image" function. It is an empty function, so users can
create a bbappend to overwrite this function with their specific settings.

Otherwise, users need to overwrite the "uboot_fitimage_assemble" function.

Add a variable "UBOOT_FIT_USER_IMAGE" to enable users specific ITS generation
and it is disable by default.

Add a variable "UBOOT_FIT_CONF_USER_LOADABLES" to load users specific images
and it is an empty by default.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 meta/classes-recipe/uboot-sign.bbclass | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
index f1b3470127..3b61f239e3 100644
--- a/meta/classes-recipe/uboot-sign.bbclass
+++ b/meta/classes-recipe/uboot-sign.bbclass
@@ -100,6 +100,13 @@ UBOOT_FIT_ARM_TRUSTED_FIRMWARE_IMAGE ?= "bl31.bin"
 UBOOT_FIT_TEE ?= "0"
 UBOOT_FIT_TEE_IMAGE ?= "tee-raw.bin"
 
+# User specific image
+UBOOT_FIT_USER_IMAGE ?= "0"
+
+# Unit name containing a list of users additional binaries to be loaded.
+# It is a comma-separated list of strings.
+UBOOT_FIT_CONF_USER_LOADABLES ?= ''
+
 UBOOT_FIT_UBOOT_LOADADDRESS ?= "${UBOOT_LOADADDRESS}"
 UBOOT_FIT_UBOOT_ENTRYPOINT ?= "${UBOOT_ENTRYPOINT}"
 
@@ -246,6 +253,12 @@ do_uboot_generate_rsa_keys() {
 
 addtask uboot_generate_rsa_keys before do_uboot_assemble_fitimage after do_compile
 
+# Create a ITS file for the user specific image. It is an empty function and
+# users should create a bbappend to overwrite this function
+uboot_fitimage_user_image() {
+	bbwarn "Please add your specific image ITS settings for u-boot FIT image generation."
+}
+
 # Create a ITS file for the atf
 uboot_fitimage_atf() {
 	cat << EOF >> ${UBOOT_ITS}
@@ -366,6 +379,13 @@ EOF
 		conf_loadables="\"atf\", ${conf_loadables}"
 		uboot_fitimage_atf
 	fi
+
+	if [ "${UBOOT_FIT_USER_IMAGE}" = "1" ] ; then
+		if [ -n "${UBOOT_FIT_CONF_USER_LOADABLES}" ] ; then
+			conf_loadables="${conf_loadables}${UBOOT_FIT_CONF_USER_LOADABLES}"
+		fi
+		uboot_fitimage_user_image
+	fi
 	cat << EOF >> ${UBOOT_ITS}
     };
 
-- 
2.25.1

[PATCH v4 3/3] oe-selftest: fitimage: add testcases to test ATF and TEE

[Jamin Lin]

Add "test_uboot_atf_tee_fit_image" test caste to check u-boot FIT image and
Image Tree Source(ITS) are built and the ITS has the correct fields.

Add "test_sign_standalone_uboot_atf_tee_fit_image" test case to check if u-boot
FIT image and Image Tree Source (ITS) are created and signed correctly for the
scenario where only the u-boot proper fitImage is being created and signed.

Currently, ATF and TEE(optee-os) recipes are placed in meta-arm layer.
OpenEmbedded-Core is a basic and core meta layer. To avoid OpenEmbedded-core
depends meta-arm, both test cases are used dummy images for testing.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 meta/lib/oeqa/selftest/cases/fitimage.py | 288 +++++++++++++++++++++++
 1 file changed, 288 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/fitimage.py b/meta/lib/oeqa/selftest/cases/fitimage.py
index 0b5f4602fb..fc9d224f50 100644
--- a/meta/lib/oeqa/selftest/cases/fitimage.py
+++ b/meta/lib/oeqa/selftest/cases/fitimage.py
@@ -852,3 +852,291 @@ FIT_HASH_ALG = "sha256"
         # Verify the signature
         uboot_tools_sysroot_native = self._setup_uboot_tools_native()
         self._verify_fit_image_signature(uboot_tools_sysroot_native, fitimage_path, os.path.join(bb_vars['DEPLOY_DIR_IMAGE'], 'am335x-bone.dtb'))
+
+
+    def test_uboot_atf_tee_fit_image(self):
+        """
+        Summary:     Check if U-boot FIT image and Image Tree Source
+                     (its) are built and the Image Tree Source has the
+                     correct fields.
+        Expected:    1. Create atf and tee dummy images
+                     2. Both u-boot-fitImage and u-boot-its can be built
+                     3. The os, load address, entrypoint address and
+                        default values of U-boot, ATF and TEE images are
+                        correct in the Image Tree Source. Not all the
+                        fields are tested, only the key fields that wont
+                        vary between different architectures.
+                Product:     oe-core
+                Author:      Jamin Lin <jamin_lin@aspeedtech.com>
+        """
+        config = """
+# We need at least CONFIG_SPL_LOAD_FIT and CONFIG_SPL_OF_CONTROL set
+MACHINE = "qemuarm"
+UBOOT_MACHINE = "am57xx_evm_defconfig"
+SPL_BINARY = "MLO"
+
+# Enable creation of the U-Boot fitImage
+UBOOT_FITIMAGE_ENABLE = "1"
+
+# (U-boot) fitImage properties
+UBOOT_LOADADDRESS = "0x80080000"
+UBOOT_ENTRYPOINT = "0x80080000"
+UBOOT_FIT_DESC = "A model description"
+
+# Enable creation of the TEE fitImage
+UBOOT_FIT_TEE = "1"
+
+# TEE fitImage properties
+UBOOT_FIT_TEE_IMAGE = "${TOPDIR}/tee-dummy.bin"
+UBOOT_FIT_TEE_LOADADDRESS = "0x80180000"
+UBOOT_FIT_TEE_ENTRYPOINT = "0x80180000"
+
+# Enable creation of the ATF fitImage
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE = "1"
+
+# ATF fitImage properties
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE_IMAGE = "${TOPDIR}/atf-dummy.bin"
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE_LOADADDRESS = "0x80280000"
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE_ENTRYPOINT = "0x80280000"
+"""
+        self.write_config(config)
+
+        # Create an ATF dummy image
+        atf_dummy_image = os.path.join(self.builddir, "atf-dummy.bin")
+        cmd = 'dd if=/dev/random of=%s bs=1k count=64' % (atf_dummy_image)
+        result = runCmd(cmd)
+        self.logger.debug("%s\nreturned: %s\n%s", cmd, str(result.status), result.output)
+
+        # Create a TEE dummy image
+        tee_dummy_image = os.path.join(self.builddir, "tee-dummy.bin")
+        cmd = 'dd if=/dev/random of=%s bs=1k count=64' % (tee_dummy_image)
+        result = runCmd(cmd)
+        self.logger.debug("%s\nreturned: %s\n%s", cmd, str(result.status), result.output)
+
+        # The U-Boot fitImage is created as part of the U-Boot recipe
+        bitbake("virtual/bootloader")
+
+        deploy_dir_image = get_bb_var('DEPLOY_DIR_IMAGE')
+        machine = get_bb_var('MACHINE')
+        fitimage_its_path = os.path.join(deploy_dir_image,
+            "u-boot-its-%s" % (machine,))
+        fitimage_path = os.path.join(deploy_dir_image,
+            "u-boot-fitImage-%s" % (machine,))
+
+        self.assertExists(fitimage_its_path, "%s image tree source doesn't exist" % (fitimage_its_path))
+        self.assertExists(fitimage_path, "%s FIT image doesn't exist" % (fitimage_path))
+
+        # Check that the os, load address, entrypoint address and default
+        # values for u-boot, ATF and TEE in Image Tree Source are as expected.
+        # The order of fields in the below array is important. Not all the
+        # fields are tested, only the key fields that wont vary between
+        # different architectures.
+        its_field_check = [
+            'description = "A model description";',
+            'os = "u-boot";',
+            'load = <0x80080000>;',
+            'entry = <0x80080000>;',
+            'description = "Trusted Execution Environment";',
+            'os = "tee";',
+            'load = <0x80180000>;',
+            'entry = <0x80180000>;',
+            'description = "ARM Trusted Firmware";',
+            'os = "arm-trusted-firmware";',
+            'load = <0x80280000>;',
+            'entry = <0x80280000>;',
+            'default = "conf";',
+            'loadables = "atf", "tee", "uboot";',
+            'fdt = "fdt";'
+        ]
+
+        with open(fitimage_its_path) as its_file:
+            field_index = 0
+            for line in its_file:
+                if field_index == len(its_field_check):
+                    break
+                if its_field_check[field_index] in line:
+                    field_index +=1
+
+        if field_index != len(its_field_check): # if its equal, the test passed
+            self.assertTrue(field_index == len(its_field_check),
+                "Fields in Image Tree Source File %s did not match, error in finding %s"
+                % (fitimage_its_path, its_field_check[field_index]))
+
+
+    def test_sign_standalone_uboot_atf_tee_fit_image(self):
+        """
+        Summary:     Check if U-Boot FIT image and Image Tree Source (its) are
+                     created and signed correctly for the scenario where only
+                     the U-Boot proper fitImage is being created and signed.
+        Expected:    1. Create atf and tee dummy images
+                     2. U-Boot its and FIT image are built successfully
+                     3. Scanning the its file indicates signing is enabled
+                        as requested by SPL_SIGN_ENABLE (using keys generated
+                        via UBOOT_FIT_GENERATE_KEYS)
+                     4. Dumping the FIT image indicates signature values
+                        are present
+                     5. Examination of the do_uboot_assemble_fitimage
+                     runfile/logfile indicate that UBOOT_MKIMAGE, UBOOT_MKIMAGE_SIGN
+                     and SPL_MKIMAGE_SIGN_ARGS are working as expected.
+                Product:     oe-core
+                Author:      Jamin Lin <jamin_lin@aspeedtech.com>
+        """
+        a_comment = "a smart cascaded U-Boot ATF TEE comment"
+        config = """
+# There's no U-boot deconfig with CONFIG_FIT_SIGNATURE yet, so we need at
+# least CONFIG_SPL_LOAD_FIT and CONFIG_SPL_OF_CONTROL set
+MACHINE = "qemuarm"
+UBOOT_MACHINE = "am57xx_evm_defconfig"
+SPL_BINARY = "MLO"
+
+# The kernel-fitimage class is a dependency even if we're only
+# creating/signing the U-Boot fitImage
+KERNEL_CLASSES = " kernel-fitimage"
+
+# Enable creation and signing of the U-Boot fitImage
+UBOOT_FITIMAGE_ENABLE = "1"
+SPL_SIGN_ENABLE = "1"
+SPL_SIGN_KEYNAME = "spl-oe-selftest"
+SPL_SIGN_KEYDIR = "${TOPDIR}/signing-keys"
+UBOOT_DTB_BINARY = "u-boot.dtb"
+UBOOT_ARCH = "arm"
+SPL_MKIMAGE_DTCOPTS = "-I dts -O dtb -p 2000"
+SPL_MKIMAGE_SIGN_ARGS = "-c '%s'"
+UBOOT_EXTLINUX = "0"
+UBOOT_FIT_GENERATE_KEYS = "1"
+UBOOT_FIT_HASH_ALG = "sha256"
+
+# (U-boot) fitImage properties
+UBOOT_LOADADDRESS = "0x80080000"
+UBOOT_ENTRYPOINT = "0x80080000"
+UBOOT_FIT_DESC = "A model description"
+
+# Enable creation of the TEE fitImage
+UBOOT_FIT_TEE = "1"
+
+# TEE fitImage properties
+UBOOT_FIT_TEE_IMAGE = "${TOPDIR}/tee-dummy.bin"
+UBOOT_FIT_TEE_LOADADDRESS = "0x80180000"
+UBOOT_FIT_TEE_ENTRYPOINT = "0x80180000"
+
+# Enable creation of the ATF fitImage
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE = "1"
+
+# ATF fitImage properties
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE_IMAGE = "${TOPDIR}/atf-dummy.bin"
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE_LOADADDRESS = "0x80280000"
+UBOOT_FIT_ARM_TRUSTED_FIRMWARE_ENTRYPOINT = "0x80280000"
+""" % a_comment
+
+        self.write_config(config)
+
+        # Create an ATF dummy image
+        atf_dummy_image = os.path.join(self.builddir, "atf-dummy.bin")
+        cmd = 'dd if=/dev/random of=%s bs=1k count=64' % (atf_dummy_image)
+        result = runCmd(cmd)
+        self.logger.debug("%s\nreturned: %s\n%s", cmd, str(result.status), result.output)
+
+        # Create a TEE dummy image
+        tee_dummy_image = os.path.join(self.builddir, "tee-dummy.bin")
+        cmd = 'dd if=/dev/random of=%s bs=1k count=64' % (tee_dummy_image)
+        result = runCmd(cmd)
+        self.logger.debug("%s\nreturned: %s\n%s", cmd, str(result.status), result.output)
+
+        # The U-Boot fitImage is created as part of the U-Boot recipe
+        bitbake("virtual/bootloader")
+
+        deploy_dir_image = get_bb_var('DEPLOY_DIR_IMAGE')
+        machine = get_bb_var('MACHINE')
+        fitimage_its_path = os.path.join(deploy_dir_image,
+            "u-boot-its-%s" % (machine,))
+        fitimage_path = os.path.join(deploy_dir_image,
+            "u-boot-fitImage-%s" % (machine,))
+
+        self.assertExists(fitimage_its_path, "%s image tree source doesn't exist" % (fitimage_its_path))
+        self.assertExists(fitimage_path, "%s FIT image doesn't exist" % (fitimage_path))
+
+        req_itspaths = [
+            ['/', 'images', 'uboot'],
+            ['/', 'images', 'uboot', 'signature'],
+            ['/', 'images', 'fdt'],
+            ['/', 'images', 'fdt', 'signature'],
+            ['/', 'images', 'tee'],
+            ['/', 'images', 'tee', 'signature'],
+            ['/', 'images', 'atf'],
+            ['/', 'images', 'atf', 'signature'],
+        ]
+
+        itspath = []
+        itspaths = []
+        linect = 0
+        sigs = {}
+        with open(fitimage_its_path) as its_file:
+            linect += 1
+            for line in its_file:
+                line = line.strip()
+                if line.endswith('};'):
+                    itspath.pop()
+                elif line.endswith('{'):
+                    itspath.append(line[:-1].strip())
+                    itspaths.append(itspath[:])
+                elif itspath and itspath[-1] == 'signature':
+                    itsdotpath = '.'.join(itspath)
+                    if not itsdotpath in sigs:
+                        sigs[itsdotpath] = {}
+                    if not '=' in line or not line.endswith(';'):
+                        self.fail('Unexpected formatting in %s sigs section line %d:%s' % (fitimage_its_path, linect, line))
+                    key, value = line.split('=', 1)
+                    sigs[itsdotpath][key.rstrip()] = value.lstrip().rstrip(';')
+
+        for reqpath in req_itspaths:
+            if not reqpath in itspaths:
+                self.fail('Missing section in its file: %s' % reqpath)
+
+        reqsigvalues_image = {
+            'algo': '"sha256,rsa2048"',
+            'key-name-hint': '"spl-oe-selftest"',
+        }
+
+        for itspath, values in sigs.items():
+            reqsigvalues = reqsigvalues_image
+            for reqkey, reqvalue in reqsigvalues.items():
+                value = values.get(reqkey, None)
+                if value is None:
+                    self.fail('Missing key "%s" in its file signature section %s' % (reqkey, itspath))
+                self.assertEqual(value, reqvalue)
+
+        # Dump the image to see if it really got signed
+        uboot_tools_sysroot_native = self._setup_uboot_tools_native()
+        dumpimage_path = os.path.join(uboot_tools_sysroot_native, 'usr', 'bin', 'dumpimage')
+        result = runCmd('%s -l %s' % (dumpimage_path, fitimage_path))
+        in_signed = None
+        signed_sections = {}
+        for line in result.output.splitlines():
+            if line.startswith((' Image')):
+                in_signed = re.search(r'\((.*)\)', line).groups()[0]
+            elif re.match(' \w', line):
+                in_signed = None
+            elif in_signed:
+                if not in_signed in signed_sections:
+                    signed_sections[in_signed] = {}
+                key, value = line.split(':', 1)
+                signed_sections[in_signed][key.strip()] = value.strip()
+        self.assertIn('uboot', signed_sections)
+        self.assertIn('fdt', signed_sections)
+        self.assertIn('tee', signed_sections)
+        self.assertIn('atf', signed_sections)
+        for signed_section, values in signed_sections.items():
+            value = values.get('Sign algo', None)
+            self.assertEqual(value, 'sha256,rsa2048:spl-oe-selftest', 'Signature algorithm for %s not expected value' % signed_section)
+            value = values.get('Sign value', None)
+            self.assertEqual(len(value), 512, 'Signature value for section %s not expected length' % signed_section)
+
+        # Check for SPL_MKIMAGE_SIGN_ARGS
+        # Looks like mkimage supports to add a comment but does not support to read it back.
+        found_comments = FitImageTests._find_string_in_bin_file(fitimage_path, a_comment)
+        self.assertEqual(found_comments, 4, "Expected 4 signed and commented section in the fitImage.")
+
+        # Verify the signature
+        self._verify_fit_image_signature(uboot_tools_sysroot_native, fitimage_path,
+                                         os.path.join(deploy_dir_image, 'u-boot-spl.dtb'))
+
-- 
2.25.1

Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[Ross Burton]

Hi Jamin,

Thanks for the patches.

However, I’m getting more convinced that as our FIT generation is spread between uboot-sign and kernel-fitimage, maybe we should just create a new class that is dedicated to creating a FIT image from arbitrary inputs, so in this case you’d have dependencies on tf-a/uboot/kernel and write a dts that describes the layout.  I’m unconvinced that the complexity of these classes is sustainable and a truly generic class might be a more maintainable alternative.

What’s your opinion on this?  The fact that you had to add custom hooks in 2/3 seems to indicate that a more inherently flexible class would be the right approach.

Cheers,
Ross


> On 18 Nov 2024, at 06:32, Jamin Lin via lists.openembedded.org <jamin_lin=aspeedtech.com@lists.openembedded.org> wrote:
> 
> v0:
>  1. add variable to set load address and entrypoint.
>  2. Fix to install nonexistent dtb file.
>  3. support to verify signed FIT
>  4. support to load optee-os and TFA images
> v1:
>  Fix patch for code review
> v2:
>  created a series patch
> v3:
>  add cover letter
> v4:
>  Add oe-self testcases for reviewer suggestion
>  Add documentation for reviewer suggestion.
>  Link: https://patchwork.yoctoproject.org/project/docs/patch/20241118062113.269253-1-jamin_lin@aspeedtech.com/
> 
>  The following patches had been applied from v0 changes.
>  a. Fix to install nonexistent dtb file
>  b. support to verify signed FIT
>  c. add variable to set load address and entrypoint.
> 
> Jamin Lin (3):
>  uboot-sign: support to create TEE and ATF image tree source
>  uboot-sign: support to create users specific image tree source
>  oe-selftest: fitimage: add testcases to test ATF and TEE
> 
> meta/classes-recipe/uboot-sign.bbclass   | 100 +++++++-
> meta/lib/oeqa/selftest/cases/fitimage.py | 288 +++++++++++++++++++++++
> 2 files changed, 387 insertions(+), 1 deletion(-)
> 
> -- 
> 2.25.1
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#207203): https://lists.openembedded.org/g/openembedded-core/message/207203
> Mute This Topic: https://lists.openembedded.org/mt/109640118/6875888
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ross.burton@arm.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

RE: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[Jamin Lin]

Hi Ross

> Subject: Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS
> generation
> 
> Hi Jamin,
> 
> Thanks for the patches.
> 
> However, I’m getting more convinced that as our FIT generation is spread
> between uboot-sign and kernel-fitimage, maybe we should just create a new
> class that is dedicated to creating a FIT image from arbitrary inputs, so in this
> case you’d have dependencies on tf-a/uboot/kernel and write a dts that
> describes the layout.  I’m unconvinced that the complexity of these classes is
> sustainable and a truly generic class might be a more maintainable
> alternative.
> 
> What’s your opinion on this?  The fact that you had to add custom hooks in
> 2/3 seems to indicate that a more inherently flexible class would be the right
> approach.
> 
Thanks for your review and suggestion.

I agree to create a new bbclass to generate the ITS with users arbitrary inputs.
Could you please give me the following comments?

1.	What is the name of this bbclass?
2.	This new bbclass should be placed in which directory and meta layer?
3.	My briefly proposal of this new bbclass as following, could you please give me any suggestion?

create_its() {
	cat << EOF >> ${UBOOT_ITS}
    ${USER_FIRMWARE} {
            description = ${USER_DESCRIPTION};
            data = /incbin/("${UBOOT_FIT_SSP_IMAGE}");
            type = ${USER_FIRMWARE};
            arch = "${USER_ARCH}";
            os = "${USER_OS}";
            load = <${USER_LOADADDRESS}>;
            entry = <${USER_ENTRYPOINT}>;
            compression = ${USER_COMPRESS};
EOF
	if [ "${SPL_SIGN_ENABLE}" = "1" ] ; then
		cat << EOF >> ${UBOOT_ITS}
            signature {
                algo = "${UBOOT_FIT_HASH_ALG},${UBOOT_FIT_SIGN_ALG}";
                key-name-hint = "${SPL_SIGN_KEYNAME}";
            };
EOF
	fi
	cat << EOF >> ${UBOOT_ITS}
    };
EOF
}

Then, users set “USER_XXX” variables to create their own ITS file for u-boot/kernel FIT image generation.
Or can you give me any suggestion about your goal and ITS improvement?

Thanks-Jamin

> Cheers,
> Ross
> 
> 
> > On 18 Nov 2024, at 06:32, Jamin Lin via lists.openembedded.org
> <jamin_lin=aspeedtech.com@lists.openembedded.org> wrote:
> >
> > v0:
> >  1. add variable to set load address and entrypoint.
> >  2. Fix to install nonexistent dtb file.
> >  3. support to verify signed FIT
> >  4. support to load optee-os and TFA images
> > v1:
> >  Fix patch for code review
> > v2:
> >  created a series patch
> > v3:
> >  add cover letter
> > v4:
> >  Add oe-self testcases for reviewer suggestion  Add documentation for
> > reviewer suggestion.
> >  Link:
> > https://patchwork.yoctoproject.org/project/docs/patch/20241118062113.2
> > 69253-1-jamin_lin@aspeedtech.com/
> >
> >  The following patches had been applied from v0 changes.
> >  a. Fix to install nonexistent dtb file  b. support to verify signed
> > FIT  c. add variable to set load address and entrypoint.
> >
> > Jamin Lin (3):
> >  uboot-sign: support to create TEE and ATF image tree source
> >  uboot-sign: support to create users specific image tree source
> >  oe-selftest: fitimage: add testcases to test ATF and TEE
> >
> > meta/classes-recipe/uboot-sign.bbclass   | 100 +++++++-
> > meta/lib/oeqa/selftest/cases/fitimage.py | 288 +++++++++++++++++++++++
> > 2 files changed, 387 insertions(+), 1 deletion(-)
> >
> > --
> > 2.25.1
> >
> >
> >
> >

Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[Ross Burton]

Hi Jamin,

On 2 Dec 2024, at 05:39, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> create_its() {
> cat << EOF >> ${UBOOT_ITS}
>    ${USER_FIRMWARE} {
>            description = ${USER_DESCRIPTION};
>            data = /incbin/("${UBOOT_FIT_SSP_IMAGE}");
>            type = ${USER_FIRMWARE};
>            arch = "${USER_ARCH}";
>            os = "${USER_OS}";
>            load = <${USER_LOADADDRESS}>;
>            entry = <${USER_ENTRYPOINT}>;
>            compression = ${USER_COMPRESS};
> EOF
> if [ "${SPL_SIGN_ENABLE}" = "1" ] ; then
> cat << EOF >> ${UBOOT_ITS}
>            signature {
>                algo = "${UBOOT_FIT_HASH_ALG},${UBOOT_FIT_SIGN_ALG}";
>                key-name-hint = "${SPL_SIGN_KEYNAME}";
>            };
> EOF
> fi
> cat << EOF >> ${UBOOT_ITS}
>    };
> EOF
> }
> 
> Then, users set “USER_XXX” variables to create their own ITS file for u-boot/kernel FIT image generation.
> Or can you give me any suggestion about your goal and ITS improvement?

I was actually wondering if it would be sensible to have the user of the class write the .its from scratch.  It could support variable expansion to allow the use of variables that are already defined in the machine configuration, but I could see someone wanting more flexibility in your design.

Important note: I’ve not used fitimages so I might not be right, this is just an observation from seeing increasing amounts of customisation needed on the existing fit generation.

Ross

Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[Adrian Freihofer]

Hi Jamin, hi Ross

On Thu, 2024-11-28 at 13:51 +0000, Ross Burton via
lists.openembedded.org wrote:
> Hi Jamin,
> 
> Thanks for the patches.
> 
> However, I’m getting more convinced that as our FIT generation is
> spread between uboot-sign and kernel-fitimage, maybe we should just
> create a new class that is dedicated to creating a FIT image from
> arbitrary inputs, so in this case you’d have dependencies on tf-
> a/uboot/kernel and write a dts that describes the layout.  I’m
> unconvinced that the complexity of these classes is sustainable and a
> truly generic class might be a more maintainable alternative.

I apologize for digressing a bit now. But the complexity which requires
a redesign of the ftiImage generation code only becomes clear when you
look at the kernel and u-boot fitImage handling together.

I think that the generation of its files, as done by kernel-
fitimage.bbclass and uboot-sign.bbclass, is not generally wrong.
Writing an its file is not something that a user can do quickly. It
requires quite a lot of know-how, which is taken away from the user by
the existing code. The code is also relatively well tested, which I
can't imagine it can be achieved with handwritten its files.
Standardizing how its files should be written looks helpful to me in
general.

Handling the task dependencies is not easy either. Many artifacts and
keys must be provided by different recipes before the assembly of 
fitImages can take place. Simply leaving it up to the user certainly
doesn't make it any easier.

But I agree, it ended up at an unmaintainable state. The main issue is
that the generator code in the kernel-fitimage.bbclass currently gets
executed from the kernel build folder and from the u-boot build folder
and that the u-boot and the kernel recipes have many inter task
dependencies. It does not properly work with the sstate-cache. For
example building with empty temp means rebuilding from scratch at least
when also an initramfs is involved. I tried to solve this with this
series here
https://patchwork.yoctoproject.org/project/oe-core/list/?series=27108.
It is also possible to end up with circular task dependencies by
setting the UBOOT_* variables to SIGNED images, for example, and
packing a boot.txt file into the fitImage.

The idea that seems most promising to me to solve these problems (
sstate, maintainability, complexity, task dependencies) is to
completely remove the fitimage code from the kernel and the u-boot
recipes and create a linux-fitimage and an uboot-fitimage recipe that
take the artifacts from the linux and the u-boot recipes and simply put
them into a fit container. The fitimage_assemble and
uboot_fitimage_assemble functions would be reusable. All the rest would
probably be rewritten from scratch.
Some use cases in which, for example, the kernel Makefile (which can
run from the kernel build tree only not from a setscene folder
structure) generates the fitImage should simply be omitted in favor of
simplification.

Having for example a linux-yocto recipe which does not support building
fitImages at all and a second recipe linux-yocto-fitimage which depends
on the linux-yocto recipe would allow several improvements:
- The kernel artifacts can be forwarded via sysroots from linux-yocto
to the linux-yocto-fitimage. Currently it goes via deploy.
- The kernel-yocto-fitImage class could also provide a kernel package.
Currently this kernel artifact is only available from the deploy
folder.
- Standard sstate tasks (sysroot and deploy) can be used for building
the kernel and for building the fitImage. Doing all these steps in the
contecxt of one recipe leads to unusual sstated tasks which causes some
issues.
- I hope the code would be much simpler. But I'm not set sure. I want
to try this but did not find the time so far.

> 
> What’s your opinion on this?  The fact that you had to add custom
> hooks in 2/3 seems to indicate that a more inherently flexible class
> would be the right approach.

I think the two new functions uboot_fitimage_atf and uboot_fitimage_tee
look reasonable and are in line with the existing code.

But the uboot_fitimage_user_image hook looks a little unusual. What's
the use case for that? Would it be possible to pass the its snippet as
a variable? Or even support the use case with a standard snippet?

All in all, I think these patches do not make it worse than it already
is. Solving the issues with the fitImage is a different topic.

Regards,
Adrian




> 
> Cheers,
> Ross
> 
> 
> > On 18 Nov 2024, at 06:32, Jamin Lin via lists.openembedded.org
> > <jamin_lin=aspeedtech.com@lists.openembedded.org> wrote:
> > 
> > v0:
> >  1. add variable to set load address and entrypoint.
> >  2. Fix to install nonexistent dtb file.
> >  3. support to verify signed FIT
> >  4. support to load optee-os and TFA images
> > v1:
> >  Fix patch for code review
> > v2:
> >  created a series patch
> > v3:
> >  add cover letter
> > v4:
> >  Add oe-self testcases for reviewer suggestion
> >  Add documentation for reviewer suggestion.
> >  Link:
> > https://patchwork.yoctoproject.org/project/docs/patch/20241118062113.269253-1-jamin_lin@aspeedtech.com/
> > 
> >  The following patches had been applied from v0 changes.
> >  a. Fix to install nonexistent dtb file
> >  b. support to verify signed FIT
> >  c. add variable to set load address and entrypoint.
> > 
> > Jamin Lin (3):
> >  uboot-sign: support to create TEE and ATF image tree source
> >  uboot-sign: support to create users specific image tree source
> >  oe-selftest: fitimage: add testcases to test ATF and TEE
> > 
> > meta/classes-recipe/uboot-sign.bbclass   | 100 +++++++-
> > meta/lib/oeqa/selftest/cases/fitimage.py | 288
> > +++++++++++++++++++++++
> > 2 files changed, 387 insertions(+), 1 deletion(-)
> > 
> > -- 
> > 2.25.1
> > 
> > 
> > 
> > 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#207991):
> https://lists.openembedded.org/g/openembedded-core/message/207991
> Mute This Topic: https://lists.openembedded.org/mt/109640118/4454582
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/openembedded-core/unsub [
> adrian.freihofer@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

RE: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[Jamin Lin]

Hi Adrian, Ross

Thanks for your input.
Sorry, I am working on another task and will update you soon.
The following are my briefly answer about uboot_fitimage_user_image function.

> Subject: Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS
> generation
> 
> Hi Jamin, hi Ross
> 
> On Thu, 2024-11-28 at 13:51 +0000, Ross Burton via lists.openembedded.org
> wrote:
> > Hi Jamin,
> >
> > Thanks for the patches.
> >
> > However, I’m getting more convinced that as our FIT generation is
> > spread between uboot-sign and kernel-fitimage, maybe we should just
> > create a new class that is dedicated to creating a FIT image from
> > arbitrary inputs, so in this case you’d have dependencies on tf-
> > a/uboot/kernel and write a dts that describes the layout.  I’m
> > unconvinced that the complexity of these classes is sustainable and a
> > truly generic class might be a more maintainable alternative.
> 
> I apologize for digressing a bit now. But the complexity which requires a
> redesign of the ftiImage generation code only becomes clear when you look at
> the kernel and u-boot fitImage handling together.
> 
> I think that the generation of its files, as done by kernel- fitimage.bbclass and
> uboot-sign.bbclass, is not generally wrong.
> Writing an its file is not something that a user can do quickly. It requires quite
> a lot of know-how, which is taken away from the user by the existing code. The
> code is also relatively well tested, which I can't imagine it can be achieved
> with handwritten its files.
> Standardizing how its files should be written looks helpful to me in general.
> 
> Handling the task dependencies is not easy either. Many artifacts and keys
> must be provided by different recipes before the assembly of fitImages can
> take place. Simply leaving it up to the user certainly doesn't make it any easier.
> 
> But I agree, it ended up at an unmaintainable state. The main issue is that the
> generator code in the kernel-fitimage.bbclass currently gets executed from the
> kernel build folder and from the u-boot build folder and that the u-boot and
> the kernel recipes have many inter task dependencies. It does not properly
> work with the sstate-cache. For example building with empty temp means
> rebuilding from scratch at least when also an initramfs is involved. I tried to
> solve this with this series here
> https://patchwork.yoctoproject.org/project/oe-core/list/?series=27108.
> It is also possible to end up with circular task dependencies by setting the
> UBOOT_* variables to SIGNED images, for example, and packing a boot.txt file
> into the fitImage.
> 
> The idea that seems most promising to me to solve these problems ( sstate,
> maintainability, complexity, task dependencies) is to completely remove the
> fitimage code from the kernel and the u-boot recipes and create a
> linux-fitimage and an uboot-fitimage recipe that take the artifacts from the
> linux and the u-boot recipes and simply put them into a fit container. The
> fitimage_assemble and uboot_fitimage_assemble functions would be reusable.
> All the rest would probably be rewritten from scratch.
> Some use cases in which, for example, the kernel Makefile (which can run
> from the kernel build tree only not from a setscene folder
> structure) generates the fitImage should simply be omitted in favor of
> simplification.
> 
> Having for example a linux-yocto recipe which does not support building
> fitImages at all and a second recipe linux-yocto-fitimage which depends on the
> linux-yocto recipe would allow several improvements:
> - The kernel artifacts can be forwarded via sysroots from linux-yocto to the
> linux-yocto-fitimage. Currently it goes via deploy.
> - The kernel-yocto-fitImage class could also provide a kernel package.
> Currently this kernel artifact is only available from the deploy folder.
> - Standard sstate tasks (sysroot and deploy) can be used for building the kernel
> and for building the fitImage. Doing all these steps in the contecxt of one
> recipe leads to unusual sstated tasks which causes some issues.
> - I hope the code would be much simpler. But I'm not set sure. I want to try this
> but did not find the time so far.
> 
> >
> > What’s your opinion on this?  The fact that you had to add custom
> > hooks in 2/3 seems to indicate that a more inherently flexible class
> > would be the right approach.
> 
> I think the two new functions uboot_fitimage_atf and uboot_fitimage_tee look
> reasonable and are in line with the existing code.
> 
> But the uboot_fitimage_user_image hook looks a little unusual. What's the use
> case for that? Would it be possible to pass the its snippet as a variable? Or
> even support the use case with a standard snippet?

The reason is ASPEED want to add our images into u-boot FIT image which are TSP and SSP.
Both images are ASPEED only and they are not generic images such as TEE and ATF.
Therefore, I added this hook function to make users add their ITS for specific need.
Our ITS looks like as following for ASPEED latest SOCs(AST2700)

Thanks -Jamin

/dts-v1/;

/ {
    description = "U-Boot fitImage for Phosphor OpenBMC (Phosphor OpenBMC Project Reference Distro)/v2023.10+git/ast2700-default";
    #address-cells = <1>;

    images {
        uboot {
            description = "U-Boot image";
            data = /incbin/("u-boot-nodtb.bin");
            type = "standalone";
            os = "u-boot";
            arch = "";
            compression = "none";
            load = <0x80000000>;
            entry = <0x80000000>;
        };
        fdt {
            description = "U-Boot FDT";
            data = /incbin/("u-boot.dtb");
            type = "flat_dt";
            arch = "";
            compression = "none";
        };
        tee {
            description = "Trusted Execution Environment";
            data = /incbin/("tee-raw.bin");
            type = "tee";
            arch = "";
            os = "tee";
            load = <0xb0080000>;
            entry = <0xb0080000>;
            compression = "none";
        };
        atf {
            description = "ARM Trusted Firmware";
            data = /incbin/("bl31.bin");
            type = "firmware";
            arch = "";
            os = "arm-trusted-firmware";
            load = <0xb0000000>;
            entry = <0xb0000000>;
            compression = "none";
        };
        sspfw {
            description = "SSP Firmware";
            data = /incbin/("ast2700-ssp.bin");
            type = "firmware";
            arch = "arm";
            os = "zephyr";
            load = <0xb2000000>;
            entry = <0xb2000000>;
            compression = "none";
        };
        tspfw {
            description = "TSP Firmware";
            data = /incbin/("ast2700-tsp.bin");
            type = "firmware";
            arch = "arm";
            os = "zephyr";
            load = <0xb4000000>;
            entry = <0xb4000000>;
            compression = "none";
        };
    };

    configurations {
        default = "conf";
        conf {
            description = "Boot with signed U-Boot FIT";
            loadables = "atf", "tee", "uboot" ,"sspfw" ,"tspfw";
            fdt = "fdt";
        };
    };
};


> 
> All in all, I think these patches do not make it worse than it already is. Solving
> the issues with the fitImage is a different topic.
> 
> Regards,
> Adrian
> 
> 
> 
> 
> >
> > Cheers,
> > Ross
> >
> >
> > > On 18 Nov 2024, at 06:32, Jamin Lin via lists.openembedded.org
> > > <jamin_lin=aspeedtech.com@lists.openembedded.org> wrote:
> > >
> > > v0:
> > >  1. add variable to set load address and entrypoint.
> > >  2. Fix to install nonexistent dtb file.
> > >  3. support to verify signed FIT
> > >  4. support to load optee-os and TFA images
> > > v1:
> > >  Fix patch for code review
> > > v2:
> > >  created a series patch
> > > v3:
> > >  add cover letter
> > > v4:
> > >  Add oe-self testcases for reviewer suggestion
> > >  Add documentation for reviewer suggestion.
> > >  Link:
> > > https://patchwork.yoctoproject.org/project/docs/patch/20241118062113
> > > .269253-1-jamin_lin@aspeedtech.com/
> > >
> > >  The following patches had been applied from v0 changes.
> > >  a. Fix to install nonexistent dtb file
> > >  b. support to verify signed FIT
> > >  c. add variable to set load address and entrypoint.
> > >
> > > Jamin Lin (3):
> > >  uboot-sign: support to create TEE and ATF image tree source
> > >  uboot-sign: support to create users specific image tree source
> > >  oe-selftest: fitimage: add testcases to test ATF and TEE
> > >
> > > meta/classes-recipe/uboot-sign.bbclass   | 100 +++++++-
> > > meta/lib/oeqa/selftest/cases/fitimage.py | 288
> > > +++++++++++++++++++++++
> > > 2 files changed, 387 insertions(+), 1 deletion(-)
> > >
> > > --
> > > 2.25.1
> > >
> > >
> > >
> > >
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#207991):
> > https://lists.openembedded.org/g/openembedded-core/message/207991
> > Mute This Topic: https://lists.openembedded.org/mt/109640118/4454582
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe:
> > https://lists.openembedded.org/g/openembedded-core/unsub [
> > adrian.freihofer@gmail.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >

RE: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[Jamin Lin]

Hi Ross, Adrian

Sorry reply you late.

I successfully added users snippet ITS in the variable and my changes as following.

1. remove hook function uboot_fitimage_user_image
2. replace "UBOOT_FIT_USER_IMAGE" with "UBOOT_FIT_USER_SETTINGS" variable

# User specific settings
UBOOT_FIT_USER_SETTINGS ?= ""

# Unit name containing a list of users additional binaries to be loaded.
# It is a comma-separated list of strings.
UBOOT_FIT_CONF_USER_LOADABLES ?= ''


Create a ITS file for the U-boot FIT, for use when
# we want to sign it so that the SPL can verify it
uboot_fitimage_assemble() {
---
	if [ -n "${UBOOT_FIT_USER_SETTINGS}" ] ; then
		echo -e "${UBOOT_FIT_USER_SETTINGS}" >> ${UBOOT_ITS}
	fi

	if [ -n "${UBOOT_FIT_CONF_USER_LOADABLES}" ] ; then
		conf_loadables="${conf_loadables}${UBOOT_FIT_CONF_USER_LOADABLES}"
	fi
---

3. Set UBOOT_FIT_CONF_USER_LOADABLES to load users image

UBOOT_FIT_CONF_USER_LOADABLES = '  \"sspfw\", \"tspfw\"  '

4. Users add their own snippet ITS into UBOOT_FIT_USER_SETTINGS variable.

Ex:

UBOOT_FIT_SSP_ITS = '\
       sspfw {\n\
            description = \"SSP Firmware\";\n\
            data = /incbin/(\"${UBOOT_FIT_SSP_IMAGE}\");\n\
            type = \"firmware\";\n\
            arch = \"${UBOOT_FIT_SSP_ARCH}\";\n\
            os = \"${UBOOT_FIT_SSP_OS}\";\n\
            load = <${UBOOT_FIT_SSP_LOADADDRESS}>;\n\
            entry = <${UBOOT_FIT_SSP_ENTRYPOINT}>;\n\
            compression = \"none\";\n\
        };\n\
'
UBOOT_FIT_TSP_ITS = '\
       tspfw {\n\
            description = \"TSP Firmware\";\n\
            data = /incbin/(\"${UBOOT_FIT_TSP_IMAGE}\");\n\
            type = \"firmware\";\n\
            arch = \"${UBOOT_FIT_TSP_ARCH}\";\n\
            os = \"${UBOOT_FIT_TSP_OS}\";\n\
            load = <${UBOOT_FIT_TSP_LOADADDRESS}>;\n\
            entry = <${UBOOT_FIT_TSP_ENTRYPOINT}>;\n\
            compression = \"none\";\n\
        };\n\
'

UBOOT_FIT_USER_SETTINGS = " ${UBOOT_FIT_SSP_ITS}  ${UBOOT_FIT_TSP_ITS} "

Then, I got the following generated ITS.

```
/dts-v1/;

/ {
    description = "U-Boot fitImage for Phosphor OpenBMC (Phosphor OpenBMC Project Reference Distro)/v2023.10+git/ast2700-default";
    #address-cells = <1>;

    images {
        uboot {
            description = "U-Boot image";
            data = /incbin/("u-boot-nodtb.bin");
            type = "standalone";
            os = "u-boot";
            arch = "";
            compression = "none";
            load = <0x80000000>;
            entry = <0x80000000>;
        };
        fdt {
            description = "U-Boot FDT";
            data = /incbin/("u-boot.dtb");
            type = "flat_dt";
            arch = "";
            compression = "none";
        };
        tee {
            description = "Trusted Execution Environment";
            data = /incbin/("/home/jamin_lin/openbmc-ast2700/1206/build-ast2700/tmp/deploy/images/ast2700-default/optee/tee-raw.bin");
            type = "tee";
            arch = "";
            os = "tee";
            load = <0xb0080000>;
            entry = <0xb0080000>;
            compression = "none";
        };
        atf {
            description = "ARM Trusted Firmware";
            data = /incbin/("/home/jamin_lin/openbmc-ast2700/1206/build-ast2700/tmp/deploy/images/ast2700-default/bl31.bin");
            type = "firmware";
            arch = "";
            os = "arm-trusted-firmware";
            load = <0xb0000000>;
            entry = <0xb0000000>;
            compression = "none";
        };
        sspfw {
            description = "SSP Firmware";
            data = /incbin/("/home/jamin_lin/openbmc-ast2700/1206/build-ast2700/tmp/deploy/images/ast2700-default/ast2700-ssp.bin");
            type = "firmware";
            arch = "arm";
            os = "zephyr";
            load = <0xb2000000>;
            entry = <0xb2000000>;
            compression = "none";
        };
        tspfw {
            description = "TSP Firmware";
            data = /incbin/("/home/jamin_lin/openbmc-ast2700/1206/build-ast2700/tmp/deploy/images/ast2700-default/ast2700-tsp.bin");
            type = "firmware";
            arch = "arm";
            os = "zephyr";
            load = <0xb4000000>;
            entry = <0xb4000000>;
            compression = "none";
        };

    };

    configurations {
        default = "conf";
        conf {
            description = "Boot with signed U-Boot FIT";
            loadables = "atf", "tee", "uboot" ,"sspfw" ,"tspfw";
            fdt = "fdt";
        };
    };
};
```

If you agree, this new design, I will resend v4 patch.
Do you have any concern or could you please give me any suggestion?

Thanks-Jamin

> Subject: Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS
> generation
> 
> Hi Adrian, Ross
> 
> Thanks for your input.
> Sorry, I am working on another task and will update you soon.
> The following are my briefly answer about uboot_fitimage_user_image
> function.
> 
> > Subject: Re: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE
> > ITS generation
> >
> > Hi Jamin, hi Ross
> >
> > On Thu, 2024-11-28 at 13:51 +0000, Ross Burton via
> > lists.openembedded.org
> > wrote:
> > > Hi Jamin,
> > >
> > > Thanks for the patches.
> > >
> > > However, I’m getting more convinced that as our FIT generation is
> > > spread between uboot-sign and kernel-fitimage, maybe we should just
> > > create a new class that is dedicated to creating a FIT image from
> > > arbitrary inputs, so in this case you’d have dependencies on tf-
> > > a/uboot/kernel and write a dts that describes the layout.  I’m
> > > unconvinced that the complexity of these classes is sustainable and
> > > a truly generic class might be a more maintainable alternative.
> >
> > I apologize for digressing a bit now. But the complexity which
> > requires a redesign of the ftiImage generation code only becomes clear
> > when you look at the kernel and u-boot fitImage handling together.
> >
> > I think that the generation of its files, as done by kernel-
> > fitimage.bbclass and uboot-sign.bbclass, is not generally wrong.
> > Writing an its file is not something that a user can do quickly. It
> > requires quite a lot of know-how, which is taken away from the user by
> > the existing code. The code is also relatively well tested, which I
> > can't imagine it can be achieved with handwritten its files.
> > Standardizing how its files should be written looks helpful to me in general.
> >
> > Handling the task dependencies is not easy either. Many artifacts and
> > keys must be provided by different recipes before the assembly of
> > fitImages can take place. Simply leaving it up to the user certainly doesn't
> make it any easier.
> >
> > But I agree, it ended up at an unmaintainable state. The main issue is
> > that the generator code in the kernel-fitimage.bbclass currently gets
> > executed from the kernel build folder and from the u-boot build folder
> > and that the u-boot and the kernel recipes have many inter task
> > dependencies. It does not properly work with the sstate-cache. For
> > example building with empty temp means rebuilding from scratch at
> > least when also an initramfs is involved. I tried to solve this with
> > this series here
> https://patchwork.yoctoproject.org/project/oe-core/list/?series=27108.
> > It is also possible to end up with circular task dependencies by
> > setting the
> > UBOOT_* variables to SIGNED images, for example, and packing a
> > boot.txt file into the fitImage.
> >
> > The idea that seems most promising to me to solve these problems (
> > sstate, maintainability, complexity, task dependencies) is to
> > completely remove the fitimage code from the kernel and the u-boot
> > recipes and create a linux-fitimage and an uboot-fitimage recipe that
> > take the artifacts from the linux and the u-boot recipes and simply
> > put them into a fit container. The fitimage_assemble and
> uboot_fitimage_assemble functions would be reusable.
> > All the rest would probably be rewritten from scratch.
> > Some use cases in which, for example, the kernel Makefile (which can
> > run from the kernel build tree only not from a setscene folder
> > structure) generates the fitImage should simply be omitted in favor of
> > simplification.
> >
> > Having for example a linux-yocto recipe which does not support
> > building fitImages at all and a second recipe linux-yocto-fitimage
> > which depends on the linux-yocto recipe would allow several improvements:
> > - The kernel artifacts can be forwarded via sysroots from linux-yocto
> > to the linux-yocto-fitimage. Currently it goes via deploy.
> > - The kernel-yocto-fitImage class could also provide a kernel package.
> > Currently this kernel artifact is only available from the deploy folder.
> > - Standard sstate tasks (sysroot and deploy) can be used for building
> > the kernel and for building the fitImage. Doing all these steps in the
> > contecxt of one recipe leads to unusual sstated tasks which causes some
> issues.
> > - I hope the code would be much simpler. But I'm not set sure. I want
> > to try this but did not find the time so far.
> >
> > >
> > > What’s your opinion on this?  The fact that you had to add custom
> > > hooks in 2/3 seems to indicate that a more inherently flexible class
> > > would be the right approach.
> >
> > I think the two new functions uboot_fitimage_atf and
> > uboot_fitimage_tee look reasonable and are in line with the existing code.
> >
> > But the uboot_fitimage_user_image hook looks a little unusual. What's
> > the use case for that? Would it be possible to pass the its snippet as
> > a variable? Or even support the use case with a standard snippet?
> 
> The reason is ASPEED want to add our images into u-boot FIT image which are
> TSP and SSP.
> Both images are ASPEED only and they are not generic images such as TEE and
> ATF.
> Therefore, I added this hook function to make users add their ITS for specific
> need.
> Our ITS looks like as following for ASPEED latest SOCs(AST2700)
> 
> Thanks -Jamin
> 
> /dts-v1/;
> 
> / {
>     description = "U-Boot fitImage for Phosphor OpenBMC (Phosphor
> OpenBMC Project Reference Distro)/v2023.10+git/ast2700-default";
>     #address-cells = <1>;
> 
>     images {
>         uboot {
>             description = "U-Boot image";
>             data = /incbin/("u-boot-nodtb.bin");
>             type = "standalone";
>             os = "u-boot";
>             arch = "";
>             compression = "none";
>             load = <0x80000000>;
>             entry = <0x80000000>;
>         };
>         fdt {
>             description = "U-Boot FDT";
>             data = /incbin/("u-boot.dtb");
>             type = "flat_dt";
>             arch = "";
>             compression = "none";
>         };
>         tee {
>             description = "Trusted Execution Environment";
>             data = /incbin/("tee-raw.bin");
>             type = "tee";
>             arch = "";
>             os = "tee";
>             load = <0xb0080000>;
>             entry = <0xb0080000>;
>             compression = "none";
>         };
>         atf {
>             description = "ARM Trusted Firmware";
>             data = /incbin/("bl31.bin");
>             type = "firmware";
>             arch = "";
>             os = "arm-trusted-firmware";
>             load = <0xb0000000>;
>             entry = <0xb0000000>;
>             compression = "none";
>         };
>         sspfw {
>             description = "SSP Firmware";
>             data = /incbin/("ast2700-ssp.bin");
>             type = "firmware";
>             arch = "arm";
>             os = "zephyr";
>             load = <0xb2000000>;
>             entry = <0xb2000000>;
>             compression = "none";
>         };
>         tspfw {
>             description = "TSP Firmware";
>             data = /incbin/("ast2700-tsp.bin");
>             type = "firmware";
>             arch = "arm";
>             os = "zephyr";
>             load = <0xb4000000>;
>             entry = <0xb4000000>;
>             compression = "none";
>         };
>     };
> 
>     configurations {
>         default = "conf";
>         conf {
>             description = "Boot with signed U-Boot FIT";
>             loadables = "atf", "tee", "uboot" ,"sspfw" ,"tspfw";
>             fdt = "fdt";
>         };
>     };
> };
> 
> 
> >
> > All in all, I think these patches do not make it worse than it already
> > is. Solving the issues with the fitImage is a different topic.
> >
> > Regards,
> > Adrian
> >
> >
> >
> >
> > >
> > > Cheers,
> > > Ross
> > >
> > >
> > > > On 18 Nov 2024, at 06:32, Jamin Lin via lists.openembedded.org
> > > > <jamin_lin=aspeedtech.com@lists.openembedded.org> wrote:
> > > >
> > > > v0:
> > > >  1. add variable to set load address and entrypoint.
> > > >  2. Fix to install nonexistent dtb file.
> > > >  3. support to verify signed FIT
> > > >  4. support to load optee-os and TFA images
> > > > v1:
> > > >  Fix patch for code review
> > > > v2:
> > > >  created a series patch
> > > > v3:
> > > >  add cover letter
> > > > v4:
> > > >  Add oe-self testcases for reviewer suggestion
> > > >  Add documentation for reviewer suggestion.
> > > >  Link:
> > > > https://patchwork.yoctoproject.org/project/docs/patch/202411180621
> > > > 13 .269253-1-jamin_lin@aspeedtech.com/
> > > >
> > > >  The following patches had been applied from v0 changes.
> > > >  a. Fix to install nonexistent dtb file
> > > >  b. support to verify signed FIT
> > > >  c. add variable to set load address and entrypoint.
> > > >
> > > > Jamin Lin (3):
> > > >  uboot-sign: support to create TEE and ATF image tree source
> > > >  uboot-sign: support to create users specific image tree source
> > > >  oe-selftest: fitimage: add testcases to test ATF and TEE
> > > >
> > > > meta/classes-recipe/uboot-sign.bbclass   | 100 +++++++-
> > > > meta/lib/oeqa/selftest/cases/fitimage.py | 288
> > > > +++++++++++++++++++++++
> > > > 2 files changed, 387 insertions(+), 1 deletion(-)
> > > >
> > > > --
> > > > 2.25.1
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >

RE: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[Jamin Lin]

Hi Adrian, Ross

I resend v5 patch series here, https://patchwork.yoctoproject.org/project/oe-core/cover/20241211023515.2108415-1-jamin_lin@aspeedtech.com/

Thanks-Jamin

> Subject: RE: [OE-core] [PATCH v4 0/3] uboot-sign: support ATF and TFE ITS
> generation
> 

[PATCH v4 0/3] uboot-sign: support ATF and TFE ITS generation

[Jamin Lin]

v0:
  1. add variable to set load address and entrypoint.
  2. Fix to install nonexistent dtb file.
  3. support to verify signed FIT
  4. support to load optee-os and TFA images
v1:
  Fix patch for code review
v2:
  created a series patch
v3:
  add cover letter
v4:
  Add oe-self testcases for reviewer suggestion
  Add documentation for reviewer suggestion.
  Link: https://patchwork.yoctoproject.org/project/docs/patch/20241118062113.269253-1-jamin_lin@aspeedtech.com/

  The following patches had been applied from v0 changes.
  a. Fix to install nonexistent dtb file
  b. support to verify signed FIT
  c. add variable to set load address and entrypoint.
 
Jamin Lin (3):
  uboot-sign: support to create TEE and ATF image tree source
  uboot-sign: support to create users specific image tree source
  oe-selftest: fitimage: add testcases to test ATF and TEE

 meta/classes-recipe/uboot-sign.bbclass   | 100 +++++++-
 meta/lib/oeqa/selftest/cases/fitimage.py | 288 +++++++++++++++++++++++
 2 files changed, 387 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v4 2/3] uboot-sign: support to create users specific image tree source

[Jamin Lin]

Currently, uboot-sign.bbclass only supports to create Image Tree Source(ITS)
for "u-boot" and "flat_dt". However, users may want to support their private
images for specific application and purpose.

To make this bbclass more flexible and support users specific ITS, creates new
"uboot_fitimage_user_image" function. It is an empty function, so users can
create a bbappend to overwrite this function with their specific settings.

Otherwise, users need to overwrite the "uboot_fitimage_assemble" function.

Add a variable "UBOOT_FIT_USER_IMAGE" to enable users specific ITS generation
and it is disable by default.

Add a variable "UBOOT_FIT_CONF_USER_LOADABLES" to load users specific images
and it is an empty by default.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 meta/classes-recipe/uboot-sign.bbclass | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
index f1b3470127..3b61f239e3 100644
--- a/meta/classes-recipe/uboot-sign.bbclass
+++ b/meta/classes-recipe/uboot-sign.bbclass
@@ -100,6 +100,13 @@ UBOOT_FIT_ARM_TRUSTED_FIRMWARE_IMAGE ?= "bl31.bin"
 UBOOT_FIT_TEE ?= "0"
 UBOOT_FIT_TEE_IMAGE ?= "tee-raw.bin"
 
+# User specific image
+UBOOT_FIT_USER_IMAGE ?= "0"
+
+# Unit name containing a list of users additional binaries to be loaded.
+# It is a comma-separated list of strings.
+UBOOT_FIT_CONF_USER_LOADABLES ?= ''
+
 UBOOT_FIT_UBOOT_LOADADDRESS ?= "${UBOOT_LOADADDRESS}"
 UBOOT_FIT_UBOOT_ENTRYPOINT ?= "${UBOOT_ENTRYPOINT}"
 
@@ -246,6 +253,12 @@ do_uboot_generate_rsa_keys() {
 
 addtask uboot_generate_rsa_keys before do_uboot_assemble_fitimage after do_compile
 
+# Create a ITS file for the user specific image. It is an empty function and
+# users should create a bbappend to overwrite this function
+uboot_fitimage_user_image() {
+	bbwarn "Please add your specific image ITS settings for u-boot FIT image generation."
+}
+
 # Create a ITS file for the atf
 uboot_fitimage_atf() {
 	cat << EOF >> ${UBOOT_ITS}
@@ -366,6 +379,13 @@ EOF
 		conf_loadables="\"atf\", ${conf_loadables}"
 		uboot_fitimage_atf
 	fi
+
+	if [ "${UBOOT_FIT_USER_IMAGE}" = "1" ] ; then
+		if [ -n "${UBOOT_FIT_CONF_USER_LOADABLES}" ] ; then
+			conf_loadables="${conf_loadables}${UBOOT_FIT_CONF_USER_LOADABLES}"
+		fi
+		uboot_fitimage_user_image
+	fi
 	cat << EOF >> ${UBOOT_ITS}
     };
 
-- 
2.25.1