[PATCH v2 0/6] improving OP-TEE support for u-boot-based platforms

[PATCH v2 0/6] improving OP-TEE support for u-boot-based platforms

[Yann Dirson]

From: Yann Dirson <yann@blade-group.com>

While working on integrating OP-TEE on rk3399 with a readonly rootfs,
I figured that a couple of things would be good to have in the optee
recipes.

Also, having started to work on old meta-linaro/meta-optee initially:
 - I had to duplicate some of the work done here and had submitted a
   similar patch upstream
 - pycrypto simplification became obvious

Changes from v1:
 - Denys' Reviewed-by in patch 1
 - include tee.elf in do_install rather than do_deploy in patch 2, as suggested
   by Denys (but wondering whether optee-os-dbg would not be the suitable
   place for all of those files, rather than the main optee-os package)
 - simpler downstream example in patch 3 commit message (thx Denys for
   the suggestion)
 - patch 4 completely replaced as suggested by Ricardo
 - new patches 5 and 6 trying to clarify and cleanup LOG_LEVEL issues

Yann Dirson (6):
  arm/optee: use pycryptodome rather pycryptodomex
  arm/optee-os: mark LDFLAGS patch as submitted
  arm/optee-os: deploy tee.elf for u-boot
  optee-client: use /var/lib/tee instead of /data/tee for persistent
    storage
  optee-os: don't override upstream TA_LOG_LEVEL setting
  RFC: optee-os: provide a "debug" PACKAGECONFIG flag

 meta-arm/recipes-security/optee/optee-client.inc         | 5 ++++-
 meta-arm/recipes-security/optee/optee-examples.inc       | 2 +-
 .../0001-make-Pass-ldflags-during-link.patch             | 2 +-
 meta-arm/recipes-security/optee/optee-os.inc             | 9 ++++++---
 meta-arm/recipes-security/optee/optee-test.inc           | 2 +-
 5 files changed, 13 insertions(+), 7 deletions(-)

-- 
2.30.2

[PATCH v2 1/6] arm/optee: use pycryptodome rather pycryptodomex

[Yann Dirson]

From: Yann Dirson <yann@blade-group.com>

Recent versions of OP-TEE perfectly build with pycryptodome, no need for
the pycryptodomex transition package any more.

Signed-off-by: Yann Dirson <yann@blade-group.com>
Reviewed-by: Denys Dmytriyenko <denis@denix.org>
---
 meta-arm/recipes-security/optee/optee-examples.inc | 2 +-
 meta-arm/recipes-security/optee/optee-os.inc       | 2 +-
 meta-arm/recipes-security/optee/optee-test.inc     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta-arm/recipes-security/optee/optee-examples.inc b/meta-arm/recipes-security/optee/optee-examples.inc
index 851043b..81c31bc 100644
--- a/meta-arm/recipes-security/optee/optee-examples.inc
+++ b/meta-arm/recipes-security/optee/optee-examples.inc
@@ -5,7 +5,7 @@ HOMEPAGE = "https://github.com/linaro-swg/optee_examples"
 LICENSE = "BSD-2-Clause"
 LIC_FILES_CHKSUM = "file://LICENSE;md5=cd95ab417e23b94f381dafc453d70c30"
 
-DEPENDS = "optee-client optee-os python3-pycryptodomex-native"
+DEPENDS = "optee-client optee-os python3-pycryptodome-native"
 
 inherit python3native
 
diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
index f8697cf..34f586a 100644
--- a/meta-arm/recipes-security/optee/optee-os.inc
+++ b/meta-arm/recipes-security/optee/optee-os.inc
@@ -10,7 +10,7 @@ require optee.inc
 
 CVE_PRODUCT = "linaro:op-tee op-tee:op-tee_os"
 
-DEPENDS = "python3-pycryptodome-native python3-pycryptodomex-native python3-pyelftools-native"
+DEPENDS = "python3-pycryptodome-native python3-pyelftools-native"
 
 DEPENDS_append_toolchain-clang = " compiler-rt"
 
diff --git a/meta-arm/recipes-security/optee/optee-test.inc b/meta-arm/recipes-security/optee/optee-test.inc
index 877bdeb..f09b9d2 100644
--- a/meta-arm/recipes-security/optee/optee-test.inc
+++ b/meta-arm/recipes-security/optee/optee-test.inc
@@ -13,7 +13,7 @@ require optee.inc
 # When upgraded we should be able to remove this limitation
 COMPATIBLE_HOST_libc-musl = 'null'
 
-DEPENDS = "optee-client optee-os python3-pycryptodomex-native"
+DEPENDS = "optee-client optee-os python3-pycryptodome-native"
 
 SRC_URI = "git://github.com/OP-TEE/optee_test.git \
            file://0001-host-xtest-Adjust-order-of-including-compiler.h.patch \
-- 
2.30.2

