diff mbox series

[PATCHv3,2/3] optee: use uuid for sysfs driver entry

Message ID 20200525115235.5405-3-maxim.uvarov@linaro.org (mailing list archive)
State New, archived
Headers show
Series optee: register drivers on optee bus | expand

Commit Message

Maxim Uvarov May 25, 2020, 11:52 a.m. UTC
Optee device names for sysfs needed to be unique
and it's better if they will mean something. UUID for name
looks like good solution:
/sys/bus/tee/devices/optee-clnt-<uuid>

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 drivers/tee/optee/device.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Jerome Forissier May 25, 2020, 12:10 p.m. UTC | #1
On 5/25/20 1:52 PM, Maxim Uvarov wrote:
> Optee device names for sysfs needed to be unique

s/Optee/OP-TEE/
s/needed/need/

> and it's better if they will mean something. UUID for name
> looks like good solution:
> /sys/bus/tee/devices/optee-clnt-<uuid>

How about mentioning it is the UUID of the Trusted Application on the
TEE side?

> 
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  drivers/tee/optee/device.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks,
Greg Kroah-Hartman May 25, 2020, 12:47 p.m. UTC | #2
On Mon, May 25, 2020 at 02:52:34PM +0300, Maxim Uvarov wrote:
> Optee device names for sysfs needed to be unique
> and it's better if they will mean something. UUID for name
> looks like good solution:
> /sys/bus/tee/devices/optee-clnt-<uuid>

Can you document that in Documentation/ABI/ ?

And why UUID?  Those are usually huge, is that easier than just a unique
number?

thanks,

greg k-h
Maxim Uvarov May 25, 2020, 1:33 p.m. UTC | #3
On Mon, 25 May 2020 at 15:47, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, May 25, 2020 at 02:52:34PM +0300, Maxim Uvarov wrote:
> > Optee device names for sysfs needed to be unique
> > and it's better if they will mean something. UUID for name
> > looks like good solution:
> > /sys/bus/tee/devices/optee-clnt-<uuid>
>
> Can you document that in Documentation/ABI/ ?
>
yes, sure if we agree to go with uuid.

> And why UUID?  Those are usually huge, is that easier than just a unique
> number?
>

UUID here is connected to Trusted Application (TA) in a secure world.
If you need to 'find'  sysfs entry for the corresponding driver
becomes very easy.
Also UUID here are not really huge, like:
/sys/bus/tee/devices/optee-clnt-71d950bc-c9d4-c442-82cb-343fb7f37896
/sys/bus/tee/devices/optee-clnt-ba3ac5b6-6996-6846-a7f2-205629d00f86

I think that is better then optee-clnt-0, optee-clnt-1.. which can be
reordered on each boot and does not carry any information. And on
module unload there will be missing numbers.

Regards,
Maxim.

> thanks,
>
> greg k-h
Maxim Uvarov May 25, 2020, 1:36 p.m. UTC | #4
On Mon, 25 May 2020 at 15:10, Jerome Forissier <jerome@forissier.org> wrote:
>
>
>
> On 5/25/20 1:52 PM, Maxim Uvarov wrote:
> > Optee device names for sysfs needed to be unique
>
> s/Optee/OP-TEE/
> s/needed/need/
>
> > and it's better if they will mean something. UUID for name
> > looks like good solution:
> > /sys/bus/tee/devices/optee-clnt-<uuid>
>
> How about mentioning it is the UUID of the Trusted Application on the
> TEE side?
>

Jerome, do you think optee-ta-<uuid> is more suitable here?


> >
> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> > ---
> >  drivers/tee/optee/device.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
>
> Thanks,
> --
> Jerome
Jerome Forissier May 25, 2020, 3:53 p.m. UTC | #5
On 5/25/20 3:36 PM, Maxim Uvarov wrote:
> On Mon, 25 May 2020 at 15:10, Jerome Forissier <jerome@forissier.org> wrote:
>>
>>
>>
>> On 5/25/20 1:52 PM, Maxim Uvarov wrote:
>>> Optee device names for sysfs needed to be unique
>>
>> s/Optee/OP-TEE/
>> s/needed/need/
>>
>>> and it's better if they will mean something. UUID for name
>>> looks like good solution:
>>> /sys/bus/tee/devices/optee-clnt-<uuid>
>>
>> How about mentioning it is the UUID of the Trusted Application on the
>> TEE side?
>>
> 
> Jerome, do you think optee-ta-<uuid> is more suitable here?

Yes, a bit better I think. More "self explanatory"... kind of :)

Is it possible to have several devices bound to the same TA? I think
nothing forbids this although we may not have any use case for now...
Sumit Garg May 26, 2020, 10:37 a.m. UTC | #6
On Mon, 25 May 2020 at 21:24, Jerome Forissier <jerome@forissier.org> wrote:
>
>
>
> On 5/25/20 3:36 PM, Maxim Uvarov wrote:
> > On Mon, 25 May 2020 at 15:10, Jerome Forissier <jerome@forissier.org> wrote:
> >>
> >>
> >>
> >> On 5/25/20 1:52 PM, Maxim Uvarov wrote:
> >>> Optee device names for sysfs needed to be unique
> >>
> >> s/Optee/OP-TEE/
> >> s/needed/need/
> >>
> >>> and it's better if they will mean something. UUID for name
> >>> looks like good solution:
> >>> /sys/bus/tee/devices/optee-clnt-<uuid>
> >>
> >> How about mentioning it is the UUID of the Trusted Application on the
> >> TEE side?
> >>
> >
> > Jerome, do you think optee-ta-<uuid> is more suitable here?
>
> Yes, a bit better I think. More "self explanatory"... kind of :)
>

+1

> Is it possible to have several devices bound to the same TA? I think
> nothing forbids this although we may not have any use case for now...
>

A single TA is represented via a single device represented via UUID on
the TEE bus. And I can't think of a scenario where the user may not
want to split the TA so as to support a particular driver in Linux.

-Sumit

> --
> Jerome
> _______________________________________________
> Tee-dev mailing list
> Tee-dev@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/tee-dev
diff mbox series

Patch

diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c
index d4931dad07aa..aab917605e74 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,7 @@  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);
+	dev_set_name(&optee_device->dev, "optee-clnt-%pUl", device_uuid);
 	uuid_copy(&optee_device->id.uuid, device_uuid);
 
 	rc = device_register(&optee_device->dev);
@@ -144,7 +144,7 @@  static int __optee_enumerate_devices(u32 func)
 	num_devices = shm_size / sizeof(uuid_t);
 
 	for (idx = 0; idx < num_devices; idx++) {
-		rc = optee_register_device(&device_uuid[idx], idx);
+		rc = optee_register_device(&device_uuid[idx]);
 		if (rc)
 			goto out_shm;
 	}