Changes in v3: Removed GlobalPlatform TEE return codes (IndustryStandard/GlobalPlatform.h) that were rejected by EDK2 maintainers. Rather used custom ones for this OP-TEE driver.
Changes in v2: 1. Separate patch for MdePkg/Include/IndustryStandard/GlobalPlatform.h. 2. Correct comments style for struct members. 3. Update commit message.
Sumit Garg (1): ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
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
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
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
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:
- OpteeInit
- OpteeOpenSession
- OpteeCloseSession
- 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 ParamsOPTEE_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 ApplicationUINT32 Session; // [out] Session idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return value+} OPTEE_OPEN_SESSION_ARG;
+typedef struct {
UINT32 Func; // [in] Trusted App func, specific to the TAUINT32 Session; // [in] Session idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return valueOPTEE_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
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:
- OpteeInit
- OpteeOpenSession
- OpteeCloseSession
- 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%27s-unkempt-git... 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/conten...
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 ParamsOPTEE_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 idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return value+} OPTEE_OPEN_SESSION_ARG;
+typedef struct {
UINT32 Func; // [in] Trusted App func, specific to the TAUINT32 Session; // [in] Session idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return valueOPTEE_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
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:
- OpteeInit
- OpteeOpenSession
- OpteeCloseSession
- 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%27s-unkempt-git... 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/conten...
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 ParamsOPTEE_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 ApplicationIs 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 idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return value+} OPTEE_OPEN_SESSION_ARG;
+typedef struct {
UINT32 Func; // [in] Trusted App func, specific to the TAUINT32 Session; // [in] Session idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return valueOPTEE_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
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:
- OpteeInit
- OpteeOpenSession
- OpteeCloseSession
- 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%27s-unkempt-git... 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/conten...
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]:
- MSG/Msg
- MEM/Mem
- INFO/Info
- FUNC/Func
- RET/Ret
- ATTR/Attr
- 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 ParamsOPTEE_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 ApplicationIs 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 idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return value+} OPTEE_OPEN_SESSION_ARG;
+typedef struct {
UINT32 Func; // [in] Trusted App func, specific to the TAUINT32 Session; // [in] Session idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return valueOPTEE_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
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:
- OpteeInit
- OpteeOpenSession
- OpteeCloseSession
- 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%27s-unkempt-git... 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/conten...
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]:
- MSG/Msg
- MEM/Mem
- INFO/Info
- FUNC/Func
- RET/Ret
- ATTR/Attr
- 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 ParamsOPTEE_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 ApplicationIs 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 idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return value+} OPTEE_OPEN_SESSION_ARG;
+typedef struct {
UINT32 Func; // [in] Trusted App func, specific to the TAUINT32 Session; // [in] Session idUINT32 Ret; // [out] Return valueUINT32 RetOrigin; // [out] Origin of the return valueOPTEE_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
-
Ard Biesheuvel -
Leif Lindholm -
Sumit Garg