diff mbox series

[PATCHv5,1/3] optee: do drivers initialization before and after tee-supplicant run

Message ID 20200529082701.13457-2-maxim.uvarov@linaro.org (mailing list archive)
State Rejected
Headers show
Series optee: register drivers on optee bus | expand

Commit Message

Maxim Uvarov May 29, 2020, 8:26 a.m. UTC 
Some drivers (like ftpm) can operate only after tee-supplicant
runs because of tee-supplicant provides things like storage
services.  This patch splits probe of non tee-supplicant dependable
drivers to the 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          | 24 +++++++++++++++++++++---
 drivers/tee/optee/device.c        | 17 +++++++++++------
 drivers/tee/optee/optee_private.h | 10 +++++++++-
 3 files changed, 41 insertions(+), 10 deletions(-)

Comments

Jarkko Sakkinen June 1, 2020, 1:49 a.m. UTC | #1 
On Fri, May 29, 2020 at 11:26:59AM +0300, Maxim Uvarov wrote:
> Some drivers (like ftpm) can operate only after tee-supplicant
> runs because of tee-supplicant provides things like storage
> services.  This patch splits probe of non tee-supplicant dependable
> drivers to the 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>

$ scripts/get_maintainer.pl drivers/tee 
Jens Wiklander <jens.wiklander@linaro.org> (maintainer:TEE SUBSYSTEM)
tee-dev@lists.linaro.org (open list:TEE SUBSYSTEM)
linux-kernel@vger.kernel.org (open list)

tee-dev is missing. Please resend.

/Jarkko
Maxim Uvarov June 1, 2020, 7:13 a.m. UTC | #2 
On Mon, 1 Jun 2020 at 04:49, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Fri, May 29, 2020 at 11:26:59AM +0300, Maxim Uvarov wrote:
> > Some drivers (like ftpm) can operate only after tee-supplicant
> > runs because of tee-supplicant provides things like storage
> > services.  This patch splits probe of non tee-supplicant dependable
> > drivers to the 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>
>
> $ scripts/get_maintainer.pl drivers/tee
> Jens Wiklander <jens.wiklander@linaro.org> (maintainer:TEE SUBSYSTEM)
> tee-dev@lists.linaro.org (open list:TEE SUBSYSTEM)
> linux-kernel@vger.kernel.org (open list)
>
> tee-dev is missing. Please resend.
>
> /Jarkko

tee-dev@ is in CC.
Jarkko Sakkinen June 1, 2020, 7:59 a.m. UTC | #3 
On Mon, Jun 01, 2020 at 10:13:40AM +0300, Maxim Uvarov wrote:
> On Mon, 1 Jun 2020 at 04:49, Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Fri, May 29, 2020 at 11:26:59AM +0300, Maxim Uvarov wrote:
> > > Some drivers (like ftpm) can operate only after tee-supplicant
> > > runs because of tee-supplicant provides things like storage
> > > services.  This patch splits probe of non tee-supplicant dependable
> > > drivers to the 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>
> >
> > $ scripts/get_maintainer.pl drivers/tee
> > Jens Wiklander <jens.wiklander@linaro.org> (maintainer:TEE SUBSYSTEM)
> > tee-dev@lists.linaro.org (open list:TEE SUBSYSTEM)
> > linux-kernel@vger.kernel.org (open list)
> >
> > tee-dev is missing. Please resend.
> >
> > /Jarkko
> 
> tee-dev@ is in CC.

Oops. I'm sorry, missed it.

/Jarkko
Sumit Garg June 1, 2020, 10:45 a.m. UTC | #4 
On Fri, 29 May 2020 at 13:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> Some drivers (like ftpm) can operate only after tee-supplicant
> runs because of tee-supplicant provides things like storage
> services.  This patch splits probe of non tee-supplicant dependable
> drivers to the 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          | 24 +++++++++++++++++++++---
>  drivers/tee/optee/device.c        | 17 +++++++++++------
>  drivers/tee/optee/optee_private.h | 10 +++++++++-
>  3 files changed, 41 insertions(+), 10 deletions(-)
>

Commit subject sounds a little vague, so how about:

====
optee: enable support for multi-stage bus enumeration
====

Then in the commit description, you can elaborate on what it actually means.

> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 99698b8a3a74..bf0851fdf108 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,11 @@ static void optee_get_version(struct tee_device *teedev,
>         *vers = v;
>  }
>
> +static void optee_bus_scan(struct work_struct *work)
> +{
> +       WARN_ON(optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP));
> +}
> +
>  static int optee_open(struct tee_context *ctx)
>  {
>         struct optee_context_data *ctxdata;
> @@ -241,8 +247,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) {
> +                               kfree(ctxdata);
> +                               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 +312,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);

Doesn't it deserve a prior check "if(optee->scan_bus_wq)" as we only
allocate it once during multiple tee-supplicant instances?

>                 optee_supp_release(&optee->supp);
> +       }
>  }
>
>  static const struct tee_driver_ops optee_ops = {
> @@ -675,7 +693,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
>   */

This comment needs to be moved as well.

> -#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(&param, 0, sizeof(param));
>
>         /* Invoke PTA_CMD_GET_DEVICES function */

You can get rid of this comment.

-Sumit

> -       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
>
diff mbox series

Patch

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 99698b8a3a74..bf0851fdf108 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,11 @@  static void optee_get_version(struct tee_device *teedev,
 	*vers = v;
 }
 
+static void optee_bus_scan(struct work_struct *work)
+{
+	WARN_ON(optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP));
+}
+
 static int optee_open(struct tee_context *ctx)
 {
 	struct optee_context_data *ctxdata;
@@ -241,8 +247,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) {
+				kfree(ctxdata);
+				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 +312,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 +693,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(&param, 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