[PATCH 1/4] optee-os: asm debug prefix fixes

[PATCH 1/4] optee-os: asm debug prefix fixes

[Mikko Rapeli]

The .S files compiled by optee-os were including
absolute path of the recipe git tree. Fix this by
applying CFLAGS with correct debug prefix maps to AFLAGS
used by optee makefiles. Fixes optee-os and optee-os-tadevkit
buildpaths QA errors.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta-arm/recipes-security/optee/optee-os.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
index e9f252e3..d5a12642 100644
--- a/meta-arm/recipes-security/optee/optee-os.inc
+++ b/meta-arm/recipes-security/optee/optee-os.inc
@@ -24,6 +24,7 @@ EXTRA_OEMAKE += " \
     CFG_${OPTEE_CORE}_core=y \
     CROSS_COMPILE_core=${HOST_PREFIX} \
     CROSS_COMPILE_ta_${OPTEE_ARCH}=${HOST_PREFIX} \
+    AFLAGS="${CFLAGS}" \
     ta-targets=ta_${OPTEE_ARCH} \
     O=${B} \
 "
-- 
2.34.1

[PATCH 2/4] optee-os: remove absolute paths

[Mikko Rapeli]

Change optee-os build scripts to not use absolute
build time paths in generated header files and scripts.

One patch is backport from master/4.3 and the other has been
submitted.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 ...not-use-full-path-to-generate-guard-.patch | 45 +++++++++++++++++++
 ....mk-remove-absolute-build-time-paths.patch | 32 +++++++++++++
 .../recipes-security/optee/optee-os_4.2.0.bb  |  4 +-
 3 files changed, 80 insertions(+), 1 deletion(-)
 create mode 100644 meta-arm/recipes-security/optee/optee-os/0001-checkconf.mk-do-not-use-full-path-to-generate-guard-.patch
 create mode 100644 meta-arm/recipes-security/optee/optee-os/0001-mk-compile.mk-remove-absolute-build-time-paths.patch

