On Thu, 18 Oct 2018 at 14:54, Leif Lindholm <leif.lindh...@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.lindh...@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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel