Re: [isar-cip-core][PATCH 1/7] add recipe for optee qemu arm64

[Jan Kiszka <jan.kiszka@siemens.com>]

On 19.10.22 12:44, Jan Kiszka wrote:
> On 19.10.22 11:21, sven.schultschik@siemens.com wrote:
>> From: Sven Schultschik <sven.schultschik@siemens.com>
>>
>> The recipe provides the possibility to create optee-os binaries for use inside of an qemu secureboot setup with edk2, rpmb, u-boot and uefi
>>
>> Signed-off-by: Sven Schultschik <sven.schultschik@siemens.com>
>> ---
>>  .../op-tee/optee-os-qemu-arm64_3.17.0.bb      | 57 +++++++++++++++++++
>>  recipes-bsp/u-boot/files/secure-boot.cfg.tmpl |  7 +++
>>  recipes-bsp/u-boot/u-boot-common.inc          |  6 +-
>>  3 files changed, 67 insertions(+), 3 deletions(-)
>>  create mode 100644 recipes-bsp/op-tee/optee-os-qemu-arm64_3.17.0.bb
>>
>> diff --git a/recipes-bsp/op-tee/optee-os-qemu-arm64_3.17.0.bb b/recipes-bsp/op-tee/optee-os-qemu-arm64_3.17.0.bb
>> new file mode 100644
>> index 000000000..5e60041af
>> --- /dev/null
>> +++ b/recipes-bsp/op-tee/optee-os-qemu-arm64_3.17.0.bb
>> @@ -0,0 +1,57 @@
>> +#
>> +# CIP Core, generic profile
>> +#
>> +# Copyright (c) Siemens AG, 2022
>> +#
>> +# Authors:
>> +#  Sven Schultschik <sven.schultschik@siemens.com>
>> +#
>> +# SPDX-License-Identifier: MIT
>> +#
>> +
>> +HOMEPAGE = "https://github.com/OP-TEE/optee_os"
>> +MAINTAINER = "Sven Schultschik <sven.schultschik@siemens.com>"
>> +LICENSE = "BSD-2-Clause"
>> +
>> +require recipes-bsp/optee-os/optee-os-custom.inc
>> +
>> +SRC_URI += " \
>> +            gitsm://github.com/OP-TEE/optee_os.git;branch=master;protocol=https;destsuffix=git;rev=${PV}"
> 
> Do we really need the second-class supported gitsm fetcher here? Also,
> destsuffix and rev are both redundant (they are defaults).
> 
> Oh, indention by 4 spaces would be sufficient.
> 
>> +
>> +S = "${WORKDIR}/git"
>> +
>> +OPTEE_PLATFORM = "vexpress-qemu_armv8a"
>> +
>> +OPTEE_BINARIES = "tee-header_v2.bin \
>> +    tee-pager_v2.bin \
>> +    tee-pageable_v2.bin"
>> +
>> +DEPENDS = "edk2"
>> +DEBIAN_BUILD_DEPENDS += " ,\
>> +    debhelper(>= 11~), \
>> +    build-essential, \
> 
> Redundant.
> 
>> +    cpio, \
>> +    python3-cryptography, \
>> +    python3-pycryptodome, \
>> +    python3-serial, \
>> +    device-tree-compiler, \
>> +    edk2, \
> 
> Do we need Debian's edk2 here?

Nope, self-built one -> wrong patch ordering, 2/7 must come first.

Jan

