From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::a41; helo=mail-vk1-xa41.google.com; envelope-from=sumit.garg@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-vk1-xa41.google.com (mail-vk1-xa41.google.com [IPv6:2607:f8b0:4864:20::a41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id EE49721168228 for ; Thu, 18 Oct 2018 03:12:23 -0700 (PDT) Received: by mail-vk1-xa41.google.com with SMTP id l186so6496153vke.0 for ; Thu, 18 Oct 2018 03:12:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=JXije2HmQzezaO5ek8eha93i4R953bKpVYlT+vkxIgs=; b=Ax+9y1t4Q0LIyW4Y/bdvt+IfuvuJ/8rmUHeNsKxZy0ilUTdeTeq9rE2/i98yXDKYXf 6wKOIyV4OHufrfF3IpgrCE980QeSrNQUaTt6S6hLQakN7kbzGoM/pZbGDfPf6iHUp312 a7x2D88KjmRTpoR8zDLD3JeLgehkd+5QDPohg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=JXije2HmQzezaO5ek8eha93i4R953bKpVYlT+vkxIgs=; b=am2xhJxWmjv4hugBSlmZwecwSqzrbzeQzCN5Zgr010r0f/A986mtgUFcEv+Y5oS4Zg Zi0jvg2QSmJX+jvf9eOCARHTm/ti0Q3o0jTLMY1yM/xxLW0BG7/Jy9/muBaOp6l5X9c+ baeNiHvRL0YbhcFJCz2Dbz4l0ioEcwdYI4+9Y1zF2Q4QuzJLzXee5HshzuFaQJC0fgap eAFsRtFdN5jLpv4okPOqqC5mJcXJ+r9qtcYjfeyRCsJjvuC3boOKWU1rbWjbxZO6mVzh uzq7y4v1QN90FbCCcr/XowiqJU2ctfA6pQ4/rD8k6L3CCX585Fp462++jCjpyhOX8Zha rwZg== X-Gm-Message-State: ABuFfoiYRC89T13RmO98DSR15oy0m+l2yHcIzfaalowt6IdPcw2OoCoO 9fdObAdufeJRFNfCYWR+OUr7i0/6jW4tdUS2rJ6DC10B2IE= X-Google-Smtp-Source: ACcGV63ZyqMKsnJnlwAOeiLy09+R18tghFwmfYMUGvuf+Kx60B4Sk8pSASPOuUOagGt58+WlHnr2bO8eZfX/0G2zB5k= X-Received: by 2002:a1f:56:: with SMTP id 83mr10734707vka.66.1539857542414; Thu, 18 Oct 2018 03:12:22 -0700 (PDT) MIME-Version: 1.0 References: <1539148733-5426-1-git-send-email-sumit.garg@linaro.org> <1539148733-5426-2-git-send-email-sumit.garg@linaro.org> <20181018062343.w3343srhcq4dv65p@bivouac.eciton.net> <20181018083411.iplsdsrrswvuhacl@bivouac.eciton.net> <20181018092432.y7y2hvq5s4vdhmnv@bivouac.eciton.net> In-Reply-To: <20181018092432.y7y2hvq5s4vdhmnv@bivouac.eciton.net> From: Sumit Garg Date: Thu, 18 Oct 2018 15:42:11 +0530 Message-ID: To: Leif Lindholm Cc: edk2-devel@lists.01.org, Ard Biesheuvel , Michael D Kinney , tee-dev@lists.linaro.org, Daniel Thompson , Joakim Bech , 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 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Oct 2018 10:12:24 -0000 Content-Type: text/plain; charset="UTF-8" On Thu, 18 Oct 2018 at 14:54, Leif Lindholm wrote: > > On Thu, Oct 18, 2018 at 02:43:37PM +0530, Sumit Garg wrote: > > On Thu, 18 Oct 2018 at 14:04, Leif Lindholm 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