| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHUa44G1yQtgF1eAUJVA+wtctKHfqYFBhs0PBnpoN-gD8_x8eg@mail.gmail.com>
Date: Tue, 7 Feb 2023 11:36:25 +0100
From: Jens Wiklander <jens.wiklander@...aro.org>
To: Sumit Garg <sumit.garg@...aro.org>,
Etienne Carriere <etienne.carriere@...aro.org>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
Sudeep Holla <sudeep.holla@....com>,
Cristian Marussi <cristian.marussi@....com>
Subject: Re: [PATCH 1/2] tee: system invocation
On Tue, Feb 7, 2023 at 10:52 AM Sumit Garg <sumit.garg@...aro.org> wrote:
>
> On Tue, 7 Feb 2023 at 14:38, Jens Wiklander <jens.wiklander@...aro.org> wrote:
> >
> > Hi,
> >
> > On Tue, Feb 7, 2023 at 8:27 AM Sumit Garg <sumit.garg@...aro.org> wrote:
> > >
> > > Hi Etienne,
> > >
> > > On Mon, 30 Jan 2023 at 15:12, Etienne Carriere
> > > <etienne.carriere@...aro.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@...aro.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
Powered by blists - more mailing lists