OP-TEE / optee_os

Trusted side of the TEE
Other
1.53k stars 1.03k forks source link

Not simple to use a CC that needs arguments passed #4188

Closed rossburton closed 10 months ago

rossburton commented 3 years ago

When building in Yocto/OpenEmbedded, the cross compiler is "poisoned" so you need to tell it where the sysroot is. This is because the sysroot is per package for isolation purposes.

Typically, this is done by setting CC = "arch-prefix-gcc --sysroot=/path/to/sysroot. Build systems use that compiler, and everything works.

However, CC is explicitly not used. optee-os sets CC$(sm) to $(CROSS_COMPILE)gcc, and there are many variations of sm: core, ldelf, ta_arm32, ta_arm64 in optee-os alone. Also, the same applies to CXX.

Can there be a way to set a default CC for all builds that overrides $(CROSS_COMPILE)gcc?

rossburton commented 3 years ago

Add avb, pkcs11 and trusted_keys to the list of candidates.

Further context: the primary failure is any linking to libgcc orlibgcc_eh, as those as CC where the files are and without the sysroot, it has no idea.

Further fun: optee-os can find libgcc as we can set CFLAGS to include the sysroot, but libgcc_ehis found using CXX and there is no easy flag for that, it's the $(sm) combinational explosion again.

Emantor commented 3 years ago

FWIW, the meta-linaro layer contains a patch to the OP-TEE build system and explicitly passes the libgcc location, see here. I remember doing a similar operation for libgcc_eh, but would of couse prefer an upstream solution.

jenswi-linaro commented 3 years ago

Setting CROSS_COMPILE_core, CROSS_COMPILE_ta_arm32, and CROSS_COMPILE_ta_arm64 to the different cross compiler you need should be enough. CROSS_COMPILE_ldelf is assigned from CROSS_COMPILE_core and CROSS_COMPILE_avb and friends are assigned from CROSS_COMPILE_ta_arm32 or CROSS_COMPILE_ta_arm64.

rossburton commented 3 years ago

FWIW, the meta-linaro layer contains a patch to the OP-TEE build system and explicitly passes the libgcc location, see here. I remember doing a similar operation for libgcc_eh, but would of couse prefer an upstream solution.

Yes, an evolution of that is in meta-arm now, growing over time as it also needs to patch for clang. I was attempting a new solution that is upstreamable.

Setting CROSS_COMPILE_core, CROSS_COMPILE_ta_arm32, and CROSS_COMPILE_ta_arm64 to the different cross compiler you need should be enough

No, because CROSS_COMPILE is turned into CC in gcc.mk with (essentially) CC = $(CROSS_COMPILE)gcc. I want CC to be my-prefix-gcc --sysroot=/some/sysroot.

Alternatively, a way of setting CFLAGS that is universal and consistent.

jenswi-linaro commented 3 years ago

Setting CROSS_COMPILE_core, CROSS_COMPILE_ta_arm32, and CROSS_COMPILE_ta_arm64 to the different cross compiler you need should be enough

No, because CROSS_COMPILE is turned into CC in gcc.mk with (essentially) CC = $(CROSS_COMPILE)gcc. I want CC to be my-prefix-gcc --sysroot=/some/sysroot.

Sorry, missed that.

Alternatively, a way of setting CFLAGS that is universal and consistent.

OP-TEE does pick up CFLAGS (and CFLAGS32 and CFLAGS64) if that helps. That rings a bell, I think we had some problem in the past with yocto and undesired values in CFLAGS.

rossburton commented 3 years ago

Mostly, in some places. optee-os lets you extend CFLAGS, but optee-test uses it: override CFLAGS and the build fails.

And libgcc_eh is C++ so doesn't use CFLAGS at all, and CXXFLAGS isn't respected the same.

jenswi-linaro commented 3 years ago

To fix this it would help with something more specific. A PR might be even easier.

github-actions[bot] commented 3 years ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

rossburton commented 3 years ago

Pinging in the hope that this can be reopened.

jbech-linaro commented 3 years ago

Pinging in the hope that this can be reopened.

Done!

github-actions[bot] commented 3 years ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

rossburton commented 3 years ago

Not stale!

jbech-linaro commented 3 years ago

@rossburton , I've added the "enhancement" label to it now, which means that it shouldn't become stale again. Where are we with this issue? Is anyone actively working with it?

rossburton commented 3 years ago

I've some partial patches. Are there any plans to change the optee build system to something else or can I assume the current Makefiles are here to stay? (asking because I know TF-A has just moved to CMake).

jbech-linaro commented 3 years ago

For optee_os, no I think that'll stay with the plain old GNU Makefile(s). I did some test with CMake 2-3 years ago (for optee_os). But besides the complexity of making it work (and dependencies to optee_examples and optee_test) I saw no strong interest from others and I believe the situation is still the same more or less.

ricardosalveti commented 3 years ago

Testing 3.12, this is now the only issue I had when building OP-TEE upstream on yocto/OE.

The patch currently available in meta-arm (which is an evolution of the one in meta-linaro):

