| Message ID | 1538552637-6972-2-git-send-email-sumit.garg@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [edk2,v3,1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE | expand |
On 3 October 2018 at 09:43, Sumit Garg <sumit.garg@linaro.org> wrote: > Add following APIs to communicate with OP-TEE pseudo/early TAs: > 1. OpteeInit > 2. OpteeOpenSession > 3. OpteeCloseSession > 4. OpteeInvokeFunc > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> Given the outcome of the GP discussion, I'm fine with this approach. Leif? > --- > ArmPkg/Include/Library/OpteeLib.h | 90 +++++++++ > ArmPkg/Library/OpteeLib/Optee.c | 357 +++++++++++++++++++++++++++++++++++ > ArmPkg/Library/OpteeLib/OpteeLib.inf | 2 + > ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++++ > 4 files changed, 492 insertions(+) > create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h > > diff --git a/ArmPkg/Include/Library/OpteeLib.h b/ArmPkg/Include/Library/OpteeLib.h > index f65d8674d9b8..2d1c60632dfe 100644 > --- a/ArmPkg/Include/Library/OpteeLib.h > +++ b/ArmPkg/Include/Library/OpteeLib.h > @@ -25,10 +25,100 @@ > #define OPTEE_OS_UID2 0xaf630002 > #define OPTEE_OS_UID3 0xa5d5c51b > > +#define OPTEE_MSG_ATTR_TYPE_NONE 0x0 > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INPUT 0x1 > +#define OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT 0x2 > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INOUT 0x3 > +#define OPTEE_MSG_ATTR_TYPE_MEM_INPUT 0x9 > +#define OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT 0xa > +#define OPTEE_MSG_ATTR_TYPE_MEM_INOUT 0xb > + > +#define OPTEE_MSG_ATTR_TYPE_MASK 0xff > + > +#define OPTEE_ORIGIN_COMMS 0x00000002 > +#define OPTEE_ERROR_COMMS 0xFFFF000E > + > +typedef struct { > + UINT64 BufPtr; > + UINT64 Size; > + UINT64 ShmRef; > +} OPTEE_MSG_PARAM_MEM; > + > +typedef struct { > + UINT64 A; > + UINT64 B; > + UINT64 C; > +} OPTEE_MSG_PARAM_VALUE; > + > +typedef struct { > + UINT64 Attr; > + union { > + OPTEE_MSG_PARAM_MEM Mem; > + OPTEE_MSG_PARAM_VALUE Value; > + } U; > +} OPTEE_MSG_PARAM; > + > +#define MAX_PARAMS 4 > + > +typedef struct { > + UINT32 Cmd; > + UINT32 Func; > + UINT32 Session; > + UINT32 CancelId; > + UINT32 Pad; > + UINT32 Ret; > + UINT32 RetOrigin; > + UINT32 NumParams; > + > + // NumParams tells the actual number of element in Params > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; > +} OPTEE_MSG_ARG; > + > +#define OPTEE_UUID_LEN 16 > + > +typedef struct { > + UINT8 Uuid[OPTEE_UUID_LEN]; // [in] UUID of the Trusted Application > + UINT32 Session; // [out] Session id > + UINT32 Ret; // [out] Return value > + UINT32 RetOrigin; // [out] Origin of the return value > +} OPTEE_OPEN_SESSION_ARG; > + > +typedef struct { > + UINT32 Func; // [in] Trusted App func, specific to the TA > + UINT32 Session; // [in] Session id > + UINT32 Ret; // [out] Return value > + UINT32 RetOrigin; // [out] Origin of the return value > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; // Params for func to be invoked > +} OPTEE_INVOKE_FUNC_ARG; > + > BOOLEAN > EFIAPI > IsOpteePresent ( > VOID > ); > > +EFI_STATUS > +EFIAPI > +OpteeInit ( > + VOID > + ); > + > +EFI_STATUS > +EFIAPI > +OpteeOpenSession ( > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > + ); > + > +EFI_STATUS > +EFIAPI > +OpteeCloseSession ( > + IN UINT32 Session > + ); > + > +EFI_STATUS > +EFIAPI > +OpteeInvokeFunc ( > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > + ); > + > #endif > diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c > index 574527f8b5ea..bf7872cbbce0 100644 > --- a/ArmPkg/Library/OpteeLib/Optee.c > +++ b/ArmPkg/Library/OpteeLib/Optee.c > @@ -14,11 +14,18 @@ > > **/ > > +#include <Library/ArmMmuLib.h> > #include <Library/ArmSmcLib.h> > +#include <Library/BaseMemoryLib.h> > #include <Library/BaseLib.h> > +#include <Library/DebugLib.h> > #include <Library/OpteeLib.h> > > #include <IndustryStandard/ArmStdSmc.h> > +#include <OpteeSmc.h> > +#include <Uefi.h> > + > +STATIC OPTEE_SHARED_MEMORY_INFO OpteeShmInfo = { 0 }; > > /** > Check for OP-TEE presence. > @@ -31,6 +38,7 @@ IsOpteePresent ( > { > ARM_SMC_ARGS ArmSmcArgs; > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > // Send a Trusted OS Calls UID command > ArmSmcArgs.Arg0 = ARM_SMC_ID_TOS_UID; > ArmCallSmc (&ArmSmcArgs); > @@ -44,3 +52,352 @@ IsOpteePresent ( > return FALSE; > } > } > + > +STATIC > +EFI_STATUS > +OpteeShmMemRemap ( > + VOID > + ) > +{ > + ARM_SMC_ARGS ArmSmcArgs; > + EFI_PHYSICAL_ADDRESS Paddr; > + EFI_PHYSICAL_ADDRESS Start; > + EFI_PHYSICAL_ADDRESS End; > + EFI_STATUS Status; > + UINTN Size; > + > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > + ArmSmcArgs.Arg0 = OPTEE_SMC_GET_SHM_CONFIG; > + > + ArmCallSmc (&ArmSmcArgs); > + if (ArmSmcArgs.Arg0 != OPTEE_SMC_RETURN_OK) { > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory not supported\n")); > + return EFI_UNSUPPORTED; > + } > + > + if (ArmSmcArgs.Arg3 != OPTEE_SMC_SHM_CACHED) { > + DEBUG ((DEBUG_WARN, "OP-TEE: Only normal cached shared memory supported\n")); > + return EFI_UNSUPPORTED; > + } > + > + Start = (ArmSmcArgs.Arg1 + SIZE_4KB - 1) & ~(SIZE_4KB - 1); > + End = (ArmSmcArgs.Arg1 + ArmSmcArgs.Arg2) & ~(SIZE_4KB - 1); > + Paddr = Start; > + Size = End - Start; > + > + if (Size < SIZE_4KB) { > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory too small\n")); > + return EFI_BUFFER_TOO_SMALL; > + } > + > + Status = ArmSetMemoryAttributes (Paddr, Size, EFI_MEMORY_WB); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + OpteeShmInfo.Base = (UINTN)Paddr; > + OpteeShmInfo.Size = Size; > + > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > +OpteeInit ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + > + if (!IsOpteePresent ()) { > + DEBUG ((DEBUG_WARN, "OP-TEE not present\n")); > + return EFI_UNSUPPORTED; > + } > + > + Status = OpteeShmMemRemap (); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory remap failed\n")); > + return Status; > + } > + > + return EFI_SUCCESS; > +} > + > +/** > + Does Standard SMC to OP-TEE in secure world. > + > + @param[in] Parg Physical address of message to pass to secure world > + > + @return 0 on success, secure world return code otherwise > + > +**/ > +STATIC > +UINT32 > +OpteeCallWithArg ( > + IN EFI_PHYSICAL_ADDRESS Parg > + ) > +{ > + ARM_SMC_ARGS ArmSmcArgs; > + > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > + ArmSmcArgs.Arg0 = OPTEE_SMC_CALL_WITH_ARG; > + ArmSmcArgs.Arg1 = (UINT32)(Parg >> 32); > + ArmSmcArgs.Arg2 = (UINT32)Parg; > + > + while (TRUE) { > + ArmCallSmc (&ArmSmcArgs); > + > + if (ArmSmcArgs.Arg0 == OPTEE_SMC_RETURN_RPC_FOREIGN_INTR) { > + // > + // A foreign interrupt was raised while secure world was > + // executing, since they are handled in UEFI a dummy RPC is > + // performed to let UEFI take the interrupt through the normal > + // vector. > + // > + ArmSmcArgs.Arg0 = OPTEE_SMC_RETURN_FROM_RPC; > + } else { > + break; > + } > + } > + > + return ArmSmcArgs.Arg0; > +} > + > +EFI_STATUS > +EFIAPI > +OpteeOpenSession ( > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > + ) > +{ > + OPTEE_MSG_ARG *MsgArg; > + > + MsgArg = NULL; > + > + if (OpteeShmInfo.Base == 0) { > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > + return EFI_NOT_STARTED; > + } > + > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > + > + MsgArg->Cmd = OPTEE_MSG_CMD_OPEN_SESSION; > + > + // > + // Initialize and add the meta parameters needed when opening a > + // session. > + // > + MsgArg->Params[0].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > + OPTEE_MSG_ATTR_META; > + MsgArg->Params[1].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > + OPTEE_MSG_ATTR_META; > + CopyMem (&MsgArg->Params[0].U.Value, OpenSessionArg->Uuid, OPTEE_UUID_LEN); > + ZeroMem (&MsgArg->Params[1].U.Value, OPTEE_UUID_LEN); > + MsgArg->Params[1].U.Value.C = TEE_LOGIN_PUBLIC; > + > + MsgArg->NumParams = 2; > + > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > + MsgArg->Ret = OPTEE_ERROR_COMMS; > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > + } > + > + OpenSessionArg->Session = MsgArg->Session; > + OpenSessionArg->Ret = MsgArg->Ret; > + OpenSessionArg->RetOrigin = MsgArg->RetOrigin; > + > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > +OpteeCloseSession ( > + IN UINT32 Session > + ) > +{ > + OPTEE_MSG_ARG *MsgArg; > + > + MsgArg = NULL; > + > + if (OpteeShmInfo.Base == 0) { > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > + return EFI_NOT_STARTED; > + } > + > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > + > + MsgArg->Cmd = OPTEE_MSG_CMD_CLOSE_SESSION; > + MsgArg->Session = Session; > + > + OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg); > + > + return EFI_SUCCESS; > +} > + > +STATIC > +EFI_STATUS > +OpteeToMsgParam ( > + OUT OPTEE_MSG_PARAM *MsgParams, > + IN UINT32 NumParams, > + IN OPTEE_MSG_PARAM *InParams > + ) > +{ > + UINT32 Idx; > + UINTN ParamShmAddr; > + UINTN ShmSize; > + UINTN Size; > + > + Size = (sizeof (OPTEE_MSG_ARG) + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > + ParamShmAddr = OpteeShmInfo.Base + Size; > + ShmSize = OpteeShmInfo.Size - Size; > + > + for (Idx = 0; Idx < NumParams; Idx++) { > + CONST OPTEE_MSG_PARAM *Ip; > + OPTEE_MSG_PARAM *Mp; > + UINT32 Attr; > + > + Ip = InParams + Idx; > + Mp = MsgParams + Idx; > + Attr = Ip->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > + > + switch (Attr) { > + case OPTEE_MSG_ATTR_TYPE_NONE: > + Mp->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > + ZeroMem (&Mp->U, sizeof (Mp->U)); > + break; > + > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > + Mp->Attr = Attr; > + Mp->U.Value.A = Ip->U.Value.A; > + Mp->U.Value.B = Ip->U.Value.B; > + Mp->U.Value.C = Ip->U.Value.C; > + break; > + > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > + Mp->Attr = Attr; > + > + if (Ip->U.Mem.Size > ShmSize) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + CopyMem ((VOID *)ParamShmAddr, (VOID *)Ip->U.Mem.BufPtr, Ip->U.Mem.Size); > + Mp->U.Mem.BufPtr = (UINT64)ParamShmAddr; > + Mp->U.Mem.Size = Ip->U.Mem.Size; > + > + Size = (Ip->U.Mem.Size + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > + ParamShmAddr += Size; > + ShmSize -= Size; > + break; > + > + default: > + return EFI_INVALID_PARAMETER; > + } > + } > + > + return EFI_SUCCESS; > +} > + > +STATIC > +EFI_STATUS > +OpteeFromMsgParam ( > + OUT OPTEE_MSG_PARAM *OutParams, > + IN UINT32 NumParams, > + IN OPTEE_MSG_PARAM *MsgParams > + ) > +{ > + UINT32 Idx; > + > + for (Idx = 0; Idx < NumParams; Idx++) { > + OPTEE_MSG_PARAM *Op; > + CONST OPTEE_MSG_PARAM *Mp; > + UINT32 Attr; > + > + Op = OutParams + Idx; > + Mp = MsgParams + Idx; > + Attr = Mp->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > + > + switch (Attr) { > + case OPTEE_MSG_ATTR_TYPE_NONE: > + Op->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > + ZeroMem (&Op->U, sizeof (Op->U)); > + break; > + > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > + Op->Attr = Attr; > + Op->U.Value.A = Mp->U.Value.A; > + Op->U.Value.B = Mp->U.Value.B; > + Op->U.Value.C = Mp->U.Value.C; > + break; > + > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > + Op->Attr = Attr; > + > + if (Mp->U.Mem.Size > Op->U.Mem.Size) { > + return EFI_BAD_BUFFER_SIZE; > + } > + > + CopyMem ((VOID *)Op->U.Mem.BufPtr, (VOID *)Mp->U.Mem.BufPtr, Mp->U.Mem.Size); > + Op->U.Mem.Size = Mp->U.Mem.Size; > + break; > + > + default: > + return EFI_INVALID_PARAMETER; > + } > + } > + > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > +OpteeInvokeFunc ( > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > + ) > +{ > + EFI_STATUS Status; > + OPTEE_MSG_ARG *MsgArg; > + > + MsgArg = NULL; > + > + if (OpteeShmInfo.Base == 0) { > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > + return EFI_NOT_STARTED; > + } > + > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > + > + MsgArg->Cmd = OPTEE_MSG_CMD_INVOKE_COMMAND; > + MsgArg->Func = InvokeFuncArg->Func; > + MsgArg->Session = InvokeFuncArg->Session; > + > + Status = OpteeToMsgParam (MsgArg->Params, MAX_PARAMS, InvokeFuncArg->Params); > + if (Status) > + return Status; > + > + MsgArg->NumParams = MAX_PARAMS; > + > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > + MsgArg->Ret = OPTEE_ERROR_COMMS; > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > + } > + > + if (OpteeFromMsgParam (InvokeFuncArg->Params, MAX_PARAMS, MsgArg->Params)) { > + MsgArg->Ret = OPTEE_ERROR_COMMS; > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > + } > + > + InvokeFuncArg->Ret = MsgArg->Ret; > + InvokeFuncArg->RetOrigin = MsgArg->RetOrigin; > + > + return EFI_SUCCESS; > +} > diff --git a/ArmPkg/Library/OpteeLib/OpteeLib.inf b/ArmPkg/Library/OpteeLib/OpteeLib.inf > index 5abd427379cc..e03054a7167d 100644 > --- a/ArmPkg/Library/OpteeLib/OpteeLib.inf > +++ b/ArmPkg/Library/OpteeLib/OpteeLib.inf > @@ -23,11 +23,13 @@ [Defines] > > [Sources] > Optee.c > + OpteeSmc.h > > [Packages] > ArmPkg/ArmPkg.dec > MdePkg/MdePkg.dec > > [LibraryClasses] > + ArmMmuLib > ArmSmcLib > BaseLib > diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h b/ArmPkg/Library/OpteeLib/OpteeSmc.h > new file mode 100644 > index 000000000000..e2ea35784a0a > --- /dev/null > +++ b/ArmPkg/Library/OpteeLib/OpteeSmc.h > @@ -0,0 +1,43 @@ > +/** @file > + OP-TEE SMC header file. > + > + Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR> > + > + This program and the accompanying materials > + are licensed and made available under the terms and conditions of the BSD License > + which accompanies this distribution. The full text of the license may be found at > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > + > +**/ > + > +#ifndef _OPTEE_SMC_H_ > +#define _OPTEE_SMC_H_ > + > +/* Returned in Arg0 only from Trusted OS functions */ > +#define OPTEE_SMC_RETURN_OK 0x0 > + > +#define OPTEE_SMC_RETURN_FROM_RPC 0x32000003 > +#define OPTEE_SMC_CALL_WITH_ARG 0x32000004 > +#define OPTEE_SMC_GET_SHM_CONFIG 0xb2000007 > + > +#define OPTEE_SMC_SHM_CACHED 1 > + > +#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTR 0xffff0004 > + > +#define OPTEE_MSG_CMD_OPEN_SESSION 0 > +#define OPTEE_MSG_CMD_INVOKE_COMMAND 1 > +#define OPTEE_MSG_CMD_CLOSE_SESSION 2 > + > +#define OPTEE_MSG_ATTR_META 0x100 > + > +#define TEE_LOGIN_PUBLIC 0x0 > + > +typedef struct { > + UINTN Base; > + UINTN Size; > +} OPTEE_SHARED_MEMORY_INFO; > + > +#endif > -- > 2.7.4 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Oct 03, 2018 at 11:33:01AM +0200, Ard Biesheuvel wrote: > On 3 October 2018 at 09:43, Sumit Garg <sumit.garg@linaro.org> wrote: > > Add following APIs to communicate with OP-TEE pseudo/early TAs: > > 1. OpteeInit > > 2. OpteeOpenSession > > 3. OpteeCloseSession > > 4. OpteeInvokeFunc > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > Given the outcome of the GP discussion, I'm fine with this approach. Leif? Apologies for the delay, I needed some time to think it over. I'm not super happy about this approach, but I'm happier with this than I would be with leaving the functionality out of the upstream tree. I really hope we can get that license either changed or dropped completely. I do have a few comments below. > > --- > > ArmPkg/Include/Library/OpteeLib.h | 90 +++++++++ > > ArmPkg/Library/OpteeLib/Optee.c | 357 +++++++++++++++++++++++++++++++++++ > > ArmPkg/Library/OpteeLib/OpteeLib.inf | 2 + > > ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++++ Could you follow the instructions in https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23 when generating future patches? The --stat* effects aren't apparent here, but the -O ones are. > > 4 files changed, 492 insertions(+) > > create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h > > > > diff --git a/ArmPkg/Include/Library/OpteeLib.h b/ArmPkg/Include/Library/OpteeLib.h > > index f65d8674d9b8..2d1c60632dfe 100644 > > --- a/ArmPkg/Include/Library/OpteeLib.h > > +++ b/ArmPkg/Include/Library/OpteeLib.h > > @@ -25,10 +25,100 @@ > > #define OPTEE_OS_UID2 0xaf630002 > > #define OPTEE_OS_UID3 0xa5d5c51b > > > > +#define OPTEE_MSG_ATTR_TYPE_NONE 0x0 > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INPUT 0x1 > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT 0x2 > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INOUT 0x3 > > +#define OPTEE_MSG_ATTR_TYPE_MEM_INPUT 0x9 > > +#define OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT 0xa > > +#define OPTEE_MSG_ATTR_TYPE_MEM_INOUT 0xb > > + > > +#define OPTEE_MSG_ATTR_TYPE_MASK 0xff > > + > > +#define OPTEE_ORIGIN_COMMS 0x00000002 > > +#define OPTEE_ERROR_COMMS 0xFFFF000E > > + > > +typedef struct { > > + UINT64 BufPtr; If it's a pointer, it has a *. Otherwise it's an address. > > + UINT64 Size; > > + UINT64 ShmRef; Abbreviations in function, variable or type names (other than the ones defined in [1] are not permitted unless they are explicitly added to a glossary section of the source file header. Where possible, just write out the name in full. [1] https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/4_naming_conventions/#table-2-efi-supported-abbreviations BufPtr (as a name) gets a pass since it's unambiguous and we already have a bunch of those in the codebase. ShmRef needs to be clear. (This comment also applies to a lot of things below, I won't point them all out unless asked to.) > > +} OPTEE_MSG_PARAM_MEM; > > + > > +typedef struct { > > + UINT64 A; > > + UINT64 B; > > + UINT64 C; > > +} OPTEE_MSG_PARAM_VALUE; > > + > > +typedef struct { > > + UINT64 Attr; > > + union { > > + OPTEE_MSG_PARAM_MEM Mem; > > + OPTEE_MSG_PARAM_VALUE Value; > > + } U; > > +} OPTEE_MSG_PARAM; > > + > > +#define MAX_PARAMS 4 This is a very localised macro with a very globalised name. Suggest adding an OPTEE_ prefix, but also something describing what it is the maximum parameters for. CALL_? > > + > > +typedef struct { > > + UINT32 Cmd; > > + UINT32 Func; > > + UINT32 Session; > > + UINT32 CancelId; > > + UINT32 Pad; > > + UINT32 Ret; > > + UINT32 RetOrigin; > > + UINT32 NumParams; > > + > > + // NumParams tells the actual number of element in Params > > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; > > +} OPTEE_MSG_ARG; > > + > > +#define OPTEE_UUID_LEN 16 UUIDs are UUIDs. If optee decides on an incompatible format, we may have an interoperability issue. I assume this is not the case, so perhaps replace with sizeof (EFI_GUID) at each point of use? > > + > > +typedef struct { > > + UINT8 Uuid[OPTEE_UUID_LEN]; // [in] UUID of the Trusted Application Is there a strong reason for not using EFI_GUID here? > > + UINT32 Session; // [out] Session id > > + UINT32 Ret; // [out] Return value > > + UINT32 RetOrigin; // [out] Origin of the return value > > +} OPTEE_OPEN_SESSION_ARG; > > + > > +typedef struct { > > + UINT32 Func; // [in] Trusted App func, specific to the TA > > + UINT32 Session; // [in] Session id > > + UINT32 Ret; // [out] Return value > > + UINT32 RetOrigin; // [out] Origin of the return value > > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; // Params for func to be invoked > > +} OPTEE_INVOKE_FUNC_ARG; > > + > > BOOLEAN > > EFIAPI > > IsOpteePresent ( > > VOID > > ); > > > > +EFI_STATUS > > +EFIAPI > > +OpteeInit ( > > + VOID > > + ); > > + > > +EFI_STATUS > > +EFIAPI > > +OpteeOpenSession ( > > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > > + ); > > + > > +EFI_STATUS > > +EFIAPI > > +OpteeCloseSession ( > > + IN UINT32 Session > > + ); > > + > > +EFI_STATUS > > +EFIAPI > > +OpteeInvokeFunc ( > > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > > + ); > > + > > #endif > > diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c > > index 574527f8b5ea..bf7872cbbce0 100644 > > --- a/ArmPkg/Library/OpteeLib/Optee.c > > +++ b/ArmPkg/Library/OpteeLib/Optee.c > > @@ -14,11 +14,18 @@ > > > > **/ > > > > +#include <Library/ArmMmuLib.h> > > #include <Library/ArmSmcLib.h> > > +#include <Library/BaseMemoryLib.h> > > #include <Library/BaseLib.h> > > +#include <Library/DebugLib.h> > > #include <Library/OpteeLib.h> > > > > #include <IndustryStandard/ArmStdSmc.h> > > +#include <OpteeSmc.h> > > +#include <Uefi.h> > > + > > +STATIC OPTEE_SHARED_MEMORY_INFO OpteeShmInfo = { 0 }; > > > > /** > > Check for OP-TEE presence. > > @@ -31,6 +38,7 @@ IsOpteePresent ( > > { > > ARM_SMC_ARGS ArmSmcArgs; > > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > // Send a Trusted OS Calls UID command > > ArmSmcArgs.Arg0 = ARM_SMC_ID_TOS_UID; > > ArmCallSmc (&ArmSmcArgs); > > @@ -44,3 +52,352 @@ IsOpteePresent ( > > return FALSE; > > } > > } > > + > > +STATIC > > +EFI_STATUS > > +OpteeShmMemRemap ( > > + VOID > > + ) > > +{ > > + ARM_SMC_ARGS ArmSmcArgs; > > + EFI_PHYSICAL_ADDRESS Paddr; > > + EFI_PHYSICAL_ADDRESS Start; > > + EFI_PHYSICAL_ADDRESS End; > > + EFI_STATUS Status; > > + UINTN Size; > > + > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > + ArmSmcArgs.Arg0 = OPTEE_SMC_GET_SHM_CONFIG; > > + > > + ArmCallSmc (&ArmSmcArgs); > > + if (ArmSmcArgs.Arg0 != OPTEE_SMC_RETURN_OK) { > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory not supported\n")); > > + return EFI_UNSUPPORTED; > > + } > > + > > + if (ArmSmcArgs.Arg3 != OPTEE_SMC_SHM_CACHED) { > > + DEBUG ((DEBUG_WARN, "OP-TEE: Only normal cached shared memory supported\n")); > > + return EFI_UNSUPPORTED; > > + } > > + > > + Start = (ArmSmcArgs.Arg1 + SIZE_4KB - 1) & ~(SIZE_4KB - 1); > > + End = (ArmSmcArgs.Arg1 + ArmSmcArgs.Arg2) & ~(SIZE_4KB - 1); > > + Paddr = Start; > > + Size = End - Start; > > + > > + if (Size < SIZE_4KB) { > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory too small\n")); > > + return EFI_BUFFER_TOO_SMALL; > > + } > > + > > + Status = ArmSetMemoryAttributes (Paddr, Size, EFI_MEMORY_WB); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + > > + OpteeShmInfo.Base = (UINTN)Paddr; Would PHYSICAL_ADDRESS (always 64-bit) be more correct here, or is the OP-TEE end seeing this struct as always having fields of native width? (This is where the ordefile comes in handy, because it means I see the struct definition first, not its use.) > > + OpteeShmInfo.Size = Size; > > + > > + return EFI_SUCCESS; > > +} > > + > > +EFI_STATUS > > +EFIAPI > > +OpteeInit ( > > + VOID > > + ) > > +{ > > + EFI_STATUS Status; > > + > > + if (!IsOpteePresent ()) { > > + DEBUG ((DEBUG_WARN, "OP-TEE not present\n")); > > + return EFI_UNSUPPORTED; > > + } > > + > > + Status = OpteeShmMemRemap (); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory remap failed\n")); > > + return Status; > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > +/** > > + Does Standard SMC to OP-TEE in secure world. > > + > > + @param[in] Parg Physical address of message to pass to secure world > > + > > + @return 0 on success, secure world return code otherwise > > + > > +**/ > > +STATIC > > +UINT32 > > +OpteeCallWithArg ( > > + IN EFI_PHYSICAL_ADDRESS Parg > > + ) > > +{ > > + ARM_SMC_ARGS ArmSmcArgs; > > + > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > + ArmSmcArgs.Arg0 = OPTEE_SMC_CALL_WITH_ARG; > > + ArmSmcArgs.Arg1 = (UINT32)(Parg >> 32); > > + ArmSmcArgs.Arg2 = (UINT32)Parg; > > + > > + while (TRUE) { > > + ArmCallSmc (&ArmSmcArgs); > > + > > + if (ArmSmcArgs.Arg0 == OPTEE_SMC_RETURN_RPC_FOREIGN_INTR) { > > + // > > + // A foreign interrupt was raised while secure world was > > + // executing, since they are handled in UEFI a dummy RPC is > > + // performed to let UEFI take the interrupt through the normal > > + // vector. > > + // > > + ArmSmcArgs.Arg0 = OPTEE_SMC_RETURN_FROM_RPC; > > + } else { > > + break; > > + } > > + } > > + > > + return ArmSmcArgs.Arg0; > > +} > > + > > +EFI_STATUS > > +EFIAPI > > +OpteeOpenSession ( > > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > > + ) > > +{ > > + OPTEE_MSG_ARG *MsgArg; > > + > > + MsgArg = NULL; > > + > > + if (OpteeShmInfo.Base == 0) { > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > + return EFI_NOT_STARTED; > > + } > > + > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > + > > + MsgArg->Cmd = OPTEE_MSG_CMD_OPEN_SESSION; > > + > > + // > > + // Initialize and add the meta parameters needed when opening a > > + // session. > > + // > > + MsgArg->Params[0].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > > + OPTEE_MSG_ATTR_META; > > + MsgArg->Params[1].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > > + OPTEE_MSG_ATTR_META; > > + CopyMem (&MsgArg->Params[0].U.Value, OpenSessionArg->Uuid, OPTEE_UUID_LEN); > > + ZeroMem (&MsgArg->Params[1].U.Value, OPTEE_UUID_LEN); > > + MsgArg->Params[1].U.Value.C = TEE_LOGIN_PUBLIC; > > + > > + MsgArg->NumParams = 2; > > + > > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > + } > > + > > + OpenSessionArg->Session = MsgArg->Session; > > + OpenSessionArg->Ret = MsgArg->Ret; > > + OpenSessionArg->RetOrigin = MsgArg->RetOrigin; > > + > > + return EFI_SUCCESS; > > +} > > + > > +EFI_STATUS > > +EFIAPI > > +OpteeCloseSession ( > > + IN UINT32 Session > > + ) > > +{ > > + OPTEE_MSG_ARG *MsgArg; > > + > > + MsgArg = NULL; > > + > > + if (OpteeShmInfo.Base == 0) { > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > + return EFI_NOT_STARTED; > > + } > > + > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > + > > + MsgArg->Cmd = OPTEE_MSG_CMD_CLOSE_SESSION; > > + MsgArg->Session = Session; > > + > > + OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg); > > + > > + return EFI_SUCCESS; > > +} > > + > > +STATIC > > +EFI_STATUS > > +OpteeToMsgParam ( > > + OUT OPTEE_MSG_PARAM *MsgParams, > > + IN UINT32 NumParams, > > + IN OPTEE_MSG_PARAM *InParams > > + ) > > +{ > > + UINT32 Idx; > > + UINTN ParamShmAddr; > > + UINTN ShmSize; > > + UINTN Size; > > + > > + Size = (sizeof (OPTEE_MSG_ARG) + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > > + ParamShmAddr = OpteeShmInfo.Base + Size; > > + ShmSize = OpteeShmInfo.Size - Size; > > + > > + for (Idx = 0; Idx < NumParams; Idx++) { > > + CONST OPTEE_MSG_PARAM *Ip; > > + OPTEE_MSG_PARAM *Mp; > > + UINT32 Attr; > > + > > + Ip = InParams + Idx; > > + Mp = MsgParams + Idx; > > + Attr = Ip->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > > + > > + switch (Attr) { > > + case OPTEE_MSG_ATTR_TYPE_NONE: > > + Mp->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > > + ZeroMem (&Mp->U, sizeof (Mp->U)); > > + break; > > + > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > > + Mp->Attr = Attr; > > + Mp->U.Value.A = Ip->U.Value.A; > > + Mp->U.Value.B = Ip->U.Value.B; > > + Mp->U.Value.C = Ip->U.Value.C; > > + break; > > + > > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > > + Mp->Attr = Attr; > > + > > + if (Ip->U.Mem.Size > ShmSize) { > > + return EFI_OUT_OF_RESOURCES; > > + } > > + > > + CopyMem ((VOID *)ParamShmAddr, (VOID *)Ip->U.Mem.BufPtr, Ip->U.Mem.Size); > > + Mp->U.Mem.BufPtr = (UINT64)ParamShmAddr; While I don't think it's possible for this to end up doing something unexpected, casting pointers to a higher alignment (which this would do on 32-bit) is a bad habit to get into. > > + Mp->U.Mem.Size = Ip->U.Mem.Size; > > + > > + Size = (Ip->U.Mem.Size + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > > + ParamShmAddr += Size; > > + ShmSize -= Size; > > + break; > > + > > + default: > > + return EFI_INVALID_PARAMETER; > > + } > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > +STATIC > > +EFI_STATUS > > +OpteeFromMsgParam ( > > + OUT OPTEE_MSG_PARAM *OutParams, > > + IN UINT32 NumParams, > > + IN OPTEE_MSG_PARAM *MsgParams > > + ) > > +{ > > + UINT32 Idx; > > + > > + for (Idx = 0; Idx < NumParams; Idx++) { > > + OPTEE_MSG_PARAM *Op; > > + CONST OPTEE_MSG_PARAM *Mp; > > + UINT32 Attr; > > + > > + Op = OutParams + Idx; > > + Mp = MsgParams + Idx; > > + Attr = Mp->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > > + > > + switch (Attr) { > > + case OPTEE_MSG_ATTR_TYPE_NONE: > > + Op->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > > + ZeroMem (&Op->U, sizeof (Op->U)); > > + break; > > + > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > > + Op->Attr = Attr; > > + Op->U.Value.A = Mp->U.Value.A; > > + Op->U.Value.B = Mp->U.Value.B; > > + Op->U.Value.C = Mp->U.Value.C; > > + break; > > + > > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > > + Op->Attr = Attr; > > + > > + if (Mp->U.Mem.Size > Op->U.Mem.Size) { > > + return EFI_BAD_BUFFER_SIZE; > > + } > > + > > + CopyMem ((VOID *)Op->U.Mem.BufPtr, (VOID *)Mp->U.Mem.BufPtr, Mp->U.Mem.Size); > > + Op->U.Mem.Size = Mp->U.Mem.Size; > > + break; > > + > > + default: > > + return EFI_INVALID_PARAMETER; > > + } > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > +EFI_STATUS > > +EFIAPI > > +OpteeInvokeFunc ( > > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > > + ) > > +{ > > + EFI_STATUS Status; > > + OPTEE_MSG_ARG *MsgArg; > > + > > + MsgArg = NULL; > > + > > + if (OpteeShmInfo.Base == 0) { > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > + return EFI_NOT_STARTED; > > + } > > + > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > + > > + MsgArg->Cmd = OPTEE_MSG_CMD_INVOKE_COMMAND; > > + MsgArg->Func = InvokeFuncArg->Func; > > + MsgArg->Session = InvokeFuncArg->Session; > > + > > + Status = OpteeToMsgParam (MsgArg->Params, MAX_PARAMS, InvokeFuncArg->Params); > > + if (Status) > > + return Status; Always use {} (coding style requirement). > > + > > + MsgArg->NumParams = MAX_PARAMS; > > + > > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > + } > > + > > + if (OpteeFromMsgParam (InvokeFuncArg->Params, MAX_PARAMS, MsgArg->Params)) { > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > + } > > + > > + InvokeFuncArg->Ret = MsgArg->Ret; > > + InvokeFuncArg->RetOrigin = MsgArg->RetOrigin; > > + > > + return EFI_SUCCESS; > > +} > > diff --git a/ArmPkg/Library/OpteeLib/OpteeLib.inf b/ArmPkg/Library/OpteeLib/OpteeLib.inf > > index 5abd427379cc..e03054a7167d 100644 > > --- a/ArmPkg/Library/OpteeLib/OpteeLib.inf > > +++ b/ArmPkg/Library/OpteeLib/OpteeLib.inf > > @@ -23,11 +23,13 @@ [Defines] > > > > [Sources] > > Optee.c > > + OpteeSmc.h > > > > [Packages] > > ArmPkg/ArmPkg.dec > > MdePkg/MdePkg.dec > > > > [LibraryClasses] > > + ArmMmuLib > > ArmSmcLib > > BaseLib > > diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h b/ArmPkg/Library/OpteeLib/OpteeSmc.h > > new file mode 100644 > > index 000000000000..e2ea35784a0a > > --- /dev/null > > +++ b/ArmPkg/Library/OpteeLib/OpteeSmc.h > > @@ -0,0 +1,43 @@ > > +/** @file > > + OP-TEE SMC header file. > > + > > + Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR> > > + > > + This program and the accompanying materials > > + are licensed and made available under the terms and conditions of the BSD License > > + which accompanies this distribution. The full text of the license may be found at > > + http://opensource.org/licenses/bsd-license.php > > + > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > + > > +**/ > > + > > +#ifndef _OPTEE_SMC_H_ > > +#define _OPTEE_SMC_H_ > > + > > +/* Returned in Arg0 only from Trusted OS functions */ > > +#define OPTEE_SMC_RETURN_OK 0x0 > > + > > +#define OPTEE_SMC_RETURN_FROM_RPC 0x32000003 > > +#define OPTEE_SMC_CALL_WITH_ARG 0x32000004 > > +#define OPTEE_SMC_GET_SHM_CONFIG 0xb2000007 > > + > > +#define OPTEE_SMC_SHM_CACHED 1 > > + > > +#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTR 0xffff0004 > > + > > +#define OPTEE_MSG_CMD_OPEN_SESSION 0 > > +#define OPTEE_MSG_CMD_INVOKE_COMMAND 1 > > +#define OPTEE_MSG_CMD_CLOSE_SESSION 2 > > + > > +#define OPTEE_MSG_ATTR_META 0x100 > > + > > +#define TEE_LOGIN_PUBLIC 0x0 I'm a bit apprehensive about taking over the whole TEE namespace in an implementation-specific header file. Can/should this be OPTEE_? > > + > > +typedef struct { > > + UINTN Base; > > + UINTN Size; > > +} OPTEE_SHARED_MEMORY_INFO; (See comments above.) / Leif > > + > > +#endif > > -- > > 2.7.4 > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Fri, 5 Oct 2018 at 20:33, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Wed, Oct 03, 2018 at 11:33:01AM +0200, Ard Biesheuvel wrote: > > On 3 October 2018 at 09:43, Sumit Garg <sumit.garg@linaro.org> wrote: > > > Add following APIs to communicate with OP-TEE pseudo/early TAs: > > > 1. OpteeInit > > > 2. OpteeOpenSession > > > 3. OpteeCloseSession > > > 4. OpteeInvokeFunc > > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > > > Given the outcome of the GP discussion, I'm fine with this approach. Leif? > > Apologies for the delay, I needed some time to think it over. > > I'm not super happy about this approach, but I'm happier with this > than I would be with leaving the functionality out of the upstream > tree. I really hope we can get that license either changed or dropped > completely. > Thanks Leif for this go ahead. > I do have a few comments below. > > > > --- > > > ArmPkg/Include/Library/OpteeLib.h | 90 +++++++++ > > > ArmPkg/Library/OpteeLib/Optee.c | 357 +++++++++++++++++++++++++++++++++++ > > > ArmPkg/Library/OpteeLib/OpteeLib.inf | 2 + > > > ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++++ > > Could you follow the instructions in > https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23 > when generating future patches? > The --stat* effects aren't apparent here, but the -O ones are. > Sure. > > > 4 files changed, 492 insertions(+) > > > create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h > > > > > > diff --git a/ArmPkg/Include/Library/OpteeLib.h b/ArmPkg/Include/Library/OpteeLib.h > > > index f65d8674d9b8..2d1c60632dfe 100644 > > > --- a/ArmPkg/Include/Library/OpteeLib.h > > > +++ b/ArmPkg/Include/Library/OpteeLib.h > > > @@ -25,10 +25,100 @@ > > > #define OPTEE_OS_UID2 0xaf630002 > > > #define OPTEE_OS_UID3 0xa5d5c51b > > > > > > +#define OPTEE_MSG_ATTR_TYPE_NONE 0x0 > > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INPUT 0x1 > > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT 0x2 > > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INOUT 0x3 > > > +#define OPTEE_MSG_ATTR_TYPE_MEM_INPUT 0x9 > > > +#define OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT 0xa > > > +#define OPTEE_MSG_ATTR_TYPE_MEM_INOUT 0xb > > > + > > > +#define OPTEE_MSG_ATTR_TYPE_MASK 0xff > > > + > > > +#define OPTEE_ORIGIN_COMMS 0x00000002 > > > +#define OPTEE_ERROR_COMMS 0xFFFF000E > > > + > > > +typedef struct { > > > + UINT64 BufPtr; > > If it's a pointer, it has a *. > Otherwise it's an address. > Will rather use BufferAddress. > > > + UINT64 Size; > > > + UINT64 ShmRef; > > Abbreviations in function, variable or type names (other than the ones > defined in [1] are not permitted unless they are explicitly added to a > glossary section of the source file header. Where possible, just write > out the name in full. > > [1] https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/4_naming_conventions/#table-2-efi-supported-abbreviations > > BufPtr (as a name) gets a pass since it's unambiguous and we already > have a bunch of those in the codebase. ShmRef needs to be clear. > Will replace it with SharedMemoryReference. > (This comment also applies to a lot of things below, I won't point > them all out unless asked to.) > I will try to replace most of them with full names. But I have confusion regarding usage of following not included in [1]: 1. MSG/Msg 2. MEM/Mem 3. INFO/Info 4. FUNC/Func 5. RET/Ret 6. ATTR/Attr 7. CMD/Cmd But I do see their usage in the codebase. Please help to clarify. Also can I use 1-2 letter variable or struct member names like Mp, Ip, Op, U etc.? > > > +} OPTEE_MSG_PARAM_MEM; > > > + > > > +typedef struct { > > > + UINT64 A; > > > + UINT64 B; > > > + UINT64 C; > > > +} OPTEE_MSG_PARAM_VALUE; > > > + > > > +typedef struct { > > > + UINT64 Attr; > > > + union { > > > + OPTEE_MSG_PARAM_MEM Mem; > > > + OPTEE_MSG_PARAM_VALUE Value; > > > + } U; > > > +} OPTEE_MSG_PARAM; > > > + > > > +#define MAX_PARAMS 4 > > This is a very localised macro with a very globalised name. > Suggest adding an OPTEE_ prefix, but also something describing what it > is the maximum parameters for. CALL_? > Ok will rename it to OPTEE_CALL_MAX_PARAMS. > > > + > > > +typedef struct { > > > + UINT32 Cmd; > > > + UINT32 Func; > > > + UINT32 Session; > > > + UINT32 CancelId; > > > + UINT32 Pad; > > > + UINT32 Ret; > > > + UINT32 RetOrigin; > > > + UINT32 NumParams; > > > + > > > + // NumParams tells the actual number of element in Params > > > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; > > > +} OPTEE_MSG_ARG; > > > + > > > +#define OPTEE_UUID_LEN 16 > > UUIDs are UUIDs. If optee decides on an incompatible format, we may > have an interoperability issue. I assume this is not the case, so > perhaps replace with sizeof (EFI_GUID) at each point of use? > Agree will use sizeof (EFI_GUID) instead. > > > + > > > +typedef struct { > > > + UINT8 Uuid[OPTEE_UUID_LEN]; // [in] UUID of the Trusted Application > > Is there a strong reason for not using EFI_GUID here? > EFI_GUID can be used here as UUID are also known as GUID. But here we pass UUID as 16 octets to OP-TEE. So will use EFI_GUID instead and add an api to convert uuid to octets. > > > + UINT32 Session; // [out] Session id > > > + UINT32 Ret; // [out] Return value > > > + UINT32 RetOrigin; // [out] Origin of the return value > > > +} OPTEE_OPEN_SESSION_ARG; > > > + > > > +typedef struct { > > > + UINT32 Func; // [in] Trusted App func, specific to the TA > > > + UINT32 Session; // [in] Session id > > > + UINT32 Ret; // [out] Return value > > > + UINT32 RetOrigin; // [out] Origin of the return value > > > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; // Params for func to be invoked > > > +} OPTEE_INVOKE_FUNC_ARG; > > > + > > > BOOLEAN > > > EFIAPI > > > IsOpteePresent ( > > > VOID > > > ); > > > > > > +EFI_STATUS > > > +EFIAPI > > > +OpteeInit ( > > > + VOID > > > + ); > > > + > > > +EFI_STATUS > > > +EFIAPI > > > +OpteeOpenSession ( > > > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > > > + ); > > > + > > > +EFI_STATUS > > > +EFIAPI > > > +OpteeCloseSession ( > > > + IN UINT32 Session > > > + ); > > > + > > > +EFI_STATUS > > > +EFIAPI > > > +OpteeInvokeFunc ( > > > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > > > + ); > > > + > > > #endif > > > diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c > > > index 574527f8b5ea..bf7872cbbce0 100644 > > > --- a/ArmPkg/Library/OpteeLib/Optee.c > > > +++ b/ArmPkg/Library/OpteeLib/Optee.c > > > @@ -14,11 +14,18 @@ > > > > > > **/ > > > > > > +#include <Library/ArmMmuLib.h> > > > #include <Library/ArmSmcLib.h> > > > +#include <Library/BaseMemoryLib.h> > > > #include <Library/BaseLib.h> > > > +#include <Library/DebugLib.h> > > > #include <Library/OpteeLib.h> > > > > > > #include <IndustryStandard/ArmStdSmc.h> > > > +#include <OpteeSmc.h> > > > +#include <Uefi.h> > > > + > > > +STATIC OPTEE_SHARED_MEMORY_INFO OpteeShmInfo = { 0 }; > > > > > > /** > > > Check for OP-TEE presence. > > > @@ -31,6 +38,7 @@ IsOpteePresent ( > > > { > > > ARM_SMC_ARGS ArmSmcArgs; > > > > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > > // Send a Trusted OS Calls UID command > > > ArmSmcArgs.Arg0 = ARM_SMC_ID_TOS_UID; > > > ArmCallSmc (&ArmSmcArgs); > > > @@ -44,3 +52,352 @@ IsOpteePresent ( > > > return FALSE; > > > } > > > } > > > + > > > +STATIC > > > +EFI_STATUS > > > +OpteeShmMemRemap ( > > > + VOID > > > + ) > > > +{ > > > + ARM_SMC_ARGS ArmSmcArgs; > > > + EFI_PHYSICAL_ADDRESS Paddr; > > > + EFI_PHYSICAL_ADDRESS Start; > > > + EFI_PHYSICAL_ADDRESS End; > > > + EFI_STATUS Status; > > > + UINTN Size; > > > + > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > > + ArmSmcArgs.Arg0 = OPTEE_SMC_GET_SHM_CONFIG; > > > + > > > + ArmCallSmc (&ArmSmcArgs); > > > + if (ArmSmcArgs.Arg0 != OPTEE_SMC_RETURN_OK) { > > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory not supported\n")); > > > + return EFI_UNSUPPORTED; > > > + } > > > + > > > + if (ArmSmcArgs.Arg3 != OPTEE_SMC_SHM_CACHED) { > > > + DEBUG ((DEBUG_WARN, "OP-TEE: Only normal cached shared memory supported\n")); > > > + return EFI_UNSUPPORTED; > > > + } > > > + > > > + Start = (ArmSmcArgs.Arg1 + SIZE_4KB - 1) & ~(SIZE_4KB - 1); > > > + End = (ArmSmcArgs.Arg1 + ArmSmcArgs.Arg2) & ~(SIZE_4KB - 1); > > > + Paddr = Start; > > > + Size = End - Start; > > > + > > > + if (Size < SIZE_4KB) { > > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory too small\n")); > > > + return EFI_BUFFER_TOO_SMALL; > > > + } > > > + > > > + Status = ArmSetMemoryAttributes (Paddr, Size, EFI_MEMORY_WB); > > > + if (EFI_ERROR (Status)) { > > > + return Status; > > > + } > > > + > > > + OpteeShmInfo.Base = (UINTN)Paddr; > > Would PHYSICAL_ADDRESS (always 64-bit) be more correct here, or is the > OP-TEE end seeing this struct as always having fields of native width? > Its a global struct not shared with OP-TEE. Used native width to have compatibility with 32-bit system. > (This is where the ordefile comes in handy, because it means I see the > struct definition first, not its use.) > > > > + OpteeShmInfo.Size = Size; > > > + > > > + return EFI_SUCCESS; > > > +} > > > + > > > +EFI_STATUS > > > +EFIAPI > > > +OpteeInit ( > > > + VOID > > > + ) > > > +{ > > > + EFI_STATUS Status; > > > + > > > + if (!IsOpteePresent ()) { > > > + DEBUG ((DEBUG_WARN, "OP-TEE not present\n")); > > > + return EFI_UNSUPPORTED; > > > + } > > > + > > > + Status = OpteeShmMemRemap (); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory remap failed\n")); > > > + return Status; > > > + } > > > + > > > + return EFI_SUCCESS; > > > +} > > > + > > > +/** > > > + Does Standard SMC to OP-TEE in secure world. > > > + > > > + @param[in] Parg Physical address of message to pass to secure world > > > + > > > + @return 0 on success, secure world return code otherwise > > > + > > > +**/ > > > +STATIC > > > +UINT32 > > > +OpteeCallWithArg ( > > > + IN EFI_PHYSICAL_ADDRESS Parg > > > + ) > > > +{ > > > + ARM_SMC_ARGS ArmSmcArgs; > > > + > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > > + ArmSmcArgs.Arg0 = OPTEE_SMC_CALL_WITH_ARG; > > > + ArmSmcArgs.Arg1 = (UINT32)(Parg >> 32); > > > + ArmSmcArgs.Arg2 = (UINT32)Parg; > > > + > > > + while (TRUE) { > > > + ArmCallSmc (&ArmSmcArgs); > > > + > > > + if (ArmSmcArgs.Arg0 == OPTEE_SMC_RETURN_RPC_FOREIGN_INTR) { > > > + // > > > + // A foreign interrupt was raised while secure world was > > > + // executing, since they are handled in UEFI a dummy RPC is > > > + // performed to let UEFI take the interrupt through the normal > > > + // vector. > > > + // > > > + ArmSmcArgs.Arg0 = OPTEE_SMC_RETURN_FROM_RPC; > > > + } else { > > > + break; > > > + } > > > + } > > > + > > > + return ArmSmcArgs.Arg0; > > > +} > > > + > > > +EFI_STATUS > > > +EFIAPI > > > +OpteeOpenSession ( > > > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > > > + ) > > > +{ > > > + OPTEE_MSG_ARG *MsgArg; > > > + > > > + MsgArg = NULL; > > > + > > > + if (OpteeShmInfo.Base == 0) { > > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > > + return EFI_NOT_STARTED; > > > + } > > > + > > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > > + > > > + MsgArg->Cmd = OPTEE_MSG_CMD_OPEN_SESSION; > > > + > > > + // > > > + // Initialize and add the meta parameters needed when opening a > > > + // session. > > > + // > > > + MsgArg->Params[0].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > > > + OPTEE_MSG_ATTR_META; > > > + MsgArg->Params[1].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > > > + OPTEE_MSG_ATTR_META; > > > + CopyMem (&MsgArg->Params[0].U.Value, OpenSessionArg->Uuid, OPTEE_UUID_LEN); > > > + ZeroMem (&MsgArg->Params[1].U.Value, OPTEE_UUID_LEN); > > > + MsgArg->Params[1].U.Value.C = TEE_LOGIN_PUBLIC; > > > + > > > + MsgArg->NumParams = 2; > > > + > > > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > > + } > > > + > > > + OpenSessionArg->Session = MsgArg->Session; > > > + OpenSessionArg->Ret = MsgArg->Ret; > > > + OpenSessionArg->RetOrigin = MsgArg->RetOrigin; > > > + > > > + return EFI_SUCCESS; > > > +} > > > + > > > +EFI_STATUS > > > +EFIAPI > > > +OpteeCloseSession ( > > > + IN UINT32 Session > > > + ) > > > +{ > > > + OPTEE_MSG_ARG *MsgArg; > > > + > > > + MsgArg = NULL; > > > + > > > + if (OpteeShmInfo.Base == 0) { > > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > > + return EFI_NOT_STARTED; > > > + } > > > + > > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > > + > > > + MsgArg->Cmd = OPTEE_MSG_CMD_CLOSE_SESSION; > > > + MsgArg->Session = Session; > > > + > > > + OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg); > > > + > > > + return EFI_SUCCESS; > > > +} > > > + > > > +STATIC > > > +EFI_STATUS > > > +OpteeToMsgParam ( > > > + OUT OPTEE_MSG_PARAM *MsgParams, > > > + IN UINT32 NumParams, > > > + IN OPTEE_MSG_PARAM *InParams > > > + ) > > > +{ > > > + UINT32 Idx; > > > + UINTN ParamShmAddr; > > > + UINTN ShmSize; > > > + UINTN Size; > > > + > > > + Size = (sizeof (OPTEE_MSG_ARG) + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > > > + ParamShmAddr = OpteeShmInfo.Base + Size; > > > + ShmSize = OpteeShmInfo.Size - Size; > > > + > > > + for (Idx = 0; Idx < NumParams; Idx++) { > > > + CONST OPTEE_MSG_PARAM *Ip; > > > + OPTEE_MSG_PARAM *Mp; > > > + UINT32 Attr; > > > + > > > + Ip = InParams + Idx; > > > + Mp = MsgParams + Idx; > > > + Attr = Ip->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > > > + > > > + switch (Attr) { > > > + case OPTEE_MSG_ATTR_TYPE_NONE: > > > + Mp->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > > > + ZeroMem (&Mp->U, sizeof (Mp->U)); > > > + break; > > > + > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > > > + Mp->Attr = Attr; > > > + Mp->U.Value.A = Ip->U.Value.A; > > > + Mp->U.Value.B = Ip->U.Value.B; > > > + Mp->U.Value.C = Ip->U.Value.C; > > > + break; > > > + > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > > > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > > > + Mp->Attr = Attr; > > > + > > > + if (Ip->U.Mem.Size > ShmSize) { > > > + return EFI_OUT_OF_RESOURCES; > > > + } > > > + > > > + CopyMem ((VOID *)ParamShmAddr, (VOID *)Ip->U.Mem.BufPtr, Ip->U.Mem.Size); > > > + Mp->U.Mem.BufPtr = (UINT64)ParamShmAddr; > > While I don't think it's possible for this to end up doing something > unexpected, casting pointers to a higher alignment (which this would > do on 32-bit) is a bad habit to get into. > This is a shared message param structure with OP-TEE. So needed to cast it. > > > + Mp->U.Mem.Size = Ip->U.Mem.Size; > > > + > > > + Size = (Ip->U.Mem.Size + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > > > + ParamShmAddr += Size; > > > + ShmSize -= Size; > > > + break; > > > + > > > + default: > > > + return EFI_INVALID_PARAMETER; > > > + } > > > + } > > > + > > > + return EFI_SUCCESS; > > > +} > > > + > > > +STATIC > > > +EFI_STATUS > > > +OpteeFromMsgParam ( > > > + OUT OPTEE_MSG_PARAM *OutParams, > > > + IN UINT32 NumParams, > > > + IN OPTEE_MSG_PARAM *MsgParams > > > + ) > > > +{ > > > + UINT32 Idx; > > > + > > > + for (Idx = 0; Idx < NumParams; Idx++) { > > > + OPTEE_MSG_PARAM *Op; > > > + CONST OPTEE_MSG_PARAM *Mp; > > > + UINT32 Attr; > > > + > > > + Op = OutParams + Idx; > > > + Mp = MsgParams + Idx; > > > + Attr = Mp->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > > > + > > > + switch (Attr) { > > > + case OPTEE_MSG_ATTR_TYPE_NONE: > > > + Op->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > > > + ZeroMem (&Op->U, sizeof (Op->U)); > > > + break; > > > + > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > > > + Op->Attr = Attr; > > > + Op->U.Value.A = Mp->U.Value.A; > > > + Op->U.Value.B = Mp->U.Value.B; > > > + Op->U.Value.C = Mp->U.Value.C; > > > + break; > > > + > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > > > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > > > + Op->Attr = Attr; > > > + > > > + if (Mp->U.Mem.Size > Op->U.Mem.Size) { > > > + return EFI_BAD_BUFFER_SIZE; > > > + } > > > + > > > + CopyMem ((VOID *)Op->U.Mem.BufPtr, (VOID *)Mp->U.Mem.BufPtr, Mp->U.Mem.Size); > > > + Op->U.Mem.Size = Mp->U.Mem.Size; > > > + break; > > > + > > > + default: > > > + return EFI_INVALID_PARAMETER; > > > + } > > > + } > > > + > > > + return EFI_SUCCESS; > > > +} > > > + > > > +EFI_STATUS > > > +EFIAPI > > > +OpteeInvokeFunc ( > > > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > > > + ) > > > +{ > > > + EFI_STATUS Status; > > > + OPTEE_MSG_ARG *MsgArg; > > > + > > > + MsgArg = NULL; > > > + > > > + if (OpteeShmInfo.Base == 0) { > > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > > + return EFI_NOT_STARTED; > > > + } > > > + > > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > > + > > > + MsgArg->Cmd = OPTEE_MSG_CMD_INVOKE_COMMAND; > > > + MsgArg->Func = InvokeFuncArg->Func; > > > + MsgArg->Session = InvokeFuncArg->Session; > > > + > > > + Status = OpteeToMsgParam (MsgArg->Params, MAX_PARAMS, InvokeFuncArg->Params); > > > + if (Status) > > > + return Status; > > Always use {} (coding style requirement). > Ok. > > > + > > > + MsgArg->NumParams = MAX_PARAMS; > > > + > > > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > > + } > > > + > > > + if (OpteeFromMsgParam (InvokeFuncArg->Params, MAX_PARAMS, MsgArg->Params)) { > > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > > + } > > > + > > > + InvokeFuncArg->Ret = MsgArg->Ret; > > > + InvokeFuncArg->RetOrigin = MsgArg->RetOrigin; > > > + > > > + return EFI_SUCCESS; > > > +} > > > diff --git a/ArmPkg/Library/OpteeLib/OpteeLib.inf b/ArmPkg/Library/OpteeLib/OpteeLib.inf > > > index 5abd427379cc..e03054a7167d 100644 > > > --- a/ArmPkg/Library/OpteeLib/OpteeLib.inf > > > +++ b/ArmPkg/Library/OpteeLib/OpteeLib.inf > > > @@ -23,11 +23,13 @@ [Defines] > > > > > > [Sources] > > > Optee.c > > > + OpteeSmc.h > > > > > > [Packages] > > > ArmPkg/ArmPkg.dec > > > MdePkg/MdePkg.dec > > > > > > [LibraryClasses] > > > + ArmMmuLib > > > ArmSmcLib > > > BaseLib > > > diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h b/ArmPkg/Library/OpteeLib/OpteeSmc.h > > > new file mode 100644 > > > index 000000000000..e2ea35784a0a > > > --- /dev/null > > > +++ b/ArmPkg/Library/OpteeLib/OpteeSmc.h > > > @@ -0,0 +1,43 @@ > > > +/** @file > > > + OP-TEE SMC header file. > > > + > > > + Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR> > > > + > > > + This program and the accompanying materials > > > + are licensed and made available under the terms and conditions of the BSD License > > > + which accompanies this distribution. The full text of the license may be found at > > > + http://opensource.org/licenses/bsd-license.php > > > + > > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > > + > > > +**/ > > > + > > > +#ifndef _OPTEE_SMC_H_ > > > +#define _OPTEE_SMC_H_ > > > + > > > +/* Returned in Arg0 only from Trusted OS functions */ > > > +#define OPTEE_SMC_RETURN_OK 0x0 > > > + > > > +#define OPTEE_SMC_RETURN_FROM_RPC 0x32000003 > > > +#define OPTEE_SMC_CALL_WITH_ARG 0x32000004 > > > +#define OPTEE_SMC_GET_SHM_CONFIG 0xb2000007 > > > + > > > +#define OPTEE_SMC_SHM_CACHED 1 > > > + > > > +#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTR 0xffff0004 > > > + > > > +#define OPTEE_MSG_CMD_OPEN_SESSION 0 > > > +#define OPTEE_MSG_CMD_INVOKE_COMMAND 1 > > > +#define OPTEE_MSG_CMD_CLOSE_SESSION 2 > > > + > > > +#define OPTEE_MSG_ATTR_META 0x100 > > > + > > > +#define TEE_LOGIN_PUBLIC 0x0 > > I'm a bit apprehensive about taking over the whole TEE namespace in an > implementation-specific header file. > Can/should this be OPTEE_? > Ok will change this to OPTEE_. -Sumit > > > + > > > +typedef struct { > > > + UINTN Base; > > > + UINTN Size; > > > +} OPTEE_SHARED_MEMORY_INFO; > > (See comments above.) > > / > Leif > > > > + > > > +#endif > > > -- > > > 2.7.4 > > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Mon, Oct 08, 2018 at 03:20:27PM +0530, Sumit Garg wrote: > On Fri, 5 Oct 2018 at 20:33, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > > > On Wed, Oct 03, 2018 at 11:33:01AM +0200, Ard Biesheuvel wrote: > > > On 3 October 2018 at 09:43, Sumit Garg <sumit.garg@linaro.org> wrote: > > > > Add following APIs to communicate with OP-TEE pseudo/early TAs: > > > > 1. OpteeInit > > > > 2. OpteeOpenSession > > > > 3. OpteeCloseSession > > > > 4. OpteeInvokeFunc > > > > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > > > > > Given the outcome of the GP discussion, I'm fine with this approach. Leif? > > > > Apologies for the delay, I needed some time to think it over. > > > > I'm not super happy about this approach, but I'm happier with this > > than I would be with leaving the functionality out of the upstream > > tree. I really hope we can get that license either changed or dropped > > completely. > > > > Thanks Leif for this go ahead. > > > I do have a few comments below. > > > > > > --- > > > > ArmPkg/Include/Library/OpteeLib.h | 90 +++++++++ > > > > ArmPkg/Library/OpteeLib/Optee.c | 357 +++++++++++++++++++++++++++++++++++ > > > > ArmPkg/Library/OpteeLib/OpteeLib.inf | 2 + > > > > ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++++ > > > > Could you follow the instructions in > > https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23 > > when generating future patches? > > The --stat* effects aren't apparent here, but the -O ones are. > > > > Sure. > > > > > 4 files changed, 492 insertions(+) > > > > create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h > > > > > > > > diff --git a/ArmPkg/Include/Library/OpteeLib.h b/ArmPkg/Include/Library/OpteeLib.h > > > > index f65d8674d9b8..2d1c60632dfe 100644 > > > > --- a/ArmPkg/Include/Library/OpteeLib.h > > > > +++ b/ArmPkg/Include/Library/OpteeLib.h > > > > @@ -25,10 +25,100 @@ > > > > #define OPTEE_OS_UID2 0xaf630002 > > > > #define OPTEE_OS_UID3 0xa5d5c51b > > > > > > > > +#define OPTEE_MSG_ATTR_TYPE_NONE 0x0 > > > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INPUT 0x1 > > > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT 0x2 > > > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INOUT 0x3 > > > > +#define OPTEE_MSG_ATTR_TYPE_MEM_INPUT 0x9 > > > > +#define OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT 0xa > > > > +#define OPTEE_MSG_ATTR_TYPE_MEM_INOUT 0xb > > > > + > > > > +#define OPTEE_MSG_ATTR_TYPE_MASK 0xff > > > > + > > > > +#define OPTEE_ORIGIN_COMMS 0x00000002 > > > > +#define OPTEE_ERROR_COMMS 0xFFFF000E > > > > + > > > > +typedef struct { > > > > + UINT64 BufPtr; > > > > If it's a pointer, it has a *. > > Otherwise it's an address. > > > > Will rather use BufferAddress. > > > > > + UINT64 Size; > > > > + UINT64 ShmRef; > > > > Abbreviations in function, variable or type names (other than the ones > > defined in [1] are not permitted unless they are explicitly added to a > > glossary section of the source file header. Where possible, just write > > out the name in full. > > > > [1] https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/4_naming_conventions/#table-2-efi-supported-abbreviations > > > > BufPtr (as a name) gets a pass since it's unambiguous and we already > > have a bunch of those in the codebase. ShmRef needs to be clear. > > > > Will replace it with SharedMemoryReference. > > > (This comment also applies to a lot of things below, I won't point > > them all out unless asked to.) > > > > I will try to replace most of them with full names. But I have > confusion regarding usage of following not included in [1]: > > 1. MSG/Msg > 2. MEM/Mem > 3. INFO/Info > 4. FUNC/Func > 5. RET/Ret > 6. ATTR/Attr > 7. CMD/Cmd > > But I do see their usage in the codebase. Please help to clarify. It's a horrible secret, but not all of the codebase conforms to the coding style. We're continuously trying to improve it. > Also can I use 1-2 letter variable or struct member names like Mp, Ip, > Op, U etc.? Ideally not. I'm not a huge fan of ReallyLongCamelCase coding styles, but the benefit it does bring is that you don't need to stop and think about what a variable may be for. And correspondingly, mixing between abbreviations an non-abbreviations make for hard reading. Another completely unscientific observation I would make is that once you force yourself to write a full variable name out, it becomes a lot more clear when you've left valuable semantic information out. The codebase ends up with a bunch of fewer variables called only 'Msg', 'Mem', 'Info', Func', 'Ret', 'Attr' 'Cmd' (, or 'Data' ...). (Alternatively, the mailing list ends up with a lot fewer emails asking "what message", "what memory", "what information" ...) > > > > +} OPTEE_MSG_PARAM_MEM; > > > > + > > > > +typedef struct { > > > > + UINT64 A; > > > > + UINT64 B; > > > > + UINT64 C; > > > > +} OPTEE_MSG_PARAM_VALUE; > > > > + > > > > +typedef struct { > > > > + UINT64 Attr; > > > > + union { > > > > + OPTEE_MSG_PARAM_MEM Mem; > > > > + OPTEE_MSG_PARAM_VALUE Value; > > > > + } U; > > > > +} OPTEE_MSG_PARAM; > > > > + > > > > +#define MAX_PARAMS 4 > > > > This is a very localised macro with a very globalised name. > > Suggest adding an OPTEE_ prefix, but also something describing what it > > is the maximum parameters for. CALL_? > > > > Ok will rename it to OPTEE_CALL_MAX_PARAMS. I'd take that or OPTEE_MAX_CALL_PARAMS. > > > > + > > > > +typedef struct { > > > > + UINT32 Cmd; > > > > + UINT32 Func; > > > > + UINT32 Session; > > > > + UINT32 CancelId; > > > > + UINT32 Pad; > > > > + UINT32 Ret; > > > > + UINT32 RetOrigin; > > > > + UINT32 NumParams; > > > > + > > > > + // NumParams tells the actual number of element in Params > > > > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; > > > > +} OPTEE_MSG_ARG; > > > > + > > > > +#define OPTEE_UUID_LEN 16 > > > > UUIDs are UUIDs. If optee decides on an incompatible format, we may > > have an interoperability issue. I assume this is not the case, so > > perhaps replace with sizeof (EFI_GUID) at each point of use? > > Agree will use sizeof (EFI_GUID) instead. > > > > > + > > > > +typedef struct { > > > > + UINT8 Uuid[OPTEE_UUID_LEN]; // [in] UUID of the Trusted Application > > > > Is there a strong reason for not using EFI_GUID here? > > > > EFI_GUID can be used here as UUID are also known as GUID. But here we > pass UUID as 16 octets to OP-TEE. So will use EFI_GUID instead and add > an api to convert uuid to octets. Right. That may be tedious, but I think it will be an improvement. If OPTEE uses UUIDs without the Microsoft timestamp endianness variant (as described in the UEFI spec), then ideally those functions could byte reverse the timestamp as well. > > > > + UINT32 Session; // [out] Session id > > > > + UINT32 Ret; // [out] Return value > > > > + UINT32 RetOrigin; // [out] Origin of the return value > > > > +} OPTEE_OPEN_SESSION_ARG; > > > > + > > > > +typedef struct { > > > > + UINT32 Func; // [in] Trusted App func, specific to the TA > > > > + UINT32 Session; // [in] Session id > > > > + UINT32 Ret; // [out] Return value > > > > + UINT32 RetOrigin; // [out] Origin of the return value > > > > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; // Params for func to be invoked > > > > +} OPTEE_INVOKE_FUNC_ARG; > > > > + > > > > BOOLEAN > > > > EFIAPI > > > > IsOpteePresent ( > > > > VOID > > > > ); > > > > > > > > +EFI_STATUS > > > > +EFIAPI > > > > +OpteeInit ( > > > > + VOID > > > > + ); > > > > + > > > > +EFI_STATUS > > > > +EFIAPI > > > > +OpteeOpenSession ( > > > > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > > > > + ); > > > > + > > > > +EFI_STATUS > > > > +EFIAPI > > > > +OpteeCloseSession ( > > > > + IN UINT32 Session > > > > + ); > > > > + > > > > +EFI_STATUS > > > > +EFIAPI > > > > +OpteeInvokeFunc ( > > > > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > > > > + ); > > > > + > > > > #endif > > > > diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c > > > > index 574527f8b5ea..bf7872cbbce0 100644 > > > > --- a/ArmPkg/Library/OpteeLib/Optee.c > > > > +++ b/ArmPkg/Library/OpteeLib/Optee.c > > > > @@ -14,11 +14,18 @@ > > > > > > > > **/ > > > > > > > > +#include <Library/ArmMmuLib.h> > > > > #include <Library/ArmSmcLib.h> > > > > +#include <Library/BaseMemoryLib.h> > > > > #include <Library/BaseLib.h> > > > > +#include <Library/DebugLib.h> > > > > #include <Library/OpteeLib.h> > > > > > > > > #include <IndustryStandard/ArmStdSmc.h> > > > > +#include <OpteeSmc.h> > > > > +#include <Uefi.h> > > > > + > > > > +STATIC OPTEE_SHARED_MEMORY_INFO OpteeShmInfo = { 0 }; > > > > > > > > /** > > > > Check for OP-TEE presence. > > > > @@ -31,6 +38,7 @@ IsOpteePresent ( > > > > { > > > > ARM_SMC_ARGS ArmSmcArgs; > > > > > > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > > > // Send a Trusted OS Calls UID command > > > > ArmSmcArgs.Arg0 = ARM_SMC_ID_TOS_UID; > > > > ArmCallSmc (&ArmSmcArgs); > > > > @@ -44,3 +52,352 @@ IsOpteePresent ( > > > > return FALSE; > > > > } > > > > } > > > > + > > > > +STATIC > > > > +EFI_STATUS > > > > +OpteeShmMemRemap ( > > > > + VOID > > > > + ) > > > > +{ > > > > + ARM_SMC_ARGS ArmSmcArgs; > > > > + EFI_PHYSICAL_ADDRESS Paddr; > > > > + EFI_PHYSICAL_ADDRESS Start; > > > > + EFI_PHYSICAL_ADDRESS End; > > > > + EFI_STATUS Status; > > > > + UINTN Size; > > > > + > > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > > > + ArmSmcArgs.Arg0 = OPTEE_SMC_GET_SHM_CONFIG; > > > > + > > > > + ArmCallSmc (&ArmSmcArgs); > > > > + if (ArmSmcArgs.Arg0 != OPTEE_SMC_RETURN_OK) { > > > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory not supported\n")); > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + if (ArmSmcArgs.Arg3 != OPTEE_SMC_SHM_CACHED) { > > > > + DEBUG ((DEBUG_WARN, "OP-TEE: Only normal cached shared memory supported\n")); > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + Start = (ArmSmcArgs.Arg1 + SIZE_4KB - 1) & ~(SIZE_4KB - 1); > > > > + End = (ArmSmcArgs.Arg1 + ArmSmcArgs.Arg2) & ~(SIZE_4KB - 1); > > > > + Paddr = Start; > > > > + Size = End - Start; > > > > + > > > > + if (Size < SIZE_4KB) { > > > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory too small\n")); > > > > + return EFI_BUFFER_TOO_SMALL; > > > > + } > > > > + > > > > + Status = ArmSetMemoryAttributes (Paddr, Size, EFI_MEMORY_WB); > > > > + if (EFI_ERROR (Status)) { > > > > + return Status; > > > > + } > > > > + > > > > + OpteeShmInfo.Base = (UINTN)Paddr; > > > > Would PHYSICAL_ADDRESS (always 64-bit) be more correct here, or is the > > OP-TEE end seeing this struct as always having fields of native width? > > Its a global struct not shared with OP-TEE. Used native width to have > compatibility with 32-bit system. OK, that is not a problem then. > > (This is where the ordefile comes in handy, because it means I see the > > struct definition first, not its use.) > > > > > > + OpteeShmInfo.Size = Size; > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +EFI_STATUS > > > > +EFIAPI > > > > +OpteeInit ( > > > > + VOID > > > > + ) > > > > +{ > > > > + EFI_STATUS Status; > > > > + > > > > + if (!IsOpteePresent ()) { > > > > + DEBUG ((DEBUG_WARN, "OP-TEE not present\n")); > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + Status = OpteeShmMemRemap (); > > > > + if (EFI_ERROR (Status)) { > > > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory remap failed\n")); > > > > + return Status; > > > > + } > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +/** > > > > + Does Standard SMC to OP-TEE in secure world. > > > > + > > > > + @param[in] Parg Physical address of message to pass to secure world > > > > + > > > > + @return 0 on success, secure world return code otherwise > > > > + > > > > +**/ > > > > +STATIC > > > > +UINT32 > > > > +OpteeCallWithArg ( > > > > + IN EFI_PHYSICAL_ADDRESS Parg > > > > + ) > > > > +{ > > > > + ARM_SMC_ARGS ArmSmcArgs; > > > > + > > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > > > + ArmSmcArgs.Arg0 = OPTEE_SMC_CALL_WITH_ARG; > > > > + ArmSmcArgs.Arg1 = (UINT32)(Parg >> 32); > > > > + ArmSmcArgs.Arg2 = (UINT32)Parg; > > > > + > > > > + while (TRUE) { > > > > + ArmCallSmc (&ArmSmcArgs); > > > > + > > > > + if (ArmSmcArgs.Arg0 == OPTEE_SMC_RETURN_RPC_FOREIGN_INTR) { > > > > + // > > > > + // A foreign interrupt was raised while secure world was > > > > + // executing, since they are handled in UEFI a dummy RPC is > > > > + // performed to let UEFI take the interrupt through the normal > > > > + // vector. > > > > + // > > > > + ArmSmcArgs.Arg0 = OPTEE_SMC_RETURN_FROM_RPC; > > > > + } else { > > > > + break; > > > > + } > > > > + } > > > > + > > > > + return ArmSmcArgs.Arg0; > > > > +} > > > > + > > > > +EFI_STATUS > > > > +EFIAPI > > > > +OpteeOpenSession ( > > > > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > > > > + ) > > > > +{ > > > > + OPTEE_MSG_ARG *MsgArg; > > > > + > > > > + MsgArg = NULL; > > > > + > > > > + if (OpteeShmInfo.Base == 0) { > > > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > > > + return EFI_NOT_STARTED; > > > > + } > > > > + > > > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > > > + > > > > + MsgArg->Cmd = OPTEE_MSG_CMD_OPEN_SESSION; > > > > + > > > > + // > > > > + // Initialize and add the meta parameters needed when opening a > > > > + // session. > > > > + // > > > > + MsgArg->Params[0].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > > > > + OPTEE_MSG_ATTR_META; > > > > + MsgArg->Params[1].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > > > > + OPTEE_MSG_ATTR_META; > > > > + CopyMem (&MsgArg->Params[0].U.Value, OpenSessionArg->Uuid, OPTEE_UUID_LEN); > > > > + ZeroMem (&MsgArg->Params[1].U.Value, OPTEE_UUID_LEN); > > > > + MsgArg->Params[1].U.Value.C = TEE_LOGIN_PUBLIC; > > > > + > > > > + MsgArg->NumParams = 2; > > > > + > > > > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > > > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > > > + } > > > > + > > > > + OpenSessionArg->Session = MsgArg->Session; > > > > + OpenSessionArg->Ret = MsgArg->Ret; > > > > + OpenSessionArg->RetOrigin = MsgArg->RetOrigin; > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +EFI_STATUS > > > > +EFIAPI > > > > +OpteeCloseSession ( > > > > + IN UINT32 Session > > > > + ) > > > > +{ > > > > + OPTEE_MSG_ARG *MsgArg; > > > > + > > > > + MsgArg = NULL; > > > > + > > > > + if (OpteeShmInfo.Base == 0) { > > > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > > > + return EFI_NOT_STARTED; > > > > + } > > > > + > > > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > > > + > > > > + MsgArg->Cmd = OPTEE_MSG_CMD_CLOSE_SESSION; > > > > + MsgArg->Session = Session; > > > > + > > > > + OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg); > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +STATIC > > > > +EFI_STATUS > > > > +OpteeToMsgParam ( > > > > + OUT OPTEE_MSG_PARAM *MsgParams, > > > > + IN UINT32 NumParams, > > > > + IN OPTEE_MSG_PARAM *InParams > > > > + ) > > > > +{ > > > > + UINT32 Idx; > > > > + UINTN ParamShmAddr; > > > > + UINTN ShmSize; > > > > + UINTN Size; > > > > + > > > > + Size = (sizeof (OPTEE_MSG_ARG) + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > > > > + ParamShmAddr = OpteeShmInfo.Base + Size; > > > > + ShmSize = OpteeShmInfo.Size - Size; > > > > + > > > > + for (Idx = 0; Idx < NumParams; Idx++) { > > > > + CONST OPTEE_MSG_PARAM *Ip; > > > > + OPTEE_MSG_PARAM *Mp; > > > > + UINT32 Attr; > > > > + > > > > + Ip = InParams + Idx; > > > > + Mp = MsgParams + Idx; > > > > + Attr = Ip->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > > > > + > > > > + switch (Attr) { > > > > + case OPTEE_MSG_ATTR_TYPE_NONE: > > > > + Mp->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > > > > + ZeroMem (&Mp->U, sizeof (Mp->U)); > > > > + break; > > > > + > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > > > > + Mp->Attr = Attr; > > > > + Mp->U.Value.A = Ip->U.Value.A; > > > > + Mp->U.Value.B = Ip->U.Value.B; > > > > + Mp->U.Value.C = Ip->U.Value.C; > > > > + break; > > > > + > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > > > > + Mp->Attr = Attr; > > > > + > > > > + if (Ip->U.Mem.Size > ShmSize) { > > > > + return EFI_OUT_OF_RESOURCES; > > > > + } > > > > + > > > > + CopyMem ((VOID *)ParamShmAddr, (VOID *)Ip->U.Mem.BufPtr, Ip->U.Mem.Size); > > > > + Mp->U.Mem.BufPtr = (UINT64)ParamShmAddr; > > > > While I don't think it's possible for this to end up doing something > > unexpected, casting pointers to a higher alignment (which this would > > do on 32-bit) is a bad habit to get into. > > This is a shared message param structure with OP-TEE. So needed to cast it. Then you need to also ensure that you are not accidentally casting a less-than-8-byte aligned pointer here. I would suggest an explicit test with an error return and an assert. Then if that thing ever does come back to bite you, you will know why, rather than needing to spend a few days debugging. Alternatively, make ParamShmAddr an UINT64 * to begin with. > > > > + Mp->U.Mem.Size = Ip->U.Mem.Size; > > > > + > > > > + Size = (Ip->U.Mem.Size + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > > > > + ParamShmAddr += Size; > > > > + ShmSize -= Size; > > > > + break; > > > > + > > > > + default: > > > > + return EFI_INVALID_PARAMETER; > > > > + } > > > > + } > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +STATIC > > > > +EFI_STATUS > > > > +OpteeFromMsgParam ( > > > > + OUT OPTEE_MSG_PARAM *OutParams, > > > > + IN UINT32 NumParams, > > > > + IN OPTEE_MSG_PARAM *MsgParams > > > > + ) > > > > +{ > > > > + UINT32 Idx; > > > > + > > > > + for (Idx = 0; Idx < NumParams; Idx++) { > > > > + OPTEE_MSG_PARAM *Op; > > > > + CONST OPTEE_MSG_PARAM *Mp; > > > > + UINT32 Attr; > > > > + > > > > + Op = OutParams + Idx; > > > > + Mp = MsgParams + Idx; > > > > + Attr = Mp->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > > > > + > > > > + switch (Attr) { > > > > + case OPTEE_MSG_ATTR_TYPE_NONE: > > > > + Op->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > > > > + ZeroMem (&Op->U, sizeof (Op->U)); > > > > + break; > > > > + > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > > > > + Op->Attr = Attr; > > > > + Op->U.Value.A = Mp->U.Value.A; > > > > + Op->U.Value.B = Mp->U.Value.B; > > > > + Op->U.Value.C = Mp->U.Value.C; > > > > + break; > > > > + > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > > > > + Op->Attr = Attr; > > > > + > > > > + if (Mp->U.Mem.Size > Op->U.Mem.Size) { > > > > + return EFI_BAD_BUFFER_SIZE; > > > > + } > > > > + > > > > + CopyMem ((VOID *)Op->U.Mem.BufPtr, (VOID *)Mp->U.Mem.BufPtr, Mp->U.Mem.Size); > > > > + Op->U.Mem.Size = Mp->U.Mem.Size; > > > > + break; > > > > + > > > > + default: > > > > + return EFI_INVALID_PARAMETER; > > > > + } > > > > + } > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +EFI_STATUS > > > > +EFIAPI > > > > +OpteeInvokeFunc ( > > > > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > > > > + ) > > > > +{ > > > > + EFI_STATUS Status; > > > > + OPTEE_MSG_ARG *MsgArg; > > > > + > > > > + MsgArg = NULL; > > > > + > > > > + if (OpteeShmInfo.Base == 0) { > > > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > > > + return EFI_NOT_STARTED; > > > > + } > > > > + > > > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > > > + > > > > + MsgArg->Cmd = OPTEE_MSG_CMD_INVOKE_COMMAND; > > > > + MsgArg->Func = InvokeFuncArg->Func; > > > > + MsgArg->Session = InvokeFuncArg->Session; > > > > + > > > > + Status = OpteeToMsgParam (MsgArg->Params, MAX_PARAMS, InvokeFuncArg->Params); > > > > + if (Status) > > > > + return Status; > > > > Always use {} (coding style requirement). > > > > Ok. > > > > > + > > > > + MsgArg->NumParams = MAX_PARAMS; > > > > + > > > > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > > > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > > > + } > > > > + > > > > + if (OpteeFromMsgParam (InvokeFuncArg->Params, MAX_PARAMS, MsgArg->Params)) { > > > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > > > + } > > > > + > > > > + InvokeFuncArg->Ret = MsgArg->Ret; > > > > + InvokeFuncArg->RetOrigin = MsgArg->RetOrigin; > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > diff --git a/ArmPkg/Library/OpteeLib/OpteeLib.inf b/ArmPkg/Library/OpteeLib/OpteeLib.inf > > > > index 5abd427379cc..e03054a7167d 100644 > > > > --- a/ArmPkg/Library/OpteeLib/OpteeLib.inf > > > > +++ b/ArmPkg/Library/OpteeLib/OpteeLib.inf > > > > @@ -23,11 +23,13 @@ [Defines] > > > > > > > > [Sources] > > > > Optee.c > > > > + OpteeSmc.h > > > > > > > > [Packages] > > > > ArmPkg/ArmPkg.dec > > > > MdePkg/MdePkg.dec > > > > > > > > [LibraryClasses] > > > > + ArmMmuLib > > > > ArmSmcLib > > > > BaseLib > > > > diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h b/ArmPkg/Library/OpteeLib/OpteeSmc.h > > > > new file mode 100644 > > > > index 000000000000..e2ea35784a0a > > > > --- /dev/null > > > > +++ b/ArmPkg/Library/OpteeLib/OpteeSmc.h > > > > @@ -0,0 +1,43 @@ > > > > +/** @file > > > > + OP-TEE SMC header file. > > > > + > > > > + Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR> > > > > + > > > > + This program and the accompanying materials > > > > + are licensed and made available under the terms and conditions of the BSD License > > > > + which accompanies this distribution. The full text of the license may be found at > > > > + http://opensource.org/licenses/bsd-license.php > > > > + > > > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > > > + > > > > +**/ > > > > + > > > > +#ifndef _OPTEE_SMC_H_ > > > > +#define _OPTEE_SMC_H_ > > > > + > > > > +/* Returned in Arg0 only from Trusted OS functions */ > > > > +#define OPTEE_SMC_RETURN_OK 0x0 > > > > + > > > > +#define OPTEE_SMC_RETURN_FROM_RPC 0x32000003 > > > > +#define OPTEE_SMC_CALL_WITH_ARG 0x32000004 > > > > +#define OPTEE_SMC_GET_SHM_CONFIG 0xb2000007 > > > > + > > > > +#define OPTEE_SMC_SHM_CACHED 1 > > > > + > > > > +#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTR 0xffff0004 > > > > + > > > > +#define OPTEE_MSG_CMD_OPEN_SESSION 0 > > > > +#define OPTEE_MSG_CMD_INVOKE_COMMAND 1 > > > > +#define OPTEE_MSG_CMD_CLOSE_SESSION 2 > > > > + > > > > +#define OPTEE_MSG_ATTR_META 0x100 > > > > + > > > > +#define TEE_LOGIN_PUBLIC 0x0 > > > > I'm a bit apprehensive about taking over the whole TEE namespace in an > > implementation-specific header file. > > Can/should this be OPTEE_? > > > > Ok will change this to OPTEE_. Thanks! / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Mon, 8 Oct 2018 at 19:35, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Mon, Oct 08, 2018 at 03:20:27PM +0530, Sumit Garg wrote: > > On Fri, 5 Oct 2018 at 20:33, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > > > > > On Wed, Oct 03, 2018 at 11:33:01AM +0200, Ard Biesheuvel wrote: > > > > On 3 October 2018 at 09:43, Sumit Garg <sumit.garg@linaro.org> wrote: > > > > > Add following APIs to communicate with OP-TEE pseudo/early TAs: > > > > > 1. OpteeInit > > > > > 2. OpteeOpenSession > > > > > 3. OpteeCloseSession > > > > > 4. OpteeInvokeFunc > > > > > > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > > > > > > > Given the outcome of the GP discussion, I'm fine with this approach. Leif? > > > > > > Apologies for the delay, I needed some time to think it over. > > > > > > I'm not super happy about this approach, but I'm happier with this > > > than I would be with leaving the functionality out of the upstream > > > tree. I really hope we can get that license either changed or dropped > > > completely. > > > > > > > Thanks Leif for this go ahead. > > > > > I do have a few comments below. > > > > > > > > --- > > > > > ArmPkg/Include/Library/OpteeLib.h | 90 +++++++++ > > > > > ArmPkg/Library/OpteeLib/Optee.c | 357 +++++++++++++++++++++++++++++++++++ > > > > > ArmPkg/Library/OpteeLib/OpteeLib.inf | 2 + > > > > > ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++++ > > > > > > Could you follow the instructions in > > > https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23 > > > when generating future patches? > > > The --stat* effects aren't apparent here, but the -O ones are. > > > > > > > Sure. > > > > > > > 4 files changed, 492 insertions(+) > > > > > create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h > > > > > > > > > > diff --git a/ArmPkg/Include/Library/OpteeLib.h b/ArmPkg/Include/Library/OpteeLib.h > > > > > index f65d8674d9b8..2d1c60632dfe 100644 > > > > > --- a/ArmPkg/Include/Library/OpteeLib.h > > > > > +++ b/ArmPkg/Include/Library/OpteeLib.h > > > > > @@ -25,10 +25,100 @@ > > > > > #define OPTEE_OS_UID2 0xaf630002 > > > > > #define OPTEE_OS_UID3 0xa5d5c51b > > > > > > > > > > +#define OPTEE_MSG_ATTR_TYPE_NONE 0x0 > > > > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INPUT 0x1 > > > > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT 0x2 > > > > > +#define OPTEE_MSG_ATTR_TYPE_VALUE_INOUT 0x3 > > > > > +#define OPTEE_MSG_ATTR_TYPE_MEM_INPUT 0x9 > > > > > +#define OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT 0xa > > > > > +#define OPTEE_MSG_ATTR_TYPE_MEM_INOUT 0xb > > > > > + > > > > > +#define OPTEE_MSG_ATTR_TYPE_MASK 0xff > > > > > + > > > > > +#define OPTEE_ORIGIN_COMMS 0x00000002 > > > > > +#define OPTEE_ERROR_COMMS 0xFFFF000E > > > > > + > > > > > +typedef struct { > > > > > + UINT64 BufPtr; > > > > > > If it's a pointer, it has a *. > > > Otherwise it's an address. > > > > > > > Will rather use BufferAddress. > > > > > > > + UINT64 Size; > > > > > + UINT64 ShmRef; > > > > > > Abbreviations in function, variable or type names (other than the ones > > > defined in [1] are not permitted unless they are explicitly added to a > > > glossary section of the source file header. Where possible, just write > > > out the name in full. > > > > > > [1] https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/4_naming_conventions/#table-2-efi-supported-abbreviations > > > > > > BufPtr (as a name) gets a pass since it's unambiguous and we already > > > have a bunch of those in the codebase. ShmRef needs to be clear. > > > > > > > Will replace it with SharedMemoryReference. > > > > > (This comment also applies to a lot of things below, I won't point > > > them all out unless asked to.) > > > > > > > I will try to replace most of them with full names. But I have > > confusion regarding usage of following not included in [1]: > > > > 1. MSG/Msg > > 2. MEM/Mem > > 3. INFO/Info > > 4. FUNC/Func > > 5. RET/Ret > > 6. ATTR/Attr > > 7. CMD/Cmd > > > > But I do see their usage in the codebase. Please help to clarify. > > It's a horrible secret, but not all of the codebase conforms to the > coding style. We're continuously trying to improve it. > > > Also can I use 1-2 letter variable or struct member names like Mp, Ip, > > Op, U etc.? > > Ideally not. I'm not a huge fan of ReallyLongCamelCase coding styles, > but the benefit it does bring is that you don't need to stop and think > about what a variable may be for. And correspondingly, mixing between > abbreviations an non-abbreviations make for hard reading. > > Another completely unscientific observation I would make is that once > you force yourself to write a full variable name out, it becomes a lot > more clear when you've left valuable semantic information out. > The codebase ends up with a bunch of fewer variables called only > 'Msg', 'Mem', 'Info', Func', 'Ret', 'Attr' 'Cmd' (, or 'Data' ...). > (Alternatively, the mailing list ends up with a lot fewer emails > asking "what message", "what memory", "what information" ...) > Ok then I will replace these abbreviations with full name too in v4. > > > > > +} OPTEE_MSG_PARAM_MEM; > > > > > + > > > > > +typedef struct { > > > > > + UINT64 A; > > > > > + UINT64 B; > > > > > + UINT64 C; > > > > > +} OPTEE_MSG_PARAM_VALUE; > > > > > + > > > > > +typedef struct { > > > > > + UINT64 Attr; > > > > > + union { > > > > > + OPTEE_MSG_PARAM_MEM Mem; > > > > > + OPTEE_MSG_PARAM_VALUE Value; > > > > > + } U; > > > > > +} OPTEE_MSG_PARAM; > > > > > + > > > > > +#define MAX_PARAMS 4 > > > > > > This is a very localised macro with a very globalised name. > > > Suggest adding an OPTEE_ prefix, but also something describing what it > > > is the maximum parameters for. CALL_? > > > > > > > Ok will rename it to OPTEE_CALL_MAX_PARAMS. > > I'd take that or OPTEE_MAX_CALL_PARAMS. > OPTEE_MAX_CALL_PARAMS sounds more apt. Will use it instead. > > > > > + > > > > > +typedef struct { > > > > > + UINT32 Cmd; > > > > > + UINT32 Func; > > > > > + UINT32 Session; > > > > > + UINT32 CancelId; > > > > > + UINT32 Pad; > > > > > + UINT32 Ret; > > > > > + UINT32 RetOrigin; > > > > > + UINT32 NumParams; > > > > > + > > > > > + // NumParams tells the actual number of element in Params > > > > > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; > > > > > +} OPTEE_MSG_ARG; > > > > > + > > > > > +#define OPTEE_UUID_LEN 16 > > > > > > UUIDs are UUIDs. If optee decides on an incompatible format, we may > > > have an interoperability issue. I assume this is not the case, so > > > perhaps replace with sizeof (EFI_GUID) at each point of use? > > > > Agree will use sizeof (EFI_GUID) instead. > > > > > > > + > > > > > +typedef struct { > > > > > + UINT8 Uuid[OPTEE_UUID_LEN]; // [in] UUID of the Trusted Application > > > > > > Is there a strong reason for not using EFI_GUID here? > > > > > > > EFI_GUID can be used here as UUID are also known as GUID. But here we > > pass UUID as 16 octets to OP-TEE. So will use EFI_GUID instead and add > > an api to convert uuid to octets. > > Right. That may be tedious, but I think it will be an improvement. > If OPTEE uses UUIDs without the Microsoft timestamp endianness variant > (as described in the UEFI spec), then ideally those functions could > byte reverse the timestamp as well. > Yes, OPTEE uses UUID variant as per RFC-4122 [1] which states that each UUID field is encoded with the Most Significant Byte first (known as network byte order). So api to convert UUID to octets would do following operation: +STATIC +VOID +UuidToOctets ( + OUT UINT8 *UuidOctet, + IN EFI_GUID *Uuid + ) +{ + UuidOctet[0] = Uuid->Data1 >> 24; + UuidOctet[1] = Uuid->Data1 >> 16; + UuidOctet[2] = Uuid->Data1 >> 8; + UuidOctet[3] = Uuid->Data1; + UuidOctet[4] = Uuid->Data2 >> 8; + UuidOctet[5] = Uuid->Data2; + UuidOctet[6] = Uuid->Data3 >> 8; + UuidOctet[7] = Uuid->Data3; + CopyMem (UuidOctet + 8, Uuid->Data4, sizeof (Uuid->Data4)); +} [1] https://tools.ietf.org/html/rfc4122 > > > > > + UINT32 Session; // [out] Session id > > > > > + UINT32 Ret; // [out] Return value > > > > > + UINT32 RetOrigin; // [out] Origin of the return value > > > > > +} OPTEE_OPEN_SESSION_ARG; > > > > > + > > > > > +typedef struct { > > > > > + UINT32 Func; // [in] Trusted App func, specific to the TA > > > > > + UINT32 Session; // [in] Session id > > > > > + UINT32 Ret; // [out] Return value > > > > > + UINT32 RetOrigin; // [out] Origin of the return value > > > > > + OPTEE_MSG_PARAM Params[MAX_PARAMS]; // Params for func to be invoked > > > > > +} OPTEE_INVOKE_FUNC_ARG; > > > > > + > > > > > BOOLEAN > > > > > EFIAPI > > > > > IsOpteePresent ( > > > > > VOID > > > > > ); > > > > > > > > > > +EFI_STATUS > > > > > +EFIAPI > > > > > +OpteeInit ( > > > > > + VOID > > > > > + ); > > > > > + > > > > > +EFI_STATUS > > > > > +EFIAPI > > > > > +OpteeOpenSession ( > > > > > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > > > > > + ); > > > > > + > > > > > +EFI_STATUS > > > > > +EFIAPI > > > > > +OpteeCloseSession ( > > > > > + IN UINT32 Session > > > > > + ); > > > > > + > > > > > +EFI_STATUS > > > > > +EFIAPI > > > > > +OpteeInvokeFunc ( > > > > > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > > > > > + ); > > > > > + > > > > > #endif > > > > > diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c > > > > > index 574527f8b5ea..bf7872cbbce0 100644 > > > > > --- a/ArmPkg/Library/OpteeLib/Optee.c > > > > > +++ b/ArmPkg/Library/OpteeLib/Optee.c > > > > > @@ -14,11 +14,18 @@ > > > > > > > > > > **/ > > > > > > > > > > +#include <Library/ArmMmuLib.h> > > > > > #include <Library/ArmSmcLib.h> > > > > > +#include <Library/BaseMemoryLib.h> > > > > > #include <Library/BaseLib.h> > > > > > +#include <Library/DebugLib.h> > > > > > #include <Library/OpteeLib.h> > > > > > > > > > > #include <IndustryStandard/ArmStdSmc.h> > > > > > +#include <OpteeSmc.h> > > > > > +#include <Uefi.h> > > > > > + > > > > > +STATIC OPTEE_SHARED_MEMORY_INFO OpteeShmInfo = { 0 }; > > > > > > > > > > /** > > > > > Check for OP-TEE presence. > > > > > @@ -31,6 +38,7 @@ IsOpteePresent ( > > > > > { > > > > > ARM_SMC_ARGS ArmSmcArgs; > > > > > > > > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > > > > // Send a Trusted OS Calls UID command > > > > > ArmSmcArgs.Arg0 = ARM_SMC_ID_TOS_UID; > > > > > ArmCallSmc (&ArmSmcArgs); > > > > > @@ -44,3 +52,352 @@ IsOpteePresent ( > > > > > return FALSE; > > > > > } > > > > > } > > > > > + > > > > > +STATIC > > > > > +EFI_STATUS > > > > > +OpteeShmMemRemap ( > > > > > + VOID > > > > > + ) > > > > > +{ > > > > > + ARM_SMC_ARGS ArmSmcArgs; > > > > > + EFI_PHYSICAL_ADDRESS Paddr; > > > > > + EFI_PHYSICAL_ADDRESS Start; > > > > > + EFI_PHYSICAL_ADDRESS End; > > > > > + EFI_STATUS Status; > > > > > + UINTN Size; > > > > > + > > > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > > > > + ArmSmcArgs.Arg0 = OPTEE_SMC_GET_SHM_CONFIG; > > > > > + > > > > > + ArmCallSmc (&ArmSmcArgs); > > > > > + if (ArmSmcArgs.Arg0 != OPTEE_SMC_RETURN_OK) { > > > > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory not supported\n")); > > > > > + return EFI_UNSUPPORTED; > > > > > + } > > > > > + > > > > > + if (ArmSmcArgs.Arg3 != OPTEE_SMC_SHM_CACHED) { > > > > > + DEBUG ((DEBUG_WARN, "OP-TEE: Only normal cached shared memory supported\n")); > > > > > + return EFI_UNSUPPORTED; > > > > > + } > > > > > + > > > > > + Start = (ArmSmcArgs.Arg1 + SIZE_4KB - 1) & ~(SIZE_4KB - 1); > > > > > + End = (ArmSmcArgs.Arg1 + ArmSmcArgs.Arg2) & ~(SIZE_4KB - 1); > > > > > + Paddr = Start; > > > > > + Size = End - Start; > > > > > + > > > > > + if (Size < SIZE_4KB) { > > > > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory too small\n")); > > > > > + return EFI_BUFFER_TOO_SMALL; > > > > > + } > > > > > + > > > > > + Status = ArmSetMemoryAttributes (Paddr, Size, EFI_MEMORY_WB); > > > > > + if (EFI_ERROR (Status)) { > > > > > + return Status; > > > > > + } > > > > > + > > > > > + OpteeShmInfo.Base = (UINTN)Paddr; > > > > > > Would PHYSICAL_ADDRESS (always 64-bit) be more correct here, or is the > > > OP-TEE end seeing this struct as always having fields of native width? > > > > Its a global struct not shared with OP-TEE. Used native width to have > > compatibility with 32-bit system. > > OK, that is not a problem then. > > > > (This is where the ordefile comes in handy, because it means I see the > > > struct definition first, not its use.) > > > > > > > > + OpteeShmInfo.Size = Size; > > > > > + > > > > > + return EFI_SUCCESS; > > > > > +} > > > > > + > > > > > +EFI_STATUS > > > > > +EFIAPI > > > > > +OpteeInit ( > > > > > + VOID > > > > > + ) > > > > > +{ > > > > > + EFI_STATUS Status; > > > > > + > > > > > + if (!IsOpteePresent ()) { > > > > > + DEBUG ((DEBUG_WARN, "OP-TEE not present\n")); > > > > > + return EFI_UNSUPPORTED; > > > > > + } > > > > > + > > > > > + Status = OpteeShmMemRemap (); > > > > > + if (EFI_ERROR (Status)) { > > > > > + DEBUG ((DEBUG_WARN, "OP-TEE shared memory remap failed\n")); > > > > > + return Status; > > > > > + } > > > > > + > > > > > + return EFI_SUCCESS; > > > > > +} > > > > > + > > > > > +/** > > > > > + Does Standard SMC to OP-TEE in secure world. > > > > > + > > > > > + @param[in] Parg Physical address of message to pass to secure world > > > > > + > > > > > + @return 0 on success, secure world return code otherwise > > > > > + > > > > > +**/ > > > > > +STATIC > > > > > +UINT32 > > > > > +OpteeCallWithArg ( > > > > > + IN EFI_PHYSICAL_ADDRESS Parg > > > > > + ) > > > > > +{ > > > > > + ARM_SMC_ARGS ArmSmcArgs; > > > > > + > > > > > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); > > > > > + ArmSmcArgs.Arg0 = OPTEE_SMC_CALL_WITH_ARG; > > > > > + ArmSmcArgs.Arg1 = (UINT32)(Parg >> 32); > > > > > + ArmSmcArgs.Arg2 = (UINT32)Parg; > > > > > + > > > > > + while (TRUE) { > > > > > + ArmCallSmc (&ArmSmcArgs); > > > > > + > > > > > + if (ArmSmcArgs.Arg0 == OPTEE_SMC_RETURN_RPC_FOREIGN_INTR) { > > > > > + // > > > > > + // A foreign interrupt was raised while secure world was > > > > > + // executing, since they are handled in UEFI a dummy RPC is > > > > > + // performed to let UEFI take the interrupt through the normal > > > > > + // vector. > > > > > + // > > > > > + ArmSmcArgs.Arg0 = OPTEE_SMC_RETURN_FROM_RPC; > > > > > + } else { > > > > > + break; > > > > > + } > > > > > + } > > > > > + > > > > > + return ArmSmcArgs.Arg0; > > > > > +} > > > > > + > > > > > +EFI_STATUS > > > > > +EFIAPI > > > > > +OpteeOpenSession ( > > > > > + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg > > > > > + ) > > > > > +{ > > > > > + OPTEE_MSG_ARG *MsgArg; > > > > > + > > > > > + MsgArg = NULL; > > > > > + > > > > > + if (OpteeShmInfo.Base == 0) { > > > > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > > > > + return EFI_NOT_STARTED; > > > > > + } > > > > > + > > > > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > > > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > > > > + > > > > > + MsgArg->Cmd = OPTEE_MSG_CMD_OPEN_SESSION; > > > > > + > > > > > + // > > > > > + // Initialize and add the meta parameters needed when opening a > > > > > + // session. > > > > > + // > > > > > + MsgArg->Params[0].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > > > > > + OPTEE_MSG_ATTR_META; > > > > > + MsgArg->Params[1].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | > > > > > + OPTEE_MSG_ATTR_META; > > > > > + CopyMem (&MsgArg->Params[0].U.Value, OpenSessionArg->Uuid, OPTEE_UUID_LEN); > > > > > + ZeroMem (&MsgArg->Params[1].U.Value, OPTEE_UUID_LEN); > > > > > + MsgArg->Params[1].U.Value.C = TEE_LOGIN_PUBLIC; > > > > > + > > > > > + MsgArg->NumParams = 2; > > > > > + > > > > > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > > > > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > > > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > > > > + } > > > > > + > > > > > + OpenSessionArg->Session = MsgArg->Session; > > > > > + OpenSessionArg->Ret = MsgArg->Ret; > > > > > + OpenSessionArg->RetOrigin = MsgArg->RetOrigin; > > > > > + > > > > > + return EFI_SUCCESS; > > > > > +} > > > > > + > > > > > +EFI_STATUS > > > > > +EFIAPI > > > > > +OpteeCloseSession ( > > > > > + IN UINT32 Session > > > > > + ) > > > > > +{ > > > > > + OPTEE_MSG_ARG *MsgArg; > > > > > + > > > > > + MsgArg = NULL; > > > > > + > > > > > + if (OpteeShmInfo.Base == 0) { > > > > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > > > > + return EFI_NOT_STARTED; > > > > > + } > > > > > + > > > > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > > > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > > > > + > > > > > + MsgArg->Cmd = OPTEE_MSG_CMD_CLOSE_SESSION; > > > > > + MsgArg->Session = Session; > > > > > + > > > > > + OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg); > > > > > + > > > > > + return EFI_SUCCESS; > > > > > +} > > > > > + > > > > > +STATIC > > > > > +EFI_STATUS > > > > > +OpteeToMsgParam ( > > > > > + OUT OPTEE_MSG_PARAM *MsgParams, > > > > > + IN UINT32 NumParams, > > > > > + IN OPTEE_MSG_PARAM *InParams > > > > > + ) > > > > > +{ > > > > > + UINT32 Idx; > > > > > + UINTN ParamShmAddr; > > > > > + UINTN ShmSize; > > > > > + UINTN Size; > > > > > + > > > > > + Size = (sizeof (OPTEE_MSG_ARG) + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > > > > > + ParamShmAddr = OpteeShmInfo.Base + Size; > > > > > + ShmSize = OpteeShmInfo.Size - Size; > > > > > + > > > > > + for (Idx = 0; Idx < NumParams; Idx++) { > > > > > + CONST OPTEE_MSG_PARAM *Ip; > > > > > + OPTEE_MSG_PARAM *Mp; > > > > > + UINT32 Attr; > > > > > + > > > > > + Ip = InParams + Idx; > > > > > + Mp = MsgParams + Idx; > > > > > + Attr = Ip->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > > > > > + > > > > > + switch (Attr) { > > > > > + case OPTEE_MSG_ATTR_TYPE_NONE: > > > > > + Mp->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > > > > > + ZeroMem (&Mp->U, sizeof (Mp->U)); > > > > > + break; > > > > > + > > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > > > > > + Mp->Attr = Attr; > > > > > + Mp->U.Value.A = Ip->U.Value.A; > > > > > + Mp->U.Value.B = Ip->U.Value.B; > > > > > + Mp->U.Value.C = Ip->U.Value.C; > > > > > + break; > > > > > + > > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > > > > > + Mp->Attr = Attr; > > > > > + > > > > > + if (Ip->U.Mem.Size > ShmSize) { > > > > > + return EFI_OUT_OF_RESOURCES; > > > > > + } > > > > > + > > > > > + CopyMem ((VOID *)ParamShmAddr, (VOID *)Ip->U.Mem.BufPtr, Ip->U.Mem.Size); > > > > > + Mp->U.Mem.BufPtr = (UINT64)ParamShmAddr; > > > > > > While I don't think it's possible for this to end up doing something > > > unexpected, casting pointers to a higher alignment (which this would > > > do on 32-bit) is a bad habit to get into. > > > > This is a shared message param structure with OP-TEE. So needed to cast it. > > Then you need to also ensure that you are not accidentally casting a > less-than-8-byte aligned pointer here. > I would suggest an explicit test with an error return and an > assert. Then if that thing ever does come back to bite you, you will > know why, rather than needing to spend a few days debugging. > > Alternatively, make ParamShmAddr an UINT64 * to begin with. > If you see above ParamShmAddr is computed taking care of 8-byte alignment as follows: + Size = (sizeof (OPTEE_MSG_ARG) + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); + ParamShmAddr = OpteeShmInfo.Base + Size; + ShmSize = OpteeShmInfo.Size - Size; Here OpteeShmInfo.Base is already 4K aligned address. -Sumit > > > > > + Mp->U.Mem.Size = Ip->U.Mem.Size; > > > > > + > > > > > + Size = (Ip->U.Mem.Size + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); > > > > > + ParamShmAddr += Size; > > > > > + ShmSize -= Size; > > > > > + break; > > > > > + > > > > > + default: > > > > > + return EFI_INVALID_PARAMETER; > > > > > + } > > > > > + } > > > > > + > > > > > + return EFI_SUCCESS; > > > > > +} > > > > > + > > > > > +STATIC > > > > > +EFI_STATUS > > > > > +OpteeFromMsgParam ( > > > > > + OUT OPTEE_MSG_PARAM *OutParams, > > > > > + IN UINT32 NumParams, > > > > > + IN OPTEE_MSG_PARAM *MsgParams > > > > > + ) > > > > > +{ > > > > > + UINT32 Idx; > > > > > + > > > > > + for (Idx = 0; Idx < NumParams; Idx++) { > > > > > + OPTEE_MSG_PARAM *Op; > > > > > + CONST OPTEE_MSG_PARAM *Mp; > > > > > + UINT32 Attr; > > > > > + > > > > > + Op = OutParams + Idx; > > > > > + Mp = MsgParams + Idx; > > > > > + Attr = Mp->Attr & OPTEE_MSG_ATTR_TYPE_MASK; > > > > > + > > > > > + switch (Attr) { > > > > > + case OPTEE_MSG_ATTR_TYPE_NONE: > > > > > + Op->Attr = OPTEE_MSG_ATTR_TYPE_NONE; > > > > > + ZeroMem (&Op->U, sizeof (Op->U)); > > > > > + break; > > > > > + > > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > > > > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > > > > > + Op->Attr = Attr; > > > > > + Op->U.Value.A = Mp->U.Value.A; > > > > > + Op->U.Value.B = Mp->U.Value.B; > > > > > + Op->U.Value.C = Mp->U.Value.C; > > > > > + break; > > > > > + > > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: > > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: > > > > > + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: > > > > > + Op->Attr = Attr; > > > > > + > > > > > + if (Mp->U.Mem.Size > Op->U.Mem.Size) { > > > > > + return EFI_BAD_BUFFER_SIZE; > > > > > + } > > > > > + > > > > > + CopyMem ((VOID *)Op->U.Mem.BufPtr, (VOID *)Mp->U.Mem.BufPtr, Mp->U.Mem.Size); > > > > > + Op->U.Mem.Size = Mp->U.Mem.Size; > > > > > + break; > > > > > + > > > > > + default: > > > > > + return EFI_INVALID_PARAMETER; > > > > > + } > > > > > + } > > > > > + > > > > > + return EFI_SUCCESS; > > > > > +} > > > > > + > > > > > +EFI_STATUS > > > > > +EFIAPI > > > > > +OpteeInvokeFunc ( > > > > > + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg > > > > > + ) > > > > > +{ > > > > > + EFI_STATUS Status; > > > > > + OPTEE_MSG_ARG *MsgArg; > > > > > + > > > > > + MsgArg = NULL; > > > > > + > > > > > + if (OpteeShmInfo.Base == 0) { > > > > > + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); > > > > > + return EFI_NOT_STARTED; > > > > > + } > > > > > + > > > > > + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; > > > > > + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); > > > > > + > > > > > + MsgArg->Cmd = OPTEE_MSG_CMD_INVOKE_COMMAND; > > > > > + MsgArg->Func = InvokeFuncArg->Func; > > > > > + MsgArg->Session = InvokeFuncArg->Session; > > > > > + > > > > > + Status = OpteeToMsgParam (MsgArg->Params, MAX_PARAMS, InvokeFuncArg->Params); > > > > > + if (Status) > > > > > + return Status; > > > > > > Always use {} (coding style requirement). > > > > > > > Ok. > > > > > > > + > > > > > + MsgArg->NumParams = MAX_PARAMS; > > > > > + > > > > > + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { > > > > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > > > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > > > > + } > > > > > + > > > > > + if (OpteeFromMsgParam (InvokeFuncArg->Params, MAX_PARAMS, MsgArg->Params)) { > > > > > + MsgArg->Ret = OPTEE_ERROR_COMMS; > > > > > + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; > > > > > + } > > > > > + > > > > > + InvokeFuncArg->Ret = MsgArg->Ret; > > > > > + InvokeFuncArg->RetOrigin = MsgArg->RetOrigin; > > > > > + > > > > > + return EFI_SUCCESS; > > > > > +} > > > > > diff --git a/ArmPkg/Library/OpteeLib/OpteeLib.inf b/ArmPkg/Library/OpteeLib/OpteeLib.inf > > > > > index 5abd427379cc..e03054a7167d 100644 > > > > > --- a/ArmPkg/Library/OpteeLib/OpteeLib.inf > > > > > +++ b/ArmPkg/Library/OpteeLib/OpteeLib.inf > > > > > @@ -23,11 +23,13 @@ [Defines] > > > > > > > > > > [Sources] > > > > > Optee.c > > > > > + OpteeSmc.h > > > > > > > > > > [Packages] > > > > > ArmPkg/ArmPkg.dec > > > > > MdePkg/MdePkg.dec > > > > > > > > > > [LibraryClasses] > > > > > + ArmMmuLib > > > > > ArmSmcLib > > > > > BaseLib > > > > > diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h b/ArmPkg/Library/OpteeLib/OpteeSmc.h > > > > > new file mode 100644 > > > > > index 000000000000..e2ea35784a0a > > > > > --- /dev/null > > > > > +++ b/ArmPkg/Library/OpteeLib/OpteeSmc.h > > > > > @@ -0,0 +1,43 @@ > > > > > +/** @file > > > > > + OP-TEE SMC header file. > > > > > + > > > > > + Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR> > > > > > + > > > > > + This program and the accompanying materials > > > > > + are licensed and made available under the terms and conditions of the BSD License > > > > > + which accompanies this distribution. The full text of the license may be found at > > > > > + http://opensource.org/licenses/bsd-license.php > > > > > + > > > > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > > > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > > > > + > > > > > +**/ > > > > > + > > > > > +#ifndef _OPTEE_SMC_H_ > > > > > +#define _OPTEE_SMC_H_ > > > > > + > > > > > +/* Returned in Arg0 only from Trusted OS functions */ > > > > > +#define OPTEE_SMC_RETURN_OK 0x0 > > > > > + > > > > > +#define OPTEE_SMC_RETURN_FROM_RPC 0x32000003 > > > > > +#define OPTEE_SMC_CALL_WITH_ARG 0x32000004 > > > > > +#define OPTEE_SMC_GET_SHM_CONFIG 0xb2000007 > > > > > + > > > > > +#define OPTEE_SMC_SHM_CACHED 1 > > > > > + > > > > > +#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTR 0xffff0004 > > > > > + > > > > > +#define OPTEE_MSG_CMD_OPEN_SESSION 0 > > > > > +#define OPTEE_MSG_CMD_INVOKE_COMMAND 1 > > > > > +#define OPTEE_MSG_CMD_CLOSE_SESSION 2 > > > > > + > > > > > +#define OPTEE_MSG_ATTR_META 0x100 > > > > > + > > > > > +#define TEE_LOGIN_PUBLIC 0x0 > > > > > > I'm a bit apprehensive about taking over the whole TEE namespace in an > > > implementation-specific header file. > > > Can/should this be OPTEE_? > > > > > > > Ok will change this to OPTEE_. > > Thanks! > > > / > Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPkg/Include/Library/OpteeLib.h b/ArmPkg/Include/Library/OpteeLib.h index f65d8674d9b8..2d1c60632dfe 100644 --- a/ArmPkg/Include/Library/OpteeLib.h +++ b/ArmPkg/Include/Library/OpteeLib.h @@ -25,10 +25,100 @@ #define OPTEE_OS_UID2 0xaf630002 #define OPTEE_OS_UID3 0xa5d5c51b +#define OPTEE_MSG_ATTR_TYPE_NONE 0x0 +#define OPTEE_MSG_ATTR_TYPE_VALUE_INPUT 0x1 +#define OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT 0x2 +#define OPTEE_MSG_ATTR_TYPE_VALUE_INOUT 0x3 +#define OPTEE_MSG_ATTR_TYPE_MEM_INPUT 0x9 +#define OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT 0xa +#define OPTEE_MSG_ATTR_TYPE_MEM_INOUT 0xb + +#define OPTEE_MSG_ATTR_TYPE_MASK 0xff + +#define OPTEE_ORIGIN_COMMS 0x00000002 +#define OPTEE_ERROR_COMMS 0xFFFF000E + +typedef struct { + UINT64 BufPtr; + UINT64 Size; + UINT64 ShmRef; +} OPTEE_MSG_PARAM_MEM; + +typedef struct { + UINT64 A; + UINT64 B; + UINT64 C; +} OPTEE_MSG_PARAM_VALUE; + +typedef struct { + UINT64 Attr; + union { + OPTEE_MSG_PARAM_MEM Mem; + OPTEE_MSG_PARAM_VALUE Value; + } U; +} OPTEE_MSG_PARAM; + +#define MAX_PARAMS 4 + +typedef struct { + UINT32 Cmd; + UINT32 Func; + UINT32 Session; + UINT32 CancelId; + UINT32 Pad; + UINT32 Ret; + UINT32 RetOrigin; + UINT32 NumParams; + + // NumParams tells the actual number of element in Params + OPTEE_MSG_PARAM Params[MAX_PARAMS]; +} OPTEE_MSG_ARG; + +#define OPTEE_UUID_LEN 16 + +typedef struct { + UINT8 Uuid[OPTEE_UUID_LEN]; // [in] UUID of the Trusted Application + UINT32 Session; // [out] Session id + UINT32 Ret; // [out] Return value + UINT32 RetOrigin; // [out] Origin of the return value +} OPTEE_OPEN_SESSION_ARG; + +typedef struct { + UINT32 Func; // [in] Trusted App func, specific to the TA + UINT32 Session; // [in] Session id + UINT32 Ret; // [out] Return value + UINT32 RetOrigin; // [out] Origin of the return value + OPTEE_MSG_PARAM Params[MAX_PARAMS]; // Params for func to be invoked +} OPTEE_INVOKE_FUNC_ARG; + BOOLEAN EFIAPI IsOpteePresent ( VOID ); +EFI_STATUS +EFIAPI +OpteeInit ( + VOID + ); + +EFI_STATUS +EFIAPI +OpteeOpenSession ( + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg + ); + +EFI_STATUS +EFIAPI +OpteeCloseSession ( + IN UINT32 Session + ); + +EFI_STATUS +EFIAPI +OpteeInvokeFunc ( + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg + ); + #endif diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c index 574527f8b5ea..bf7872cbbce0 100644 --- a/ArmPkg/Library/OpteeLib/Optee.c +++ b/ArmPkg/Library/OpteeLib/Optee.c @@ -14,11 +14,18 @@ **/ +#include <Library/ArmMmuLib.h> #include <Library/ArmSmcLib.h> +#include <Library/BaseMemoryLib.h> #include <Library/BaseLib.h> +#include <Library/DebugLib.h> #include <Library/OpteeLib.h> #include <IndustryStandard/ArmStdSmc.h> +#include <OpteeSmc.h> +#include <Uefi.h> + +STATIC OPTEE_SHARED_MEMORY_INFO OpteeShmInfo = { 0 }; /** Check for OP-TEE presence. @@ -31,6 +38,7 @@ IsOpteePresent ( { ARM_SMC_ARGS ArmSmcArgs; + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); // Send a Trusted OS Calls UID command ArmSmcArgs.Arg0 = ARM_SMC_ID_TOS_UID; ArmCallSmc (&ArmSmcArgs); @@ -44,3 +52,352 @@ IsOpteePresent ( return FALSE; } } + +STATIC +EFI_STATUS +OpteeShmMemRemap ( + VOID + ) +{ + ARM_SMC_ARGS ArmSmcArgs; + EFI_PHYSICAL_ADDRESS Paddr; + EFI_PHYSICAL_ADDRESS Start; + EFI_PHYSICAL_ADDRESS End; + EFI_STATUS Status; + UINTN Size; + + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); + ArmSmcArgs.Arg0 = OPTEE_SMC_GET_SHM_CONFIG; + + ArmCallSmc (&ArmSmcArgs); + if (ArmSmcArgs.Arg0 != OPTEE_SMC_RETURN_OK) { + DEBUG ((DEBUG_WARN, "OP-TEE shared memory not supported\n")); + return EFI_UNSUPPORTED; + } + + if (ArmSmcArgs.Arg3 != OPTEE_SMC_SHM_CACHED) { + DEBUG ((DEBUG_WARN, "OP-TEE: Only normal cached shared memory supported\n")); + return EFI_UNSUPPORTED; + } + + Start = (ArmSmcArgs.Arg1 + SIZE_4KB - 1) & ~(SIZE_4KB - 1); + End = (ArmSmcArgs.Arg1 + ArmSmcArgs.Arg2) & ~(SIZE_4KB - 1); + Paddr = Start; + Size = End - Start; + + if (Size < SIZE_4KB) { + DEBUG ((DEBUG_WARN, "OP-TEE shared memory too small\n")); + return EFI_BUFFER_TOO_SMALL; + } + + Status = ArmSetMemoryAttributes (Paddr, Size, EFI_MEMORY_WB); + if (EFI_ERROR (Status)) { + return Status; + } + + OpteeShmInfo.Base = (UINTN)Paddr; + OpteeShmInfo.Size = Size; + + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI +OpteeInit ( + VOID + ) +{ + EFI_STATUS Status; + + if (!IsOpteePresent ()) { + DEBUG ((DEBUG_WARN, "OP-TEE not present\n")); + return EFI_UNSUPPORTED; + } + + Status = OpteeShmMemRemap (); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, "OP-TEE shared memory remap failed\n")); + return Status; + } + + return EFI_SUCCESS; +} + +/** + Does Standard SMC to OP-TEE in secure world. + + @param[in] Parg Physical address of message to pass to secure world + + @return 0 on success, secure world return code otherwise + +**/ +STATIC +UINT32 +OpteeCallWithArg ( + IN EFI_PHYSICAL_ADDRESS Parg + ) +{ + ARM_SMC_ARGS ArmSmcArgs; + + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS)); + ArmSmcArgs.Arg0 = OPTEE_SMC_CALL_WITH_ARG; + ArmSmcArgs.Arg1 = (UINT32)(Parg >> 32); + ArmSmcArgs.Arg2 = (UINT32)Parg; + + while (TRUE) { + ArmCallSmc (&ArmSmcArgs); + + if (ArmSmcArgs.Arg0 == OPTEE_SMC_RETURN_RPC_FOREIGN_INTR) { + // + // A foreign interrupt was raised while secure world was + // executing, since they are handled in UEFI a dummy RPC is + // performed to let UEFI take the interrupt through the normal + // vector. + // + ArmSmcArgs.Arg0 = OPTEE_SMC_RETURN_FROM_RPC; + } else { + break; + } + } + + return ArmSmcArgs.Arg0; +} + +EFI_STATUS +EFIAPI +OpteeOpenSession ( + IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg + ) +{ + OPTEE_MSG_ARG *MsgArg; + + MsgArg = NULL; + + if (OpteeShmInfo.Base == 0) { + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); + return EFI_NOT_STARTED; + } + + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); + + MsgArg->Cmd = OPTEE_MSG_CMD_OPEN_SESSION; + + // + // Initialize and add the meta parameters needed when opening a + // session. + // + MsgArg->Params[0].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | + OPTEE_MSG_ATTR_META; + MsgArg->Params[1].Attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT | + OPTEE_MSG_ATTR_META; + CopyMem (&MsgArg->Params[0].U.Value, OpenSessionArg->Uuid, OPTEE_UUID_LEN); + ZeroMem (&MsgArg->Params[1].U.Value, OPTEE_UUID_LEN); + MsgArg->Params[1].U.Value.C = TEE_LOGIN_PUBLIC; + + MsgArg->NumParams = 2; + + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { + MsgArg->Ret = OPTEE_ERROR_COMMS; + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; + } + + OpenSessionArg->Session = MsgArg->Session; + OpenSessionArg->Ret = MsgArg->Ret; + OpenSessionArg->RetOrigin = MsgArg->RetOrigin; + + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI +OpteeCloseSession ( + IN UINT32 Session + ) +{ + OPTEE_MSG_ARG *MsgArg; + + MsgArg = NULL; + + if (OpteeShmInfo.Base == 0) { + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); + return EFI_NOT_STARTED; + } + + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); + + MsgArg->Cmd = OPTEE_MSG_CMD_CLOSE_SESSION; + MsgArg->Session = Session; + + OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg); + + return EFI_SUCCESS; +} + +STATIC +EFI_STATUS +OpteeToMsgParam ( + OUT OPTEE_MSG_PARAM *MsgParams, + IN UINT32 NumParams, + IN OPTEE_MSG_PARAM *InParams + ) +{ + UINT32 Idx; + UINTN ParamShmAddr; + UINTN ShmSize; + UINTN Size; + + Size = (sizeof (OPTEE_MSG_ARG) + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); + ParamShmAddr = OpteeShmInfo.Base + Size; + ShmSize = OpteeShmInfo.Size - Size; + + for (Idx = 0; Idx < NumParams; Idx++) { + CONST OPTEE_MSG_PARAM *Ip; + OPTEE_MSG_PARAM *Mp; + UINT32 Attr; + + Ip = InParams + Idx; + Mp = MsgParams + Idx; + Attr = Ip->Attr & OPTEE_MSG_ATTR_TYPE_MASK; + + switch (Attr) { + case OPTEE_MSG_ATTR_TYPE_NONE: + Mp->Attr = OPTEE_MSG_ATTR_TYPE_NONE; + ZeroMem (&Mp->U, sizeof (Mp->U)); + break; + + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: + Mp->Attr = Attr; + Mp->U.Value.A = Ip->U.Value.A; + Mp->U.Value.B = Ip->U.Value.B; + Mp->U.Value.C = Ip->U.Value.C; + break; + + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: + Mp->Attr = Attr; + + if (Ip->U.Mem.Size > ShmSize) { + return EFI_OUT_OF_RESOURCES; + } + + CopyMem ((VOID *)ParamShmAddr, (VOID *)Ip->U.Mem.BufPtr, Ip->U.Mem.Size); + Mp->U.Mem.BufPtr = (UINT64)ParamShmAddr; + Mp->U.Mem.Size = Ip->U.Mem.Size; + + Size = (Ip->U.Mem.Size + sizeof (UINT64) - 1) & ~(sizeof (UINT64) - 1); + ParamShmAddr += Size; + ShmSize -= Size; + break; + + default: + return EFI_INVALID_PARAMETER; + } + } + + return EFI_SUCCESS; +} + +STATIC +EFI_STATUS +OpteeFromMsgParam ( + OUT OPTEE_MSG_PARAM *OutParams, + IN UINT32 NumParams, + IN OPTEE_MSG_PARAM *MsgParams + ) +{ + UINT32 Idx; + + for (Idx = 0; Idx < NumParams; Idx++) { + OPTEE_MSG_PARAM *Op; + CONST OPTEE_MSG_PARAM *Mp; + UINT32 Attr; + + Op = OutParams + Idx; + Mp = MsgParams + Idx; + Attr = Mp->Attr & OPTEE_MSG_ATTR_TYPE_MASK; + + switch (Attr) { + case OPTEE_MSG_ATTR_TYPE_NONE: + Op->Attr = OPTEE_MSG_ATTR_TYPE_NONE; + ZeroMem (&Op->U, sizeof (Op->U)); + break; + + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: + Op->Attr = Attr; + Op->U.Value.A = Mp->U.Value.A; + Op->U.Value.B = Mp->U.Value.B; + Op->U.Value.C = Mp->U.Value.C; + break; + + case OPTEE_MSG_ATTR_TYPE_MEM_INPUT: + case OPTEE_MSG_ATTR_TYPE_MEM_OUTPUT: + case OPTEE_MSG_ATTR_TYPE_MEM_INOUT: + Op->Attr = Attr; + + if (Mp->U.Mem.Size > Op->U.Mem.Size) { + return EFI_BAD_BUFFER_SIZE; + } + + CopyMem ((VOID *)Op->U.Mem.BufPtr, (VOID *)Mp->U.Mem.BufPtr, Mp->U.Mem.Size); + Op->U.Mem.Size = Mp->U.Mem.Size; + break; + + default: + return EFI_INVALID_PARAMETER; + } + } + + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI +OpteeInvokeFunc ( + IN OUT OPTEE_INVOKE_FUNC_ARG *InvokeFuncArg + ) +{ + EFI_STATUS Status; + OPTEE_MSG_ARG *MsgArg; + + MsgArg = NULL; + + if (OpteeShmInfo.Base == 0) { + DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n")); + return EFI_NOT_STARTED; + } + + MsgArg = (OPTEE_MSG_ARG *)OpteeShmInfo.Base; + ZeroMem (MsgArg, sizeof (OPTEE_MSG_ARG)); + + MsgArg->Cmd = OPTEE_MSG_CMD_INVOKE_COMMAND; + MsgArg->Func = InvokeFuncArg->Func; + MsgArg->Session = InvokeFuncArg->Session; + + Status = OpteeToMsgParam (MsgArg->Params, MAX_PARAMS, InvokeFuncArg->Params); + if (Status) + return Status; + + MsgArg->NumParams = MAX_PARAMS; + + if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MsgArg)) { + MsgArg->Ret = OPTEE_ERROR_COMMS; + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; + } + + if (OpteeFromMsgParam (InvokeFuncArg->Params, MAX_PARAMS, MsgArg->Params)) { + MsgArg->Ret = OPTEE_ERROR_COMMS; + MsgArg->RetOrigin = OPTEE_ORIGIN_COMMS; + } + + InvokeFuncArg->Ret = MsgArg->Ret; + InvokeFuncArg->RetOrigin = MsgArg->RetOrigin; + + return EFI_SUCCESS; +} diff --git a/ArmPkg/Library/OpteeLib/OpteeLib.inf b/ArmPkg/Library/OpteeLib/OpteeLib.inf index 5abd427379cc..e03054a7167d 100644 --- a/ArmPkg/Library/OpteeLib/OpteeLib.inf +++ b/ArmPkg/Library/OpteeLib/OpteeLib.inf @@ -23,11 +23,13 @@ [Defines] [Sources] Optee.c + OpteeSmc.h [Packages] ArmPkg/ArmPkg.dec MdePkg/MdePkg.dec [LibraryClasses] + ArmMmuLib ArmSmcLib BaseLib diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h b/ArmPkg/Library/OpteeLib/OpteeSmc.h new file mode 100644 index 000000000000..e2ea35784a0a --- /dev/null +++ b/ArmPkg/Library/OpteeLib/OpteeSmc.h @@ -0,0 +1,43 @@ +/** @file + OP-TEE SMC header file. + + Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR> + + This program and the accompanying materials + are licensed and made available under the terms and conditions of the BSD License + which accompanies this distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#ifndef _OPTEE_SMC_H_ +#define _OPTEE_SMC_H_ + +/* Returned in Arg0 only from Trusted OS functions */ +#define OPTEE_SMC_RETURN_OK 0x0 + +#define OPTEE_SMC_RETURN_FROM_RPC 0x32000003 +#define OPTEE_SMC_CALL_WITH_ARG 0x32000004 +#define OPTEE_SMC_GET_SHM_CONFIG 0xb2000007 + +#define OPTEE_SMC_SHM_CACHED 1 + +#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTR 0xffff0004 + +#define OPTEE_MSG_CMD_OPEN_SESSION 0 +#define OPTEE_MSG_CMD_INVOKE_COMMAND 1 +#define OPTEE_MSG_CMD_CLOSE_SESSION 2 + +#define OPTEE_MSG_ATTR_META 0x100 + +#define TEE_LOGIN_PUBLIC 0x0 + +typedef struct { + UINTN Base; + UINTN Size; +} OPTEE_SHARED_MEMORY_INFO; + +#endif
Add following APIs to communicate with OP-TEE pseudo/early TAs: 1. OpteeInit 2. OpteeOpenSession 3. OpteeCloseSession 4. OpteeInvokeFunc Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Leif Lindholm <leif.lindholm@linaro.org> Cc: Michael D Kinney <michael.d.kinney@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Sumit Garg <sumit.garg@linaro.org> --- ArmPkg/Include/Library/OpteeLib.h | 90 +++++++++ ArmPkg/Library/OpteeLib/Optee.c | 357 +++++++++++++++++++++++++++++++++++ ArmPkg/Library/OpteeLib/OpteeLib.inf | 2 + ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++++ 4 files changed, 492 insertions(+) create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel