Hi Ilias, Please see my response inline marked [SAMI].
Regards, Sami Mujawar -----Original Message----- From: Ilias Apalodimas <ilias.apalodi...@linaro.org> Sent: 01 February 2021 02:01 PM To: Sami Mujawar <sami.muja...@arm.com> Cc: Sughosh Ganu <sughosh.g...@linaro.org>; devel@edk2.groups.io; Ard Biesheuvel <ard.biesheu...@arm.com>; Leif Lindholm <l...@nuviainc.com>; Sahil Malhotra <sahil.malho...@linaro.org> Subject: Re: [PATCH edk2-platforms v3 1/2] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver Hi Sami, [...] > > +STATIC > > +EFI_STATUS > > +ReadWriteRpmb ( > > + UINTN SvcAct, > > + UINTN Addr, > > + UINTN NumBytes, > > + UINTN Offset > > + ) > > +{ > > + ARM_SVC_ARGS SvcArgs; > > + EFI_STATUS Status; > > + > > + ZeroMem (&SvcArgs, sizeof (SvcArgs)); > > + > > + SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64; > > + SvcArgs.Arg1 = storage_id; > > + SvcArgs.Arg2 = 0; > > + SvcArgs.Arg3 = SvcAct; > > + SvcArgs.Arg4 = Addr; > > + SvcArgs.Arg5 = NumBytes; > > + SvcArgs.Arg6 = Offset; > > + > > + ArmCallSvc (&SvcArgs); > > + if (SvcArgs.Arg3) { > > + DEBUG ((DEBUG_ERROR, "%a: Svc Call 0x%08x Addr: 0x%08x len: 0x%x > > Offset: 0x%x failed with 0x%x\n", > > + __func__, SvcAct, Addr, NumBytes, Offset, SvcArgs.Arg3)); > > + } > > + > > + switch (SvcArgs.Arg3) { > > + case ARM_SVC_SPM_RET_SUCCESS: > > + Status = EFI_SUCCESS; > > + break; > > + > > + case ARM_SVC_SPM_RET_NOT_SUPPORTED: > > + Status = EFI_UNSUPPORTED; > > + break; > > + > > + case ARM_SVC_SPM_RET_INVALID_PARAMS: > > + Status = EFI_INVALID_PARAMETER; > > + break; > > + > > + case ARM_SVC_SPM_RET_DENIED: > > + Status = EFI_ACCESS_DENIED; > > + break; > > + > > + case ARM_SVC_SPM_RET_NO_MEMORY: > > + Status = EFI_BAD_BUFFER_SIZE; > > + break; > > + > > + default: > > + Status = EFI_ACCESS_DENIED; > > + } > > [SAMI] Should the error handling here be updated similar to the FF-A > > StandaloneMmPkg patches? > > [/SAMI] > > 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. 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) - If the message reaches the target endpoint, then callee shall invoke one of the following interfaces: * FFA_MSG_SEND_DIRECT_RESP * 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71053): https://edk2.groups.io/g/devel/message/71053 Mute This Topic: https://groups.io/mt/78998101/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-