Re: [cip-dev] [isar-cip-core][PATCH 1/8] add recipe for edk2

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

On 09.01.23 07:42, Su, Bao Cheng (DI FA CTR IPC CN PRC4) wrote:
> On Sun, 2022-11-20 at 21:47 +0100, Schultschik, Sven via lists.cip-
> project.org wrote:
>> From: Sven Schultschik <sven.schultschik@siemens.com>
>>
>> provide a recipe to create the BL32_AP_MM.fd binary by edk2 which is needed for the qemu optee generation as dependency.

Should mention the StandaloneMm for RPMB, what it is and then mention
where it will be used.

Also adjust the subject - we are not building complete EDK2.

>>
>> Signed-off-by: Sven Schultschik <sven.schultschik@siemens.com>
>> ---
>>  .../edk2/edk2-platformstandalonemmrpmb.inc    | 56 +++++++++++++++++
>>  .../edk2-platformstandalonemmrpmb_202205.bb   | 12 ++++

Recipe name became too unreadable. "edk2-standalonemm-rpmb" maybe? By
convention, we don't use camel-cases in recipes, so this cannot help
with separating words.

>>  recipes-bsp/edk2/files/rules.tmpl             | 61 +++++++++++++++++++
>>  3 files changed, 129 insertions(+)
>>  create mode 100644 recipes-bsp/edk2/edk2-platformstandalonemmrpmb.inc
>>  create mode 100644 recipes-bsp/edk2/edk2-platformstandalonemmrpmb_202205.bb
>>  create mode 100755 recipes-bsp/edk2/files/rules.tmpl
>>
>> diff --git a/recipes-bsp/edk2/edk2-platformstandalonemmrpmb.inc b/recipes-bsp/edk2/edk2-platformstandalonemmrpmb.inc
>> new file mode 100644
>> index 0000000..3277cc8
>> --- /dev/null
>> +++ b/recipes-bsp/edk2/edk2-platformstandalonemmrpmb.inc
>> @@ -0,0 +1,56 @@
>> +#
>> +# CIP Core, generic profile
>> +#
>> +# Copyright (c) Siemens AG, 2022
>> +#
>> +# Authors:
>> +#  Sven Schultschik <sven.schultschik@siemens.com>
>> +#
>> +# SPDX-License-Identifier: MIT
>> +#
>> +
>> +HOMEPAGE = "https://github.com/tianocore/edk2"
>> +MAINTAINER = "Sven Schultschik <sven.schultschik@siemens.com>"
>> +LICENSE = "BSD-2-Clause-Patent"

We don't track licenses this way, so better drop this line.

>> +
>> +inherit dpkg
>> +
>> +SRC_URI = "https://github.com/tianocore/edk2/archive/refs/tags/edk2-stable${PV}.tar.gz;name=edk2 \
>> +    https://github.com/google/brotli/archive/${SRC_REV_brotli}.tar.gz;name=brotli \
>> +    https://github.com/openssl/openssl/archive/refs/tags/${SRC_REV_openssl}.tar.gz;name=openssl \

Can we use the Debian sources here? Would avoid separate version
tracking (SVM, OSS license checking...).

>> +    git://github.com/tianocore/edk2-platforms.git;protocol=https;destsuffix=git/edk2-platforms;name=edk2-platforms \

Why not using an archive here as well? Even works when there is no
release tag.

>> +    file://rules.tmpl \
>> +    "
>> +SRC_URI[edk2.sha256sum] = "e6cf93bae78b30a10732b8afb5cc438735dc9ec976ae65d12dab041c18bb7987"
>> +SRC_URI[brotli.sha256sum] = "6d6cacce05086b7debe75127415ff9c3661849f564fe2f5f3b0383d48aa4ed77"
>> +SRC_URI[openssl.sha256sum] = "6b2d2440ced8c802aaa61475919f0870ec556694c466ebea460e35ea2b14839e"
>> +
>> +SRC_REV_brotli          = "f4153a09f87cbb9c826d8fc12c74642bb2d879ea"
>> +SRC_REV_openssl         = "OpenSSL_1_1_1n"
>> +SRCREV_edk2-platforms   = "3b896d1a325686de3942723c42f286090453e37a"

