fvp: add path to clang for hafnium build

[jenswi-linaro]

Set PATH to include the directory with the clang toolchain when building Hafnium.

[jenswi-linaro]

@odeprez

[odeprez]

Hi Jens,

Does this mean we expect FVP users to build clang everytime? This is a quite long and tedious process and to my understanding a developer would prefer adding clang to PATH manually, or source it from docker in the CI. I suspect with this change it would break latter assumption. I know FVP (using SPMC_AT_EL2=1) isn't part of the CI but perhaps it should be?

@jforissier if you have some perspective on how it should be done?

[jenswi-linaro]

If you want to save time you do workarounds, but if you want it to just work you shouldn't need to do workarounds. My workaround to save time is to make all the toolchains once when they have been updated and then move the toolchains directory out of the repo directory and add a symbolic link to the toolchain directory.

[jenswi-linaro]

By the way, this fix doesn't prevent someone else from having clang in the PATH as you describe.

[jforissier]

There may be a misunderstanding about the meaning of "build the toolchain". Is it "download the toolchain" or "build it from source"? The answer depends on which version of Clang is needed (12 or 18).

• $(TOOLCHAIN_ROOT)/clang-$(CLANG_BUILD_VER)/bin points to what is downloaded when running make clang-toolchains in build/, and that is Clang 12 currently. It does not take much time because it is a pre-built toolchain.
• On the other hand, if a more recent version is needed, I found no prebuilt toolchain that includes the proper builtins library. So we can't just update the "make clang-toolchains" command easily. That is why I added "make clang-toolchains-build" in https://github.com/OP-TEE/build/pull/765, then subsequently added that toolchain in the CI image in the default PATH for convenience. That is Clang 18. At some point I suppose we will need to get rid of Clang 12 and find a way for "make clang-toolchains" to download a proper prebuilt binary from somewhere. Perhaps we should tell Arm that the toolchain binary should contain both the armhf and aarch64 compiler-rt library(ies)?

[jforissier]

I know FVP (using SPMC_AT_EL2=1) isn't part of the CI but perhaps it should be?

Well, it should not be too hard to add a FVP job to optee_os/.github/workflows/ci.yml. But we would need a dedicated Docker image with the FVP binaries I suppose. Unless we add the FVP to the qemu_check image.

[odeprez]

Hi,

If you want to save time you do workarounds, but if you want it to just work you shouldn't need to do workarounds

sorry I'm not talking about workarounds, but a scalable way to build the SPMC_AT_EL2=1 configuration (applies to FVP but others like qemu_v8 in the long run) presently there are 3 different ways to find clang: download clang 12 prebuilt, build clang 18 from source, or use clang 18 from the docker image.

Clang 12 currently.

jfyi this can't build TF-A/Hafnium from v2.11 onwards

if a more recent version is needed [...] That is why I added "make clang-toolchains-build"

yes this works, but as you pointed out elsewhere this is unpractical if the CI needs to build it on each run, or for the developer if he needs to build the toolchain himself.

then subsequently added that toolchain in the CI image in the default PATH for convenience. That is Clang 18.

yes this works as well however , the configuration in question FVP SPMC_AT_EL2=1 isn't built in the CI so I'm not sure there is any usage presently?

At some point I suppose we will need to get rid of Clang 12 and find a way for "make clang-toolchains" to download a proper prebuilt binary from somewhere. Perhaps we should tell Arm that the toolchain binary should contain both the armhf and aarch64 compiler-rt library(ies)?

All in all, I don't really mind, it's really OP-TEE maintainers call (you!)

I'm just hinting there are different ways to get a clang compiler most likely fitting different usages. make clang-toolchains (download clang 12 prebuilt but cannot be used anymore for mentioned configuration) or make clang-toolchains-build (build clang 18 from source and manually add to PATH), or use clang 18 from the docker image (I guess for CI usage)

The platform makefile might have to choose the latter (clang 18 default in PATH) if the intent is to run in the CI some day. that was the approach I thought about when committing earlier FVP changes.

Well, it should not be too hard to add a FVP job to optee_os/.github/workflows/ci.yml. But we would need a dedicated Docker image with the FVP binaries I suppose. Unless we add the FVP to the qemu_check image.

For starters it could just be a build configuration, no need for FVP then.

[jenswi-linaro]

Our make files in this build git are supposed to be convenient enough to be usable without wrapper scripts, they are our wrapper scripts. Typing PATH=<path-to-an-odd-place>:${PATH} make SPMC_AT_EL=2 run is a bit more to type than I'd like. But even with this patch, it's still possible to do if CI or whatever for some reason can't provide toolchains in the expected directories.

[odeprez]

Typing PATH=:${PATH} make SPMC_AT_EL=2 run is a bit more to type than I'd like. But even with this patch, it's still possible to do if CI or whatever for some reason can't provide toolchains in the expected directories.

Notice this is already what you must do if you want to build optee_os with clang (aka COMPILER=clang), clang toolchain must be present in PATH as a prerequisite and that's what I intended to mimic for building hafnium.

[jenswi-linaro]

I want to avoid typing more than necessary. Is this patch a problem?

[jforissier]

I want to avoid typing more than necessary. Is this patch a problem?

It is totally fine with me:

Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

[jenswi-linaro]

Tag applied

[odeprez]

Ok to me!

[jenswi-linaro]

Ready to merge?