On Thu, 25 Jun 2020 at 18:22, Maxim Uvarov maxim.uvarov@linaro.org wrote:
On Wed, 24 Jun 2020 at 18:44, Jerome Forissier jerome@forissier.org wrote:
On 6/24/20 5:21 PM, James Bottomley wrote:
On Wed, 2020-06-24 at 16:17 +0530, Sumit Garg wrote:
Apologies for delay in my reply as I was busy with some other stuff.
On Fri, 19 Jun 2020 at 20:30, James Bottomley James.Bottomley@hansenpartnership.com wrote:
[...]
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.
As mentioned above, using different terminology is meant to cause more confusion than just difference in endianness which is manageable inside TEE.
And I think it's safe to say that the kernel implements UUID in big endian format and thus uses %pUb whereas OP-TEE implements UUID in little endian format and thus uses %pUl.
So what I think you're saying is that if we still had uuid_be and uuid_le you'd use uuid_le, because that's exactly the structure described in the docs. But because we renamed
uuid_be -> uuid_t uuid_le -> guid_t
You can't use guid_t as a kernel type because it has the wrong name?
Let me try to clear the confusion that I introduce myself I believe :-/ IMO:
- optee_register_device(const uuid_t *device_uuid) *is* the correct
prototype.
- device_uuid is *guaranteed* to be BE because OP-TEE makes this
guarantee (it converts from its internal LE representation to BE when enumerating the devices, but it doesn't matter to the kernel).
- Therefore %pUb is the correct format.
I'm sorry for doubting the BE order initially. I am so used to OP-TEE using LE internally, that I missed the fact that we have an explicit conversion...
Does this sound good?
Thanks,
Jerome
I think your description is correct. But I think this problem would be solved outside of the current patchset. All places should use one single format (LE):
I guess you missed the point that uuid_t is implemented in BE format in the kernel which is compliant as per RFC 4122.
- internal optee representation;
- device enumeration pta;
- this kernel driver which creates sysfs entry and sets
uid_copy(&optee_device->id.uuid, device_uuid);
- matching function;
- drivers use UUID_INIT();
See carefully the implementation of UUID_INIT() which is in BE format.
-Sumit
In that way everything will be consistent. But it will require changing other pieces, not just the kernel. While these patches add functionality to support current device enumeration in optee os. So I think this version is ok to be applied.
Regards, Maxim.