2023-02-14 15:30:17

by Etienne Carriere

[permalink] [raw]

Subject: [PATCH v2 1/2] tee: system invocation

---

Adds TEE system invocation context provisioning for a Linux driver
to provision execution contexts for invocation of system service
hosted in TEE. OP-TEE SMC ABI implements such invocation context
provisioning.

This feature is needed when a TEE invocation cannot afford to wait for
a free TEE thread when all TEE threads context are used and suspended
as these may be suspended waiting for a system service, as an SCMI clock
or voltage regulator, to be enabled. An example is when OP-TEE invokes
a Linux OS remote service (RPC) to access an eMMC RPMB partition and
the eMMC device is supplied by an OP-TEE SCMI regulator.

Signed-off-by: Etienne Carriere <[email protected]>
---
Change since v1
- Addressed comment on Linux client to claim reservation on TEE context.
This brings 2 new operations from client to TEE to request and release
system thread contexts: 2 new tee_drv.h API functions, 2 new ops
functions in struct tee_driver_ops. The OP-TEE implement shall implement
2 new fastcall SMC funcIDs.
- Fixed typos in commit message.
---
drivers/tee/optee/optee_smc.h | 60 +++++++++++++++++++++++++++++++++--
drivers/tee/optee/smc_abi.c | 34 +++++++++++++++++++-
drivers/tee/tee_core.c | 30 ++++++++++++++++++
include/linux/tee_drv.h | 21 ++++++++++++
4 files changed, 141 insertions(+), 4 deletions(-)

diff --git a/drivers/tee/optee/optee_smc.h b/drivers/tee/optee/optee_smc.h
index 73b5e7760d10..75b19e1bd185 100644
--- a/drivers/tee/optee/optee_smc.h
+++ b/drivers/tee/optee/optee_smc.h
@@ -108,7 +108,8 @@ struct optee_smc_call_get_os_revision_result {
* Call with struct optee_msg_arg as argument
*
* When called with OPTEE_SMC_CALL_WITH_RPC_ARG or
- * OPTEE_SMC_CALL_WITH_REGD_ARG in a0 there is one RPC struct optee_msg_arg
+ * OPTEE_SMC_CALL_WITH_REGD_ARG or OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG
+ * in a0 there is one RPC struct optee_msg_arg
* following after the first struct optee_msg_arg. The RPC struct
* optee_msg_arg has reserved space for the number of RPC parameters as
* returned by OPTEE_SMC_EXCHANGE_CAPABILITIES.
@@ -130,8 +131,8 @@ struct optee_smc_call_get_os_revision_result {
* a4-6 Not used
* a7 Hypervisor Client ID register
*
- * Call register usage, OPTEE_SMC_CALL_WITH_REGD_ARG:
- * a0 SMC Function ID, OPTEE_SMC_CALL_WITH_REGD_ARG
+ * Call register usage, OPTEE_SMC_CALL_WITH_REGD_ARG and OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG:
+ * a0 SMC Function ID, OPTEE_SMC_CALL_WITH_REGD_ARG or OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG
* a1 Upper 32 bits of a 64-bit shared memory cookie
* a2 Lower 32 bits of a 64-bit shared memory cookie
* a3 Offset of the struct optee_msg_arg in the shared memory with the
@@ -175,6 +176,8 @@ struct optee_smc_call_get_os_revision_result {
OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_WITH_RPC_ARG)
#define OPTEE_SMC_CALL_WITH_REGD_ARG \
OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_WITH_REGD_ARG)
+#define OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG \
+ OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG)

/*
* Get Shared Memory Config
@@ -254,6 +257,8 @@ struct optee_smc_get_shm_config_result {
#define OPTEE_SMC_SEC_CAP_ASYNC_NOTIF BIT(5)
/* Secure world supports pre-allocating RPC arg struct */
#define OPTEE_SMC_SEC_CAP_RPC_ARG BIT(6)
+/* Secure world provisions thread for system service invocation */
+#define OPTEE_SMC_SEC_CAP_SYSTEM_THREAD BIT(7)

