| Message ID | 20240304231639.4187147-2-mathieu.poirier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Support for Arm RMM Firmware | expand |
On Mon, Mar 04, 2024 at 04:16:39PM -0700, Mathieu Poirier wrote: > Initial checking providing support for RMM on QEMU's "virt" machine. > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > --- > ...Add-repositories-for-system-includes.patch | 45 ++++++++++++++++ > meta-arm/recipes-bsp/rmm/rmm.inc | 54 +++++++++++++++++++ > meta-arm/recipes-bsp/rmm/rmm_1.0.bb | 5 ++ > 3 files changed, 104 insertions(+) > create mode 100644 meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > create mode 100644 meta-arm/recipes-bsp/rmm/rmm.inc > create mode 100644 meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > diff --git a/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > new file mode 100644 > index 000000000000..a2f2137012d3 > --- /dev/null > +++ b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > @@ -0,0 +1,45 @@ > +From e9e6d5ded4c3e53254438a73a79f04209035603e Mon Sep 17 00:00:00 2001 > +From: Mathieu Poirier <mathieu.poirier@linaro.org> > +Date: Mon, 4 Mar 2024 13:38:03 -0700 > +Subject: [PATCH] CMakeLists: Add repositories for system includes > + > +Yocto's default aarch64 toolchain, i.e aarch64-poky-linux-, doesn't > +include assert.h and stderr.h by default, leading to a compilation error > +for files that need them. As such add the path of the base sysroot > +directory to the list of directories to be included when compiling. > + > +Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> This needs an upstream status. See https://docs.yoctoproject.org/current/contributor-guide/recipe-style-guide.html#patch-upstream-status > +--- > + lib/arch/CMakeLists.txt | 1 + > + lib/libc/CMakeLists.txt | 3 ++- > + 2 files changed, 3 insertions(+), 1 deletion(-) > + > +diff --git a/lib/arch/CMakeLists.txt b/lib/arch/CMakeLists.txt > +index 93c0ae76bb65..628f4c981051 100644 > +--- a/lib/arch/CMakeLists.txt > ++++ b/lib/arch/CMakeLists.txt > +@@ -12,6 +12,7 @@ target_link_libraries(rmm-lib-arch > + target_include_directories(rmm-lib-arch > + PUBLIC "include" > + "include/${RMM_ARCH}" > ++ "../../../recipe-sysroot/usr/include/" > + PRIVATE "src/${RMM_ARCH}" > + "src/include") > + > +diff --git a/lib/libc/CMakeLists.txt b/lib/libc/CMakeLists.txt > +index 1631332dbc72..94d559b7aae0 100644 > +--- a/lib/libc/CMakeLists.txt > ++++ b/lib/libc/CMakeLists.txt > +@@ -12,7 +12,8 @@ if(NOT RMM_ARCH STREQUAL fake_host) > + rmm-lib-debug) > + > + target_include_directories(rmm-lib-libc SYSTEM > +- PUBLIC "include") > ++ PUBLIC "include" > ++ "../../../recipe-sysroot/usr/include/") > + > + target_sources(rmm-lib-libc > + PRIVATE "src/abort.c" > +-- > +2.43.0 > + > diff --git a/meta-arm/recipes-bsp/rmm/rmm.inc b/meta-arm/recipes-bsp/rmm/rmm.inc > new file mode 100644 > index 000000000000..0cdf62b412d5 > --- /dev/null > +++ b/meta-arm/recipes-bsp/rmm/rmm.inc > @@ -0,0 +1,54 @@ > +SUMMARY = "RMM Firmware" > +DESCRIPTION = "RMM Firmware for Arm reference platforms" > +LICENSE = "BSD-3-Clause & MIT" > + > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > + file://0001-CMakeLists-Add-repositories-for-system-includes.patch \ > + " > + > +LIC_FILES_CHKSUM += "file://${WORKDIR}/git/docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" This should probably be LIC_FILES_CHKSUM = "file://docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" You are doing cmake wrong below. Look at https://docs.yoctoproject.org/current/ref-manual/classes.html?highlight=cmake#ref-classes-cmake There are lots of examples in meta-arm and poky of recipes using cmake. Please reference those and clean-up below. > + > +inherit deploy > + > +DEPENDS += "cmake-native" > + > +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}" > + > +export CROSS_COMPILE="${TARGET_PREFIX}" > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > + > +do_configure[cleandirs] = "${B}" > +do_configure() { > + cmake -S ${S} -B ${B} ${EXTRA_OECMAKE} > +} > + > +do_compile() { > + cmake --build ${B} > +} > + > +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 > diff --git a/meta-arm/recipes-bsp/rmm/rmm_1.0.bb b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb Looking at the git tree referenced below, this isn't version 1.0 It actually looks like you picked a SHA that was probably the HEAD a month ago. I would've thought you'd want to use tf-rmm-v0.4.0, which would make the recipe name rmm_0.4.0 Also, I'm thinking that since it is from trusted-firmware-a, it would live in that directory (but this is purely a guess). > new file mode 100644 > index 000000000000..95a4f4a4667a > --- /dev/null > +++ b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb > @@ -0,0 +1,5 @@ > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > +SRCREV_rmm ?= "01a3cb75c6e6c50851d5d939d237966d110ed91d" > +SRCBRANCH_rmm ?= "main" > + > +require recipes-bsp/rmm/rmm.inc Unless there is going to be mulitple versions living in the same tree, I don't see a need for an inc file, especially one so trivial. Thanks, Jon > -- > 2.43.0 > >
Hi Jon, On Thu, 7 Mar 2024 at 09:25, Jon Mason <jdmason@kudzu.us> wrote: > > On Mon, Mar 04, 2024 at 04:16:39PM -0700, Mathieu Poirier wrote: > > Initial checking providing support for RMM on QEMU's "virt" machine. > > > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > --- > > ...Add-repositories-for-system-includes.patch | 45 ++++++++++++++++ > > meta-arm/recipes-bsp/rmm/rmm.inc | 54 +++++++++++++++++++ > > meta-arm/recipes-bsp/rmm/rmm_1.0.bb | 5 ++ > > 3 files changed, 104 insertions(+) > > create mode 100644 meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > create mode 100644 meta-arm/recipes-bsp/rmm/rmm.inc > > create mode 100644 meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > > diff --git a/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > new file mode 100644 > > index 000000000000..a2f2137012d3 > > --- /dev/null > > +++ b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > @@ -0,0 +1,45 @@ > > +From e9e6d5ded4c3e53254438a73a79f04209035603e Mon Sep 17 00:00:00 2001 > > +From: Mathieu Poirier <mathieu.poirier@linaro.org> > > +Date: Mon, 4 Mar 2024 13:38:03 -0700 > > +Subject: [PATCH] CMakeLists: Add repositories for system includes > > + > > +Yocto's default aarch64 toolchain, i.e aarch64-poky-linux-, doesn't > > +include assert.h and stderr.h by default, leading to a compilation error > > +for files that need them. As such add the path of the base sysroot > > +directory to the list of directories to be included when compiling. > > + > > +Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > This needs an upstream status. See > https://docs.yoctoproject.org/current/contributor-guide/recipe-style-guide.html#patch-upstream-status > I wrote another revision of this patch that was accepted upstream. Reading the documentation the next version should have a "Backport" tag. > > +--- > > + lib/arch/CMakeLists.txt | 1 + > > + lib/libc/CMakeLists.txt | 3 ++- > > + 2 files changed, 3 insertions(+), 1 deletion(-) > > + > > +diff --git a/lib/arch/CMakeLists.txt b/lib/arch/CMakeLists.txt > > +index 93c0ae76bb65..628f4c981051 100644 > > +--- a/lib/arch/CMakeLists.txt > > ++++ b/lib/arch/CMakeLists.txt > > +@@ -12,6 +12,7 @@ target_link_libraries(rmm-lib-arch > > + target_include_directories(rmm-lib-arch > > + PUBLIC "include" > > + "include/${RMM_ARCH}" > > ++ "../../../recipe-sysroot/usr/include/" > > + PRIVATE "src/${RMM_ARCH}" > > + "src/include") > > + > > +diff --git a/lib/libc/CMakeLists.txt b/lib/libc/CMakeLists.txt > > +index 1631332dbc72..94d559b7aae0 100644 > > +--- a/lib/libc/CMakeLists.txt > > ++++ b/lib/libc/CMakeLists.txt > > +@@ -12,7 +12,8 @@ if(NOT RMM_ARCH STREQUAL fake_host) > > + rmm-lib-debug) > > + > > + target_include_directories(rmm-lib-libc SYSTEM > > +- PUBLIC "include") > > ++ PUBLIC "include" > > ++ "../../../recipe-sysroot/usr/include/") > > + > > + target_sources(rmm-lib-libc > > + PRIVATE "src/abort.c" > > +-- > > +2.43.0 > > + > > diff --git a/meta-arm/recipes-bsp/rmm/rmm.inc b/meta-arm/recipes-bsp/rmm/rmm.inc > > new file mode 100644 > > index 000000000000..0cdf62b412d5 > > --- /dev/null > > +++ b/meta-arm/recipes-bsp/rmm/rmm.inc > > @@ -0,0 +1,54 @@ > > +SUMMARY = "RMM Firmware" > > +DESCRIPTION = "RMM Firmware for Arm reference platforms" > > +LICENSE = "BSD-3-Clause & MIT" > > + > > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > > + file://0001-CMakeLists-Add-repositories-for-system-includes.patch \ > > + " > > + > > +LIC_FILES_CHKSUM += "file://${WORKDIR}/git/docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" > > This should probably be > LIC_FILES_CHKSUM = "file://docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" > > You are doing cmake wrong below. Look at > https://docs.yoctoproject.org/current/ref-manual/classes.html?highlight=cmake#ref-classes-cmake > > There are lots of examples in meta-arm and poky of recipes using > cmake. Please reference those and clean-up below. > I used the "inherited cmake" statement and was able to get rid of do_configure() and do_compile(). > > + > > +inherit deploy > > + > > +DEPENDS += "cmake-native" > > + > > +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}" > > + > > +export CROSS_COMPILE="${TARGET_PREFIX}" > > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > > + > > +do_configure[cleandirs] = "${B}" > > +do_configure() { > > + cmake -S ${S} -B ${B} ${EXTRA_OECMAKE} > > +} > > + > > +do_compile() { > > + cmake --build ${B} > > +} > > + > > +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 > > diff --git a/meta-arm/recipes-bsp/rmm/rmm_1.0.bb b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > Looking at the git tree referenced below, this isn't version 1.0 > It actually looks like you picked a SHA that was probably the HEAD a > month ago. I would've thought you'd want to use tf-rmm-v0.4.0, which > would make the recipe name rmm_0.4.0 > You are correct. I fixed that now. > Also, I'm thinking that since it is from trusted-firmware-a, it would > live in that directory (but this is purely a guess). > On trustedfirmware.org TF-RMM is on the same level as TF-A, TF-M and Hafnium. As such I enacted the same structure under recipes-bsp. How should I proceed for the next revision? > > new file mode 100644 > > index 000000000000..95a4f4a4667a > > --- /dev/null > > +++ b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > @@ -0,0 +1,5 @@ > > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > > +SRCREV_rmm ?= "01a3cb75c6e6c50851d5d939d237966d110ed91d" > > +SRCBRANCH_rmm ?= "main" > > + > > +require recipes-bsp/rmm/rmm.inc > > Unless there is going to be mulitple versions living in the same tree, > I don't see a need for an inc file, especially one so trivial. > If I understand correctly, you are suggesting to rename rmm_0.4.0.bb to rmm.bb and get rid of the rmm.inc file. Which means that rmm.bb would have a SRCREV_rmm variable that points to the latest stable tag. Please confirm that I got this right. Thanks for the review, Mathieu > Thanks, > Jon > > > -- > > 2.43.0 > > > >
On Fri, Mar 22, 2024 at 10:15:13AM -0600, Mathieu Poirier wrote: > Hi Jon, > > On Thu, 7 Mar 2024 at 09:25, Jon Mason <jdmason@kudzu.us> wrote: > > > > On Mon, Mar 04, 2024 at 04:16:39PM -0700, Mathieu Poirier wrote: > > > Initial checking providing support for RMM on QEMU's "virt" machine. > > > > > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > > --- > > > ...Add-repositories-for-system-includes.patch | 45 ++++++++++++++++ > > > meta-arm/recipes-bsp/rmm/rmm.inc | 54 +++++++++++++++++++ > > > meta-arm/recipes-bsp/rmm/rmm_1.0.bb | 5 ++ > > > 3 files changed, 104 insertions(+) > > > create mode 100644 meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > > create mode 100644 meta-arm/recipes-bsp/rmm/rmm.inc > > > create mode 100644 meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > > > > diff --git a/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > > new file mode 100644 > > > index 000000000000..a2f2137012d3 > > > --- /dev/null > > > +++ b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > > @@ -0,0 +1,45 @@ > > > +From e9e6d5ded4c3e53254438a73a79f04209035603e Mon Sep 17 00:00:00 2001 > > > +From: Mathieu Poirier <mathieu.poirier@linaro.org> > > > +Date: Mon, 4 Mar 2024 13:38:03 -0700 > > > +Subject: [PATCH] CMakeLists: Add repositories for system includes > > > + > > > +Yocto's default aarch64 toolchain, i.e aarch64-poky-linux-, doesn't > > > +include assert.h and stderr.h by default, leading to a compilation error > > > +for files that need them. As such add the path of the base sysroot > > > +directory to the list of directories to be included when compiling. > > > + > > > +Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > > > This needs an upstream status. See > > https://docs.yoctoproject.org/current/contributor-guide/recipe-style-guide.html#patch-upstream-status > > > > I wrote another revision of this patch that was accepted upstream. > Reading the documentation the next version should have a "Backport" > tag. Yes, and a link to the patch gets extra kudos. As an example, see meta-arm-bsp/recipes-security/optee/optee-os-3.20.0/0003-core-link-add-no-warn-rwx-segments.patch or meta-arm-bsp/recipes-security/optee/optee-os-3.20.0/0005-core-arm-S-EL1-SPMC-boot-ABI-update.patch > > > > +--- > > > + lib/arch/CMakeLists.txt | 1 + > > > + lib/libc/CMakeLists.txt | 3 ++- > > > + 2 files changed, 3 insertions(+), 1 deletion(-) > > > + > > > +diff --git a/lib/arch/CMakeLists.txt b/lib/arch/CMakeLists.txt > > > +index 93c0ae76bb65..628f4c981051 100644 > > > +--- a/lib/arch/CMakeLists.txt > > > ++++ b/lib/arch/CMakeLists.txt > > > +@@ -12,6 +12,7 @@ target_link_libraries(rmm-lib-arch > > > + target_include_directories(rmm-lib-arch > > > + PUBLIC "include" > > > + "include/${RMM_ARCH}" > > > ++ "../../../recipe-sysroot/usr/include/" > > > + PRIVATE "src/${RMM_ARCH}" > > > + "src/include") > > > + > > > +diff --git a/lib/libc/CMakeLists.txt b/lib/libc/CMakeLists.txt > > > +index 1631332dbc72..94d559b7aae0 100644 > > > +--- a/lib/libc/CMakeLists.txt > > > ++++ b/lib/libc/CMakeLists.txt > > > +@@ -12,7 +12,8 @@ if(NOT RMM_ARCH STREQUAL fake_host) > > > + rmm-lib-debug) > > > + > > > + target_include_directories(rmm-lib-libc SYSTEM > > > +- PUBLIC "include") > > > ++ PUBLIC "include" > > > ++ "../../../recipe-sysroot/usr/include/") > > > + > > > + target_sources(rmm-lib-libc > > > + PRIVATE "src/abort.c" > > > +-- > > > +2.43.0 > > > + > > > diff --git a/meta-arm/recipes-bsp/rmm/rmm.inc b/meta-arm/recipes-bsp/rmm/rmm.inc > > > new file mode 100644 > > > index 000000000000..0cdf62b412d5 > > > --- /dev/null > > > +++ b/meta-arm/recipes-bsp/rmm/rmm.inc > > > @@ -0,0 +1,54 @@ > > > +SUMMARY = "RMM Firmware" > > > +DESCRIPTION = "RMM Firmware for Arm reference platforms" > > > +LICENSE = "BSD-3-Clause & MIT" > > > + > > > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > > > + file://0001-CMakeLists-Add-repositories-for-system-includes.patch \ > > > + " > > > + > > > +LIC_FILES_CHKSUM += "file://${WORKDIR}/git/docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" > > > > This should probably be > > LIC_FILES_CHKSUM = "file://docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" > > > > You are doing cmake wrong below. Look at > > https://docs.yoctoproject.org/current/ref-manual/classes.html?highlight=cmake#ref-classes-cmake > > > > There are lots of examples in meta-arm and poky of recipes using > > cmake. Please reference those and clean-up below. > > > > I used the "inherited cmake" statement and was able to get rid of > do_configure() and do_compile(). Awesome. The goal is to make these as simple as possible, because the maintenance is cause of most of the problems. The less these have in there, the less exposure to potential problems (in theory). > > > + > > > +inherit deploy > > > + > > > +DEPENDS += "cmake-native" > > > + > > > +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}" > > > + > > > +export CROSS_COMPILE="${TARGET_PREFIX}" > > > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > > > + > > > +do_configure[cleandirs] = "${B}" > > > +do_configure() { > > > + cmake -S ${S} -B ${B} ${EXTRA_OECMAKE} > > > +} > > > + > > > +do_compile() { > > > + cmake --build ${B} > > > +} > > > + > > > +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 > > > diff --git a/meta-arm/recipes-bsp/rmm/rmm_1.0.bb b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > > Looking at the git tree referenced below, this isn't version 1.0 > > It actually looks like you picked a SHA that was probably the HEAD a > > month ago. I would've thought you'd want to use tf-rmm-v0.4.0, which > > would make the recipe name rmm_0.4.0 > > > > You are correct. I fixed that now. > > > Also, I'm thinking that since it is from trusted-firmware-a, it would > > live in that directory (but this is purely a guess). > > > > On trustedfirmware.org TF-RMM is on the same level as TF-A, TF-M and > Hafnium. As such I enacted the same structure under recipes-bsp. How > should I proceed for the next revision? If this is the case, then you were correct originally. Should this be named tf-rmm/tf-rmm_0.4.bb (or should we spell out trusted-firmware-rmm) then? Asking if this should be name similar to the others, or if this matters. Might be a question of opinion/style. I'll let Ross weigh in on it. > > > > new file mode 100644 > > > index 000000000000..95a4f4a4667a > > > --- /dev/null > > > +++ b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > @@ -0,0 +1,5 @@ > > > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > > > +SRCREV_rmm ?= "01a3cb75c6e6c50851d5d939d237966d110ed91d" > > > +SRCBRANCH_rmm ?= "main" > > > + > > > +require recipes-bsp/rmm/rmm.inc > > > > Unless there is going to be mulitple versions living in the same tree, > > I don't see a need for an inc file, especially one so trivial. > > > > If I understand correctly, you are suggesting to rename rmm_0.4.0.bb > to rmm.bb and get rid of the rmm.inc file. Which means that rmm.bb > would have a SRCREV_rmm variable that points to the latest stable tag. > Please confirm that I got this right. I'm suggesting that rmm.inc should be merged with rmm_1.0.bb and renamed to rmm_0.4.bb (or trusted-firmware-rmm_0.4.bb, per the comment above). The only reason you would not want to do it this way would be if you forsee multiple releases needing to coexist in the same tree/build env. In that case, you would want the .inc file and have a .bb file that only contains the SHAs for each release. See meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.10.2.bb meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a.inc for what I mean by this, but I don't think you will actually need to do this (and it can be done at some point in the future should it be needed). Thanks, Jon > > Thanks for the review, > Mathieu > > > Thanks, > > Jon > > > > > -- > > > 2.43.0 > > > > > > >
On Mon, 25 Mar 2024 at 09:41, Jon Mason <jdmason@kudzu.us> wrote: > > On Fri, Mar 22, 2024 at 10:15:13AM -0600, Mathieu Poirier wrote: > > Hi Jon, > > > > On Thu, 7 Mar 2024 at 09:25, Jon Mason <jdmason@kudzu.us> wrote: > > > > > > On Mon, Mar 04, 2024 at 04:16:39PM -0700, Mathieu Poirier wrote: > > > > Initial checking providing support for RMM on QEMU's "virt" machine. > > > > > > > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > > > --- > > > > ...Add-repositories-for-system-includes.patch | 45 ++++++++++++++++ > > > > meta-arm/recipes-bsp/rmm/rmm.inc | 54 +++++++++++++++++++ > > > > meta-arm/recipes-bsp/rmm/rmm_1.0.bb | 5 ++ > > > > 3 files changed, 104 insertions(+) > > > > create mode 100644 meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > > > create mode 100644 meta-arm/recipes-bsp/rmm/rmm.inc > > > > create mode 100644 meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > > > > > > diff --git a/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > > > new file mode 100644 > > > > index 000000000000..a2f2137012d3 > > > > --- /dev/null > > > > +++ b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > > > @@ -0,0 +1,45 @@ > > > > +From e9e6d5ded4c3e53254438a73a79f04209035603e Mon Sep 17 00:00:00 2001 > > > > +From: Mathieu Poirier <mathieu.poirier@linaro.org> > > > > +Date: Mon, 4 Mar 2024 13:38:03 -0700 > > > > +Subject: [PATCH] CMakeLists: Add repositories for system includes > > > > + > > > > +Yocto's default aarch64 toolchain, i.e aarch64-poky-linux-, doesn't > > > > +include assert.h and stderr.h by default, leading to a compilation error > > > > +for files that need them. As such add the path of the base sysroot > > > > +directory to the list of directories to be included when compiling. > > > > + > > > > +Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > > > > > This needs an upstream status. See > > > https://docs.yoctoproject.org/current/contributor-guide/recipe-style-guide.html#patch-upstream-status > > > > > > > I wrote another revision of this patch that was accepted upstream. > > Reading the documentation the next version should have a "Backport" > > tag. > > Yes, and a link to the patch gets extra kudos. As an example, see > meta-arm-bsp/recipes-security/optee/optee-os-3.20.0/0003-core-link-add-no-warn-rwx-segments.patch > or > meta-arm-bsp/recipes-security/optee/optee-os-3.20.0/0005-core-arm-S-EL1-SPMC-boot-ABI-update.patch > Ok > > > > > > +--- > > > > + lib/arch/CMakeLists.txt | 1 + > > > > + lib/libc/CMakeLists.txt | 3 ++- > > > > + 2 files changed, 3 insertions(+), 1 deletion(-) > > > > + > > > > +diff --git a/lib/arch/CMakeLists.txt b/lib/arch/CMakeLists.txt > > > > +index 93c0ae76bb65..628f4c981051 100644 > > > > +--- a/lib/arch/CMakeLists.txt > > > > ++++ b/lib/arch/CMakeLists.txt > > > > +@@ -12,6 +12,7 @@ target_link_libraries(rmm-lib-arch > > > > + target_include_directories(rmm-lib-arch > > > > + PUBLIC "include" > > > > + "include/${RMM_ARCH}" > > > > ++ "../../../recipe-sysroot/usr/include/" > > > > + PRIVATE "src/${RMM_ARCH}" > > > > + "src/include") > > > > + > > > > +diff --git a/lib/libc/CMakeLists.txt b/lib/libc/CMakeLists.txt > > > > +index 1631332dbc72..94d559b7aae0 100644 > > > > +--- a/lib/libc/CMakeLists.txt > > > > ++++ b/lib/libc/CMakeLists.txt > > > > +@@ -12,7 +12,8 @@ if(NOT RMM_ARCH STREQUAL fake_host) > > > > + rmm-lib-debug) > > > > + > > > > + target_include_directories(rmm-lib-libc SYSTEM > > > > +- PUBLIC "include") > > > > ++ PUBLIC "include" > > > > ++ "../../../recipe-sysroot/usr/include/") > > > > + > > > > + target_sources(rmm-lib-libc > > > > + PRIVATE "src/abort.c" > > > > +-- > > > > +2.43.0 > > > > + > > > > diff --git a/meta-arm/recipes-bsp/rmm/rmm.inc b/meta-arm/recipes-bsp/rmm/rmm.inc > > > > new file mode 100644 > > > > index 000000000000..0cdf62b412d5 > > > > --- /dev/null > > > > +++ b/meta-arm/recipes-bsp/rmm/rmm.inc > > > > @@ -0,0 +1,54 @@ > > > > +SUMMARY = "RMM Firmware" > > > > +DESCRIPTION = "RMM Firmware for Arm reference platforms" > > > > +LICENSE = "BSD-3-Clause & MIT" > > > > + > > > > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > > > > + file://0001-CMakeLists-Add-repositories-for-system-includes.patch \ > > > > + " > > > > + > > > > +LIC_FILES_CHKSUM += "file://${WORKDIR}/git/docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" > > > > > > This should probably be > > > LIC_FILES_CHKSUM = "file://docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" > > > > > > You are doing cmake wrong below. Look at > > > https://docs.yoctoproject.org/current/ref-manual/classes.html?highlight=cmake#ref-classes-cmake > > > > > > There are lots of examples in meta-arm and poky of recipes using > > > cmake. Please reference those and clean-up below. > > > > > > > I used the "inherited cmake" statement and was able to get rid of > > do_configure() and do_compile(). > > Awesome. The goal is to make these as simple as possible, because the > maintenance is cause of most of the problems. The less these have in > there, the less exposure to potential problems (in theory). > > > > > + > > > > +inherit deploy > > > > + > > > > +DEPENDS += "cmake-native" > > > > + > > > > +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}" > > > > + > > > > +export CROSS_COMPILE="${TARGET_PREFIX}" > > > > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > > > > + > > > > +do_configure[cleandirs] = "${B}" > > > > +do_configure() { > > > > + cmake -S ${S} -B ${B} ${EXTRA_OECMAKE} > > > > +} > > > > + > > > > +do_compile() { > > > > + cmake --build ${B} > > > > +} > > > > + > > > > +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 > > > > diff --git a/meta-arm/recipes-bsp/rmm/rmm_1.0.bb b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > > > > Looking at the git tree referenced below, this isn't version 1.0 > > > It actually looks like you picked a SHA that was probably the HEAD a > > > month ago. I would've thought you'd want to use tf-rmm-v0.4.0, which > > > would make the recipe name rmm_0.4.0 > > > > > > > You are correct. I fixed that now. > > > > > Also, I'm thinking that since it is from trusted-firmware-a, it would > > > live in that directory (but this is purely a guess). > > > > > > > On trustedfirmware.org TF-RMM is on the same level as TF-A, TF-M and > > Hafnium. As such I enacted the same structure under recipes-bsp. How > > should I proceed for the next revision? > > If this is the case, then you were correct originally. Should this be > named tf-rmm/tf-rmm_0.4.bb (or should we spell out > trusted-firmware-rmm) then? Asking if this should be name similar to > the others, or if this matters. Might be a question of opinion/style. > I'll let Ross weigh in on it. > I was also puzzled about the naming convention to use. tf-rmm would make sense, if only to follow the git repository's convention. I'll send out another revision with tf-rmm and see where it gets us. > > > > > > new file mode 100644 > > > > index 000000000000..95a4f4a4667a > > > > --- /dev/null > > > > +++ b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > > @@ -0,0 +1,5 @@ > > > > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > > > > +SRCREV_rmm ?= "01a3cb75c6e6c50851d5d939d237966d110ed91d" > > > > +SRCBRANCH_rmm ?= "main" > > > > + > > > > +require recipes-bsp/rmm/rmm.inc > > > > > > Unless there is going to be mulitple versions living in the same tree, > > > I don't see a need for an inc file, especially one so trivial. > > > > > > > If I understand correctly, you are suggesting to rename rmm_0.4.0.bb > > to rmm.bb and get rid of the rmm.inc file. Which means that rmm.bb > > would have a SRCREV_rmm variable that points to the latest stable tag. > > Please confirm that I got this right. > > I'm suggesting that rmm.inc should be merged with rmm_1.0.bb and > renamed to rmm_0.4.bb (or trusted-firmware-rmm_0.4.bb, per the comment > above). The only reason you would not want to do it this way would be > if you forsee multiple releases needing to coexist in the same > tree/build env. In that case, you would want the .inc file and have a > .bb file that only contains the SHAs for each release. See > meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.10.2.bb > meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a.inc > for what I mean by this, but I don't think you will actually need to > do this (and it can be done at some point in the future should it be > needed). I agree that for the time being there is no need to support multiple releases. That said, I'm not sure about "rmm_0.4.bb" since we will need to rename the file everytime a new revision of the RMM code base gets released. That is why I proposed to simply go with rmm.bb. What is your take on that? > > Thanks, > Jon > > > > > Thanks for the review, > > Mathieu > > > > > Thanks, > > > Jon > > > > > > > -- > > > > 2.43.0 > > > > > > > > > >
On Mon, Mar 25, 2024 at 11:35:57AM -0600, Mathieu Poirier wrote: > On Mon, 25 Mar 2024 at 09:41, Jon Mason <jdmason@kudzu.us> wrote: > > > > On Fri, Mar 22, 2024 at 10:15:13AM -0600, Mathieu Poirier wrote: > > > Hi Jon, > > > > > > On Thu, 7 Mar 2024 at 09:25, Jon Mason <jdmason@kudzu.us> wrote: > > > > > > > > On Mon, Mar 04, 2024 at 04:16:39PM -0700, Mathieu Poirier wrote: > > > > > Initial checking providing support for RMM on QEMU's "virt" machine. > > > > > > > > > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > > > > --- > > > > > ...Add-repositories-for-system-includes.patch | 45 ++++++++++++++++ > > > > > meta-arm/recipes-bsp/rmm/rmm.inc | 54 +++++++++++++++++++ > > > > > meta-arm/recipes-bsp/rmm/rmm_1.0.bb | 5 ++ > > > > > 3 files changed, 104 insertions(+) > > > > > create mode 100644 meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > > > > create mode 100644 meta-arm/recipes-bsp/rmm/rmm.inc > > > > > create mode 100644 meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > > > > > > > > diff --git a/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > > > > new file mode 100644 > > > > > index 000000000000..a2f2137012d3 > > > > > --- /dev/null > > > > > +++ b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch > > > > > @@ -0,0 +1,45 @@ > > > > > +From e9e6d5ded4c3e53254438a73a79f04209035603e Mon Sep 17 00:00:00 2001 > > > > > +From: Mathieu Poirier <mathieu.poirier@linaro.org> > > > > > +Date: Mon, 4 Mar 2024 13:38:03 -0700 > > > > > +Subject: [PATCH] CMakeLists: Add repositories for system includes > > > > > + > > > > > +Yocto's default aarch64 toolchain, i.e aarch64-poky-linux-, doesn't > > > > > +include assert.h and stderr.h by default, leading to a compilation error > > > > > +for files that need them. As such add the path of the base sysroot > > > > > +directory to the list of directories to be included when compiling. > > > > > + > > > > > +Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > > > > > > > This needs an upstream status. See > > > > https://docs.yoctoproject.org/current/contributor-guide/recipe-style-guide.html#patch-upstream-status > > > > > > > > > > I wrote another revision of this patch that was accepted upstream. > > > Reading the documentation the next version should have a "Backport" > > > tag. > > > > Yes, and a link to the patch gets extra kudos. As an example, see > > meta-arm-bsp/recipes-security/optee/optee-os-3.20.0/0003-core-link-add-no-warn-rwx-segments.patch > > or > > meta-arm-bsp/recipes-security/optee/optee-os-3.20.0/0005-core-arm-S-EL1-SPMC-boot-ABI-update.patch > > > > Ok > > > > > > > > > +--- > > > > > + lib/arch/CMakeLists.txt | 1 + > > > > > + lib/libc/CMakeLists.txt | 3 ++- > > > > > + 2 files changed, 3 insertions(+), 1 deletion(-) > > > > > + > > > > > +diff --git a/lib/arch/CMakeLists.txt b/lib/arch/CMakeLists.txt > > > > > +index 93c0ae76bb65..628f4c981051 100644 > > > > > +--- a/lib/arch/CMakeLists.txt > > > > > ++++ b/lib/arch/CMakeLists.txt > > > > > +@@ -12,6 +12,7 @@ target_link_libraries(rmm-lib-arch > > > > > + target_include_directories(rmm-lib-arch > > > > > + PUBLIC "include" > > > > > + "include/${RMM_ARCH}" > > > > > ++ "../../../recipe-sysroot/usr/include/" > > > > > + PRIVATE "src/${RMM_ARCH}" > > > > > + "src/include") > > > > > + > > > > > +diff --git a/lib/libc/CMakeLists.txt b/lib/libc/CMakeLists.txt > > > > > +index 1631332dbc72..94d559b7aae0 100644 > > > > > +--- a/lib/libc/CMakeLists.txt > > > > > ++++ b/lib/libc/CMakeLists.txt > > > > > +@@ -12,7 +12,8 @@ if(NOT RMM_ARCH STREQUAL fake_host) > > > > > + rmm-lib-debug) > > > > > + > > > > > + target_include_directories(rmm-lib-libc SYSTEM > > > > > +- PUBLIC "include") > > > > > ++ PUBLIC "include" > > > > > ++ "../../../recipe-sysroot/usr/include/") > > > > > + > > > > > + target_sources(rmm-lib-libc > > > > > + PRIVATE "src/abort.c" > > > > > +-- > > > > > +2.43.0 > > > > > + > > > > > diff --git a/meta-arm/recipes-bsp/rmm/rmm.inc b/meta-arm/recipes-bsp/rmm/rmm.inc > > > > > new file mode 100644 > > > > > index 000000000000..0cdf62b412d5 > > > > > --- /dev/null > > > > > +++ b/meta-arm/recipes-bsp/rmm/rmm.inc > > > > > @@ -0,0 +1,54 @@ > > > > > +SUMMARY = "RMM Firmware" > > > > > +DESCRIPTION = "RMM Firmware for Arm reference platforms" > > > > > +LICENSE = "BSD-3-Clause & MIT" > > > > > + > > > > > +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ > > > > > + file://0001-CMakeLists-Add-repositories-for-system-includes.patch \ > > > > > + " > > > > > + > > > > > +LIC_FILES_CHKSUM += "file://${WORKDIR}/git/docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" > > > > > > > > This should probably be > > > > LIC_FILES_CHKSUM = "file://docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" > > > > > > > > You are doing cmake wrong below. Look at > > > > https://docs.yoctoproject.org/current/ref-manual/classes.html?highlight=cmake#ref-classes-cmake > > > > > > > > There are lots of examples in meta-arm and poky of recipes using > > > > cmake. Please reference those and clean-up below. > > > > > > > > > > I used the "inherited cmake" statement and was able to get rid of > > > do_configure() and do_compile(). > > > > Awesome. The goal is to make these as simple as possible, because the > > maintenance is cause of most of the problems. The less these have in > > there, the less exposure to potential problems (in theory). > > > > > > > + > > > > > +inherit deploy > > > > > + > > > > > +DEPENDS += "cmake-native" > > > > > + > > > > > +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}" > > > > > + > > > > > +export CROSS_COMPILE="${TARGET_PREFIX}" > > > > > +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" > > > > > + > > > > > +do_configure[cleandirs] = "${B}" > > > > > +do_configure() { > > > > > + cmake -S ${S} -B ${B} ${EXTRA_OECMAKE} > > > > > +} > > > > > + > > > > > +do_compile() { > > > > > + cmake --build ${B} > > > > > +} > > > > > + > > > > > +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 > > > > > diff --git a/meta-arm/recipes-bsp/rmm/rmm_1.0.bb b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > > > > > > Looking at the git tree referenced below, this isn't version 1.0 > > > > It actually looks like you picked a SHA that was probably the HEAD a > > > > month ago. I would've thought you'd want to use tf-rmm-v0.4.0, which > > > > would make the recipe name rmm_0.4.0 > > > > > > > > > > You are correct. I fixed that now. > > > > > > > Also, I'm thinking that since it is from trusted-firmware-a, it would > > > > live in that directory (but this is purely a guess). > > > > > > > > > > On trustedfirmware.org TF-RMM is on the same level as TF-A, TF-M and > > > Hafnium. As such I enacted the same structure under recipes-bsp. How > > > should I proceed for the next revision? > > > > If this is the case, then you were correct originally. Should this be > > named tf-rmm/tf-rmm_0.4.bb (or should we spell out > > trusted-firmware-rmm) then? Asking if this should be name similar to > > the others, or if this matters. Might be a question of opinion/style. > > I'll let Ross weigh in on it. > > > > I was also puzzled about the naming convention to use. tf-rmm would > make sense, if only to follow the git repository's convention. I'll > send out another revision with tf-rmm and see where it gets us. Discussion with Ross, trusted-firmware-rmm is the preferred name. > > > > > > > > new file mode 100644 > > > > > index 000000000000..95a4f4a4667a > > > > > --- /dev/null > > > > > +++ b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb > > > > > @@ -0,0 +1,5 @@ > > > > > +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" > > > > > +SRCREV_rmm ?= "01a3cb75c6e6c50851d5d939d237966d110ed91d" > > > > > +SRCBRANCH_rmm ?= "main" > > > > > + > > > > > +require recipes-bsp/rmm/rmm.inc > > > > > > > > Unless there is going to be mulitple versions living in the same tree, > > > > I don't see a need for an inc file, especially one so trivial. > > > > > > > > > > If I understand correctly, you are suggesting to rename rmm_0.4.0.bb > > > to rmm.bb and get rid of the rmm.inc file. Which means that rmm.bb > > > would have a SRCREV_rmm variable that points to the latest stable tag. > > > Please confirm that I got this right. > > > > I'm suggesting that rmm.inc should be merged with rmm_1.0.bb and > > renamed to rmm_0.4.bb (or trusted-firmware-rmm_0.4.bb, per the comment > > above). The only reason you would not want to do it this way would be > > if you forsee multiple releases needing to coexist in the same > > tree/build env. In that case, you would want the .inc file and have a > > .bb file that only contains the SHAs for each release. See > > meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a_2.10.2.bb > > meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a.inc > > for what I mean by this, but I don't think you will actually need to > > do this (and it can be done at some point in the future should it be > > needed). > > I agree that for the time being there is no need to support multiple > releases. That said, I'm not sure about "rmm_0.4.bb" since we will > need to rename the file everytime a new revision of the RMM code base > gets released. That is why I proposed to simply go with rmm.bb. What > is your take on that? This is how things are done in Yocto. We add the version to the recipe name, and the recipe logic expects it to be there. See https://docs.yoctoproject.org/ref-manual/variables.html#term-PV > > > > > Thanks, > > Jon > > > > > > > > Thanks for the review, > > > Mathieu > > > > > > > Thanks, > > > > Jon > > > > > > > > > -- > > > > > 2.43.0 > > > > > > > > > > > > > >
diff --git a/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch new file mode 100644 index 000000000000..a2f2137012d3 --- /dev/null +++ b/meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch @@ -0,0 +1,45 @@ +From e9e6d5ded4c3e53254438a73a79f04209035603e Mon Sep 17 00:00:00 2001 +From: Mathieu Poirier <mathieu.poirier@linaro.org> +Date: Mon, 4 Mar 2024 13:38:03 -0700 +Subject: [PATCH] CMakeLists: Add repositories for system includes + +Yocto's default aarch64 toolchain, i.e aarch64-poky-linux-, doesn't +include assert.h and stderr.h by default, leading to a compilation error +for files that need them. As such add the path of the base sysroot +directory to the list of directories to be included when compiling. + +Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> +--- + lib/arch/CMakeLists.txt | 1 + + lib/libc/CMakeLists.txt | 3 ++- + 2 files changed, 3 insertions(+), 1 deletion(-) + +diff --git a/lib/arch/CMakeLists.txt b/lib/arch/CMakeLists.txt +index 93c0ae76bb65..628f4c981051 100644 +--- a/lib/arch/CMakeLists.txt ++++ b/lib/arch/CMakeLists.txt +@@ -12,6 +12,7 @@ target_link_libraries(rmm-lib-arch + target_include_directories(rmm-lib-arch + PUBLIC "include" + "include/${RMM_ARCH}" ++ "../../../recipe-sysroot/usr/include/" + PRIVATE "src/${RMM_ARCH}" + "src/include") + +diff --git a/lib/libc/CMakeLists.txt b/lib/libc/CMakeLists.txt +index 1631332dbc72..94d559b7aae0 100644 +--- a/lib/libc/CMakeLists.txt ++++ b/lib/libc/CMakeLists.txt +@@ -12,7 +12,8 @@ if(NOT RMM_ARCH STREQUAL fake_host) + rmm-lib-debug) + + target_include_directories(rmm-lib-libc SYSTEM +- PUBLIC "include") ++ PUBLIC "include" ++ "../../../recipe-sysroot/usr/include/") + + target_sources(rmm-lib-libc + PRIVATE "src/abort.c" +-- +2.43.0 + diff --git a/meta-arm/recipes-bsp/rmm/rmm.inc b/meta-arm/recipes-bsp/rmm/rmm.inc new file mode 100644 index 000000000000..0cdf62b412d5 --- /dev/null +++ b/meta-arm/recipes-bsp/rmm/rmm.inc @@ -0,0 +1,54 @@ +SUMMARY = "RMM Firmware" +DESCRIPTION = "RMM Firmware for Arm reference platforms" +LICENSE = "BSD-3-Clause & MIT" + +SRC_URI = "${SRC_URI_RMM};name=rmm;branch=${SRCBRANCH_rmm} \ + file://0001-CMakeLists-Add-repositories-for-system-includes.patch \ + " + +LIC_FILES_CHKSUM += "file://${WORKDIR}/git/docs/about/license.rst;md5=1375c7c641558198ffe401c2a799d79b" + +inherit deploy + +DEPENDS += "cmake-native" + +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}" + +export CROSS_COMPILE="${TARGET_PREFIX}" +export CMAKE_BUILD_PARALLEL_LEVEL = "${@oe.utils.parallel_make(d, False)}" + +do_configure[cleandirs] = "${B}" +do_configure() { + cmake -S ${S} -B ${B} ${EXTRA_OECMAKE} +} + +do_compile() { + cmake --build ${B} +} + +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 diff --git a/meta-arm/recipes-bsp/rmm/rmm_1.0.bb b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb new file mode 100644 index 000000000000..95a4f4a4667a --- /dev/null +++ b/meta-arm/recipes-bsp/rmm/rmm_1.0.bb @@ -0,0 +1,5 @@ +SRC_URI_RMM ?= "gitsm://git.trustedfirmware.org/TF-RMM/tf-rmm.git;protocol=https" +SRCREV_rmm ?= "01a3cb75c6e6c50851d5d939d237966d110ed91d" +SRCBRANCH_rmm ?= "main" + +require recipes-bsp/rmm/rmm.inc
Initial checking providing support for RMM on QEMU's "virt" machine. Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> --- ...Add-repositories-for-system-includes.patch | 45 ++++++++++++++++ meta-arm/recipes-bsp/rmm/rmm.inc | 54 +++++++++++++++++++ meta-arm/recipes-bsp/rmm/rmm_1.0.bb | 5 ++ 3 files changed, 104 insertions(+) create mode 100644 meta-arm/recipes-bsp/rmm/files/0001-CMakeLists-Add-repositories-for-system-includes.patch create mode 100644 meta-arm/recipes-bsp/rmm/rmm.inc create mode 100644 meta-arm/recipes-bsp/rmm/rmm_1.0.bb