On 6/17/20 9:52 PM, Maxim Uvarov wrote:
On Wed, 17 Jun 2020 at 18:16, Jerome Forissier jerome@forissier.org wrote:
On 6/17/20 3:58 PM, Sumit Garg wrote:
Hi Maxim,
On Thu, 4 Jun 2020 at 23:28, Maxim Uvarov maxim.uvarov@linaro.org wrote:
With the evolving use-cases for TEE bus, now it's required to support multi-stage enumeration process. But using a simple index doesn't suffice this requirement and instead leads to duplicate sysfs entries. So instead switch to use more informative device UUID for sysfs entry like: /sys/bus/tee/devices/optee-ta-<uuid>
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org Reviewed-by: Sumit Garg sumit.garg@linaro.org
Documentation/ABI/testing/sysfs-bus-optee-devices | 8 ++++++++ MAINTAINERS | 1 + drivers/tee/optee/device.c | 9 ++++++--- 3 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-optee-devices
diff --git a/Documentation/ABI/testing/sysfs-bus-optee-devices b/Documentation/ABI/testing/sysfs-bus-optee-devices new file mode 100644 index 000000000000..0ae04ae5374a --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-optee-devices @@ -0,0 +1,8 @@ +What: /sys/bus/tee/devices/optee-ta-<uuid>/ +Date: May 2020 +KernelVersion 5.7 +Contact: tee-dev@lists.linaro.org +Description:
OP-TEE bus provides reference to registered drivers under this directory. The <uuid>matches Trusted Application (TA) driver and corresponding TA in secure OS. Driversare free to create needed API under optee-ta-<uuid> directory.diff --git a/MAINTAINERS b/MAINTAINERS index ecc0749810b0..6717afef2de3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12516,6 +12516,7 @@ OP-TEE DRIVER M: Jens Wiklander jens.wiklander@linaro.org L: tee-dev@lists.linaro.org S: Maintained +F: Documentation/ABI/testing/sysfs-bus-optee-devices F: drivers/tee/optee/
OP-TEE RANDOM NUMBER GENERATOR (RNG) DRIVER diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index e3a148521ec1..23d264c8146e 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -65,7 +65,7 @@ static int get_devices(struct tee_context *ctx, u32 session, return 0; }
-static int optee_register_device(const uuid_t *device_uuid, u32 device_id) +static int optee_register_device(const uuid_t *device_uuid) { struct tee_client_device *optee_device = NULL; int rc; @@ -75,7 +75,10 @@ static int optee_register_device(const uuid_t *device_uuid, u32 device_id) return -ENOMEM;
optee_device->dev.bus = &tee_bus_type;
dev_set_name(&optee_device->dev, "optee-clnt%u", device_id);
if (dev_set_name(&optee_device->dev, "optee-ta-%pUl", device_uuid)) {You should be using format specifier as: "%pUb" instead of "%pUl" as UUID representation for TAs is in big endian format. See below:
Where does device_uuid come from? If it comes directly from OP-TEE, then it should be a pointer to the following struct:
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 warns
about 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.
- On the other hand, the UUID in a Trusted App header is always encoded
big endian by the Python script that signs and optionally encrypts the TA. This should not have any visible impact on UUIDs exchanged between the secure and non-secure world though.
So I am wondering why you had to use %pUb. There must be some inconsistency somewhere :-/
-- Jerome
From linux side it is for example:
static const struct tee_client_device_id optee_ftpm_id_table[] = { {UUID_INIT(0xbc50d971, 0xd4c9, 0x42c4, 0x82, 0xcb, 0x34, 0x3f, 0xb7, 0xf3, 0x78, 0x96)}, {} };
static struct tee_client_driver ftpm_tee_driver = { .id_table = optee_ftpm_id_table, .driver = {
So sysfs name has to be the same as the driver has. And UUD is simple 16 bytes:#define UUID_SIZE 16 typedef struct { __u8 b[UUID_SIZE]; } uuid_t;
From TA it also: #define TA_UUID { 0xBC50D971, 0xD4C9, 0x42C4, \ {0x82, 0xCB, 0x34, 0x3F, 0xB7, 0xF3, 0x78, 0x96}}
Compare uuid from optee and kernel driver version is simple: static inline bool uuid_equal(const uuid_t *u1, const uuid_t *u2) { return memcmp(u1, u2, sizeof(uuid_t)) == 0; }
So to support better code navigation. For example grep sources for 0xBC50D971, or find in sysfs "*bc50d971-*" I would say we need to use BE format. optee might also need to switch to BE prints for the same reason.
Sorry but this does not make much sense to me :-/
All I want to say is, if you ever need to use %pUb for things to work as expected then it is *very* suspect and you should try to understand why, because as I said and as far as I can tell OP-TEE stores all it's UUIDs in memory in little endian format (more precisely, host endian with all platforms being little endian currently), and %pUb is not even implemented in OP-TEE.