[PATCH v2 2/6] arm/optee-os: mark LDFLAGS patch as submitted

[Yann Dirson]

From: Yann Dirson <yann@blade-group.com>

Signed-off-by: Yann Dirson <yann@blade-group.com>
---
 .../optee-examples/0001-make-Pass-ldflags-during-link.patch     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta-arm/recipes-security/optee/optee-examples/0001-make-Pass-ldflags-during-link.patch b/meta-arm/recipes-security/optee/optee-examples/0001-make-Pass-ldflags-during-link.patch
index 0894a7a..84202ef 100644
--- a/meta-arm/recipes-security/optee/optee-examples/0001-make-Pass-ldflags-during-link.patch
+++ b/meta-arm/recipes-security/optee/optee-examples/0001-make-Pass-ldflags-during-link.patch
@@ -8,7 +8,7 @@ for gnu_hash among others which sets the linking straight
 using LDFLAGS varible here means, we can affect the linker flags
 from build environment
 
-Upstream-Status: Pending
+Upstream-Status: Submitted [https://github.com/linaro-swg/optee_examples/pull/85]
 
 Signed-off-by: Khem Raj <raj.khem@gmail.com>
 ---
-- 
2.30.2

[PATCH v2 3/6] arm/optee-os: deploy tee.elf for u-boot

[Yann Dirson]

From: Yann Dirson <yann@blade-group.com>

Integration of OP-TEE in the u-boot image is done with `TEE=tee.elf`,
so this build artifact must be exported.

U-boot integration can now be done in a BSP layer with something like
this in `u-boot_%.bbappend`:

 # OP-TEE, dependent on "optee" DISTRO_FEATURES

 OPTEE_OEMAKE ?= " TEE=${DEPLOY_DIR_IMAGE}/optee/tee.elf"

 EXTRA_OEMAKE_append = " ${PACKAGECONFIG_CONFARGS}"
 PACKAGECONFIG[optee] = "${OPTEE_OEMAKE},,optee-os"
 PACKAGECONFIG_append = " ${@bb.utils.filter('DISTRO_FEATURES', 'optee', d)}"

Signed-off-by: Yann Dirson <yann@blade-group.com>
---
 meta-arm/recipes-security/optee/optee-os.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
index 34f586a..08cc2f0 100644
--- a/meta-arm/recipes-security/optee/optee-os.inc
+++ b/meta-arm/recipes-security/optee/optee-os.inc
@@ -45,7 +45,7 @@ do_compile[cleandirs] = "${B}"
 do_install() {
     #install core in firmware
     install -d ${D}${nonarch_base_libdir}/firmware/
-    install -m 644 ${B}/core/*.bin ${D}${nonarch_base_libdir}/firmware/
+    install -m 644 ${B}/core/*.bin ${B}/core/tee.elf ${D}${nonarch_base_libdir}/firmware/
 
     #install TA devkit
     install -d ${D}${includedir}/optee/export-user_ta/
-- 
2.30.2

[PATCH v2 4/6] optee-client: use /var/lib/tee instead of /data/tee for persistent storage

[Yann Dirson]

From: Yann Dirson <yann@blade-group.com>

This is more FHS-conformant, and allows tee-supplicant to work on a
readonly rootfs setup (although then it uses volatile storage by
default, and it is still a matter of distro policy how to arrange for
that storage to be persistent in a readonly setup).

Signed-off-by: Yann Dirson <yann@blade-group.com>
Suggested-by: Ricardo Salveti <ricardo@foundries.io>
---
 meta-arm/recipes-security/optee/optee-client.inc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/meta-arm/recipes-security/optee/optee-client.inc b/meta-arm/recipes-security/optee/optee-client.inc
index f697386..65c9a44 100644
--- a/meta-arm/recipes-security/optee/optee-client.inc
+++ b/meta-arm/recipes-security/optee/optee-client.inc
@@ -17,7 +17,10 @@ UPSTREAM_CHECK_GITTAGREGEX = "^(?P<pver>\d+(\.\d+)+)$"
 
 S = "${WORKDIR}/git"
 
-EXTRA_OECMAKE = "-DBUILD_SHARED_LIBS=ON"
+EXTRA_OECMAKE = " \
+    -DBUILD_SHARED_LIBS=ON \
+    -DCFG_TEE_FS_PARENT_PATH='${localstatedir}/lib/tee' \
+"
 EXTRA_OECMAKE_append_toolchain-clang = " -DCFG_WERROR=0"
 
 do_install_append() {
-- 
2.30.2

[PATCH v2 5/6] optee-os: don't override upstream TA_LOG_LEVEL setting

[Yann Dirson]

From: Yann Dirson <yann@blade-group.com>

Aside from diverging from upstream, appending such a change it at the end of
oe_runmake makes it hard to override it in a bbappend.

This value does not seem to be inherited in TA builds, and
eg. optee-examples do use the the values set in their individual
Makefiles.  Setting this to 0 however seems to cause the TA traces not
to be relayed to the console, so the examples end up less
self-illustrative than they should.

This patch restores the upstream choice, while allowing downstream layers to
choose to set it to zero if they wish to do so.

Signed-off-by: Yann Dirson <yann@blade-group.com>
---
 meta-arm/recipes-security/optee/optee-os.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
index 08cc2f0..ca6e01b 100644
--- a/meta-arm/recipes-security/optee/optee-os.inc
+++ b/meta-arm/recipes-security/optee/optee-os.inc
@@ -38,7 +38,7 @@ LD[unexport] = "1"
 do_configure[noexec] = "1"
 
 do_compile() {
-    oe_runmake -C ${S} all CFG_TEE_TA_LOG_LEVEL=0
+    oe_runmake -C ${S} all
 }
 do_compile[cleandirs] = "${B}"
 
-- 
2.30.2

[PATCH v2 6/6] RFC: optee-os: provide a "debug" PACKAGECONFIG flag

[Yann Dirson]

From: Yann Dirson <yann@blade-group.com>

This provides a simple way to enable verbose logs to check whether OP-TEE
is indeed started or not.

Note I don't include here CFG_TEE_TA_LOG_LEVEL=3, which does not have any
apparent effect, the default value of 1 not preventing TA debug messages
to be printed.

This may be a bit too low-level for this usage, though, and CORE_LOG_LEVEL=2
would be enough for the intended usage, letting the following appear on
console log to show OP-TEE starting up:

  I/TC:
  I/TC: No non-secure external DT
  I/TC: OP-TEE version: 3.11.0-dev (gcc version 10.2.0 (GCC)) #1 Fri 16 Oct 2020 02:33:06 PM UTC aarch64
  I/TC: Primary CPU initializing
  I/TC: Primary CPU switching to normal world boot

... but then "debug" would be a bad choice for a name.

It could even make sense to have CORE_LOG_LEVEL=2 be the default, if
everyone is happy that deviating from upstream on this would be good, maybe
with a PACKAGECONFIG option of "quite" which could be set at CORE_LOG_LEVEL=1
and TA_LOG_LEVEL=0 to match today's behaviour ?
---
 meta-arm/recipes-security/optee/optee-os.inc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
index ca6e01b..9928129 100644
--- a/meta-arm/recipes-security/optee/optee-os.inc
+++ b/meta-arm/recipes-security/optee/optee-os.inc
@@ -29,6 +29,9 @@ EXTRA_OEMAKE += " \
     O=${B} \
 "
 
+PACKAGECONFIG[debug] = "CFG_TEE_CORE_LOG_LEVEL=3"
+EXTRA_OEMAKE_append = " ${PACKAGECONFIG_CONFARGS}"
+
 CFLAGS[unexport] = "1"
 LDFLAGS[unexport] = "1"
 CPPFLAGS[unexport] = "1"
-- 
2.30.2

Re: [meta-arm] [PATCH v2 3/6] arm/optee-os: deploy tee.elf for u-boot

[Ross Burton]

This is causing the CI to break:

ERROR: optee-os-3.11.0-r0 do_package_qa: QA Issue: optee-os: ELF
binary /lib/firmware/tee.elf has relocations in .text [textrel]

Can you either fix or add an INSANE_SKIP with a comment?

Ross

On Wed, 14 Apr 2021 at 14:48, Yann Dirson <yann.dirson@blade-group.com> wrote:
>
> From: Yann Dirson <yann@blade-group.com>
>
> Integration of OP-TEE in the u-boot image is done with `TEE=tee.elf`,
> so this build artifact must be exported.
>
> U-boot integration can now be done in a BSP layer with something like
> this in `u-boot_%.bbappend`:
>
>  # OP-TEE, dependent on "optee" DISTRO_FEATURES
>
>  OPTEE_OEMAKE ?= " TEE=${DEPLOY_DIR_IMAGE}/optee/tee.elf"
>
>  EXTRA_OEMAKE_append = " ${PACKAGECONFIG_CONFARGS}"
>  PACKAGECONFIG[optee] = "${OPTEE_OEMAKE},,optee-os"
>  PACKAGECONFIG_append = " ${@bb.utils.filter('DISTRO_FEATURES', 'optee', d)}"
>
> Signed-off-by: Yann Dirson <yann@blade-group.com>
> ---
>  meta-arm/recipes-security/optee/optee-os.inc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
> index 34f586a..08cc2f0 100644
> --- a/meta-arm/recipes-security/optee/optee-os.inc
> +++ b/meta-arm/recipes-security/optee/optee-os.inc
> @@ -45,7 +45,7 @@ do_compile[cleandirs] = "${B}"
>  do_install() {
>      #install core in firmware
>      install -d ${D}${nonarch_base_libdir}/firmware/
> -    install -m 644 ${B}/core/*.bin ${D}${nonarch_base_libdir}/firmware/
> +    install -m 644 ${B}/core/*.bin ${B}/core/tee.elf ${D}${nonarch_base_libdir}/firmware/
>
>      #install TA devkit
>      install -d ${D}${includedir}/optee/export-user_ta/
> --
> 2.30.2
>
>
> 
>

Re: [meta-arm] [PATCH v2 3/6] arm/optee-os: deploy tee.elf for u-boot

[Denys Dmytriyenko]

On Mon, Apr 19, 2021 at 05:34:13PM +0100, Ross Burton wrote:
> This is causing the CI to break:
> 
> ERROR: optee-os-3.11.0-r0 do_package_qa: QA Issue: optee-os: ELF
> binary /lib/firmware/tee.elf has relocations in .text [textrel]
> 
> Can you either fix or add an INSANE_SKIP with a comment?

Ah, sorry, forgot to mention that textrel needs to be added to INSANE_SKIP now 
that tee.elf gets packaged.


> Ross
> 
> On Wed, 14 Apr 2021 at 14:48, Yann Dirson <yann.dirson@blade-group.com> wrote:
> >
> > From: Yann Dirson <yann@blade-group.com>
> >
> > Integration of OP-TEE in the u-boot image is done with `TEE=tee.elf`,
> > so this build artifact must be exported.
> >
> > U-boot integration can now be done in a BSP layer with something like
> > this in `u-boot_%.bbappend`:
> >
> >  # OP-TEE, dependent on "optee" DISTRO_FEATURES
> >
> >  OPTEE_OEMAKE ?= " TEE=${DEPLOY_DIR_IMAGE}/optee/tee.elf"
> >
> >  EXTRA_OEMAKE_append = " ${PACKAGECONFIG_CONFARGS}"
> >  PACKAGECONFIG[optee] = "${OPTEE_OEMAKE},,optee-os"
> >  PACKAGECONFIG_append = " ${@bb.utils.filter('DISTRO_FEATURES', 'optee', d)}"
> >
> > Signed-off-by: Yann Dirson <yann@blade-group.com>
> > ---
> >  meta-arm/recipes-security/optee/optee-os.inc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
> > index 34f586a..08cc2f0 100644
> > --- a/meta-arm/recipes-security/optee/optee-os.inc
> > +++ b/meta-arm/recipes-security/optee/optee-os.inc
> > @@ -45,7 +45,7 @@ do_compile[cleandirs] = "${B}"
> >  do_install() {
> >      #install core in firmware
> >      install -d ${D}${nonarch_base_libdir}/firmware/
> > -    install -m 644 ${B}/core/*.bin ${D}${nonarch_base_libdir}/firmware/
> > +    install -m 644 ${B}/core/*.bin ${B}/core/tee.elf ${D}${nonarch_base_libdir}/firmware/
> >
> >      #install TA devkit
> >      install -d ${D}${includedir}/optee/export-user_ta/
> > --
> > 2.30.2
> >
> >
> > 
> >

-- 
Regards,
Denys Dmytriyenko <denis@denix.org>
PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964

Re: [meta-arm] [PATCH v2 3/6] arm/optee-os: deploy tee.elf for u-boot

[Jon Mason]

I applied the series, but didn't catch the warning/error that Ross
emntioned.  So, I had to revert this patch.  Could you please correct
it and resubmit?

Thanks,
Jon

On Wed, Apr 14, 2021 at 03:47:23PM +0200, Yann Dirson wrote:
> From: Yann Dirson <yann@blade-group.com>
> 
> Integration of OP-TEE in the u-boot image is done with `TEE=tee.elf`,
> so this build artifact must be exported.
> 
> U-boot integration can now be done in a BSP layer with something like
> this in `u-boot_%.bbappend`:
> 
>  # OP-TEE, dependent on "optee" DISTRO_FEATURES
> 
>  OPTEE_OEMAKE ?= " TEE=${DEPLOY_DIR_IMAGE}/optee/tee.elf"
> 
>  EXTRA_OEMAKE_append = " ${PACKAGECONFIG_CONFARGS}"
>  PACKAGECONFIG[optee] = "${OPTEE_OEMAKE},,optee-os"
>  PACKAGECONFIG_append = " ${@bb.utils.filter('DISTRO_FEATURES', 'optee', d)}"
> 
> Signed-off-by: Yann Dirson <yann@blade-group.com>
> ---
>  meta-arm/recipes-security/optee/optee-os.inc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
> index 34f586a..08cc2f0 100644
> --- a/meta-arm/recipes-security/optee/optee-os.inc
> +++ b/meta-arm/recipes-security/optee/optee-os.inc
> @@ -45,7 +45,7 @@ do_compile[cleandirs] = "${B}"
>  do_install() {
>      #install core in firmware
>      install -d ${D}${nonarch_base_libdir}/firmware/
> -    install -m 644 ${B}/core/*.bin ${D}${nonarch_base_libdir}/firmware/
> +    install -m 644 ${B}/core/*.bin ${B}/core/tee.elf ${D}${nonarch_base_libdir}/firmware/
>  
>      #install TA devkit
>      install -d ${D}${includedir}/optee/export-user_ta/
> -- 
> 2.30.2
> 

> 
> 
> 

Re: [meta-arm] [PATCH v2 3/6] arm/optee-os: deploy tee.elf for u-boot

[Yann Dirson]

Le mar. 20 avr. 2021 à 02:18, Denys Dmytriyenko <denis@denix.org> a écrit :
>
> On Mon, Apr 19, 2021 at 05:34:13PM +0100, Ross Burton wrote:
> > This is causing the CI to break:
> >
> > ERROR: optee-os-3.11.0-r0 do_package_qa: QA Issue: optee-os: ELF
> > binary /lib/firmware/tee.elf has relocations in .text [textrel]
> >
> > Can you either fix or add an INSANE_SKIP with a comment?
>
> Ah, sorry, forgot to mention that textrel needs to be added to INSANE_SKIP now
> that tee.elf gets packaged.

I'm surprised, how is it that this test is not triggered on all boxes
(ie. on the master CI but not on testing
or on my box) ?  I'm all for submitting a fix but it's harder to be
correct why I cannot reproduce the problem :)

>
>
> > Ross
> >
> > On Wed, 14 Apr 2021 at 14:48, Yann Dirson <yann.dirson@blade-group.com> wrote:
> > >
> > > From: Yann Dirson <yann@blade-group.com>
> > >
> > > Integration of OP-TEE in the u-boot image is done with `TEE=tee.elf`,
> > > so this build artifact must be exported.
> > >
> > > U-boot integration can now be done in a BSP layer with something like
> > > this in `u-boot_%.bbappend`:
> > >
> > >  # OP-TEE, dependent on "optee" DISTRO_FEATURES
> > >
> > >  OPTEE_OEMAKE ?= " TEE=${DEPLOY_DIR_IMAGE}/optee/tee.elf"
> > >
> > >  EXTRA_OEMAKE_append = " ${PACKAGECONFIG_CONFARGS}"
> > >  PACKAGECONFIG[optee] = "${OPTEE_OEMAKE},,optee-os"
> > >  PACKAGECONFIG_append = " ${@bb.utils.filter('DISTRO_FEATURES', 'optee', d)}"
> > >
> > > Signed-off-by: Yann Dirson <yann@blade-group.com>
> > > ---
> > >  meta-arm/recipes-security/optee/optee-os.inc | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
> > > index 34f586a..08cc2f0 100644
> > > --- a/meta-arm/recipes-security/optee/optee-os.inc
> > > +++ b/meta-arm/recipes-security/optee/optee-os.inc
> > > @@ -45,7 +45,7 @@ do_compile[cleandirs] = "${B}"
> > >  do_install() {
> > >      #install core in firmware
> > >      install -d ${D}${nonarch_base_libdir}/firmware/
> > > -    install -m 644 ${B}/core/*.bin ${D}${nonarch_base_libdir}/firmware/
> > > +    install -m 644 ${B}/core/*.bin ${B}/core/tee.elf ${D}${nonarch_base_libdir}/firmware/
> > >
> > >      #install TA devkit
> > >      install -d ${D}${includedir}/optee/export-user_ta/
> > > --
> > > 2.30.2
> > >
> > >
> > >
> > >
>
> --
> Regards,
> Denys Dmytriyenko <denis@denix.org>
> PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
> Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964



--
Yann Dirson <yann@blade-group.com>
Blade / Shadow -- http://shadow.tech

Re: [meta-arm] [PATCH v2 3/6] arm/optee-os: deploy tee.elf for u-boot

[Yann Dirson]

*

Le mar. 20 avr. 2021 à 09:54, Yann Dirson
<yann.dirson@blade-group.com> a écrit :
>
> Le mar. 20 avr. 2021 à 02:18, Denys Dmytriyenko <denis@denix.org> a écrit :
> >
> > On Mon, Apr 19, 2021 at 05:34:13PM +0100, Ross Burton wrote:
> > > This is causing the CI to break:
> > >
> > > ERROR: optee-os-3.11.0-r0 do_package_qa: QA Issue: optee-os: ELF
> > > binary /lib/firmware/tee.elf has relocations in .text [textrel]
> > >
> > > Can you either fix or add an INSANE_SKIP with a comment?
> >
> > Ah, sorry, forgot to mention that textrel needs to be added to INSANE_SKIP now
> > that tee.elf gets packaged.
>
> I'm surprised, how is it that this test is not triggered on all boxes
> (ie. on the master CI but not on testing
> or on my box) ?  I'm all for submitting a fix but it's harder to be
> correct why I cannot reproduce the problem :)

Digging this I am surprised by how hard it is to force a package_qa rerun:
- the package_qa_* functions text is not included, so even changing
them does not
  cause them to re-run
- WARN_QA and ERROR_QA are part of BB_HASHBASE_WHITELIST, so adjusting
  this does not cause a rerun either

It would seem interesting to have all of this part of the package_qa
siginfo, right ?

I have the clear feeling the workflow to investigate the issue is
really suboptimal
("bitbake -c clean && find sstate-cache -delete" between each run).


As for the issue at hand: I can now trigger package_qa at will, and
adding more traces
in textrel test I can see it is really executed on tee.elf, but it
just passes the test.

"objdump -p" and "objdump -r" just show nothing, so indeed no TEXTREL.

On what configuration does the CI get this ?

>
> >
> >
> > > Ross
> > >
> > > On Wed, 14 Apr 2021 at 14:48, Yann Dirson <yann.dirson@blade-group.com> wrote:
> > > >
> > > > From: Yann Dirson <yann@blade-group.com>
> > > >
> > > > Integration of OP-TEE in the u-boot image is done with `TEE=tee.elf`,
> > > > so this build artifact must be exported.
> > > >
> > > > U-boot integration can now be done in a BSP layer with something like
> > > > this in `u-boot_%.bbappend`:
> > > >
> > > >  # OP-TEE, dependent on "optee" DISTRO_FEATURES
> > > >
> > > >  OPTEE_OEMAKE ?= " TEE=${DEPLOY_DIR_IMAGE}/optee/tee.elf"
> > > >
> > > >  EXTRA_OEMAKE_append = " ${PACKAGECONFIG_CONFARGS}"
> > > >  PACKAGECONFIG[optee] = "${OPTEE_OEMAKE},,optee-os"
> > > >  PACKAGECONFIG_append = " ${@bb.utils.filter('DISTRO_FEATURES', 'optee', d)}"
> > > >
> > > > Signed-off-by: Yann Dirson <yann@blade-group.com>
> > > > ---
> > > >  meta-arm/recipes-security/optee/optee-os.inc | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
> > > > index 34f586a..08cc2f0 100644
> > > > --- a/meta-arm/recipes-security/optee/optee-os.inc
> > > > +++ b/meta-arm/recipes-security/optee/optee-os.inc
> > > > @@ -45,7 +45,7 @@ do_compile[cleandirs] = "${B}"
> > > >  do_install() {
> > > >      #install core in firmware
> > > >      install -d ${D}${nonarch_base_libdir}/firmware/
> > > > -    install -m 644 ${B}/core/*.bin ${D}${nonarch_base_libdir}/firmware/
> > > > +    install -m 644 ${B}/core/*.bin ${B}/core/tee.elf ${D}${nonarch_base_libdir}/firmware/
> > > >
> > > >      #install TA devkit
> > > >      install -d ${D}${includedir}/optee/export-user_ta/
> > > > --
> > > > 2.30.2
> > > >
> > > >
> > > >
> > > >
> >
> > --
> > Regards,
> > Denys Dmytriyenko <denis@denix.org>
> > PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
> > Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964
>
>
>
> --
> Yann Dirson <yann@blade-group.com>
> Blade / Shadow -- http://shadow.tech



-- 
Yann Dirson <yann@blade-group.com>
Blade / Shadow -- http://shadow.tech

Re: [meta-arm] [PATCH v2 3/6] arm/optee-os: deploy tee.elf for u-boot

[Denys Dmytriyenko]

On Tue, Apr 20, 2021 at 11:35:55AM +0200, Yann Dirson wrote:
> *
> 
> Le mar. 20 avr. 2021 à 09:54, Yann Dirson
> <yann.dirson@blade-group.com> a écrit :
> >
> > Le mar. 20 avr. 2021 à 02:18, Denys Dmytriyenko <denis@denix.org> a écrit :
> > >
> > > On Mon, Apr 19, 2021 at 05:34:13PM +0100, Ross Burton wrote:
> > > > This is causing the CI to break:
> > > >
> > > > ERROR: optee-os-3.11.0-r0 do_package_qa: QA Issue: optee-os: ELF
> > > > binary /lib/firmware/tee.elf has relocations in .text [textrel]
> > > >
> > > > Can you either fix or add an INSANE_SKIP with a comment?
> > >
> > > Ah, sorry, forgot to mention that textrel needs to be added to INSANE_SKIP now
> > > that tee.elf gets packaged.
> >
> > I'm surprised, how is it that this test is not triggered on all boxes
> > (ie. on the master CI but not on testing
> > or on my box) ?  I'm all for submitting a fix but it's harder to be
> > correct why I cannot reproduce the problem :)
> 
> Digging this I am surprised by how hard it is to force a package_qa rerun:
> - the package_qa_* functions text is not included, so even changing
> them does not
>   cause them to re-run
> - WARN_QA and ERROR_QA are part of BB_HASHBASE_WHITELIST, so adjusting
>   this does not cause a rerun either
> 
> It would seem interesting to have all of this part of the package_qa
> siginfo, right ?
> 
> I have the clear feeling the workflow to investigate the issue is
> really suboptimal
> ("bitbake -c clean && find sstate-cache -delete" between each run).

I usually do -c cleansstate and then force -c compile -f to invalidate sstate 
for a package.


> As for the issue at hand: I can now trigger package_qa at will, and
> adding more traces
> in textrel test I can see it is really executed on tee.elf, but it
> just passes the test.
> 
> "objdump -p" and "objdump -r" just show nothing, so indeed no TEXTREL.
> 
> On what configuration does the CI get this ?

I wonder if tee.elf for your platform is different. Are you building for qemu 
or a real machine? I saw this QA issue reported for TI platforms before.


> > > > Ross
> > > >
> > > > On Wed, 14 Apr 2021 at 14:48, Yann Dirson <yann.dirson@blade-group.com> wrote:
> > > > >
> > > > > From: Yann Dirson <yann@blade-group.com>
> > > > >
> > > > > Integration of OP-TEE in the u-boot image is done with `TEE=tee.elf`,
> > > > > so this build artifact must be exported.
> > > > >
> > > > > U-boot integration can now be done in a BSP layer with something like
> > > > > this in `u-boot_%.bbappend`:
> > > > >
> > > > >  # OP-TEE, dependent on "optee" DISTRO_FEATURES
> > > > >
> > > > >  OPTEE_OEMAKE ?= " TEE=${DEPLOY_DIR_IMAGE}/optee/tee.elf"
> > > > >
> > > > >  EXTRA_OEMAKE_append = " ${PACKAGECONFIG_CONFARGS}"
> > > > >  PACKAGECONFIG[optee] = "${OPTEE_OEMAKE},,optee-os"
> > > > >  PACKAGECONFIG_append = " ${@bb.utils.filter('DISTRO_FEATURES', 'optee', d)}"
> > > > >
> > > > > Signed-off-by: Yann Dirson <yann@blade-group.com>
> > > > > ---
> > > > >  meta-arm/recipes-security/optee/optee-os.inc | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
> > > > > index 34f586a..08cc2f0 100644
> > > > > --- a/meta-arm/recipes-security/optee/optee-os.inc
> > > > > +++ b/meta-arm/recipes-security/optee/optee-os.inc
> > > > > @@ -45,7 +45,7 @@ do_compile[cleandirs] = "${B}"
> > > > >  do_install() {
> > > > >      #install core in firmware
> > > > >      install -d ${D}${nonarch_base_libdir}/firmware/
> > > > > -    install -m 644 ${B}/core/*.bin ${D}${nonarch_base_libdir}/firmware/
> > > > > +    install -m 644 ${B}/core/*.bin ${B}/core/tee.elf ${D}${nonarch_base_libdir}/firmware/
> > > > >
> > > > >      #install TA devkit
> > > > >      install -d ${D}${includedir}/optee/export-user_ta/
> > > > > --
> > > > > 2.30.2
> > > > >
> > > > >
> > > > >
> > > > >
> > >
> > > --
> > > Regards,
> > > Denys Dmytriyenko <denis@denix.org>
> > > PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
> > > Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964
> >
> >
> >
> > --
> > Yann Dirson <yann@blade-group.com>
> > Blade / Shadow -- http://shadow.tech
> 
> 
> 
> -- 
> Yann Dirson <yann@blade-group.com>
> Blade / Shadow -- http://shadow.tech
> 

-- 
Regards,
Denys Dmytriyenko <denis@denix.org>
PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964

Re: [meta-arm] [PATCH v2 3/6] arm/optee-os: deploy tee.elf for u-boot

[Yann Dirson]

Le mar. 20 avr. 2021 à 17:50, Denys Dmytriyenko <denis@denix.org> a écrit :
>
> On Tue, Apr 20, 2021 at 11:35:55AM +0200, Yann Dirson wrote:
> > *
> >
> > Le mar. 20 avr. 2021 à 09:54, Yann Dirson
> > <yann.dirson@blade-group.com> a écrit :
> > >
> > > Le mar. 20 avr. 2021 à 02:18, Denys Dmytriyenko <denis@denix.org> a écrit :
> > > >
> > > > On Mon, Apr 19, 2021 at 05:34:13PM +0100, Ross Burton wrote:
> > > > > This is causing the CI to break:
> > > > >
> > > > > ERROR: optee-os-3.11.0-r0 do_package_qa: QA Issue: optee-os: ELF
> > > > > binary /lib/firmware/tee.elf has relocations in .text [textrel]
> > > > >
> > > > > Can you either fix or add an INSANE_SKIP with a comment?
> > > >
> > > > Ah, sorry, forgot to mention that textrel needs to be added to INSANE_SKIP now
> > > > that tee.elf gets packaged.
> > >
> > > I'm surprised, how is it that this test is not triggered on all boxes
> > > (ie. on the master CI but not on testing
> > > or on my box) ?  I'm all for submitting a fix but it's harder to be
> > > correct why I cannot reproduce the problem :)
> >
> > Digging this I am surprised by how hard it is to force a package_qa rerun:
> > - the package_qa_* functions text is not included, so even changing
> > them does not
> >   cause them to re-run
> > - WARN_QA and ERROR_QA are part of BB_HASHBASE_WHITELIST, so adjusting
> >   this does not cause a rerun either
> >
> > It would seem interesting to have all of this part of the package_qa
> > siginfo, right ?
> >
> > I have the clear feeling the workflow to investigate the issue is
> > really suboptimal
> > ("bitbake -c clean && find sstate-cache -delete" between each run).
>
> I usually do -c cleansstate and then force -c compile -f to invalidate sstate
> for a package.
>
>
> > As for the issue at hand: I can now trigger package_qa at will, and
> > adding more traces
> > in textrel test I can see it is really executed on tee.elf, but it
> > just passes the test.
> >
> > "objdump -p" and "objdump -r" just show nothing, so indeed no TEXTREL.
> >
> > On what configuration does the CI get this ?
>
> I wonder if tee.elf for your platform is different. Are you building for qemu
> or a real machine? I saw this QA issue reported for TI platforms before.

Yes I'm building for rk3399-based nanopi-m4.

>
>
> > > > > Ross
> > > > >
> > > > > On Wed, 14 Apr 2021 at 14:48, Yann Dirson <yann.dirson@blade-group.com> wrote:
> > > > > >
> > > > > > From: Yann Dirson <yann@blade-group.com>
> > > > > >
> > > > > > Integration of OP-TEE in the u-boot image is done with `TEE=tee.elf`,
> > > > > > so this build artifact must be exported.
> > > > > >
> > > > > > U-boot integration can now be done in a BSP layer with something like
> > > > > > this in `u-boot_%.bbappend`:
> > > > > >
> > > > > >  # OP-TEE, dependent on "optee" DISTRO_FEATURES
> > > > > >
> > > > > >  OPTEE_OEMAKE ?= " TEE=${DEPLOY_DIR_IMAGE}/optee/tee.elf"
> > > > > >
> > > > > >  EXTRA_OEMAKE_append = " ${PACKAGECONFIG_CONFARGS}"
> > > > > >  PACKAGECONFIG[optee] = "${OPTEE_OEMAKE},,optee-os"
> > > > > >  PACKAGECONFIG_append = " ${@bb.utils.filter('DISTRO_FEATURES', 'optee', d)}"
> > > > > >
> > > > > > Signed-off-by: Yann Dirson <yann@blade-group.com>
> > > > > > ---
> > > > > >  meta-arm/recipes-security/optee/optee-os.inc | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
> > > > > > index 34f586a..08cc2f0 100644
> > > > > > --- a/meta-arm/recipes-security/optee/optee-os.inc
> > > > > > +++ b/meta-arm/recipes-security/optee/optee-os.inc
> > > > > > @@ -45,7 +45,7 @@ do_compile[cleandirs] = "${B}"
> > > > > >  do_install() {
> > > > > >      #install core in firmware
> > > > > >      install -d ${D}${nonarch_base_libdir}/firmware/
> > > > > > -    install -m 644 ${B}/core/*.bin ${D}${nonarch_base_libdir}/firmware/
> > > > > > +    install -m 644 ${B}/core/*.bin ${B}/core/tee.elf ${D}${nonarch_base_libdir}/firmware/
> > > > > >
> > > > > >      #install TA devkit
> > > > > >      install -d ${D}${includedir}/optee/export-user_ta/
> > > > > > --
> > > > > > 2.30.2
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > >
> > > > --
> > > > Regards,
> > > > Denys Dmytriyenko <denis@denix.org>
> > > > PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
> > > > Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964
> > >
> > >
> > >
> > > --
> > > Yann Dirson <yann@blade-group.com>
> > > Blade / Shadow -- http://shadow.tech
> >
> >
> >
> > --
> > Yann Dirson <yann@blade-group.com>
> > Blade / Shadow -- http://shadow.tech
> >
>
> --
> Regards,
> Denys Dmytriyenko <denis@denix.org>
> PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
> Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964



-- 
Yann Dirson <yann@blade-group.com>
Blade / Shadow -- http://shadow.tech