Would be good to clarify how these revisions were selected. Do they come
from the EDK2 build manifest?

>> +
>> +S = "${WORKDIR}/git"
>> +
>> +DEBIAN_BUILD_DEPENDS = "python3:native, dh-python, uuid-dev:native"
>> +
>> +EDK2_BINARIES ?= "Build/MmStandaloneRpmb/RELEASE_GCC5/FV/BL32_AP_MM.fd"
>> +
>> +BUILD_DEPENDS   += ""

Useless line.

>> +
>> +TEMPLATE_FILES = "rules.tmpl"

Which variables are you actually replacing?

>> +
>> +do_prepare_build() {
>> +    deb_debianize
>> +
>> +    mkdir -p ${S}/edk2
>> +    cp -a ${WORKDIR}/edk2-edk2-stable${PV}/* "${S}/edk2/"
>> +    cp -a ${WORKDIR}/brotli-${SRC_REV_brotli}/* "${S}/edk2/BaseTools/Source/C/BrotliCompress/brotli"
>> +    cp -a ${WORKDIR}/brotli-${SRC_REV_brotli}/* "${S}/edk2/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli"
>> +    cp -a ${WORKDIR}/openssl-${SRC_REV_openssl}/* "${S}/edk2/CryptoPkg/Library/OpensslLib/openssl"
>> +
>> +    rm -f ${S}/debian/edk2.install
>> +    for binary in ${EDK2_BINARIES}; do
>> +        echo "$binary /usr/lib/edk2/" >> \
>> +            ${S}/debian/edk2-platformstandalonemmrpmb.install
>> +    done
>> +}
>> diff --git a/recipes-bsp/edk2/edk2-platformstandalonemmrpmb_202205.bb b/recipes-bsp/edk2/edk2-platformstandalonemmrpmb_202205.bb
>> new file mode 100644
>> index 0000000..84761c9
>> --- /dev/null
>> +++ b/recipes-bsp/edk2/edk2-platformstandalonemmrpmb_202205.bb
> 
> 2022.05 is a bit older version, the latest ones are 2022.08 and even
> 2022.11.
> 
> Not sure if any security updates between these versions, but I would
> blindly use the newer version.

Ack.

> 
> - Baocheng
> 
>> @@ -0,0 +1,12 @@
>> +#
>> +# CIP Core, generic profile
>> +#
>> +# Copyright (c) Siemens AG, 2022
>> +#
>> +# Authors:
>> +#  Sven Schultschik <sven.schultschik@siemens.com>
>> +#
>> +# SPDX-License-Identifier: MIT
>> +#
>> +
>> +require edk2-platformstandalonemmrpmb.inc
> 
> Sorry, after some rethinking, I find maybe it's not a good idea to have
> the multiple version supportive for EDK2, since:
> 
> 1. EDK2 has too many sub-components, if multi version need be supported,
> somebody must have to maintain a list of these sub-components for each
> version, which could be a not easy job. Also, it's hard to notice if the
> wrong versions of some sub-components are selected. So it would increase
> the chance of making mistake.
> 
> 2. Unlike kernel, there should be no real requirement of using different
> version of EDK2 stmm for different products/projects, from security
> point of view, the latest security version should always be used.
> 
> 3. Best practice, per my understanding, is we have an unified EDK2-stmm
> version in cip-core and downstreams just use it. if new version is
> required, then upsteam it into cip-core.
> 
> - Baocheng
> 
>> diff --git a/recipes-bsp/edk2/files/rules.tmpl b/recipes-bsp/edk2/files/rules.tmpl
>> new file mode 100755
>> index 0000000..388e49a
>> --- /dev/null
>> +++ b/recipes-bsp/edk2/files/rules.tmpl
>> @@ -0,0 +1,61 @@
>> +#!/usr/bin/make -f
>> +#
>> +# Copyright (c) Siemens AG, 2022
>> +#
>> +# SPDX-License-Identifier: MIT
>> +
>> +ifneq ($(DEB_BUILD_GNU_TYPE),$(DEB_HOST_GNU_TYPE))
>> +export CROSS_COMPILE=$(DEB_HOST_GNU_TYPE)-
>> +endif
>> +
>> +export WORKSPACE=$(shell pwd)
>> +export PACKAGES_PATH=$(WORKSPACE)/edk2:$(WORKSPACE)/edk2-platforms
>> +export ACTIVE_PLATFORM="Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc"
>> +
>> +# https://github.com/tianocore/edk2-platforms/blob/master/Readme.md#if-cross-compiling
>> +ifeq (arm64,$(DEB_TARGET_ARCH))
>> +export TARGET_ARCH = 'AARCH64'
>> +else ifeq ((armhf,$(DEB_TARGET_ARCH))
>> +export TARGET_ARCH = 'ARM'
>> +else ifeq ((amd64,$(DEB_TARGET_ARCH))
>> +export TARGET_ARCH = 'X64'
>> +else ifeq ((i386,$(DEB_TARGET_ARCH))
>> +export TARGET_ARCH = 'IA32'
>> +else
>> +$(error DEB_TARGET_ARCH $(DEB_TARGET_ARCH) unsupported)
>> +endif

Maybe a blank line here to make things more readable.

>> +# When cross-compiling, or building with a different version of the compiler than
>> +# the default `gcc`, we additionally need to inform the
>> +# build command which toolchain to use. We do this by setting the environment
>> +# variable `{TOOL_CHAIN_TAG}_{TARGET_ARCH}_PREFIX` - in the case above,
>> +# **GCC5_AARCH64_PREFIX**.
>> +# export GCC5_AARCH64_PREFIX=aarch64-linux-gnu-
>> +# using export here at TOP Level does not work, because
>> +# GCC5_$(TARGET_ARCH)_PREFIX gets deleted again for what reason ever
>> +# Therefore it is set right before the build command
>> +# export GCC5_$(TARGET_ARCH)_PREFIX=$(DEB_HOST_GNU_TYPE)-

Context? This refers to something done much more below.

>> +
>> +

One blank line too much.

>> +export SHELL=/bin/bash

Needed?

>> +
>> +# ENV Vars which should get set by edksetup.sh
>> +export PYTHON_COMMAND=python3
>> +export PYTHONHASHSEED=1
>> +export CONF_PATH=$(WORKSPACE)/edk2/Conf
>> +export EDK_TOOLS_PATH=$(WORKSPACE)/edk2/BaseTools
>> +export PATH=$(WORKSPACE)/edk2/BaseTools/Bin/Linux-$(TARGET_ARCH):$(WORKSPACE)/edk2/BaseTools/BinWrappers/PosixLike::/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
>> +
>> +override_dh_auto_build:
>> +source edk2/edksetup.sh --reconfig
>> +
>> +CFLAGS= LDFLAGS=  make -C edk2/BaseTools
>> +
>> +(export GCC5_$(TARGET_ARCH)_PREFIX=$(DEB_HOST_GNU_TYPE)- && \
>> +build -p $(ACTIVE_PLATFORM) -b RELEASE -a $(TARGET_ARCH) -t GCC5 -n $(shell nproc))
>> +
>> +override_dh_auto_install:
>> +
>> +override_dh_auto_test:
>> +
>> +%:
>> +dh $@ --no-parallel

I've discussed with Baocheng to pull this in earlier. Unfortunately, the
situation for QEMU is still ugly, and I do not like that U-Boot hack
which does not allow us to truly explore EFI vars in RPMB (even more if
kernel patches will show up soon that allow using those via the normal
kernel APIs).

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux