[PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Mikko Rapeli]

Use backported upstream patch for udev rule and systemd service file.
sysvinit script is still used from meta-arm. Don't install systemd
service without systemd distro feature, other way round for
sysvinit script.

tee-supplicant started by systemd service runs as non-root teesuppl
user with teepriv group. sysvinit still runs as root since busybox
start-stop-daemon doesn't support -g group parameter and -u teesuppl
doesn't seem to change the effective user.

udev rules allow non-root /dev/tee* access from tee and
/dev/teepriv* access from teepriv groups.

Tested sysvinit changes with:

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

and systemd changes with:

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

Cc: tom.hochstein@nxp.com
Cc: sahil.malhotra@nxp.com
Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 .../recipes-security/optee/optee-client.inc   |  30 +--
 ...dd-udev-rule-and-systemd-service-fil.patch | 186 ++++++++++++++++++
 .../optee/optee-client/optee-udev.rules       |   6 -
 .../optee-client/tee-supplicant@.service      |  13 --
 .../optee/optee-client_4.3.0.bb               |   2 +
 5 files changed, 205 insertions(+), 32 deletions(-)
 create mode 100644 meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch
 delete mode 100644 meta-arm/recipes-security/optee/optee-client/optee-udev.rules
 delete mode 100644 meta-arm/recipes-security/optee/optee-client/tee-supplicant@.service

diff --git a/meta-arm/recipes-security/optee/optee-client.inc b/meta-arm/recipes-security/optee/optee-client.inc
index f387c805..fc48c302 100644
--- a/meta-arm/recipes-security/optee/optee-client.inc
+++ b/meta-arm/recipes-security/optee/optee-client.inc
@@ -9,9 +9,7 @@ inherit systemd update-rc.d cmake useradd
 
 SRC_URI = " \
     git://github.com/OP-TEE/optee_client.git;branch=master;protocol=https \
-    file://tee-supplicant@.service \
     file://tee-supplicant.sh \
-    file://optee-udev.rules \
 "
 
 UPSTREAM_CHECK_GITTAGREGEX = "^(?P<pver>\d+(\.\d+)+)$"
@@ -20,20 +18,21 @@ S = "${WORKDIR}/git"
 
 EXTRA_OECMAKE = " \
     -DBUILD_SHARED_LIBS=ON \
-    -DCFG_TEE_FS_PARENT_PATH='${localstatedir}/lib/tee' \
 "
 EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"
 
 do_install:append() {
-    install -D -p -m0644 ${UNPACKDIR}/tee-supplicant@.service ${D}${systemd_system_unitdir}/tee-supplicant@.service
-    install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
-    install -d ${D}${sysconfdir}/udev/rules.d
-    install -m 0644 ${UNPACKDIR}/optee-udev.rules ${D}${sysconfdir}/udev/rules.d/optee.rules
-
-    sed -i -e s:@sysconfdir@:${sysconfdir}:g \
-           -e s:@sbindir@:${sbindir}:g \
-              ${D}${systemd_system_unitdir}/tee-supplicant@.service \
-              ${D}${sysconfdir}/init.d/tee-supplicant
+    # installed by default
+    if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
+        rm -rf ${D}${libdir}/systemd
+    fi
+    if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
+        install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
+        sed -i -e s:@sysconfdir@:${sysconfdir}:g \
+               -e s:@sbindir@:${sbindir}:g \
+                  ${D}${sysconfdir}/init.d/tee-supplicant
+    fi
+    install -o teesuppl -g teesuppl -m 0700 -d ${D}${localstatedir}/lib/tee
 }
 
 SYSTEMD_SERVICE:${PN} = "tee-supplicant@.service"
@@ -42,5 +41,10 @@ INITSCRIPT_PACKAGES = "${PN}"
 INITSCRIPT_NAME:${PN} = "tee-supplicant"
 INITSCRIPT_PARAMS:${PN} = "start 10 1 2 3 4 5 . stop 90 0 6 ."
 
+# Users and groups:
+# tee group to access /dev/tee*
+# teepriv group to acess /dev/teepriv*, only tee-supplicant
+# teesuppl user and group teesuppl to run tee-supplicant
 USERADD_PACKAGES = "${PN}"
-GROUPADD_PARAM:${PN} = "--system teeclnt"
+GROUPADD_PARAM:${PN} = "--system tee; --system teepriv; --system teesuppl"
+USERADD_PARAM:${PN} = "--system -g teesuppl --groups teepriv --home-dir ${localstatedir}/lib/tee -M --shell /sbin/nologin teesuppl;"
diff --git a/meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch b/meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch
new file mode 100644
index 00000000..18c0d950
--- /dev/null
+++ b/meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch
@@ -0,0 +1,186 @@
+From bf0d02758696ee7a9f7af9e95f85f5c238d0e109 Mon Sep 17 00:00:00 2001
+From: Mikko Rapeli <mikko.rapeli@linaro.org>
+Date: Wed, 2 Oct 2024 15:24:21 +0100
+Subject: [PATCH] tee-supplicant: add udev rule and systemd service file
+
+tee-supplicant startup with systemd init based
+is non-trivial. Add sample udev rule and systemd
+service files here so that distros can co-operate maintaining
+them.
+
+Files are from meta-arm https://git.yoctoproject.org/meta-arm
+at commit 7cce43e632daa8650f683ac726f9124681b302a4 with license
+MIT and authors:
+
+Peter Griffin <peter.griffin@linaro.org>
+Joshua Watt <JPEWhacker@gmail.com>
+Javier Tia <javier.tia@linaro.org>
+Mikko Rapeli <mikko.rapeli@linaro.org>
+
+With permission from the authors, files can be relicensed to
+BSD-2-Clause like rest of optee client repo.
+
+The config files expect to find tee and teepriv system groups
+and teesuppl user and group (part of teepriv group) for running
+tee-supplicant. Additionally state directory /var/lib/tee
+must be owned by teesuppl user and group with no rights
+to other users. The groups and user can be changed via
+CMake variables:
+
+CFG_TEE_GROUP
+CFG_TEEPRIV_GROUP
+CFG_TEE_SUPPL_USER
+CFG_TEE_SUPPL_GROUP
+
+Change storage path from /data to /var/lib and
+use standard CMake variables also for constructing install
+paths which can be override to change the defaults:
+
+CMAKE_INSTALL_PREFIX, e.g. /
+CMAKE_INSTALL_LIBDIR, e.g. /usr/lib
+CMAKE_INSTALL_LOCALSTATEDIR /var
+
+Once these are setup, udev will start tee-supplicant in initramfs
+or rootfs with teesuppl user and group when /dev/teepriv
+device appears. The systemd service starts before tpm2.target
+(new in systemd 256) which starts early in initramfs and in main rootfs.
+This covers firmware TPM TA usecases for main rootfs encryption. When
+stopping tee-supplicant, the ftpm kernel modules are removed and only
+then the main process stopped to avoid fTPM breakage. These workarounds
+may be removed once RPMB kernel and optee patches without tee-supplicant
+are merged (Linux kernel >= 6.12-rc1, optee_os latest master or >= 4.4).
+
+Tested on yocto meta-arm setup which runs fTPM and optee-test/xtest
+under qemuarm64:
+
+$ git clone https://git.yoctoproject.org/meta-arm
+$ cd meta-arm
+$ SSTATE_DIR=$HOME/sstate DL_DIR=$HOME/download kas build \
+ci/qemuarm64-secureboot.yml:ci/poky-altcfg.yml:ci/testimage.yml
+
+Compiled image can be manually started to qemu serial console with:
+
+$ SSTATE_DIR=$HOME/sstate DL_DIR=$HOME/download kas shell \
+ci/qemuarm64-secureboot.yml:ci/poky-altcfg.yml:ci/testimage.yml
+$ runqemu slirp nographic
+
+meta-arm maintainers run these tests as part of their CI.
+
+Note that if the tee-supplicant state directory /var/lib/tee
+can not be accessed due permissions or other problems, then
+tee-supplicant startup with systemd still works. Only optee-test/xtest
+will be failing and fTPM kernel drivers fail to load with error
+messages.
+
+Cc: Peter Griffin <peter.griffin@linaro.org>
+Cc: Joshua Watt <JPEWhacker@gmail.com>
+Cc: Javier Tia <javier.tia@linaro.org>
+Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
+Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
+---
+ config.mk                                 |  2 +-
+ libteec/CMakeLists.txt                    |  2 +-
+ tee-supplicant/CMakeLists.txt             | 13 +++++++++++--
+ tee-supplicant/optee-udev.rules.in        |  7 +++++++
+ tee-supplicant/tee-supplicant@.service.in | 17 +++++++++++++++++
+ 5 files changed, 37 insertions(+), 4 deletions(-)
+ create mode 100644 tee-supplicant/optee-udev.rules.in
+ create mode 100644 tee-supplicant/tee-supplicant@.service.in
+
+Upstream-Status: Backport
+
+diff --git a/config.mk b/config.mk
+index eae481f..3def087 100644
+--- a/config.mk
++++ b/config.mk
+@@ -23,7 +23,7 @@ CFG_TEE_SUPP_LOG_LEVEL?=1
+ #   This folder can be created with the required permission in an init
+ #   script during boot, else it will be created by the tee-supplicant on
+ #   first REE FS access.
+-CFG_TEE_FS_PARENT_PATH ?= /data/tee
++CFG_TEE_FS_PARENT_PATH ?= /var/lib/tee
+ 
+ # CFG_TEE_CLIENT_LOG_FILE
+ #   The location of the client log file when logging to file is enabled.
+diff --git a/libteec/CMakeLists.txt b/libteec/CMakeLists.txt
+index c742d31..c857369 100644
+--- a/libteec/CMakeLists.txt
++++ b/libteec/CMakeLists.txt
+@@ -14,7 +14,7 @@ endif()
+ # Configuration flags always included
+ ################################################################################
+ set(CFG_TEE_CLIENT_LOG_LEVEL "1" CACHE STRING "libteec log level")
+-set(CFG_TEE_CLIENT_LOG_FILE "/data/tee/teec.log" CACHE STRING "Location of libteec log")
++set(CFG_TEE_CLIENT_LOG_FILE "${CMAKE_INSTALL_LOCALSTATEDIR}/lib/tee/teec.log" CACHE STRING "Location of libteec log")
+ 
+ ################################################################################
+ # Source files
+diff --git a/tee-supplicant/CMakeLists.txt b/tee-supplicant/CMakeLists.txt
+index 54a34c7..8df9bef 100644
+--- a/tee-supplicant/CMakeLists.txt
++++ b/tee-supplicant/CMakeLists.txt
+@@ -11,10 +11,15 @@ option(CFG_TEE_SUPP_PLUGINS "Enable tee-supplicant plugin support" ON)
+ set(CFG_TEE_SUPP_LOG_LEVEL "1" CACHE STRING "tee-supplicant log level")
+ # FIXME: Question is, is this really needed? Should just use defaults from # GNUInstallDirs?
+ set(CFG_TEE_CLIENT_LOAD_PATH "/lib" CACHE STRING "Colon-separated list of paths where to look for TAs (see also --ta-dir)")
+-set(CFG_TEE_FS_PARENT_PATH "/data/tee" CACHE STRING "Location of TEE filesystem (secure storage)")
++set(CFG_TEE_FS_PARENT_PATH "${CMAKE_INSTALL_LOCALSTATEDIR}/lib/tee" CACHE STRING "Location of TEE filesystem (secure storage)")
+ # FIXME: Why do we have if defined(CFG_GP_SOCKETS) && CFG_GP_SOCKETS == 1 in the c-file?
+ set(CFG_GP_SOCKETS "1" CACHE STRING "Enable GlobalPlatform Socket API support")
+-set(CFG_TEE_PLUGIN_LOAD_PATH "/usr/lib/tee-supplicant/plugins/" CACHE STRING "tee-supplicant's plugins path")
++set(CFG_TEE_PLUGIN_LOAD_PATH "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${PROJECT_NAME}/plugins/" CACHE STRING "tee-supplicant's plugins path")
++
++set(CFG_TEE_GROUP "tee" CACHE STRING "Group which has access to /dev/tee* devices")
++set(CFG_TEEPRIV_GROUP "teepriv" CACHE STRING "Group which has access to /dev/teepriv* devices")
++set(CFG_TEE_SUPPL_USER "teesuppl" CACHE STRING "User account which tee-supplicant is started with")
++set(CFG_TEE_SUPPL_GROUP "teesuppl" CACHE STRING "Group account which tee-supplicant is started with")
+ 
+ if(CFG_TEE_SUPP_PLUGINS)
+ 	set(CMAKE_INSTALL_RPATH "${CFG_TEE_PLUGIN_LOAD_PATH}")
+@@ -113,3 +118,7 @@ endif()
+ # Install targets
+ ################################################################################
+ install(TARGETS ${PROJECT_NAME} RUNTIME DESTINATION ${CMAKE_INSTALL_SBINDIR})
++configure_file(tee-supplicant@.service.in tee-supplicant@.service @ONLY)
++install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/tee-supplicant@.service DESTINATION ${CMAKE_INSTALL_LIBDIR}/systemd/system)
++configure_file(optee-udev.rules.in optee-udev.rules @ONLY)
++install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/optee-udev.rules DESTINATION ${CMAKE_INSTALL_SYSCONFDIR}/udev/rules.d)
+diff --git a/tee-supplicant/optee-udev.rules.in b/tee-supplicant/optee-udev.rules.in
+new file mode 100644
+index 0000000..275e833
+--- /dev/null
++++ b/tee-supplicant/optee-udev.rules.in
+@@ -0,0 +1,7 @@
++# SPDX-License-Identifier: BSD-2-Clause
++KERNEL=="tee[0-9]*", MODE="0660", OWNER="root", GROUP="@CFG_TEE_GROUP@", TAG+="systemd"
++
++# If a /dev/teepriv[0-9]* device is detected, start an instance of
++# tee-supplicant.service with the device name as parameter
++KERNEL=="teepriv[0-9]*", MODE="0660", OWNER="root", GROUP="@CFG_TEEPRIV_GROUP@", \
++    TAG+="systemd", ENV{SYSTEMD_WANTS}+="tee-supplicant@%k.service"
+diff --git a/tee-supplicant/tee-supplicant@.service.in b/tee-supplicant/tee-supplicant@.service.in
+new file mode 100644
+index 0000000..e53a935
+--- /dev/null
++++ b/tee-supplicant/tee-supplicant@.service.in
+@@ -0,0 +1,17 @@
++# SPDX-License-Identifier: BSD-2-Clause
++[Unit]
++Description=TEE Supplicant on %i
++DefaultDependencies=no
++After=dev-%i.device
++Wants=dev-%i.device
++Conflicts=shutdown.target
++Before=tpm2.target sysinit.target shutdown.target
++
++[Service]
++Type=notify
++User=@CFG_TEE_SUPPL_USER@
++Group=@CFG_TEE_SUPPL_GROUP@
++EnvironmentFile=-@CMAKE_INSTALL_SYSCONFDIR@/default/tee-supplicant
++ExecStart=@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_SBINDIR@/tee-supplicant $OPTARGS
++# Workaround for fTPM TA: stop kernel module before tee-supplicant
++ExecStop=-/bin/sh -c "/sbin/modprobe -v -r tpm_ftpm_tee ; /bin/kill $MAINPID"
+-- 
+2.34.1
+
diff --git a/meta-arm/recipes-security/optee/optee-client/optee-udev.rules b/meta-arm/recipes-security/optee/optee-client/optee-udev.rules
deleted file mode 100644
index 075f469c..00000000
--- a/meta-arm/recipes-security/optee/optee-client/optee-udev.rules
+++ /dev/null
@@ -1,6 +0,0 @@
-KERNEL=="tee[0-9]*", MODE="0660", OWNER="root", GROUP="teeclnt", TAG+="systemd"
-
-# If a /dev/teepriv[0-9]* device is detected, start an instance of
-# tee-supplicant.service with the device name as parameter
-KERNEL=="teepriv[0-9]*", MODE="0660", OWNER="root", GROUP="teeclnt", \
-    TAG+="systemd", ENV{SYSTEMD_WANTS}+="tee-supplicant@%k.service"
diff --git a/meta-arm/recipes-security/optee/optee-client/tee-supplicant@.service b/meta-arm/recipes-security/optee/optee-client/tee-supplicant@.service
deleted file mode 100644
index e3039fde..00000000
--- a/meta-arm/recipes-security/optee/optee-client/tee-supplicant@.service
+++ /dev/null
@@ -1,13 +0,0 @@
-[Unit]
-Description=TEE Supplicant on %i
-DefaultDependencies=no
-After=dev-%i.device
-Wants=dev-%i.device
-Conflicts=shutdown.target
-Before=tpm2.target sysinit.target shutdown.target
-
-[Service]
-Type=notify
-EnvironmentFile=-@sysconfdir@/default/tee-supplicant
-ExecStart=@sbindir@/tee-supplicant $OPTARGS
-ExecStop=-/bin/sh -c "/sbin/modprobe -v -r tpm_ftpm_tee ; /bin/kill $MAINPID"
diff --git a/meta-arm/recipes-security/optee/optee-client_4.3.0.bb b/meta-arm/recipes-security/optee/optee-client_4.3.0.bb
index 4a088004..edab4583 100644
--- a/meta-arm/recipes-security/optee/optee-client_4.3.0.bb
+++ b/meta-arm/recipes-security/optee/optee-client_4.3.0.bb
@@ -2,6 +2,8 @@ require recipes-security/optee/optee-client.inc
 
 SRCREV = "a5b1ffcd26e328af0bbf18ab448a38ecd558e05c"
 
