* [PATCH v4 0/1] Add ArmPkg/Optee library APIs
@ 2018-10-10 5:18 Sumit Garg
2018-10-10 5:18 ` [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE Sumit Garg
2018-10-17 12:00 ` [PATCH v4 0/1] Add ArmPkg/Optee library APIs Sumit Garg
0 siblings, 2 replies; 9+ messages in thread
From: Sumit Garg @ 2018-10-10 5:18 UTC (permalink / raw)
To: edk2-devel
Cc: ard.biesheuvel, leif.lindholm, michael.d.kinney, tee-dev,
daniel.thompson, joakim.bech, Matteo.Carlini, Achin.Gupta,
udit.kumar, Sumit Garg
Changes in v4:
Replaced abbreviations with full name which are not defined in [1]. Also
used EFI_GUID for Trusted Application UUIDs.
[1] https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/4_naming_conventions/#table-2-efi-supported-abbreviations
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/Library/OpteeLib/OpteeLib.inf | 2 +
ArmPkg/Include/Library/OpteeLib.h | 88 +++++
ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++
ArmPkg/Library/OpteeLib/Optee.c | 397 ++++++++++++++++++++
4 files changed, 530 insertions(+)
create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
2018-10-10 5:18 [PATCH v4 0/1] Add ArmPkg/Optee library APIs Sumit Garg
@ 2018-10-10 5:18 ` Sumit Garg
2018-10-18 6:23 ` Leif Lindholm
2018-10-17 12:00 ` [PATCH v4 0/1] Add ArmPkg/Optee library APIs Sumit Garg
1 sibling, 1 reply; 9+ messages in thread
From: Sumit Garg @ 2018-10-10 5:18 UTC (permalink / raw)
To: edk2-devel
Cc: ard.biesheuvel, leif.lindholm, michael.d.kinney, tee-dev,
daniel.thompson, joakim.bech, Matteo.Carlini, Achin.Gupta,
udit.kumar, Sumit Garg
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/Library/OpteeLib/OpteeLib.inf | 2 +
ArmPkg/Include/Library/OpteeLib.h | 88 +++++
ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++
ArmPkg/Library/OpteeLib/Optee.c | 397 ++++++++++++++++++++
4 files changed, 530 insertions(+)
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/Include/Library/OpteeLib.h b/ArmPkg/Include/Library/OpteeLib.h
index f65d8674d9b8..6884d5681831 100644
--- a/ArmPkg/Include/Library/OpteeLib.h
+++ b/ArmPkg/Include/Library/OpteeLib.h
@@ -25,10 +25,98 @@
#define OPTEE_OS_UID2 0xaf630002
#define OPTEE_OS_UID3 0xa5d5c51b
+#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_NONE 0x0
+#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INPUT 0x1
+#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_OUTPUT 0x2
+#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INOUT 0x3
+#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INPUT 0x9
+#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_OUTPUT 0xa
+#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INOUT 0xb
+
+#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MASK 0xff
+
+#define OPTEE_ORIGIN_COMMUNICATION 0x00000002
+#define OPTEE_ERROR_COMMUNICATION 0xFFFF000E
+
+typedef struct {
+ UINT64 BufferAddress;
+ UINT64 Size;
+ UINT64 SharedMemoryReference;
+} OPTEE_MESSAGE_PARAM_MEMORY;
+
+typedef struct {
+ UINT64 A;
+ UINT64 B;
+ UINT64 C;
+} OPTEE_MESSAGE_PARAM_VALUE;
+
+typedef struct {
+ UINT64 Attribute;
+ union {
+ OPTEE_MESSAGE_PARAM_MEMORY Memory;
+ OPTEE_MESSAGE_PARAM_VALUE Value;
+ } Union;
+} OPTEE_MESSAGE_PARAM;
+
+#define OPTEE_MAX_CALL_PARAMS 4
+
+typedef struct {
+ UINT32 Command;
+ UINT32 Function;
+ UINT32 Session;
+ UINT32 CancelId;
+ UINT32 Pad;
+ UINT32 Return;
+ UINT32 ReturnOrigin;
+ UINT32 NumParams;
+
+ // NumParams tells the actual number of element in Params
+ OPTEE_MESSAGE_PARAM Params[OPTEE_MAX_CALL_PARAMS];
+} OPTEE_MESSAGE_ARG;
+
+typedef struct {
+ EFI_GUID Uuid; // [in] GUID/UUID of the Trusted Application
+ UINT32 Session; // [out] Session id
+ UINT32 Return; // [out] Return value
+ UINT32 ReturnOrigin; // [out] Origin of the return value
+} OPTEE_OPEN_SESSION_ARG;
+
+typedef struct {
+ UINT32 Function; // [in] Trusted Application function, specific to the TA
+ UINT32 Session; // [in] Session id
+ UINT32 Return; // [out] Return value
+ UINT32 ReturnOrigin; // [out] Origin of the return value
+ OPTEE_MESSAGE_PARAM Params[OPTEE_MAX_CALL_PARAMS]; // Params for function to be invoked
+} OPTEE_INVOKE_FUNCTION_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
+OpteeInvokeFunction (
+ IN OUT OPTEE_INVOKE_FUNCTION_ARG *InvokeFunctionArg
+ );
+
#endif
diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h b/ArmPkg/Library/OpteeLib/OpteeSmc.h
new file mode 100644
index 000000000000..21ff4b22ab92
--- /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_SHARED_MEMORY_CONFIG 0xb2000007
+
+#define OPTEE_SMC_SHARED_MEMORY_CACHED 1
+
+#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTERRUPT 0xffff0004
+
+#define OPTEE_MESSAGE_COMMAND_OPEN_SESSION 0
+#define OPTEE_MESSAGE_COMMAND_INVOKE_FUNCTION 1
+#define OPTEE_MESSAGE_COMMAND_CLOSE_SESSION 2
+
+#define OPTEE_MESSAGE_ATTRIBUTE_META 0x100
+
+#define OPTEE_LOGIN_PUBLIC 0x0
+
+typedef struct {
+ UINTN Base;
+ UINTN Size;
+} OPTEE_SHARED_MEMORY_INFORMATION;
+
+#endif
diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c
index 574527f8b5ea..6617126e8bdb 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_INFORMATION OpteeSharedMemoryInformation = { 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,392 @@ IsOpteePresent (
return FALSE;
}
}
+
+STATIC
+EFI_STATUS
+OpteeSharedMemoryRemap (
+ VOID
+ )
+{
+ ARM_SMC_ARGS ArmSmcArgs;
+ EFI_PHYSICAL_ADDRESS PhysicalAddress;
+ EFI_PHYSICAL_ADDRESS Start;
+ EFI_PHYSICAL_ADDRESS End;
+ EFI_STATUS Status;
+ UINTN Size;
+
+ ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS));
+ ArmSmcArgs.Arg0 = OPTEE_SMC_GET_SHARED_MEMORY_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_SHARED_MEMORY_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);
+ PhysicalAddress = 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 (PhysicalAddress, Size, EFI_MEMORY_WB);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ OpteeSharedMemoryInformation.Base = (UINTN)PhysicalAddress;
+ OpteeSharedMemoryInformation.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 = OpteeSharedMemoryRemap ();
+ 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] PhysicalArg 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 PhysicalArg
+ )
+{
+ ARM_SMC_ARGS ArmSmcArgs;
+
+ ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS));
+ ArmSmcArgs.Arg0 = OPTEE_SMC_CALL_WITH_ARG;
+ ArmSmcArgs.Arg1 = (UINT32)(PhysicalArg >> 32);
+ ArmSmcArgs.Arg2 = (UINT32)PhysicalArg;
+
+ while (TRUE) {
+ ArmCallSmc (&ArmSmcArgs);
+
+ if (ArmSmcArgs.Arg0 == OPTEE_SMC_RETURN_RPC_FOREIGN_INTERRUPT) {
+ //
+ // 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;
+}
+
+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));
+}
+
+EFI_STATUS
+EFIAPI
+OpteeOpenSession (
+ IN OUT OPTEE_OPEN_SESSION_ARG *OpenSessionArg
+ )
+{
+ OPTEE_MESSAGE_ARG *MessageArg;
+
+ MessageArg = NULL;
+
+ if (OpteeSharedMemoryInformation.Base == 0) {
+ DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n"));
+ return EFI_NOT_STARTED;
+ }
+
+ MessageArg = (OPTEE_MESSAGE_ARG *)OpteeSharedMemoryInformation.Base;
+ ZeroMem (MessageArg, sizeof (OPTEE_MESSAGE_ARG));
+
+ MessageArg->Command = OPTEE_MESSAGE_COMMAND_OPEN_SESSION;
+
+ //
+ // Initialize and add the meta parameters needed when opening a
+ // session.
+ //
+ MessageArg->Params[0].Attribute = OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INPUT |
+ OPTEE_MESSAGE_ATTRIBUTE_META;
+ MessageArg->Params[1].Attribute = OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INPUT |
+ OPTEE_MESSAGE_ATTRIBUTE_META;
+ UuidToOctets (
+ (UINT8 *)&MessageArg->Params[0].Union.Value,
+ &OpenSessionArg->Uuid
+ );
+ ZeroMem (&MessageArg->Params[1].Union.Value, sizeof (EFI_GUID));
+ MessageArg->Params[1].Union.Value.C = OPTEE_LOGIN_PUBLIC;
+
+ MessageArg->NumParams = 2;
+
+ if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MessageArg)) {
+ MessageArg->Return = OPTEE_ERROR_COMMUNICATION;
+ MessageArg->ReturnOrigin = OPTEE_ORIGIN_COMMUNICATION;
+ }
+
+ OpenSessionArg->Session = MessageArg->Session;
+ OpenSessionArg->Return = MessageArg->Return;
+ OpenSessionArg->ReturnOrigin = MessageArg->ReturnOrigin;
+
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+OpteeCloseSession (
+ IN UINT32 Session
+ )
+{
+ OPTEE_MESSAGE_ARG *MessageArg;
+
+ MessageArg = NULL;
+
+ if (OpteeSharedMemoryInformation.Base == 0) {
+ DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n"));
+ return EFI_NOT_STARTED;
+ }
+
+ MessageArg = (OPTEE_MESSAGE_ARG *)OpteeSharedMemoryInformation.Base;
+ ZeroMem (MessageArg, sizeof (OPTEE_MESSAGE_ARG));
+
+ MessageArg->Command = OPTEE_MESSAGE_COMMAND_CLOSE_SESSION;
+ MessageArg->Session = Session;
+
+ OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MessageArg);
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+OpteeToMessageParam (
+ OUT OPTEE_MESSAGE_PARAM *MessageParams,
+ IN UINT32 NumParams,
+ IN OPTEE_MESSAGE_PARAM *InParams
+ )
+{
+ UINT32 Idx;
+ UINTN ParamSharedMemoryAddress;
+ UINTN SharedMemorySize;
+ UINTN Size;
+
+ Size = (sizeof (OPTEE_MESSAGE_ARG) + sizeof (UINT64) - 1) &
+ ~(sizeof (UINT64) - 1);
+ ParamSharedMemoryAddress = OpteeSharedMemoryInformation.Base + Size;
+ SharedMemorySize = OpteeSharedMemoryInformation.Size - Size;
+
+ for (Idx = 0; Idx < NumParams; Idx++) {
+ CONST OPTEE_MESSAGE_PARAM *InParam;
+ OPTEE_MESSAGE_PARAM *MessageParam;
+ UINT32 Attribute;
+
+ InParam = InParams + Idx;
+ MessageParam = MessageParams + Idx;
+ Attribute = InParam->Attribute & OPTEE_MESSAGE_ATTRIBUTE_TYPE_MASK;
+
+ switch (Attribute) {
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_NONE:
+ MessageParam->Attribute = OPTEE_MESSAGE_ATTRIBUTE_TYPE_NONE;
+ ZeroMem (&MessageParam->Union, sizeof (MessageParam->Union));
+ break;
+
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INPUT:
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_OUTPUT:
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INOUT:
+ MessageParam->Attribute = Attribute;
+ MessageParam->Union.Value.A = InParam->Union.Value.A;
+ MessageParam->Union.Value.B = InParam->Union.Value.B;
+ MessageParam->Union.Value.C = InParam->Union.Value.C;
+ break;
+
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INPUT:
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_OUTPUT:
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INOUT:
+ MessageParam->Attribute = Attribute;
+
+ if (InParam->Union.Memory.Size > SharedMemorySize) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ CopyMem (
+ (VOID *)ParamSharedMemoryAddress,
+ (VOID *)InParam->Union.Memory.BufferAddress,
+ InParam->Union.Memory.Size
+ );
+ MessageParam->Union.Memory.BufferAddress = (UINT64)ParamSharedMemoryAddress;
+ MessageParam->Union.Memory.Size = InParam->Union.Memory.Size;
+
+ Size = (InParam->Union.Memory.Size + sizeof (UINT64) - 1) &
+ ~(sizeof (UINT64) - 1);
+ ParamSharedMemoryAddress += Size;
+ SharedMemorySize -= Size;
+ break;
+
+ default:
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+OpteeFromMessageParam (
+ OUT OPTEE_MESSAGE_PARAM *OutParams,
+ IN UINT32 NumParams,
+ IN OPTEE_MESSAGE_PARAM *MessageParams
+ )
+{
+ UINT32 Idx;
+
+ for (Idx = 0; Idx < NumParams; Idx++) {
+ OPTEE_MESSAGE_PARAM *OutParam;
+ CONST OPTEE_MESSAGE_PARAM *MessageParam;
+ UINT32 Attribute;
+
+ OutParam = OutParams + Idx;
+ MessageParam = MessageParams + Idx;
+ Attribute = MessageParam->Attribute & OPTEE_MESSAGE_ATTRIBUTE_TYPE_MASK;
+
+ switch (Attribute) {
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_NONE:
+ OutParam->Attribute = OPTEE_MESSAGE_ATTRIBUTE_TYPE_NONE;
+ ZeroMem (&OutParam->Union, sizeof (OutParam->Union));
+ break;
+
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INPUT:
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_OUTPUT:
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INOUT:
+ OutParam->Attribute = Attribute;
+ OutParam->Union.Value.A = MessageParam->Union.Value.A;
+ OutParam->Union.Value.B = MessageParam->Union.Value.B;
+ OutParam->Union.Value.C = MessageParam->Union.Value.C;
+ break;
+
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INPUT:
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_OUTPUT:
+ case OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INOUT:
+ OutParam->Attribute = Attribute;
+
+ if (MessageParam->Union.Memory.Size > OutParam->Union.Memory.Size) {
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ CopyMem (
+ (VOID *)OutParam->Union.Memory.BufferAddress,
+ (VOID *)MessageParam->Union.Memory.BufferAddress,
+ MessageParam->Union.Memory.Size
+ );
+ OutParam->Union.Memory.Size = MessageParam->Union.Memory.Size;
+ break;
+
+ default:
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+OpteeInvokeFunction (
+ IN OUT OPTEE_INVOKE_FUNCTION_ARG *InvokeFunctionArg
+ )
+{
+ EFI_STATUS Status;
+ OPTEE_MESSAGE_ARG *MessageArg;
+
+ MessageArg = NULL;
+
+ if (OpteeSharedMemoryInformation.Base == 0) {
+ DEBUG ((DEBUG_WARN, "OP-TEE not initialized\n"));
+ return EFI_NOT_STARTED;
+ }
+
+ MessageArg = (OPTEE_MESSAGE_ARG *)OpteeSharedMemoryInformation.Base;
+ ZeroMem (MessageArg, sizeof (OPTEE_MESSAGE_ARG));
+
+ MessageArg->Command = OPTEE_MESSAGE_COMMAND_INVOKE_FUNCTION;
+ MessageArg->Function = InvokeFunctionArg->Function;
+ MessageArg->Session = InvokeFunctionArg->Session;
+
+ Status = OpteeToMessageParam (
+ MessageArg->Params,
+ OPTEE_MAX_CALL_PARAMS,
+ InvokeFunctionArg->Params
+ );
+ if (Status) {
+ return Status;
+ }
+
+ MessageArg->NumParams = OPTEE_MAX_CALL_PARAMS;
+
+ if (OpteeCallWithArg ((EFI_PHYSICAL_ADDRESS)MessageArg)) {
+ MessageArg->Return = OPTEE_ERROR_COMMUNICATION;
+ MessageArg->ReturnOrigin = OPTEE_ORIGIN_COMMUNICATION;
+ }
+
+ if (OpteeFromMessageParam (
+ InvokeFunctionArg->Params,
+ OPTEE_MAX_CALL_PARAMS,
+ MessageArg->Params
+ )) {
+ MessageArg->Return = OPTEE_ERROR_COMMUNICATION;
+ MessageArg->ReturnOrigin = OPTEE_ORIGIN_COMMUNICATION;
+ }
+
+ InvokeFunctionArg->Return = MessageArg->Return;
+ InvokeFunctionArg->ReturnOrigin = MessageArg->ReturnOrigin;
+
+ return EFI_SUCCESS;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 0/1] Add ArmPkg/Optee library APIs
2018-10-10 5:18 [PATCH v4 0/1] Add ArmPkg/Optee library APIs Sumit Garg
2018-10-10 5:18 ` [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE Sumit Garg
@ 2018-10-17 12:00 ` Sumit Garg
1 sibling, 0 replies; 9+ messages in thread
From: Sumit Garg @ 2018-10-17 12:00 UTC (permalink / raw)
To: Ard Biesheuvel, Leif Lindholm
Cc: edk2-devel, Michael D Kinney, tee-dev, Daniel Thompson,
Joakim Bech, Matteo.Carlini, Achin.Gupta, udit.kumar
Gentle reminder.
Please let me know if you have any further review comments.
-Sumit
On Wed, 10 Oct 2018 at 10:49, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Changes in v4:
> Replaced abbreviations with full name which are not defined in [1]. Also
> used EFI_GUID for Trusted Application UUIDs.
>
> [1] https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/4_naming_conventions/#table-2-efi-supported-abbreviations
>
> 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/Library/OpteeLib/OpteeLib.inf | 2 +
> ArmPkg/Include/Library/OpteeLib.h | 88 +++++
> ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++
> ArmPkg/Library/OpteeLib/Optee.c | 397 ++++++++++++++++++++
> 4 files changed, 530 insertions(+)
> create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
2018-10-10 5:18 ` [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE Sumit Garg
@ 2018-10-18 6:23 ` Leif Lindholm
2018-10-18 7:29 ` Sumit Garg
0 siblings, 1 reply; 9+ messages in thread
From: Leif Lindholm @ 2018-10-18 6:23 UTC (permalink / raw)
To: Sumit Garg
Cc: edk2-devel, ard.biesheuvel, michael.d.kinney, tee-dev,
daniel.thompson, joakim.bech, Matteo.Carlini, Achin.Gupta,
udit.kumar
Hi Sumit,
I have some further comments/suggestions on UUID/GUID handling below.
On Wed, Oct 10, 2018 at 10:48:53AM +0530, Sumit Garg wrote:
> Add following APIs to communicate with OP-TEE pseudo/early TAs:
> 1. OpteeInit
> 2. OpteeOpenSession
> 3. OpteeCloseSession
> 4. OpteeInvokeFunc
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
> ArmPkg/Library/OpteeLib/OpteeLib.inf | 2 +
> ArmPkg/Include/Library/OpteeLib.h | 88 +++++
> ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++
> ArmPkg/Library/OpteeLib/Optee.c | 397 ++++++++++++++++++++
> 4 files changed, 530 insertions(+)
>
> 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/Include/Library/OpteeLib.h b/ArmPkg/Include/Library/OpteeLib.h
> index f65d8674d9b8..6884d5681831 100644
> --- a/ArmPkg/Include/Library/OpteeLib.h
> +++ b/ArmPkg/Include/Library/OpteeLib.h
> @@ -25,10 +25,98 @@
> #define OPTEE_OS_UID2 0xaf630002
> #define OPTEE_OS_UID3 0xa5d5c51b
>
> +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_NONE 0x0
> +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INPUT 0x1
> +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_OUTPUT 0x2
> +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INOUT 0x3
> +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INPUT 0x9
> +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_OUTPUT 0xa
> +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INOUT 0xb
> +
> +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MASK 0xff
> +
> +#define OPTEE_ORIGIN_COMMUNICATION 0x00000002
> +#define OPTEE_ERROR_COMMUNICATION 0xFFFF000E
> +
> +typedef struct {
> + UINT64 BufferAddress;
> + UINT64 Size;
> + UINT64 SharedMemoryReference;
> +} OPTEE_MESSAGE_PARAM_MEMORY;
> +
> +typedef struct {
> + UINT64 A;
> + UINT64 B;
> + UINT64 C;
> +} OPTEE_MESSAGE_PARAM_VALUE;
> +
> +typedef struct {
> + UINT64 Attribute;
> + union {
> + OPTEE_MESSAGE_PARAM_MEMORY Memory;
> + OPTEE_MESSAGE_PARAM_VALUE Value;
> + } Union;
> +} OPTEE_MESSAGE_PARAM;
> +
> +#define OPTEE_MAX_CALL_PARAMS 4
> +
> +typedef struct {
> + UINT32 Command;
> + UINT32 Function;
> + UINT32 Session;
> + UINT32 CancelId;
> + UINT32 Pad;
> + UINT32 Return;
> + UINT32 ReturnOrigin;
> + UINT32 NumParams;
> +
> + // NumParams tells the actual number of element in Params
> + OPTEE_MESSAGE_PARAM Params[OPTEE_MAX_CALL_PARAMS];
> +} OPTEE_MESSAGE_ARG;
> +
> +typedef struct {
> + EFI_GUID Uuid; // [in] GUID/UUID of the Trusted Application
Forward reference: I have a longer comment on UUID/GUID struct usage
below. I believe this field describes a UUID held in the TEE_UUID
struct format on the OpTee side. If so, can we change this to an
OPTEE_UUID (mentioned below).
> + UINT32 Session; // [out] Session id
> + UINT32 Return; // [out] Return value
> + UINT32 ReturnOrigin; // [out] Origin of the return value
> +} OPTEE_OPEN_SESSION_ARG;
> +
> +typedef struct {
> + UINT32 Function; // [in] Trusted Application function, specific to the TA
> + UINT32 Session; // [in] Session id
> + UINT32 Return; // [out] Return value
> + UINT32 ReturnOrigin; // [out] Origin of the return value
> + OPTEE_MESSAGE_PARAM Params[OPTEE_MAX_CALL_PARAMS]; // Params for function to be invoked
> +} OPTEE_INVOKE_FUNCTION_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
> +OpteeInvokeFunction (
> + IN OUT OPTEE_INVOKE_FUNCTION_ARG *InvokeFunctionArg
> + );
> +
> #endif
> diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h b/ArmPkg/Library/OpteeLib/OpteeSmc.h
> new file mode 100644
> index 000000000000..21ff4b22ab92
> --- /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_SHARED_MEMORY_CONFIG 0xb2000007
> +
> +#define OPTEE_SMC_SHARED_MEMORY_CACHED 1
> +
> +#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTERRUPT 0xffff0004
> +
> +#define OPTEE_MESSAGE_COMMAND_OPEN_SESSION 0
> +#define OPTEE_MESSAGE_COMMAND_INVOKE_FUNCTION 1
> +#define OPTEE_MESSAGE_COMMAND_CLOSE_SESSION 2
> +
> +#define OPTEE_MESSAGE_ATTRIBUTE_META 0x100
> +
> +#define OPTEE_LOGIN_PUBLIC 0x0
> +
> +typedef struct {
> + UINTN Base;
> + UINTN Size;
> +} OPTEE_SHARED_MEMORY_INFORMATION;
> +
> +#endif
> diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c
> index 574527f8b5ea..6617126e8bdb 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_INFORMATION OpteeSharedMemoryInformation = { 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,392 @@ IsOpteePresent (
> return FALSE;
> }
> }
> +
> +STATIC
> +EFI_STATUS
> +OpteeSharedMemoryRemap (
> + VOID
> + )
> +{
> + ARM_SMC_ARGS ArmSmcArgs;
> + EFI_PHYSICAL_ADDRESS PhysicalAddress;
> + EFI_PHYSICAL_ADDRESS Start;
> + EFI_PHYSICAL_ADDRESS End;
> + EFI_STATUS Status;
> + UINTN Size;
> +
> + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS));
> + ArmSmcArgs.Arg0 = OPTEE_SMC_GET_SHARED_MEMORY_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_SHARED_MEMORY_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);
> + PhysicalAddress = 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 (PhysicalAddress, Size, EFI_MEMORY_WB);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + OpteeSharedMemoryInformation.Base = (UINTN)PhysicalAddress;
> + OpteeSharedMemoryInformation.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 = OpteeSharedMemoryRemap ();
> + 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] PhysicalArg 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 PhysicalArg
> + )
> +{
> + ARM_SMC_ARGS ArmSmcArgs;
> +
> + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS));
> + ArmSmcArgs.Arg0 = OPTEE_SMC_CALL_WITH_ARG;
> + ArmSmcArgs.Arg1 = (UINT32)(PhysicalArg >> 32);
> + ArmSmcArgs.Arg2 = (UINT32)PhysicalArg;
> +
> + while (TRUE) {
> + ArmCallSmc (&ArmSmcArgs);
> +
> + if (ArmSmcArgs.Arg0 == OPTEE_SMC_RETURN_RPC_FOREIGN_INTERRUPT) {
> + //
> + // 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;
> +}
> +
So, looking at the OpTee sources, TEE_UUID is defined as a struct, to
exactly the same layout as the EFI_GUID type (which is a typedef of
the GUID struct). Could we add a OPTEE_UUID typedef for the same
struct in OpteeLib.h?
Since it comes in as an OPTEE_MESSAGE_PARAM_VALUE, alignment is
already guaranteed to be 64-bit.
(This also deserves a comment explaining how EFI_GUID basically
follows rfc4122, but uses little-endian for the timestamp fields.)
> +STATIC
> +VOID
> +UuidToOctets (
EfiGuidToOpteeUuid would be a better.
> + OUT UINT8 *UuidOctet,
Rename Uuid?
> + IN EFI_GUID *Uuid
Guid?
> + )
> +{
> + UuidOctet[0] = Uuid->Data1 >> 24;
> + UuidOctet[1] = Uuid->Data1 >> 16;
> + UuidOctet[2] = Uuid->Data1 >> 8;
> + UuidOctet[3] = Uuid->Data1;
Then we could do
Uuid->Data1 = SwapBytes32 (Guid->Data1);
> + UuidOctet[4] = Uuid->Data2 >> 8;
> + UuidOctet[5] = Uuid->Data2;
Uuid->Data2 = SwapBytes16 (Guid->Data2);
> + UuidOctet[6] = Uuid->Data3 >> 8;
> + UuidOctet[7] = Uuid->Data3;
Uuid->Data3 = SwapBytes16 (Guid->Data3);
> + CopyMem (UuidOctet + 8, Uuid->Data4, sizeof (Uuid->Data4));
CopyMem (Uuid->Data4, Guid->Data4, sizeof (Uuid->Data4));
/
Leif
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
2018-10-18 6:23 ` Leif Lindholm
@ 2018-10-18 7:29 ` Sumit Garg
2018-10-18 8:34 ` Leif Lindholm
0 siblings, 1 reply; 9+ messages in thread
From: Sumit Garg @ 2018-10-18 7:29 UTC (permalink / raw)
To: Leif Lindholm
Cc: edk2-devel, Ard Biesheuvel, Michael D Kinney, tee-dev,
Daniel Thompson, Joakim Bech, Matteo.Carlini, Achin.Gupta,
udit.kumar
Hi Leif,
On Thu, 18 Oct 2018 at 11:53, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> Hi Sumit,
>
> I have some further comments/suggestions on UUID/GUID handling below.
>
> On Wed, Oct 10, 2018 at 10:48:53AM +0530, Sumit Garg wrote:
> > Add following APIs to communicate with OP-TEE pseudo/early TAs:
> > 1. OpteeInit
> > 2. OpteeOpenSession
> > 3. OpteeCloseSession
> > 4. OpteeInvokeFunc
> >
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> > ArmPkg/Library/OpteeLib/OpteeLib.inf | 2 +
> > ArmPkg/Include/Library/OpteeLib.h | 88 +++++
> > ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++
> > ArmPkg/Library/OpteeLib/Optee.c | 397 ++++++++++++++++++++
> > 4 files changed, 530 insertions(+)
> >
> > 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/Include/Library/OpteeLib.h b/ArmPkg/Include/Library/OpteeLib.h
> > index f65d8674d9b8..6884d5681831 100644
> > --- a/ArmPkg/Include/Library/OpteeLib.h
> > +++ b/ArmPkg/Include/Library/OpteeLib.h
> > @@ -25,10 +25,98 @@
> > #define OPTEE_OS_UID2 0xaf630002
> > #define OPTEE_OS_UID3 0xa5d5c51b
> >
> > +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_NONE 0x0
> > +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INPUT 0x1
> > +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_OUTPUT 0x2
> > +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INOUT 0x3
> > +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INPUT 0x9
> > +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_OUTPUT 0xa
> > +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MEMORY_INOUT 0xb
> > +
> > +#define OPTEE_MESSAGE_ATTRIBUTE_TYPE_MASK 0xff
> > +
> > +#define OPTEE_ORIGIN_COMMUNICATION 0x00000002
> > +#define OPTEE_ERROR_COMMUNICATION 0xFFFF000E
> > +
> > +typedef struct {
> > + UINT64 BufferAddress;
> > + UINT64 Size;
> > + UINT64 SharedMemoryReference;
> > +} OPTEE_MESSAGE_PARAM_MEMORY;
> > +
> > +typedef struct {
> > + UINT64 A;
> > + UINT64 B;
> > + UINT64 C;
> > +} OPTEE_MESSAGE_PARAM_VALUE;
> > +
> > +typedef struct {
> > + UINT64 Attribute;
> > + union {
> > + OPTEE_MESSAGE_PARAM_MEMORY Memory;
> > + OPTEE_MESSAGE_PARAM_VALUE Value;
> > + } Union;
> > +} OPTEE_MESSAGE_PARAM;
> > +
> > +#define OPTEE_MAX_CALL_PARAMS 4
> > +
> > +typedef struct {
> > + UINT32 Command;
> > + UINT32 Function;
> > + UINT32 Session;
> > + UINT32 CancelId;
> > + UINT32 Pad;
> > + UINT32 Return;
> > + UINT32 ReturnOrigin;
> > + UINT32 NumParams;
> > +
> > + // NumParams tells the actual number of element in Params
> > + OPTEE_MESSAGE_PARAM Params[OPTEE_MAX_CALL_PARAMS];
> > +} OPTEE_MESSAGE_ARG;
> > +
> > +typedef struct {
> > + EFI_GUID Uuid; // [in] GUID/UUID of the Trusted Application
>
> Forward reference: I have a longer comment on UUID/GUID struct usage
> below. I believe this field describes a UUID held in the TEE_UUID
> struct format on the OpTee side. If so, can we change this to an
> OPTEE_UUID (mentioned below).
>
> > + UINT32 Session; // [out] Session id
> > + UINT32 Return; // [out] Return value
> > + UINT32 ReturnOrigin; // [out] Origin of the return value
> > +} OPTEE_OPEN_SESSION_ARG;
> > +
> > +typedef struct {
> > + UINT32 Function; // [in] Trusted Application function, specific to the TA
> > + UINT32 Session; // [in] Session id
> > + UINT32 Return; // [out] Return value
> > + UINT32 ReturnOrigin; // [out] Origin of the return value
> > + OPTEE_MESSAGE_PARAM Params[OPTEE_MAX_CALL_PARAMS]; // Params for function to be invoked
> > +} OPTEE_INVOKE_FUNCTION_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
> > +OpteeInvokeFunction (
> > + IN OUT OPTEE_INVOKE_FUNCTION_ARG *InvokeFunctionArg
> > + );
> > +
> > #endif
> > diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h b/ArmPkg/Library/OpteeLib/OpteeSmc.h
> > new file mode 100644
> > index 000000000000..21ff4b22ab92
> > --- /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_SHARED_MEMORY_CONFIG 0xb2000007
> > +
> > +#define OPTEE_SMC_SHARED_MEMORY_CACHED 1
> > +
> > +#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTERRUPT 0xffff0004
> > +
> > +#define OPTEE_MESSAGE_COMMAND_OPEN_SESSION 0
> > +#define OPTEE_MESSAGE_COMMAND_INVOKE_FUNCTION 1
> > +#define OPTEE_MESSAGE_COMMAND_CLOSE_SESSION 2
> > +
> > +#define OPTEE_MESSAGE_ATTRIBUTE_META 0x100
> > +
> > +#define OPTEE_LOGIN_PUBLIC 0x0
> > +
> > +typedef struct {
> > + UINTN Base;
> > + UINTN Size;
> > +} OPTEE_SHARED_MEMORY_INFORMATION;
> > +
> > +#endif
> > diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c
> > index 574527f8b5ea..6617126e8bdb 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_INFORMATION OpteeSharedMemoryInformation = { 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,392 @@ IsOpteePresent (
> > return FALSE;
> > }
> > }
> > +
> > +STATIC
> > +EFI_STATUS
> > +OpteeSharedMemoryRemap (
> > + VOID
> > + )
> > +{
> > + ARM_SMC_ARGS ArmSmcArgs;
> > + EFI_PHYSICAL_ADDRESS PhysicalAddress;
> > + EFI_PHYSICAL_ADDRESS Start;
> > + EFI_PHYSICAL_ADDRESS End;
> > + EFI_STATUS Status;
> > + UINTN Size;
> > +
> > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS));
> > + ArmSmcArgs.Arg0 = OPTEE_SMC_GET_SHARED_MEMORY_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_SHARED_MEMORY_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);
> > + PhysicalAddress = 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 (PhysicalAddress, Size, EFI_MEMORY_WB);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + OpteeSharedMemoryInformation.Base = (UINTN)PhysicalAddress;
> > + OpteeSharedMemoryInformation.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 = OpteeSharedMemoryRemap ();
> > + 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] PhysicalArg 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 PhysicalArg
> > + )
> > +{
> > + ARM_SMC_ARGS ArmSmcArgs;
> > +
> > + ZeroMem (&ArmSmcArgs, sizeof (ARM_SMC_ARGS));
> > + ArmSmcArgs.Arg0 = OPTEE_SMC_CALL_WITH_ARG;
> > + ArmSmcArgs.Arg1 = (UINT32)(PhysicalArg >> 32);
> > + ArmSmcArgs.Arg2 = (UINT32)PhysicalArg;
> > +
> > + while (TRUE) {
> > + ArmCallSmc (&ArmSmcArgs);
> > +
> > + if (ArmSmcArgs.Arg0 == OPTEE_SMC_RETURN_RPC_FOREIGN_INTERRUPT) {
> > + //
> > + // 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;
> > +}
> > +
>
> So, looking at the OpTee sources, TEE_UUID is defined as a struct, to
> exactly the same layout as the EFI_GUID type (which is a typedef of
> the GUID struct). Could we add a OPTEE_UUID typedef for the same
> struct in OpteeLib.h?
>
> Since it comes in as an OPTEE_MESSAGE_PARAM_VALUE, alignment is
> already guaranteed to be 64-bit.
>
> (This also deserves a comment explaining how EFI_GUID basically
> follows rfc4122, but uses little-endian for the timestamp fields.)
>
Actually, OP-TEE also uses little-endian format for timestamp fields.
You can refer to [1] for conversion from network byte order (octets)
to little-endian and vice-versa.
So for communications among secure world and non-secure world it uses
network byte order for UUID/GUID to comply with rfc4122.
[1] https://github.com/OP-TEE/optee_os/blob/master/core/tee/uuid.c
-Sumit
> > +STATIC
> > +VOID
> > +UuidToOctets (
>
> EfiGuidToOpteeUuid would be a better.
>
> > + OUT UINT8 *UuidOctet,
>
> Rename Uuid?
>
> > + IN EFI_GUID *Uuid
>
> Guid?
>
> > + )
> > +{
> > + UuidOctet[0] = Uuid->Data1 >> 24;
> > + UuidOctet[1] = Uuid->Data1 >> 16;
> > + UuidOctet[2] = Uuid->Data1 >> 8;
> > + UuidOctet[3] = Uuid->Data1;
>
> Then we could do
> Uuid->Data1 = SwapBytes32 (Guid->Data1);
>
> > + UuidOctet[4] = Uuid->Data2 >> 8;
> > + UuidOctet[5] = Uuid->Data2;
>
> Uuid->Data2 = SwapBytes16 (Guid->Data2);
>
> > + UuidOctet[6] = Uuid->Data3 >> 8;
> > + UuidOctet[7] = Uuid->Data3;
>
> Uuid->Data3 = SwapBytes16 (Guid->Data3);
>
> > + CopyMem (UuidOctet + 8, Uuid->Data4, sizeof (Uuid->Data4));
>
> CopyMem (Uuid->Data4, Guid->Data4, sizeof (Uuid->Data4));
>
> /
> Leif
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
2018-10-18 7:29 ` Sumit Garg
@ 2018-10-18 8:34 ` Leif Lindholm
2018-10-18 9:13 ` Sumit Garg
0 siblings, 1 reply; 9+ messages in thread
From: Leif Lindholm @ 2018-10-18 8:34 UTC (permalink / raw)
To: Sumit Garg
Cc: edk2-devel, Ard Biesheuvel, Michael D Kinney, tee-dev,
Daniel Thompson, Joakim Bech, Matteo.Carlini, Achin.Gupta,
udit.kumar
On Thu, Oct 18, 2018 at 12:59:32PM +0530, Sumit Garg wrote:
> > So, looking at the OpTee sources, TEE_UUID is defined as a struct, to
> > exactly the same layout as the EFI_GUID type (which is a typedef of
> > the GUID struct). Could we add a OPTEE_UUID typedef for the same
> > struct in OpteeLib.h?
> >
> > Since it comes in as an OPTEE_MESSAGE_PARAM_VALUE, alignment is
> > already guaranteed to be 64-bit.
> >
> > (This also deserves a comment explaining how EFI_GUID basically
> > follows rfc4122, but uses little-endian for the timestamp fields.)
>
> Actually, OP-TEE also uses little-endian format for timestamp fields.
> You can refer to [1] for conversion from network byte order (octets)
> to little-endian and vice-versa.
>
> So for communications among secure world and non-secure world it uses
> network byte order for UUID/GUID to comply with rfc4122.
>
> [1] https://github.com/OP-TEE/optee_os/blob/master/core/tee/uuid.c
Huh, ok. That's good to know.
It does however not change my comments. Since we're dealing with data
structures of a known layout, I am not a fan of treating them as byte
arrays.
/
Leif
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
2018-10-18 8:34 ` Leif Lindholm
@ 2018-10-18 9:13 ` Sumit Garg
2018-10-18 9:24 ` Leif Lindholm
0 siblings, 1 reply; 9+ messages in thread
From: Sumit Garg @ 2018-10-18 9:13 UTC (permalink / raw)
To: Leif Lindholm
Cc: edk2-devel, Ard Biesheuvel, Michael D Kinney, tee-dev,
Daniel Thompson, Joakim Bech, Matteo.Carlini, Achin.Gupta,
udit.kumar
On Thu, 18 Oct 2018 at 14:04, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Oct 18, 2018 at 12:59:32PM +0530, Sumit Garg wrote:
> > > So, looking at the OpTee sources, TEE_UUID is defined as a struct, to
> > > exactly the same layout as the EFI_GUID type (which is a typedef of
> > > the GUID struct). Could we add a OPTEE_UUID typedef for the same
> > > struct in OpteeLib.h?
> > >
> > > Since it comes in as an OPTEE_MESSAGE_PARAM_VALUE, alignment is
> > > already guaranteed to be 64-bit.
> > >
> > > (This also deserves a comment explaining how EFI_GUID basically
> > > follows rfc4122, but uses little-endian for the timestamp fields.)
> >
> > Actually, OP-TEE also uses little-endian format for timestamp fields.
> > You can refer to [1] for conversion from network byte order (octets)
> > to little-endian and vice-versa.
> >
> > So for communications among secure world and non-secure world it uses
> > network byte order for UUID/GUID to comply with rfc4122.
> >
> > [1] https://github.com/OP-TEE/optee_os/blob/master/core/tee/uuid.c
>
> Huh, ok. That's good to know.
> It does however not change my comments. Since we're dealing with data
> structures of a known layout, I am not a fan of treating them as byte
> arrays.
>
But calling UUID struct with swapped timestamp as OPTEE_UUID would
also be misnomer. I am not sure regarding appropriate naming for that
struct.
On the other hand, we have byte array of 16 octets as per network byte
order complying with rfc4122 which also doesn't imply swapped
timestamp.
-Sumit
> /
> Leif
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
2018-10-18 9:13 ` Sumit Garg
@ 2018-10-18 9:24 ` Leif Lindholm
2018-10-18 10:12 ` Sumit Garg
0 siblings, 1 reply; 9+ messages in thread
From: Leif Lindholm @ 2018-10-18 9:24 UTC (permalink / raw)
To: Sumit Garg
Cc: edk2-devel, Ard Biesheuvel, Michael D Kinney, tee-dev,
Daniel Thompson, Joakim Bech, Matteo.Carlini, Achin.Gupta,
udit.kumar
On Thu, Oct 18, 2018 at 02:43:37PM +0530, Sumit Garg wrote:
> On Thu, 18 Oct 2018 at 14:04, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Thu, Oct 18, 2018 at 12:59:32PM +0530, Sumit Garg wrote:
> > > > So, looking at the OpTee sources, TEE_UUID is defined as a struct, to
> > > > exactly the same layout as the EFI_GUID type (which is a typedef of
> > > > the GUID struct). Could we add a OPTEE_UUID typedef for the same
> > > > struct in OpteeLib.h?
> > > >
> > > > Since it comes in as an OPTEE_MESSAGE_PARAM_VALUE, alignment is
> > > > already guaranteed to be 64-bit.
> > > >
> > > > (This also deserves a comment explaining how EFI_GUID basically
> > > > follows rfc4122, but uses little-endian for the timestamp fields.)
> > >
> > > Actually, OP-TEE also uses little-endian format for timestamp fields.
> > > You can refer to [1] for conversion from network byte order (octets)
> > > to little-endian and vice-versa.
> > >
> > > So for communications among secure world and non-secure world it uses
> > > network byte order for UUID/GUID to comply with rfc4122.
> > >
> > > [1] https://github.com/OP-TEE/optee_os/blob/master/core/tee/uuid.c
> >
> > Huh, ok. That's good to know.
> > It does however not change my comments. Since we're dealing with data
> > structures of a known layout, I am not a fan of treating them as byte
> > arrays.
> >
>
> But calling UUID struct with swapped timestamp as OPTEE_UUID would
> also be misnomer. I am not sure regarding appropriate naming for that
> struct.
That's a fair point. We could call it RFC4122_UUID for now.
There could even be a case to add that to BaseLib at some point (but
probably not while there is only one user).
Regards,
Leif
> On the other hand, we have byte array of 16 octets as per network byte
> order complying with rfc4122 which also doesn't imply swapped
> timestamp.
>
> -Sumit
>
> > /
> > Leif
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
2018-10-18 9:24 ` Leif Lindholm
@ 2018-10-18 10:12 ` Sumit Garg
0 siblings, 0 replies; 9+ messages in thread
From: Sumit Garg @ 2018-10-18 10:12 UTC (permalink / raw)
To: Leif Lindholm
Cc: edk2-devel, Ard Biesheuvel, Michael D Kinney, tee-dev,
Daniel Thompson, Joakim Bech, Matteo.Carlini, Achin.Gupta,
udit.kumar
On Thu, 18 Oct 2018 at 14:54, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Oct 18, 2018 at 02:43:37PM +0530, Sumit Garg wrote:
> > On Thu, 18 Oct 2018 at 14:04, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > >
> > > On Thu, Oct 18, 2018 at 12:59:32PM +0530, Sumit Garg wrote:
> > > > > So, looking at the OpTee sources, TEE_UUID is defined as a struct, to
> > > > > exactly the same layout as the EFI_GUID type (which is a typedef of
> > > > > the GUID struct). Could we add a OPTEE_UUID typedef for the same
> > > > > struct in OpteeLib.h?
> > > > >
> > > > > Since it comes in as an OPTEE_MESSAGE_PARAM_VALUE, alignment is
> > > > > already guaranteed to be 64-bit.
> > > > >
> > > > > (This also deserves a comment explaining how EFI_GUID basically
> > > > > follows rfc4122, but uses little-endian for the timestamp fields.)
> > > >
> > > > Actually, OP-TEE also uses little-endian format for timestamp fields.
> > > > You can refer to [1] for conversion from network byte order (octets)
> > > > to little-endian and vice-versa.
> > > >
> > > > So for communications among secure world and non-secure world it uses
> > > > network byte order for UUID/GUID to comply with rfc4122.
> > > >
> > > > [1] https://github.com/OP-TEE/optee_os/blob/master/core/tee/uuid.c
> > >
> > > Huh, ok. That's good to know.
> > > It does however not change my comments. Since we're dealing with data
> > > structures of a known layout, I am not a fan of treating them as byte
> > > arrays.
> > >
> >
> > But calling UUID struct with swapped timestamp as OPTEE_UUID would
> > also be misnomer. I am not sure regarding appropriate naming for that
> > struct.
>
> That's a fair point. We could call it RFC4122_UUID for now.
>
Ok then in v5 I will define this as internal communication structure
in ArmPkg/Library/OpteeLib/OpteeSmc.h and use it instead in following
manner. Please review it.
diff --git a/ArmPkg/Library/OpteeLib/OpteeSmc.h
b/ArmPkg/Library/OpteeLib/OpteeSmc.h
index 21ff4b22ab92..9cccd81810c9 100644
--- a/ArmPkg/Library/OpteeLib/OpteeSmc.h
+++ b/ArmPkg/Library/OpteeLib/OpteeSmc.h
@@ -40,4 +40,14 @@ typedef struct {
UINTN Size;
} OPTEE_SHARED_MEMORY_INFORMATION;
+//
+// UUID struct compliant with RFC4122 (network byte order).
+//
+typedef struct {
+ UINT32 Data1;
+ UINT16 Data2;
+ UINT16 Data3;
+ UINT8 Data4[8];
+} RFC4122_UUID;
+
#endif
diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c
index 6617126e8bdb..8ac31cb28266 100644
--- a/ArmPkg/Library/OpteeLib/Optee.c
+++ b/ArmPkg/Library/OpteeLib/Optee.c
@@ -165,20 +165,15 @@ OpteeCallWithArg (
STATIC
VOID
-UuidToOctets (
- OUT UINT8 *UuidOctet,
- IN EFI_GUID *Uuid
+EfiGuidToRfc4122Uuid (
+ OUT RFC4122_UUID *Rfc4122Uuid,
+ IN EFI_GUID *Guid
)
{
- 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));
+ Rfc4122Uuid->Data1 = SwapBytes32 (Guid->Data1);
+ Rfc4122Uuid->Data2 = SwapBytes16 (Guid->Data2);
+ Rfc4122Uuid->Data3 = SwapBytes16 (Guid->Data3);
+ CopyMem (Rfc4122Uuid->Data4, Guid->Data4, sizeof (Rfc4122Uuid->Data4));
}
EFI_STATUS
@@ -209,8 +204,8 @@ OpteeOpenSession (
OPTEE_MESSAGE_ATTRIBUTE_META;
MessageArg->Params[1].Attribute = OPTEE_MESSAGE_ATTRIBUTE_TYPE_VALUE_INPUT |
OPTEE_MESSAGE_ATTRIBUTE_META;
- UuidToOctets (
- (UINT8 *)&MessageArg->Params[0].Union.Value,
+ EfiGuidToRfc4122Uuid (
+ (RFC4122_UUID *)&MessageArg->Params[0].Union.Value,
&OpenSessionArg->Uuid
);
ZeroMem (&MessageArg->Params[1].Union.Value, sizeof (EFI_GUID));
-Sumit
> There could even be a case to add that to BaseLib at some point (but
> probably not while there is only one user).
>
> Regards,
>
> Leif
>
> > On the other hand, we have byte array of 16 octets as per network byte
> > order complying with rfc4122 which also doesn't imply swapped
> > timestamp.
> >
> > -Sumit
> >
> > > /
> > > Leif
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-18 10:12 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-10 5:18 [PATCH v4 0/1] Add ArmPkg/Optee library APIs Sumit Garg
2018-10-10 5:18 ` [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE Sumit Garg
2018-10-18 6:23 ` Leif Lindholm
2018-10-18 7:29 ` Sumit Garg
2018-10-18 8:34 ` Leif Lindholm
2018-10-18 9:13 ` Sumit Garg
2018-10-18 9:24 ` Leif Lindholm
2018-10-18 10:12 ` Sumit Garg
2018-10-17 12:00 ` [PATCH v4 0/1] Add ArmPkg/Optee library APIs Sumit Garg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox