diff mbox series

arm-bsp/corstone1000: Remove inappropriate kernel delay patch

Message ID 20231109154827.4103-1-mariam.elshakfy@arm.com
State New
Headers show
Series arm-bsp/corstone1000: Remove inappropriate kernel delay patch | expand

Commit Message

Mariam Elshakfy Nov. 9, 2023, 3:48 p.m. UTC 
From: Mariam Elshakfy <mariam.elshakfy@arm.com>

When booting, the tee driver from kernel side
invokes a yielding call to OP-TEE, which gets
stuck because OP-TEE never sends Done response:
OPTEE_FFA_YIELDING_CALL_RETURN_DONE
This issue was previously resolved by introducing
an inappropriate patch to the kernel with 1 ms delay
in ffa_msg_send_direct_req.
Further investigation proved that OP-TEE doesn't
get enough processing time and is constantly interrupted
by the kernel requests. To remove this patch, TF-A logging
level is lowered to default (40 in debug builds and
20 in release builds), which eliminates the time consumed
previously by TF-A VERBOSE logs (giving OP-TEE more
processing time).

Signed-off-by: Mariam Elshakfy <mariam.elshakfy@arm.com>
---
 .../trusted-firmware-a-corstone1000.inc       | 11 +++++-
 ...dds-workaround-for-cs1k-specific-bug.patch | 37 -------------------
 .../linux/linux-arm-platforms.inc             |  1 -
 3 files changed, 10 insertions(+), 39 deletions(-)
 delete mode 100644 meta-arm-bsp/recipes-kernel/linux/files/corstone1000/0002-Adds-workaround-for-cs1k-specific-bug.patch

Comments

Jon Mason Nov. 10, 2023, 3:03 p.m. UTC | #1 
On Thu, 09 Nov 2023 15:48:27 +0000, mariam.elshakfy@arm.com wrote:
> When booting, the tee driver from kernel side
> invokes a yielding call to OP-TEE, which gets
> stuck because OP-TEE never sends Done response:
> OPTEE_FFA_YIELDING_CALL_RETURN_DONE
> This issue was previously resolved by introducing
> an inappropriate patch to the kernel with 1 ms delay
> in ffa_msg_send_direct_req.
> Further investigation proved that OP-TEE doesn't
> get enough processing time and is constantly interrupted
> by the kernel requests. To remove this patch, TF-A logging
> level is lowered to default (40 in debug builds and
> 20 in release builds), which eliminates the time consumed
> previously by TF-A VERBOSE logs (giving OP-TEE more
> processing time).
> 
> [...]

Applied, thanks!

[1/1] arm-bsp/corstone1000: Remove inappropriate kernel delay patch
      commit: eb49bb6ea2315d28d268a15416829f65640ed7a6

Best regards,
diff mbox series

Patch

diff --git a/meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a-corstone1000.inc b/meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a-corstone1000.inc
index ee071371..8673199d 100644
--- a/meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a-corstone1000.inc
+++ b/meta-arm-bsp/recipes-bsp/trusted-firmware-a/trusted-firmware-a-corstone1000.inc
@@ -24,6 +24,16 @@  TFA_SPMD_SPM_AT_SEL2 = "0"
 # BL2 loads BL32 (optee). So, optee needs to be built first:
 DEPENDS += "optee-os"
 
+# Note: Regarding the build option: LOG_LEVEL. 
+# There seems to be an issue when setting it
+# to 50 (LOG_LEVEL_VERBOSE), where the kernel 
+# tee driver sends yielding requests to OP-TEE
+# at a faster pace than OP-TEE processes them,
+# as the processing time is consumed by logging 
+# in TF-A. When this issue occurs, booting halts 
+# as soon as optee driver starts initialization.
+# Therefore, it's not currently recommended to
+# set LOG_LEVEL to 50 at all.
 EXTRA_OEMAKE:append = " \
                         ARCH=aarch64 \
                         TARGET_PLATFORM=${TFA_TARGET_PLATFORM} \
@@ -41,5 +51,4 @@  EXTRA_OEMAKE:append = " \
                         ERRATA_A35_855472=1 \
                         ROT_KEY=plat/arm/board/common/rotpk/arm_rotprivk_rsa.pem \
                         BL32=${RECIPE_SYSROOT}/${nonarch_base_libdir}/firmware/tee-pager_v2.bin \
-                        LOG_LEVEL=50 \
                         "
diff --git a/meta-arm-bsp/recipes-kernel/linux/files/corstone1000/0002-Adds-workaround-for-cs1k-specific-bug.patch b/meta-arm-bsp/recipes-kernel/linux/files/corstone1000/0002-Adds-workaround-for-cs1k-specific-bug.patch
deleted file mode 100644
index 4fbeb23e..00000000
--- a/meta-arm-bsp/recipes-kernel/linux/files/corstone1000/0002-Adds-workaround-for-cs1k-specific-bug.patch
+++ /dev/null
@@ -1,37 +0,0 @@ 
-From 555ac46f6f5157741a6fd8f21f74beb1340ed941 Mon Sep 17 00:00:00 2001
-From: Emekcan <emekcan.aras@arm.com>
-Date: Thu, 13 Oct 2022 20:53:42 +0100
-Subject: [PATCH] Adds workaround for cs1k specific bug
-
-Adds a temporary workaround to solve a possible
-race-conditioning issue in the tee driver
-for corstone1000.
-
-Upstream-Status: Inappropriate
-Signed-off-by: Emekcan Aras <emekcan.aras@arm.com>
-Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
----
- drivers/firmware/arm_ffa/driver.c | 3 ++-
- 1 file changed, 2 insertions(+), 1 deletion(-)
-
-diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
-index 2109cd178ff7..c15b3a83c720 100644
---- a/drivers/firmware/arm_ffa/driver.c
-+++ b/drivers/firmware/arm_ffa/driver.c
-@@ -32,6 +32,7 @@
- #include <linux/scatterlist.h>
- #include <linux/slab.h>
- #include <linux/uuid.h>
-+#include <linux/delay.h>
- 
- #include "common.h"
- 
-@@ -282,7 +283,7 @@ static int ffa_msg_send_direct_req(u16 src_id, u16 dst_id, bool mode_32bit,
- {
- 	u32 req_id, resp_id, src_dst_ids = PACK_TARGET_INFO(src_id, dst_id);
- 	ffa_value_t ret;
--
-+	msleep(1);
- 	if (mode_32bit) {
- 		req_id = FFA_MSG_SEND_DIRECT_REQ;
- 		resp_id = FFA_MSG_SEND_DIRECT_RESP;
diff --git a/meta-arm-bsp/recipes-kernel/linux/linux-arm-platforms.inc b/meta-arm-bsp/recipes-kernel/linux/linux-arm-platforms.inc
index 045c8568..6c132c97 100644
--- a/meta-arm-bsp/recipes-kernel/linux/linux-arm-platforms.inc
+++ b/meta-arm-bsp/recipes-kernel/linux/linux-arm-platforms.inc
@@ -30,7 +30,6 @@  LINUX_KERNEL_TYPE:corstone1000 = "standard"
 KERNEL_EXTRA_ARGS:corstone1000 += "CONFIG_INITRAMFS_COMPRESSION_NONE=y"
 SRC_URI:append:corstone1000 = " \
            file://defconfig  \
-           file://0002-Adds-workaround-for-cs1k-specific-bug.patch \
         "
 
 SRC_URI:append:corstone1000 = " ${@bb.utils.contains('MACHINE_FEATURES', \