Message-ID: <f334ae8c-2f42-4177-9d18-5274d1ec91b1@quicinc.com>
Date: Thu, 28 Nov 2024 08:02:54 +1100
From: Amirreza Zarrabi <quic_azarrabi@...cinc.com>
To: Jens Wiklander <jens.wiklander@...aro.org>,
        Sumit Garg
	<sumit.garg@...aro.org>
CC: <op-tee@...ts.trustedfirmware.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH RFC 3/3] tee: introduce orphan tee_shm and default context



On 11/27/2024 6:22 PM, Jens Wiklander wrote:
> On Wed, Nov 27, 2024 at 7:02 AM Sumit Garg <sumit.garg@...aro.org> wrote:
>>
>> On Tue, 26 Nov 2024 at 20:52, Jens Wiklander <jens.wiklander@...aro.org> wrote:
>>>
>>> On Tue, Nov 26, 2024 at 1:27 PM Sumit Garg <sumit.garg@...aro.org> wrote:
>>>>
>>>> On Tue, 26 Nov 2024 at 14:03, Jens Wiklander <jens.wiklander@...aro.org> wrote:
>>>>>
>>>>> On Mon, Nov 25, 2024 at 9:55 PM Amirreza Zarrabi
>>>>> <quic_azarrabi@...cinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 11/25/2024 6:51 PM, Sumit Garg wrote:
>>>>>>> On Mon, 25 Nov 2024 at 12:53, Jens Wiklander <jens.wiklander@...aro.org> wrote:
>>>>>>>>
>>>>>>>> On Mon, Nov 25, 2024 at 7:14 AM Sumit Garg <sumit.garg@...aro.org> wrote:
>>>>>>>>>
>>>>>>>>> On Mon, 25 Nov 2024 at 03:00, Amirreza Zarrabi
>>>>>>>>> <quic_azarrabi@...cinc.com> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi Sumit,
>>>>>>>>>>
>>>>>>>>>> Thank you so much for the comemnts :).
>>>>>>>>>>
>>>>>>>>>> On 11/23/2024 9:32 PM, Sumit Garg wrote:
>>>>>>>>>>> Hi Amirreza,
>>>>>>>>>>>
>>>>>>>>>>> Thanks for proposing this.
>>>>>>>>>>>
>>>>>>>>>>> On Fri, 22 Nov 2024 at 06:38, Amirreza Zarrabi
>>>>>>>>>>> <quic_azarrabi@...cinc.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 11/21/2024 11:08 PM, Jens Wiklander wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Jens,
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Amirreza,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, Nov 21, 2024 at 2:37 AM Amirreza Zarrabi
>>>>>>>>>>>>> <quic_azarrabi@...cinc.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The default context has a lifespan similar to the tee_device.
>>>>>>>>>>>
>>>>>>>>>>> Since it's associated with tee_device context, let's call it obvious
>>>>>>>>>>> via renaming it as device context instead (s/def_ctx/dev_ctx/ in this
>>>>>>>>>>> patch).
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Make sense, I'll rename it.
>>>>>>>>>>
>>>>>>>>>>>>>> It is used as a context for shared memory if the context to which the
>>>>>>>>>>>>>> shared memory belongs is released, making the tee_shm an orphan.
>>>>>>>>>>>>>> This allows the driver implementing shm_unregister to safely make
>>>>>>>>>>>>>> subsequent calls, such as to a supplicant if needed.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> It also enables users to free the shared memory while the driver is
>>>>>>>>>>>>>> blocked on unregister_tee_device safely.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Preferably, this should be used for all driver internal uses, using
>>>>>>>>>>>>>> teedev_get_def_context rather than calling teedev_open.
>>>>>>>>>>>
>>>>>>>>>>> Makes sense to me.
>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Signed-off-by: Amirreza Zarrabi <quic_azarrabi@...cinc.com>
>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>  drivers/tee/optee/core.c    |  2 +-
>>>>>>>>>>>>>>  drivers/tee/optee/ffa_abi.c |  2 +-
>>>>>>>>>>>>>>  drivers/tee/optee/smc_abi.c |  2 +-
>>>>>>>>>>>>>>  drivers/tee/tee_core.c      | 83 +++++++++++++++++++++++++++++----------------
>>>>>>>>>>>>>>  drivers/tee/tee_private.h   |  3 --
>>>>>>>>>>>>>>  drivers/tee/tee_shm.c       | 18 ++--------
>>>>>>>>>>>>>>  include/linux/tee_core.h    | 15 ++++++++
>>>>>>>>>>>>>>  include/linux/tee_drv.h     |  7 ----
>>>>>>>>>>>>>>  8 files changed, 73 insertions(+), 59 deletions(-)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
>>>>>>>>>>>>>> index c75fddc83576..78d43d0c8014 100644
>>>>>>>>>>>>>> --- a/drivers/tee/optee/core.c
>>>>>>>>>>>>>> +++ b/drivers/tee/optee/core.c
>>>>>>>>>>>>>> @@ -173,7 +173,7 @@ void optee_remove_common(struct optee *optee)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>         optee_notif_uninit(optee);
>>>>>>>>>>>>>>         optee_shm_arg_cache_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 f3af5666bb11..6ad94f0788ad 100644
>>>>>>>>>>>>>> --- a/drivers/tee/optee/ffa_abi.c
>>>>>>>>>>>>>> +++ b/drivers/tee/optee/ffa_abi.c
>>>>>>>>>>>>>> @@ -949,7 +949,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
>>>>>>>>>>>>>>         optee_shm_arg_cache_init(optee, arg_cache_flags);
>>>>>>>>>>>>>>         mutex_init(&optee->rpmb_dev_mutex);
>>>>>>>>>>>>>>         ffa_dev_set_drvdata(ffa_dev, optee);
>>>>>>>>>>>>>> -       ctx = teedev_open(optee->teedev);
>>>>>>>>>>>>>> +       ctx = teedev_get_def_context(optee->teedev);
>>>>>>>>>>>>>>         if (IS_ERR(ctx)) {
>>>>>>>>>>>>>>                 rc = PTR_ERR(ctx);
>>>>>>>>>>>>>>                 goto err_rhashtable_free;
>>>>>>>>>>>>>> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
>>>>>>>>>>>>>> index e9456e3e74cc..c77a3e631d04 100644
>>>>>>>>>>>>>> --- a/drivers/tee/optee/smc_abi.c
>>>>>>>>>>>>>> +++ b/drivers/tee/optee/smc_abi.c
>>>>>>>>>>>>>> @@ -1722,7 +1722,7 @@ static int optee_probe(struct platform_device *pdev)
>>>>>>>>>>>>>>         mutex_init(&optee->rpmb_dev_mutex);
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>         platform_set_drvdata(pdev, optee);
>>>>>>>>>>>>>> -       ctx = teedev_open(optee->teedev);
>>>>>>>>>>>>>> +       ctx = teedev_get_def_context(optee->teedev);
>>>>>>>>>>>>>>         if (IS_ERR(ctx)) {
>>>>>>>>>>>>>>                 rc = PTR_ERR(ctx);
>>>>>>>>>>>>>>                 goto err_supp_uninit;
>>>>>>>>>>>>>> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
>>>>>>>>>>>>>> index 93f3b330aec8..805e1336089d 100644
>>>>>>>>>>>>>> --- a/drivers/tee/tee_core.c
>>>>>>>>>>>>>> +++ b/drivers/tee/tee_core.c
>>>>>>>>>>>>>> @@ -57,7 +57,6 @@ struct tee_context *teedev_open(struct tee_device *teedev)
>>>>>>>>>>>>>>                 goto err;
>>>>>>>>>>>>>>         }
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -       kref_init(&ctx->refcount);
>>>>>>>>>>>>>>         ctx->teedev = teedev;
>>>>>>>>>>>>>>         INIT_LIST_HEAD(&ctx->list_shm);
>>>>>>>>>>>>>>         rc = teedev->desc->ops->open(ctx);
>>>>>>>>>>>>>> @@ -73,36 +72,43 @@ struct tee_context *teedev_open(struct tee_device *teedev)
>>>>>>>>>>>>>>  }
>>>>>>>>>>>>>>  EXPORT_SYMBOL_GPL(teedev_open);
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -void teedev_ctx_get(struct tee_context *ctx)
>>>>>>>>>>>>>> +struct tee_context *teedev_get_def_context(struct tee_device *teedev)
>>>>>>>>>>>>>>  {
>>>>>>>>>>>>>> -       if (ctx->releasing)
>>>>>>>>>>>>>> -               return;
>>>>>>>>>>>>>> +       int rc;
>>>>>>>>>>>>>> +       struct tee_context *ctx = &teedev->def_ctx;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -       kref_get(&ctx->refcount);
>>>>>>>>>>>>>> -}
>>>>>>>>>>>>>> +       ctx->teedev = teedev;
>>>>>>>>>>>>>> +       INIT_LIST_HEAD(&ctx->list_shm);
>>>>>>>>>>>>>> +       rc = teedev->desc->ops->open(ctx);
>>>>>>>>>>>>>> +       if (rc)
>>>>>>>>>>>>>> +               return ERR_PTR(rc);
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think ctx->teedev and ctx->list_shm must always be initialized or
>>>>>>>>>>>>> &teedev->def_ctx can't be used in teedev_close_context().
>>>>>>>>>>>>
>>>>>>>>>>>> True, but &teedev->def_ctx is never used in teedev_close_context().
>>>>>>>>>>>> The closing of the &teedev->def_ctx simply ignored. So once opened,
>>>>>>>>>>>> &teedev->def_ctx will always remain open until the tee_device is alive.
>>>>>>>>>>>>
>>>>>>>>>>>>> We could initialize teedev->def_ctx on the first call to teedev_open()
>>>>>>>>>>>>> on that tee_device. We need a way to tell the
>>>>>>>>>>>>> teedev->desc->ops->open() to the backed driver that it's initializing
>>>>>>>>>>>>> the default context though, or optee_open() can't handle the
>>>>>>>>>>>>> tee-supplicant case properly.
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> That's a good point. This way, it is guaranteed that there is one def_ctx
>>>>>>>>>>>> per teedev. There should be a way to tell the open() callback that it is
>>>>>>>>>>>> a def_ctx, so it is not registered as a supplicant context.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Should we allow this function to be called more than once for each teedev?
>>>>>>>>>>>>
>>>>>>>>>>>> Yes, moving to teedev_open() will fix the issue.
>>>>>>>>>>>>
>>>>>>>>>>>>> Do we need serialization in this function if it's called after the
>>>>>>>>>>>>> driver is probed?
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> True. I'll make sure there is no race.
>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -static void teedev_ctx_release(struct kref *ref)
>>>>>>>>>>>>>> -{
>>>>>>>>>>>>>> -       struct tee_context *ctx = container_of(ref, struct tee_context,
>>>>>>>>>>>>>> -                                              refcount);
>>>>>>>>>>>>>> -       ctx->releasing = true;
>>>>>>>>>>>>>> -       ctx->teedev->desc->ops->release(ctx);
>>>>>>>>>>>>>> -       kfree(ctx);
>>>>>>>>>>>>>> +       return ctx;
>>>>>>>>>>>>>>  }
>>>>>>>>>>>>>> +EXPORT_SYMBOL_GPL(teedev_get_def_context);
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -void teedev_ctx_put(struct tee_context *ctx)
>>>>>>>>>>>>>> +void teedev_close_context(struct tee_context *ctx)
>>>>>>>>>>>>>>  {
>>>>>>>>>>>>>> -       if (ctx->releasing)
>>>>>>>>>>>>>> +       struct tee_device *teedev = ctx->teedev;
>>>>>>>>>>>>>> +       struct tee_shm *shm;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +       if (ctx == &teedev->def_ctx)
>>>>>>>>>>>>>>                 return;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -       kref_put(&ctx->refcount, teedev_ctx_release);
>>>>>>>>>>>>>> -}
>>>>>>>>>>>>>> +       teedev->desc->ops->release(ctx);
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -void teedev_close_context(struct tee_context *ctx)
>>>>>>>>>>>>>> -{
>>>>>>>>>>>>>> -       struct tee_device *teedev = ctx->teedev;
>>>>>>>>>>>>>> +       mutex_lock(&teedev->mutex);
>>>>>>>>>>>>>> +       list_for_each_entry(shm, &ctx->list_shm, link) {
>>>>>>>>>>>>>> +               /* Context released. However, shm still holding a teedev reference.
>>>>>>>>>>>>>> +                * Replace shm->ctx with the default context so that tee_shm_get_from_id()
>>>>>>>>>>>>>> +                * fails (i.e. it is not accessible from userspace) but shm still
>>>>>>>>>>>>>> +                * holds a valid context for further clean up, e.g. shm_unregister().
>>>>>>>>>>>>>> +                */
>>>>>>>>>>>>>
>>>>>>>>>>>>> /*
>>>>>>>>>>>>>  * Please format
>>>>>>>>>>>>>  * multiline comments
>>>>>>>>>>>>>  * like this. Please
>>>>>>>>>>>>>  * keep the lines at
>>>>>>>>>>>>>  * max 80 columns
>>>>>>>>>>>>>  * here and at other
>>>>>>>>>>>>>  * places in the patch-
>>>>>>>>>>>>>  * set.
>>>>>>>>>>>>>  */
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Ack.
>>>>>>>>>>>>
>>>>>>>>>>>>>> +               shm->ctx = &teedev->def_ctx;
>>>>>>>>>>>>>
>>>>>>>>>>>>> shm->ctx will always point to a valid context, even if it is the
>>>>>>>>>>>>> default context. It seems that we can always get hold of the correct
>>>>>>>>>>>>> teedev via shm->ctx->teedev. Do we need "tee: revert removal of
>>>>>>>>>>>>> redundant teedev in struct tee_shm"?
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> It was there in case we wanted to use NULL, but with def_ctx, it is not
>>>>>>>>>>>> necessary. I am withdrawing that commit. :).
>>>>>>>>>>>>
>>>>>>>>>>>>> Shouldn't the shm be removed from the ctx->list_shm and be moved to
>>>>>>>>>>>>> teedev->def_ctx.list_shm?
>>>>>>>>>>>
>>>>>>>>>>> +1
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Ack.
>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Not really. If we put shm in the teedev->def_ctx.list_shm, by the time
>>>>>>>>>>>> we are closing the def_ctx, the list is guaranteed to be empty.
>>>>>>>>>>>>
>>>>>>>>>>>> However, I understand it is cleaner and more consistent to do that rather
>>>>>>>>>>>> than making changes to tee_shm_put().
>>>>>>>>>>>>
>>>>>>>>>>>> I'll do it.
>>>>>>>>>>>>
>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>> +       mutex_unlock(&teedev->mutex);
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -       teedev_ctx_put(ctx);
>>>>>>>>>>>>>> +       kfree(ctx);
>>>>>>>>>>>>>>         tee_device_put(teedev);
>>>>>>>>>>>>>>  }
>>>>>>>>>>>>>>  EXPORT_SYMBOL_GPL(teedev_close_context);
>>>>>>>>>>>>>> @@ -946,6 +952,8 @@ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>         teedev->desc = teedesc;
>>>>>>>>>>>>>>         teedev->pool = pool;
>>>>>>>>>>>>>> +       /* Only open default context when teedev_get_def_context() called. */
>>>>>>>>>>>>>> +       teedev->def_ctx.teedev = NULL;
>>>>>>>>>>>
>>>>>>>>>>> Why don't you open the device context here only? This will associate
>>>>>>>>>>> it automatically with teedev lifespan and then
>>>>>>>>>>> teedev_get_def_context() will just return a reference to that.
>>>>>>>>>>>
>>>>>>>>>>> -Sumit
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> So my assumption is that the tee_devic_alloc() is called as part of
>>>>>>>>>> the driver initialization; there is no guarantee that at this time the
>>>>>>>>>> driver is actually ready to accept any open() callback.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The drivers should be able to handle open() callback since we already
>>>>>>>>> check for !teedesc->ops->open in the beginning of tee_devic_alloc().
>>>>>>>>> Also, we need to open a device context for !TEE_DESC_PRIVILEGED such
>>>>>>>>> that we don't open a supplicant device context there.
>>>>>>>>
>>>>>>>> It would be nice to have the device context fully initialized when the
>>>>>>>> probe function returns. How about adding a "bool is_dev_ctx" to struct
>>>>>>>> tee_context so the open() callback can tell that this is a special
>>>>>>>> tee_contex?
>>>>>>>
>>>>>>> Sure, that will be useful to distinguish the device context from
>>>>>>> normal client context.
>>>>>>>
>>>>>>> -Sumit
>>>>>>>
>>>>>>
>>>>>> So, as far as the open() callback, I do not believe checking if it is not null
>>>>>> is reasonable for calling it here. Most drivers allocate resources and then
>>>>>> initialize them. So, assume these steps for a TEE driver:
>>>>>>  (1) allocate internal data structures,
>>>>>>  (2) allocate the device,
>>>>>>  (3) initialize the internal data structurse and then
>>>>>>  (4) register the device.
>>>>>>
>>>>>> Having these steps for a backend driver means that if you call open() at
>>>>>> step (2), the internal data structures are not ready.
>>>>
>>>> As part of tee_device_alloc(), every driver has to pass "const struct
>>>> tee_desc *teedesc" fully initialized. Which internal data structures
>>>> are you referring too? Is there any upstream example?
>>>
>>> It's reasonable to wait with the open() callback until step 4 above,
>>> which should correspond with the tee_device_register() call. Data
>>> written only once doesn't need serialized access if the fields are
>>> only accessed after they have been fully initialized.
>>
>> Fair enough, I can live with the device context opened after registering it.
>>
>>>
>>>>
>>>>>>
>>>>>> I was originally thinking of going with Jens' suggestion to open dev_ctx in
>>>>>> the teedev_open(), and use a flag to distinguish the type of context for
>>>>>> the open() callback
>>>>>>
>>>>>> What about this:
>>>>>> Open the dev_ctx in the tee_device_register(), at the last step before
>>>>>> setting the TEE_DEVICE_FLAG_REGISTERED flag. Then the open() callback can
>>>>>> check for this flag to determine if it is a normal context or dev_ctx.
>>>>>> If the open() is called while the device has not been registered, it should
>>>>>> handle it differently
>>>>>
>>>>> That makes sense, the driver should be prepared to handle open() calls
>>>>> after tee_device_register() anyway.
>>>>> However, there is no serialization of the flags field in struct
>>>>> tee_device. Hmm, would it be too hacky for the open() callback to
>>>>> check if &ctx->teedev.dev_ctx == ctx? We could add a helper function
>>>>> to wrap that check.
>>>>>
>>>>
>>>> Your suggested change requires every driver to update open() callback
>>>> and later other callbacks may have to support it too. IMHO, only
>>>> teedev_get_dev_ctx() should be able to return a reference to device
>>>> context for usage within the TEE and the implementation driver.
>>>
>>> Yes, but it's only the OP-TEE driver that needs anything special. It
>>> looks like the others can be left unchanged.
>>
>> I suppose it's most likely the upcoming QTEE driver requiring it.
>>
>>>
>>>>
>>>> I am still not able to understand why the following won't work with a
>>>> clear lifetime for the device context?
>>>>
>>>> tee_device_alloc()
>>>>   -> if (!(teedesc->flags & TEE_DESC_PRIVILEGED))
>>>> desc->ops->open(&teedev->dev_ctx);
>>>
>>> We must also have a fully initialized dev_ctx for the supplicant
>>> device.
>>
>> Currently I only see following for OP-TEE driver:
>>
>> ctx = teedev_open(optee->teedev);
>>
>> And I can't see anything like below:
>>
>> ctx = teedev_open(optee->supp_teedev);
>>
>> Where do you think that the dev_ctx is required for a supplicant
>> device? AFAICS, currently opening a context with the supplicant device
>> means that the supplicant daemon is available to handle RPCs which
>> won't be possible during OP-TEE driver probe. Am I missing something?
> 
> One reason for initializing dev_ctx for all tee_devices is in
> teedev_close_context(), where the tee_shms still active are
> transferred to dev_ctx. The teedev member was re-introduced in this
> patch set, but it can be removed again if we can depend on the dev_ctx
> to always be available in teedev_close_context(). Even the
> tee-supplicant may close its tee_context with active tee_shms at some
> point. It might be possible to use half-baked dev_ctx, but then we'd
> be burdened with keeping track of which dev_ctx can be used for what.
> We want as few special cases as possible.
> 