+SRC_URI += "file://0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch"
+
 inherit pkgconfig
 DEPENDS += "util-linux"
 EXTRA_OEMAKE += "PKG_CONFIG=pkg-config"
-- 
2.34.1

[PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Mikko Rapeli]

optee-client now provides them and sets "tee" group for
/dev/tee* and "teepriv" group for /dev/teepriv* access.
tee-supplicant runs as "teesuppl" user account.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 .../trusted-services/libts/tee-udev.rules     |  7 -------
 .../trusted-services/libts_git.bb             | 19 ++-----------------
 2 files changed, 2 insertions(+), 24 deletions(-)
 delete mode 100644 meta-arm/recipes-security/trusted-services/libts/tee-udev.rules

diff --git a/meta-arm/recipes-security/trusted-services/libts/tee-udev.rules b/meta-arm/recipes-security/trusted-services/libts/tee-udev.rules
deleted file mode 100644
index 43fafd8c..00000000
--- a/meta-arm/recipes-security/trusted-services/libts/tee-udev.rules
+++ /dev/null
@@ -1,7 +0,0 @@
-# tee devices can only be accessed by the teeclnt group members
-KERNEL=="tee[0-9]*", TAG+="systemd", MODE="0660", GROUP="teeclnt"
-
-# If a /dev/teepriv[0-9]* device is detected, start an instance of
-# tee-supplicant.service with the device name as parameter
-KERNEL=="teepriv[0-9]*", MODE="0660", OWNER="root", GROUP="tee", \
-    TAG+="systemd", ENV{SYSTEMD_WANTS}+="tee-supplicant@%k.service"
diff --git a/meta-arm/recipes-security/trusted-services/libts_git.bb b/meta-arm/recipes-security/trusted-services/libts_git.bb
index 635e4769..9d0e7373 100644
--- a/meta-arm/recipes-security/trusted-services/libts_git.bb
+++ b/meta-arm/recipes-security/trusted-services/libts_git.bb
@@ -5,24 +5,13 @@ TS_ENV = "arm-linux"
 
 require trusted-services.inc
 
-SRC_URI += "file://tee-udev.rules \
-            file://0001-Remove-TEE-driver-external-component.patch \
-           "
+SRC_URI += "file://0001-Remove-TEE-driver-external-component.patch"
 
 OECMAKE_SOURCEPATH="${S}/deployments/libts/${TS_ENV}"
 
 DEPENDS           += "arm-ffa-user"
 
-# Unix group name for dev/tee* ownership.
-TEE_GROUP_NAME ?= "teeclnt"
-
 do_install:append () {
-    if ${@oe.utils.conditional('VIRTUAL-RUNTIME_dev_manager', 'busybox-mdev', 'false', 'true', d)}; then
-        install -d ${D}${nonarch_base_libdir}/udev/rules.d/
-        install -m 755 ${UNPACKDIR}/tee-udev.rules ${D}${nonarch_base_libdir}/udev/rules.d/
-        sed -i -e "s/teeclnt/${TEE_GROUP_NAME}/" ${D}${nonarch_base_libdir}/udev/rules.d/tee-udev.rules
-    fi
-
     # Move the dynamic libraries into the standard place.
     install -d ${D}${libdir}
     mv ${D}${TS_INSTALL}/lib/libts* ${D}${libdir}
@@ -34,9 +23,5 @@ do_install:append () {
     fi
 }
 
-inherit ${@oe.utils.conditional('VIRTUAL-RUNTIME_dev_manager', 'busybox-mdev', '', 'useradd', d)}
-USERADD_PACKAGES = "${PN}"
-GROUPADD_PARAM:${PN} = "--system ${TEE_GROUP_NAME}"
-
-FILES:${PN} = "${libdir}/libts.so.* ${nonarch_base_libdir}/udev/rules.d/"
+FILES:${PN} = "${libdir}/libts.so.*"
 FILES:${PN}-dev = "${TS_INSTALL}/lib/cmake ${TS_INSTALL}/include ${libdir}/libts.so"
-- 
2.34.1

Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Gyorgy Szing]