#define OPTEE_SMC_FUNCID_EXCHANGE_CAPABILITIES 9
#define OPTEE_SMC_EXCHANGE_CAPABILITIES \
@@ -426,6 +431,55 @@ struct optee_smc_disable_shm_cache_result {
/* See OPTEE_SMC_CALL_WITH_REGD_ARG above */
#define OPTEE_SMC_FUNCID_CALL_WITH_REGD_ARG 19

+/* See OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG above */
+#define OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG 20
+
+/*
+ * Request reservation of a system invocation thread context in OP-TEE
+ *
+ * Call register usage:
+ * a0 SMC Function ID: OPTEE_SMC_CALL_RESERVE_SYS_THREAD
+ * a1-6 Not used
+ * a7 Hypervisor Client ID register
+ *
+ * Normal return register usage:
+ * a0 Return value, OPTEE_SMC_RETURN_*
+ * a1-3 Not used
+ * a4-7 Preserved
+ *
+ * Possible return values:
+ * OPTEE_SMC_RETURN_UNKNOWN_FUNCTION Trusted OS does not recognize this
+ * function.
+ * OPTEE_SMC_RETURN_OK Call successfully completed.
+ * OPTEE_SMC_RETURN_ETHREAD_LIMIT Number of Trusted OS threads exceeded
+ * for the request.
+ */
+#define OPTEE_SMC_FUNCID_CALL_RESERVE_SYS_THREAD 21
+#define OPTEE_SMC_CALL_RESERVE_SYS_THREAD \
+ OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_RESERVE_SYS_THREAD)
+
+/*
+ * Unregister reservation of a system invocation thread context in OP-TEE
+ *
+ * Call register usage:
+ * a0 SMC Function ID: OPTEE_SMC_CALL_UNRESERVE_SYS_THREAD
+ * a1-6 Not used
+ * a7 Hypervisor Client ID register
+ *
+ * Normal return register usage:
+ * a0 Return value, OPTEE_SMC_RETURN_*
+ * a1-3 Not used
+ * a4-7 Preserved
+ *
+ * Possible return values:
+ * OPTEE_SMC_RETURN_UNKNOWN_FUNCTION Trusted OS does not recognize this
+ * function.
+ * OPTEE_SMC_RETURN_OK Call successfully completed.
+ */
+#define OPTEE_SMC_FUNCID_CALL_UNRESERVE_SYS_THREAD 22
+#define OPTEE_SMC_CALL_UNRESERVE_SYS_THREAD \
+ OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_UNRESERVE_SYS_THREAD)
+
/*
* Resume from RPC (for example after processing a foreign interrupt)
*
diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
index a1c1fa1a9c28..013b5ae31c0e 100644
--- a/drivers/tee/optee/smc_abi.c
+++ b/drivers/tee/optee/smc_abi.c
@@ -889,7 +889,10 @@ static int optee_smc_do_call_with_arg(struct tee_context *ctx,
}

if (rpc_arg && tee_shm_is_dynamic(shm)) {
- param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG;
+ if (ctx->system_ctx_count)
+ param.a0 = OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG;
+ else
+ param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG;
reg_pair_from_64(&param.a1, &param.a2, (u_long)shm);
param.a3 = offs;
} else {
@@ -1085,6 +1088,33 @@ static int optee_smc_open(struct tee_context *ctx)
return optee_open(ctx, sec_caps & OPTEE_SMC_SEC_CAP_MEMREF_NULL);
}

+static int optee_request_sys_ctx(struct tee_context *ctx)
+{
+ struct optee *optee = tee_get_drvdata(ctx->teedev);
+ struct arm_smccc_res res;
+
+ if (!(optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SYSTEM_THREAD))
+ return -EINVAL;
+
+ optee->smc.invoke_fn(OPTEE_SMC_CALL_RESERVE_SYS_THREAD,
+ 0, 0, 0, 0, 0, 0, 0, &res);
+
+ if (res.a0 != OPTEE_SMC_RETURN_OK)
+ return -EINVAL;
+
+ return 0;
+}
+
+static void optee_release_sys_ctx(struct tee_context *ctx)
+{
+ struct optee *optee = tee_get_drvdata(ctx->teedev);
+ struct arm_smccc_res res;
+
+ if (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SYSTEM_THREAD)
+ optee->smc.invoke_fn(OPTEE_SMC_CALL_UNRESERVE_SYS_THREAD,
+ 0, 0, 0, 0, 0, 0, 0, &res);
+}
+
static const struct tee_driver_ops optee_clnt_ops = {
.get_version = optee_get_version,
.open = optee_smc_open,
@@ -1095,6 +1125,8 @@ static const struct tee_driver_ops optee_clnt_ops = {
.cancel_req = optee_cancel_req,
.shm_register = optee_shm_register,
.shm_unregister = optee_shm_unregister,
+ .system_ctx_request = optee_request_sys_ctx,
+ .system_ctx_release = optee_release_sys_ctx,
};

static const struct tee_desc optee_clnt_desc = {
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 98da206cd761..a7dfdea5d85b 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -5,6 +5,7 @@

#define pr_fmt(fmt) "%s: " fmt, __func__

+#include <linux/bug.h>
#include <linux/cdev.h>
#include <linux/cred.h>
#include <linux/fs.h>
@@ -1141,10 +1142,39 @@ EXPORT_SYMBOL_GPL(tee_client_open_context);

void tee_client_close_context(struct tee_context *ctx)
{
+ while (ctx->system_ctx_count)
+ tee_client_release_system_context(ctx);
+
teedev_close_context(ctx);
}
EXPORT_SYMBOL_GPL(tee_client_close_context);

+int tee_client_request_system_context(struct tee_context *ctx)
+{
+ int ret;
+
+ if (!ctx->teedev->desc->ops->system_ctx_request ||
+ !ctx->teedev->desc->ops->system_ctx_release)
+ return -EINVAL;
+
+ ret = ctx->teedev->desc->ops->system_ctx_request(ctx);
+ if (!ret)
+ ctx->system_ctx_count++;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(tee_client_request_system_context);
+
+void tee_client_release_system_context(struct tee_context *ctx)
+{
+ if (ctx->system_ctx_count &&
+ ctx->teedev->desc->ops->system_ctx_release) {
+ ctx->teedev->desc->ops->system_ctx_release(ctx);
+ ctx->system_ctx_count--;
+ }
+}
+EXPORT_SYMBOL_GPL(tee_client_release_system_context);
+
void tee_client_get_version(struct tee_context *ctx,
struct tee_ioctl_version_data *vers)
{
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index 17eb1c5205d3..45577256bb71 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -47,6 +47,8 @@ struct tee_shm_pool;
* non-blocking in nature.
* @cap_memref_null: flag indicating if the TEE Client support shared
* memory buffer with a NULL pointer.
+ * @system_ctx_count: Number of system invocation contexts provisioned for
+ * this TEE client or 0.
*/
struct tee_context {
struct tee_device *teedev;
@@ -55,6 +57,7 @@ struct tee_context {
bool releasing;
bool supp_nowait;
bool cap_memref_null;
+ unsigned int system_ctx_count;
};

struct tee_param_memref {
@@ -90,6 +93,8 @@ struct tee_param {
* @supp_send: called for supplicant to send a response
* @shm_register: register shared memory buffer in TEE
* @shm_unregister: unregister shared memory buffer in TEE
+ * @system_ctx_request: Request provisioning of a new system context in TEE
+ * @system_ctx_release: Release a provisioned system context in TEE
*/
struct tee_driver_ops {
void (*get_version)(struct tee_device *teedev,
@@ -112,6 +117,8 @@ struct tee_driver_ops {
struct page **pages, size_t num_pages,
unsigned long start);
int (*shm_unregister)(struct tee_context *ctx, struct tee_shm *shm);
+ int (*system_ctx_request)(struct tee_context *ctx);
+ void (*system_ctx_release)(struct tee_context *ctx);
};

/**
@@ -397,6 +404,20 @@ tee_client_open_context(struct tee_context *start,
*/
void tee_client_close_context(struct tee_context *ctx);

+/**
+ * tee_client_request_system_context() - Close a TEE context
+ * @ctx: TEE context to close
+ *
+ * @return 0 on success else an error code
+ */
+int tee_client_request_system_context(struct tee_context *ctx);
+
+/**
+ * tee_client_release_system_context() - Release a reserved system exec context
+ * @ctx: TEE context reference
+ */
+void tee_client_release_system_context(struct tee_context *ctx);
+
/**
* tee_client_get_version() - Query version of TEE
* @ctx: TEE context to TEE to query
--
2.25.1

2023-02-14 15:30:20

by Etienne Carriere

[permalink] [raw]

Subject: [PATCH v2 2/2] firmware: arm_scmi: optee: use optee system invocation

---

Changes SCMI optee transport to request an OP-TEE system invocation
context per SCMI channel. This prevents possible deadlock when many
Linux clients invoke OP-TEE are all suspended waiting for an OP-TEE
RPC request access an SCMI resource through the SCMI OP-TEE PTA service.

Signed-off-by: Etienne Carriere <[email protected]>
--
Changes since v1:
- Updated to use new tee API functions tee_client_request_system_context()
and tee_client_release_system_context().
---
drivers/firmware/arm_scmi/optee.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
index 2a7aeab40e54..fe91e2de3f9c 100644
--- a/drivers/firmware/arm_scmi/optee.c
+++ b/drivers/firmware/arm_scmi/optee.c
@@ -119,6 +119,7 @@ struct scmi_optee_channel {
u32 tee_session;
u32 caps;
u32 rx_len;
+ bool sys_thread;
struct mutex mu;
struct scmi_chan_info *cinfo;
union {
@@ -432,9 +433,15 @@ static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *de
channel->channel_id = channel_id;
mutex_init(&channel->mu);

+ ret = tee_client_request_system_context(scmi_optee_private->tee_ctx)
+ if (ret)
+ dev_warn(dev, "Couldn't provision an OP-TEE system context\n");
+ else
+ channel->sys_thread = true;
+
ret = setup_shmem(dev, cinfo, channel);
if (ret)
- return ret;
+ goto err_release_sysctx;

ret = open_session(scmi_optee_private, &channel->tee_session);
if (ret)
@@ -458,6 +465,9 @@ static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *de
err_free_shm:
if (channel->tee_shm)
tee_shm_free(channel->tee_shm);
+err_release_sysctx:
+ if (channel->sys_thread)
+ tee_client_release_system_context(scmi_optee_private->tee_ctx)

return ret;
}
@@ -483,6 +493,9 @@ static int scmi_optee_chan_free(int id, void *p, void *data)

scmi_free_channel(cinfo, data, id);

+ if (channel->sys_thread)
+ tee_client_release_system_context(scmi_optee_private->tee_ctx)
+
return 0;
}

--
2.25.1

2023-02-15 09:46:01

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] firmware: arm_scmi: optee: use optee system invocation

---

Hi Etienne,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.2-rc8]
[cannot apply to soc/for-next next-20230215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Etienne-Carriere/firmware-arm_scmi-optee-use-optee-system-invocation/20230214-233230
patch link: https://lore.kernel.org/r/20230214152047.1143106-2-etienne.carriere%40linaro.org
patch subject: [PATCH v2 2/2] firmware: arm_scmi: optee: use optee system invocation
config: arm-allyesconfig
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/7a2f7bc17be8c4a18d32c439458d2f0f99e18cd6
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Etienne-Carriere/firmware-arm_scmi-optee-use-optee-system-invocation/20230214-233230
git checkout 7a2f7bc17be8c4a18d32c439458d2f0f99e18cd6
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/firmware/arm_scmi/optee.c: In function 'scmi_optee_chan_setup':
>> drivers/firmware/arm_scmi/optee.c:436:77: error: expected ';' before 'if'
436 | ret = tee_client_request_system_context(scmi_optee_private->tee_ctx)
| ^
| ;
437 | if (ret)
| ~~
>> drivers/firmware/arm_scmi/optee.c:439:9: error: 'else' without a previous 'if'
439 | else
| ^~~~
>> drivers/firmware/arm_scmi/optee.c:470:79: error: expected ';' before 'return'
470 | tee_client_release_system_context(scmi_optee_private->tee_ctx)
| ^
| ;
471 |
472 | return ret;
| ~~~~~~
drivers/firmware/arm_scmi/optee.c: In function 'scmi_optee_chan_free':
drivers/firmware/arm_scmi/optee.c:497:79: error: expected ';' before 'return'
497 | tee_client_release_system_context(scmi_optee_private->tee_ctx)
| ^
| ;
498 |
499 | return 0;
| ~~~~~~
drivers/firmware/arm_scmi/optee.c:500:1: error: no return statement in function returning non-void [-Werror=return-type]
500 | }
| ^
drivers/firmware/arm_scmi/optee.c: In function 'scmi_optee_chan_setup':
drivers/firmware/arm_scmi/optee.c:473:1: error: control reaches end of non-void function [-Werror=return-type]
473 | }
| ^
cc1: some warnings being treated as errors


vim +436 drivers/firmware/arm_scmi/optee.c

412
413 static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, bool tx)
414 {
415 struct scmi_optee_channel *channel;
416 uint32_t channel_id;
417 int ret;
418
419 if (!tx)
420 return -ENODEV;
421
422 channel = devm_kzalloc(dev, sizeof(*channel), GFP_KERNEL);
423 if (!channel)
424 return -ENOMEM;
425
426 ret = of_property_read_u32_index(cinfo->dev->of_node, "linaro,optee-channel-id",
427 0, &channel_id);
428 if (ret)
429 return ret;
430
431 cinfo->transport_info = channel;
432 channel->cinfo = cinfo;
433 channel->channel_id = channel_id;
434 mutex_init(&channel->mu);
435
> 436 ret = tee_client_request_system_context(scmi_optee_private->tee_ctx)
437 if (ret)
438 dev_warn(dev, "Couldn't provision an OP-TEE system context\n");
> 439 else
440 channel->sys_thread = true;
441
442 ret = setup_shmem(dev, cinfo, channel);
443 if (ret)
444 goto err_release_sysctx;
445
446 ret = open_session(scmi_optee_private, &channel->tee_session);
447 if (ret)
448 goto err_free_shm;
449
450 ret = get_channel(channel);
451 if (ret)
452 goto err_close_sess;
453
454 /* Enable polling */
455 cinfo->no_completion_irq = true;
456
457 mutex_lock(&scmi_optee_private->mu);
458 list_add(&channel->link, &scmi_optee_private->channel_list);
459 mutex_unlock(&scmi_optee_private->mu);
460
461 return 0;
462
463 err_close_sess:
464 close_session(scmi_optee_private, channel->tee_session);
465 err_free_shm:
466 if (channel->tee_shm)
467 tee_shm_free(channel->tee_shm);
468 err_release_sysctx:
469 if (channel->sys_thread)
> 470 tee_client_release_system_context(scmi_optee_private->tee_ctx)
471
472 return ret;
473 }
474

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

---

Attachments:

(No filename) (5.85 kB)
config (355.74 kB)
Download all attachments

2023-02-15 11:50:32

by Jens Wiklander

[permalink] [raw]

Subject: Re: [PATCH v2 1/2] tee: system invocation

---

On Tue, Feb 14, 2023 at 04:20:46PM +0100, Etienne Carriere wrote:
> Adds TEE system invocation context provisioning for a Linux driver
> to provision execution contexts for invocation of system service
> hosted in TEE. OP-TEE SMC ABI implements such invocation context
> provisioning.
>
> This feature is needed when a TEE invocation cannot afford to wait for
> a free TEE thread when all TEE threads context are used and suspended
> as these may be suspended waiting for a system service, as an SCMI clock
> or voltage regulator, to be enabled. An example is when OP-TEE invokes
> a Linux OS remote service (RPC) to access an eMMC RPMB partition and
> the eMMC device is supplied by an OP-TEE SCMI regulator.
>
> Signed-off-by: Etienne Carriere <[email protected]>
> ---
> Change since v1
> - Addressed comment on Linux client to claim reservation on TEE context.
> This brings 2 new operations from client to TEE to request and release
> system thread contexts: 2 new tee_drv.h API functions, 2 new ops
> functions in struct tee_driver_ops. The OP-TEE implement shall implement
> 2 new fastcall SMC funcIDs.
> - Fixed typos in commit message.
> ---
> drivers/tee/optee/optee_smc.h | 60 +++++++++++++++++++++++++++++++++--
> drivers/tee/optee/smc_abi.c | 34 +++++++++++++++++++-
> drivers/tee/tee_core.c | 30 ++++++++++++++++++
> include/linux/tee_drv.h | 21 ++++++++++++
> 4 files changed, 141 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tee/optee/optee_smc.h b/drivers/tee/optee/optee_smc.h
> index 73b5e7760d10..75b19e1bd185 100644
> --- a/drivers/tee/optee/optee_smc.h
> +++ b/drivers/tee/optee/optee_smc.h
> @@ -108,7 +108,8 @@ struct optee_smc_call_get_os_revision_result {
> * Call with struct optee_msg_arg as argument
> *
> * When called with OPTEE_SMC_CALL_WITH_RPC_ARG or
> - * OPTEE_SMC_CALL_WITH_REGD_ARG in a0 there is one RPC struct optee_msg_arg
> + * OPTEE_SMC_CALL_WITH_REGD_ARG or OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG
> + * in a0 there is one RPC struct optee_msg_arg
> * following after the first struct optee_msg_arg. The RPC struct
> * optee_msg_arg has reserved space for the number of RPC parameters as
> * returned by OPTEE_SMC_EXCHANGE_CAPABILITIES.
> @@ -130,8 +131,8 @@ struct optee_smc_call_get_os_revision_result {
> * a4-6 Not used
> * a7 Hypervisor Client ID register
> *
> - * Call register usage, OPTEE_SMC_CALL_WITH_REGD_ARG:
> - * a0 SMC Function ID, OPTEE_SMC_CALL_WITH_REGD_ARG
> + * Call register usage, OPTEE_SMC_CALL_WITH_REGD_ARG and OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG:
> + * a0 SMC Function ID, OPTEE_SMC_CALL_WITH_REGD_ARG or OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG
> * a1 Upper 32 bits of a 64-bit shared memory cookie
> * a2 Lower 32 bits of a 64-bit shared memory cookie
> * a3 Offset of the struct optee_msg_arg in the shared memory with the
> @@ -175,6 +176,8 @@ struct optee_smc_call_get_os_revision_result {
> OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_WITH_RPC_ARG)
> #define OPTEE_SMC_CALL_WITH_REGD_ARG \
> OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_WITH_REGD_ARG)
> +#define OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG \
> + OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG)
>
> /*
> * Get Shared Memory Config
> @@ -254,6 +257,8 @@ struct optee_smc_get_shm_config_result {
> #define OPTEE_SMC_SEC_CAP_ASYNC_NOTIF BIT(5)
> /* Secure world supports pre-allocating RPC arg struct */
> #define OPTEE_SMC_SEC_CAP_RPC_ARG BIT(6)
> +/* Secure world provisions thread for system service invocation */
> +#define OPTEE_SMC_SEC_CAP_SYSTEM_THREAD BIT(7)
>
> #define OPTEE_SMC_FUNCID_EXCHANGE_CAPABILITIES 9
> #define OPTEE_SMC_EXCHANGE_CAPABILITIES \
> @@ -426,6 +431,55 @@ struct optee_smc_disable_shm_cache_result {
> /* See OPTEE_SMC_CALL_WITH_REGD_ARG above */
> #define OPTEE_SMC_FUNCID_CALL_WITH_REGD_ARG 19
>
> +/* See OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG above */
> +#define OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG 20
> +
> +/*
> + * Request reservation of a system invocation thread context in OP-TEE
> + *
> + * Call register usage:
> + * a0 SMC Function ID: OPTEE_SMC_CALL_RESERVE_SYS_THREAD
> + * a1-6 Not used
> + * a7 Hypervisor Client ID register
> + *
> + * Normal return register usage:
> + * a0 Return value, OPTEE_SMC_RETURN_*
> + * a1-3 Not used
> + * a4-7 Preserved
> + *
> + * Possible return values:
> + * OPTEE_SMC_RETURN_UNKNOWN_FUNCTION Trusted OS does not recognize this
> + * function.
> + * OPTEE_SMC_RETURN_OK Call successfully completed.
> + * OPTEE_SMC_RETURN_ETHREAD_LIMIT Number of Trusted OS threads exceeded
> + * for the request.
> + */
> +#define OPTEE_SMC_FUNCID_CALL_RESERVE_SYS_THREAD 21
> +#define OPTEE_SMC_CALL_RESERVE_SYS_THREAD \
> + OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_RESERVE_SYS_THREAD)
> +
> +/*
> + * Unregister reservation of a system invocation thread context in OP-TEE
> + *
> + * Call register usage:
> + * a0 SMC Function ID: OPTEE_SMC_CALL_UNRESERVE_SYS_THREAD
> + * a1-6 Not used
> + * a7 Hypervisor Client ID register
> + *
> + * Normal return register usage:
> + * a0 Return value, OPTEE_SMC_RETURN_*
> + * a1-3 Not used
> + * a4-7 Preserved
> + *
> + * Possible return values:
> + * OPTEE_SMC_RETURN_UNKNOWN_FUNCTION Trusted OS does not recognize this
> + * function.
> + * OPTEE_SMC_RETURN_OK Call successfully completed.
> + */
> +#define OPTEE_SMC_FUNCID_CALL_UNRESERVE_SYS_THREAD 22
> +#define OPTEE_SMC_CALL_UNRESERVE_SYS_THREAD \
> + OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_UNRESERVE_SYS_THREAD)
> +
> /*
> * Resume from RPC (for example after processing a foreign interrupt)
> *
> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> index a1c1fa1a9c28..013b5ae31c0e 100644
> --- a/drivers/tee/optee/smc_abi.c
> +++ b/drivers/tee/optee/smc_abi.c
> @@ -889,7 +889,10 @@ static int optee_smc_do_call_with_arg(struct tee_context *ctx,
> }
>
> if (rpc_arg && tee_shm_is_dynamic(shm)) {
> - param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG;
> + if (ctx->system_ctx_count)
> + param.a0 = OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG;
> + else
> + param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG;
> reg_pair_from_64(&param.a1, &param.a2, (u_long)shm);
> param.a3 = offs;
> } else {
> @@ -1085,6 +1088,33 @@ static int optee_smc_open(struct tee_context *ctx)
> return optee_open(ctx, sec_caps & OPTEE_SMC_SEC_CAP_MEMREF_NULL);
> }
>
> +static int optee_request_sys_ctx(struct tee_context *ctx)
> +{
> + struct optee *optee = tee_get_drvdata(ctx->teedev);
> + struct arm_smccc_res res;
> +
> + if (!(optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SYSTEM_THREAD))
> + return -EINVAL;
> +
> + optee->smc.invoke_fn(OPTEE_SMC_CALL_RESERVE_SYS_THREAD,
> + 0, 0, 0, 0, 0, 0, 0, &res);
> +
> + if (res.a0 != OPTEE_SMC_RETURN_OK)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static void optee_release_sys_ctx(struct tee_context *ctx)
> +{
> + struct optee *optee = tee_get_drvdata(ctx->teedev);
> + struct arm_smccc_res res;
> +
> + if (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SYSTEM_THREAD)
> + optee->smc.invoke_fn(OPTEE_SMC_CALL_UNRESERVE_SYS_THREAD,
> + 0, 0, 0, 0, 0, 0, 0, &res);
> +}
> +
> static const struct tee_driver_ops optee_clnt_ops = {
> .get_version = optee_get_version,
> .open = optee_smc_open,
> @@ -1095,6 +1125,8 @@ static const struct tee_driver_ops optee_clnt_ops = {
> .cancel_req = optee_cancel_req,
> .shm_register = optee_shm_register,
> .shm_unregister = optee_shm_unregister,
> + .system_ctx_request = optee_request_sys_ctx,
> + .system_ctx_release = optee_release_sys_ctx,
> };
>
> static const struct tee_desc optee_clnt_desc = {
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 98da206cd761..a7dfdea5d85b 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -5,6 +5,7 @@
>
> #define pr_fmt(fmt) "%s: " fmt, __func__
>
> +#include <linux/bug.h>
> #include <linux/cdev.h>
> #include <linux/cred.h>
> #include <linux/fs.h>
> @@ -1141,10 +1142,39 @@ EXPORT_SYMBOL_GPL(tee_client_open_context);
>
> void tee_client_close_context(struct tee_context *ctx)
> {
> + while (ctx->system_ctx_count)
> + tee_client_release_system_context(ctx);
> +
> teedev_close_context(ctx);
> }
> EXPORT_SYMBOL_GPL(tee_client_close_context);
>
> +int tee_client_request_system_context(struct tee_context *ctx)
> +{
> + int ret;
> +
> + if (!ctx->teedev->desc->ops->system_ctx_request ||
> + !ctx->teedev->desc->ops->system_ctx_release)
> + return -EINVAL;
> +
> + ret = ctx->teedev->desc->ops->system_ctx_request(ctx);
> + if (!ret)
> + ctx->system_ctx_count++;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(tee_client_request_system_context);
> +
> +void tee_client_release_system_context(struct tee_context *ctx)
> +{
> + if (ctx->system_ctx_count &&
> + ctx->teedev->desc->ops->system_ctx_release) {
> + ctx->teedev->desc->ops->system_ctx_release(ctx);
> + ctx->system_ctx_count--;
> + }
> +}
> +EXPORT_SYMBOL_GPL(tee_client_release_system_context);
> +
> void tee_client_get_version(struct tee_context *ctx,
> struct tee_ioctl_version_data *vers)
> {
> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> index 17eb1c5205d3..45577256bb71 100644
> --- a/include/linux/tee_drv.h
> +++ b/include/linux/tee_drv.h
> @@ -47,6 +47,8 @@ struct tee_shm_pool;
> * non-blocking in nature.
> * @cap_memref_null: flag indicating if the TEE Client support shared
> * memory buffer with a NULL pointer.
> + * @system_ctx_count: Number of system invocation contexts provisioned for
> + * this TEE client or 0.
> */
> struct tee_context {
> struct tee_device *teedev;
> @@ -55,6 +57,7 @@ struct tee_context {
> bool releasing;
> bool supp_nowait;
> bool cap_memref_null;
> + unsigned int system_ctx_count;

Does it make sense for a tee_context to reserve more than one system
context/thread? I wonder if keeping track on this shouldn't be done in a
driver specific struct, perhaps struct optee_context_data for OP-TEE.

> };
>
> struct tee_param_memref {
> @@ -90,6 +93,8 @@ struct tee_param {
> * @supp_send: called for supplicant to send a response
> * @shm_register: register shared memory buffer in TEE
> * @shm_unregister: unregister shared memory buffer in TEE
> + * @system_ctx_request: Request provisioning of a new system context in TEE
> + * @system_ctx_release: Release a provisioned system context in TEE
> */
> struct tee_driver_ops {
> void (*get_version)(struct tee_device *teedev,
> @@ -112,6 +117,8 @@ struct tee_driver_ops {
> struct page **pages, size_t num_pages,
> unsigned long start);
> int (*shm_unregister)(struct tee_context *ctx, struct tee_shm *shm);
> + int (*system_ctx_request)(struct tee_context *ctx);
> + void (*system_ctx_release)(struct tee_context *ctx);
> };
>
> /**
> @@ -397,6 +404,20 @@ tee_client_open_context(struct tee_context *start,
> */
> void tee_client_close_context(struct tee_context *ctx);
>
> +/**
> + * tee_client_request_system_context() - Close a TEE context

Copy & paste error

> + * @ctx: TEE context to close
> + *
> + * @return 0 on success else an error code
> + */
> +int tee_client_request_system_context(struct tee_context *ctx);
> +
> +/**
> + * tee_client_release_system_context() - Release a reserved system exec context

Please drop "exec", it's not mentioned anywhere else.

Thanks,
Jens

> + * @ctx: TEE context reference
> + */
> +void tee_client_release_system_context(struct tee_context *ctx);
> +
> /**
> * tee_client_get_version() - Query version of TEE
> * @ctx: TEE context to TEE to query
> --
> 2.25.1
>

2023-02-15 16:03:35

by Etienne Carriere

[permalink] [raw]

Subject: Re: [PATCH v2 1/2] tee: system invocation

---

Hi Jens,

On Wed, 15 Feb 2023 at 12:50, Jens Wiklander <[email protected]> wrote:
>
> On Tue, Feb 14, 2023 at 04:20:46PM +0100, Etienne Carriere wrote:
> > Adds TEE system invocation context provisioning for a Linux driver
> > to provision execution contexts for invocation of system service
> > hosted in TEE. OP-TEE SMC ABI implements such invocation context
> > provisioning.
> >
> > This feature is needed when a TEE invocation cannot afford to wait for
> > a free TEE thread when all TEE threads context are used and suspended
> > as these may be suspended waiting for a system service, as an SCMI clock
> > or voltage regulator, to be enabled. An example is when OP-TEE invokes
> > a Linux OS remote service (RPC) to access an eMMC RPMB partition and
> > the eMMC device is supplied by an OP-TEE SCMI regulator.
> >
> > Signed-off-by: Etienne Carriere <[email protected]>
> > ---
> > Change since v1
> > - Addressed comment on Linux client to claim reservation on TEE context.
> > This brings 2 new operations from client to TEE to request and release
> > system thread contexts: 2 new tee_drv.h API functions, 2 new ops
> > functions in struct tee_driver_ops. The OP-TEE implement shall implement
> > 2 new fastcall SMC funcIDs.
> > - Fixed typos in commit message.
> > ---
> > drivers/tee/optee/optee_smc.h | 60 +++++++++++++++++++++++++++++++++--
> > drivers/tee/optee/smc_abi.c | 34 +++++++++++++++++++-
> > drivers/tee/tee_core.c | 30 ++++++++++++++++++
> > include/linux/tee_drv.h | 21 ++++++++++++
> > 4 files changed, 141 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tee/optee/optee_smc.h b/drivers/tee/optee/optee_smc.h
> > index 73b5e7760d10..75b19e1bd185 100644
> > --- a/drivers/tee/optee/optee_smc.h
> > +++ b/drivers/tee/optee/optee_smc.h
> > @@ -108,7 +108,8 @@ struct optee_smc_call_get_os_revision_result {
> > * Call with struct optee_msg_arg as argument
> > *
> > * When called with OPTEE_SMC_CALL_WITH_RPC_ARG or
> > - * OPTEE_SMC_CALL_WITH_REGD_ARG in a0 there is one RPC struct optee_msg_arg
> > + * OPTEE_SMC_CALL_WITH_REGD_ARG or OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG
> > + * in a0 there is one RPC struct optee_msg_arg
> > * following after the first struct optee_msg_arg. The RPC struct
> > * optee_msg_arg has reserved space for the number of RPC parameters as
> > * returned by OPTEE_SMC_EXCHANGE_CAPABILITIES.
> > @@ -130,8 +131,8 @@ struct optee_smc_call_get_os_revision_result {
> > * a4-6 Not used
> > * a7 Hypervisor Client ID register
> > *
> > - * Call register usage, OPTEE_SMC_CALL_WITH_REGD_ARG:
> > - * a0 SMC Function ID, OPTEE_SMC_CALL_WITH_REGD_ARG
> > + * Call register usage, OPTEE_SMC_CALL_WITH_REGD_ARG and OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG:
> > + * a0 SMC Function ID, OPTEE_SMC_CALL_WITH_REGD_ARG or OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG
> > * a1 Upper 32 bits of a 64-bit shared memory cookie
> > * a2 Lower 32 bits of a 64-bit shared memory cookie
> > * a3 Offset of the struct optee_msg_arg in the shared memory with the
> > @@ -175,6 +176,8 @@ struct optee_smc_call_get_os_revision_result {
> > OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_WITH_RPC_ARG)
> > #define OPTEE_SMC_CALL_WITH_REGD_ARG \
> > OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_WITH_REGD_ARG)
> > +#define OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG \
> > + OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG)
> >
> > /*
> > * Get Shared Memory Config
> > @@ -254,6 +257,8 @@ struct optee_smc_get_shm_config_result {
> > #define OPTEE_SMC_SEC_CAP_ASYNC_NOTIF BIT(5)
> > /* Secure world supports pre-allocating RPC arg struct */
> > #define OPTEE_SMC_SEC_CAP_RPC_ARG BIT(6)
> > +/* Secure world provisions thread for system service invocation */
> > +#define OPTEE_SMC_SEC_CAP_SYSTEM_THREAD BIT(7)
> >
> > #define OPTEE_SMC_FUNCID_EXCHANGE_CAPABILITIES 9
> > #define OPTEE_SMC_EXCHANGE_CAPABILITIES \
> > @@ -426,6 +431,55 @@ struct optee_smc_disable_shm_cache_result {
> > /* See OPTEE_SMC_CALL_WITH_REGD_ARG above */
> > #define OPTEE_SMC_FUNCID_CALL_WITH_REGD_ARG 19
> >
> > +/* See OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG above */
> > +#define OPTEE_SMC_FUNCID_CALL_SYSTEM_WITH_REGD_ARG 20
> > +
> > +/*
> > + * Request reservation of a system invocation thread context in OP-TEE
> > + *
> > + * Call register usage:
> > + * a0 SMC Function ID: OPTEE_SMC_CALL_RESERVE_SYS_THREAD
> > + * a1-6 Not used
> > + * a7 Hypervisor Client ID register
> > + *
> > + * Normal return register usage:
> > + * a0 Return value, OPTEE_SMC_RETURN_*
> > + * a1-3 Not used
> > + * a4-7 Preserved
> > + *
> > + * Possible return values:
> > + * OPTEE_SMC_RETURN_UNKNOWN_FUNCTION Trusted OS does not recognize this
> > + * function.
> > + * OPTEE_SMC_RETURN_OK Call successfully completed.
> > + * OPTEE_SMC_RETURN_ETHREAD_LIMIT Number of Trusted OS threads exceeded
> > + * for the request.
> > + */
> > +#define OPTEE_SMC_FUNCID_CALL_RESERVE_SYS_THREAD 21
> > +#define OPTEE_SMC_CALL_RESERVE_SYS_THREAD \
> > + OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_RESERVE_SYS_THREAD)
> > +
> > +/*
> > + * Unregister reservation of a system invocation thread context in OP-TEE
> > + *
> > + * Call register usage:
> > + * a0 SMC Function ID: OPTEE_SMC_CALL_UNRESERVE_SYS_THREAD
> > + * a1-6 Not used
> > + * a7 Hypervisor Client ID register
> > + *
> > + * Normal return register usage:
> > + * a0 Return value, OPTEE_SMC_RETURN_*
> > + * a1-3 Not used
> > + * a4-7 Preserved
> > + *
> > + * Possible return values:
> > + * OPTEE_SMC_RETURN_UNKNOWN_FUNCTION Trusted OS does not recognize this
> > + * function.
> > + * OPTEE_SMC_RETURN_OK Call successfully completed.
> > + */
> > +#define OPTEE_SMC_FUNCID_CALL_UNRESERVE_SYS_THREAD 22
> > +#define OPTEE_SMC_CALL_UNRESERVE_SYS_THREAD \
> > + OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_UNRESERVE_SYS_THREAD)
> > +
> > /*
> > * Resume from RPC (for example after processing a foreign interrupt)
> > *
> > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> > index a1c1fa1a9c28..013b5ae31c0e 100644
> > --- a/drivers/tee/optee/smc_abi.c
> > +++ b/drivers/tee/optee/smc_abi.c
> > @@ -889,7 +889,10 @@ static int optee_smc_do_call_with_arg(struct tee_context *ctx,
> > }
> >
> > if (rpc_arg && tee_shm_is_dynamic(shm)) {
> > - param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG;
> > + if (ctx->system_ctx_count)
> > + param.a0 = OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG;
> > + else
> > + param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG;
> > reg_pair_from_64(&param.a1, &param.a2, (u_long)shm);
> > param.a3 = offs;
> > } else {
> > @@ -1085,6 +1088,33 @@ static int optee_smc_open(struct tee_context *ctx)
> > return optee_open(ctx, sec_caps & OPTEE_SMC_SEC_CAP_MEMREF_NULL);
> > }
> >
> > +static int optee_request_sys_ctx(struct tee_context *ctx)
> > +{
> > + struct optee *optee = tee_get_drvdata(ctx->teedev);
> > + struct arm_smccc_res res;
> > +
> > + if (!(optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SYSTEM_THREAD))
> > + return -EINVAL;
> > +
> > + optee->smc.invoke_fn(OPTEE_SMC_CALL_RESERVE_SYS_THREAD,
> > + 0, 0, 0, 0, 0, 0, 0, &res);
> > +
> > + if (res.a0 != OPTEE_SMC_RETURN_OK)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +static void optee_release_sys_ctx(struct tee_context *ctx)
> > +{
> > + struct optee *optee = tee_get_drvdata(ctx->teedev);
> > + struct arm_smccc_res res;
> > +
> > + if (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SYSTEM_THREAD)
> > + optee->smc.invoke_fn(OPTEE_SMC_CALL_UNRESERVE_SYS_THREAD,
> > + 0, 0, 0, 0, 0, 0, 0, &res);
> > +}
> > +
> > static const struct tee_driver_ops optee_clnt_ops = {
> > .get_version = optee_get_version,
> > .open = optee_smc_open,
> > @@ -1095,6 +1125,8 @@ static const struct tee_driver_ops optee_clnt_ops = {
> > .cancel_req = optee_cancel_req,
> > .shm_register = optee_shm_register,
> > .shm_unregister = optee_shm_unregister,
> > + .system_ctx_request = optee_request_sys_ctx,
> > + .system_ctx_release = optee_release_sys_ctx,
> > };
> >
> > static const struct tee_desc optee_clnt_desc = {
> > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > index 98da206cd761..a7dfdea5d85b 100644
> > --- a/drivers/tee/tee_core.c
> > +++ b/drivers/tee/tee_core.c
> > @@ -5,6 +5,7 @@
> >
> > #define pr_fmt(fmt) "%s: " fmt, __func__
> >
> > +#include <linux/bug.h>
> > #include <linux/cdev.h>
> > #include <linux/cred.h>
> > #include <linux/fs.h>
> > @@ -1141,10 +1142,39 @@ EXPORT_SYMBOL_GPL(tee_client_open_context);
> >
> > void tee_client_close_context(struct tee_context *ctx)
> > {
> > + while (ctx->system_ctx_count)
> > + tee_client_release_system_context(ctx);
> > +
> > teedev_close_context(ctx);
> > }
> > EXPORT_SYMBOL_GPL(tee_client_close_context);
> >
> > +int tee_client_request_system_context(struct tee_context *ctx)
> > +{
> > + int ret;
> > +
> > + if (!ctx->teedev->desc->ops->system_ctx_request ||
> > + !ctx->teedev->desc->ops->system_ctx_release)
> > + return -EINVAL;
> > +
> > + ret = ctx->teedev->desc->ops->system_ctx_request(ctx);
> > + if (!ret)
> > + ctx->system_ctx_count++;
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(tee_client_request_system_context);
> > +
> > +void tee_client_release_system_context(struct tee_context *ctx)
> > +{
> > + if (ctx->system_ctx_count &&
> > + ctx->teedev->desc->ops->system_ctx_release) {
> > + ctx->teedev->desc->ops->system_ctx_release(ctx);
> > + ctx->system_ctx_count--;
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(tee_client_release_system_context);
> > +
> > void tee_client_get_version(struct tee_context *ctx,
> > struct tee_ioctl_version_data *vers)
> > {
> > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> > index 17eb1c5205d3..45577256bb71 100644
> > --- a/include/linux/tee_drv.h
> > +++ b/include/linux/tee_drv.h
> > @@ -47,6 +47,8 @@ struct tee_shm_pool;
> > * non-blocking in nature.
> > * @cap_memref_null: flag indicating if the TEE Client support shared
> > * memory buffer with a NULL pointer.
> > + * @system_ctx_count: Number of system invocation contexts provisioned for
> > + * this TEE client or 0.
> > */
> > struct tee_context {
> > struct tee_device *teedev;
> > @@ -55,6 +57,7 @@ struct tee_context {
> > bool releasing;
> > bool supp_nowait;
> > bool cap_memref_null;
> > + unsigned int system_ctx_count;
>
> Does it make sense for a tee_context to reserve more than one system
> context/thread?

A single Linux driver optee context (opened from
tee_client_open_context()) can be used for concurrent invocations of
TEE hence the implementation here supporting a linux context to
reserve several OP-TEE thread contexts.

> I wonder if keeping track on this shouldn't be done in a
> driver specific struct, perhaps struct optee_context_data for OP-TEE.

Yes, I can move that to optee private data since how TEE drivers
handle this are specific to their TEE.

>
> > };
> >
> > struct tee_param_memref {
> > @@ -90,6 +93,8 @@ struct tee_param {
> > * @supp_send: called for supplicant to send a response
> > * @shm_register: register shared memory buffer in TEE
> > * @shm_unregister: unregister shared memory buffer in TEE
> > + * @system_ctx_request: Request provisioning of a new system context in TEE
> > + * @system_ctx_release: Release a provisioned system context in TEE
> > */
> > struct tee_driver_ops {
> > void (*get_version)(struct tee_device *teedev,
> > @@ -112,6 +117,8 @@ struct tee_driver_ops {
> > struct page **pages, size_t num_pages,
> > unsigned long start);
> > int (*shm_unregister)(struct tee_context *ctx, struct tee_shm *shm);
> > + int (*system_ctx_request)(struct tee_context *ctx);
> > + void (*system_ctx_release)(struct tee_context *ctx);
> > };
> >
> > /**
> > @@ -397,6 +404,20 @@ tee_client_open_context(struct tee_context *start,
> > */
> > void tee_client_close_context(struct tee_context *ctx);
> >
> > +/**
> > + * tee_client_request_system_context() - Close a TEE context
>
> Copy & paste error
>
> > + * @ctx: TEE context to close
> > + *
> > + * @return 0 on success else an error code
> > + */
> > +int tee_client_request_system_context(struct tee_context *ctx);
> > +
> > +/**
> > + * tee_client_release_system_context() - Release a reserved system exec context
>
> Please drop "exec", it's not mentioned anywhere else.

Ok.

Etienne

>
> Thanks,
> Jens
>
> > + * @ctx: TEE context reference
> > + */
> > +void tee_client_release_system_context(struct tee_context *ctx);
> > +
> > /**
> > * tee_client_get_version() - Query version of TEE
> > * @ctx: TEE context to TEE to query
> > --
> > 2.25.1
> >