From: Sumit Garg <sumit.garg@linaro.org>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: edk2-devel@lists.01.org,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Michael D Kinney <michael.d.kinney@intel.com>,
tee-dev@lists.linaro.org,
Daniel Thompson <daniel.thompson@linaro.org>,
Joakim Bech <joakim.bech@linaro.org>,
Matteo.Carlini@arm.com, Achin.Gupta@arm.com, udit.kumar@nxp.com
Subject: Re: [PATCH v4 1/1] ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
Date: Thu, 18 Oct 2018 15:42:11 +0530 [thread overview]
Message-ID: <CAFA6WYMriN=EtwxKpuO1rPaa3WHY7mn-CaVkj-93PQwoi_Hr_w@mail.gmail.com> (raw)
In-Reply-To: <20181018092432.y7y2hvq5s4vdhmnv@bivouac.eciton.net>
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
next prev parent reply other threads:[~2018-10-18 10:12 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2018-10-17 12:00 ` [PATCH v4 0/1] Add ArmPkg/Optee library APIs Sumit Garg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFA6WYMriN=EtwxKpuO1rPaa3WHY7mn-CaVkj-93PQwoi_Hr_w@mail.gmail.com' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox