Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230505173012.881083-1-etienne.carriere@linaro.org> <20230505173012.881083-2-etienne.carriere@linaro.org>
In-Reply-To: <20230505173012.881083-2-etienne.carriere@linaro.org>
From:   Sumit Garg <sumit.garg@linaro.org>
Date:   Wed, 10 May 2023 15:44:25 +0530
Message-ID: <CAFA6WYPC36vqQLzOfArvy2GnVJAXLwbDGenhBARzJ7-Fri-sVA@mail.gmail.com>
Subject: Re: [PATCH v6 2/4] tee: system session
To:     Etienne Carriere <etienne.carriere@linaro.org>
Cc:     linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        op-tee@lists.trustedfirmware.org,
        Jens Wiklander <jens.wiklander@linaro.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Cristian Marussi <cristian.marussi@arm.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 5 May 2023 at 23:01, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> Adds kernel client API function tee_client_system_session() for a client
> to request a system service entry in TEE context.
>
> 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 one of the consumed TEE thread is freed.
>

s/waiting one/waiting until one/
s/thread/threads/

> Co-developed-by: Jens Wiklander <jens.wiklander@linaro.org>
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> No change since v5
>

With above typos fixes, feel free to add:

Reviewed-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit

> Changes since v4:
> - Changes extracted from "[PATCH v4 1/2] tee: system invocation" and
>   revised with Jens contribution to cover only definition of tee driver
>   new API function tee_client_system_session() for kernel clients to
>   register their session as a system session.
> - Commit message rephrased, including header line changed from
>   "tee: system invocation" to "tee: system session" has the feature
>   relates to system attributes of tee sessions.
>
> Changes since v3:
> - Fixed new SMC funcIDs to reserved/unreserve OP-TEE thread contexts:
>   minor renaming + define as fastcall funcIDs.
> - Moved system_ctx_count from generic struct tee_context to optee's
>   private struct optee_context_data. This changes optee smc_abi.c
>   to release reserved thread contexts when the optee device is released.
> - Fixed inline description comments.
>
> No change since v2
>
> 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/tee_core.c  |  8 ++++++++
>  include/linux/tee_drv.h | 16 ++++++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 0eb342de0b00..91932835d0f7 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -1170,6 +1170,14 @@ int tee_client_close_session(struct tee_context *ctx, u32 session)
>  }
>  EXPORT_SYMBOL_GPL(tee_client_close_session);
>
> +int tee_client_system_session(struct tee_context *ctx, u32 session)
> +{
> +       if (!ctx->teedev->desc->ops->system_session)
> +               return -EINVAL;
> +       return ctx->teedev->desc->ops->system_session(ctx, session);
> +}
> +EXPORT_SYMBOL_GPL(tee_client_system_session);
> +
>  int tee_client_invoke_func(struct tee_context *ctx,
>                            struct tee_ioctl_invoke_arg *arg,
>                            struct tee_param *param)
> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> index 17eb1c5205d3..911ddf92dcee 100644
> --- a/include/linux/tee_drv.h
> +++ b/include/linux/tee_drv.h
> @@ -84,6 +84,7 @@ struct tee_param {
>   * @release:           release this open file
>   * @open_session:      open a new session
>   * @close_session:     close a session
> + * @system_session:    declare session as a system session
>   * @invoke_func:       invoke a trusted function
>   * @cancel_req:                request cancel of an ongoing invoke or open
>   * @supp_recv:         called for supplicant to get a command
> @@ -100,6 +101,7 @@ struct tee_driver_ops {
>                             struct tee_ioctl_open_session_arg *arg,
>                             struct tee_param *param);
>         int (*close_session)(struct tee_context *ctx, u32 session);
> +       int (*system_session)(struct tee_context *ctx, u32 session);
>         int (*invoke_func)(struct tee_context *ctx,
>                            struct tee_ioctl_invoke_arg *arg,
>                            struct tee_param *param);
> @@ -429,6 +431,20 @@ int tee_client_open_session(struct tee_context *ctx,
>   */
>  int tee_client_close_session(struct tee_context *ctx, u32 session);
>
> +/**
> + * tee_client_system_session() - Declare session as a system session
> + * @ctx:       TEE Context
> + * @session:   Session id
> + *
> + * This function requests TEE to provision an entry context ready to use for
> + * that session only. The provisioned entry context is used for command
> + * invocation and session closure, not for command cancelling requests.
> + * TEE releases the provisioned context upon session closure.
> + *
> + * Return < 0 on error else 0 if an entry context has been provisioned.
> + */
> +int tee_client_system_session(struct tee_context *ctx, u32 session);
> +
>  /**
>   * tee_client_invoke_func() - Invoke a function in a Trusted Application
>   * @ctx:       TEE Context
> --
> 2.25.1
>