* [PATCHv3 0/3] optee: register drivers on optee bus
@ 2020-05-25 11:52 Maxim Uvarov
2020-05-25 11:52 ` [PATCHv3 1/3] optee: do drivers initialization before and after tee-supplicant run Maxim Uvarov
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Maxim Uvarov @ 2020-05-25 11:52 UTC (permalink / raw)
To: linux-kernel, tee-dev
Cc: peterhuewe, jarkko.sakkinen, jgg, gregkh, jens.wiklander,
linux-integrity, arnd, sumit.garg, Maxim Uvarov
v3: - support tee-suppicant restart (Jens Wiklander)
- description and comments ( Jarkko Sakkinen)
- do not name optee drivers by index in sysfs (Sumit Garg)
v2: - write TEE with capital letters.
- declare __optee_enumerate_device() as static.
Hello,
This patchset fixes issues with probing() tee, optee and optee driver
if they were compiled into kernel, built as modules or any mixed
combination.
These changes require optee-os changes which already were merged.
Main corresponding commits are:
https://github.com/OP-TEE/optee_os/commit/9389d8030ef198c9d7b8ab7ea8e877e0ace3369d
https://github.com/OP-TEE/optee_os/commit/bc5921cdab538c8ae48422f5ffd600f1cbdd95b2
optee_enumerate_devices() which discovers Trusted Applications on tee
bus is split up on 2 changes. Do probe of drivers which do not require
userspace support of tee-supplicant and stage two to run drivers with
support of tee-supplicant only after tee supplicant run.
Best regards,
Maxim.
Maxim Uvarov (3):
optee: do drivers initialization before and after tee-supplicant run
optee: use uuid for sysfs driver entry
tpm_ftpm_tee: register driver on TEE bus
drivers/char/tpm/tpm_ftpm_tee.c | 70 ++++++++++++++++++++++++++-----
drivers/tee/optee/core.c | 28 +++++++++++--
drivers/tee/optee/device.c | 23 ++++++----
drivers/tee/optee/optee_private.h | 10 ++++-
4 files changed, 108 insertions(+), 23 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCHv3 1/3] optee: do drivers initialization before and after tee-supplicant run 2020-05-25 11:52 [PATCHv3 0/3] optee: register drivers on optee bus Maxim Uvarov @ 2020-05-25 11:52 ` Maxim Uvarov 2020-05-27 19:55 ` Jarkko Sakkinen 2020-05-25 11:52 ` [PATCHv3 2/3] optee: use uuid for sysfs driver entry Maxim Uvarov 2020-05-25 11:52 ` [PATCHv3 3/3] tpm_ftpm_tee: register driver on TEE bus Maxim Uvarov 2 siblings, 1 reply; 12+ messages in thread From: Maxim Uvarov @ 2020-05-25 11:52 UTC (permalink / raw) To: linux-kernel, tee-dev Cc: peterhuewe, jarkko.sakkinen, jgg, gregkh, jens.wiklander, linux-integrity, arnd, sumit.garg, Maxim Uvarov Some drivers (like ftpm) can operate only after tee-supplicant runs becase of tee-supplicant provides things like storage services. This patch splits probe of non tee-supplicant dependable drivers to early stage, and after tee-supplicant run probe other drivers. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> Suggested-by: Sumit Garg <sumit.garg@linaro.org> Suggested-by: Arnd Bergmann <arnd@linaro.org> --- drivers/tee/optee/core.c | 28 +++++++++++++++++++++++++--- drivers/tee/optee/device.c | 17 +++++++++++------ drivers/tee/optee/optee_private.h | 10 +++++++++- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 99698b8a3a74..d059e3ac491c 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -17,6 +17,7 @@ #include <linux/tee_drv.h> #include <linux/types.h> #include <linux/uaccess.h> +#include <linux/workqueue.h> #include "optee_private.h" #include "optee_smc.h" #include "shm_pool.h" @@ -218,6 +219,15 @@ static void optee_get_version(struct tee_device *teedev, *vers = v; } +static void optee_bus_scan(struct work_struct *work) +{ + int rc; + + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP); + if (rc) + pr_err("optee_enumerate_devices failed %d\n", rc); +} + static int optee_open(struct tee_context *ctx) { struct optee_context_data *ctxdata; @@ -241,8 +251,18 @@ static int optee_open(struct tee_context *ctx) kfree(ctxdata); return -EBUSY; } - } + if (!optee->scan_bus_done) { + INIT_WORK(&optee->scan_bus_work, optee_bus_scan); + optee->scan_bus_wq = create_workqueue("optee_bus_scan"); + if (!optee->scan_bus_wq) { + pr_err("optee: couldn't create workqueue\n"); + return -ECHILD; + } + queue_work(optee->scan_bus_wq, &optee->scan_bus_work); + optee->scan_bus_done = true; + } + } mutex_init(&ctxdata->mutex); INIT_LIST_HEAD(&ctxdata->sess_list); @@ -296,8 +316,10 @@ static void optee_release(struct tee_context *ctx) ctx->data = NULL; - if (teedev == optee->supp_teedev) + if (teedev == optee->supp_teedev) { + destroy_workqueue(optee->scan_bus_wq); optee_supp_release(&optee->supp); + } } static const struct tee_driver_ops optee_ops = { @@ -675,7 +697,7 @@ static int optee_probe(struct platform_device *pdev) platform_set_drvdata(pdev, optee); - rc = optee_enumerate_devices(); + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES); if (rc) { optee_remove(pdev); return rc; diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index e3a148521ec1..d4931dad07aa 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -21,7 +21,6 @@ * TEE_ERROR_BAD_PARAMETERS - Incorrect input param * TEE_ERROR_SHORT_BUFFER - Output buffer size less than required */ -#define PTA_CMD_GET_DEVICES 0x0 static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) { @@ -32,7 +31,8 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) } static int get_devices(struct tee_context *ctx, u32 session, - struct tee_shm *device_shm, u32 *shm_size) + struct tee_shm *device_shm, u32 *shm_size, + u32 func) { int ret = 0; struct tee_ioctl_invoke_arg inv_arg; @@ -42,7 +42,7 @@ static int get_devices(struct tee_context *ctx, u32 session, memset(¶m, 0, sizeof(param)); /* Invoke PTA_CMD_GET_DEVICES function */ - inv_arg.func = PTA_CMD_GET_DEVICES; + inv_arg.func = func; inv_arg.session = session; inv_arg.num_params = 4; @@ -87,7 +87,7 @@ static int optee_register_device(const uuid_t *device_uuid, u32 device_id) return rc; } -int optee_enumerate_devices(void) +static int __optee_enumerate_devices(u32 func) { const uuid_t pta_uuid = UUID_INIT(0x7011a688, 0xddde, 0x4053, @@ -118,7 +118,7 @@ int optee_enumerate_devices(void) goto out_ctx; } - rc = get_devices(ctx, sess_arg.session, NULL, &shm_size); + rc = get_devices(ctx, sess_arg.session, NULL, &shm_size, func); if (rc < 0 || !shm_size) goto out_sess; @@ -130,7 +130,7 @@ int optee_enumerate_devices(void) goto out_sess; } - rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size); + rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size, func); if (rc < 0) goto out_shm; @@ -158,3 +158,8 @@ int optee_enumerate_devices(void) return rc; } + +int optee_enumerate_devices(u32 func) +{ + return __optee_enumerate_devices(func); +} diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index d9c5037b4e03..8b71839a357e 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -78,6 +78,9 @@ struct optee_supp { * @memremaped_shm virtual address of memory in shared memory pool * @sec_caps: secure world capabilities defined by * OPTEE_SMC_SEC_CAP_* in optee_smc.h + * @scan_bus_done flag if device registation was already done. + * @scan_bus_wq workqueue to scan optee bus and register optee drivers + * @scan_bus_work workq to scan optee bus and register optee drivers */ struct optee { struct tee_device *supp_teedev; @@ -89,6 +92,9 @@ struct optee { struct tee_shm_pool *pool; void *memremaped_shm; u32 sec_caps; + bool scan_bus_done; + struct workqueue_struct *scan_bus_wq; + struct work_struct scan_bus_work; }; struct optee_session { @@ -173,7 +179,9 @@ void optee_free_pages_list(void *array, size_t num_entries); void optee_fill_pages_list(u64 *dst, struct page **pages, int num_pages, size_t page_offset); -int optee_enumerate_devices(void); +#define PTA_CMD_GET_DEVICES 0x0 +#define PTA_CMD_GET_DEVICES_SUPP 0x1 +int optee_enumerate_devices(u32 func); /* * Small helpers -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv3 1/3] optee: do drivers initialization before and after tee-supplicant run 2020-05-25 11:52 ` [PATCHv3 1/3] optee: do drivers initialization before and after tee-supplicant run Maxim Uvarov @ 2020-05-27 19:55 ` Jarkko Sakkinen 2020-05-28 8:26 ` Maxim Uvarov 0 siblings, 1 reply; 12+ messages in thread From: Jarkko Sakkinen @ 2020-05-27 19:55 UTC (permalink / raw) To: Maxim Uvarov, linux-kernel, tee-dev Cc: peterhuewe, jgg, gregkh, jens.wiklander, linux-integrity, arnd, sumit.garg On Mon, 2020-05-25 at 14:52 +0300, Maxim Uvarov wrote: > Some drivers (like ftpm) can operate only after tee-supplicant > runs becase of tee-supplicant provides things like storage > services. This patch splits probe of non tee-supplicant dependable > drivers to early stage, and after tee-supplicant run probe other > drivers. Fix the typos and remove extra spaces (one space after '.'). Why "tee-supplicant" and not "TEE-supplicant"? > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > Suggested-by: Sumit Garg <sumit.garg@linaro.org> > Suggested-by: Arnd Bergmann <arnd@linaro.org> > --- > drivers/tee/optee/core.c | 28 +++++++++++++++++++++++++--- > drivers/tee/optee/device.c | 17 +++++++++++------ > drivers/tee/optee/optee_private.h | 10 +++++++++- > 3 files changed, 45 insertions(+), 10 deletions(-) > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c > index 99698b8a3a74..d059e3ac491c 100644 > --- a/drivers/tee/optee/core.c > +++ b/drivers/tee/optee/core.c > @@ -17,6 +17,7 @@ > #include <linux/tee_drv.h> > #include <linux/types.h> > #include <linux/uaccess.h> > +#include <linux/workqueue.h> > #include "optee_private.h" > #include "optee_smc.h" > #include "shm_pool.h" > @@ -218,6 +219,15 @@ static void optee_get_version(struct tee_device *teedev, > *vers = v; > } > > +static void optee_bus_scan(struct work_struct *work) > +{ > + int rc; > + > + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP); > + if (rc) > + pr_err("optee_enumerate_devices failed %d\n", rc); > +} Is pr_err() required really here? Why not just pr_info()? Why have it all and not just ftrace when needed? There is series going on to add CONFIG_FTRACE_FUNC_PROTOTYPE to that to get return values and arguments. > + > static int optee_open(struct tee_context *ctx) > { > struct optee_context_data *ctxdata; > @@ -241,8 +251,18 @@ static int optee_open(struct tee_context *ctx) > kfree(ctxdata); > return -EBUSY; > } > - } > > + if (!optee->scan_bus_done) { > + INIT_WORK(&optee->scan_bus_work, optee_bus_scan); > + optee->scan_bus_wq = create_workqueue("optee_bus_scan"); > + if (!optee->scan_bus_wq) { > + pr_err("optee: couldn't create workqueue\n"); Neither get pr_err() here. Please remove this one completely. > + return -ECHILD; > + } > + queue_work(optee->scan_bus_wq, &optee->scan_bus_work); > + optee->scan_bus_done = true; > + } > + } > mutex_init(&ctxdata->mutex); > INIT_LIST_HEAD(&ctxdata->sess_list); > > @@ -296,8 +316,10 @@ static void optee_release(struct tee_context *ctx) > > ctx->data = NULL; > > - if (teedev == optee->supp_teedev) > + if (teedev == optee->supp_teedev) { > + destroy_workqueue(optee->scan_bus_wq); > optee_supp_release(&optee->supp); > + } > } > > static const struct tee_driver_ops optee_ops = { > @@ -675,7 +697,7 @@ static int optee_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, optee); > > - rc = optee_enumerate_devices(); > + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES); > if (rc) { > optee_remove(pdev); > return rc; > diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c > index e3a148521ec1..d4931dad07aa 100644 > --- a/drivers/tee/optee/device.c > +++ b/drivers/tee/optee/device.c > @@ -21,7 +21,6 @@ > * TEE_ERROR_BAD_PARAMETERS - Incorrect input param > * TEE_ERROR_SHORT_BUFFER - Output buffer size less than required > */ > -#define PTA_CMD_GET_DEVICES 0x0 > > static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) > { > @@ -32,7 +31,8 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) > } > > static int get_devices(struct tee_context *ctx, u32 session, > - struct tee_shm *device_shm, u32 *shm_size) > + struct tee_shm *device_shm, u32 *shm_size, > + u32 func) > { > int ret = 0; > struct tee_ioctl_invoke_arg inv_arg; > @@ -42,7 +42,7 @@ static int get_devices(struct tee_context *ctx, u32 session, > memset(¶m, 0, sizeof(param)); > > /* Invoke PTA_CMD_GET_DEVICES function */ > - inv_arg.func = PTA_CMD_GET_DEVICES; > + inv_arg.func = func; > inv_arg.session = session; > inv_arg.num_params = 4; > > @@ -87,7 +87,7 @@ static int optee_register_device(const uuid_t *device_uuid, u32 device_id) > return rc; > } > > -int optee_enumerate_devices(void) > +static int __optee_enumerate_devices(u32 func) > { > const uuid_t pta_uuid = > UUID_INIT(0x7011a688, 0xddde, 0x4053, > @@ -118,7 +118,7 @@ int optee_enumerate_devices(void) > goto out_ctx; > } > > - rc = get_devices(ctx, sess_arg.session, NULL, &shm_size); > + rc = get_devices(ctx, sess_arg.session, NULL, &shm_size, func); > if (rc < 0 || !shm_size) > goto out_sess; > > @@ -130,7 +130,7 @@ int optee_enumerate_devices(void) > goto out_sess; > } > > - rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size); > + rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size, func); > if (rc < 0) > goto out_shm; > > @@ -158,3 +158,8 @@ int optee_enumerate_devices(void) > > return rc; > } > + > +int optee_enumerate_devices(u32 func) > +{ > + return __optee_enumerate_devices(func); > +} > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h > index d9c5037b4e03..8b71839a357e 100644 > --- a/drivers/tee/optee/optee_private.h > +++ b/drivers/tee/optee/optee_private.h > @@ -78,6 +78,9 @@ struct optee_supp { > * @memremaped_shm virtual address of memory in shared memory pool > * @sec_caps: secure world capabilities defined by > * OPTEE_SMC_SEC_CAP_* in optee_smc.h > + * @scan_bus_done flag if device registation was already done. > + * @scan_bus_wq workqueue to scan optee bus and register optee drivers > + * @scan_bus_work workq to scan optee bus and register optee drivers > */ > struct optee { > struct tee_device *supp_teedev; > @@ -89,6 +92,9 @@ struct optee { > struct tee_shm_pool *pool; > void *memremaped_shm; > u32 sec_caps; > + bool scan_bus_done; > + struct workqueue_struct *scan_bus_wq; > + struct work_struct scan_bus_work; > }; > > struct optee_session { > @@ -173,7 +179,9 @@ void optee_free_pages_list(void *array, size_t num_entries); > void optee_fill_pages_list(u64 *dst, struct page **pages, int num_pages, > size_t page_offset); > > -int optee_enumerate_devices(void); > +#define PTA_CMD_GET_DEVICES 0x0 > +#define PTA_CMD_GET_DEVICES_SUPP 0x1 > +int optee_enumerate_devices(u32 func); > > /* > * Small helpers No exalanation of "scan bus" neither in the comments nor in the code. /Jarkko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 1/3] optee: do drivers initialization before and after tee-supplicant run 2020-05-27 19:55 ` Jarkko Sakkinen @ 2020-05-28 8:26 ` Maxim Uvarov 0 siblings, 0 replies; 12+ messages in thread From: Maxim Uvarov @ 2020-05-28 8:26 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Linux Kernel Mailing List, tee-dev @ lists . linaro . org, peterhuewe, Jason Gunthorpe, Greg Kroah-Hartman, Jens Wiklander, linux-integrity, Arnd Bergmann, Sumit Garg On Wed, 27 May 2020 at 22:56, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > On Mon, 2020-05-25 at 14:52 +0300, Maxim Uvarov wrote: > > Some drivers (like ftpm) can operate only after tee-supplicant > > runs becase of tee-supplicant provides things like storage > > services. This patch splits probe of non tee-supplicant dependable > > drivers to early stage, and after tee-supplicant run probe other > > drivers. > > Fix the typos and remove extra spaces (one space after '.'). > > Why "tee-supplicant" and not "TEE-supplicant"? > > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > > Suggested-by: Sumit Garg <sumit.garg@linaro.org> > > Suggested-by: Arnd Bergmann <arnd@linaro.org> > > --- > > drivers/tee/optee/core.c | 28 +++++++++++++++++++++++++--- > > drivers/tee/optee/device.c | 17 +++++++++++------ > > drivers/tee/optee/optee_private.h | 10 +++++++++- > > 3 files changed, 45 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c > > index 99698b8a3a74..d059e3ac491c 100644 > > --- a/drivers/tee/optee/core.c > > +++ b/drivers/tee/optee/core.c > > @@ -17,6 +17,7 @@ > > #include <linux/tee_drv.h> > > #include <linux/types.h> > > #include <linux/uaccess.h> > > +#include <linux/workqueue.h> > > #include "optee_private.h" > > #include "optee_smc.h" > > #include "shm_pool.h" > > @@ -218,6 +219,15 @@ static void optee_get_version(struct tee_device *teedev, > > *vers = v; > > } > > > > +static void optee_bus_scan(struct work_struct *work) > > +{ > > + int rc; > > + > > + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP); > > + if (rc) > > + pr_err("optee_enumerate_devices failed %d\n", rc); > > +} > > Is pr_err() required really here? Why not just pr_info()? > > Why have it all and not just ftrace when needed? There is series going > on to add CONFIG_FTRACE_FUNC_PROTOTYPE to that to get return values and > arguments. > Actually if optee_enumerate_devices() fail then I would say there is something wrong with memory or Secure OS and more likely we should stop execution. I think something line WARN_ON might be more suitable here. What do you think? > > + > > static int optee_open(struct tee_context *ctx) > > { > > struct optee_context_data *ctxdata; > > @@ -241,8 +251,18 @@ static int optee_open(struct tee_context *ctx) > > kfree(ctxdata); > > return -EBUSY; > > } > > - } > > > > + if (!optee->scan_bus_done) { > > + INIT_WORK(&optee->scan_bus_work, optee_bus_scan); > > + optee->scan_bus_wq = create_workqueue("optee_bus_scan"); > > + if (!optee->scan_bus_wq) { > > + pr_err("optee: couldn't create workqueue\n"); > > Neither get pr_err() here. Please remove this one completely. > What is the benefit of removing print here? Kernel binary size? > > + return -ECHILD; > > + } > > + queue_work(optee->scan_bus_wq, &optee->scan_bus_work); > > + optee->scan_bus_done = true; > > + } > > + } > > mutex_init(&ctxdata->mutex); > > INIT_LIST_HEAD(&ctxdata->sess_list); > > > > @@ -296,8 +316,10 @@ static void optee_release(struct tee_context *ctx) > > > > ctx->data = NULL; > > > > - if (teedev == optee->supp_teedev) > > + if (teedev == optee->supp_teedev) { > > + destroy_workqueue(optee->scan_bus_wq); > > optee_supp_release(&optee->supp); > > + } > > } > > > > static const struct tee_driver_ops optee_ops = { > > @@ -675,7 +697,7 @@ static int optee_probe(struct platform_device *pdev) > > > > platform_set_drvdata(pdev, optee); > > > > - rc = optee_enumerate_devices(); > > + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES); > > if (rc) { > > optee_remove(pdev); > > return rc; > > diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c > > index e3a148521ec1..d4931dad07aa 100644 > > --- a/drivers/tee/optee/device.c > > +++ b/drivers/tee/optee/device.c > > @@ -21,7 +21,6 @@ > > * TEE_ERROR_BAD_PARAMETERS - Incorrect input param > > * TEE_ERROR_SHORT_BUFFER - Output buffer size less than required > > */ > > -#define PTA_CMD_GET_DEVICES 0x0 > > > > static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) > > { > > @@ -32,7 +31,8 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) > > } > > > > static int get_devices(struct tee_context *ctx, u32 session, > > - struct tee_shm *device_shm, u32 *shm_size) > > + struct tee_shm *device_shm, u32 *shm_size, > > + u32 func) > > { > > int ret = 0; > > struct tee_ioctl_invoke_arg inv_arg; > > @@ -42,7 +42,7 @@ static int get_devices(struct tee_context *ctx, u32 session, > > memset(¶m, 0, sizeof(param)); > > > > /* Invoke PTA_CMD_GET_DEVICES function */ > > - inv_arg.func = PTA_CMD_GET_DEVICES; > > + inv_arg.func = func; > > inv_arg.session = session; > > inv_arg.num_params = 4; > > > > @@ -87,7 +87,7 @@ static int optee_register_device(const uuid_t *device_uuid, u32 device_id) > > return rc; > > } > > > > -int optee_enumerate_devices(void) > > +static int __optee_enumerate_devices(u32 func) > > { > > const uuid_t pta_uuid = > > UUID_INIT(0x7011a688, 0xddde, 0x4053, > > @@ -118,7 +118,7 @@ int optee_enumerate_devices(void) > > goto out_ctx; > > } > > > > - rc = get_devices(ctx, sess_arg.session, NULL, &shm_size); > > + rc = get_devices(ctx, sess_arg.session, NULL, &shm_size, func); > > if (rc < 0 || !shm_size) > > goto out_sess; > > > > @@ -130,7 +130,7 @@ int optee_enumerate_devices(void) > > goto out_sess; > > } > > > > - rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size); > > + rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size, func); > > if (rc < 0) > > goto out_shm; > > > > @@ -158,3 +158,8 @@ int optee_enumerate_devices(void) > > > > return rc; > > } > > + > > +int optee_enumerate_devices(u32 func) > > +{ > > + return __optee_enumerate_devices(func); > > +} > > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h > > index d9c5037b4e03..8b71839a357e 100644 > > --- a/drivers/tee/optee/optee_private.h > > +++ b/drivers/tee/optee/optee_private.h > > @@ -78,6 +78,9 @@ struct optee_supp { > > * @memremaped_shm virtual address of memory in shared memory pool > > * @sec_caps: secure world capabilities defined by > > * OPTEE_SMC_SEC_CAP_* in optee_smc.h > > + * @scan_bus_done flag if device registation was already done. > > + * @scan_bus_wq workqueue to scan optee bus and register optee drivers > > + * @scan_bus_work workq to scan optee bus and register optee drivers > > */ > > struct optee { > > struct tee_device *supp_teedev; > > @@ -89,6 +92,9 @@ struct optee { > > struct tee_shm_pool *pool; > > void *memremaped_shm; > > u32 sec_caps; > > + bool scan_bus_done; > > + struct workqueue_struct *scan_bus_wq; > > + struct work_struct scan_bus_work; > > }; > > > > struct optee_session { > > @@ -173,7 +179,9 @@ void optee_free_pages_list(void *array, size_t num_entries); > > void optee_fill_pages_list(u64 *dst, struct page **pages, int num_pages, > > size_t page_offset); > > > > -int optee_enumerate_devices(void); > > +#define PTA_CMD_GET_DEVICES 0x0 > > +#define PTA_CMD_GET_DEVICES_SUPP 0x1 > > +int optee_enumerate_devices(u32 func); > > > > /* > > * Small helpers > > No exalanation of "scan bus" neither in the comments nor in the code. > Idea was when tee-supplicant opens /dev/tee-priv0 then we run workqueue (to not hold current open()) and do bus scan for TA applications which require tee-supplicant run. There is no "scan bus" as API. The is optee_enumerate_devices() is split up on 2 stages. Maxim. > /Jarkko > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv3 2/3] optee: use uuid for sysfs driver entry 2020-05-25 11:52 [PATCHv3 0/3] optee: register drivers on optee bus Maxim Uvarov 2020-05-25 11:52 ` [PATCHv3 1/3] optee: do drivers initialization before and after tee-supplicant run Maxim Uvarov @ 2020-05-25 11:52 ` Maxim Uvarov 2020-05-25 12:10 ` [Tee-dev] " Jerome Forissier 2020-05-25 12:47 ` Greg KH 2020-05-25 11:52 ` [PATCHv3 3/3] tpm_ftpm_tee: register driver on TEE bus Maxim Uvarov 2 siblings, 2 replies; 12+ messages in thread From: Maxim Uvarov @ 2020-05-25 11:52 UTC (permalink / raw) To: linux-kernel, tee-dev Cc: peterhuewe, jarkko.sakkinen, jgg, gregkh, jens.wiklander, linux-integrity, arnd, sumit.garg, Maxim Uvarov 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(-) 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; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Tee-dev] [PATCHv3 2/3] optee: use uuid for sysfs driver entry 2020-05-25 11:52 ` [PATCHv3 2/3] optee: use uuid for sysfs driver entry Maxim Uvarov @ 2020-05-25 12:10 ` Jerome Forissier 2020-05-25 13:36 ` Maxim Uvarov 2020-05-25 12:47 ` Greg KH 1 sibling, 1 reply; 12+ messages in thread From: Jerome Forissier @ 2020-05-25 12:10 UTC (permalink / raw) To: Maxim Uvarov, linux-kernel, tee-dev Cc: gregkh, jarkko.sakkinen, jgg, arnd, linux-integrity, peterhuewe 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, -- Jerome ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tee-dev] [PATCHv3 2/3] optee: use uuid for sysfs driver entry 2020-05-25 12:10 ` [Tee-dev] " Jerome Forissier @ 2020-05-25 13:36 ` Maxim Uvarov 2020-05-25 15:53 ` Jerome Forissier 0 siblings, 1 reply; 12+ messages in thread From: Maxim Uvarov @ 2020-05-25 13:36 UTC (permalink / raw) To: Jerome Forissier Cc: Linux Kernel Mailing List, tee-dev @ lists . linaro . org, Greg Kroah-Hartman, Jarkko Sakkinen, Jason Gunthorpe, Arnd Bergmann, linux-integrity, peterhuewe 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tee-dev] [PATCHv3 2/3] optee: use uuid for sysfs driver entry 2020-05-25 13:36 ` Maxim Uvarov @ 2020-05-25 15:53 ` Jerome Forissier 2020-05-26 10:37 ` Sumit Garg 0 siblings, 1 reply; 12+ messages in thread From: Jerome Forissier @ 2020-05-25 15:53 UTC (permalink / raw) To: Maxim Uvarov Cc: Linux Kernel Mailing List, tee-dev @ lists . linaro . org, Greg Kroah-Hartman, Jarkko Sakkinen, Jason Gunthorpe, Arnd Bergmann, linux-integrity, peterhuewe 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... -- Jerome ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tee-dev] [PATCHv3 2/3] optee: use uuid for sysfs driver entry 2020-05-25 15:53 ` Jerome Forissier @ 2020-05-26 10:37 ` Sumit Garg 0 siblings, 0 replies; 12+ messages in thread From: Sumit Garg @ 2020-05-26 10:37 UTC (permalink / raw) To: Jerome Forissier Cc: Maxim Uvarov, Greg Kroah-Hartman, Linux Kernel Mailing List, Jarkko Sakkinen, tee-dev @ lists . linaro . org, Jason Gunthorpe, linux-integrity, Arnd Bergmann, peterhuewe 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 2/3] optee: use uuid for sysfs driver entry 2020-05-25 11:52 ` [PATCHv3 2/3] optee: use uuid for sysfs driver entry Maxim Uvarov 2020-05-25 12:10 ` [Tee-dev] " Jerome Forissier @ 2020-05-25 12:47 ` Greg KH 2020-05-25 13:33 ` Maxim Uvarov 1 sibling, 1 reply; 12+ messages in thread From: Greg KH @ 2020-05-25 12:47 UTC (permalink / raw) To: Maxim Uvarov Cc: linux-kernel, tee-dev, peterhuewe, jarkko.sakkinen, jgg, jens.wiklander, linux-integrity, arnd, sumit.garg 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 2/3] optee: use uuid for sysfs driver entry 2020-05-25 12:47 ` Greg KH @ 2020-05-25 13:33 ` Maxim Uvarov 0 siblings, 0 replies; 12+ messages in thread From: Maxim Uvarov @ 2020-05-25 13:33 UTC (permalink / raw) To: Greg KH Cc: Linux Kernel Mailing List, tee-dev @ lists . linaro . org, peterhuewe, Jarkko Sakkinen, Jason Gunthorpe, Jens Wiklander, linux-integrity, Arnd Bergmann, Sumit Garg 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv3 3/3] tpm_ftpm_tee: register driver on TEE bus 2020-05-25 11:52 [PATCHv3 0/3] optee: register drivers on optee bus Maxim Uvarov 2020-05-25 11:52 ` [PATCHv3 1/3] optee: do drivers initialization before and after tee-supplicant run Maxim Uvarov 2020-05-25 11:52 ` [PATCHv3 2/3] optee: use uuid for sysfs driver entry Maxim Uvarov @ 2020-05-25 11:52 ` Maxim Uvarov 2 siblings, 0 replies; 12+ messages in thread From: Maxim Uvarov @ 2020-05-25 11:52 UTC (permalink / raw) To: linux-kernel, tee-dev Cc: peterhuewe, jarkko.sakkinen, jgg, gregkh, jens.wiklander, linux-integrity, arnd, sumit.garg, Maxim Uvarov Register driver on the TEE bus. The :module tee registers bus, and module optee calls optee_enumerate_devices() to scan all devices on the bus. Trusted Application for this driver can be Early TA's (can be compiled into optee-os). In that case it will be on OPTEE bus before linux booting. Also optee-suplicant application is needed to be loaded between OPTEE module and ftpm module to maintain functionality for fTPM driver. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> Suggested-by: Sumit Garg <sumit.garg@linaro.org> Suggested-by: Arnd Bergmann <arnd@linaro.org> --- drivers/char/tpm/tpm_ftpm_tee.c | 70 ++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c index 22bf553ccf9d..28da638360d8 100644 --- a/drivers/char/tpm/tpm_ftpm_tee.c +++ b/drivers/char/tpm/tpm_ftpm_tee.c @@ -214,11 +214,10 @@ static int ftpm_tee_match(struct tee_ioctl_version_data *ver, const void *data) * Return: * On success, 0. On failure, -errno. */ -static int ftpm_tee_probe(struct platform_device *pdev) +static int ftpm_tee_probe(struct device *dev) { int rc; struct tpm_chip *chip; - struct device *dev = &pdev->dev; struct ftpm_tee_private *pvt_data = NULL; struct tee_ioctl_open_session_arg sess_arg; @@ -297,6 +296,13 @@ static int ftpm_tee_probe(struct platform_device *pdev) return rc; } +static int ftpm_plat_tee_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + + return ftpm_tee_probe(dev); +} + /** * ftpm_tee_remove() - remove the TPM device * @pdev: the platform_device description. @@ -304,9 +310,9 @@ static int ftpm_tee_probe(struct platform_device *pdev) * Return: * 0 always. */ -static int ftpm_tee_remove(struct platform_device *pdev) +static int ftpm_tee_remove(struct device *dev) { - struct ftpm_tee_private *pvt_data = dev_get_drvdata(&pdev->dev); + struct ftpm_tee_private *pvt_data = dev_get_drvdata(dev); /* Release the chip */ tpm_chip_unregister(pvt_data->chip); @@ -328,11 +334,18 @@ static int ftpm_tee_remove(struct platform_device *pdev) return 0; } +static int ftpm_plat_tee_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + + return ftpm_tee_remove(dev); +} + /** * ftpm_tee_shutdown() - shutdown the TPM device * @pdev: the platform_device description. */ -static void ftpm_tee_shutdown(struct platform_device *pdev) +static void ftpm_plat_tee_shutdown(struct platform_device *pdev) { struct ftpm_tee_private *pvt_data = dev_get_drvdata(&pdev->dev); @@ -347,17 +360,54 @@ static const struct of_device_id of_ftpm_tee_ids[] = { }; MODULE_DEVICE_TABLE(of, of_ftpm_tee_ids); -static struct platform_driver ftpm_tee_driver = { +static struct platform_driver ftpm_tee_plat_driver = { .driver = { .name = "ftpm-tee", .of_match_table = of_match_ptr(of_ftpm_tee_ids), }, - .probe = ftpm_tee_probe, - .remove = ftpm_tee_remove, - .shutdown = ftpm_tee_shutdown, + .shutdown = ftpm_plat_tee_shutdown, + .probe = ftpm_plat_tee_probe, + .remove = ftpm_plat_tee_remove, +}; + +/* UUID of the fTPM TA */ +static const struct tee_client_device_id optee_ftpm_id_table[] = { + {UUID_INIT(0xbc50d971, 0xd4c9, 0x42c4, + 0x82, 0xcb, 0x34, 0x3f, 0xb7, 0xf3, 0x78, 0x96)}, + {} }; -module_platform_driver(ftpm_tee_driver); +MODULE_DEVICE_TABLE(tee, optee_ftpm_id_table); + +static struct tee_client_driver ftpm_tee_driver = { + .id_table = optee_ftpm_id_table, + .driver = { + .name = "optee-ftpm", + .bus = &tee_bus_type, + .probe = ftpm_tee_probe, + .remove = ftpm_tee_remove, + }, +}; + +static int __init ftpm_mod_init(void) +{ + int rc; + + rc = platform_driver_register(&ftpm_tee_plat_driver); + if (rc) + return rc; + + return driver_register(&ftpm_tee_driver.driver); +} + +static void __exit ftpm_mod_exit(void) +{ + platform_driver_unregister(&ftpm_tee_plat_driver); + driver_unregister(&ftpm_tee_driver.driver); +} + +module_init(ftpm_mod_init); +module_exit(ftpm_mod_exit); MODULE_AUTHOR("Thirupathaiah Annapureddy <thiruan@microsoft.com>"); MODULE_DESCRIPTION("TPM Driver for fTPM TA in TEE"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-05-28 8:26 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-25 11:52 [PATCHv3 0/3] optee: register drivers on optee bus Maxim Uvarov 2020-05-25 11:52 ` [PATCHv3 1/3] optee: do drivers initialization before and after tee-supplicant run Maxim Uvarov 2020-05-27 19:55 ` Jarkko Sakkinen 2020-05-28 8:26 ` Maxim Uvarov 2020-05-25 11:52 ` [PATCHv3 2/3] optee: use uuid for sysfs driver entry Maxim Uvarov 2020-05-25 12:10 ` [Tee-dev] " Jerome Forissier 2020-05-25 13:36 ` Maxim Uvarov 2020-05-25 15:53 ` Jerome Forissier 2020-05-26 10:37 ` Sumit Garg 2020-05-25 12:47 ` Greg KH 2020-05-25 13:33 ` Maxim Uvarov 2020-05-25 11:52 ` [PATCHv3 3/3] tpm_ftpm_tee: register driver on TEE bus Maxim Uvarov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.