[-- Attachment #1: Type: text/plain, Size: 3665 bytes --]

Hi,

How is the system going to work if op-tee client is not deployed, but libts is? E.g. if the SPMC is Hafnium and no S-EL1 OP-TEE SP is needed, but the FWU SP is used?

/George

From: meta-arm@lists.yoctoproject.org <meta-arm@lists.yoctoproject.org> on behalf of Mikko Rapeli via lists.yoctoproject.org <mikko.rapeli=linaro.org@lists.yoctoproject.org>
Date: Thursday, 17 October 2024 at 09:01
To: meta-arm@lists.yoctoproject.org <meta-arm@lists.yoctoproject.org>
Cc: Mikko Rapeli <mikko.rapeli@linaro.org>
Subject: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings
optee-client now provides them and sets "tee" group for
/dev/tee* and "teepriv" group for /dev/teepriv* access.
tee-supplicant runs as "teesuppl" user account.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 .../trusted-services/libts/tee-udev.rules     |  7 -------
 .../trusted-services/libts_git.bb             | 19 ++-----------------
 2 files changed, 2 insertions(+), 24 deletions(-)
 delete mode 100644 meta-arm/recipes-security/trusted-services/libts/tee-udev.rules

diff --git a/meta-arm/recipes-security/trusted-services/libts/tee-udev.rules b/meta-arm/recipes-security/trusted-services/libts/tee-udev.rules
deleted file mode 100644
index 43fafd8c..00000000
--- a/meta-arm/recipes-security/trusted-services/libts/tee-udev.rules
+++ /dev/null
@@ -1,7 +0,0 @@
-# tee devices can only be accessed by the teeclnt group members
-KERNEL=="tee[0-9]*", TAG+="systemd", MODE="0660", GROUP="teeclnt"
-
-# If a /dev/teepriv[0-9]* device is detected, start an instance of
-# tee-supplicant.service with the device name as parameter
-KERNEL=="teepriv[0-9]*", MODE="0660", OWNER="root", GROUP="tee", \
-    TAG+="systemd", ENV{SYSTEMD_WANTS}+="tee-supplicant@%k.service"
diff --git a/meta-arm/recipes-security/trusted-services/libts_git.bb b/meta-arm/recipes-security/trusted-services/libts_git.bb
index 635e4769..9d0e7373 100644
--- a/meta-arm/recipes-security/trusted-services/libts_git.bb
+++ b/meta-arm/recipes-security/trusted-services/libts_git.bb
@@ -5,24 +5,13 @@ TS_ENV = "arm-linux"

 require trusted-services.inc

-SRC_URI += "file://tee-udev.rules \
-            file://0001-Remove-TEE-driver-external-component.patch<file:///0001-Remove-TEE-driver-external-component.patch> \
-           "
+SRC_URI += "file://0001-Remove-TEE-driver-external-component.patch<file:///0001-Remove-TEE-driver-external-component.patch>"

 OECMAKE_SOURCEPATH="${S}/deployments/libts/${TS_ENV}"

 DEPENDS           += "arm-ffa-user"

-# Unix group name for dev/tee* ownership.
-TEE_GROUP_NAME ?= "teeclnt"
-
 do_install:append () {
-    if ${@oe.utils.conditional('VIRTUAL-RUNTIME_dev_manager', 'busybox-mdev', 'false', 'true', d)}; then
-        install -d ${D}${nonarch_base_libdir}/udev/rules.d/
-        install -m 755 ${UNPACKDIR}/tee-udev.rules ${D}${nonarch_base_libdir}/udev/rules.d/
-        sed -i -e "s/teeclnt/${TEE_GROUP_NAME}/" ${D}${nonarch_base_libdir}/udev/rules.d/tee-udev.rules
-    fi
-
     # Move the dynamic libraries into the standard place.
     install -d ${D}${libdir}
     mv ${D}${TS_INSTALL}/lib/libts* ${D}${libdir}
@@ -34,9 +23,5 @@ do_install:append () {
     fi
 }

-inherit ${@oe.utils.conditional('VIRTUAL-RUNTIME_dev_manager', 'busybox-mdev', '', 'useradd', d)}
-USERADD_PACKAGES = "${PN}"
-GROUPADD_PARAM:${PN} = "--system ${TEE_GROUP_NAME}"
-
-FILES:${PN} = "${libdir}/libts.so.* ${nonarch_base_libdir}/udev/rules.d/"
+FILES:${PN} = "${libdir}/libts.so.*"
 FILES:${PN}-dev = "${TS_INSTALL}/lib/cmake ${TS_INSTALL}/include ${libdir}/libts.so"
--
2.34.1

[-- Attachment #2: Type: text/html, Size: 7211 bytes --]

Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Mikko Rapeli]

Hi,

On Thu, Oct 17, 2024 at 08:17:32AM +0000, Gyorgy Szing wrote:
> How is the system going to work if op-tee client is not deployed, but libts is? E.g. if the SPMC is Hafnium and no S-EL1 OP-TEE SP is needed, but the FWU SP is used?

If optee-client is not installed, then optee is not functional
in Linux userspace and rootfs so optee-test fails. The plain
kernel driver for optee will work and /dev/tee* and /dev/teepriv* interfaces
will pop up with root access rights, but not with the setup expected by
tee-supplicant systemd service file from optee-client. This may be enough for TS
use cases.

In the firmware side, optee services are not affected.

If this impacts libts users, then IMO they need to install optee-client or
setup the udev rules etc in some other way.

Is there some problem I missed or a setup which is not covered in meta-arm testing?

Cheers,

-Mikko

Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Gyorgy Szing]

[-- Attachment #1: Type: text/plain, Size: 2230 bytes --]

Hi,

The Trusted Services protocol is implemented by the tstee driver and libts.  This means there are multiple drivers using the tee subsystem and multiple ecosystems relying on the /dev/tee devices.
Your changes move the tee driver access configuration to the op-tee client only and this adds a dependency between the two ecosystems. The TS ecosystem will not work without the op-tee client, but op-tee client is not part of that ecosystem.

Yes, the /dev/tee* devices will be still there, but only accessible with root privileges which is kind of a feature degradation.

“If this impacts libts users, then IMO they need to install optee-client orsetup the udev rules etc in some other way.“
Yes, this “some other way” is the udev rules config you are removing.

“Is there some problem I missed or a setup which is not covered in meta-arm testing?”
AFAIK currently there is no meta-arm config which enables FF-A but does not use the OP-TEE SPMC. But this will change soon. So why not do this properly now?

/George


From: Mikko Rapeli <mikko.rapeli@linaro.org>
Date: Thursday, 17 October 2024 at 10:34
To: Gyorgy Szing <Gyorgy.Szing@arm.com>
Cc: meta-arm@lists.yoctoproject.org <meta-arm@lists.yoctoproject.org>
Subject: Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings
Hi,

On Thu, Oct 17, 2024 at 08:17:32AM +0000, Gyorgy Szing wrote:
> How is the system going to work if op-tee client is not deployed, but libts is? E.g. if the SPMC is Hafnium and no S-EL1 OP-TEE SP is needed, but the FWU SP is used?

If optee-client is not installed, then optee is not functional
in Linux userspace and rootfs so optee-test fails. The plain
kernel driver for optee will work and /dev/tee* and /dev/teepriv* interfaces
will pop up with root access rights, but not with the setup expected by
tee-supplicant systemd service file from optee-client. This may be enough for TS
use cases.

In the firmware side, optee services are not affected.

If this impacts libts users, then IMO they need to install optee-client or
setup the udev rules etc in some other way.

Is there some problem I missed or a setup which is not covered in meta-arm testing?

Cheers,

-Mikko

[-- Attachment #2: Type: text/html, Size: 5736 bytes --]

Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Mikko Rapeli]

Hi,

On Thu, Oct 17, 2024 at 09:44:07AM +0000, Gyorgy Szing wrote:
> Hi,
> 
> The Trusted Services protocol is implemented by the tstee driver and libts.  This means there are multiple drivers using the tee subsystem and multiple ecosystems relying on the /dev/tee devices.
> Your changes move the tee driver access configuration to the op-tee client only and this adds a dependency between the two ecosystems. The TS ecosystem will not work without the op-tee client, but op-tee client is not part of that ecosystem.

But optee and optee-client are part of the TS images and configs so there is a link.

> Yes, the /dev/tee* devices will be still there, but only accessible with root privileges which is kind of a feature degradation.

I don't see any user being added to the previously used "teeclnt" group.
So which non-root users are there? If the users are in Cassini, I have
proposed fixes there.

> “If this impacts libts users, then IMO they need to install optee-client orsetup the udev rules etc in some other way.“
> Yes, this “some other way” is the udev rules config you are removing.
> 
> “Is there some problem I missed or a setup which is not covered in meta-arm testing?”
> AFAIK currently there is no meta-arm config which enables FF-A but does not use the OP-TEE SPMC. But this will change soon. So why not do this properly now?

What is the proper way to fix this?

I don't think duplicating optee/tee-supplicant udev rules is the answer.
I could move optee-client recipe udev rules to a separate binary package
to enable installing without tee-supplicant. But I'd like to see the
users and have a test case, preferably in meta-arm.

Cheers,

-Mikko

Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Gyorgy Szing]

[-- Attachment #1: Type: text/plain, Size: 3088 bytes --]

Hi,

“But optee and optee-client are part of the TS images and configs so there is a link.”
“I don't see any user being added to the previously used "teeclnt" group. So which non-root users are there? If the users are in Cassini, I have proposed fixes there.”
I am not saying the TS recipes are perfect and there might be a dependency, which is an error. Except for the OP-TEE SPMC tests SPs which indeed depend on op-tee and xtest.

“What is the proper way to fix this?

I don't think duplicating optee/tee-supplicant udev rules is the answer.
I could move optee-client recipe udev rules to a separate binary package
to enable installing without tee-supplicant.”
I think a dedicated recipe on which both libts and optee-client depends is one way to fix.

“But I'd like to see the users and have a test case, preferably in meta-arm.”
Well, I cannot provide a setup where TS is used without OP-TEE currently, but your config can and will be tested in the CI. Yes, TS only world might still be broken, but the change at least would make a step in the right direction.

/George

From: Mikko Rapeli <mikko.rapeli@linaro.org>
Date: Thursday, 17 October 2024 at 11:52
To: Gyorgy Szing <Gyorgy.Szing@arm.com>
Cc: meta-arm@lists.yoctoproject.org <meta-arm@lists.yoctoproject.org>
Subject: Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings
Hi,

On Thu, Oct 17, 2024 at 09:44:07AM +0000, Gyorgy Szing wrote:
> Hi,
>
> The Trusted Services protocol is implemented by the tstee driver and libts.  This means there are multiple drivers using the tee subsystem and multiple ecosystems relying on the /dev/tee devices.
> Your changes move the tee driver access configuration to the op-tee client only and this adds a dependency between the two ecosystems. The TS ecosystem will not work without the op-tee client, but op-tee client is not part of that ecosystem.

But optee and optee-client are part of the TS images and configs so there is a link.

> Yes, the /dev/tee* devices will be still there, but only accessible with root privileges which is kind of a feature degradation.

I don't see any user being added to the previously used "teeclnt" group.
So which non-root users are there? If the users are in Cassini, I have
proposed fixes there.

> “If this impacts libts users, then IMO they need to install optee-client orsetup the udev rules etc in some other way.“
> Yes, this “some other way” is the udev rules config you are removing.
>
> “Is there some problem I missed or a setup which is not covered in meta-arm testing?”
> AFAIK currently there is no meta-arm config which enables FF-A but does not use the OP-TEE SPMC. But this will change soon. So why not do this properly now?

What is the proper way to fix this?

I don't think duplicating optee/tee-supplicant udev rules is the answer.
I could move optee-client recipe udev rules to a separate binary package
to enable installing without tee-supplicant. But I'd like to see the
users and have a test case, preferably in meta-arm.

Cheers,

-Mikko

[-- Attachment #2: Type: text/html, Size: 6555 bytes --]

Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Mikko Rapeli]

Hi,

On Thu, Oct 17, 2024 at 10:54:41AM +0000, Gyorgy Szing wrote:
> Hi,
> 
> “But optee and optee-client are part of the TS images and configs so there is a link.”
> “I don't see any user being added to the previously used "teeclnt" group. So which non-root users are there? If the users are in Cassini, I have proposed fixes there.”
> I am not saying the TS recipes are perfect and there might be a dependency, which is an error. Except for the OP-TEE SPMC tests SPs which indeed depend on op-tee and xtest.
>
> “What is the proper way to fix this?
> 
> I don't think duplicating optee/tee-supplicant udev rules is the answer.
> I could move optee-client recipe udev rules to a separate binary package
> to enable installing without tee-supplicant.”
> I think a dedicated recipe on which both libts and optee-client depends is one way to fix.

A dedicated recipe is not ok. optee-client upstream provides the udev rule.
A dedicated binary package from optee-client for the udev rule could be
created. The udev rule and matching systemd service (and possibly sysvinit
script) are non-trivial to setup and thus in meta-arm recipe some aspects
were wrong and other Linux distros have even more issues. Thus it's better
to collaborate with upstream when setting them up.

> “But I'd like to see the users and have a test case, preferably in meta-arm.”
> Well, I cannot provide a setup where TS is used without OP-TEE currently, but your config can and will be tested in the CI. Yes, TS only world might still be broken, but the change at least would make a step in the right direction.

I'm still not sure of the right direction. Which recipes and layers have userspace
SW which needs to access /dev/tee* or /dev/teepriv* devices nodes without root
rights in userspace? Where is a test for these recipes or functionality?

The old teeclnt setup was a bit broken. For example tee-supplicant was still
started in systemd service file as root user. Both /dev/tee* and /dev/teepriv*
used the same teeclnt group which was not right according to upstream,
they need to have different groups. Then the systemd service which did not
work in initrd. Etc.

Cheers,

-Mikko

Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Gyorgy Szing]

[-- Attachment #1: Type: text/plain, Size: 3316 bytes --]



On 2024-10-17, 13:09, "Mikko Rapeli" <mikko.rapeli@linaro.org> wrote:

> Hi,
>
> On Thu, Oct 17, 2024 at 10:54:41AM +0000, Gyorgy Szing wrote:
> > Hi,
> >
> > “But optee and optee-client are part of the TS images and configs so there is a link.”
> > “I don't see any user being added to the previously used "teeclnt" group. So which non-root users are there? If the users are in Cassini, I have proposed fixes there.”
> > I am not saying the TS recipes are perfect and there might be a dependency, which is an error. Except for the OP-TEE SPMC tests SPs which indeed depend on op-tee and xtest.
> >
> > “What is the proper way to fix this?
> >
> > I don't think duplicating optee/tee-supplicant udev rules is the answer.
> > I could move optee-client recipe udev rules to a separate binary package
> > to enable installing without tee-supplicant.”
> > I think a dedicated recipe on which both libts and optee-client depends is one way to fix.
>
> A dedicated recipe is not ok. optee-client upstream provides the udev rule.
> A dedicated binary package from optee-client for the udev rule could be
> created. The udev rule and matching systemd service (and possibly sysvinit
> script) are non-trivial to setup and thus in meta-arm recipe some aspects
> were wrong and other Linux distros have even more issues. Thus it's better
> to collaborate with upstream when setting them up.

Op-tee client up-stream providing the udev rule might be a wrong decision based on you highlights of the complexities and the conclusion, this should be managed in sync with distros. Does this mean Yocto (a distro) should provide its own solution and only use op-tee client up-stream implementation as an example. Also, the optee-client up-stream implementation might be tailored for the op-tee integration system (op-tee build repo).

>
> > “But I'd like to see the users and have a test case, preferably in meta-arm.”
> > Well, I cannot provide a setup where TS is used without OP-TEE currently, but your config can and will be tested in the CI. Yes, TS only world might still be broken, but the change at least would make a step in the right direction.
>
> I'm still not sure of the right direction. Which recipes and layers have userspace
> SW which needs to access /dev/tee* or /dev/teepriv* devices nodes without root
> rights in userspace? Where is a test for these recipes or functionality?

Any platform deploying Trusted Services will need access to /dev/tee*. Please see the recipes not starting with ts-sp- prefix under meta-arm/meta-arm/recipes-security/trusted-services.
TS is currently deployed on base_fvp, qeamuarm64-secureboot, n1sdp, and Corstone100.
All of these is using OP-TEE SPMC and deploying optee-client. There will be platforms using Hafnium SPMC and TS, and not deploying optee-client.

You can find the test here: meta-arm/lib/oeqa/runtime/cases/trusted_services.py

> The old teeclnt setup was a bit broken. For example tee-supplicant was still
> started in systemd service file as root user. Both /dev/tee* and /dev/teepriv*
> used the same teeclnt group which was not right according to upstream,
> they need to have different groups. Then the systemd service which did not
> work in initrd. Etc.
Great you fixed this.

>
> Cheers,
>
> -Mikko
>

[-- Attachment #2: Type: text/html, Size: 9014 bytes --]

Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Adam Johnston]

[-- Attachment #1: Type: text/plain, Size: 3377 bytes --]

>
> I'm still not sure of the right direction. Which recipes and layers have
> userspace
> SW which needs to access /dev/tee* or /dev/teepriv* devices nodes without
> root
> rights in userspace? Where is a test for these recipes or functionality?


Parsec uses the teeclnt group for access to the PSA APIs when TS is in
PACKAGECONFIG but I'm not sure they test that configuration.
Cassini tests that config and also use the same group to run the PSA API
tests

Regards

Adam

On Thu, 17 Oct 2024 at 12:09, Mikko Rapeli via lists.yoctoproject.org
<mikko.rapeli=linaro.org@lists.yoctoproject.org> wrote:

> Hi,
>
> On Thu, Oct 17, 2024 at 10:54:41AM +0000, Gyorgy Szing wrote:
> > Hi,
> >
> > “But optee and optee-client are part of the TS images and configs so
> there is a link.”
> > “I don't see any user being added to the previously used "teeclnt"
> group. So which non-root users are there? If the users are in Cassini, I
> have proposed fixes there.”
> > I am not saying the TS recipes are perfect and there might be a
> dependency, which is an error. Except for the OP-TEE SPMC tests SPs which
> indeed depend on op-tee and xtest.
> >
> > “What is the proper way to fix this?
> >
> > I don't think duplicating optee/tee-supplicant udev rules is the answer.
> > I could move optee-client recipe udev rules to a separate binary package
> > to enable installing without tee-supplicant.”
> > I think a dedicated recipe on which both libts and optee-client depends
> is one way to fix.
>
> A dedicated recipe is not ok. optee-client upstream provides the udev rule.
> A dedicated binary package from optee-client for the udev rule could be
> created. The udev rule and matching systemd service (and possibly sysvinit
> script) are non-trivial to setup and thus in meta-arm recipe some aspects
> were wrong and other Linux distros have even more issues. Thus it's better
> to collaborate with upstream when setting them up.
>
> > “But I'd like to see the users and have a test case, preferably in
> meta-arm.”
> > Well, I cannot provide a setup where TS is used without OP-TEE
> currently, but your config can and will be tested in the CI. Yes, TS only
> world might still be broken, but the change at least would make a step in
> the right direction.
>
> I'm still not sure of the right direction. Which recipes and layers have
> userspace
> SW which needs to access /dev/tee* or /dev/teepriv* devices nodes without
> root
> rights in userspace? Where is a test for these recipes or functionality?
>
> The old teeclnt setup was a bit broken. For example tee-supplicant was
> still
> started in systemd service file as root user. Both /dev/tee* and
> /dev/teepriv*
> used the same teeclnt group which was not right according to upstream,
> they need to have different groups. Then the systemd service which did not
> work in initrd. Etc.
>
> Cheers,
>
> -Mikko
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#6210):
> https://lists.yoctoproject.org/g/meta-arm/message/6210
> Mute This Topic: https://lists.yoctoproject.org/mt/109059457/8940727
> Group Owner: meta-arm+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/meta-arm/unsub [
> adam.johnston@linaro.org]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

[-- Attachment #2: Type: text/html, Size: 4473 bytes --]

Re: [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Anton Antonov]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

On Thu, Oct 17, 2024 at 04:09 AM, Mikko Rapeli wrote:

> 
> I'm still not sure of the right direction. Which recipes and layers have
> userspace
> SW which needs to access /dev/tee* or /dev/teepriv* devices nodes without
> root
> rights in userspace? Where is a test for these recipes or functionality?

Parsec is the example you're looking for. And parsec oeqa tests would test TS provider if included

Anton

[-- Attachment #2: Type: text/html, Size: 480 bytes --]

Re: [meta-arm] [PATCH v3 2/2] trusted-service: remove optee udev and group settings

[Mikko Rapeli]

Hi,

On Thu, Oct 17, 2024 at 07:48:27AM -0700, Anton Antonov via lists.yoctoproject.org wrote:
> On Thu, Oct 17, 2024 at 04:09 AM, Mikko Rapeli wrote:
> 
> > 
> > I'm still not sure of the right direction. Which recipes and layers have
> > userspace
> > SW which needs to access /dev/tee* or /dev/teepriv* devices nodes without
> > root
> > rights in userspace? Where is a test for these recipes or functionality?
> 
> Parsec is the example you're looking for. And parsec oeqa tests would test TS provider if included

Thanks! So meta-security/recipes-parsec/parsec-service/parsec-service_1.4.1.bb
would need to change from "teeclnt" to "tee" group to access /dev/tee* devices
according to optee-client udev rule. I can provide the change when/if this
series gets merged. Same for meta-cassini.

But does the really need to be a separate copy of the optee-client udev rule
for TS setups which need optee but not tee-supplicant from optee-client binary
package?

Cheers,

-Mikko

RE: [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Tom Hochstein (OSS)]

Hi Mikko,

We have multilib testing that is failing now because the systemd folder is being installed in `libdir` instead of the non-arch-specific `systemd_system_unitdir = "${nonarch_base_libdir}/systemd/system"` as is codified in `bitbake.conf`.

ERROR: optee-client-4.4.0.imx-r0 do_package: QA Issue: optee-client: Files/directories were installed but not shipped in any package:
  /usr/lib64/systemd
  /usr/lib64/systemd/system
  /usr/lib64/systemd/system/tee-supplicant@.service

I'm not sure how this should be fixed, as it doesn't appear that CMAKE can handle more than one libdir. Can the install of the service file be hard-coded to use /usr/lib? Or does this need to be fixed in the recipe?

Tom

-----Original Message-----
From: Mikko Rapeli <mikko.rapeli@linaro.org>
Sent: Thursday, October 17, 2024 1:59 AM
To: meta-arm@lists.yoctoproject.org
Cc: Mikko Rapeli <mikko.rapeli@linaro.org>; Tom Hochstein <tom.hochstein@nxp.com>; Sahil Malhotra <sahil.malhotra@nxp.com>
Subject: [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

Use backported upstream patch for udev rule and systemd service file.
sysvinit script is still used from meta-arm. Don't install systemd
service without systemd distro feature, other way round for
sysvinit script.

tee-supplicant started by systemd service runs as non-root teesuppl
user with teepriv group. sysvinit still runs as root since busybox
start-stop-daemon doesn't support -g group parameter and -u teesuppl
doesn't seem to change the effective user.

udev rules allow non-root /dev/tee* access from tee and
/dev/teepriv* access from teepriv groups.

Tested sysvinit changes with:

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

and systemd changes with:

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

Cc: tom.hochstein@nxp.com
Cc: sahil.malhotra@nxp.com
Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 .../recipes-security/optee/optee-client.inc   |  30 +--
 ...dd-udev-rule-and-systemd-service-fil.patch | 186 ++++++++++++++++++
 .../optee/optee-client/optee-udev.rules       |   6 -
 .../optee-client/tee-supplicant@.service      |  13 --
 .../optee/optee-client_4.3.0.bb               |   2 +
 5 files changed, 205 insertions(+), 32 deletions(-)
 create mode 100644 meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch
 delete mode 100644 meta-arm/recipes-security/optee/optee-client/optee-udev.rules
 delete mode 100644 meta-arm/recipes-security/optee/optee-client/tee-supplicant@.service

diff --git a/meta-arm/recipes-security/optee/optee-client.inc b/meta-arm/recipes-security/optee/optee-client.inc
index f387c805..fc48c302 100644
--- a/meta-arm/recipes-security/optee/optee-client.inc
+++ b/meta-arm/recipes-security/optee/optee-client.inc
@@ -9,9 +9,7 @@ inherit systemd update-rc.d cmake useradd

 SRC_URI = " \
     git://github.com/OP-TEE/optee_client.git;branch=master;protocol=https \
-    file://tee-supplicant@.service \
     file://tee-supplicant.sh \
-    file://optee-udev.rules \
 "

 UPSTREAM_CHECK_GITTAGREGEX = "^(?P<pver>\d+(\.\d+)+)$"
@@ -20,20 +18,21 @@ S = "${WORKDIR}/git"

 EXTRA_OECMAKE = " \
     -DBUILD_SHARED_LIBS=ON \
-    -DCFG_TEE_FS_PARENT_PATH='${localstatedir}/lib/tee' \
 "
 EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"

 do_install:append() {
-    install -D -p -m0644 ${UNPACKDIR}/tee-supplicant@.service ${D}${systemd_system_unitdir}/tee-supplicant@.service
-    install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
-    install -d ${D}${sysconfdir}/udev/rules.d
-    install -m 0644 ${UNPACKDIR}/optee-udev.rules ${D}${sysconfdir}/udev/rules.d/optee.rules
-
-    sed -i -e s:@sysconfdir@:${sysconfdir}:g \
-           -e s:@sbindir@:${sbindir}:g \
-              ${D}${systemd_system_unitdir}/tee-supplicant@.service \
-              ${D}${sysconfdir}/init.d/tee-supplicant
+    # installed by default
+    if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
+        rm -rf ${D}${libdir}/systemd
+    fi
+    if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
+        install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
+        sed -i -e s:@sysconfdir@:${sysconfdir}:g \
+               -e s:@sbindir@:${sbindir}:g \
+                  ${D}${sysconfdir}/init.d/tee-supplicant
+    fi
+    install -o teesuppl -g teesuppl -m 0700 -d ${D}${localstatedir}/lib/tee
 }

 SYSTEMD_SERVICE:${PN} = "tee-supplicant@.service"
@@ -42,5 +41,10 @@ INITSCRIPT_PACKAGES = "${PN}"
 INITSCRIPT_NAME:${PN} = "tee-supplicant"
 INITSCRIPT_PARAMS:${PN} = "start 10 1 2 3 4 5 . stop 90 0 6 ."

+# Users and groups:
+# tee group to access /dev/tee*
+# teepriv group to acess /dev/teepriv*, only tee-supplicant
+# teesuppl user and group teesuppl to run tee-supplicant
 USERADD_PACKAGES = "${PN}"
-GROUPADD_PARAM:${PN} = "--system teeclnt"
+GROUPADD_PARAM:${PN} = "--system tee; --system teepriv; --system teesuppl"
+USERADD_PARAM:${PN} = "--system -g teesuppl --groups teepriv --home-dir ${localstatedir}/lib/tee -M --shell /sbin/nologin teesuppl;"
diff --git a/meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch b/meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch
new file mode 100644
index 00000000..18c0d950
--- /dev/null
+++ b/meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch
@@ -0,0 +1,186 @@
+From bf0d02758696ee7a9f7af9e95f85f5c238d0e109 Mon Sep 17 00:00:00 2001
+From: Mikko Rapeli <mikko.rapeli@linaro.org>
+Date: Wed, 2 Oct 2024 15:24:21 +0100
+Subject: [PATCH] tee-supplicant: add udev rule and systemd service file
+
+tee-supplicant startup with systemd init based
+is non-trivial. Add sample udev rule and systemd
+service files here so that distros can co-operate maintaining
+them.
+
+Files are from meta-arm https://git.yoctoproject.org/meta-arm
+at commit 7cce43e632daa8650f683ac726f9124681b302a4 with license
+MIT and authors:
+
+Peter Griffin <peter.griffin@linaro.org>
+Joshua Watt <JPEWhacker@gmail.com>
+Javier Tia <javier.tia@linaro.org>
+Mikko Rapeli <mikko.rapeli@linaro.org>
+
+With permission from the authors, files can be relicensed to
+BSD-2-Clause like rest of optee client repo.
+
+The config files expect to find tee and teepriv system groups
+and teesuppl user and group (part of teepriv group) for running
+tee-supplicant. Additionally state directory /var/lib/tee
+must be owned by teesuppl user and group with no rights
+to other users. The groups and user can be changed via
+CMake variables:
+
+CFG_TEE_GROUP
+CFG_TEEPRIV_GROUP
+CFG_TEE_SUPPL_USER
+CFG_TEE_SUPPL_GROUP
+
+Change storage path from /data to /var/lib and
+use standard CMake variables also for constructing install
+paths which can be override to change the defaults:
+
+CMAKE_INSTALL_PREFIX, e.g. /
+CMAKE_INSTALL_LIBDIR, e.g. /usr/lib
+CMAKE_INSTALL_LOCALSTATEDIR /var
+
+Once these are setup, udev will start tee-supplicant in initramfs
+or rootfs with teesuppl user and group when /dev/teepriv
+device appears. The systemd service starts before tpm2.target
+(new in systemd 256) which starts early in initramfs and in main rootfs.
+This covers firmware TPM TA usecases for main rootfs encryption. When
+stopping tee-supplicant, the ftpm kernel modules are removed and only
+then the main process stopped to avoid fTPM breakage. These workarounds
+may be removed once RPMB kernel and optee patches without tee-supplicant
+are merged (Linux kernel >= 6.12-rc1, optee_os latest master or >= 4.4).
+
+Tested on yocto meta-arm setup which runs fTPM and optee-test/xtest
+under qemuarm64:
+
+$ git clone https://git.yoctoproject.org/meta-arm
+$ cd meta-arm
+$ SSTATE_DIR=$HOME/sstate DL_DIR=$HOME/download kas build \
+ci/qemuarm64-secureboot.yml:ci/poky-altcfg.yml:ci/testimage.yml
+
+Compiled image can be manually started to qemu serial console with:
+
+$ SSTATE_DIR=$HOME/sstate DL_DIR=$HOME/download kas shell \
+ci/qemuarm64-secureboot.yml:ci/poky-altcfg.yml:ci/testimage.yml
+$ runqemu slirp nographic
+
+meta-arm maintainers run these tests as part of their CI.
+
+Note that if the tee-supplicant state directory /var/lib/tee
+can not be accessed due permissions or other problems, then
+tee-supplicant startup with systemd still works. Only optee-test/xtest
+will be failing and fTPM kernel drivers fail to load with error
+messages.
+
+Cc: Peter Griffin <peter.griffin@linaro.org>
+Cc: Joshua Watt <JPEWhacker@gmail.com>
+Cc: Javier Tia <javier.tia@linaro.org>
+Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
+Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
+---
+ config.mk                                 |  2 +-
+ libteec/CMakeLists.txt                    |  2 +-
+ tee-supplicant/CMakeLists.txt             | 13 +++++++++++--
+ tee-supplicant/optee-udev.rules.in        |  7 +++++++
+ tee-supplicant/tee-supplicant@.service.in | 17 +++++++++++++++++
+ 5 files changed, 37 insertions(+), 4 deletions(-)
+ create mode 100644 tee-supplicant/optee-udev.rules.in
+ create mode 100644 tee-supplicant/tee-supplicant@.service.in
+
+Upstream-Status: Backport
+
+diff --git a/config.mk b/config.mk
+index eae481f..3def087 100644
+--- a/config.mk
++++ b/config.mk
+@@ -23,7 +23,7 @@ CFG_TEE_SUPP_LOG_LEVEL?=1
+ #   This folder can be created with the required permission in an init
+ #   script during boot, else it will be created by the tee-supplicant on
+ #   first REE FS access.
+-CFG_TEE_FS_PARENT_PATH ?= /data/tee
++CFG_TEE_FS_PARENT_PATH ?= /var/lib/tee
+
+ # CFG_TEE_CLIENT_LOG_FILE
+ #   The location of the client log file when logging to file is enabled.
+diff --git a/libteec/CMakeLists.txt b/libteec/CMakeLists.txt
+index c742d31..c857369 100644
+--- a/libteec/CMakeLists.txt
++++ b/libteec/CMakeLists.txt
+@@ -14,7 +14,7 @@ endif()
+ # Configuration flags always included
+ ################################################################################
+ set(CFG_TEE_CLIENT_LOG_LEVEL "1" CACHE STRING "libteec log level")
+-set(CFG_TEE_CLIENT_LOG_FILE "/data/tee/teec.log" CACHE STRING "Location of libteec log")
++set(CFG_TEE_CLIENT_LOG_FILE "${CMAKE_INSTALL_LOCALSTATEDIR}/lib/tee/teec.log" CACHE STRING "Location of libteec log")
+
+ ################################################################################
+ # Source files
+diff --git a/tee-supplicant/CMakeLists.txt b/tee-supplicant/CMakeLists.txt
+index 54a34c7..8df9bef 100644
+--- a/tee-supplicant/CMakeLists.txt
++++ b/tee-supplicant/CMakeLists.txt
+@@ -11,10 +11,15 @@ option(CFG_TEE_SUPP_PLUGINS "Enable tee-supplicant plugin support" ON)
+ set(CFG_TEE_SUPP_LOG_LEVEL "1" CACHE STRING "tee-supplicant log level")
+ # FIXME: Question is, is this really needed? Should just use defaults from # GNUInstallDirs?
+ set(CFG_TEE_CLIENT_LOAD_PATH "/lib" CACHE STRING "Colon-separated list of paths where to look for TAs (see also --ta-dir)")
+-set(CFG_TEE_FS_PARENT_PATH "/data/tee" CACHE STRING "Location of TEE filesystem (secure storage)")
++set(CFG_TEE_FS_PARENT_PATH "${CMAKE_INSTALL_LOCALSTATEDIR}/lib/tee" CACHE STRING "Location of TEE filesystem (secure storage)")
+ # FIXME: Why do we have if defined(CFG_GP_SOCKETS) && CFG_GP_SOCKETS == 1 in the c-file?
+ set(CFG_GP_SOCKETS "1" CACHE STRING "Enable GlobalPlatform Socket API support")
+-set(CFG_TEE_PLUGIN_LOAD_PATH "/usr/lib/tee-supplicant/plugins/" CACHE STRING "tee-supplicant's plugins path")
++set(CFG_TEE_PLUGIN_LOAD_PATH "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${PROJECT_NAME}/plugins/" CACHE STRING "tee-supplicant's plugins path")
++
++set(CFG_TEE_GROUP "tee" CACHE STRING "Group which has access to /dev/tee* devices")
++set(CFG_TEEPRIV_GROUP "teepriv" CACHE STRING "Group which has access to /dev/teepriv* devices")
++set(CFG_TEE_SUPPL_USER "teesuppl" CACHE STRING "User account which tee-supplicant is started with")
++set(CFG_TEE_SUPPL_GROUP "teesuppl" CACHE STRING "Group account which tee-supplicant is started with")
+
+ if(CFG_TEE_SUPP_PLUGINS)
+       set(CMAKE_INSTALL_RPATH "${CFG_TEE_PLUGIN_LOAD_PATH}")
+@@ -113,3 +118,7 @@ endif()
+ # Install targets
+ ################################################################################
+ install(TARGETS ${PROJECT_NAME} RUNTIME DESTINATION ${CMAKE_INSTALL_SBINDIR})
++configure_file(tee-supplicant@.service.in tee-supplicant@.service @ONLY)
++install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/tee-supplicant@.service DESTINATION ${CMAKE_INSTALL_LIBDIR}/systemd/system)
++configure_file(optee-udev.rules.in optee-udev.rules @ONLY)
++install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/optee-udev.rules DESTINATION ${CMAKE_INSTALL_SYSCONFDIR}/udev/rules.d)
+diff --git a/tee-supplicant/optee-udev.rules.in b/tee-supplicant/optee-udev.rules.in
+new file mode 100644
+index 0000000..275e833
+--- /dev/null
++++ b/tee-supplicant/optee-udev.rules.in
+@@ -0,0 +1,7 @@
++# SPDX-License-Identifier: BSD-2-Clause
++KERNEL=="tee[0-9]*", MODE="0660", OWNER="root", GROUP="@CFG_TEE_GROUP@", TAG+="systemd"
++
++# If a /dev/teepriv[0-9]* device is detected, start an instance of
++# tee-supplicant.service with the device name as parameter
++KERNEL=="teepriv[0-9]*", MODE="0660", OWNER="root", GROUP="@CFG_TEEPRIV_GROUP@", \
++    TAG+="systemd", ENV{SYSTEMD_WANTS}+="tee-supplicant@%k.service"
+diff --git a/tee-supplicant/tee-supplicant@.service.in b/tee-supplicant/tee-supplicant@.service.in
+new file mode 100644
+index 0000000..e53a935
+--- /dev/null
++++ b/tee-supplicant/tee-supplicant@.service.in
+@@ -0,0 +1,17 @@
++# SPDX-License-Identifier: BSD-2-Clause
++[Unit]
++Description=TEE Supplicant on %i
++DefaultDependencies=no
++After=dev-%i.device
++Wants=dev-%i.device
++Conflicts=shutdown.target
++Before=tpm2.target sysinit.target shutdown.target
++
++[Service]
++Type=notify
++User=@CFG_TEE_SUPPL_USER@
++Group=@CFG_TEE_SUPPL_GROUP@
++EnvironmentFile=-@CMAKE_INSTALL_SYSCONFDIR@/default/tee-supplicant
++ExecStart=@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_SBINDIR@/tee-supplicant $OPTARGS
++# Workaround for fTPM TA: stop kernel module before tee-supplicant
++ExecStop=-/bin/sh -c "/sbin/modprobe -v -r tpm_ftpm_tee ; /bin/kill $MAINPID"
+--
+2.34.1
+
diff --git a/meta-arm/recipes-security/optee/optee-client/optee-udev.rules b/meta-arm/recipes-security/optee/optee-client/optee-udev.rules
deleted file mode 100644
index 075f469c..00000000
--- a/meta-arm/recipes-security/optee/optee-client/optee-udev.rules
+++ /dev/null
@@ -1,6 +0,0 @@
-KERNEL=="tee[0-9]*", MODE="0660", OWNER="root", GROUP="teeclnt", TAG+="systemd"
-
-# If a /dev/teepriv[0-9]* device is detected, start an instance of
-# tee-supplicant.service with the device name as parameter
-KERNEL=="teepriv[0-9]*", MODE="0660", OWNER="root", GROUP="teeclnt", \
-    TAG+="systemd", ENV{SYSTEMD_WANTS}+="tee-supplicant@%k.service"
diff --git a/meta-arm/recipes-security/optee/optee-client/tee-supplicant@.service b/meta-arm/recipes-security/optee/optee-client/tee-supplicant@.service
deleted file mode 100644
index e3039fde..00000000
--- a/meta-arm/recipes-security/optee/optee-client/tee-supplicant@.service
+++ /dev/null
@@ -1,13 +0,0 @@
-[Unit]
-Description=TEE Supplicant on %i
-DefaultDependencies=no
-After=dev-%i.device
-Wants=dev-%i.device
-Conflicts=shutdown.target
-Before=tpm2.target sysinit.target shutdown.target
-
-[Service]
-Type=notify
-EnvironmentFile=-@sysconfdir@/default/tee-supplicant
-ExecStart=@sbindir@/tee-supplicant $OPTARGS
-ExecStop=-/bin/sh -c "/sbin/modprobe -v -r tpm_ftpm_tee ; /bin/kill $MAINPID"
diff --git a/meta-arm/recipes-security/optee/optee-client_4.3.0.bb b/meta-arm/recipes-security/optee/optee-client_4.3.0.bb
index 4a088004..edab4583 100644
--- a/meta-arm/recipes-security/optee/optee-client_4.3.0.bb
+++ b/meta-arm/recipes-security/optee/optee-client_4.3.0.bb
@@ -2,6 +2,8 @@ require recipes-security/optee/optee-client.inc

 SRCREV = "a5b1ffcd26e328af0bbf18ab448a38ecd558e05c"

+SRC_URI += "file://0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch"
+
 inherit pkgconfig
 DEPENDS += "util-linux"
 EXTRA_OEMAKE += "PKG_CONFIG=pkg-config"
--
2.34.1

Re: [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Mikko Rapeli]

Hi,

On Wed, Oct 23, 2024 at 02:22:30PM +0000, Tom Hochstein (OSS) wrote:
> Hi Mikko,
> 
> We have multilib testing that is failing now because the systemd folder is being installed in `libdir` instead of the non-arch-specific `systemd_system_unitdir = "${nonarch_base_libdir}/systemd/system"` as is codified in `bitbake.conf`.
> 
> ERROR: optee-client-4.4.0.imx-r0 do_package: QA Issue: optee-client: Files/directories were installed but not shipped in any package:
>   /usr/lib64/systemd
>   /usr/lib64/systemd/system
>   /usr/lib64/systemd/system/tee-supplicant@.service
> 
> I'm not sure how this should be fixed, as it doesn't appear that CMAKE can handle more than one libdir. Can the install of the service file be hard-coded to use /usr/lib? Or does this need to be fixed in the recipe?

As mentioned in https://github.com/OP-TEE/optee_client/issues/393
your use case is "sysvinit" without "usrmerge" in distro features.
"sysvinit" conflicts with "systemd" init.

These recipe changes only install the systemd service file if "systemd" is
enabled in DISTRO_FEATURES. Your usecase should be covered by that.

Cheers,

-Mikko

Re: [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Mikko Rapeli]

Hi,

On Wed, Oct 23, 2024 at 02:34:15PM +0000, Tom Hochstein wrote:
> Sorry I wasn't clear. The new build break is for when we do use systemd. We use sysvinit for QorIQ, but systemd for i.MX.

Ok got it now.

${nonarch_base_libdir} isn't in the CMake toolchain file. We could
move ${libdir}/systemd unconditionlly to ${nonarch_base_libdir}/systemd
in the do_install task. Could you test and submit a patch like below?

--- a/meta-arm/recipes-security/optee/optee-client.inc
+++ b/meta-arm/recipes-security/optee/optee-client.inc
@@ -23,8 +23,9 @@ EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"
 
 do_install:append() {
     # installed by default
+    mv ${D}${libdir}/systemd ${D}${noarch_base_libdir}/systemd
     if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
-        rm -rf ${D}${libdir}/systemd
+        rm -rf ${D}${noarch_base_libdir}/systemd
     fi
     if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
         install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant

Cheers,

-Mikko

Re: [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Tom Hochstein]

On 10/23/2024 9:54 AM, Mikko Rapeli wrote:
> Ok got it now.
> 
> ${nonarch_base_libdir} isn't in the CMake toolchain file. We could
> move ${libdir}/systemd unconditionlly to ${nonarch_base_libdir}/systemd
> in the do_install task. Could you test and submit a patch like below?
> 
> --- a/meta-arm/recipes-security/optee/optee-client.inc
> +++ b/meta-arm/recipes-security/optee/optee-client.inc
> @@ -23,8 +23,9 @@ EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"
>   
>   do_install:append() {
>       # installed by default
> +    mv ${D}${libdir}/systemd ${D}${noarch_base_libdir}/systemd
>       if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
> -        rm -rf ${D}${libdir}/systemd
> +        rm -rf ${D}${noarch_base_libdir}/systemd
>       fi
>       if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
>           install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
> 
> Cheers,
> 
> -Mikko

This works fine in our testing. Sorry for the late response.

Thanks again!

Tom

Re: [meta-arm] [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Gyorgy Szing]

[-- Attachment #1: Type: text/plain, Size: 2947 bytes --]

Hi,

(Sorry for the delayed reply.)

Another way to address this would be to use PkgConfig from CMakeLists.txt (see [1]). This might be considered a nicer solution as the yocto recipe would not have to correct the install location, and thus there would be a single source of truth. (Kudos for Ross for the suggestion.)
Something like (code is not tested):

Include(FindPkgConfig)
If (PKG_CONFIG_FOUND)
                # TODO: there are other variables. Doublecheck if systemdsystemconfdir is the right one to be used. On X86 Ubuntu this returns /etc/systemd/system. Yocto might define distro specific values.
                pkg_get_variable(UNIT_DIR systemd systemdsystemconfdir)
                # TODO: check the value to see if pkgconfig successfully found the value. Behavior is not documented, probably if (UNIT_DIR) would work.
                # TODO: UNIT_DIR is a string list, use only the first or last value? Rewrite the line below accordingly
set(CMAKE_INSTALL_SYSTEMDSYSCONFDIR “${ UNIT_DIR }” CACHE PATH “Target directory for system config files”)
endif()
# By default, use LIBDIR
# TODO: CMAKE_INSTALL_SYSCONFDIR might be a better default base dir.
set(CMAKE_INSTALL_ SYSTEMDSYSCONFDIR “${ CMAKE_INSTALL_LIBDIR}}/systemd/system” CACHE PATH “Target directory for system config files”)

install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/tee-supplicant@.service DESTINATION ${ CMAKE_INSTALL_SYSTEMDSYSCONFDIR})


The above code would default to the current install location under LIBDIR,  and would use PkgConfig if available. Setting CMAKE_INSTALL_ SYSTEMDSYSCONFDIR from the command line allows manual override.

1: https://cmake.org/cmake/help/latest/module/FindPkgConfig.html

/George

On 2024-10-29, 18:03, "meta-arm@lists.yoctoproject.org" <meta-arm@lists.yoctoproject.org> wrote:

On 10/23/2024 9:54 AM, Mikko Rapeli wrote:
> Ok got it now.
>
> ${nonarch_base_libdir} isn't in the CMake toolchain file. We could
> move ${libdir}/systemd unconditionlly to ${nonarch_base_libdir}/systemd
> in the do_install task. Could you test and submit a patch like below?
>
> --- a/meta-arm/recipes-security/optee/optee-client.inc
> +++ b/meta-arm/recipes-security/optee/optee-client.inc
> @@ -23,8 +23,9 @@ EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"
>
>   do_install:append() {
>       # installed by default
> +    mv ${D}${libdir}/systemd ${D}${noarch_base_libdir}/systemd
>       if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
> -        rm -rf ${D}${libdir}/systemd
> +        rm -rf ${D}${noarch_base_libdir}/systemd
>       fi
>       if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
>           install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
>
> Cheers,
>
> -Mikko

This works fine in our testing. Sorry for the late response.

Thanks again!

Tom

[-- Attachment #2: Type: text/html, Size: 10883 bytes --]

Re: [meta-arm] [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Mikko Rapeli]

Hi,

On Wed, Oct 30, 2024 at 08:02:19AM +0000, Gyorgy Szing wrote:
> Hi,
> 
> (Sorry for the delayed reply.)
> 
> Another way to address this would be to use PkgConfig from CMakeLists.txt (see [1]). This might be considered a nicer solution as the yocto recipe would not have to correct the install location, and thus there would be a single source of truth. (Kudos for Ross for the suggestion.)
> Something like (code is not tested):
> 
> Include(FindPkgConfig)
> If (PKG_CONFIG_FOUND)
>                 # TODO: there are other variables. Doublecheck if systemdsystemconfdir is the right one to be used. On X86 Ubuntu this returns /etc/systemd/system. Yocto might define distro specific values.
>                 pkg_get_variable(UNIT_DIR systemd systemdsystemconfdir)
>                 # TODO: check the value to see if pkgconfig successfully found the value. Behavior is not documented, probably if (UNIT_DIR) would work.
>                 # TODO: UNIT_DIR is a string list, use only the first or last value? Rewrite the line below accordingly
> set(CMAKE_INSTALL_SYSTEMDSYSCONFDIR “${ UNIT_DIR }” CACHE PATH “Target directory for system config files”)
> endif()
> # By default, use LIBDIR
> # TODO: CMAKE_INSTALL_SYSCONFDIR might be a better default base dir.
> set(CMAKE_INSTALL_ SYSTEMDSYSCONFDIR “${ CMAKE_INSTALL_LIBDIR}}/systemd/system” CACHE PATH “Target directory for system config files”)
> 
> install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/tee-supplicant@.service DESTINATION ${ CMAKE_INSTALL_SYSTEMDSYSCONFDIR})
> 
> 
> The above code would default to the current install location under LIBDIR,  and would use PkgConfig if available. Setting CMAKE_INSTALL_ SYSTEMDSYSCONFDIR from the command line allows manual override.
> 
> 1: https://cmake.org/cmake/help/latest/module/FindPkgConfig.html

This would only work if optee-client has "systemd" build time dependency in DEPENDS.
I don't think we should add this dependency. The sd_notify() support for example
dlopen()'s systemd libs at runtime to avoid making systemd a hard build time
dependency. I don't think we should add this dependency due to systemd
service file install paths.

Cheers,

-Mikko

> /George
> 
> On 2024-10-29, 18:03, "meta-arm@lists.yoctoproject.org" <meta-arm@lists.yoctoproject.org> wrote:
> 
> On 10/23/2024 9:54 AM, Mikko Rapeli wrote:
> > Ok got it now.
> >
> > ${nonarch_base_libdir} isn't in the CMake toolchain file. We could
> > move ${libdir}/systemd unconditionlly to ${nonarch_base_libdir}/systemd
> > in the do_install task. Could you test and submit a patch like below?
> >
> > --- a/meta-arm/recipes-security/optee/optee-client.inc
> > +++ b/meta-arm/recipes-security/optee/optee-client.inc
> > @@ -23,8 +23,9 @@ EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"
> >
> >   do_install:append() {
> >       # installed by default
> > +    mv ${D}${libdir}/systemd ${D}${noarch_base_libdir}/systemd
> >       if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
> > -        rm -rf ${D}${libdir}/systemd
> > +        rm -rf ${D}${noarch_base_libdir}/systemd
> >       fi
> >       if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
> >           install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
> >
> > Cheers,
> >
> > -Mikko
> 
> This works fine in our testing. Sorry for the late response.
> 
> Thanks again!
> 
> Tom

Re: [meta-arm] [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Gyorgy Szing]

[-- Attachment #1.1: Type: text/plain, Size: 6355 bytes --]

> Hi,
>
> On Wed, Oct 30, 2024 at 08:02:19AM +0000, Gyorgy Szing wrote:
> > Hi,
> >
> > (Sorry for the delayed reply.)
> >
> > Another way to address this would be to use PkgConfig from CMakeLists.txt (see [1]). This might be considered a nicer solution as the yocto recipe would not have to correct the install location, and thus there would be a single source of truth. (Kudos for Ross for the suggestion.)
> > Something like (code is not tested):
> >
> > Include(FindPkgConfig)
> > If (PKG_CONFIG_FOUND)
> >                 # TODO: there are other variables. Doublecheck if systemdsystemconfdir is the right one to be used. On X86 Ubuntu this returns /etc/systemd/system. Yocto might define distro specific values.
> >                 pkg_get_variable(UNIT_DIR systemd systemdsystemconfdir)
> >                 # TODO: check the value to see if pkgconfig successfully found the value. Behavior is not documented, probably if (UNIT_DIR) would work.
> >                 # TODO: UNIT_DIR is a string list, use only the first or last value? Rewrite the line below accordingly
> > set(CMAKE_INSTALL_SYSTEMDSYSCONFDIR “${ UNIT_DIR }” CACHE PATH “Target directory for system config files”)
> > endif()
> > # By default, use LIBDIR
> > # TODO: CMAKE_INSTALL_SYSCONFDIR might be a better default base dir.
> > set(CMAKE_INSTALL_ SYSTEMDSYSCONFDIR “${ CMAKE_INSTALL_LIBDIR}}/systemd/system” CACHE PATH “Target directory for system config files”)
> >
> > install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/tee-supplicant@.service DESTINATION ${ CMAKE_INSTALL_SYSTEMDSYSCONFDIR})
> >
> >
> > The above code would default to the current install location under LIBDIR,  and would use PkgConfig if available. Setting CMAKE_INSTALL_ SYSTEMDSYSCONFDIR from the command line allows manual override.
> >
> > 1: https://cmake.org/cmake/help/latest/module/FindPkgConfig.html
>
> This would only work if optee-client has "systemd" build time dependency in DEPENDS.
> I don't think we should add this dependency. The sd_notify() support for example
> dlopen()'s systemd libs at runtime to avoid making systemd a hard build time
> dependency. I don't think we should add this dependency due to systemd
> service file install paths.

DISTRO_FEATURES seem to reflect if systemd is deployed and this allows the recipe to behave accordingly. Not sure if this is the case for all distros, but poky and meta-arm already depend on DISTRO_FEATURES based systemd configuration and thus might be an acceptable risk.
AFAIK systemd was added part of uefi-secure boot enablement. If this feature is enabled the native sysroot of the optee-client recipe has systemd already because of some uefi-secureboot specific transitive dependency. Might not be bulletproof but if systemd is needed, things seem to be ok as is. On the other hand, adding systemd conditionally to DEPENDS might not make things much worse since it is included already.

Please check the attached patch file. I cherry-picked you commit “optee-client: use udev rule and systemd service from upstream“ to my topic branch and added the attached patch on top (among other patches which are hopefully have no effect on optee-client).
I did some manual testing by hacking DISTRO_FEATURES and running bitbake-getvar, plus testing fvp_base and quemuarm64-secureboot configurations. It’s not 100% tested as e.g. I did not check how the cmake modification behaves if pkg-config is enabled, but the executable is not available or is available but either system or udev package is missing. The cmake code is there to handle these cases though. Anyways these scenarios seem to be out of scope for yocto.

I added a dependency on udev to the optee-client recipe, to make udevs pkg config data available. This might be considered similar “unwanted bloat” you mentioned related to systemd. I tried to make this conditional but could not find a safe way. According to [1] there is a variable called USE_DEVFS which tells if a device manager is available, but the variable is not visible to the recipe. AFAIK the image class would need to be inherited to get access to it, but that does not seem to be a good idea.

The op-tee client patch:
  - adds build options to control if the system service file shall be installed or not. The default value is on, to keep backwards compatibility.
  - another option is added to enable using pkg-config. This is by default off, and the op-tee client recipe enables it for yocto. If enabled both system and/or udev specific directories are discovered with pkg-config.
  - the intention was to implement option and pkg-config processing in a flexible way. There are hard-coded default values which keep existing behavior and will be used if pkg-config is disabled or failing.

Note: I missed the systemd dependency issue and conditional appending to DEPENDS is not implemented.

1: https://docs.yoctoproject.org/dev/dev-manual/device-manager.html

/George

> Cheers,
>
> -Mikko
>
> > /George
> >
> > On 2024-10-29, 18:03, "meta-arm@lists.yoctoproject.org" <meta-arm@lists.yoctoproject.org> wrote:
> >
> > On 10/23/2024 9:54 AM, Mikko Rapeli wrote:
> > > Ok got it now.
> > >
> > > ${nonarch_base_libdir} isn't in the CMake toolchain file. We could
> > > move ${libdir}/systemd unconditionlly to ${nonarch_base_libdir}/systemd
> > > in the do_install task. Could you test and submit a patch like below?
> > >
> > > --- a/meta-arm/recipes-security/optee/optee-client.inc
> > > +++ b/meta-arm/recipes-security/optee/optee-client.inc
> > > @@ -23,8 +23,9 @@ EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"
> > >
> > >   do_install:append() {
> > >       # installed by default
> > > +    mv ${D}${libdir}/systemd ${D}${noarch_base_libdir}/systemd
> > >       if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
> > > -        rm -rf ${D}${libdir}/systemd
> > > +        rm -rf ${D}${noarch_base_libdir}/systemd
> > >       fi
> > >       if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
> > >           install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
> > >
> > > Cheers,
> > >
> > > -Mikko
> >
> > This works fine in our testing. Sorry for the late response.
> >
> > Thanks again!
> >
> > Tom
>


[-- Attachment #1.2: Type: text/html, Size: 16122 bytes --]

[-- Attachment #2: 0001-optee-client-fix-udev-and-systemd-handling.patch --]
[-- Type: application/octet-stream, Size: 7267 bytes --]

From 9509a6e37f17db10d131cf4c333487106d6f9b8d Mon Sep 17 00:00:00 2001
From: Gyorgy Szing <gyorgy.szing@arm.com>
Date: Mon, 28 Oct 2024 23:48:21 +0100
Subject: [PATCH 1/1] optee-client: fix udev and systemd handling

Eliminate the systemd specific install content fix-up commands appended
to do_install.
For this patch tee-suplicant to use pkg-config for discovering udev and
systemd specific install locations.

Signed-off-by: Gyorgy Szing <gyorgy.szing@arm.com>
---
 .../recipes-security/optee/optee-client.inc   | 11 +--
 ...pdate-udev-rule-and-systemd-service-.patch | 71 +++++++++++++++++++
 .../optee/optee-client_4.3.0.bb               |  3 +-
 .../optee/optee-client_4.4.0.bb               |  1 +
 4 files changed, 81 insertions(+), 5 deletions(-)
 create mode 100644 meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-update-udev-rule-and-systemd-service-.patch

diff --git a/meta-arm/recipes-security/optee/optee-client.inc b/meta-arm/recipes-security/optee/optee-client.inc
index fc48c302..0f8665ad 100644
--- a/meta-arm/recipes-security/optee/optee-client.inc
+++ b/meta-arm/recipes-security/optee/optee-client.inc
@@ -16,16 +16,19 @@ UPSTREAM_CHECK_GITTAGREGEX = "^(?P<pver>\d+(\.\d+)+)$"
 
 S = "${WORKDIR}/git"
 
+DEPENDS += "udev"
+
 EXTRA_OECMAKE = " \
     -DBUILD_SHARED_LIBS=ON \
+    -DCFG_USE_PKGCONFIG=On \
 "
+
+EXTRA_OECMAKE += "${@bb.utils.contains('DISTRO_FEATURES', 'systemd', \
+                                        '-DCFG_ENABLE_SYSTEMD=On', ' -DCFG_ENABLE_SYSTEMD=Off' , d)}"
+
 EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"
 
 do_install:append() {
-    # installed by default
-    if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
-        rm -rf ${D}${libdir}/systemd
-    fi
     if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
         install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
         sed -i -e s:@sysconfdir@:${sysconfdir}:g \
diff --git a/meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-update-udev-rule-and-systemd-service-.patch b/meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-update-udev-rule-and-systemd-service-.patch
new file mode 100644
index 00000000..9e8099bf
--- /dev/null
+++ b/meta-arm/recipes-security/optee/optee-client/0001-tee-supplicant-update-udev-rule-and-systemd-service-.patch
@@ -0,0 +1,71 @@
+From ed7d18ae174f648db5ccdc20f07aa86ebf1410ce Mon Sep 17 00:00:00 2001
+From: Gyorgy Szing <gyorgy.szing@arm.com>
+Date: Thu, 14 Nov 2024 23:08:35 +0100
+Subject: [PATCH 1/1] tee-supplicant: update udev rule and systemd service file
+ handling
+
+Allow optionally using pkg-config to discover install location of
+systemd service and udev rule files.
+Make systemd service file generation and installation optional.
+
+Upstream-Status: Pending
+
+Signed-off-by: Gyorgy Szing <gyorgy.szing@arm.com>
+---
+ tee-supplicant/CMakeLists.txt | 34 +++++++++++++++++++++++++++++++---
+ 1 file changed, 31 insertions(+), 3 deletions(-)
+
+diff --git a/tee-supplicant/CMakeLists.txt b/tee-supplicant/CMakeLists.txt
+index 8df9bef..a877171 100644
+--- a/tee-supplicant/CMakeLists.txt
++++ b/tee-supplicant/CMakeLists.txt
+@@ -7,6 +7,8 @@ option(RPMB_EMU "Enable tee-supplicant to emulate RPMB" ON)
+ option(CFG_TA_GPROF_SUPPORT "Enable tee-supplicant support for TAs instrumented with gprof" ON)
+ option(CFG_FTRACE_SUPPORT "Enable tee-supplicant support for TAs instrumented with ftrace" ON)
+ option(CFG_TEE_SUPP_PLUGINS "Enable tee-supplicant plugin support" ON)
++option(CFG_ENABLE_SYSTEMD "Enable systemd service unit file generation." ON)
++option(CFG_USE_PKGCONFIG "Use pkg-config for discovering install target directory for systemd and udev files." OFF)
+ 
+ set(CFG_TEE_SUPP_LOG_LEVEL "1" CACHE STRING "tee-supplicant log level")
+ # FIXME: Question is, is this really needed? Should just use defaults from # GNUInstallDirs?
+@@ -117,8 +119,34 @@ endif()
+ ################################################################################
+ # Install targets
+ ################################################################################
++# Discover target install location of the systemd and udev files using pkg-config
++if (CFG_USE_PKGCONFIG)
++	# Note: pkg-config should return setting valid for the target platform and not the host.
++	include(FindPkgConfig)
++	if (PKG_CONFIG_FOUND)
++		pkg_search_module(SYSTEMD systemd)
++		if (SYSTEMD_FOUND AND CFG_ENABLE_SYSTEMD)
++			pkg_get_variable(UNIT_DIR systemd systemd_system_unit_dir)
++			set(SYSTEMD_UNIT_DIR "${UNIT_DIR}" CACHE PATH "Location of systemd unit files.")
++			unset(UNIT_DIR)
++		endif()
++		pkg_search_module(UDEV udev)
++		if (UDEV_FOUND)
++			pkg_get_variable(UDEV_DIR udev udev_dir)
++			set(UDEV_UDEV_DIR "${UDEV_DIR}" CACHE PATH "Location of udev files.")
++			unset(UDEV_DIR)
++		endif()
++	endif()
++endif()
++
++# Some sane defaults is discovering through pkgconfig fails or is disabled.
++set(SYSTEMD_UNIT_DIR "${CMAKE_INSTALL_LIBDIR}/systemd/system" CACHE PATH "Location of systemd unit files.")
++set(UDEV_UDEV_DIR "${CMAKE_INSTALL_SYSCONFDIR}/udev" CACHE PATH "Location of udev files.")
++
+ install(TARGETS ${PROJECT_NAME} RUNTIME DESTINATION ${CMAKE_INSTALL_SBINDIR})
+-configure_file(tee-supplicant@.service.in tee-supplicant@.service @ONLY)
+-install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/tee-supplicant@.service DESTINATION ${CMAKE_INSTALL_LIBDIR}/systemd/system)
++if (CFG_ENABLE_SYSTEMD)
++	configure_file(tee-supplicant@.service.in tee-supplicant@.service @ONLY)
++	install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/tee-supplicant@.service DESTINATION ${SYSTEMD_UNIT_DIR})
++endif()
+ configure_file(optee-udev.rules.in optee-udev.rules @ONLY)
+-install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/optee-udev.rules DESTINATION ${CMAKE_INSTALL_SYSCONFDIR}/udev/rules.d)
++install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/optee-udev.rules DESTINATION ${UDEV_UDEV_DIR}/rules.d)
+-- 
+2.43.0
+
diff --git a/meta-arm/recipes-security/optee/optee-client_4.3.0.bb b/meta-arm/recipes-security/optee/optee-client_4.3.0.bb
index edab4583..9dc301f4 100644
--- a/meta-arm/recipes-security/optee/optee-client_4.3.0.bb
+++ b/meta-arm/recipes-security/optee/optee-client_4.3.0.bb
@@ -2,7 +2,8 @@ require recipes-security/optee/optee-client.inc
 
 SRCREV = "a5b1ffcd26e328af0bbf18ab448a38ecd558e05c"
 
-SRC_URI += "file://0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch"
+SRC_URI += "file://0001-tee-supplicant-add-udev-rule-and-systemd-service-fil.patch \
+            file://0001-tee-supplicant-update-udev-rule-and-systemd-service-.patch"
 
 inherit pkgconfig
 DEPENDS += "util-linux"
diff --git a/meta-arm/recipes-security/optee/optee-client_4.4.0.bb b/meta-arm/recipes-security/optee/optee-client_4.4.0.bb
index fa7b9564..fa649143 100644
--- a/meta-arm/recipes-security/optee/optee-client_4.4.0.bb
+++ b/meta-arm/recipes-security/optee/optee-client_4.4.0.bb
@@ -2,6 +2,7 @@ require recipes-security/optee/optee-client.inc
 
 # v4.4.0
 SRCREV = "d221676a58b305bddbf97db00395205b3038de8e"
+SRC_URI += "file://0001-tee-supplicant-update-udev-rule-and-systemd-service-.patch"
 
 inherit pkgconfig
 DEPENDS += "util-linux"
-- 
2.43.0

Re: [meta-arm] [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Mikko Rapeli]

Hi,

On Tue, Nov 26, 2024 at 09:05:15PM +0000, Gyorgy Szing wrote:
> > Hi,
> >
> > On Wed, Oct 30, 2024 at 08:02:19AM +0000, Gyorgy Szing wrote:
> > > Hi,
> > >
> > > (Sorry for the delayed reply.)
> > >
> > > Another way to address this would be to use PkgConfig from CMakeLists.txt (see [1]). This might be considered a nicer solution as the yocto recipe would not have to correct the install location, and thus there would be a single source of truth. (Kudos for Ross for the suggestion.)
> > > Something like (code is not tested):
> > >
> > > Include(FindPkgConfig)
> > > If (PKG_CONFIG_FOUND)
> > >                 # TODO: there are other variables. Doublecheck if systemdsystemconfdir is the right one to be used. On X86 Ubuntu this returns /etc/systemd/system. Yocto might define distro specific values.
> > >                 pkg_get_variable(UNIT_DIR systemd systemdsystemconfdir)
> > >                 # TODO: check the value to see if pkgconfig successfully found the value. Behavior is not documented, probably if (UNIT_DIR) would work.
> > >                 # TODO: UNIT_DIR is a string list, use only the first or last value? Rewrite the line below accordingly
> > > set(CMAKE_INSTALL_SYSTEMDSYSCONFDIR “${ UNIT_DIR }” CACHE PATH “Target directory for system config files”)
> > > endif()
> > > # By default, use LIBDIR
> > > # TODO: CMAKE_INSTALL_SYSCONFDIR might be a better default base dir.
> > > set(CMAKE_INSTALL_ SYSTEMDSYSCONFDIR “${ CMAKE_INSTALL_LIBDIR}}/systemd/system” CACHE PATH “Target directory for system config files”)
> > >
> > > install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/tee-supplicant@.service DESTINATION ${ CMAKE_INSTALL_SYSTEMDSYSCONFDIR})
> > >
> > >
> > > The above code would default to the current install location under LIBDIR,  and would use PkgConfig if available. Setting CMAKE_INSTALL_ SYSTEMDSYSCONFDIR from the command line allows manual override.
> > >
> > > 1: https://cmake.org/cmake/help/latest/module/FindPkgConfig.html
> >
> > This would only work if optee-client has "systemd" build time dependency in DEPENDS.
> > I don't think we should add this dependency. The sd_notify() support for example
> > dlopen()'s systemd libs at runtime to avoid making systemd a hard build time
> > dependency. I don't think we should add this dependency due to systemd
> > service file install paths.
> 
> DISTRO_FEATURES seem to reflect if systemd is deployed and this allows the recipe to behave accordingly. Not sure if this is the case for all distros, but poky and meta-arm already depend on DISTRO_FEATURES based systemd configuration and thus might be an acceptable risk.
> AFAIK systemd was added part of uefi-secure boot enablement. If this feature is enabled the native sysroot of the optee-client recipe has systemd already because of some uefi-secureboot specific transitive dependency. Might not be bulletproof but if systemd is needed, things seem to be ok as is. On the other hand, adding systemd conditionally to DEPENDS might not make things much worse since it is included already.

Even when "systemd" is the selected init and in DISTRO_FEATURES,
it is not a build dependency and in DEPENDS of optee-client and thus
systemd pkg-config files are not available.

Yocto recipes and open source SW which ships systemd service files
and even notify systemd at startup using sd_notify() are not meant
to have systemd in their build dependencies. This makes too
many things depend on systemd which results in longer build
times.

Thus I don't want to add systemd to DEPENDS of optee-client.
 
> Please check the attached patch file. I cherry-picked you commit “optee-client: use udev rule and systemd service from upstream“ to my topic branch and added the attached patch on top (among other patches which are hopefully have no effect on optee-client).
> I did some manual testing by hacking DISTRO_FEATURES and running bitbake-getvar, plus testing fvp_base and quemuarm64-secureboot configurations. It’s not 100% tested as e.g. I did not check how the cmake modification behaves if pkg-config is enabled, but the executable is not available or is available but either system or udev package is missing. The cmake code is there to handle these cases though. Anyways these scenarios seem to be out of scope for yocto.

I think this patch is complex enough that it needs upstreaming
to optee-client. I don't agree with using pkg-config settings
from systemd to figure out systemd service file install paths
because that requires adding systemd to build dependencies.

> I added a dependency on udev to the optee-client recipe, to make udevs pkg config data available. This might be considered similar “unwanted bloat” you mentioned related to systemd. I tried to make this conditional but could not find a safe way. According to [1] there is a variable called USE_DEVFS which tells if a device manager is available, but the variable is not visible to the recipe. AFAIK the image class would need to be inherited to get access to it, but that does not seem to be a good idea.

As with systemd, I don't think udev should be in build time
dependencies of recipes which just ship udev rules.
In the same way, udev is not in the build dependencies by default
and I don't think it's a good idea to add it there.

> The op-tee client patch:
>   - adds build options to control if the system service file shall be installed or not. The default value is on, to keep backwards compatibility.
>   - another option is added to enable using pkg-config. This is by default off, and the op-tee client recipe enables it for yocto. If enabled both system and/or udev specific directories are discovered with pkg-config.
>   - the intention was to implement option and pkg-config processing in a flexible way. There are hard-coded default values which keep existing behavior and will be used if pkg-config is disabled or failing.
> 
> Note: I missed the systemd dependency issue and conditional appending to DEPENDS is not implemented.
> 
> 1: https://docs.yoctoproject.org/dev/dev-manual/device-manager.html

Since we disagree on the solution, I'll stop proposing these changes.

It's up to maintainer of optee-client recipe, Jon and Ross,
to decide what to do with newer optee-client releases which do install
a default udev rule and systemd service which use different users and
groups than the meta-arm side optee-client recipe.

I don't think shipping two optee udev rules is good either and thus would
have kept that rule only in optee-client, possibly a separate binary
package.

Cheers,

-Mikko

> /George
> 
> > Cheers,
> >
> > -Mikko
> >
> > > /George
> > >
> > > On 2024-10-29, 18:03, "meta-arm@lists.yoctoproject.org" <meta-arm@lists.yoctoproject.org> wrote:
> > >
> > > On 10/23/2024 9:54 AM, Mikko Rapeli wrote:
> > > > Ok got it now.
> > > >
> > > > ${nonarch_base_libdir} isn't in the CMake toolchain file. We could
> > > > move ${libdir}/systemd unconditionlly to ${nonarch_base_libdir}/systemd
> > > > in the do_install task. Could you test and submit a patch like below?
> > > >
> > > > --- a/meta-arm/recipes-security/optee/optee-client.inc
> > > > +++ b/meta-arm/recipes-security/optee/optee-client.inc
> > > > @@ -23,8 +23,9 @@ EXTRA_OECMAKE:append:toolchain-clang = " -DCFG_WERROR=0"
> > > >
> > > >   do_install:append() {
> > > >       # installed by default
> > > > +    mv ${D}${libdir}/systemd ${D}${noarch_base_libdir}/systemd
> > > >       if ! ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
> > > > -        rm -rf ${D}${libdir}/systemd
> > > > +        rm -rf ${D}${noarch_base_libdir}/systemd
> > > >       fi
> > > >       if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
> > > >           install -D -p -m0755 ${UNPACKDIR}/tee-supplicant.sh ${D}${sysconfdir}/init.d/tee-supplicant
> > > >
> > > > Cheers,
> > > >
> > > > -Mikko
> > >
> > > This works fine in our testing. Sorry for the late response.
> > >
> > > Thanks again!
> > >
> > > Tom
> >
> 

Re: [meta-arm] [PATCH v3 1/2] optee-client: use udev rule and systemd service from upstream

[Ross Burton]

On 26 Nov 2024, at 14:55, Mikko Rapeli via lists.yoctoproject.org <mikko.rapeli=linaro.org@lists.yoctoproject.org> wrote:
> This would only work if optee-client has "systemd" build time dependency in DEPENDS.
> I don't think we should add this dependency. The sd_notify() support for example
> dlopen()'s systemd libs at runtime to avoid making systemd a hard build time
> dependency. I don't think we should add this dependency due to systemd
> service file install paths.

If this was all my code, I’d be flexible: add a cmake option so the caller can do -DSYSTEMD_UNIT_DIR=…, but if that isn’t set then ask pkg-config for the systemdsystemconfdir.

Then in our recipes as we know that path is fixed, we can simply pass -DSYSTEMD_UNIT_DIR=${systemd_system_unitdir} when configuring and we don’t need to add a DEPENDS on systemd.   You can always pass that option to cmake, and inheriting the systemd class will result in the units being deleted if systemd isn’t actually enabled in DISTRO_FEATURES.

Ross