From: Sumit Garg <sumit.garg@linaro.org>
To: Jens Wiklander <jens.wiklander@linaro.org>
Cc: linux-kernel@vger.kernel.org, op-tee@lists.trustedfirmware.org,
Herbert Xu <herbert@gondor.apana.org.au>,
Devaraj Rangasamy <Devaraj.Rangasamy@amd.com>,
Rijo Thomas <Rijo-john.Thomas@amd.com>,
David Howells <dhowells@redhat.com>,
Tyler Hicks <tyhicks@linux.microsoft.com>
Subject: Re: [PATCH v3 06/12] optee: add driver private tee_context
Date: Thu, 27 Jan 2022 11:54:26 +0530 [thread overview]
Message-ID: <CAFA6WYNWHtBKU67yWddFVSKt+a3SxNdzu82NUtwcG0y5v-sTLw@mail.gmail.com> (raw)
In-Reply-To: <20220125162938.838382-7-jens.wiklander@linaro.org>
On Tue, 25 Jan 2022 at 21:59, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> Adds a driver private tee_context by moving the tee_context in struct
> optee_notif to struct optee. This tee_context is used when doing
> internal calls to secure world to deliver notification and later also
> when sharing driver private memory with secure world.
>
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
> drivers/tee/optee/core.c | 1 +
> drivers/tee/optee/ffa_abi.c | 61 ++++++++++++++++++-------------
> drivers/tee/optee/optee_private.h | 5 ++-
> drivers/tee/optee/smc_abi.c | 40 ++++++--------------
> 4 files changed, 51 insertions(+), 56 deletions(-)
>
Reviewed-by: Sumit Garg <sumit.garg@linaro.org>
-Sumit
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 2a369e346b85..f4bccb5f0e93 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -161,6 +161,7 @@ void optee_remove_common(struct optee *optee)
> optee_unregister_devices();
>
> optee_notif_uninit(optee);
> + teedev_close_context(optee->ctx);
> /*
> * The two devices have to be unregistered before we can free the
> * other resources.
> diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> index d71880ede6d6..954e88866968 100644
> --- a/drivers/tee/optee/ffa_abi.c
> +++ b/drivers/tee/optee/ffa_abi.c
> @@ -765,7 +765,9 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> {
> const struct ffa_dev_ops *ffa_ops;
> unsigned int rpc_arg_count;
> + struct tee_shm_pool *pool;
> struct tee_device *teedev;
> + struct tee_context *ctx;
> struct optee *optee;
> int rc;
>
> @@ -785,12 +787,12 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> if (!optee)
> return -ENOMEM;
>
> - optee->pool = optee_ffa_shm_pool_alloc_pages();
> - if (IS_ERR(optee->pool)) {
> - rc = PTR_ERR(optee->pool);
> - optee->pool = NULL;
> - goto err;
> + pool = optee_ffa_shm_pool_alloc_pages();
> + if (IS_ERR(pool)) {
> + rc = PTR_ERR(pool);
> + goto err_free_optee;
> }
> + optee->pool = pool;
>
> optee->ops = &optee_ffa_ops;
> optee->ffa.ffa_dev = ffa_dev;
> @@ -801,7 +803,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> optee);
> if (IS_ERR(teedev)) {
> rc = PTR_ERR(teedev);
> - goto err;
> + goto err_free_pool;
> }
> optee->teedev = teedev;
>
> @@ -809,50 +811,57 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> optee);
> if (IS_ERR(teedev)) {
> rc = PTR_ERR(teedev);
> - goto err;
> + goto err_unreg_teedev;
> }
> optee->supp_teedev = teedev;
>
> rc = tee_device_register(optee->teedev);
> if (rc)
> - goto err;
> + goto err_unreg_supp_teedev;
>
> rc = tee_device_register(optee->supp_teedev);
> if (rc)
> - goto err;
> + goto err_unreg_supp_teedev;
>
> rc = rhashtable_init(&optee->ffa.global_ids, &shm_rhash_params);
> if (rc)
> - goto err;
> + goto err_unreg_supp_teedev;
> mutex_init(&optee->ffa.mutex);
> mutex_init(&optee->call_queue.mutex);
> INIT_LIST_HEAD(&optee->call_queue.waiters);
> optee_supp_init(&optee->supp);
> ffa_dev_set_drvdata(ffa_dev, optee);
> + ctx = teedev_open(optee->teedev);
> + if (IS_ERR(ctx))
> + goto err_rhashtable_free;
> + optee->ctx = ctx;
> rc = optee_notif_init(optee, OPTEE_DEFAULT_MAX_NOTIF_VALUE);
> - if (rc) {
> - optee_ffa_remove(ffa_dev);
> - return rc;
> - }
> + if (rc)
> + goto err_close_ctx;
>
> rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES);
> - if (rc) {
> - optee_ffa_remove(ffa_dev);
> - return rc;
> - }
> + if (rc)
> + goto err_unregister_devices;
>
> pr_info("initialized driver\n");
> return 0;
> -err:
> - /*
> - * tee_device_unregister() is safe to call even if the
> - * devices hasn't been registered with
> - * tee_device_register() yet.
> - */
> +
> +err_unregister_devices:
> + optee_unregister_devices();
> + optee_notif_uninit(optee);
> +err_close_ctx:
> + teedev_close_context(ctx);
> +err_rhashtable_free:
> + rhashtable_free_and_destroy(&optee->ffa.global_ids, rh_free_fn, NULL);
> + optee_supp_uninit(&optee->supp);
> + mutex_destroy(&optee->call_queue.mutex);
> +err_unreg_supp_teedev:
> tee_device_unregister(optee->supp_teedev);
> +err_unreg_teedev:
> tee_device_unregister(optee->teedev);
> - if (optee->pool)
> - tee_shm_pool_free(optee->pool);
> +err_free_pool:
> + tee_shm_pool_free(pool);
> +err_free_optee:
> kfree(optee);
> return rc;
> }
> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> index df2450921464..df3a483bbf46 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -53,7 +53,6 @@ struct optee_call_queue {
>
> struct optee_notif {
> u_int max_key;
> - struct tee_context *ctx;
> /* Serializes access to the elements below in this struct */
> spinlock_t lock;
> struct list_head db;
> @@ -134,9 +133,10 @@ struct optee_ops {
> /**
> * struct optee - main service struct
> * @supp_teedev: supplicant device
> + * @teedev: client device
> * @ops: internal callbacks for different ways to reach secure
> * world
> - * @teedev: client device
> + * @ctx: driver internal TEE context
> * @smc: specific to SMC ABI
> * @ffa: specific to FF-A ABI
> * @call_queue: queue of threads waiting to call @invoke_fn
> @@ -152,6 +152,7 @@ struct optee {
> struct tee_device *supp_teedev;
> struct tee_device *teedev;
> const struct optee_ops *ops;
> + struct tee_context *ctx;
> union {
> struct optee_smc smc;
> struct optee_ffa ffa;
> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> index 66d5d1b56b95..b1082a34cda2 100644
> --- a/drivers/tee/optee/smc_abi.c
> +++ b/drivers/tee/optee/smc_abi.c
> @@ -952,57 +952,34 @@ static irqreturn_t notif_irq_thread_fn(int irq, void *dev_id)
> {
> struct optee *optee = dev_id;
>
> - optee_smc_do_bottom_half(optee->notif.ctx);
> + optee_smc_do_bottom_half(optee->ctx);
>
> return IRQ_HANDLED;
> }
>
> static int optee_smc_notif_init_irq(struct optee *optee, u_int irq)
> {
> - struct tee_context *ctx;
> int rc;
>
> - ctx = teedev_open(optee->teedev);
> - if (IS_ERR(ctx))
> - return PTR_ERR(ctx);
> -
> - optee->notif.ctx = ctx;
> rc = request_threaded_irq(irq, notif_irq_handler,
> notif_irq_thread_fn,
> 0, "optee_notification", optee);
> if (rc)
> - goto err_close_ctx;
> + return rc;
>
> optee->smc.notif_irq = irq;
>
> return 0;
> -
> -err_close_ctx:
> - teedev_close_context(optee->notif.ctx);
> - optee->notif.ctx = NULL;
> -
> - return rc;
> }
>
> static void optee_smc_notif_uninit_irq(struct optee *optee)
> {
> - if (optee->notif.ctx) {
> - optee_smc_stop_async_notif(optee->notif.ctx);
> + if (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_ASYNC_NOTIF) {
> + optee_smc_stop_async_notif(optee->ctx);
> if (optee->smc.notif_irq) {
> free_irq(optee->smc.notif_irq, optee);
> irq_dispose_mapping(optee->smc.notif_irq);
> }
> -
> - /*
> - * The thread normally working with optee->notif.ctx was
> - * stopped with free_irq() above.
> - *
> - * Note we're not using teedev_close_context() or
> - * tee_client_close_context() since we have already called
> - * tee_device_put() while initializing to avoid a circular
> - * reference counting.
> - */
> - teedev_close_context(optee->notif.ctx);
> }
> }
>
> @@ -1307,6 +1284,7 @@ static int optee_probe(struct platform_device *pdev)
> struct optee *optee = NULL;
> void *memremaped_shm = NULL;
> struct tee_device *teedev;
> + struct tee_context *ctx;
> u32 max_notif_value;
> u32 sec_caps;
> int rc;
> @@ -1387,9 +1365,13 @@ static int optee_probe(struct platform_device *pdev)
> optee->pool = pool;
>
> platform_set_drvdata(pdev, optee);
> + ctx = teedev_open(optee->teedev);
> + if (IS_ERR(ctx))
> + goto err_supp_uninit;
> + optee->ctx = ctx;
> rc = optee_notif_init(optee, max_notif_value);
> if (rc)
> - goto err_supp_uninit;
> + goto err_close_ctx;
>
> if (sec_caps & OPTEE_SMC_SEC_CAP_ASYNC_NOTIF) {
> unsigned int irq;
> @@ -1437,6 +1419,8 @@ static int optee_probe(struct platform_device *pdev)
> optee_unregister_devices();
> err_notif_uninit:
> optee_notif_uninit(optee);
> +err_close_ctx:
> + teedev_close_context(ctx);
> err_supp_uninit:
> optee_supp_uninit(&optee->supp);
> mutex_destroy(&optee->call_queue.mutex);
> --
> 2.31.1
>
next prev parent reply other threads:[~2022-01-27 6:24 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-25 16:29 [PATCH v3 00/12] tee: shared memory updates Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 01/12] hwrng: optee-rng: use tee_shm_alloc_kernel_buf() Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 02/12] tee: remove unused tee_shm_pool_alloc_res_mem() Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 03/12] tee: add tee_shm_alloc_user_buf() Jens Wiklander
2022-01-27 5:56 ` Sumit Garg
2022-01-27 8:09 ` Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 04/12] tee: simplify shm pool handling Jens Wiklander
2022-01-27 6:09 ` Sumit Garg
2022-01-27 8:27 ` Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 05/12] tee: replace tee_shm_alloc() Jens Wiklander
2022-01-27 6:16 ` Sumit Garg
2022-01-27 8:31 ` Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 06/12] optee: add driver private tee_context Jens Wiklander
2022-01-27 6:24 ` Sumit Garg [this message]
2022-01-25 16:29 ` [PATCH v3 07/12] optee: use driver internal tee_contex for some rpc Jens Wiklander
2022-01-27 6:32 ` Sumit Garg
2022-01-27 12:39 ` Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 08/12] optee: add optee_pool_op_free_helper() Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 09/12] tee: add tee_shm_register_{user,kernel}_buf() Jens Wiklander
2022-01-27 6:35 ` Sumit Garg
2022-01-25 16:29 ` [PATCH v3 10/12] KEYS: trusted: tee: use tee_shm_register_kernel_buf() Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 11/12] tee: replace tee_shm_register() Jens Wiklander
2022-01-27 6:59 ` Sumit Garg
2022-01-27 13:00 ` Jens Wiklander
2022-01-25 16:29 ` [PATCH v3 12/12] tee: refactor TEE_SHM_* flags Jens Wiklander
2022-01-27 7:05 ` Sumit Garg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFA6WYNWHtBKU67yWddFVSKt+a3SxNdzu82NUtwcG0y5v-sTLw@mail.gmail.com \
--to=sumit.garg@linaro.org \
--cc=Devaraj.Rangasamy@amd.com \
--cc=Rijo-john.Thomas@amd.com \
--cc=dhowells@redhat.com \
--cc=herbert@gondor.apana.org.au \
--cc=jens.wiklander@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=op-tee@lists.trustedfirmware.org \
--cc=tyhicks@linux.microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).