-
Notifications
You must be signed in to change notification settings - Fork 154
Multi-DTB Fitimage implementation #1274
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
base: master
Are you sure you want to change the base?
Changes from all commits
9c2532c
a13b274
dafd926
0f47ad9
329f784
cec628e
51fca9f
0a2a693
5c141cb
1c25851
9b80f00
db43c5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| inherit kernel-arch uboot-config | ||
|
|
||
| require conf/image-fitimage.conf | ||
|
|
||
| DEPENDS += "\ | ||
| u-boot-tools-native dtc-native \ | ||
| qcom-dtb-metadata \ | ||
| " | ||
|
|
||
| # Initialize root node | ||
| python fitimage_init_rootnode() { | ||
| import sys, os | ||
| file_path = d.getVar('FILE') | ||
| customfit = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(file_path))), 'lib') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use pathlib instead of this nested dirname calls. |
||
| if os.path.isdir(customfit) and customfit not in sys.path: | ||
| sys.path.insert(0, customfit) | ||
| bb.note("Added to sys.path (parse-time): %s" % customfit) | ||
| import oe.types | ||
| from qcom_fitimage import QcomItsNodeRoot | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move import lines together to the initial ones, add an empty line after. |
||
| root_node = QcomItsNodeRoot( | ||
| d.getVar("FIT_DESC"), d.getVar("FIT_ADDRESS_CELLS"), | ||
| d.getVar('HOST_PREFIX'), d.getVar('UBOOT_ARCH'), d.getVar("FIT_CONF_PREFIX"), | ||
| oe.types.boolean(d.getVar('UBOOT_SIGN_ENABLE')), d.getVar("UBOOT_SIGN_KEYDIR"), | ||
| d.getVar("UBOOT_MKIMAGE"), d.getVar("UBOOT_MKIMAGE_DTCOPTS"), | ||
| d.getVar("UBOOT_MKIMAGE_SIGN"), d.getVar("UBOOT_MKIMAGE_SIGN_ARGS"), | ||
| d.getVar('FIT_HASH_ALG'), d.getVar('FIT_SIGN_ALG'), d.getVar('FIT_PAD_ALG'), | ||
| d.getVar('UBOOT_SIGN_KEYNAME'), | ||
| oe.types.boolean(d.getVar('FIT_SIGN_INDIVIDUAL')), d.getVar('UBOOT_SIGN_IMG_KEYNAME') | ||
| ) | ||
| d.setVar("__fit_root_node", root_node) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a more standard OE var, use caps here. |
||
| bb.debug(1, "Global root_node initialized") | ||
| } | ||
|
|
||
| # Add DTB section | ||
| python fitimage_add_dtb_section() { | ||
| import shutil, os | ||
| deploydir = d.getVar('DEPLOY_DIR_IMAGE') | ||
| kerneldeploydir = d.getVar("DEPLOYDIR") | ||
| root_node = d.getVar("__fit_root_node") | ||
| if not root_node: | ||
| bb.fatal("root_node not initialized.") | ||
|
|
||
| # Pass additional options | ||
| root_node.set_extra_opts(d.getVar("UBOOT_MKIMAGE_EXTRA_OPTS")) | ||
| kernel_devicetree = d.getVar('KERNEL_DEVICETREE') | ||
|
|
||
| # Handle qcom metadata | ||
| kernel_devicetree = 'qcom-metadata.dtb ' + kernel_devicetree | ||
| shutil.copy(os.path.join(deploydir, 'qcom-metadata.dtb'), os.path.join(kerneldeploydir, 'qcom-metadata.dtb')) | ||
|
|
||
| if kernel_devicetree: | ||
| for dtb in kernel_devicetree.split(): | ||
| dtb_name = os.path.basename(dtb) | ||
| dtb_base = os.path.splitext(dtb_name)[0] | ||
| compatible = d.getVarFlag("FIT_DTB_COMPATIBLE", dtb_base) or "" | ||
| abs_path = os.path.join(deploydir, dtb_name) | ||
| root_node.fitimage_emit_section_dtb(dtb_name, dtb_name, | ||
| d.getVar("UBOOT_DTB_LOADADDRESS"), d.getVar("UBOOT_DTBO_LOADADDRESS"), True, | ||
| compatible_str=compatible, dtb_abspath=abs_path) | ||
| } | ||
|
|
||
| # Modify ITS file | ||
| python fitimage_modify_its_file() { | ||
| import re | ||
|
|
||
| deploy_dir = d.getVar("DEPLOYDIR") | ||
| itsfile = os.path.join(deploy_dir, "fit-image.its") | ||
|
|
||
| with open(itsfile, 'r') as f: | ||
| content = f.read() | ||
|
|
||
| # Replace type as qcom_metadata for qcom-dtb-metadata | ||
| content = re.sub(r'(fdt-qcom-metadata.dtb\s*\{[^}]*?)type\s*=\s*"flat_dt";', | ||
| r'\1type = "qcom_metadata";', content, flags=re.DOTALL) | ||
|
|
||
| # Remove conf-0 entry which corresponds to fdt-0 | ||
| content = re.sub(r'conf-0\s*\{[^}]*\};', '', content, flags=re.DOTALL) | ||
|
|
||
| with open(itsfile, 'w') as f: | ||
| f.write(content) | ||
| } | ||
|
|
||
| # Generate ITS and FIT image | ||
| python fitimage_generate_its() { | ||
| deploy_dir = d.getVar("DEPLOYDIR") | ||
| itsfile = os.path.join(deploy_dir, "fit-image.its") | ||
| fitname = os.path.join(deploy_dir, "fitImage") | ||
|
|
||
| bb.build.exec_func('fitimage_init_rootnode', d) | ||
| bb.build.exec_func('fitimage_add_dtb_section', d) | ||
| root_node = d.getVar("__fit_root_node") | ||
| root_node.fitimage_emit_section_config(d.getVar("FIT_CONF_DEFAULT_DTB")) | ||
| root_node.write_its_file(itsfile) | ||
| bb.build.exec_func('fitimage_modify_its_file', d) | ||
| root_node.run_mkimage_assemble(itsfile, fitname) | ||
| root_node.run_mkimage_sign(fitname) | ||
| } | ||
|
|
||
| do_compile_qcom_fitimage[depends] += "qcom-dtb-metadata:do_deploy" | ||
|
|
||
| python do_compile_qcom_fitimage() { | ||
| bb.build.exec_func('fitimage_generate_its', d) | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need to create all these functions here? It is fine to have it all under a single do_compile_qcom_fitimage function, similar to the way it is done at meta/classes-recipe/kernel-fit-image.bbclass. |
||
|
|
||
|
|
||
| do_deploy_qcom_fitimage() { | ||
| install -m 0644 "${DEPLOYDIR}/fitImage" "${DEPLOY_DIR_IMAGE}/fitImage" | ||
| install -m 0644 "${DEPLOYDIR}/fit-image.its" "${DEPLOY_DIR_IMAGE}/fit-image.its" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's how the normal kernel-fit is also deployed, pick a different name for both the binary and its. |
||
| } | ||
| addtask compile_qcom_fitimage after do_deploy | ||
| addtask deploy_qcom_fitimage after do_compile_qcom_fitimage do_deploy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,10 @@ | |
| # SPDX-License-Identifier: BSD-3-Clause-Clear | ||
| # | ||
|
|
||
| inherit dtb-fit-image | ||
|
|
||
| DTBBIN_DEPLOYDIR = "${WORKDIR}/qcom_dtbbin_deploy-${PN}" | ||
| DTBBIN_SIZE ?= "4096" | ||
| DTBBIN_SIZE ?= "65536" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will also impact the size of every dtb vfat image we generate. |
||
|
|
||
| do_qcom_dtbbin_deploy[depends] += "dosfstools-native:do_populate_sysroot mtools-native:do_populate_sysroot" | ||
| do_qcom_dtbbin_deploy[cleandirs] = "${DTBBIN_DEPLOYDIR}" | ||
|
|
@@ -21,8 +23,13 @@ do_qcom_dtbbin_deploy() { | |
| mcopy -i "${DTBBIN_DEPLOYDIR}/dtb-${dtb_base_name}-image.vfat" -vsmpQ ${DTBBIN_DEPLOYDIR}/$dtb_base_name/* ::/ | ||
| rm -rf ${DTBBIN_DEPLOYDIR}/$dtb_base_name | ||
| done | ||
| # Generate qclinux_fit.img along side combined-dtb.dtb | ||
| if [ -f "${DEPLOY_DIR_IMAGE}/fitImage" ]; then | ||
| mkfs.vfat -S ${QCOM_VFAT_SECTOR_SIZE} -C ${DTBBIN_DEPLOYDIR}/multi-dtb.vfat ${DTBBIN_SIZE} | ||
| mcopy -i "${DTBBIN_DEPLOYDIR}/multi-dtb.vfat" -vsmpQ ${DEPLOY_DIR_IMAGE}/fitImage ::/qclinux_fit.img | ||
| fi | ||
| } | ||
| addtask qcom_dtbbin_deploy after do_populate_sysroot do_packagedata before do_deploy | ||
| addtask qcom_dtbbin_deploy after do_populate_sysroot do_packagedata do_deploy do_deploy_qcom_fitimage before do_build | ||
|
|
||
| # Setup sstate, see deploy.bbclass | ||
| SSTATETASKS += "do_qcom_dtbbin_deploy" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,3 +46,16 @@ EFI_PROVIDER = "systemd-boot" | |
|
|
||
| # Unified Kernel Image (UKI) name | ||
| EFI_LINUX_IMG ?= "linux-${MACHINE}.efi" | ||
|
|
||
| # Variables to generate ITS file | ||
| UBOOT_CONFIG ??= "qcom" | ||
| UBOOT_CONFIG[qcom] ?= "qcom_defconfig" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this related to fit at all? The only thing we're using from u-boot is mkimage. |
||
|
|
||
| # Pass additional options to mkimage command | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should instead explain why the extra options are needed here. |
||
| UBOOT_MKIMAGE_EXTRA_OPTS = "-E -B 0x0001000" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? |
||
|
|
||
| # Set default configuration | ||
| FIT_CONF_DEFAULT_DTB = "1" | ||
|
|
||
| # Set hash algorithm | ||
| FIT_HASH_ALG = "" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These options will also affect users using kernel-fit (e.g. u-boot users). |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| # Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rather rename this file to be specific to DTB (e.g. qcom_dtb_fitimage or similar), as this will later confuse people since we do also expect u-boot users to leverage the traditional fit image from oe-core (making it just qcom_fit gives the impression that this is just a qcom implementation of the generic fit). |
||
| # SPDX-License-Identifier: BSD-3-Clause-Clear | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are extending fitimage.py from OE, which is GPL-2.0-only, we should align the license here. |
||
| # | ||
| # This file contains functions for Qualcomm specific DTB-only fitimage generation, | ||
| # which imports classes from OE-Core fitimage.py and enhances to meet Qualcomm FIT | ||
| # specifications. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a link to https://github.com/qualcomm-linux/qcom-dtb-metadata/blob/main/Documentation.md here. |
||
|
|
||
| import os | ||
| import shlex | ||
| import subprocess | ||
| import bb | ||
| from typing import Tuple | ||
| import oe.fitimage | ||
| from oe.fitimage import ItsNodeRootKernel, get_compatible_from_dtb | ||
|
|
||
| class QcomItsNodeRoot(ItsNodeRootKernel): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to document the Qualcomm-specific requirements for DTB-only FIT images to be able to properly review this series of patches.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually, it seems to be documented here https://github.com/qualcomm-linux/qcom-dtb-metadata/blob/main/Documentation.md. it should have been clear in the PR :)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment to this class explaining why it needs to extend ItsNodeRootKernel. |
||
|
|
||
| def __init__(self, description, address_cells, host_prefix, arch, conf_prefix, | ||
| sign_enable=False, sign_keydir=None, | ||
| mkimage=None, mkimage_dtcopts=None, | ||
| mkimage_sign=None, mkimage_sign_args=None, | ||
| hash_algo=None, sign_algo=None, pad_algo=None, | ||
| sign_keyname_conf=None, | ||
| sign_individual=False, sign_keyname_img=None): | ||
| # Call parent constructor | ||
| super().__init__(description, address_cells, host_prefix, arch, conf_prefix, | ||
| sign_enable, sign_keydir, mkimage, mkimage_dtcopts, | ||
| mkimage_sign, mkimage_sign_args, hash_algo, sign_algo, | ||
| pad_algo, sign_keyname_conf, sign_individual, sign_keyname_img) | ||
|
|
||
| self._mkimage_extra_opts = [] | ||
| self._dtbs = [] | ||
|
|
||
| def set_extra_opts(self, mkimage_extra_opts): | ||
| self._mkimage_extra_opts = shlex.split(mkimage_extra_opts) if mkimage_extra_opts else [] | ||
|
|
||
| # Override DTB section to allow compatible_override | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this a python-based function specific comment instead. |
||
| def fitimage_emit_section_dtb(self, dtb_id, dtb_path, dtb_loadaddress=None, | ||
| dtbo_loadaddress=None, add_compatible=False, | ||
| compatible_str=None, dtb_abspath=None): | ||
| load = None | ||
| dtb_ext = os.path.splitext(dtb_path)[1] | ||
| if dtb_ext == ".dtbo": | ||
| if dtbo_loadaddress: | ||
| load = dtbo_loadaddress | ||
| elif dtb_loadaddress: | ||
| load = dtb_loadaddress | ||
|
|
||
| opt_props = { | ||
| "data": '/incbin/("' + dtb_path + '")', | ||
| "arch": self._arch | ||
| } | ||
| if load: | ||
| opt_props["load"] = f"<{load}>" | ||
|
|
||
| compatibles = None | ||
| if add_compatible: | ||
| if compatible_str: | ||
| compatibles = str(compatible_str).split() | ||
| elif dtb_ext != ".dtbo": | ||
| compatibles = get_compatible_from_dtb(dtb_abspath) | ||
|
|
||
| dtb_node = self.its_add_node_dtb( | ||
| "fdt-" + dtb_id, | ||
| "Flattened Device Tree blob", | ||
| "flat_dt", | ||
| "none", | ||
| opt_props, | ||
| compatibles | ||
| ) | ||
| self._dtbs.append((dtb_node, compatibles or [])) | ||
|
|
||
| def fitimage_emit_section_config(self, default_dtb_image=None): | ||
| if self._dtbs: | ||
| counter = 0 | ||
| for entry in self._dtbs: | ||
| if isinstance(entry, tuple) and len(entry) == 2: | ||
| dtb_node, compatibles = entry | ||
| if compatibles: | ||
| for comp in compatibles: | ||
| conf_name = f"{self._conf_prefix}{counter}" | ||
| counter += 1 | ||
| self._fitimage_emit_one_section_config(conf_name, dtb_node) | ||
| conf_node = self.configurations.sub_nodes[-1] | ||
| conf_node.add_property('compatible', comp) | ||
| else: | ||
| conf_name = f"{self._conf_prefix}{counter}" | ||
| counter += 1 | ||
| self._fitimage_emit_one_section_config(conf_name, dtb_node) | ||
| else: | ||
| dtb_node = entry | ||
| conf_name = f"{self._conf_prefix}{counter}" | ||
| counter += 1 | ||
| self._fitimage_emit_one_section_config(conf_name, dtb_node) | ||
| else: | ||
| # Currently exactly one kernel is supported. | ||
| self._fitimage_emit_one_section_config(self._conf_prefix + "1") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kernel? |
||
|
|
||
| default_conf = self.configurations.sub_nodes[0].name | ||
| if default_dtb_image and self._dtbs: | ||
| default_conf = self._conf_prefix + default_dtb_image | ||
| self.configurations.add_property('default', default_conf) | ||
|
|
||
| # Override mkimage assemble to inject extra opts | ||
| def run_mkimage_assemble(self, itsfile, fitfile): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change can probably go directly to oe-core upstream, you are just adding support for extra mkimage arguments.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Waiting for https://patchwork.yoctoproject.org/project/oe-core/patch/[email protected]/ to be accepted.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, replied asking for clarification for the reason why we need to customize the options here, it is not clear even in this PR. |
||
| cmd = [self._mkimage, *self._mkimage_extra_opts, '-f', itsfile, fitfile] | ||
| if self._mkimage_dtcopts: | ||
| cmd.insert(1, '-D') | ||
| cmd.insert(2, self._mkimage_dtcopts) | ||
|
|
||
| bb.note(f"Running mkimage with extra opts: {' '.join(cmd)}") | ||
|
|
||
| try: | ||
| subprocess.run(cmd, check=True, capture_output=True) | ||
| except subprocess.CalledProcessError as e: | ||
| bb.fatal( | ||
| f"Command '{' '.join(cmd)}' failed with return code {e.returncode}\n" | ||
| f"stdout: {e.stdout.decode()}\n" | ||
| f"stderr: {e.stderr.decode()}\n" | ||
| f"itsfile: {os.path.abspath(itsfile)}" | ||
| ) | ||
|
|
||
| def get_compatible_from_dtb(dtb_path, fdtget_path="fdtget"): | ||
| compatible = None | ||
| cmd = [fdtget_path, "-t", "s", dtb_path, "/", "compatible"] | ||
| try: | ||
| ret = subprocess.run(cmd, check=True, capture_output=True, text=True) | ||
| compatible = ret.stdout.strip().split() | ||
| except subprocess.CalledProcessError: | ||
| compatible = None | ||
| return compatible | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like you just duplicated this function? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| SUMMARY = "Build qcom-metadata.dtb from vendored qcom-dtb-metadata" | ||
| HOMEPAGE = "https://github.com/qualcomm-linux/qcom-dtb-metadata" | ||
|
|
||
| LICENSE = "BSD-3-Clause-Clear" | ||
| LIC_FILES_CHKSUM = "file://LICENSE.txt;md5=2998c54c288b081076c9af987bdf4838" | ||
|
|
||
| DEPENDS = "dtc-native" | ||
|
|
||
| SRC_URI = "git://github.com/qualcomm-linux/qcom-dtb-metadata.git;branch=main;protocol=https" | ||
|
|
||
| SRCREV = "6b9e4b9c093cba36a80035af103015f0a7d3f9fc" | ||
|
|
||
| PV = "0.1+git" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This revision is exactly the v0.1 tag, no need for +git.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also rename the recipe to be specific to the version used, which is v0.1 (instead of _git). |
||
|
|
||
| inherit deploy | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| do_compile() { | ||
| oe_runmake | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need, already covered by base.bbclass. |
||
|
|
||
| do_deploy() { | ||
| install -d ${DEPLOY_DIR_IMAGE} | ||
| install -m 0644 ${S}/qcom-metadata.dtb ${DEPLOY_DIR_IMAGE}/qcom-metadata.dtb | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| addtask deploy after do_compile before do_build | ||
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.
uboot-config?