Hello arm-soc maintainers,
Please pull this AMDTEE driver fix to add a return origin field to the
struct tee_cmd_load_ta used when loading a Trusted Application into the
AMDTEE. This change is backward compatible.
Note that this isn't a usual Arm driver update. This targets AMD instead,
but is part of the TEE subsystem.
Thanks,
Jens
The following changes since commit f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6:
Linux 6.4-rc2 (2023-05-14 12:51:40 -0700)
are available in the Git repository at:
https://git.linaro.org/people/jens.wiklander/linux-tee.git tags/amdtee-fix-for-v6.5
for you to fetch changes up to 436eeae0411acdfc54521ddea80ee76d4ae8a7ea:
tee: amdtee: Add return_origin to 'struct tee_cmd_load_ta' (2023-05-15 08:29:52 +0200)
----------------------------------------------------------------
AMDTEE add return origin to load TA command
----------------------------------------------------------------
Rijo Thomas (1):
tee: amdtee: Add return_origin to 'struct tee_cmd_load_ta'
drivers/tee/amdtee/amdtee_if.h | 10 ++++++----
drivers/tee/amdtee/call.c | 30 +++++++++++++++++-------------
2 files changed, 23 insertions(+), 17 deletions(-)
Hi, Olivier,
That makes it clearer to me. Thanks a lot.
Regards,
Yuye.
------------------------------------------------------------------
发件人:Olivier Deprez <Olivier.Deprez(a)arm.com>
发送时间:2023年5月30日(星期二) 21:07
收件人:Jens Wiklander <jens.wiklander(a)linaro.org>; 梅建强(禹夜) <meijianqiang.mjq(a)alibaba-inc.com>
抄 送:op-tee <op-tee(a)lists.trustedfirmware.org>; hafnium <hafnium(a)lists.trustedfirmware.org>; 黄明(连一) <hm281385(a)alibaba-inc.com>
主 题:Re: optee_benchmark pmccfiltr_el0
Hi Yuye,
In general the consensus is that PMU cycle and event counting in EL3 & secure world has to be disabled. I gather this is to avoid probing crypto algorithms timings, or leverage cache-based timing side channels (e.g. spectre).
See Arm ARM D11.5.3 Prohibiting event and cycle counting
Cycle and event counting is disabled by:
https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/include/ar… <https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/include/ar… >
See also https://trustedfirmware-a.readthedocs.io/en/latest/process/security-hardeni… <https://trustedfirmware-a.readthedocs.io/en/latest/process/security-hardeni… >
Note there are various knobs depending on implemented architecture extensions FEAT_PMUv3 / FEAT_PMUv3pX / FEAT_Debugv8p2
You could try to permit cycle counting in the secure world for the sake of a one shot experiment,
but note that this has never been tried, and this should probably not be productized as things stand.
Regards,
Olivier.
From: 梅建强(禹夜) <meijianqiang.mjq(a)alibaba-inc.com>
Sent: 30 May 2023 10:38
To: Jens Wiklander <jens.wiklander(a)linaro.org>; Olivier Deprez <Olivier.Deprez(a)arm.com>
Cc: op-tee <op-tee(a)lists.trustedfirmware.org>; hafnium <hafnium(a)lists.trustedfirmware.org>; 黄明(连一) <hm281385(a)alibaba-inc.com>
Subject: optee_benchmark pmccfiltr_el0
Hi,
It is confirmed that the problem is related to the pmu register configuration and that pmccntr_el0 can be read at any exception level.
Regards,
Yuye.
------------------------------------------------------------------
发件人:梅建强(禹夜) <meijianqiang.mjq(a)alibaba-inc.com>
发送时间:2023年5月26日(星期五) 16:59
收件人:Jens Wiklander <jens.wiklander(a)linaro.org>; Olivier Deprez <olivier.deprez(a)arm.com>
抄 送:op-tee <op-tee(a)lists.trustedfirmware.org>; hafnium <hafnium(a)lists.trustedfirmware.org>; 黄明(连一) <hm281385(a)alibaba-inc.com>
主 题:optee_benchmark pmccfiltr_el0
Hi, Jens, Olivier,
In case of that optee runs at sel1 and hafnium runs at sel2, we want to test benchmark by executing the following command at optee_benchmark path:
./out/benchmark ../optee_examples/out/ca/optee_example_hello_world
After entering into the benchmark pta, the bm_timestamp function attempts to read the pmccfiltr_el0 register.
In cold boot, the following code will be executed during hafnium initialization:
vm->arch.trapped_features |= HF_FEATURE_PERFMON;
This will prevent the secondary vm from accessing the performance counter registers.
We remove the code, the bm_timestamp function can read pmccfiltr_el0 without trapping into hafnium.
But the value of pmccfiltr_el0 remains unchanged and cannot be counted.
We tried to read the register in hafnium and found that there was no change either.
In contrast, in the normal world, pmccfiltr_el0 counts normally.
Is it related to the pmu register configuration or does sel1 not support the pmccfiltr_el0 count at present?
Thanks for the support.
Regards,
Yuye.
This RFC proposes an implementation of a remoteproc tee driver to
communicate with a TEE trusted application in charge of authenticating
and loading remoteproc firmware image in an Arm secure context.
The services implemented are the same as those offered by the Linux
remoteproc framework:
- load of a signed firmware
- start/stop of a coprocessor
- get the resource table
The OP-TEE code in charge of providing the service in a trusted application
is proposed for upstream here:
https://github.com/OP-TEE/optee_os/pull/6027
For more details on the implementation a presentation is available here:
https://resources.linaro.org/en/resource/6c5bGvZwUAjX56fvxthxds
Arnaud Pouliquen (4):
tee: Re-enable vmalloc page support for shared memory
remoteproc: Add TEE support
dt-bindings: remoteproc: add compatibility for TEE support
remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
.../bindings/remoteproc/st,stm32-rproc.yaml | 33 +-
drivers/remoteproc/Kconfig | 9 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/stm32_rproc.c | 234 +++++++++--
drivers/remoteproc/tee_remoteproc.c | 397 ++++++++++++++++++
drivers/tee/tee_shm.c | 24 +-
include/linux/tee_remoteproc.h | 101 +++++
7 files changed, 753 insertions(+), 46 deletions(-)
create mode 100644 drivers/remoteproc/tee_remoteproc.c
create mode 100644 include/linux/tee_remoteproc.h
--
2.25.1
Hi,
It is confirmed that the problem is related to the pmu register configuration and that pmccntr_el0 can be read at any exception level.
Regards,
Yuye.
------------------------------------------------------------------
发件人:梅建强(禹夜) <meijianqiang.mjq(a)alibaba-inc.com>
发送时间:2023年5月26日(星期五) 16:59
收件人:Jens Wiklander <jens.wiklander(a)linaro.org>; Olivier Deprez <olivier.deprez(a)arm.com>
抄 送:op-tee <op-tee(a)lists.trustedfirmware.org>; hafnium <hafnium(a)lists.trustedfirmware.org>; 黄明(连一) <hm281385(a)alibaba-inc.com>
主 题:optee_benchmark pmccfiltr_el0
Hi, Jens, Olivier,
In case of that optee runs at sel1 and hafnium runs at sel2, we want to test benchmark by executing the following command at optee_benchmark path:
./out/benchmark ../optee_examples/out/ca/optee_example_hello_world
After entering into the benchmark pta, the bm_timestamp function attempts to read the pmccfiltr_el0 register.
In cold boot, the following code will be executed during hafnium initialization:
vm->arch.trapped_features |= HF_FEATURE_PERFMON;
This will prevent the secondary vm from accessing the performance counter registers.
We remove the code, the bm_timestamp function can read pmccfiltr_el0 without trapping into hafnium.
But the value of pmccfiltr_el0 remains unchanged and cannot be counted.
We tried to read the register in hafnium and found that there was no change either.
In contrast, in the normal world, pmccfiltr_el0 counts normally.
Is it related to the pmu register configuration or does sel1 not support the pmccfiltr_el0 count at present?
Thanks for the support.
Regards,
Yuye.
Hi Alex,
Are you still working on it or planning to resubmit it ?
[1] The current optee tee kernel driver implementation doesn't work when
IMA is used with optee implemented ftpm.
The ftpm has dependency on tee-supplicant which comes once the user-space
is up and running and IMA attestation happens at boot time and it requires
to extend ftpm PCRs.
But IMA can't use PCRs if ftpm use secure emmc RPMB partition. As optee
can only access RPMB via tee-supplicant(user space). So, there should
be a fast path to allow optee os to access the RPMB parititon without
waiting for user-space the tee supplicant.
To achieve this fast path linux optee driver and mmc driver needs
some work and finally it will need RPMB driver which you posted.
Please let me know what's your plan on this.
[1] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
Thanks,
Shyam
From: Etienne Carriere <etienne.carriere(a)linaro.org>
Adds support in the OP-TEE driver to keep track of reserved system
threads. The optee_cq_*() functions are updated to handle this if
enabled. The SMC ABI part of the driver enables this tracking, but the
FF-A ABI part does not.
The logic allows atleast 1 OP-TEE thread can be reserved to TEE system
sessions. For sake of simplicity, initialization of call queue
management is factorized into new helper function optee_cq_init().
Co-developed-by: Jens Wiklander <jens.wiklander(a)linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere(a)linaro.org>
Co-developed-by: Sumit Garg <sumit.garg(a)linaro.org>
Signed-off-by: Sumit Garg <sumit.garg(a)linaro.org>
---
Disclaimer: Compile tested only
Hi Etienne,
Overall the idea we agreed upon was okay but the implementation looked
complex to me. So I thought it would be harder to explain that via
review and I decided myself to give a try at simplification. I would
like you to test it if this still addresses the SCMI deadlock problem or
not. Also, feel free to include this in your patchset if all goes fine
wrt testing.
-Sumit
Changes since v8:
- Simplified system threads tracking implementation.
drivers/tee/optee/call.c | 72 +++++++++++++++++++++++++++++--
drivers/tee/optee/ffa_abi.c | 3 +-
drivers/tee/optee/optee_private.h | 16 +++++++
drivers/tee/optee/smc_abi.c | 16 ++++++-
4 files changed, 99 insertions(+), 8 deletions(-)
diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
index 42e478ac6ce1..09e824e4dcaf 100644
--- a/drivers/tee/optee/call.c
+++ b/drivers/tee/optee/call.c
@@ -39,9 +39,27 @@ struct optee_shm_arg_entry {
DECLARE_BITMAP(map, MAX_ARG_COUNT_PER_ENTRY);
};
+void optee_cq_init(struct optee_call_queue *cq, int thread_count)
+{
+ mutex_init(&cq->mutex);
+ INIT_LIST_HEAD(&cq->waiters);
+ /*
+ * If cq->total_thread_count is 0 then we're not trying to keep
+ * track of how many free threads we have, instead we're relying on
+ * the secure world to tell us when we're out of thread and have to
+ * wait for another thread to become available.
+ */
+ cq->total_thread_count = thread_count;
+ cq->free_thread_count = thread_count;
+}
+
void optee_cq_wait_init(struct optee_call_queue *cq,
struct optee_call_waiter *w, bool sys_thread)
{
+ bool need_wait = false;
+
+ memset(w, 0, sizeof(*w));
+
/*
* We're preparing to make a call to secure world. In case we can't
* allocate a thread in secure world we'll end up waiting in
@@ -53,15 +71,43 @@ void optee_cq_wait_init(struct optee_call_queue *cq,
mutex_lock(&cq->mutex);
/*
- * We add ourselves to the queue, but we don't wait. This
- * guarantees that we don't lose a completion if secure world
- * returns busy and another thread just exited and try to complete
- * someone.
+ * We add ourselves to a queue, but we don't wait. This guarantees
+ * that we don't lose a completion if secure world returns busy and
+ * another thread just exited and try to complete someone.
*/
init_completion(&w->c);
list_add_tail(&w->list_node, &cq->waiters);
+ if (cq->total_thread_count && sys_thread) {
+ if (cq->free_thread_count > 0)
+ cq->free_thread_count--;
+ else
+ need_wait = true;
+ } else if (cq->total_thread_count) {
+ if (cq->free_thread_count > 1)
+ cq->free_thread_count--;
+ else
+ need_wait = true;
+ }
+
mutex_unlock(&cq->mutex);
+
+ while (need_wait) {
+ optee_cq_wait_for_completion(cq, w);
+ mutex_lock(&cq->mutex);
+ if (sys_thread) {
+ if (cq->free_thread_count > 0) {
+ cq->free_thread_count--;
+ need_wait = false;
+ }
+ } else {
+ if (cq->free_thread_count > 1) {
+ cq->free_thread_count--;
+ need_wait = false;
+ }
+ }
+ mutex_unlock(&cq->mutex);
+ }
}
void optee_cq_wait_for_completion(struct optee_call_queue *cq,
@@ -104,6 +150,8 @@ void optee_cq_wait_final(struct optee_call_queue *cq,
/* Get out of the list */
list_del(&w->list_node);
+ cq->free_thread_count++;
+
/* Wake up one eventual waiting task */
optee_cq_complete_one(cq);
@@ -361,6 +409,22 @@ int optee_open_session(struct tee_context *ctx,
return rc;
}
+int optee_system_session(struct tee_context *ctx, u32 session)
+{
+ struct optee_context_data *ctxdata = ctx->data;
+ struct optee_session *sess;
+
+ mutex_lock(&ctxdata->mutex);
+
+ sess = find_session(ctxdata, session);
+ if (sess && !sess->use_sys_thread)
+ sess->use_sys_thread = true;
+
+ mutex_unlock(&ctxdata->mutex);
+
+ return 0;
+}
+
int optee_close_session_helper(struct tee_context *ctx, u32 session,
bool system_thread)
{
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
index 5fde9d4100e3..0c9055691343 100644
--- a/drivers/tee/optee/ffa_abi.c
+++ b/drivers/tee/optee/ffa_abi.c
@@ -852,8 +852,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
if (rc)
goto err_unreg_supp_teedev;
mutex_init(&optee->ffa.mutex);
- mutex_init(&optee->call_queue.mutex);
- INIT_LIST_HEAD(&optee->call_queue.waiters);
+ optee_cq_init(&optee->call_queue, 0);
optee_supp_init(&optee->supp);
optee_shm_arg_cache_init(optee, arg_cache_flags);
ffa_dev_set_drvdata(ffa_dev, optee);
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index b68273051454..6dcecb83c893 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -40,15 +40,29 @@ typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
unsigned long, unsigned long,
struct arm_smccc_res *);
+/*
+ * struct optee_call_waiter - TEE entry may need to wait for a free TEE thread
+ * @list_node Reference in waiters list
+ * @c Waiting completion reference
+ */
struct optee_call_waiter {
struct list_head list_node;
struct completion c;
};
+/*
+ * struct optee_call_queue - OP-TEE call queue management
+ * @mutex Serializes access to this struct
+ * @waiters List of threads waiting to enter OP-TEE
+ * @total_thread_count Overall number of thread context in OP-TEE or 0
+ * @free_thread_count Number of threads context free in OP-TEE
+ */
struct optee_call_queue {
/* Serializes access to this struct */
struct mutex mutex;
struct list_head waiters;
+ int total_thread_count;
+ int free_thread_count;
};
struct optee_notif {
@@ -254,6 +268,7 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params,
int optee_open_session(struct tee_context *ctx,
struct tee_ioctl_open_session_arg *arg,
struct tee_param *param);
+int optee_system_session(struct tee_context *ctx, u32 session);
int optee_close_session_helper(struct tee_context *ctx, u32 session,
bool system_thread);
int optee_close_session(struct tee_context *ctx, u32 session);
@@ -303,6 +318,7 @@ static inline void optee_to_msg_param_value(struct optee_msg_param *mp,
mp->u.value.c = p->u.value.c;
}
+void optee_cq_init(struct optee_call_queue *cq, int thread_count);
void optee_cq_wait_init(struct optee_call_queue *cq,
struct optee_call_waiter *w, bool sys_thread);
void optee_cq_wait_for_completion(struct optee_call_queue *cq,
diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
index e2763cdcf111..3314ffeb91c8 100644
--- a/drivers/tee/optee/smc_abi.c
+++ b/drivers/tee/optee/smc_abi.c
@@ -1209,6 +1209,7 @@ static const struct tee_driver_ops optee_clnt_ops = {
.release = optee_release,
.open_session = optee_open_session,
.close_session = optee_close_session,
+ .system_session = optee_system_session,
.invoke_func = optee_invoke_func,
.cancel_req = optee_cancel_req,
.shm_register = optee_shm_register,
@@ -1356,6 +1357,16 @@ static bool optee_msg_exchange_capabilities(optee_invoke_fn *invoke_fn,
return true;
}
+static unsigned int optee_msg_get_thread_count(optee_invoke_fn *invoke_fn)
+{
+ struct arm_smccc_res res;
+
+ invoke_fn(OPTEE_SMC_GET_THREAD_COUNT, 0, 0, 0, 0, 0, 0, 0, &res);
+ if (res.a0)
+ return 0;
+ return res.a1;
+}
+
static struct tee_shm_pool *
optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm)
{
@@ -1609,6 +1620,7 @@ static int optee_probe(struct platform_device *pdev)
struct optee *optee = NULL;
void *memremaped_shm = NULL;
unsigned int rpc_param_count;
+ unsigned int thread_count;
struct tee_device *teedev;
struct tee_context *ctx;
u32 max_notif_value;
@@ -1636,6 +1648,7 @@ static int optee_probe(struct platform_device *pdev)
return -EINVAL;
}
+ thread_count = optee_msg_get_thread_count(invoke_fn);
if (!optee_msg_exchange_capabilities(invoke_fn, &sec_caps,
&max_notif_value,
&rpc_param_count)) {
@@ -1725,8 +1738,7 @@ static int optee_probe(struct platform_device *pdev)
if (rc)
goto err_unreg_supp_teedev;
- mutex_init(&optee->call_queue.mutex);
- INIT_LIST_HEAD(&optee->call_queue.waiters);
+ optee_cq_init(&optee->call_queue, thread_count);
optee_supp_init(&optee->supp);
optee->smc.memremaped_shm = memremaped_shm;
optee->pool = pool;
--
2.34.1
Hi, Jens, Olivier,
In case of that optee runs at sel1 and hafnium runs at sel2, we want to test benchmark by executing the following command at optee_benchmark path:
./out/benchmark ../optee_examples/out/ca/optee_example_hello_world
After entering into the benchmark pta, the bm_timestamp function attempts to read the pmccfiltr_el0 register.
In cold boot, the following code will be executed during hafnium initialization:
vm->arch.trapped_features |= HF_FEATURE_PERFMON;
This will prevent the secondary vm from accessing the performance counter registers.
We remove the code, the bm_timestamp function can read pmccfiltr_el0 without trapping into hafnium.
But the value of pmccfiltr_el0 remains unchanged and cannot be counted.
We tried to read the register in hafnium and found that there was no change either.
In contrast, in the normal world, pmccfiltr_el0 counts normally.
Is it related to the pmu register configuration or does sel1 not support the pmccfiltr_el0 count at present?
Thanks for the support.
Regards,
Yuye.
This series introduces the tee based EFI Runtime Variable Service.
The eMMC device is typically owned by the non-secure world(linux in
this case). There is an existing solution utilizing eMMC RPMB partition
for EFI Variables, it is implemented by interacting with
OP-TEE, StandaloneMM(as EFI Variable Service Pseudo TA), eMMC driver
and tee-supplicant. The last piece is the tee-based variable access
driver to interact with OP-TEE and StandaloneMM.
Changelog:
v3 -> v4:
- replace the reference from EDK2 to PI Specification
- remove EDK2 source code reference comments
- prepare nonblocking variant of set_variable, it just returns
EFI_UNSUPPORTED
- remove redundant buffer size check
- argument name change in mm_communicate
- function interface changes in setup_mm_hdr to remove (void **) cast
v2 -> v3:
- add CONFIG_EFI dependency to TEE_STMM_EFI
- add missing return code check for tee_client_invoke_func()
- directly call efivars_register/unregister from tee_stmm_efi.c
rfc v1 -> v2:
- split patch into three patches, one for drivers/tee,
one for include/linux/efi.h, and one for the driver/firmware/efi/stmm
- context/session management into probe() and remove() same as other tee
client driver
- StMM variable driver is moved from driver/tee/optee to driver/firmware/efi
- use "tee" prefix instead of "optee" in driver/firmware/efi/stmm/tee_stmm_efi.c,
this file does not contain op-tee specific code, abstracted by tee layer and
StMM variable driver will work on other tee implementation.
- PTA_STMM_CMD_COMMUNICATE -> PTA_STMM_CMD_COMMUNICATE
- implement query_variable_store() but currently not used
- no use of TEEC_SUCCESS, it is defined in driver/tee/optee/optee_private.h.
Other tee client drivers use 0 instead of using TEEC_SUCCESS
- remove TEEC_ERROR_EXCESS_DATA status, it is referred just to output
error message
Masahisa Kojima (3):
efi: expose efivar generic ops register function
efi: Add EFI_ACCESS_DENIED status code
efi: Add tee-based EFI variable driver
drivers/firmware/efi/Kconfig | 15 +
drivers/firmware/efi/Makefile | 1 +
drivers/firmware/efi/efi.c | 12 +
drivers/firmware/efi/stmm/mm_communication.h | 236 +++++++
drivers/firmware/efi/stmm/tee_stmm_efi.c | 637 +++++++++++++++++++
include/linux/efi.h | 4 +
6 files changed, 905 insertions(+)
create mode 100644 drivers/firmware/efi/stmm/mm_communication.h
create mode 100644 drivers/firmware/efi/stmm/tee_stmm_efi.c
--
2.30.2
This series introduces the tee based EFI Runtime Variable Service.
The eMMC device is typically owned by the non-secure world(linux in
this case). There is an existing solution utilizing eMMC RPMB partition
for EFI Variables, it is implemented by interacting with
OP-TEE, StandaloneMM(as EFI Variable Service Pseudo TA), eMMC driver
and tee-supplicant. The last piece is the tee-based variable access
driver to interact with OP-TEE and StandaloneMM.
Changelog:
v2 -> v3:
- add CONFIG_EFI dependency to TEE_STMM_EFI
- add missing return code check for tee_client_invoke_func()
- directly call efivars_register/unregister from tee_stmm_efi.c
rfc v1 -> v2:
- split patch into three patches, one for drivers/tee,
one for include/linux/efi.h, and one for the driver/firmware/efi/stmm
- context/session management into probe() and remove() same as other tee
client driver
- StMM variable driver is moved from driver/tee/optee to driver/firmware/efi
- use "tee" prefix instead of "optee" in driver/firmware/efi/stmm/tee_stmm_efi.c,
this file does not contain op-tee specific code, abstracted by tee layer and
StMM variable driver will work on other tee implementation.
- PTA_STMM_CMD_COMMUNICATE -> PTA_STMM_CMD_COMMUNICATE
- implement query_variable_store() but currently not used
- no use of TEEC_SUCCESS, it is defined in driver/tee/optee/optee_private.h.
Other tee client drivers use 0 instead of using TEEC_SUCCESS
- remove TEEC_ERROR_EXCESS_DATA status, it is refered just to output
error message
Masahisa Kojima (3):
efi: expose efivar generic ops register function
efi: Add EFI_ACCESS_DENIED status code
efi: Add tee-based EFI variable driver
drivers/firmware/efi/Kconfig | 15 +
drivers/firmware/efi/Makefile | 1 +
drivers/firmware/efi/efi.c | 12 +
drivers/firmware/efi/stmm/mm_communication.h | 249 ++++++++
drivers/firmware/efi/stmm/tee_stmm_efi.c | 626 +++++++++++++++++++
include/linux/efi.h | 4 +
6 files changed, 907 insertions(+)
create mode 100644 drivers/firmware/efi/stmm/mm_communication.h
create mode 100644 drivers/firmware/efi/stmm/tee_stmm_efi.c
--
2.30.2
Hello arm-soc maintainers,
Please pull this small patch fixing an unintialized variable in the
OP-TEE driver. The error has been reported recently by the kernel test
robot.
I realize that this is late for v6.3, please queue this for v6.4 instead
if v6.3 isn't possible.
Thanks,
Jens
The following changes since commit eeac8ede17557680855031c6f305ece2378af326:
Linux 6.3-rc2 (2023-03-12 16:36:44 -0700)
are available in the Git repository at:
https://git.linaro.org/people/jens.wiklander/linux-tee.git tags/optee-async-notif-fix-for-v6.3
for you to fetch changes up to 654d0310007146fae87b0c1a68f81e53ad519b14:
optee: fix uninited async notif value (2023-04-20 14:52:25 +0200)
----------------------------------------------------------------
Fixes an uninitialized variable in OP-TEE driver
----------------------------------------------------------------
Etienne Carriere (1):
optee: fix uninited async notif value
drivers/tee/optee/smc_abi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
After TEE has completed processing of TEE_CMD_ID_LOAD_TA, set proper
value in 'return_origin' argument passed by open_session() call. To do
so, add 'return_origin' field to the structure tee_cmd_load_ta. The
Trusted OS shall update return_origin as part of TEE processing.
This change to 'struct tee_cmd_load_ta' interface requires a similar update
in AMD-TEE Trusted OS's TEE_CMD_ID_LOAD_TA interface.
This patch has been verified on Phoenix Birman setup. On older APUs,
return_origin value will be 0.
Cc: stable(a)vger.kernel.org
Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
Tested-by: Sourabh Das <sourabh.das(a)amd.com>
Signed-off-by: Rijo Thomas <Rijo-john.Thomas(a)amd.com>
---
v2:
* Added Fixes tag.
drivers/tee/amdtee/amdtee_if.h | 10 ++++++----
drivers/tee/amdtee/call.c | 30 +++++++++++++++++-------------
2 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/tee/amdtee/amdtee_if.h b/drivers/tee/amdtee/amdtee_if.h
index ff48c3e47375..e2014e21530a 100644
--- a/drivers/tee/amdtee/amdtee_if.h
+++ b/drivers/tee/amdtee/amdtee_if.h
@@ -118,16 +118,18 @@ struct tee_cmd_unmap_shared_mem {
/**
* struct tee_cmd_load_ta - load Trusted Application (TA) binary into TEE
- * @low_addr: [in] bits [31:0] of the physical address of the TA binary
- * @hi_addr: [in] bits [63:32] of the physical address of the TA binary
- * @size: [in] size of TA binary in bytes
- * @ta_handle: [out] return handle of the loaded TA
+ * @low_addr: [in] bits [31:0] of the physical address of the TA binary
+ * @hi_addr: [in] bits [63:32] of the physical address of the TA binary
+ * @size: [in] size of TA binary in bytes
+ * @ta_handle: [out] return handle of the loaded TA
+ * @return_origin: [out] origin of return code after TEE processing
*/
struct tee_cmd_load_ta {
u32 low_addr;
u32 hi_addr;
u32 size;
u32 ta_handle;
+ u32 return_origin;
};
/**
diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c
index e8cd9aaa3467..e9b63dcb3194 100644
--- a/drivers/tee/amdtee/call.c
+++ b/drivers/tee/amdtee/call.c
@@ -423,19 +423,23 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg)
if (ret) {
arg->ret_origin = TEEC_ORIGIN_COMMS;
arg->ret = TEEC_ERROR_COMMUNICATION;
- } else if (arg->ret == TEEC_SUCCESS) {
- ret = get_ta_refcount(load_cmd.ta_handle);
- if (!ret) {
- arg->ret_origin = TEEC_ORIGIN_COMMS;
- arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
-
- /* Unload the TA on error */
- unload_cmd.ta_handle = load_cmd.ta_handle;
- psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,
- (void *)&unload_cmd,
- sizeof(unload_cmd), &ret);
- } else {
- set_session_id(load_cmd.ta_handle, 0, &arg->session);
+ } else {
+ arg->ret_origin = load_cmd.return_origin;
+
+ if (arg->ret == TEEC_SUCCESS) {
+ ret = get_ta_refcount(load_cmd.ta_handle);
+ if (!ret) {
+ arg->ret_origin = TEEC_ORIGIN_COMMS;
+ arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
+
+ /* Unload the TA on error */
+ unload_cmd.ta_handle = load_cmd.ta_handle;
+ psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,
+ (void *)&unload_cmd,
+ sizeof(unload_cmd), &ret);
+ } else {
+ set_session_id(load_cmd.ta_handle, 0, &arg->session);
+ }
}
}
mutex_unlock(&ta_refcount_mutex);
--
2.25.1
Hi,
Tomorrow it's time for another LOC monthly meeting. For time and connection
details see the calendar at https://www.trustedfirmware.org/meetings/
We are considering bumping the major version number of OP-TEE to 4 and
removing old and unused or not-so-frequently tested compatibility
code. We'll switch to MbedTLS 3.4 instead of the old 2.x branch.
Any other topics?
Thanks,
Jens
On 18.05.23 08:40, Xiaoming Ding (丁晓明) wrote:
> From 35fd062d5cbc4d182eee0183843cd6350d126788 Mon Sep 17 00:00:00 2001
> From: Xiaoming Ding <xiaoming.ding(a)mediatek.com>
> Date: Wed, 10 May 2023 10:15:23 +0800
> Subject: [PATCH v2] tee: add FOLL_LONGTERM for CMA case when alloc shm
>
> CMA is widely used on insufficient memory platform for
> secure media playback case, and FOLL_LONGTERM will
> avoid tee_shm alloc pages from CMA region.
> without FOLL_LONGTERM, CMA region may alloc failed since
> tee_shm has a chance to use it in advance.
>
> modify is verified on OPTEE XTEST and kinds of secure + clear playback
>
>
> Fixes: 033ddf12bcf5 ("tee: add register user memory")
> Signed-off-by: Xiaoming Ding <xiaoming.ding(a)mediatek.com>
> ---
> v1 -> v2: take off the ifdef and apply FOLL_LONGTERM by default
>
> drivers/tee/tee_shm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> index 673cf0359494..38878e549ca4 100644
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -257,7 +257,7 @@ register_shm_helper(struct tee_context *ctx,
> unsigned long addr,
> }
>
> if (flags & TEE_SHM_USER_MAPPED)
> - rc = pin_user_pages_fast(start, num_pages, FOLL_WRITE,
> + rc = pin_user_pages_fast(start, num_pages, FOLL_WRITE |
> FOLL_LONGTERM,
> shm->pages);
> else
> rc = shm_get_kernel_pages(start, num_pages, shm-
>> pages);
I didn't dive deeply into that code, but I can spot that we can end up
long-term pinning multiple pages -- possibly unbound or is there any
sane limit on the number of pages?
Take a look at io_uring/rsrc.c and how we account long-term pinned pages
against user->locked_vm/ctx->mm_account->pinned_vm in io_account_mem().
If user space could only end up pinning one or two pages via that
interface, ok. But it looks like this interface could be abused to
create real real trouble by unprivileged users that should be able to
long-term pin that many pages.
Am I missing something important (i.e., interface is only accessible by
privileged users) or should there be proper accounting and
RLIMIT_MEMLOCK checks?
--
Thanks,
David / dhildenb
This series introduces TEE system sessions for TEE service sessions that
require TEE to provision resources to prevent deadlock when clients call
the TEE.
This deadlock situation can happen when a TEE service is used by low
level system resources as for example when Linux kernel uses SCMI
service embedded in TEE for clock, reset, regulator, etc... controls.
This case is detailled in patch 3/4:
> This feature is needed to prevent a system deadlock when several TEE
> client applications invoke TEE, consuming all TEE thread contexts
> available in the secure world. The deadlock can happen in the OP-TEE
> driver for example if all these TEE threads issue an RPC call from TEE
> to Linux OS to access an eMMC RPMB partition (TEE secure storage) which
> device clock or regulator controller is accessed through an OP-TEE SCMI
> services. In that case, Linux SCMI driver must reach OP-TEE SCMI
> service without waiting until one of the consumed TEE threads is freed.
Etienne Carriere (4):
tee: optee: system call property
tee: system session
tee: optee: support tracking system threads
firmware: arm_scmi: optee: use optee system invocation
drivers/firmware/arm_scmi/optee.c | 4 +
drivers/tee/optee/call.c | 155 +++++++++++++++++++++++++++---
drivers/tee/optee/core.c | 5 +-
drivers/tee/optee/ffa_abi.c | 13 +--
drivers/tee/optee/optee_private.h | 39 +++++++-
drivers/tee/optee/smc_abi.c | 31 ++++--
drivers/tee/tee_core.c | 8 ++
include/linux/tee_drv.h | 16 +++
8 files changed, 235 insertions(+), 36 deletions(-)
---
No change since v7
Changes since v6:
- Added this cover letter missing in previous patch series revisions.
--
2.25.1
There are now some RZ/N1 devices with 1GB rather than 256MB. The
first-stage bootloader does not support passing a DT to OP-TEE, so
static values are set at compile time. Increase the DDR size so as to
avoid OP-TEE calls failing with TEEC_ERROR_OUT_OF_MEMORY.
Signed-off-by: Ralph Siemsen <ralph.siemsen(a)linaro.org>
---
core/arch/arm/plat-rzn1/platform_config.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/core/arch/arm/plat-rzn1/platform_config.h b/core/arch/arm/plat-rzn1/platform_config.h
index a6340bb33..eb709c62f 100644
--- a/core/arch/arm/plat-rzn1/platform_config.h
+++ b/core/arch/arm/plat-rzn1/platform_config.h
@@ -11,7 +11,7 @@
/* DRAM */
#define DRAM_BASE 0x80000000
-#define DRAM_SIZE 0x10000000
+#define DRAM_SIZE 0x40000000
/* GIC */
#define GIC_BASE 0x44100000
--
2.25.1
This series introduces TEE system sessions for TEE service sessions that
require TEE to provision resources to prevent deadlock when clients call
the TEE.
This deadlock situation can happen when a TEE service is used by low
level system resources as for example when Linux kernel uses SCMI
service embedded in TEE for clock, reset, regulator, etc... controls.
This case is detailled in patch 3/4:
> This feature is needed to prevent a system deadlock when several TEE
> client applications invoke TEE, consuming all TEE thread contexts
> available in the secure world. The deadlock can happen in the OP-TEE
> driver for example if all these TEE threads issue an RPC call from TEE
> to Linux OS to access an eMMC RPMB partition (TEE secure storage) which
> device clock or regulator controller is accessed through an OP-TEE SCMI
> services. In that case, Linux SCMI driver must reach OP-TEE SCMI
> service without waiting until one of the consumed TEE threads is freed.
Etienne Carriere (4):
tee: optee: system call property
tee: system session
tee: optee: support tracking system threads
firmware: arm_scmi: optee: use optee system invocation
drivers/firmware/arm_scmi/optee.c | 4 +
drivers/tee/optee/call.c | 150 +++++++++++++++++++++++++++---
drivers/tee/optee/core.c | 5 +-
drivers/tee/optee/ffa_abi.c | 13 +--
drivers/tee/optee/optee_private.h | 20 +++-
drivers/tee/optee/smc_abi.c | 31 ++++--
drivers/tee/tee_core.c | 8 ++
include/linux/tee_drv.h | 16 ++++
8 files changed, 211 insertions(+), 36 deletions(-)
---
Changes since v6:
- Added this cover letter missing in previous patch series revisions.
--
2.25.1
After TEE has completed processing of TEE_CMD_ID_LOAD_TA, set proper
value in 'return_origin' argument passed by open_session() call. To do
so, add 'return_origin' field to the structure tee_cmd_load_ta. The
Trusted OS shall update return_origin as part of TEE processing.
This change to 'struct tee_cmd_load_ta' interface requires a similar update
in AMD-TEE Trusted OS's TEE_CMD_ID_LOAD_TA interface.
This patch has been verified on Phoenix Birman setup. On older APUs,
return_origin value will be 0.
Cc: stable(a)vger.kernel.org
Tested-by: Sourabh Das <sourabh.das(a)amd.com>
Signed-off-by: Rijo Thomas <Rijo-john.Thomas(a)amd.com>
---
drivers/tee/amdtee/amdtee_if.h | 10 ++++++----
drivers/tee/amdtee/call.c | 30 +++++++++++++++++-------------
2 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/tee/amdtee/amdtee_if.h b/drivers/tee/amdtee/amdtee_if.h
index ff48c3e47375..e2014e21530a 100644
--- a/drivers/tee/amdtee/amdtee_if.h
+++ b/drivers/tee/amdtee/amdtee_if.h
@@ -118,16 +118,18 @@ struct tee_cmd_unmap_shared_mem {
/**
* struct tee_cmd_load_ta - load Trusted Application (TA) binary into TEE
- * @low_addr: [in] bits [31:0] of the physical address of the TA binary
- * @hi_addr: [in] bits [63:32] of the physical address of the TA binary
- * @size: [in] size of TA binary in bytes
- * @ta_handle: [out] return handle of the loaded TA
+ * @low_addr: [in] bits [31:0] of the physical address of the TA binary
+ * @hi_addr: [in] bits [63:32] of the physical address of the TA binary
+ * @size: [in] size of TA binary in bytes
+ * @ta_handle: [out] return handle of the loaded TA
+ * @return_origin: [out] origin of return code after TEE processing
*/
struct tee_cmd_load_ta {
u32 low_addr;
u32 hi_addr;
u32 size;
u32 ta_handle;
+ u32 return_origin;
};
/**
diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c
index e8cd9aaa3467..e9b63dcb3194 100644
--- a/drivers/tee/amdtee/call.c
+++ b/drivers/tee/amdtee/call.c
@@ -423,19 +423,23 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg)
if (ret) {
arg->ret_origin = TEEC_ORIGIN_COMMS;
arg->ret = TEEC_ERROR_COMMUNICATION;
- } else if (arg->ret == TEEC_SUCCESS) {
- ret = get_ta_refcount(load_cmd.ta_handle);
- if (!ret) {
- arg->ret_origin = TEEC_ORIGIN_COMMS;
- arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
-
- /* Unload the TA on error */
- unload_cmd.ta_handle = load_cmd.ta_handle;
- psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,
- (void *)&unload_cmd,
- sizeof(unload_cmd), &ret);
- } else {
- set_session_id(load_cmd.ta_handle, 0, &arg->session);
+ } else {
+ arg->ret_origin = load_cmd.return_origin;
+
+ if (arg->ret == TEEC_SUCCESS) {
+ ret = get_ta_refcount(load_cmd.ta_handle);
+ if (!ret) {
+ arg->ret_origin = TEEC_ORIGIN_COMMS;
+ arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
+
+ /* Unload the TA on error */
+ unload_cmd.ta_handle = load_cmd.ta_handle;
+ psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,
+ (void *)&unload_cmd,
+ sizeof(unload_cmd), &ret);
+ } else {
+ set_session_id(load_cmd.ta_handle, 0, &arg->session);
+ }
}
}
mutex_unlock(&ta_refcount_mutex);
--
2.25.1
Hi,
I'm wondering if it is safe to call tee_invoke_supp_plugin_rpc() from within the OP-TEE kernel with both foreign and/or native exceptions masked ?
Regards,
Jacob
Hi, Jens,
>The first is the descriptor passed with FFA_MEM_RETRIEVE_REQ_32,
>this one is not likely to need fragmentation support soon.
Could you tell me why? I'm a little unclear.
Could you describe the difference between the retrieve process you mean and FFA-spec figure 18.4 in detail?
I need to make sure that I really understand what you mean,
so that I can implement the missing support for fragmented mem retrieve response as you said.
Or do you mean that we just need to implement the action done by optee after it receives FFA_MEM_RETRIEVE_RESP_32?
Regards,
Yuye.
------------------------------------------------------------------
发件人:Jens Wiklander <jens.wiklander(a)linaro.org>
发送时间:2023年4月14日(星期五) 14:23
收件人:梅建强(禹夜) <meijianqiang.mjq(a)alibaba-inc.com>
抄 送:Olivier Deprez <Olivier.Deprez(a)arm.com>; hafnium <hafnium(a)lists.trustedfirmware.org>; op-tee <op-tee(a)lists.trustedfirmware.org>
主 题:Re: fragment transmission while retrieving memory
Hi Yuye,
On Fri, Apr 14, 2023 at 4:29 AM 梅建强(禹夜)
<meijianqiang.mjq(a)alibaba-inc.com> wrote:
>
>
> Hi, Jens, Olivier,
>
> If I understand properly, for this specific issue both optee and hafnium need to do further fix.
> The following discussion and questions are based on figure 18.4 in the FFA spec.
>
> As I saw in the optee code, the retrieve mechanism would start with this code:
> retrieve_desc = spmc_retrieve_req(cookie);
> And in the function spmc_retrieve_req,
> optee sets the total length to the same size as fragment0 length before sending FFA_MEM_FRAG_REQ to hafnium,
> which means hafnium will only retrieve the fragment0 memory region.
>
> In the example process,
> optee will determines the number of TX sized fragments of descriptor before sending FFA_MEM_FRAG_REQ to hafnium.
> I have a question here.
> How should optee calculate this quantity?
>
> Hafnium then receives the FFA_MEM_FRAG_REQ.
> As I saw in the hafnium code, hafnium blocks the case that total length>fragment length.
> if (fragment_length != length) {
> dlog_verbose("Fragmentation not yet supported.\n");
> return ffa_error(FFA_INVALID_PARAMETERS);
> }
> Obviously it is not what we expected.
> Then following the example process, hafnium should allocate handle and use it to associate fragments.
> I didn't find the corresponding implementation in hafnium code for this step.
> And I want to know how to implement the associate action here.
>
> After that, optee lacks the implementation of FFA_MEM_FRAG_RX and FFA_MEM_FRAG_TX ABI in CFG_CORE_SEL2_SPMC enable case.
> Is there any support plan for the implementation of fragmented memory retrieve in the optee community?
> Does the hafnium community have a plan to implement it cooperatively?
If I've understood this correctly we have two different cases of
eventual fragmentation of descriptors. The first is the descriptor
passed with FFA_MEM_RETRIEVE_REQ_32, this one is not likely to need
fragmentation support soon. The other is the descriptor received with
FFA_MEM_RETRIEVE_RESP, I believe this is where you've run into
trouble.
From OP-TEE point of view, it would make sense if you took lead on
implementing this. I can help with review etc.
Thanks,
Jens
>
> Thanks a lot for your support.
>
> Regards,
> Yuye.
>
>
>
>
>
> ------------------------------------------------------------------
> 发件人:Olivier Deprez <Olivier.Deprez(a)arm.com>
> 发送时间:2023年4月13日(星期四) 15:52
> 收件人:梅建强(禹夜) <meijianqiang.mjq(a)alibaba-inc.com>
> 抄 送:hafnium <hafnium(a)lists.trustedfirmware.org>; Jens Wiklander <jens.wiklander(a)linaro.org>
> 主 题:Re: fragment transmission while retrieving memory
>
> Hi Yuye, Jens,
>
> For the record, and If I understand properly the last comment in the github issue:
> OP-TEE is missing the implementation for receipt of a fragmented retrieve response
> There is no further fix to be done in Hafnium for this specific issue at this moment, please let me know otherwise.
>
> Thanks, Olivier.
>
>
> ________________________________
> From: 梅建强(禹夜) <meijianqiang.mjq(a)alibaba-inc.com>
> Sent: 12 April 2023 09:27
> To: Olivier Deprez <Olivier.Deprez(a)arm.com>
> Cc: hafnium <hafnium(a)lists.trustedfirmware.org>; Jens Wiklander <jens.wiklander(a)linaro.org>
> Subject: Re: fragment transmission while retrieving memory
>
>
>
>
> Hi, Olivier,
>
> In our setup,
> Hafnium commit: 997476a74571aec4f1a23590d45edf516f3934f4
> optee version: 3.20.0
> Thanks.
>
> Regards,
> Yuye.
>
> ------------------------------------------------------------------
> 发件人:Olivier Deprez <Olivier.Deprez(a)arm.com>
> 发送时间:2023年4月12日(星期三) 15:22
> 收件人:梅建强(禹夜) <meijianqiang.mjq(a)alibaba-inc.com>
> 抄 送:hafnium <hafnium(a)lists.trustedfirmware.org>; Jens Wiklander <jens.wiklander(a)linaro.org>
> 主 题:Re: fragment transmission while retrieving memory
>
> Hi Yuye,
>
> AFAIK concerning Hafnium, fragmented mem sharing is supported for FFA_MEM_SHARE/LEND/DONATE and FFA_MEM_RETRIEVE_RESP.
> (For the sake of clarity, this isn't supported for FFA_MEM_RETRIEVE_REQ but this shouldn't be a concern as this limitation only exists in the case of mem sharing to multiple borrowers. In your case of a single borrower the mem retrieve req. shouldn't have to be fragmented).
>
> Can you tell which hafnium commit hash is used in your setup?
>
> At the moment, I cannot tell if the issue described concern a miss in Hafnium or OP-TEE.
> I need to dig a bit further into both implementations and I'll let you know.
>
> Regards,
> Olivier.
>
>
> ________________________________
> From: 梅建强(禹夜) <meijianqiang.mjq(a)alibaba-inc.com>
> Sent: 12 April 2023 08:21
> To: Olivier Deprez <Olivier.Deprez(a)arm.com>
> Cc: hafnium <hafnium(a)lists.trustedfirmware.org>; Jens Wiklander <jens.wiklander(a)linaro.org>
> Subject: fragment transmission while retrieving memory
>
>
>
>
> Hi, Olivier,
>
> Recently, I've been working on this issue.
> https://github.com/OP-TEE/optee_os/issues/5943 <https://github.com/OP-TEE/optee_os/issues/5943 >
> Do you know any differences between hafnium's current implementation of fragment transmission while retrieving memory and the example process described in FF-A 1.1 Figure 18.4?
>
> Regards,
> Yuye.
>
>