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: <a52cd6075f806eed82d577a6d437c705c7f36a26.1683617412.git.Rijo-john.Thomas@amd.com>
 <CAFA6WYNOjkU_4Kg=khuki3qv95f+bUB5hJUjRy2CoPXDvqW0Sw@mail.gmail.com>
In-Reply-To: <CAFA6WYNOjkU_4Kg=khuki3qv95f+bUB5hJUjRy2CoPXDvqW0Sw@mail.gmail.com>
From:   Jens Wiklander <jens.wiklander@linaro.org>
Date:   Tue, 23 May 2023 08:07:12 +0200
Message-ID: <CAHUa44EStHOWapDrbunf-vazBUVdObEXj+gAm6ShWUwx8AUr9g@mail.gmail.com>
Subject: Re: [PATCH v2 1/1] tee: amdtee: Add return_origin to 'struct tee_cmd_load_ta'
To:     Sumit Garg <sumit.garg@linaro.org>
Cc:     Rijo Thomas <Rijo-john.Thomas@amd.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Jan Dabros <jsd@semihalf.com>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Jarkko Nikula <jarkko.nikula@linux.intel.com>,
        op-tee@lists.trustedfirmware.org, linux-kernel@vger.kernel.org,
        Mythri PK <Mythri.Pandeshwarakrishna@amd.com>,
        Devaraj Rangasamy <Devaraj.Rangasamy@amd.com>,
        stable@vger.kernel.org, Sourabh Das <sourabh.das@amd.com>,
        Nimesh Easow <nimesh.easow@amd.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, May 10, 2023 at 1:19=E2=80=AFPM Sumit Garg <sumit.garg@linaro.org> =
wrote:
>
> On Tue, 9 May 2023 at 13:03, Rijo Thomas <Rijo-john.Thomas@amd.com> wrote=
:
> >
> > After TEE has completed processing of TEE_CMD_ID_LOAD_TA, set proper
> > value in 'return_origin' argument passed by open_session() call. To do
> > so, add 'return_origin' field to the structure tee_cmd_load_ta. The
> > Trusted OS shall update return_origin as part of TEE processing.
> >
> > This change to 'struct tee_cmd_load_ta' interface requires a similar up=
date
> > in AMD-TEE Trusted OS's TEE_CMD_ID_LOAD_TA interface.
> >
> > This patch has been verified on Phoenix Birman setup. On older APUs,
> > return_origin value will be 0.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
> > Tested-by: Sourabh Das <sourabh.das@amd.com>
> > Signed-off-by: Rijo Thomas <Rijo-john.Thomas@amd.com>
> > ---
> > v2:
> >  * Added Fixes tag.
> >
> >  drivers/tee/amdtee/amdtee_if.h | 10 ++++++----
> >  drivers/tee/amdtee/call.c      | 30 +++++++++++++++++-------------
> >  2 files changed, 23 insertions(+), 17 deletions(-)
> >
>
> Acked-by: Sumit Garg <sumit.garg@linaro.org>

I'm picking up this.

Thanks,
Jens

>
> -Sumit
>
> > diff --git a/drivers/tee/amdtee/amdtee_if.h b/drivers/tee/amdtee/amdtee=
_if.h
> > index ff48c3e47375..e2014e21530a 100644
> > --- a/drivers/tee/amdtee/amdtee_if.h
> > +++ b/drivers/tee/amdtee/amdtee_if.h
> > @@ -118,16 +118,18 @@ struct tee_cmd_unmap_shared_mem {
> >
> >  /**
> >   * struct tee_cmd_load_ta - load Trusted Application (TA) binary into =
TEE
> > - * @low_addr:    [in] bits [31:0] of the physical address of the TA bi=
nary
> > - * @hi_addr:     [in] bits [63:32] of the physical address of the TA b=
inary
> > - * @size:        [in] size of TA binary in bytes
> > - * @ta_handle:   [out] return handle of the loaded TA
> > + * @low_addr:       [in] bits [31:0] of the physical address of the TA=
 binary
> > + * @hi_addr:        [in] bits [63:32] of the physical address of the T=
A binary
> > + * @size:           [in] size of TA binary in bytes
> > + * @ta_handle:      [out] return handle of the loaded TA
> > + * @return_origin:  [out] origin of return code after TEE processing
> >   */
> >  struct tee_cmd_load_ta {
> >         u32 low_addr;
> >         u32 hi_addr;
> >         u32 size;
> >         u32 ta_handle;
> > +       u32 return_origin;
> >  };
> >
> >  /**
> > diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c
> > index e8cd9aaa3467..e9b63dcb3194 100644
> > --- a/drivers/tee/amdtee/call.c
> > +++ b/drivers/tee/amdtee/call.c
> > @@ -423,19 +423,23 @@ int handle_load_ta(void *data, u32 size, struct t=
ee_ioctl_open_session_arg *arg)
> >         if (ret) {
> >                 arg->ret_origin =3D TEEC_ORIGIN_COMMS;
> >                 arg->ret =3D TEEC_ERROR_COMMUNICATION;
> > -       } else if (arg->ret =3D=3D TEEC_SUCCESS) {
> > -               ret =3D get_ta_refcount(load_cmd.ta_handle);
> > -               if (!ret) {
> > -                       arg->ret_origin =3D TEEC_ORIGIN_COMMS;
> > -                       arg->ret =3D TEEC_ERROR_OUT_OF_MEMORY;
> > -
> > -                       /* Unload the TA on error */
> > -                       unload_cmd.ta_handle =3D load_cmd.ta_handle;
> > -                       psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,
> > -                                           (void *)&unload_cmd,
> > -                                           sizeof(unload_cmd), &ret);
> > -               } else {
> > -                       set_session_id(load_cmd.ta_handle, 0, &arg->ses=
sion);
> > +       } else {
> > +               arg->ret_origin =3D load_cmd.return_origin;
> > +
> > +               if (arg->ret =3D=3D TEEC_SUCCESS) {
> > +                       ret =3D get_ta_refcount(load_cmd.ta_handle);
> > +                       if (!ret) {
> > +                               arg->ret_origin =3D TEEC_ORIGIN_COMMS;
> > +                               arg->ret =3D TEEC_ERROR_OUT_OF_MEMORY;
> > +
> > +                               /* Unload the TA on error */
> > +                               unload_cmd.ta_handle =3D load_cmd.ta_ha=
ndle;
> > +                               psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_T=
A,
> > +                                                   (void *)&unload_cmd=
,
> > +                                                   sizeof(unload_cmd),=
 &ret);
> > +                       } else {
> > +                               set_session_id(load_cmd.ta_handle, 0, &=
arg->session);
> > +                       }
> >                 }
> >         }
> >         mutex_unlock(&ta_refcount_mutex);
> > --
> > 2.25.1
> >