On Fri, 2020-06-19 at 13:42 +0530, Sumit Garg wrote:
On Fri, 19 Jun 2020 at 00:49, James Bottomley James.Bottomley@hansenpartnership.com wrote:
On Thu, 2020-06-18 at 10:42 +0530, Sumit Garg wrote:
On Thu, 18 Jun 2020 at 10:29, Sumit Garg sumit.garg@linaro.org wrote:
[...]
typedef struct { uint32_t timeLow; uint16_t timeMid; uint16_t timeHiAndVersion; uint8_t clockSeqAndNode[8]; } TEE_UUID;
(GlobalPlatform TEE Internal Core API spec v1.2.1 section 3.2.4)
- The spec does not mandate any particular endianness and
simply warnsabout possible issues if secure and non-secure worlds differ in endianness.
- OP-TEE uses %pUl assuming that host order is little endian
(that is true for the Arm platforms that run OP-TEE currently). By the same logic %pUl should be fine in the kernel.
I think Linux adheres to this RFC [1] for UUID byte order. See below snippet from section: "Layout and Byte Order":
The fields are encoded as 16 octets, with the sizes and order of the fields defined above, and with each field encoded with the Most Significant Byte first (known as network byte order). Note that the field names, particularly for multiplexed fields, follow historical practice.
Actually, that's not quite true. We used to support both little and big endian uuids until we realised it was basically microsoft vs everyone else (as codified by RFC 4122). Now we support UUIDs which are big endian and GUIDs which are little endian. This was the commit that sorted out the confusion:
commit f9727a17db9bab71ddae91f74f11a8a2f9a0ece6 Author: Christoph Hellwig hch@lst.de Date: Wed May 17 10:02:48 2017 +0200
uuid: rename uuid typesThanks for providing the background here.
so if you're using a little endian uuid, you should probably be using GUID for TEE_UUID.
IMO, using GUID in kernel for TEE_UUID in OP-TEE OS will lead to deviation from GlobalPlatform TEE client spec [1] as the spec only references it as UUID and we would like to keep kernel TEE client interface to be compatible with GP specs.
[1] https://globalplatform.org/specs-library/tee-client-api-specifica tion/
So having read the above, you know uuid_t is for big endian and guid_t for little endian. However in your patch:
-static int optee_register_device(const uuid_t *device_uuid, u32 device_id) +static int optee_register_device(const uuid_t *device_uuid)
You're using uuid_t for little endian, you should be using guid_t. It's not about consistency with the OP-TEE docs (although I'm pretty sure they don't mandate what kernel type to use), it's about consistency with what the kernel types mean. When some checker detects your using little endian operations on a big endian structure (like in the prink for instance) they're going to keep emailing you about it.
James