-
Notifications
You must be signed in to change notification settings - Fork 153
Rb3gen2: Add support for Open Boot firmware (TF-A, OP-TEE and U-Boot) build #1172
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 6 commits
074bc4f
297f464
d25d38f
aa97c88
beff201
5b04d7e
2376dc9
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,8 @@ | ||
| # yaml-language-server: $schema=https://raw.githubusercontent.com/siemens/kas/master/kas/schema-kas.json | ||
|
|
||
| header: | ||
| version: 14 | ||
|
|
||
| local_conf_header: | ||
| firmware: | | ||
| MACHINE_FEATURES:qcm6490:append = " tfa optee u-boot" | ||
|
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 don't know if it's a functional difference or not.. but we usually put the :append or :prepend first in the list of overrides.
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 don't like changing the MACHINE config in the KAS config file. I would rather have a difference machine, what do you think?
Member
Author
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 don't think having a different machine makes sense here. Since here we are building a different boot firmware (open vs closed source) for the same machine like we do for supporting different kernel versions for the same machine.
Sure, I will fix that.
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.
It is: I also don't like the idea of changing MACHINE_FEATURES here. I'd prefer if we have enabled MACHINE_FEATURES for all machines that support TF-A and then used
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 like the COMBINED_FEATURES.
Member
Author
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. It feels like we are moving in a direction to build open boot firmware and package closed boot firmware during a common build like for RB3Gen2 in this case. The build artifacts will contain both but what is present in the device flashing tarball is based on |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ BBFILE_COLLECTIONS += "qcom" | |
| BBFILE_PATTERN_qcom := "^${LAYERDIR}/" | ||
| BBFILE_PRIORITY_qcom = "6" | ||
|
|
||
| LAYERDEPENDS_qcom = "core" | ||
| LAYERDEPENDS_qcom = "core meta-arm" | ||
|
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. Well... If OP-TEE is not a required feature, this should be handled via dynamic layers rather than layer dependency
Member
Author
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 please, boot firmware is an essential component of any Yocto BSP layer. The fact that meta-qcom uses proprietary boot firmware blobs is unfortunate. Here, meta-arm is providing the common recipes for TF-A, OP-TEE as well as edk2 for Arm based platforms is an essential dependency as we are starting to support open boot stack on Qcom platforms. Have a look at meta-ti for your reference here [1]. [1] https://git.yoctoproject.org/meta-ti/tree/meta-ti-bsp/conf/layer.conf
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'd still ask it to be handled via a dynamic layer inside meta-qcom. We try to depend only on OE-Core, however we use other layers to provide extended functionality.
Member
Author
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. Sorry but this isn't extended functionality but an essential function of the BSP layer to allow building boot firmware from source.
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. At this point, the basic functionality is to use the blobs. I welcome the usage of OS components and OP-TEE, but I don't want to enforce that dependency on everybody.
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.
At this point OP-TEE & TF-A - based stack:
Having all of that in mind, as I wrote, I'd welcome the introduction of TF-A & OP-TEE based boot stack (and documenting that in the README), but I don't want to make everybody pull in that dependency. When it's decided that the open boot stack is a default option for any of the targets, we can flip it.
Member
Author
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.
What do you mean by default here? One that is enforced by Qualcomm for every OEM? Or rather we give OEMs the choice to select among open vs closed boot firmware stacks. There are already various IoT and Auto OEMs asking for the open boot firmware stack.
As I said this is being worked upon to enable it outside QTI. Also, the next step for me is to add support for upstream edk2 whose recipes are also currently provided by meta-arm which will work out of the box as of now.
As I said in the PR itself, the plan is to support Kodiak and Lemans initially, but it will be scalable to other SoCs too with less effort.
Sooner or later, we need to add that dependency as otherwise how can we enable OEMs to build open boot stack in the same build environment.
As I said above, we should really be offering a choice for OEMs to build open boot stacks. And I don't understand when you would feel it's the right time to flip. You need to really understand; the open boot firmware stack is an essential feature of any open source Yocto BSP layer which people expect to be available by default.
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.
OEM devices are not supported by this layer. Inside this layer we provide generic code and also support for Qualcomm-manufactured devices. I know that there are use-cases which require OP-TEE and TF-A. But I really don't understand what you are arguing here about.
That doesn't change the current status of these changes.
That's perfect and really interesting.
Great.
As I wrote, I don't understand, what we are arguing here. Please move your changes under
Member
Author
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 think here the main point of argument is to keep meta-arm layer in the dependency list of meta-qcom since we need to reuse the BSP recipes for TF-A, OP-TEE and edk2. Let others also chime in about what they think here. I think I have made my arguments here.
The TF-A, OP-TEE and edk2 changes belong to the Qcom BSP layer and not to the Arm BSP layer. I will reiterate the example for meta-ti here.
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.
|
||
| LAYERRECOMMENDS_qcom = "openembedded-layer" | ||
| LAYERSERIES_COMPAT_qcom = "whinlatter" | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,13 +13,19 @@ KERNEL_DEVICETREE ?= " \ | |
| qcom/qcs6490-rb3gen2-vision-mezzanine.dtb \ | ||
| " | ||
|
|
||
| EXTRA_IMAGEDEPENDS += "${@bb.utils.contains('MACHINE_FEATURES', 'tfa', 'trusted-firmware-a', '', 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. is tfa a new MACHINE_FEATURE?
Member
Author
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. Yeah, it is a new machine feature since you asked for it in previous review.
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 should document them, somehow, somewhere..
Member
Author
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. Okay, I can document them here |
||
| PREFERRED_PROVIDER_virtual/bootloader = "${@bb.utils.contains('MACHINE_FEATURES','u-boot','u-boot','',d)}" | ||
| TFA_UBOOT:pn-trusted-firmware-a = "${@bb.utils.contains('MACHINE_FEATURES','u-boot','1','0',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. It feels strange at a first glance that machine configuration (which should also define MACHINE_FEATURES) also depends on them. Please add a comment here, how is it expected to be enabled. |
||
|
|
||
| MACHINE_ESSENTIAL_EXTRA_RRECOMMENDS += " \ | ||
| packagegroup-rb3gen2-firmware \ | ||
| packagegroup-rb3gen2-hexagon-dsp-binaries \ | ||
| packagegroup-optee \ | ||
|
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 probably don't want when optee is not enabled/available. no?
Member
Author
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 added checks for "optee" as a machine feature within the "packagegroup-optee" since yocto-check-layers complained about it. Adding a machine feature check here felt redundant but I can add it if you want.
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 see. I think it should be the other way around. packagegroup-optee should include them always, and we should include the packagegroup here only when optee is enabled.
Member
Author
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 did that initially, but yocto-check-layers complained about "packagegroup-optee" as an independent entity since a machine compatible is required to build
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 figure out if this is the right thing to do..
Member
Author
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. Okay, let me add machine check here as well for consistency.
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. Please keep the list sorted |
||
| " | ||
|
|
||
| QCOM_CDT_FILE = "cdt_core_kit" | ||
| QCOM_BOOT_FILES_SUBDIR = "qcm6490" | ||
| QCOM_PARTITION_FILES_SUBDIR ?= "partitions/qcs6490-rb3gen2/ufs" | ||
|
|
||
| QCOM_BOOT_FIRMWARE = "firmware-qcom-boot-qcs6490" | ||
| UBOOT_MACHINE = "qcm6490_defconfig" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| SRC_URI += "https://github.com/coreboot/qc_blobs/raw/refs/heads/master/sc7280/qtiseclib/libqtisec.a;name=qtiseclib" | ||
|
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. What is the licence for the blob? Does it change the licence of the generated binary?
Member
Author
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. The license for this blob can be found here: https://github.com/coreboot/qc_blobs/blob/master/sc7280/qtiseclib/LICENSE.
AFAIK, it doesn't change the overall TF-A project license but rather it is required for the binary components to be compliant with the TF-A license terms, see here: https://trustedfirmware-a.readthedocs.io/en/latest/process/contributing.html#binary-components
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. Still the licence should be included in SBOM, etc., so it should be properly annotated.
|
||
| SRC_URI[qtiseclib.sha256sum] = "6860dda0701c8709530608cc0e5a61b76484ae16cb673ba9a23510cf4b3d57bf" | ||
|
|
||
| DEPENDS += "optee-os" | ||
|
|
||
| COMPATIBLE_MACHINE = "qcm6490" | ||
| TFA_PLATFORM = "rb3gen2" | ||
| TFA_BUILD_TARGET = "bl2 fip" | ||
| TFA_SPD = "opteed" | ||
| EXTRA_OEMAKE:append = " \ | ||
| QTISECLIB_PATH=${UNPACKDIR}/libqtisec.a \ | ||
| BL32=${RECIPE_SYSROOT}/${nonarch_base_libdir}/firmware/tee-raw.bin \ | ||
|
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. If you change the order of the commits, you won't have to change the file that you have just added.
Member
Author
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 commit adds OP-TEE support to the TF-A recipe added by previous commit. So, I am not sure what do you meant by reordering commits here.
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.
|
||
| " | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| SRCBRANCH = "qcom-next" | ||
| SRC_URI = "git://github.com/qualcomm-linux/trusted-firmware-a.git;protocol=https;name=tfa;branch=${SRCBRANCH}" | ||
| SRCREV_tfa = "164d443c8e92c18fd2cca03661dd1a2b9ac37848" | ||
|
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 breaks the idea of the orignial TF-A recipes. Inside meta-arm there are 4 different versions of TF-A (2.10.17, 2.12.3, 2.13.0, git aka tip). Machine-specific includes don't change the URI and the SRCREV. In other words: if the user sets
Member
Author
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 a fair point, I will add proper versioned recipes instead. |
||
| LIC_FILES_CHKSUM = "file://docs/license.rst;md5=6ed7bace7b0bc63021c6eba7b524039e" | ||
|
|
||
| DEPENDS += "qtestsign-native" | ||
|
|
||
| MACHINE_TFA_QCOM_REQUIRE ?= "" | ||
| MACHINE_TFA_QCOM_REQUIRE:qcm6490 = "trusted-firmware-a-qcm6490.inc" | ||
|
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. Will it work on other qcm6490-based machines (e.g. IDP, Tachyon or RubkPi)?
Member
Author
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. Yeah, the TF-A/OP-TEE port is specific to the SoC. However, we need to make sure that the OEM hardware is running a QLI based boot firmware release where we can replace boot chain components with open-source ones. BTW, I have been waiting to get my hands-on with one of these boards as well to run open boot stack. |
||
|
|
||
| require ${MACHINE_TFA_QCOM_REQUIRE} | ||
|
|
||
| do_install:append:qcm6490() { | ||
| export CRYPTOGRAPHY_OPENSSL_NO_LEGACY=1 | ||
|
|
||
| ${OBJCOPY} -I binary -B aarch64 -O elf64-littleaarch64 ${D}/firmware/fip.bin ${D}/firmware/fip.o | ||
| ${LD} ${D}/firmware/fip.o -o ${D}/firmware/fip_unsigned.elf -EL -T ${S}/tools/qti/fip-elf.lds --defsym=ELFENTRY=0x9fc00000 -Ttext=0x9fc00000 | ||
| rm -f ${D}/firmware/fip.o | ||
|
|
||
| qtestsign -v6 aboot -o ${D}/firmware/fip.elf ${D}/firmware/fip_unsigned.elf | ||
| rm -f ${D}/firmware/fip_unsigned.elf | ||
| } | ||
b49020 marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| MACHINE_TFA_REQUIRE ?= "" | ||
| MACHINE_TFA_REQUIRE:qcom = "trusted-firmware-a-qcom.inc" | ||
|
|
||
| require ${MACHINE_TFA_REQUIRE} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| From d71a6fdb741455cf669f31fd4596f2ec4c757ebe Mon Sep 17 00:00:00 2001 | ||
| From: Sumit Garg <[email protected]> | ||
| Date: Fri, 31 Oct 2025 11:10:19 +0530 | ||
| Subject: [PATCH] dts: qcs6490-rb3gen2-u-boot: Add OP-TEE node | ||
|
|
||
| Since we currently only support DT based OP-TEE driver probe, lets add the | ||
| OP-TEE node here for the time being. In future we want to migrate OP-TEE | ||
| probing over to FF-A bus and then we can drop this DT node. | ||
|
|
||
| Signed-off-by: Sumit Garg <[email protected]> | ||
| Upstream-Status: Inappropriate [temporary workaround] | ||
| --- | ||
| arch/arm/dts/qcs6490-rb3gen2-u-boot.dtsi | 7 +++++++ | ||
| 1 file changed, 7 insertions(+) | ||
|
|
||
| diff --git a/arch/arm/dts/qcs6490-rb3gen2-u-boot.dtsi b/arch/arm/dts/qcs6490-rb3gen2-u-boot.dtsi | ||
| index fbe72595f5a..e60905cb6a2 100644 | ||
| --- a/arch/arm/dts/qcs6490-rb3gen2-u-boot.dtsi | ||
| +++ b/arch/arm/dts/qcs6490-rb3gen2-u-boot.dtsi | ||
| @@ -15,6 +15,13 @@ | ||
| <0 0xC3400000 0 0x3CC00000>, | ||
| <1 0x00000000 1 0x00000000>; | ||
| }; | ||
| + | ||
| + firmware { | ||
| + optee { | ||
| + compatible = "linaro,optee-tz"; | ||
| + method = "smc"; | ||
|
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 node should only be enabled if we actually provide OP-TEE for the platform
Member
Author
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. Okay, I can add the OP-TEE machine feature check for this patch. |
||
| + }; | ||
| + }; | ||
| }; | ||
|
|
||
| &usb_1_dwc3 { | ||
| -- | ||
| 2.48.1 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # RB3Gen2 specific OP-TEE support | ||
|
|
||
| require optee-os-qcom.inc | ||
|
|
||
| COMPATIBLE_MACHINE = "qcm6490" | ||
| OPTEEMACHINE = "qcom-kodiak" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| SRCBRANCH = "qcom-next" | ||
|
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. inline |
||
| SRC_URI = "git://github.com/qualcomm-linux/optee_os.git;protocol=https;name=optee;branch=${SRCBRANCH}" | ||
| SRCREV_optee = "a427f12bc60e74ccdd2ae771b50ed237397a0782" | ||
|
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. The same question. We rewrite SRC_URI and SRCREV here. It breaks user assumptions. |
||
b49020 marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| MACHINE_OPTEE_OS_TADEVKIT_REQUIRE ?= "" | ||
| MACHINE_OPTEE_OS_TADEVKIT_REQUIRE:qcm6490 = "optee-os-qcm6490.inc" | ||
|
|
||
| require ${MACHINE_OPTEE_OS_TADEVKIT_REQUIRE} |
b49020 marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| MACHINE_OPTEE_OS_REQUIRE ?= "" | ||
| MACHINE_OPTEE_OS_REQUIRE:qcm6490 = "optee-os-qcm6490.inc" | ||
|
|
||
| require ${MACHINE_OPTEE_OS_REQUIRE} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # qcm6490 specific configuration | ||
|
|
||
| COMPATIBLE_MACHINE = "qcm6490" | ||
ricardosalveti marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # Machine specific configurations | ||
|
|
||
| MACHINE_OPTEE_TEST_REQUIRE ?= "" | ||
| MACHINE_OPTEE_TEST_REQUIRE:qcm6490 = "optee-test-qcm6490.inc" | ||
|
|
||
| require ${MACHINE_OPTEE_TEST_REQUIRE} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| SUMMARY = "Packages for the OP-TEE support" | ||
|
|
||
| inherit packagegroup | ||
|
|
||
| RRECOMMENDS:${PN} = " \ | ||
| ${@bb.utils.contains('MACHINE_FEATURES', 'optee', 'optee-client', '', d)} \ | ||
| ${@bb.utils.contains('MACHINE_FEATURES', 'optee', 'optee-test', '', d)} \ | ||
| ${@bb.utils.contains('MACHINE_FEATURES', 'optee', 'optee-os-ta', '', 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. Why do we need three lines here?
Member
Author
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. Okay, I will convert them in a single line. |
||
| " | ||
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.
So, building has beek broken for all previous commits. That's not nice
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.
That's a fair point, I will move this diff to first patch instead.