Messages in this thread | | | From | Jens Wiklander <> | Date | Tue, 7 Feb 2023 11:36:25 +0100 | Subject | Re: [PATCH 1/2] tee: system invocation |
| |
On Tue, Feb 7, 2023 at 10:52 AM Sumit Garg <sumit.garg@linaro.org> wrote: > > On Tue, 7 Feb 2023 at 14:38, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > > > Hi, > > > > On Tue, Feb 7, 2023 at 8:27 AM Sumit Garg <sumit.garg@linaro.org> wrote: > > > > > > Hi Etienne, > > > > > > On Mon, 30 Jan 2023 at 15:12, Etienne Carriere > > > <etienne.carriere@linaro.org> wrote: > > > > > > > > Adds TEE context flag sys_service to be enabled for invocation contexts > > > > that should used TEE provisioned system resources. OP-TEE SMC ABI entry > > > > > > s/used/use/ > > > > > > > rely this information to use a dedicated entry function to request > > > > allocation of a system thread from a dedicated system context pool. > > > > > > > > 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 remove service (RPC) to access an eMMC RPMB partition and > > > > > > s/remove/remote/ > > > > > > > the eMMC device is supplied by an OP-TEE SCMI regulator. > > > > > > > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> > > > > --- > > > > drivers/tee/optee/optee_smc.h | 14 +++++++++++--- > > > > drivers/tee/optee/smc_abi.c | 6 +++++- > > > > include/linux/tee_drv.h | 4 ++++ > > > > 3 files changed, 20 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/tee/optee/optee_smc.h b/drivers/tee/optee/optee_smc.h > > > > index 73b5e7760d10..7c7eedf183c5 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,9 @@ 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 > > > > + > > > > /* > > > > * 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..513038a138f6 100644 > > > > --- a/drivers/tee/optee/smc_abi.c > > > > +++ b/drivers/tee/optee/smc_abi.c > > > > @@ -889,7 +889,11 @@ 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->sys_service && > > > > + (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SYSTEM_THREAD)) > > > > + param.a0 = OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG; > > > > + else > > > > + param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG; > > > > > > This system thread flag should also be applicable to platforms without > > > registered arguments support. IOW, we need similar equivalents for > > > OPTEE_SMC_FUNCID_CALL_WITH_ARG and OPTEE_SMC_FUNCID_CALL_WITH_RPC_ARG > > > too. So I would rather suggest that we add following flag to all 3 > > > call types: > > > > > > #define OPTEE_SMC_CALL_SYSTEM_THREAD_FLAG 0x8000 > > > > The main reason platforms don't support registered arguments is that > > they haven't been updated since this was introduced. So if a platform > > needs system threads it could update to use registered arguments too. > > Are we hinting at deprecating reserved shared memory support? If yes, > wouldn't it be better to be explicit about it with a boot time warning > message about its deprecation? > > Otherwise it will be difficult to debug for the end user to find out > why system thread support isn't activated. > > > The Linux kernel already supports registered arguments. An advantage > > with the current approach is that the ABI is easier to implement > > since we have distinct SMC IDs for each function. > > I see your point but my initial thought was that we don't end up > making that list too large that it becomes cumbersome to maintain, > involving all the combinatorial.
You have a point. Etienne, do you think we could give it a try at https://github.com/OP-TEE/optee_os/pull/5789 to better see how this would play out?
Cheers, Jens
| |