From: Ross Burton <ross.burton@arm.com>
Date: Tue, 26 May 2020 14:38:02 -0500
Subject: [PATCH] allow setting sysroot for libgcc lookup

Explicitly pass the new variable LIBGCC_LOCATE_CFLAGS variable when searching
for the compiler libraries as there's no easy way to reliably pass --sysroot
otherwise.

Upstream-Status: Pending [https://github.com/OP-TEE/optee_os/issues/4188]
Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 mk/gcc.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mk/gcc.mk b/mk/gcc.mk
index adc77a24..81bfa78a 100644
--- a/mk/gcc.mk
+++ b/mk/gcc.mk
@@ -13,11 +13,11 @@ nostdinc$(sm)       := -nostdinc -isystem $(shell $(CC$(sm)) \
                        -print-file-name=include 2> /dev/null)

 # Get location of libgcc from gcc
-libgcc$(sm)    := $(shell $(CC$(sm)) $(CFLAGS$(arch-bits-$(sm))) \
+libgcc$(sm)    := $(shell $(CC$(sm)) $(LIBGCC_LOCATE_CFLAGS) $(CFLAGS$(arch-bits-$(sm))) \
                        -print-libgcc-file-name 2> /dev/null)
-libstdc++$(sm) := $(shell $(CXX$(sm)) $(CXXFLAGS$(arch-bits-$(sm))) $(comp-cxxflags$(sm)) \
+libstdc++$(sm) := $(shell $(CXX$(sm)) $(LIBGCC_LOCATE_CFLAGS) $(CXXFLAGS$(arch-bits-$(sm))) $(comp-cxxflags$(sm)) \
                        -print-file-name=libstdc++.a 2> /dev/null)
-libgcc_eh$(sm) := $(shell $(CXX$(sm)) $(CXXFLAGS$(arch-bits-$(sm))) $(comp-cxxflags$(sm)) \
+libgcc_eh$(sm) := $(shell $(CXX$(sm)) $(LIBGCC_LOCATE_CFLAGS) $(CXXFLAGS$(arch-bits-$(sm))) $(comp-cxxflags$(sm)) \
                        -print-file-name=libgcc_eh.a 2> /dev/null)

 # Define these to something to discover accidental use

An upstream compatible one (probably allowing CC to be replaced), would indeed be nice to have.

jforissier commented 2 years ago

Update: please see https://github.com/OP-TEE/optee_os/pull/4829 which fixes bugs with how CFLAGS32 and CFLAGS64 are used in the build system. With this patch, you should be able to provide your sysroot parameter as CFLAGS32=--sysroot=/some/arm/sysroot and CFLAGS64=--sysroot=/some/aarch64/sysroot.

rfs613 commented 2 years ago

Update: please see #4829 which fixes bugs with how CFLAGS32 and CFLAGS64 are used in the build system. With this patch, you should be able to provide your sysroot parameter as CFLAGS32=--sysroot=/some/arm/sysroot and CFLAGS64=--sysroot=/some/aarch64/sysroot.

I've tested this against 5.15 (specifically a7b6b979). It appears to work correctly when building ldelf.elf and core.elf targets. I can see --sysroot parameter in the calls to gcc (in my case, passed via CFLAGS32).

However it fails when building ta/avb. No sysroot is passed, and the link fails with cannot find libgcc.a. I can work around this by setting CFLAGS=--sysroot=... in addition to setting it in CFLAGS32. I'm guessing however that this is not the intended way.

jforissier commented 2 years ago

Hi @rfs613,

Confirmed. Did you manage to figure out what's wrong?

optee_os $ make -s out/arm-plat-vexpress/ta/avb/entry.o CFLAGS32=--invalid
[...]
  CC      out/arm-plat-vexpress/ta/avb/entry.o
optee_os $
# No error: obviously CFLAGS32 wasn't used
jforissier commented 2 years ago

https://github.com/OP-TEE/optee_os/pull/5007

rfs613 commented 2 years ago

Confirmed. Did you manage to figure out what's wrong?

@jforissier I didn't quite get to the bottom of it, but your fix #5007 looks good to me.

jforissier commented 2 years ago

@rfs613 excellent, thanks for checking/testing.

Fermi-4 commented 1 year ago

Hitting issue mentioned here about can't find libgcc:

core-image-minimal fails building - cannot find libgcc.a on /meta-arm/recipes-security/optee/optee-os_git.bb:do_compile step:

| aarch64-poky-linux-ld.bfd: cannot find libgcc.a: No such file or directory
| make: *** [ldelf/link.mk:60: out/arm-plat-k3/ldelf/ldelf.elf] Error 1
| make: *** Waiting for unfinished jobs....
| aarch64-poky-linux-gcc -std=gnu11 -fdiagnostics-show-option -Wall -Wcast-align -Werror-implicit-function-declaration -Wextra -Wfloat-equal -Wformat-nonliteral -Wformat-security -Wformat=2 -Winit-self -Wmissing-declarations -Wmissing-format-attribute -Wmissing-include-dirs -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wshadow -Wstrict-prototypes -Wswitch-default -Wwrite-strings -Wno-missing-field-initializers -Wno-format-zero-length -Wno-c2x-extensions -Wdeclaration-after-statement -Wredundant-decls -Wold-style-definition -Wstrict-aliasing=2 -Wundef -Os -ffunction-sections -fdata-sections -pipe -g3 -fpie -mstrict-align -mno-outline-atomics -mgeneral-regs-only -fstack-protector-strong -MD -MF out/arm-plat-k3/lib/libunw/.unwind_arm32.o.d -MT out/arm-plat-k3/lib/libunw/unwind_arm32.o -nostdinc -isystem /home/fermi-4/Documents/linux/poky/build/tmp/work/am62xx_evm-poky-linux/optee-os/3.20.0+gitAUTOINC+8e74d47616-r0/recipe-sysroot-native/usr/bin/aarch64-poky-linux/../../lib/aarch64-poky-linux/gcc/aarch64-poky-linux/9.5.0/include -Ilib/libutils/isoc/include -Ilib/libutils/ext/include -Ilib/libmbedtls/include -Ilib/libmbedtls/mbedtls/include -Icore/lib/libtomcrypt/include -Icore/lib/libtomcrypt/src/headers -Icore/lib/libfdt/include -Ilib/libunw/include -Icore/arch/arm/plat-k3/. -Iout/arm-plat-k3/lib/libunw -D__KERNEL__ -Icore/include -include out/arm-plat-k3/include/generated/conf.h -Iout/arm-plat-k3/core/include -Icore/arch/arm/include -DARM64=1 -D__LP64__=1 -DTRACE_LEVEL=1 -Ildelf/include -Ilib/libutee/include -D__FILE_ID__=lib_libunw_unwind_arm32_c -c lib/libunw/unwind_arm32.c -o out/arm-plat-k3/lib/libunw/unwind_arm32.o
| aarch64-poky-linux-objcopy --rename-section .rodata=.rodata.core/tee/fs_htree.c --rename-section .rodata.str1.1=.rodata.str1.1.core/tee/fs_htree.c out/arm-plat-k3/core/tee/fs_htree.o
| aarch64-poky-linux-objcopy --rename-section .rodata=.rodata.lib/libunw/unwind_arm32.c --rename-section .rodata.str1.1=.rodata.str1.1.lib/libunw/unwind_arm32.c out/arm-plat-k3/lib/libunw/unwind_arm32.o
| aarch64-poky-linux-objcopy --rename-section .rodata=.rodata.core/tee/tee_svc_cryp.c --rename-section .rodata.str1.1=.rodata.str1.1.core/tee/tee_svc_cryp.c out/arm-plat-k3/core/tee/tee_svc_cryp.o
| ERROR: oe_runmake failed
| WARNING: exit code 1 from a shell command.
| ERROR: Execution of '/home/fermi-4/Documents/linux/poky/build/tmp/work/am62xx_evm-poky-linux/optee-os/3.20.0+gitAUTOINC+8e74d47616-r0/temp/run.do_compile.3776566' failed with exit code 1
ERROR: Task (/home/fermi-4/Documents/linux/poky/meta-arm/meta-arm/recipes-security/optee/optee-os_git.bb:do_compile) failed with exit code '1'
NOTE: Tasks Summary: Attempted 4354 tasks of which 2852 didn't need to be rerun and 1 failed.

Summary: 1 task failed:
  /home/fermi-4/Documents/linux/poky/meta-arm/meta-arm/recipes-security/optee/optee-os_git.bb:do_compile
Summary: There were 2 ERROR messages shown, returning a non-zero exit code.

make call:

Log data follows:
| DEBUG: Executing shell function do_compile
| NOTE: make -j 4 PLATFORM=k3-am62x CFG_ARM64_core=y CROSS_COMPILE_core=aarch64-poky-linux- CROSS_COMPILE_ta_arm64=aarch64-poky-linux- NOWERROR=1 V=1 ta-targets=ta_arm64 LIBGCC_LOCATE_CFLAGS=--sysroot=/home/fermi-4/Documents/linux/poky/build/tmp/work/am62xx_evm-poky-linux/optee-os/3.20.0+gitAUTOINC+8e74d47616-r0/recipe-sysroot CFG_WITH_SOFTWARE_PRNG=y CFG_TEE_CORE_LOG_LEVEL=1 all CFG_TEE_TA_LOG_LEVEL=0
rossburton commented 1 year ago

@Fermi-4 What version of meta-arm is this? We test optee constantly in the supported releases.

Fermi-4 commented 1 year ago

@rossburton Sry I forgot to mention - I am on dunfell branch:

b1fe8443 (HEAD -> dunfell, tag: yocto-3.1.6, tag: 3.1.6, origin/dunfell) CI: pin to kas 3.2 as 3.2.1 fails
rossburton commented 10 months ago

I just re-tested optee 3.22.0 without our explicit "set the sysroot" and if we pass --sysroot in CFLAGS then it builds, thanks.

jforissier commented 10 months ago

Thanks for reporting back