From: Jens Wiklander <jens.wiklander@linaro.org>
To: Sumit Garg <sumit.garg@linaro.org>,
Maxim Uvarov <maxim.uvarov@linaro.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>,
Jerome Forissier <jerome@forissier.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
Arnd Bergmann <arnd@linaro.org>,
"tee-dev @ lists . linaro . org" <tee-dev@lists.linaro.org>,
Jason Gunthorpe <jgg@ziepe.ca>,
linux-integrity@vger.kernel.org, peterhuewe@gmx.de
Subject: Re: [Tee-dev] [PATCHv8 1/3] optee: use uuid for sysfs driver entry
Date: Wed, 24 Jun 2020 11:50:55 +0200 [thread overview]
Message-ID: <20200624095055.GA1336604@jade> (raw)
In-Reply-To: <1592578844.4369.5.camel@HansenPartnership.com>
On Fri, Jun 19, 2020 at 08:00:44AM -0700, James Bottomley wrote:
> 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 types
> > >
> >
> > Thanks 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.
Thanks for the clarification. Sumit, Maxim, please take care of this.
Cheers,
Jens
>
> James
>
next prev parent reply other threads:[~2020-06-24 9:51 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-04 17:58 [PATCHv8 0/3] optee: register drivers on optee bus Maxim Uvarov
2020-06-04 17:58 ` [PATCHv8 1/3] optee: use uuid for sysfs driver entry Maxim Uvarov
2020-06-16 20:50 ` Jarkko Sakkinen
2020-06-17 6:07 ` Sumit Garg
2020-06-17 6:45 ` Maxim Uvarov
2020-06-17 13:58 ` Sumit Garg
2020-06-17 14:12 ` Maxim Uvarov
2020-06-17 15:16 ` [Tee-dev] " Jerome Forissier
2020-06-17 19:52 ` Maxim Uvarov
2020-06-17 20:45 ` Jerome Forissier
2020-06-18 4:59 ` Sumit Garg
2020-06-18 5:12 ` Sumit Garg
2020-06-18 19:18 ` James Bottomley
2020-06-19 8:12 ` Sumit Garg
2020-06-19 15:00 ` James Bottomley
2020-06-24 9:50 ` Jens Wiklander [this message]
2020-06-24 10:47 ` Sumit Garg
2020-06-24 15:21 ` James Bottomley
2020-06-24 15:44 ` Jerome Forissier
2020-06-25 12:51 ` Maxim Uvarov
2020-06-26 5:13 ` Sumit Garg
2020-06-26 8:47 ` Andy Shevchenko
2020-06-26 5:26 ` Sumit Garg
2020-06-25 14:24 ` Sumit Garg
2020-06-25 23:31 ` James Bottomley
2020-06-26 5:10 ` Sumit Garg
2020-06-26 7:26 ` Sumit Garg
2020-06-26 11:29 ` Daniel Thompson
2020-06-26 15:11 ` James Bottomley
2020-06-29 6:55 ` Sumit Garg
2020-06-29 7:03 ` Jens Wiklander
2020-06-18 6:57 ` Jerome Forissier
2020-06-18 7:44 ` Maxim Uvarov
2020-06-04 17:58 ` [PATCHv8 2/3] optee: enable support for multi-stage bus enumeration Maxim Uvarov
2020-06-16 20:56 ` Jarkko Sakkinen
2020-06-04 17:58 ` [PATCHv8 3/3] tpm_ftpm_tee: register driver on TEE bus Maxim Uvarov
2020-06-16 20:57 ` Jarkko Sakkinen
2020-06-15 14:32 ` [PATCHv8 0/3] optee: register drivers on optee bus Maxim Uvarov
2020-06-16 8:29 ` Jens Wiklander
2020-06-17 14:26 ` Maxim Uvarov
2020-06-18 8:00 ` Jens Wiklander
2020-06-17 23:37 ` Jarkko Sakkinen
2020-06-18 7:56 ` Jens Wiklander
2020-06-23 0:50 ` Jarkko Sakkinen
2020-06-16 20:49 ` Jarkko Sakkinen
2020-06-16 20:54 ` Jarkko Sakkinen
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-all 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=20200624095055.GA1336604@jade \
--to=jens.wiklander@linaro.org \
--cc=James.Bottomley@HansenPartnership.com \
--cc=arnd@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=jarkko.sakkinen@linux.intel.com \
--cc=jerome@forissier.org \
--cc=jgg@ziepe.ca \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maxim.uvarov@linaro.org \
--cc=peterhuewe@gmx.de \
--cc=sumit.garg@linaro.org \
--cc=tee-dev@lists.linaro.org \
/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;
as well as URLs for NNTP newsgroup(s).