diff --git a/meta-arm/recipes-security/optee/optee-os/0001-checkconf.mk-do-not-use-full-path-to-generate-guard-.patch b/meta-arm/recipes-security/optee/optee-os/0001-checkconf.mk-do-not-use-full-path-to-generate-guard-.patch
new file mode 100644
index 00000000..29719b45
--- /dev/null
+++ b/meta-arm/recipes-security/optee/optee-os/0001-checkconf.mk-do-not-use-full-path-to-generate-guard-.patch
@@ -0,0 +1,45 @@
+From c8a2a6529dc3ff609281ef4fe5c5bc949c805b5c Mon Sep 17 00:00:00 2001
+From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
+Date: Thu, 6 Jun 2024 11:42:46 +0200
+Subject: [PATCH] checkconf.mk: do not use full path to generate guard symbol
+ in conf.h
+
+The combination of building with -g3 (which emits definitions of all
+defined preprocessor macros to the debug info) and using a full path
+to define the name of this preprocessor guard means that the output is
+not binary reproducible across different build hosts. For example, in
+my Yocto build, the string
+
+  __home_ravi_yocto_tmp_glibc_work_stm32mp135fdk_oe_linux_gnueabi_optee_os_stm32mp_3_19_0_stm32mp_r1_1_build_stm32mp135f_dk_include_generated_conf_h_
+
+appears in several build artifacts. Another developer or buildbot
+would not build in some /home/ravi/... directory.
+
+In order to increase binary reproducibility, only use the path sans
+the $(out-dir)/ prefix of the conf.h file.
+
+Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
+Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
+---
+ mk/checkconf.mk | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+Upstream-Status: Backport [c8a2a6529dc3ff609281ef4fe5c5bc949c805b5c]
+
+diff --git a/mk/checkconf.mk b/mk/checkconf.mk
+index 449b1c2b8..bb08d6b15 100644
+--- a/mk/checkconf.mk
++++ b/mk/checkconf.mk
+@@ -17,7 +17,8 @@ define check-conf-h
+ 	cnf='$(strip $(foreach var,				\
+ 		$(call cfg-vars-by-prefix,$1),			\
+ 		$(call cfg-make-define,$(var))))';		\
+-	guard="_`echo $@ | tr -- -/.+ _`_";			\
++	guardpath="$(patsubst $(out-dir)/%,%,$@)"		\
++	guard="_`echo "$${guardpath}" | tr -- -/.+ _`_";	\
+ 	mkdir -p $(dir $@);					\
+ 	echo "#ifndef $${guard}" >$@.tmp;			\
+ 	echo "#define $${guard}" >>$@.tmp;			\
+-- 
+2.34.1
+
diff --git a/meta-arm/recipes-security/optee/optee-os/0001-mk-compile.mk-remove-absolute-build-time-paths.patch b/meta-arm/recipes-security/optee/optee-os/0001-mk-compile.mk-remove-absolute-build-time-paths.patch
new file mode 100644
index 00000000..af41599c
--- /dev/null
+++ b/meta-arm/recipes-security/optee/optee-os/0001-mk-compile.mk-remove-absolute-build-time-paths.patch
@@ -0,0 +1,32 @@
+From f881f1b42be0b1e53c0f9a0a663adbc11fa2f320 Mon Sep 17 00:00:00 2001
+From: Mikko Rapeli <mikko.rapeli@linaro.org>
+Date: Thu, 18 Jul 2024 07:54:18 +0000
+Subject: [PATCH] mk/compile.mk: remove absolute build time paths
+
+Some generated files get a __FILE_ID__ which include absolute
+build time paths. Remove the paths and use plain file name.
+Fixes yocto QA check.
+
+Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
+---
+ mk/compile.mk | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+Upstream-Status: Submitted [https://github.com/OP-TEE/optee_os/pull/6950]
+
+diff --git a/mk/compile.mk b/mk/compile.mk
+index b3d807ba4..85b4dab64 100644
+--- a/mk/compile.mk
++++ b/mk/compile.mk
+@@ -120,7 +120,7 @@ comp-cppflags-$2 = $$(filter-out $$(CPPFLAGS_REMOVE) $$(cppflags-remove) \
+ 		      $$(addprefix -I,$$(incdirs-$2)) \
+ 		      $$(cppflags$$(comp-sm-$2)) \
+ 		      $$(cppflags-lib$$(comp-lib-$2)) $$(cppflags-$2)) \
+-		      -D__FILE_ID__=$$(subst -,_,$$(subst /,_,$$(subst .,_,$1)))
++		      -D__FILE_ID__=$$(subst -,_,$$(subst /,_,$$(subst .,_,$$(notdir $1))))
+ 
+ comp-flags-$2 += -MD -MF $$(comp-dep-$2) -MT $$@
+ comp-flags-$2 += $$(comp-cppflags-$2)
+-- 
+2.34.1
+
diff --git a/meta-arm/recipes-security/optee/optee-os_4.2.0.bb b/meta-arm/recipes-security/optee/optee-os_4.2.0.bb
index 8ae219f4..cee024af 100644
--- a/meta-arm/recipes-security/optee/optee-os_4.2.0.bb
+++ b/meta-arm/recipes-security/optee/optee-os_4.2.0.bb
@@ -7,4 +7,6 @@ FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:"
 SRCREV = "12d7c4ee4642d2d761e39fbcf21a06fb77141dea"
 SRC_URI += " \
     file://0003-optee-enable-clang-support.patch \
-   "
+    file://0001-checkconf.mk-do-not-use-full-path-to-generate-guard-.patch \
+    file://0001-mk-compile.mk-remove-absolute-build-time-paths.patch \
+"
-- 
2.34.1

[PATCH 3/4] optee-os-tadevkit: remove buildpaths INSANE_SKIP

[Mikko Rapeli]

Embedded build paths are now removed and test passes.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta-arm/recipes-security/optee/optee-os-tadevkit_4.2.0.bb | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/meta-arm/recipes-security/optee/optee-os-tadevkit_4.2.0.bb b/meta-arm/recipes-security/optee/optee-os-tadevkit_4.2.0.bb
index 961d5251..2e43254a 100644
--- a/meta-arm/recipes-security/optee/optee-os-tadevkit_4.2.0.bb
+++ b/meta-arm/recipes-security/optee/optee-os-tadevkit_4.2.0.bb
@@ -20,9 +20,6 @@ do_deploy() {
 
 FILES:${PN} = "${includedir}/optee/"
 
-# Build paths are currently embedded
-INSANE_SKIP:${PN}-dev += "buildpaths"
-
 # Include extra headers needed by SPMC tests to TA DEVKIT.
 # Supported after op-tee v3.20
 EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES', 'optee-spmc-test', \
-- 
2.34.1

[PATCH 4/4] optee-os: remove buildpaths INSANE_SKIP

[Mikko Rapeli]

Embedded build paths are now removed and test passes.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta-arm/recipes-security/optee/optee-os.inc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/meta-arm/recipes-security/optee/optee-os.inc b/meta-arm/recipes-security/optee/optee-os.inc
index d5a12642..5a89e5ba 100644
--- a/meta-arm/recipes-security/optee/optee-os.inc
+++ b/meta-arm/recipes-security/optee/optee-os.inc
@@ -76,7 +76,5 @@ FILES:${PN}-ta = "${nonarch_base_libdir}/optee_armtz/*"
 
 # note: "textrel" is not triggered on all archs
 INSANE_SKIP:${PN} = "textrel"
-# Build paths are currently embedded
-INSANE_SKIP:${PN} += "buildpaths"
 INSANE_SKIP:${PN}-dev = "staticdev"
 INHIBIT_PACKAGE_STRIP = "1"
-- 
2.34.1

Re: [PATCH 1/4] optee-os: asm debug prefix fixes

[Mikko Rapeli]

Hi,

Is there something in these patches that is not ok for merging?

I'm following up the build paths patch with upstream.

Cheers,

-Mikko

Re: [PATCH 1/4] optee-os: asm debug prefix fixes

[Jon Mason]

On Tue, Jul 30, 2024 at 12:31:59PM +0300, Mikko Rapeli wrote:
> Hi,
> 
> Is there something in these patches that is not ok for merging?

Sorry for the delay.

Something in CI was breaking and I was trying to isolate it to the
individual patches, but the CI I'm running that is publicly available
has been having issues (which I think are now resolved) and the
internal one has been bogged down with a large number of reviews.

v2 that you just sent is now in master-next and hopefully will be
happy when I get up tomorrow morning.

Thanks,
Jon

> 
> I'm following up the build paths patch with upstream.
> 
> Cheers,
> 
> -Mikko
> 

Re: [PATCH 1/4] optee-os: asm debug prefix fixes

[Mikko Rapeli]

Hi,

On Tue, Jul 30, 2024 at 11:12:12PM -0400, Jon Mason wrote:
> On Tue, Jul 30, 2024 at 12:31:59PM +0300, Mikko Rapeli wrote:
> > Hi,
> > 
> > Is there something in these patches that is not ok for merging?
> 
> Sorry for the delay.
> 
> Something in CI was breaking and I was trying to isolate it to the
> individual patches, but the CI I'm running that is publicly available
> has been having issues (which I think are now resolved) and the
> internal one has been bogged down with a large number of reviews.
> 
> v2 that you just sent is now in master-next and hopefully will be
> happy when I get up tomorrow morning.

Ok then these optee changes are likely not causing issues, good to know.

I'll wait for the master branch update, can't see master-next changes.

Cheers,

-Mikko

Re: [PATCH 1/4] optee-os: asm debug prefix fixes

[Jon Mason]

On Wed, Jul 31, 2024 at 09:35:57AM +0300, Mikko Rapeli wrote:
> Hi,
> 
> On Tue, Jul 30, 2024 at 11:12:12PM -0400, Jon Mason wrote:
> > On Tue, Jul 30, 2024 at 12:31:59PM +0300, Mikko Rapeli wrote:
> > > Hi,
> > > 
> > > Is there something in these patches that is not ok for merging?
> > 
> > Sorry for the delay.
> > 
> > Something in CI was breaking and I was trying to isolate it to the
> > individual patches, but the CI I'm running that is publicly available
> > has been having issues (which I think are now resolved) and the
> > internal one has been bogged down with a large number of reviews.
> > 
> > v2 that you just sent is now in master-next and hopefully will be
> > happy when I get up tomorrow morning.
> 
> Ok then these optee changes are likely not causing issues, good to know.
> 
> I'll wait for the master branch update, can't see master-next changes.

I only do master-next (and actually all of the '-next' branches) on my
personal gitlab.  I do this because I'm constantly applying and
removing patches from it.  You can see it at
https://gitlab.com/jonmason00/meta-arm/-/pipelines

I do this in an attempt to have our CI being public and to make it
easier for people to see when there are issues.

Thanks,
Jon

> 
> Cheers,
> 
> -Mikko
> 

Re: [PATCH 1/4] optee-os: asm debug prefix fixes

[Mikko Rapeli]

Hi,

On Wed, Jul 31, 2024 at 08:57:19AM -0400, Jon Mason wrote:
> On Wed, Jul 31, 2024 at 09:35:57AM +0300, Mikko Rapeli wrote:
> > Hi,
> > 
> > On Tue, Jul 30, 2024 at 11:12:12PM -0400, Jon Mason wrote:
> > > On Tue, Jul 30, 2024 at 12:31:59PM +0300, Mikko Rapeli wrote:
> > > > Hi,
> > > > 
> > > > Is there something in these patches that is not ok for merging?
> > > 
> > > Sorry for the delay.
> > > 
> > > Something in CI was breaking and I was trying to isolate it to the
> > > individual patches, but the CI I'm running that is publicly available
> > > has been having issues (which I think are now resolved) and the
> > > internal one has been bogged down with a large number of reviews.
> > > 
> > > v2 that you just sent is now in master-next and hopefully will be
> > > happy when I get up tomorrow morning.
> > 
> > Ok then these optee changes are likely not causing issues, good to know.
> > 
> > I'll wait for the master branch update, can't see master-next changes.
> 
> I only do master-next (and actually all of the '-next' branches) on my
> personal gitlab.  I do this because I'm constantly applying and
> removing patches from it.  You can see it at
> https://gitlab.com/jonmason00/meta-arm/-/pipelines
> 
> I do this in an attempt to have our CI being public and to make it
> easier for people to see when there are issues.

Sadly I see that optee-test/xtest timed out on qemuarm-secureboot

https://gitlab.com/jonmason00/meta-arm/-/jobs/7472950337

and buildpaths QA test failing on corstone1000 with musl

https://gitlab.com/jonmason00/meta-arm/-/jobs/7472950196

2024-08-01 02:23:30 - ERROR    - Command "/builds/jonmason00/meta-arm/work/build$ /builds/jonmason00/meta-arm/work/poky/bitbake/bin/bitbake -c build arm-systemready-firmware" failed
--- Error summary ---
ERROR: mc:firmware:optee-os-4.2.0-r0 do_package_qa: QA Issue: File /lib/firmware/tee.elf in package optee-os contains reference to TMPDIR [buildpaths]
ERROR: mc:firmware:optee-os-4.2.0-r0 do_package_qa: Fatal QA errors were found, failing task.
ERROR: Logfile of failure stored in: /builds/jonmason00/meta-arm/work/build/tmp-musl_corstone1000-fvp/work/corstone1000_fvp-oe-linux-musl/optee-os/4.2.0/temp/log.do_package_qa.65026
ERROR: Task (mc:firmware:/builds/jonmason00/meta-arm/work/build/../../meta-arm/recipes-security/optee/optee-os_4.2.0.bb:do_package_qa) failed with exit code '1'
2024-08-01 02:23:30 - ERROR    - Command "/builds/jonmason00/meta-arm/work/poky/bitbake/bin/bitbake -c build arm-systemready-firmware" failed with error 1

https://gitlab.com/jonmason00/meta-arm/-/jobs/7472950159

2024-08-01 02:04:59 - ERROR    - ERROR: mc:firmware:optee-os-4.2.0-r0 do_package_qa: QA Issue: File /lib/firmware/tee.elf in package optee-os contains reference to TMPDIR [buildpaths]
2024-08-01 02:04:59 - ERROR    - ERROR: mc:firmware:optee-os-4.2.0-r0 do_package_qa: Fatal QA errors were found, failing task.
2024-08-01 02:04:59 - ERROR    - ERROR: Logfile of failure stored in: /builds/jonmason00/meta-arm/work/build/tmp-musl_corstone1000-fvp/work/corstone1000_fvp-oe-linux-musl/optee-os/4.2.0/temp/log.do_package_qa.111505

I'll try to reproduce these and fix them. Sorry for not testing these
beforehand. I was only testing with:

$ kas build ci/qemuarm64-secureboot.yml:ci/testimage.yml

and that clearly is not enough for all variants.

Cheers,

-Mikko