Additionally, Jens suggested checking something like (&ctx->teedev.dev_ctx == ctx)
in the open() callback to ensure that dev_ctx is not accidentally registered as
a supplicant context. This helps avoid the issue you're referring to.

Regards,
Amir

>>
>>> I'd rather delay the open() callback until
>>> tee_device_register() since the dev_ctx is guaranteed not to be needed
>>> before that.
>>
>> Okay, the updated call chain can look like:
>>
>> tee_device_register()
>>   -> if (!(teedev->desc->flags & TEE_DESC_PRIVILEGED))
>>          desc->ops->open(&teedev->dev_ctx);
>>>
>>>>
>>>> tee_device_put()
>>>>   -> if (teedev->dev_ctx) desc->ops->release(&teedev->dev_ctx);
>>>
>>> teedev->dev_ctx is supposed to be embedded in struct tee_device, so
>>> the if isn't needed.
>>
>> I added "if" to cover the case when dev_ctx is not initialized for the
>> supplicant device.
> 
> OK.
> 
> Cheers,
> Jens
> 
>>
>> -Sumit
>>
>>>
>>> Cheers,
>>> Jens
>>>
>>>>
>>>> -Sumit
>>>>
>>>>> Cheers,
>>>>> Jens
>>>>>
>>>>>>
>>>>>> - Amir
>>>>>>
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Jens
>>>>>>>>
>>>>>>>>>
>>>>>>>>> -Sumit

Powered by blists - more mailing lists