| Message ID | 20240326202001.1075940-1-mathieu.poirier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] arm/trusted-firmware-rmm: Add bitbake, include and patch file for RMM | expand |
Hi Mattieu, On 26 Mar 2024, at 20:20, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > +SRCREV_rmm ?= "0a02656945d69757b0779192cebb9b41dd9037d1" > +SRCBRANCH_rmm ?= "main" > + > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > + file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ > + " Is there a good reason for all of this indirection? If you want to support building from a local source tree/fork, there’s a class that does that generally (externalsrc). > +RMM_CONFIG ?= "qemu_virt_defcfg" The “default” isn’t qemu, as that isn’t useful for anything but qemu. A better solution would be: RMM_CONFIG ?= “" RMM_CONFIG:qemuarm64 = “qemu_virt_defcfg” Also this recipe needs some COMPATIBLE_MACHINE statements, so that a world build won’t try and build it for eg qemux86-64. > +B = "${WORKDIR}/build" cmake class sets B already, remove. > +# Supplement include path > +EXTRA_OECMAKE += "-DCMAKE_INCLUDE_PATH=${WORKDIR}/recipe-sysroot/usr/include" -DCMAKE_INCLUDE_PATH=${STAGING_INCDIR} would be neater, but this really does feel like a bug in the CMakeLists.txt. Why doesn’t it respect the sysroot that the compiler is told to use? We carefully configure a toolchain that specifies exactly what compiler, sysroot, flags, etc to use. > +export CROSS_COMPILE="${TARGET_PREFIX}" > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" CMake class should be doing this, remove. Ross
Hi Mattieu, On 26 Mar 2024, at 20:20, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > +SRCREV_rmm ?= "0a02656945d69757b0779192cebb9b41dd9037d1" > +SRCBRANCH_rmm ?= "main" > + > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > + file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ > + " Is there a good reason for all of this indirection? If you want to support building from a local source tree/fork, there’s a class that does that generally (externalsrc). > +RMM_CONFIG ?= "qemu_virt_defcfg" The “default” isn’t qemu, as that isn’t useful for anything but qemu. A better solution would be: RMM_CONFIG ?= “" RMM_CONFIG:qemuarm64 = “qemu_virt_defcfg” Also this recipe needs some COMPATIBLE_MACHINE statements, so that a world build won’t try and build it for eg qemux86-64. > +B = "${WORKDIR}/build" cmake class sets B already, remove. > +# Supplement include path > +EXTRA_OECMAKE += "-DCMAKE_INCLUDE_PATH=${WORKDIR}/recipe-sysroot/usr/include" -DCMAKE_INCLUDE_PATH=${STAGING_INCDIR} would be neater, but this really does feel like a bug in the CMakeLists.txt. Why doesn’t it respect the sysroot that the compiler is told to use? We carefully configure a toolchain that specifies exactly what compiler, sysroot, flags, etc to use. > +export CROSS_COMPILE="${TARGET_PREFIX}" > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" CMake class should be doing this, remove. Ross
Hi, On Wed, Mar 27, 2024 at 01:04:52PM +0000, Ross Burton wrote: <nip> > > +# Supplement include path > > +EXTRA_OECMAKE += "-DCMAKE_INCLUDE_PATH=${WORKDIR}/recipe-sysroot/usr/include" > > -DCMAKE_INCLUDE_PATH=${STAGING_INCDIR} would be neater, but this really does feel like a bug in the CMakeLists.txt. Why doesn’t it respect the sysroot that the compiler is told to use? We carefully configure a toolchain that specifies exactly what compiler, sysroot, flags, etc to use. > > > +export CROSS_COMPILE="${TARGET_PREFIX}" > > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > > CMake class should be doing this, remove. Sadly RMM build scripts really don't want to support compiling with an external CMake toolchain file: cmake/Toolchains.cmake if(DEFINED CACHE{CMAKE_TOOLCHAIN_FILE} AND NOT DEFINED RMM_TOOLCHAIN) message(WARNING "The RMM project does not support `CMAKE_TOOLCHAIN_FILE` directly. " "Please use `RMM_TOOLCHAIN` to configure your desired toolchain.") unset(CMAKE_TOOLCHAIN_FILE CACHE) endif() And then things like: $ git grep -i set.*_compiler toolchains/aarch64/llvm.cmake:set(CMAKE_ASM_COMPILER ${CMAKE_C_COMPILER}) toolchains/aarch64/llvm.cmake: set(CMAKE_${language}_COMPILER_TARGET "${A64-GCC-TRIPLET}") toolchains/fake_host/gnu.cmake:set(CMAKE_ASM_COMPILER ${CMAKE_C_COMPILER}) toolchains/fake_host/llvm.cmake:set(CMAKE_ASM_COMPILER ${CMAKE_C_COMPILER}) One way to solve these could be to patch all these away even if upstream would reject that approach: --- a/toolchains/aarch64/gnu.cmake +++ b/toolchains/aarch64/gnu.cmake @@ -7,12 +7,3 @@ include_guard() include(${CMAKE_CURRENT_LIST_DIR}/common_aarch64.cmake) -find_program(CMAKE_ASM_COMPILER - NAMES "$ENV{CROSS_COMPILE}gcc" - DOC "Path to an aarch64 compiler." - REQUIRED) - -find_program(CMAKE_C_COMPILER - NAMES "$ENV{CROSS_COMPILE}gcc" - DOC "Path to an aarch64 compiler." - REQUIRED) --- a/toolchains/common.cmake +++ b/toolchains/common.cmake @@ -5,12 +5,6 @@ include_guard() -set(CMAKE_SYSTEM_NAME "Generic") -set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER) -set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY) -set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY) -set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY) - foreach(language IN ITEMS ASM C CXX) string(APPEND CMAKE_${language}_FLAGS_INIT "-fno-common ") string(APPEND CMAKE_${language}_FLAGS_INIT "-ffunction-sections ") --- a/toolchains/fake_host/gnu.cmake +++ b/toolchains/fake_host/gnu.cmake @@ -7,19 +7,4 @@ include_guard() include(${CMAKE_CURRENT_LIST_DIR}/common_fake_host.cmake) -find_program(CMAKE_C_COMPILER - NAMES "gcc" - DOC "Path to gcc." - REQUIRED) - -# -# Needed to build CppUTest for unit tests -# -find_program(CMAKE_CXX_COMPILER - NAMES "g++" - DOC "Path to g++." - REQUIRED) - -set(CMAKE_ASM_COMPILER ${CMAKE_C_COMPILER}) - string(APPEND CMAKE_EXE_LINKER_FLAGS_INIT "-Wl,--build-id=none ") These fix standard cmake.bbclass usage though there are still some issues with -march=armv8.5-a support for the compatible machine, and stack protection which could be hacked around with SECURITY_STACK_PROTECTOR = "" which disables stack protection for this recipe. Cheers, -Mikko
On 27 Mar 2024, at 13:42, Mikko Rapeli <mikko.rapeli@linaro.org> wrote: >> CMake class should be doing this, remove. > > Sadly RMM build scripts really don't want to support compiling > with an external CMake toolchain file: Joy. One day I need to sit down with a CMake expert and design the one true way for eg firmware to build using relatively standardised toolchain files. TF-M does use its own toolchain files but mixes basic logic like “where the compiler is” with innate TF-M knowledge, which doesn’t help. Maybe in this case the solution is to not inherit cmake and reinvent the wheel: the class will be doing a lot of things, but most of them are being ignored. Ross
On Wed, 27 Mar 2024 at 09:06, Ross Burton <Ross.Burton@arm.com> wrote: > > On 27 Mar 2024, at 13:42, Mikko Rapeli <mikko.rapeli@linaro.org> wrote: > >> CMake class should be doing this, remove. > > > > Sadly RMM build scripts really don't want to support compiling > > with an external CMake toolchain file: > > Joy. > > One day I need to sit down with a CMake expert and design the one true way for eg firmware to build using relatively standardised toolchain files. TF-M does use its own toolchain files but mixes basic logic like “where the compiler is” with innate TF-M knowledge, which doesn’t help. > > Maybe in this case the solution is to not inherit cmake and reinvent the wheel: the class will be doing a lot of things, but most of them are being ignored. > May I ask you to be more precise on what you're expecting here? Otherwise I fear my next revision will completely miss the mark. Thanks, Mathieu > Ross
On 27 Mar 2024, at 15:13, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: >> Maybe in this case the solution is to not inherit cmake and reinvent the wheel: the class will be doing a lot of things, but most of them are being ignored. >> > > May I ask you to be more precise on what you're expecting here? > Otherwise I fear my next revision will completely miss the mark. Without writing the recipe twice it’s hard to gauge exactly the right thing to do. But essentially the cmake class is very useful for “normal” recipes but this isn’t a normal recipe, so the class brings lots of features that the recipe doesn’t use, and others that the recipe needs to replace anyway. I’m wondering whether the convenience of using the class is outweighed by having to fight it to do what the recipe wants. Until TF-* have a standardised way of building that is friendly with standard CMake toolchain files, each of these recipes may well have to implement the cmake glue themselves. The recipe _appears_ to mostly work with the class (which is better than some other pieces of firmware), although I’m still surprised you need to pass include paths explicitly. You’ll likely need to keep the CROSS_COMPILE assignment (please add a comment explaining that the package ignores the toolchain) but CMAKE_BUILD_PARALLEL_LEVEL should be able to be removed. Ross
Hi Ross, On Wed, 27 Mar 2024 at 07:05, Ross Burton <Ross.Burton@arm.com> wrote: > > Hi Mattieu, > > On 26 Mar 2024, at 20:20, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > > +SRCREV_rmm ?= "0a02656945d69757b0779192cebb9b41dd9037d1" > > +SRCBRANCH_rmm ?= "main" > > + > > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > > + file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ > > + " > > Is there a good reason for all of this indirection? If you want to support building from a local source tree/fork, there’s a class that does that generally (externalsrc). > I have addressed all your comments except the one above related to the externalsrc classe. What I currently have in the recipe follows what is found in trusted-firmware-a and trusted-firmware-m. Moreover, the documentation in the externalsrc class mentions it is to "build a piece of software rather than the usual fetch/unpack/patch process". But for RMM we need to fetch and patch... I am puzzled as to how to proceed - can you provide more details on what you expect? Thanks, Mathieu > > +RMM_CONFIG ?= "qemu_virt_defcfg" > > The “default” isn’t qemu, as that isn’t useful for anything but qemu. A better solution would be: > > RMM_CONFIG ?= “" > RMM_CONFIG:qemuarm64 = “qemu_virt_defcfg” > > Also this recipe needs some COMPATIBLE_MACHINE statements, so that a world build won’t try and build it for eg qemux86-64. > > > +B = "${WORKDIR}/build" > > cmake class sets B already, remove. > > > +# Supplement include path > > +EXTRA_OECMAKE += "-DCMAKE_INCLUDE_PATH=${WORKDIR}/recipe-sysroot/usr/include" > > -DCMAKE_INCLUDE_PATH=${STAGING_INCDIR} would be neater, but this really does feel like a bug in the CMakeLists.txt. Why doesn’t it respect the sysroot that the compiler is told to use? We carefully configure a toolchain that specifies exactly what compiler, sysroot, flags, etc to use. > > > +export CROSS_COMPILE="${TARGET_PREFIX}" > > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > > CMake class should be doing this, remove. > > Ross
On Mon, Apr 01, 2024 at 02:27:42PM -0600, Mathieu Poirier wrote: > Hi Ross, > > On Wed, 27 Mar 2024 at 07:05, Ross Burton <Ross.Burton@arm.com> wrote: > > > > Hi Mattieu, > > > > On 26 Mar 2024, at 20:20, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > > > +SRCREV_rmm ?= "0a02656945d69757b0779192cebb9b41dd9037d1" > > > +SRCBRANCH_rmm ?= "main" > > > + > > > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > > > + file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ > > > + " > > > > Is there a good reason for all of this indirection? If you want to support building from a local source tree/fork, there’s a class that does that generally (externalsrc). > > > > I have addressed all your comments except the one above related to the > externalsrc classe. What I currently have in the recipe follows what > is found in trusted-firmware-a and trusted-firmware-m. Moreover, the > documentation in the externalsrc class mentions it is to "build a > piece of software rather than the usual fetch/unpack/patch process". > But for RMM we need to fetch and patch... > > I am puzzled as to how to proceed - can you provide more details on > what you expect? IIUC, he is saying the above should be: SRC_URI = "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https;branch=main \ file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ " SRCREV = "0a02656945d69757b0779192cebb9b41dd9037d1" All the "_rmm" indirection is only needed if you are going to have multiple sources. Since this only has one, it just adds unnecessary complexity. Similarily, the "?=" is only needed if there is going to be an override of the values in those fields. Since this is all in one file and nothing is referencing this file, this shouldn't be neessary. TF-A and TF-M are based on ".inc" files that can (and do) have multiple "versioned" files. For example, ./meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.9.0.bb ./meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.8.6.bb ./meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.10.2.bb Each one of those is just the minor modifications to get that version working, with a core ".inc" file doing all the work. Since rmm is a more simple recipe, none of that is needed here. Thanks, Jon > > Thanks, > Mathieu > > > > +RMM_CONFIG ?= "qemu_virt_defcfg" > > > > The “default” isn’t qemu, as that isn’t useful for anything but qemu. A better solution would be: > > > > RMM_CONFIG ?= “" > > RMM_CONFIG:qemuarm64 = “qemu_virt_defcfg” > > > > Also this recipe needs some COMPATIBLE_MACHINE statements, so that a world build won’t try and build it for eg qemux86-64. > > > > > +B = "${WORKDIR}/build" > > > > cmake class sets B already, remove. > > > > > +# Supplement include path > > > +EXTRA_OECMAKE += "-DCMAKE_INCLUDE_PATH=${WORKDIR}/recipe-sysroot/usr/include" > > > > -DCMAKE_INCLUDE_PATH=${STAGING_INCDIR} would be neater, but this really does feel like a bug in the CMakeLists.txt. Why doesn’t it respect the sysroot that the compiler is told to use? We carefully configure a toolchain that specifies exactly what compiler, sysroot, flags, etc to use. > > > > > +export CROSS_COMPILE="${TARGET_PREFIX}" > > > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > > > > CMake class should be doing this, remove. > > > > Ross >
On Tue, 2 Apr 2024 at 12:05, Jon Mason <jdmason@kudzu.us> wrote: > > On Mon, Apr 01, 2024 at 02:27:42PM -0600, Mathieu Poirier wrote: > > Hi Ross, > > > > On Wed, 27 Mar 2024 at 07:05, Ross Burton <Ross.Burton@arm.com> wrote: > > > > > > Hi Mattieu, > > > > > > On 26 Mar 2024, at 20:20, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > > > > +SRCREV_rmm ?= "0a02656945d69757b0779192cebb9b41dd9037d1" > > > > +SRCBRANCH_rmm ?= "main" > > > > + > > > > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > > > > + file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ > > > > + " > > > > > > Is there a good reason for all of this indirection? If you want to support building from a local source tree/fork, there’s a class that does that generally (externalsrc). > > > > > > > I have addressed all your comments except the one above related to the > > externalsrc classe. What I currently have in the recipe follows what > > is found in trusted-firmware-a and trusted-firmware-m. Moreover, the > > documentation in the externalsrc class mentions it is to "build a > > piece of software rather than the usual fetch/unpack/patch process". > > But for RMM we need to fetch and patch... > > > > I am puzzled as to how to proceed - can you provide more details on > > what you expect? > > IIUC, he is saying the above should be: > > SRC_URI = "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https;branch=main \ > file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ > " > SRCREV = "0a02656945d69757b0779192cebb9b41dd9037d1" > > > All the "_rmm" indirection is only needed if you are going to have > multiple sources. Since this only has one, it just adds unnecessary > complexity. Similarily, the "?=" is only needed if there is going to > be an override of the values in those fields. Since this is all in > one file and nothing is referencing this file, this shouldn't be > neessary. Ok, but in my case I have a .bbappend file that does the following: SRC_URI_RMM = "gitsm://git.codelinaro.org/linaro/dcap/rmm.git;protocol=https" SRCREV_rmm = "3212a781573db80df815db4b0e493c87f734840c" SRCBRANCH_rmm = "rmm-v1.0-eac5" Should I keep my current solution as I submitted it, keep the current solution but get rid of the {_rmm | RMM] or use what you have above and redefine SRC_URI and SRCREV in the .bbappend file? > > TF-A and TF-M are based on ".inc" files that can (and do) have > multiple "versioned" files. For example, > ./meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.9.0.bb > ./meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.8.6.bb > ./meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.10.2.bb > > Each one of those is just the minor modifications to get that version > working, with a core ".inc" file doing all the work. Since rmm is a > more simple recipe, none of that is needed here. > > Thanks, > Jon > > > > > > Thanks, > > Mathieu > > > > > > +RMM_CONFIG ?= "qemu_virt_defcfg" > > > > > > The “default” isn’t qemu, as that isn’t useful for anything but qemu. A better solution would be: > > > > > > RMM_CONFIG ?= “" > > > RMM_CONFIG:qemuarm64 = “qemu_virt_defcfg” > > > > > > Also this recipe needs some COMPATIBLE_MACHINE statements, so that a world build won’t try and build it for eg qemux86-64. > > > > > > > +B = "${WORKDIR}/build" > > > > > > cmake class sets B already, remove. > > > > > > > +# Supplement include path > > > > +EXTRA_OECMAKE += "-DCMAKE_INCLUDE_PATH=${WORKDIR}/recipe-sysroot/usr/include" > > > > > > -DCMAKE_INCLUDE_PATH=${STAGING_INCDIR} would be neater, but this really does feel like a bug in the CMakeLists.txt. Why doesn’t it respect the sysroot that the compiler is told to use? We carefully configure a toolchain that specifies exactly what compiler, sysroot, flags, etc to use. > > > > > > > +export CROSS_COMPILE="${TARGET_PREFIX}" > > > > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > > > > > > CMake class should be doing this, remove. > > > > > > Ross > >
On Wed, Apr 03, 2024 at 10:40:42AM -0600, Mathieu Poirier wrote: > On Tue, 2 Apr 2024 at 12:05, Jon Mason <jdmason@kudzu.us> wrote: > > > > On Mon, Apr 01, 2024 at 02:27:42PM -0600, Mathieu Poirier wrote: > > > Hi Ross, > > > > > > On Wed, 27 Mar 2024 at 07:05, Ross Burton <Ross.Burton@arm.com> wrote: > > > > > > > > Hi Mattieu, > > > > > > > > On 26 Mar 2024, at 20:20, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > > > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > > > > > +SRCREV_rmm ?= "0a02656945d69757b0779192cebb9b41dd9037d1" > > > > > +SRCBRANCH_rmm ?= "main" > > > > > + > > > > > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > > > > > + file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ > > > > > + " > > > > > > > > Is there a good reason for all of this indirection? If you want to support building from a local source tree/fork, there’s a class that does that generally (externalsrc). > > > > > > > > > > I have addressed all your comments except the one above related to the > > > externalsrc classe. What I currently have in the recipe follows what > > > is found in trusted-firmware-a and trusted-firmware-m. Moreover, the > > > documentation in the externalsrc class mentions it is to "build a > > > piece of software rather than the usual fetch/unpack/patch process". > > > But for RMM we need to fetch and patch... > > > > > > I am puzzled as to how to proceed - can you provide more details on > > > what you expect? > > > > IIUC, he is saying the above should be: > > > > SRC_URI = "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https;branch=main \ > > file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ > > " > > SRCREV = "0a02656945d69757b0779192cebb9b41dd9037d1" > > > > > > All the "_rmm" indirection is only needed if you are going to have > > multiple sources. Since this only has one, it just adds unnecessary > > complexity. Similarily, the "?=" is only needed if there is going to > > be an override of the values in those fields. Since this is all in > > one file and nothing is referencing this file, this shouldn't be > > neessary. > > Ok, but in my case I have a .bbappend file that does the following: > > SRC_URI_RMM = > "gitsm://git.codelinaro.org/linaro/dcap/rmm.git;protocol=https" > SRCREV_rmm = "3212a781573db80df815db4b0e493c87f734840c" > SRCBRANCH_rmm = "rmm-v1.0-eac5" > > Should I keep my current solution as I submitted it, keep the current > solution but get rid of the {_rmm | RMM] or use what you have above > and redefine SRC_URI and SRCREV in the .bbappend file? If you are asking me if you should change your bbappend to match the patch you are going to submit, I would say to change your bbappend. As I expect the next version should be accepted. The example above (of your bbappend) would be a simple overwrite of the SRC_URI, because you are not reusing the git tree or branch. Thanks, Jon > > > > > TF-A and TF-M are based on ".inc" files that can (and do) have > > multiple "versioned" files. For example, > > ./meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.9.0.bb > > ./meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.8.6.bb > > ./meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.10.2.bb > > > > Each one of those is just the minor modifications to get that version > > working, with a core ".inc" file doing all the work. Since rmm is a > > more simple recipe, none of that is needed here. > > > > Thanks, > > Jon > > > > > > > > > > Thanks, > > > Mathieu > > > > > > > > +RMM_CONFIG ?= "qemu_virt_defcfg" > > > > > > > > The “default” isn’t qemu, as that isn’t useful for anything but qemu. A better solution would be: > > > > > > > > RMM_CONFIG ?= “" > > > > RMM_CONFIG:qemuarm64 = “qemu_virt_defcfg” > > > > > > > > Also this recipe needs some COMPATIBLE_MACHINE statements, so that a world build won’t try and build it for eg qemux86-64. > > > > > > > > > +B = "${WORKDIR}/build" > > > > > > > > cmake class sets B already, remove. > > > > > > > > > +# Supplement include path > > > > > +EXTRA_OECMAKE += "-DCMAKE_INCLUDE_PATH=${WORKDIR}/recipe-sysroot/usr/include" > > > > > > > > -DCMAKE_INCLUDE_PATH=${STAGING_INCDIR} would be neater, but this really does feel like a bug in the CMakeLists.txt. Why doesn’t it respect the sysroot that the compiler is told to use? We carefully configure a toolchain that specifies exactly what compiler, sysroot, flags, etc to use. > > > > > > > > > +export CROSS_COMPILE="${TARGET_PREFIX}" > > > > > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > > > > > > > > CMake class should be doing this, remove. > > > > > > > > Ross > > > >
diff --git a/meta-arm/recipes-bsp/trusted-firmware-rmm/files/0001-build-lib-Add-extra-repositories-for-system-includes.patch b/meta-arm/recipes-bsp/trusted-firmware-rmm/files/0001-build-lib-Add-extra-repositories-for-system-includes.patch new file mode 100644 index 000000000000..7c3e637f0d63 --- /dev/null +++ b/meta-arm/recipes-bsp/trusted-firmware-rmm/files/0001-build-lib-Add-extra-repositories-for-system-includes.patch @@ -0,0 +1,56 @@ +From bc7dbac20a6674eb2834bd6176665f1a2ae42edc Mon Sep 17 00:00:00 2001 +From: Mathieu Poirier <mathieu.poirier@linaro.org> +Date: Thu, 14 Mar 2024 14:59:30 -0600 +Subject: [PATCH] build(lib): Add extra repositories for system includes + +Toolchains such as aarch64-none-elf, aarch64-none-linux-gnu and +aarch64-linux-gnu include assert.h and limits.h in a directory that is +part of their search path. This is not the case when compiling with +Yocto where aarch64-poky-linux places those files in the sysroot +directory of the component being compiled. + +Since the sysroot directory of the component is not part of the cmake +search path, compiling the RMM in Yocto fails. This patch fixes the +problem by expanding the search path when needed, allowing the RMM to be +compiled in Yocto. + +Upstream-Status: Backport [bc7dbac20a6674eb2834bd6176665f1a2ae42edc] +Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> +--- + lib/arch/CMakeLists.txt | 3 +++ + lib/libc/CMakeLists.txt | 5 ++++- + 2 files changed, 7 insertions(+), 1 deletion(-) + +diff --git a/lib/arch/CMakeLists.txt b/lib/arch/CMakeLists.txt +index d3afc5f2bfc8..a52185f02695 100644 +--- a/lib/arch/CMakeLists.txt ++++ b/lib/arch/CMakeLists.txt +@@ -12,6 +12,9 @@ target_link_libraries(rmm-lib-arch + target_include_directories(rmm-lib-arch + PUBLIC "include" + "include/${RMM_ARCH}" ++ # The CMAKE_INCLUDE_PATH is included here for Yocto builds. the ++ # Yocto recipe will define this variable as part of the build. ++ ${CMAKE_INCLUDE_PATH} + PRIVATE "src/${RMM_ARCH}" + "src/include") + +diff --git a/lib/libc/CMakeLists.txt b/lib/libc/CMakeLists.txt +index 1631332dbc72..a2adf37f7cb8 100644 +--- a/lib/libc/CMakeLists.txt ++++ b/lib/libc/CMakeLists.txt +@@ -12,7 +12,10 @@ if(NOT RMM_ARCH STREQUAL fake_host) + rmm-lib-debug) + + target_include_directories(rmm-lib-libc SYSTEM +- PUBLIC "include") ++ PUBLIC "include" ++ # The CMAKE_INCLUDE_PATH is included here for Yocto builds. the ++ # Yocto recipe will define this variable as part of the build. ++ ${CMAKE_INCLUDE_PATH}) + + target_sources(rmm-lib-libc + PRIVATE "src/abort.c" +-- +2.34.1 + diff --git a/meta-arm/recipes-bsp/trusted-firmware-rmm/trusted-firmware-rmm_0.4.bb b/meta-arm/recipes-bsp/trusted-firmware-rmm/trusted-firmware-rmm_0.4.bb new file mode 100644 index 000000000000..d8a802a5175f --- /dev/null +++ b/meta-arm/recipes-bsp/trusted-firmware-rmm/trusted-firmware-rmm_0.4.bb @@ -0,0 +1,50 @@ +SUMMARY = "RMM Firmware" +DESCRIPTION = "RMM Firmware for Arm reference platforms" +LICENSE = "BSD-3-Clause & MIT" + +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" +SRCREV_rmm ?= "0a02656945d69757b0779192cebb9b41dd9037d1" +SRCBRANCH_rmm ?= "main" + +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ + file://0001-build-lib-Add-extra-repositories-for-system-includes.patch \ + " + +LIC_FILES_CHKSUM += "file://docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" + +inherit deploy cmake + +RMM_CONFIG ?= "qemu_virt_defcfg" + +PACKAGE_ARCH = "${MACHINE_ARCH}" + +S = "${WORKDIR}/git" +B = "${WORKDIR}/build" + +# Build for debug (set RMM_DEBUG to 1 to activate) +RMM_DEBUG ?= "0" +RMM_BUILD_MODE ?= "${@bb.utils.contains('RMM_DEBUG', '1', 'Debug', 'Release', d)}" + +# Handle RMM_DEBUG parameter +EXTRA_OECMAKE += "-DCMAKE_BUILD_TYPE=${RMM_BUILD_MODE}" +EXTRA_OECMAKE += "-DRMM_CONFIG=${RMM_CONFIG}" + +# Supplement include path +EXTRA_OECMAKE += "-DCMAKE_INCLUDE_PATH=${WORKDIR}/recipe-sysroot/usr/include" + +export CROSS_COMPILE="${TARGET_PREFIX}" +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" + +do_install() { + install -d -m 755 ${D}/firmware + install -m 0644 ${B}/${RMM_BUILD_MODE}/* ${D}/firmware/ +} + +FILES:${PN} = "/firmware" +SYSROOT_DIRS += "/firmware" + +do_deploy() { + cp -rf ${D}/firmware/* ${DEPLOYDIR}/ +} + +addtask deploy after do_install
Initial checking providing support for RMM on QEMU's "virt" machine. Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> --- Changes for V2: 1) Added an "Upstream-Status" tag to the patch 2) Fixed LIC_FILES_CHKSUM variable to conform to Yocto declarations 3) Using the cmake class for configuration and compilation 4) Set the recipe's SHA to align with RMM release 0.4.0 5) Using "trusted-firmware-rmm" rather than "rmm" --- ...tra-repositories-for-system-includes.patch | 56 +++++++++++++++++++ .../trusted-firmware-rmm_0.4.bb | 50 +++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 meta-arm/recipes-bsp/trusted-firmware-rmm/files/0001-build-lib-Add-extra-repositories-for-system-includes.patch create mode 100644 meta-arm/recipes-bsp/trusted-firmware-rmm/trusted-firmware-rmm_0.4.bb