Re: [PATCH edk2-platforms v3 1/2] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver

["Ilias Apalodimas" <ilias.apalodimas@linaro.org>]

Hi Sami,

On Tue, 2 Feb 2021 at 17:13, Sami Mujawar <Sami.Mujawar@arm.com> wrote:
>
> Hi Ilias,
>
> Please see my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> -----Original Message-----
> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Sent: 02 February 2021 02:50 PM
> To: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Sughosh Ganu <sughosh.ganu@linaro.org>; devel@edk2.groups.io; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Leif Lindholm <leif@nuviainc.com>; Sahil Malhotra <sahil.malhotra@linaro.org>; nd <nd@arm.com>
> Subject: Re: [PATCH edk2-platforms v3 1/2] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver
>
> Hi Sami,
>
> Inlining some additional info on my explanation.
>
> > > >
> [...]
> > > > I actually picked up the error handling from the previous non-FFA code.
> > > > I'll check what's on Sughosh latest patches and fix it if there are
> > > > any differences.
> > > > Looking at it again EFI_BAD_BUFFER_SIZE can change to indicate out of
> > > > memory properly anyway.
> > > >
> > >
> > > Had another look at this. This seems fine if I just change
> > > EFI_BAD_BUFFER_SIZE -> EFI OUT_OF_RESOURCES because OP-TEE is only
> > > using these errors from FFA. Eventually the OP-TEE code that launches
> > > StMM today, will move to FFA and become a separate SP, so that will
> > > naturally be handled once that's done. I don't see a point of adding
> > > unused error cases.
> > >
> > > [SAMI] Referring to the FFA specification, DEN0077A, v1.0, section 10.2 FFA_MSG_SEND_DIRECT_REQ and Table 10.8: FFA_ERROR encoding, I think the
> > > error codes being handled above would be returned in SvcArgs.Arg2.
> >
> > Hmm why ?
> [SAMI] This is for the case where the FFA_MSG_SEND_DIRECT_REQ does not reach the target endpoint.
> [/SAMI]
> >
> > > The message flow would be as follows:
> > >     - Caller sends FFA_MSG_SEND_DIRECT_REQ to the target endpoint.
> > >     - if the message does not reach the target endpoint, an error code from Table 10.8 may be returned in w2 (i.e. SvcArgs.Arg2)
> >
> > That would be in the case you have a working TF-A implementation and the
> > message is never dispatched to the endpoint right?
> [SAMI] Yes [/SAMI]
> >
> > The current driver is not implementing the whole range of that spec. The
> > communication between secure/non secure world is still based on the OP-TEE
> > messaging mechanism.
> > The only part that complies to the FFA spec is the communication between the
> > driver itself and OP-TEE.
> > >     - If the message reaches the target endpoint, then callee shall invoke one of the following interfaces:
> > >     * FFA_MSG_SEND_DIRECT_RESP
> >
> > So what's happening here, is that we send an SVC with ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64.
> > The op-tee relevant code is located at ./core/arch/arm/kernel/stmm_sp.c
> > There's 2 things we handle right now on OP-TEE:
> > 1. set the page permissions, after relocating the executable.
> > 2. Read/Write data on our RPMB.
> >
> > In both cases service_compose_direct_resp() is used to construct the response
> > and that set the return value on x3.
>
> What you mention and looking for is the discovery mechanism that FFA
> implements. This is not part of the patches (yet) and that's the reason the
> driver hardcodes mMemMgrId = 3U and mStorageId = 4U.
> In OP-TEE side the only reason we have to fill in x2 with the error code is if
> the request that comes in doesn't match any of these values. But that's never
> the case from this driver yet, since there's no SP discovery mechanism
> implemented to begin with.
>
> Hope that clears it up now.
>
> [SAMI] Thank you for the explanation.  It makes sense. I think a comment describing this rationale would be helpful for someone trying to understand the code.
> [/SAMI]

That's a good idea. I'll add it just before the hardcoded values for
storage and memory IDs. We need to keep in mind that this is evolving
work and some things will change once the majority of FFA has been
implemented. The architecture will be similar, we'll just have to
account for dynamic discovery and memory sharing between secure
partitions. Unfortunately this has many moving pieces at the moment,
so we might as well make the reading easier.

>
> Regards
> /Ilias
> >
> >
> > Regards
> > /Ilias
> >
> > >     * FFA_INTERRUPT
> > >     * FFA_SUCCESS
> > >     This would mean that if the callee responds with FFA_MSG_SEND_DIRECT_RESP, the callee returned error/status code shall be in w/x3-w/x7 (which I think in this case may be in SvcArgs.Arg3).
> > > [/SAMI]
> > >
> > > Regards
> > > /Ilias