> 
>> +    gcc-arm-linux-gnueabihf,"
>> +
>> +OPTEE_EXTRA_BUILDARGS = "CFG_STMM_PATH=/usr/lib/edk2/BL32_AP_MM.fd CFG_RPMB_FS=y \
>> +        CFG_RPMB_FS_DEV_ID=0 CFG_CORE_HEAP_SIZE=524288 CFG_RPMB_WRITE_KEY=1 \
>> +        CFG_CORE_DYN_SHM=y CFG_RPMB_TESTKEY=y \
>> +        CFG_REE_FS=n\
>> +        CFG_TEE_CORE_LOG_LEVEL=1 CFG_TEE_TA_LOG_LEVEL=1 CFG_SCTLR_ALIGNMENT_CHECK=n \
>> +        CFG_ARM64_core=y CFG_CORE_ARM64_PA_BITS=48"
>> +
>> +ISAR_CROSS_COMPILE = "0"
> 
> This looks wrong, specifically as you are installing a 32-bit
> cross-compiler and calling a 64-bit one as well below.
> 
>> +
>> +dpkg_runbuild_prepend() {
>> +    # $(ARCH) is the CPU architecture to be built. 
>> +    # Currently, the only supported value is arm for 32-bit or 64-bit Armv7-A or Armv8-A. 
>> +    # Please note that contrary to the Linux kernel, $(ARCH) should not be set to arm64 for 64-bit builds. 
>> +    export ARCH="arm"
>> +    export CROSS_COMPILE32=arm-linux-gnueabihf-
>> +    export CROSS_COMPILE64=aarch64-linux-gnu-
> 
> That is a deprecated style, and sbuild will complain. Move into the
> rules file.
> 
>> +}
>> \ No newline at end of file
> 
> Please have a "newline at the end of file".
> 
>> diff --git a/recipes-bsp/u-boot/files/secure-boot.cfg.tmpl b/recipes-bsp/u-boot/files/secure-boot.cfg.tmpl
>> index 956dcbfed..8e6428238 100644
>> --- a/recipes-bsp/u-boot/files/secure-boot.cfg.tmpl
>> +++ b/recipes-bsp/u-boot/files/secure-boot.cfg.tmpl
>> @@ -4,3 +4,10 @@ CONFIG_USE_BOOTCOMMAND=y
>>  CONFIG_BOOTCOMMAND="setenv scan_dev_for_boot 'if test -e ${devtype} ${devnum}:${distro_bootpart} efi/boot/boot${EFI_ARCH}.efi; then load ${devtype} ${devnum}:${distro_bootpart} ${kernel_addr_r} efi/boot/boot${EFI_ARCH}.efi; bootefi ${kernel_addr_r} ${fdtcontroladdr}; fi'; run distro_bootcmd; echo 'EFI Boot failed!'; sleep 1000; reset"
>>  CONFIG_EFI_VARIABLES_PRESEED=y
>>  CONFIG_EFI_SECURE_BOOT=y
>> +### OPTEE config
>> +CONFIG_CMD_OPTEE_RPMB=y
>> +CONFIG_MMC=y
>> +CONFIG_SUPPORT_EMMC_RPMB=y
>> +CONFIG_TEE=y
>> +CONFIG_OPTEE=y
>> +CONFIG_EFI_MM_COMM_TEE=y
>> diff --git a/recipes-bsp/u-boot/u-boot-common.inc b/recipes-bsp/u-boot/u-boot-common.inc
>> index 60f0da361..7fe4d3fad 100644
>> --- a/recipes-bsp/u-boot/u-boot-common.inc
>> +++ b/recipes-bsp/u-boot/u-boot-common.inc
>> @@ -25,12 +25,12 @@ DEBIAN_BUILD_DEPENDS += ", libssl-dev:native, libssl-dev:${DISTRO_ARCH}"
>>  
>>  DEBIAN_BUILD_DEPENDS_append_secureboot = ", \
>>      openssl, pesign, secure-boot-secrets, python3-openssl:native"
>> -DEPENDS_append_secureboot = " secure-boot-secrets"
>> +DEPENDS_append_secureboot = " secure-boot-secrets optee-os-${MACHINE}"
> 
> Nope, this must go into qemu-specific U-Boot bits. We don't have
> optee-os for the rest yet.
> 
>>  
>>  TEMPLATE_FILES_append_secureboot = " secure-boot.cfg.tmpl"
>>  TEMPLATE_VARS_append_secureboot = " EFI_ARCH"
>>  
>>  do_prepare_build_append_secureboot() {
>>      sed -ni '/### Secure boot config/q;p' ${S}/configs/${U_BOOT_CONFIG}
>> -    cat ${WORKDIR}/secure-boot.cfg >> ${S}/configs/${U_BOOT_CONFIG}
>> -}
>> +    cat ${WORKDIR}/secure-boot.cfg >> ${S}/configs/${U_BOOT_CONFIG}    
>> +}
>> \ No newline at end of file
> 
> Spurious change. Please cross-read your diffs for such things.
> 
> When you resend, I would suggest to split up the optee recipe
> introduction and the U-Boot changes.
> 
> Jan
> 

-- 
Siemens AG, Technology
Competence Center Embedded Linux