[PATCH v2 0/3] TEE driver for Trusted Services

[PATCH v2 0/3] TEE driver for Trusted Services

[Balint Dobszay]

This series introduces a TEE driver for Trusted Services [1].

Trusted Services is a TrustedFirmware.org project that provides a
framework for developing and deploying device Root of Trust services in
FF-A [2] Secure Partitions. The project hosts the reference
implementation of Arm Platform Security Architecture [3] for Arm
A-profile devices.

The FF-A Secure Partitions are accessible through the FF-A driver in
Linux. However, the FF-A driver doesn't have a user space interface so
user space clients currently cannot access Trusted Services. The goal of
this TEE driver is to bridge this gap and make Trusted Services
functionality accessible from user space.

Changelog:
v1[5] -> v2:
  - Refactor session handling to use XArray instead of IDR and linked
    list (the linked list was redundant as pointed out by Jens, and IDR
    is now deprecated in favor of XArray)
  - Refactor tstee_probe() to not call tee_device_unregister() before
    calling tee_device_register()
  - Address comments from Krzysztof and Jens
  - Address documentation comments from Randy
  - Use module_ffa_driver() macro instead of separate module init / exit
    functions
  - Reformat max line length 100 -> 80

RFC[4] -> v1:
  - Add patch for moving pool_op helper functions to the TEE subsystem,
    as suggested by Jens
  - Address comments from Sumit, add patch for documentation

[1] https://www.trustedfirmware.org/projects/trusted-services/
[2] https://developer.arm.com/documentation/den0077/
[3] https://www.arm.com/architecture/security-features/platform-security
[4] https://lore.kernel.org/linux-arm-kernel/20230927152145.111777-1-balint.dobszay@arm.com/
[5] https://lore.kernel.org/lkml/20240213145239.379875-1-balint.dobszay@arm.com/


Balint Dobszay (3):
  tee: optee: Move pool_op helper functions
  tee: tstee: Add Trusted Services TEE driver
  Documentation: tee: Add TS-TEE driver

 Documentation/tee/index.rst       |   1 +
 Documentation/tee/ts-tee.rst      |  71 +++++
 drivers/tee/Kconfig               |   1 +
 drivers/tee/Makefile              |   1 +
 drivers/tee/optee/core.c          |  64 ----
 drivers/tee/optee/ffa_abi.c       |   6 +-
 drivers/tee/optee/optee_private.h |  12 -
 drivers/tee/optee/smc_abi.c       |  11 +-
 drivers/tee/tee_shm.c             |  65 ++++
 drivers/tee/tstee/Kconfig         |  11 +
 drivers/tee/tstee/Makefile        |   3 +
 drivers/tee/tstee/core.c          | 490 ++++++++++++++++++++++++++++++
 drivers/tee/tstee/tstee_private.h |  94 ++++++
 include/linux/tee_drv.h           |  11 +
 include/uapi/linux/tee.h          |   1 +
 15 files changed, 758 insertions(+), 84 deletions(-)
 create mode 100644 Documentation/tee/ts-tee.rst
 create mode 100644 drivers/tee/tstee/Kconfig
 create mode 100644 drivers/tee/tstee/Makefile
 create mode 100644 drivers/tee/tstee/core.c
 create mode 100644 drivers/tee/tstee/tstee_private.h

-- 
2.34.1

[PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Balint Dobszay]

Move the pool alloc and free helper functions from the OP-TEE driver to
the TEE subsystem, since these could be reused in other TEE drivers.
This patch is not supposed to change behavior, it's only reorganizing
the code.

Suggested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
---
 drivers/tee/optee/core.c          | 64 ------------------------------
 drivers/tee/optee/ffa_abi.c       |  6 +--
 drivers/tee/optee/optee_private.h | 12 ------
 drivers/tee/optee/smc_abi.c       | 11 +++---
 drivers/tee/tee_shm.c             | 65 +++++++++++++++++++++++++++++++
 include/linux/tee_drv.h           | 11 ++++++
 6 files changed, 85 insertions(+), 84 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 3aed554bc8d8..9390f21f9902 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -9,7 +9,6 @@
 #include <linux/crash_dump.h>
 #include <linux/errno.h>
 #include <linux/io.h>
-#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/string.h>
@@ -17,69 +16,6 @@
 #include <linux/types.h>
 #include "optee_private.h"
 
-int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
-			       size_t size, size_t align,
-			       int (*shm_register)(struct tee_context *ctx,
-						   struct tee_shm *shm,
-						   struct page **pages,
-						   size_t num_pages,
-						   unsigned long start))
-{
-	size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
-	struct page **pages;
-	unsigned int i;
-	int rc = 0;
-
-	/*
-	 * Ignore alignment since this is already going to be page aligned
-	 * and there's no need for any larger alignment.
-	 */
-	shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
-				       GFP_KERNEL | __GFP_ZERO);
-	if (!shm->kaddr)
-		return -ENOMEM;
-
-	shm->paddr = virt_to_phys(shm->kaddr);
-	shm->size = nr_pages * PAGE_SIZE;
-
-	pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
-	if (!pages) {
-		rc = -ENOMEM;
-		goto err;
-	}
-
-	for (i = 0; i < nr_pages; i++)
-		pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
-
-	shm->pages = pages;
-	shm->num_pages = nr_pages;
-
-	if (shm_register) {
-		rc = shm_register(shm->ctx, shm, pages, nr_pages,
-				  (unsigned long)shm->kaddr);
-		if (rc)
-			goto err;
-	}
-
-	return 0;
-err:
-	free_pages_exact(shm->kaddr, shm->size);
-	shm->kaddr = NULL;
-	return rc;
-}
-
-void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
-			       int (*shm_unregister)(struct tee_context *ctx,
-						     struct tee_shm *shm))
-{
-	if (shm_unregister)
-		shm_unregister(shm->ctx, shm);
-	free_pages_exact(shm->kaddr, shm->size);
-	shm->kaddr = NULL;
-	kfree(shm->pages);
-	shm->pages = NULL;
-}
-
 static void optee_bus_scan(struct work_struct *work)
 {
 	WARN_ON(optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP));
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
index ecb5eb079408..ee11918a2b35 100644
--- a/drivers/tee/optee/ffa_abi.c
+++ b/drivers/tee/optee/ffa_abi.c
@@ -374,14 +374,14 @@ static int optee_ffa_shm_unregister_supp(struct tee_context *ctx,
 static int pool_ffa_op_alloc(struct tee_shm_pool *pool,
 			     struct tee_shm *shm, size_t size, size_t align)
 {
-	return optee_pool_op_alloc_helper(pool, shm, size, align,
-					  optee_ffa_shm_register);
+	return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
+					    optee_ffa_shm_register);
 }
 
 static void pool_ffa_op_free(struct tee_shm_pool *pool,
 			     struct tee_shm *shm)
 {
-	optee_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
+	tee_shm_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
 }
 
 static void pool_ffa_op_destroy_pool(struct tee_shm_pool *pool)
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index 7a5243c78b55..a153285a1919 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -283,18 +283,6 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session);
 int optee_enumerate_devices(u32 func);
 void optee_unregister_devices(void);
 
-int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
-			       size_t size, size_t align,
-			       int (*shm_register)(struct tee_context *ctx,
-						   struct tee_shm *shm,
-						   struct page **pages,
-						   size_t num_pages,
-						   unsigned long start));
-void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
-			       int (*shm_unregister)(struct tee_context *ctx,
-						     struct tee_shm *shm));
-
-
 void optee_remove_common(struct optee *optee);
 int optee_open(struct tee_context *ctx, bool cap_memref_null);
 void optee_release(struct tee_context *ctx);
diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
index a37f87087e5c..b0c616b6870d 100644
--- a/drivers/tee/optee/smc_abi.c
+++ b/drivers/tee/optee/smc_abi.c
@@ -592,19 +592,20 @@ static int pool_op_alloc(struct tee_shm_pool *pool,
 	 * to be registered with OP-TEE.
 	 */
 	if (shm->flags & TEE_SHM_PRIV)
-		return optee_pool_op_alloc_helper(pool, shm, size, align, NULL);
+		return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
+						    NULL);
 
-	return optee_pool_op_alloc_helper(pool, shm, size, align,
-					  optee_shm_register);
+	return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
+					    optee_shm_register);
 }
 
 static void pool_op_free(struct tee_shm_pool *pool,
 			 struct tee_shm *shm)
 {
 	if (!(shm->flags & TEE_SHM_PRIV))
-		optee_pool_op_free_helper(pool, shm, optee_shm_unregister);
+		tee_shm_pool_op_free_helper(pool, shm, optee_shm_unregister);
 	else
-		optee_pool_op_free_helper(pool, shm, NULL);
+		tee_shm_pool_op_free_helper(pool, shm, NULL);
 }
 
 static void pool_op_destroy_pool(struct tee_shm_pool *pool)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 731d9028b67f..641aad92ffe2 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -202,6 +202,71 @@ struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size)
 }
 EXPORT_SYMBOL_GPL(tee_shm_alloc_priv_buf);
 
+int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
+				 size_t size, size_t align,
+				 int (*shm_register)(struct tee_context *ctx,
+						     struct tee_shm *shm,
+						     struct page **pages,
+						     size_t num_pages,
+						     unsigned long start))
+{
+	size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
+	struct page **pages;
+	unsigned int i;
+	int rc = 0;
+
+	/*
+	 * Ignore alignment since this is already going to be page aligned
+	 * and there's no need for any larger alignment.
+	 */
+	shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
+				       GFP_KERNEL | __GFP_ZERO);
+	if (!shm->kaddr)
+		return -ENOMEM;
+
+	shm->paddr = virt_to_phys(shm->kaddr);
+	shm->size = nr_pages * PAGE_SIZE;
+
+	pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
+	if (!pages) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	for (i = 0; i < nr_pages; i++)
+		pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
+
+	shm->pages = pages;
+	shm->num_pages = nr_pages;
+
+	if (shm_register) {
+		rc = shm_register(shm->ctx, shm, pages, nr_pages,
+				  (unsigned long)shm->kaddr);
+		if (rc)
+			goto err;
+	}
+
+	return 0;
+err:
+	free_pages_exact(shm->kaddr, shm->size);
+	shm->kaddr = NULL;
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tee_shm_pool_op_alloc_helper);
+
+void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
+				 int (*shm_unregister)(struct tee_context *ctx,
+						       struct tee_shm *shm))
+{
+	if (shm_unregister)
+		shm_unregister(shm->ctx, shm);
+	free_pages_exact(shm->kaddr, shm->size);
+	shm->kaddr = NULL;
+	kfree(shm->pages);
+	shm->pages = NULL;
+}
+EXPORT_SYMBOL_GPL(tee_shm_pool_op_free_helper);
+
 static struct tee_shm *
 register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
 		    int id)
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index 911ddf92dcee..4cf402424e71 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
 struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
 struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
 
+int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
+				 size_t size, size_t align,
+				 int (*shm_register)(struct tee_context *ctx,
+						     struct tee_shm *shm,
+						     struct page **pages,
+						     size_t num_pages,
+						     unsigned long start));
+void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
+				 int (*shm_unregister)(struct tee_context *ctx,
+						       struct tee_shm *shm));
+
 struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx,
 					    void *addr, size_t length);
 
-- 
2.34.1

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Sumit Garg]

Hi Balint,

On Fri, 23 Feb 2024 at 15:22, Balint Dobszay <balint.dobszay@arm.com> wrote:
>
> Move the pool alloc and free helper functions from the OP-TEE driver to
> the TEE subsystem, since these could be reused in other TEE drivers.
> This patch is not supposed to change behavior, it's only reorganizing
> the code.
>
> Suggested-by: Jens Wiklander <jens.wiklander@linaro.org>
> Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
> ---
>  drivers/tee/optee/core.c          | 64 ------------------------------
>  drivers/tee/optee/ffa_abi.c       |  6 +--
>  drivers/tee/optee/optee_private.h | 12 ------
>  drivers/tee/optee/smc_abi.c       | 11 +++---
>  drivers/tee/tee_shm.c             | 65 +++++++++++++++++++++++++++++++
>  include/linux/tee_drv.h           | 11 ++++++
>  6 files changed, 85 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 3aed554bc8d8..9390f21f9902 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -9,7 +9,6 @@
>  #include <linux/crash_dump.h>
>  #include <linux/errno.h>
>  #include <linux/io.h>
> -#include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/string.h>
> @@ -17,69 +16,6 @@
>  #include <linux/types.h>
>  #include "optee_private.h"
>
> -int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> -                              size_t size, size_t align,
> -                              int (*shm_register)(struct tee_context *ctx,
> -                                                  struct tee_shm *shm,
> -                                                  struct page **pages,
> -                                                  size_t num_pages,
> -                                                  unsigned long start))
> -{
> -       size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
> -       struct page **pages;
> -       unsigned int i;
> -       int rc = 0;
> -
> -       /*
> -        * Ignore alignment since this is already going to be page aligned
> -        * and there's no need for any larger alignment.
> -        */
> -       shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
> -                                      GFP_KERNEL | __GFP_ZERO);
> -       if (!shm->kaddr)
> -               return -ENOMEM;
> -
> -       shm->paddr = virt_to_phys(shm->kaddr);
> -       shm->size = nr_pages * PAGE_SIZE;
> -
> -       pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
> -       if (!pages) {
> -               rc = -ENOMEM;
> -               goto err;
> -       }
> -
> -       for (i = 0; i < nr_pages; i++)
> -               pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
> -
> -       shm->pages = pages;
> -       shm->num_pages = nr_pages;
> -
> -       if (shm_register) {
> -               rc = shm_register(shm->ctx, shm, pages, nr_pages,
> -                                 (unsigned long)shm->kaddr);
> -               if (rc)
> -                       goto err;
> -       }
> -
> -       return 0;
> -err:
> -       free_pages_exact(shm->kaddr, shm->size);
> -       shm->kaddr = NULL;
> -       return rc;
> -}
> -
> -void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> -                              int (*shm_unregister)(struct tee_context *ctx,
> -                                                    struct tee_shm *shm))
> -{
> -       if (shm_unregister)
> -               shm_unregister(shm->ctx, shm);
> -       free_pages_exact(shm->kaddr, shm->size);
> -       shm->kaddr = NULL;
> -       kfree(shm->pages);
> -       shm->pages = NULL;
> -}
> -
>  static void optee_bus_scan(struct work_struct *work)
>  {
>         WARN_ON(optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP));
> diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> index ecb5eb079408..ee11918a2b35 100644
> --- a/drivers/tee/optee/ffa_abi.c
> +++ b/drivers/tee/optee/ffa_abi.c
> @@ -374,14 +374,14 @@ static int optee_ffa_shm_unregister_supp(struct tee_context *ctx,
>  static int pool_ffa_op_alloc(struct tee_shm_pool *pool,
>                              struct tee_shm *shm, size_t size, size_t align)
>  {
> -       return optee_pool_op_alloc_helper(pool, shm, size, align,
> -                                         optee_ffa_shm_register);
> +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> +                                           optee_ffa_shm_register);
>  }
>
>  static void pool_ffa_op_free(struct tee_shm_pool *pool,
>                              struct tee_shm *shm)
>  {
> -       optee_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
> +       tee_shm_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
>  }
>
>  static void pool_ffa_op_destroy_pool(struct tee_shm_pool *pool)
> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> index 7a5243c78b55..a153285a1919 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -283,18 +283,6 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session);
>  int optee_enumerate_devices(u32 func);
>  void optee_unregister_devices(void);
>
> -int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> -                              size_t size, size_t align,
> -                              int (*shm_register)(struct tee_context *ctx,
> -                                                  struct tee_shm *shm,
> -                                                  struct page **pages,
> -                                                  size_t num_pages,
> -                                                  unsigned long start));
> -void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> -                              int (*shm_unregister)(struct tee_context *ctx,
> -                                                    struct tee_shm *shm));
> -
> -
>  void optee_remove_common(struct optee *optee);
>  int optee_open(struct tee_context *ctx, bool cap_memref_null);
>  void optee_release(struct tee_context *ctx);
> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> index a37f87087e5c..b0c616b6870d 100644
> --- a/drivers/tee/optee/smc_abi.c
> +++ b/drivers/tee/optee/smc_abi.c
> @@ -592,19 +592,20 @@ static int pool_op_alloc(struct tee_shm_pool *pool,
>          * to be registered with OP-TEE.
>          */
>         if (shm->flags & TEE_SHM_PRIV)
> -               return optee_pool_op_alloc_helper(pool, shm, size, align, NULL);
> +               return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> +                                                   NULL);
>
> -       return optee_pool_op_alloc_helper(pool, shm, size, align,
> -                                         optee_shm_register);
> +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> +                                           optee_shm_register);
>  }
>
>  static void pool_op_free(struct tee_shm_pool *pool,
>                          struct tee_shm *shm)
>  {
>         if (!(shm->flags & TEE_SHM_PRIV))
> -               optee_pool_op_free_helper(pool, shm, optee_shm_unregister);
> +               tee_shm_pool_op_free_helper(pool, shm, optee_shm_unregister);
>         else
> -               optee_pool_op_free_helper(pool, shm, NULL);
> +               tee_shm_pool_op_free_helper(pool, shm, NULL);
>  }
>
>  static void pool_op_destroy_pool(struct tee_shm_pool *pool)
> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> index 731d9028b67f..641aad92ffe2 100644
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -202,6 +202,71 @@ struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size)
>  }
>  EXPORT_SYMBOL_GPL(tee_shm_alloc_priv_buf);
>
> +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,

I don't see the first argument (struct tee_shm_pool *pool) being used,
so drop that. Also, we can just rename it as
tee_dyn_shm_alloc_helper().

> +                                size_t size, size_t align,
> +                                int (*shm_register)(struct tee_context *ctx,
> +                                                    struct tee_shm *shm,
> +                                                    struct page **pages,
> +                                                    size_t num_pages,
> +                                                    unsigned long start))
> +{
> +       size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
> +       struct page **pages;
> +       unsigned int i;
> +       int rc = 0;
> +
> +       /*
> +        * Ignore alignment since this is already going to be page aligned
> +        * and there's no need for any larger alignment.
> +        */
> +       shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
> +                                      GFP_KERNEL | __GFP_ZERO);
> +       if (!shm->kaddr)
> +               return -ENOMEM;
> +
> +       shm->paddr = virt_to_phys(shm->kaddr);
> +       shm->size = nr_pages * PAGE_SIZE;
> +
> +       pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
> +       if (!pages) {
> +               rc = -ENOMEM;
> +               goto err;
> +       }
> +
> +       for (i = 0; i < nr_pages; i++)
> +               pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
> +
> +       shm->pages = pages;
> +       shm->num_pages = nr_pages;
> +
> +       if (shm_register) {
> +               rc = shm_register(shm->ctx, shm, pages, nr_pages,
> +                                 (unsigned long)shm->kaddr);
> +               if (rc)
> +                       goto err;
> +       }
> +
> +       return 0;
> +err:
> +       free_pages_exact(shm->kaddr, shm->size);
> +       shm->kaddr = NULL;
> +       return rc;
> +}
> +EXPORT_SYMBOL_GPL(tee_shm_pool_op_alloc_helper);
> +
> +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,

Ditto tee_shm_pool_op_free_helper() -> tee_dyn_shm_free_helper()

> +                                int (*shm_unregister)(struct tee_context *ctx,
> +                                                      struct tee_shm *shm))
> +{
> +       if (shm_unregister)
> +               shm_unregister(shm->ctx, shm);
> +       free_pages_exact(shm->kaddr, shm->size);
> +       shm->kaddr = NULL;
> +       kfree(shm->pages);
> +       shm->pages = NULL;
> +}
> +EXPORT_SYMBOL_GPL(tee_shm_pool_op_free_helper);
> +
>  static struct tee_shm *
>  register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
>                     int id)
> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> index 911ddf92dcee..4cf402424e71 100644
> --- a/include/linux/tee_drv.h
> +++ b/include/linux/tee_drv.h
> @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
>  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
>  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
>
> +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> +                                size_t size, size_t align,
> +                                int (*shm_register)(struct tee_context *ctx,
> +                                                    struct tee_shm *shm,
> +                                                    struct page **pages,
> +                                                    size_t num_pages,
> +                                                    unsigned long start));
> +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> +                                int (*shm_unregister)(struct tee_context *ctx,
> +                                                      struct tee_shm *shm));
> +

These rather belong to drivers/tee/tee_private.h as we shouldn't
expose them to other kernel client drivers.

-Sumit

>  struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx,
>                                             void *addr, size_t length);
>
> --
> 2.34.1
>

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Jens Wiklander]

On Tue, Feb 27, 2024 at 7:06 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Hi Balint,
>
> On Fri, 23 Feb 2024 at 15:22, Balint Dobszay <balint.dobszay@arm.com> wrote:
> >
> > Move the pool alloc and free helper functions from the OP-TEE driver to
> > the TEE subsystem, since these could be reused in other TEE drivers.
> > This patch is not supposed to change behavior, it's only reorganizing
> > the code.
> >
> > Suggested-by: Jens Wiklander <jens.wiklander@linaro.org>
> > Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
> > ---
> >  drivers/tee/optee/core.c          | 64 ------------------------------
> >  drivers/tee/optee/ffa_abi.c       |  6 +--
> >  drivers/tee/optee/optee_private.h | 12 ------
> >  drivers/tee/optee/smc_abi.c       | 11 +++---
> >  drivers/tee/tee_shm.c             | 65 +++++++++++++++++++++++++++++++
> >  include/linux/tee_drv.h           | 11 ++++++
> >  6 files changed, 85 insertions(+), 84 deletions(-)
> >
> > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> > index 3aed554bc8d8..9390f21f9902 100644
> > --- a/drivers/tee/optee/core.c
> > +++ b/drivers/tee/optee/core.c
> > @@ -9,7 +9,6 @@
> >  #include <linux/crash_dump.h>
> >  #include <linux/errno.h>
> >  #include <linux/io.h>
> > -#include <linux/mm.h>
> >  #include <linux/module.h>
> >  #include <linux/slab.h>
> >  #include <linux/string.h>
> > @@ -17,69 +16,6 @@
> >  #include <linux/types.h>
> >  #include "optee_private.h"
> >
> > -int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > -                              size_t size, size_t align,
> > -                              int (*shm_register)(struct tee_context *ctx,
> > -                                                  struct tee_shm *shm,
> > -                                                  struct page **pages,
> > -                                                  size_t num_pages,
> > -                                                  unsigned long start))
> > -{
> > -       size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
> > -       struct page **pages;
> > -       unsigned int i;
> > -       int rc = 0;
> > -
> > -       /*
> > -        * Ignore alignment since this is already going to be page aligned
> > -        * and there's no need for any larger alignment.
> > -        */
> > -       shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
> > -                                      GFP_KERNEL | __GFP_ZERO);
> > -       if (!shm->kaddr)
> > -               return -ENOMEM;
> > -
> > -       shm->paddr = virt_to_phys(shm->kaddr);
> > -       shm->size = nr_pages * PAGE_SIZE;
> > -
> > -       pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
> > -       if (!pages) {
> > -               rc = -ENOMEM;
> > -               goto err;
> > -       }
> > -
> > -       for (i = 0; i < nr_pages; i++)
> > -               pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
> > -
> > -       shm->pages = pages;
> > -       shm->num_pages = nr_pages;
> > -
> > -       if (shm_register) {
> > -               rc = shm_register(shm->ctx, shm, pages, nr_pages,
> > -                                 (unsigned long)shm->kaddr);
> > -               if (rc)
> > -                       goto err;
> > -       }
> > -
> > -       return 0;
> > -err:
> > -       free_pages_exact(shm->kaddr, shm->size);
> > -       shm->kaddr = NULL;
> > -       return rc;
> > -}
> > -
> > -void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > -                              int (*shm_unregister)(struct tee_context *ctx,
> > -                                                    struct tee_shm *shm))
> > -{
> > -       if (shm_unregister)
> > -               shm_unregister(shm->ctx, shm);
> > -       free_pages_exact(shm->kaddr, shm->size);
> > -       shm->kaddr = NULL;
> > -       kfree(shm->pages);
> > -       shm->pages = NULL;
> > -}
> > -
> >  static void optee_bus_scan(struct work_struct *work)
> >  {
> >         WARN_ON(optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP));
> > diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> > index ecb5eb079408..ee11918a2b35 100644
> > --- a/drivers/tee/optee/ffa_abi.c
> > +++ b/drivers/tee/optee/ffa_abi.c
> > @@ -374,14 +374,14 @@ static int optee_ffa_shm_unregister_supp(struct tee_context *ctx,
> >  static int pool_ffa_op_alloc(struct tee_shm_pool *pool,
> >                              struct tee_shm *shm, size_t size, size_t align)
> >  {
> > -       return optee_pool_op_alloc_helper(pool, shm, size, align,
> > -                                         optee_ffa_shm_register);
> > +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> > +                                           optee_ffa_shm_register);
> >  }
> >
> >  static void pool_ffa_op_free(struct tee_shm_pool *pool,
> >                              struct tee_shm *shm)
> >  {
> > -       optee_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
> > +       tee_shm_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
> >  }
> >
> >  static void pool_ffa_op_destroy_pool(struct tee_shm_pool *pool)
> > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> > index 7a5243c78b55..a153285a1919 100644
> > --- a/drivers/tee/optee/optee_private.h
> > +++ b/drivers/tee/optee/optee_private.h
> > @@ -283,18 +283,6 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session);
> >  int optee_enumerate_devices(u32 func);
> >  void optee_unregister_devices(void);
> >
> > -int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > -                              size_t size, size_t align,
> > -                              int (*shm_register)(struct tee_context *ctx,
> > -                                                  struct tee_shm *shm,
> > -                                                  struct page **pages,
> > -                                                  size_t num_pages,
> > -                                                  unsigned long start));
> > -void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > -                              int (*shm_unregister)(struct tee_context *ctx,
> > -                                                    struct tee_shm *shm));
> > -
> > -
> >  void optee_remove_common(struct optee *optee);
> >  int optee_open(struct tee_context *ctx, bool cap_memref_null);
> >  void optee_release(struct tee_context *ctx);
> > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> > index a37f87087e5c..b0c616b6870d 100644
> > --- a/drivers/tee/optee/smc_abi.c
> > +++ b/drivers/tee/optee/smc_abi.c
> > @@ -592,19 +592,20 @@ static int pool_op_alloc(struct tee_shm_pool *pool,
> >          * to be registered with OP-TEE.
> >          */
> >         if (shm->flags & TEE_SHM_PRIV)
> > -               return optee_pool_op_alloc_helper(pool, shm, size, align, NULL);
> > +               return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> > +                                                   NULL);
> >
> > -       return optee_pool_op_alloc_helper(pool, shm, size, align,
> > -                                         optee_shm_register);
> > +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> > +                                           optee_shm_register);
> >  }
> >
> >  static void pool_op_free(struct tee_shm_pool *pool,
> >                          struct tee_shm *shm)
> >  {
> >         if (!(shm->flags & TEE_SHM_PRIV))
> > -               optee_pool_op_free_helper(pool, shm, optee_shm_unregister);
> > +               tee_shm_pool_op_free_helper(pool, shm, optee_shm_unregister);
> >         else
> > -               optee_pool_op_free_helper(pool, shm, NULL);
> > +               tee_shm_pool_op_free_helper(pool, shm, NULL);
> >  }
> >T
> >  static void pool_op_destroy_pool(struct tee_shm_pool *pool)
> > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> > index 731d9028b67f..641aad92ffe2 100644
> > --- a/drivers/tee/tee_shm.c
> > +++ b/drivers/tee/tee_shm.c
> > @@ -202,6 +202,71 @@ struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size)
> >  }
> >  EXPORT_SYMBOL_GPL(tee_shm_alloc_priv_buf);
> >
> > +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>
> I don't see the first argument (struct tee_shm_pool *pool) being used,
> so drop that. Also, we can just rename it as
> tee_dyn_shm_alloc_helper().
>
> > +                                size_t size, size_t align,
> > +                                int (*shm_register)(struct tee_context *ctx,
> > +                                                    struct tee_shm *shm,
> > +                                                    struct page **pages,
> > +                                                    size_t num_pages,
> > +                                                    unsigned long start))
> > +{
> > +       size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
> > +       struct page **pages;
> > +       unsigned int i;
> > +       int rc = 0;
> > +
> > +       /*
> > +        * Ignore alignment since this is already going to be page aligned
> > +        * and there's no need for any larger alignment.
> > +        */
> > +       shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
> > +                                      GFP_KERNEL | __GFP_ZERO);
> > +       if (!shm->kaddr)
> > +               return -ENOMEM;
> > +
> > +       shm->paddr = virt_to_phys(shm->kaddr);
> > +       shm->size = nr_pages * PAGE_SIZE;
> > +
> > +       pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
> > +       if (!pages) {
> > +               rc = -ENOMEM;
> > +               goto err;
> > +       }
> > +
> > +       for (i = 0; i < nr_pages; i++)
> > +               pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
> > +
> > +       shm->pages = pages;
> > +       shm->num_pages = nr_pages;
> > +
> > +       if (shm_register) {
> > +               rc = shm_register(shm->ctx, shm, pages, nr_pages,
> > +                                 (unsigned long)shm->kaddr);
> > +               if (rc)
> > +                       goto err;
> > +       }
> > +
> > +       return 0;
> > +err:
> > +       free_pages_exact(shm->kaddr, shm->size);
> > +       shm->kaddr = NULL;
> > +       return rc;
> > +}
> > +EXPORT_SYMBOL_GPL(tee_shm_pool_op_alloc_helper);
> > +
> > +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>
> Ditto tee_shm_pool_op_free_helper() -> tee_dyn_shm_free_helper()
>
> > +                                int (*shm_unregister)(struct tee_context *ctx,
> > +                                                      struct tee_shm *shm))
> > +{
> > +       if (shm_unregister)
> > +               shm_unregister(shm->ctx, shm);
> > +       free_pages_exact(shm->kaddr, shm->size);
> > +       shm->kaddr = NULL;
> > +       kfree(shm->pages);
> > +       shm->pages = NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(tee_shm_pool_op_free_helper);
> > +
> >  static struct tee_shm *
> >  register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
> >                     int id)
> > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> > index 911ddf92dcee..4cf402424e71 100644
> > --- a/include/linux/tee_drv.h
> > +++ b/include/linux/tee_drv.h
> > @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
> >  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
> >  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
> >
> > +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > +                                size_t size, size_t align,
> > +                                int (*shm_register)(struct tee_context *ctx,
> > +                                                    struct tee_shm *shm,
> > +                                                    struct page **pages,
> > +                                                    size_t num_pages,
> > +                                                    unsigned long start));
> > +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > +                                int (*shm_unregister)(struct tee_context *ctx,
> > +                                                      struct tee_shm *shm));
> > +
>
> These rather belong to drivers/tee/tee_private.h as we shouldn't
> expose them to other kernel client drivers.

This is the right place, this .h file is for TEE drivers too.

Cheers,
Jens

>
> -Sumit
>
> >  struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx,
> >                                             void *addr, size_t length);
> >
> > --
> > 2.34.1
> >

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Sumit Garg]

On Tue, 27 Feb 2024 at 21:20, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> On Tue, Feb 27, 2024 at 7:06 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > Hi Balint,
> >
> > On Fri, 23 Feb 2024 at 15:22, Balint Dobszay <balint.dobszay@arm.com> wrote:
> > >
> > > Move the pool alloc and free helper functions from the OP-TEE driver to
> > > the TEE subsystem, since these could be reused in other TEE drivers.
> > > This patch is not supposed to change behavior, it's only reorganizing
> > > the code.
> > >
> > > Suggested-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
> > > ---
> > >  drivers/tee/optee/core.c          | 64 ------------------------------
> > >  drivers/tee/optee/ffa_abi.c       |  6 +--
> > >  drivers/tee/optee/optee_private.h | 12 ------
> > >  drivers/tee/optee/smc_abi.c       | 11 +++---
> > >  drivers/tee/tee_shm.c             | 65 +++++++++++++++++++++++++++++++
> > >  include/linux/tee_drv.h           | 11 ++++++
> > >  6 files changed, 85 insertions(+), 84 deletions(-)
> > >
> > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> > > index 3aed554bc8d8..9390f21f9902 100644
> > > --- a/drivers/tee/optee/core.c
> > > +++ b/drivers/tee/optee/core.c
> > > @@ -9,7 +9,6 @@
> > >  #include <linux/crash_dump.h>
> > >  #include <linux/errno.h>
> > >  #include <linux/io.h>
> > > -#include <linux/mm.h>
> > >  #include <linux/module.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/string.h>
> > > @@ -17,69 +16,6 @@
> > >  #include <linux/types.h>
> > >  #include "optee_private.h"
> > >
> > > -int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > -                              size_t size, size_t align,
> > > -                              int (*shm_register)(struct tee_context *ctx,
> > > -                                                  struct tee_shm *shm,
> > > -                                                  struct page **pages,
> > > -                                                  size_t num_pages,
> > > -                                                  unsigned long start))
> > > -{
> > > -       size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
> > > -       struct page **pages;
> > > -       unsigned int i;
> > > -       int rc = 0;
> > > -
> > > -       /*
> > > -        * Ignore alignment since this is already going to be page aligned
> > > -        * and there's no need for any larger alignment.
> > > -        */
> > > -       shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
> > > -                                      GFP_KERNEL | __GFP_ZERO);
> > > -       if (!shm->kaddr)
> > > -               return -ENOMEM;
> > > -
> > > -       shm->paddr = virt_to_phys(shm->kaddr);
> > > -       shm->size = nr_pages * PAGE_SIZE;
> > > -
> > > -       pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
> > > -       if (!pages) {
> > > -               rc = -ENOMEM;
> > > -               goto err;
> > > -       }
> > > -
> > > -       for (i = 0; i < nr_pages; i++)
> > > -               pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
> > > -
> > > -       shm->pages = pages;
> > > -       shm->num_pages = nr_pages;
> > > -
> > > -       if (shm_register) {
> > > -               rc = shm_register(shm->ctx, shm, pages, nr_pages,
> > > -                                 (unsigned long)shm->kaddr);
> > > -               if (rc)
> > > -                       goto err;
> > > -       }
> > > -
> > > -       return 0;
> > > -err:
> > > -       free_pages_exact(shm->kaddr, shm->size);
> > > -       shm->kaddr = NULL;
> > > -       return rc;
> > > -}
> > > -
> > > -void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > -                              int (*shm_unregister)(struct tee_context *ctx,
> > > -                                                    struct tee_shm *shm))
> > > -{
> > > -       if (shm_unregister)
> > > -               shm_unregister(shm->ctx, shm);
> > > -       free_pages_exact(shm->kaddr, shm->size);
> > > -       shm->kaddr = NULL;
> > > -       kfree(shm->pages);
> > > -       shm->pages = NULL;
> > > -}
> > > -
> > >  static void optee_bus_scan(struct work_struct *work)
> > >  {
> > >         WARN_ON(optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP));
> > > diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> > > index ecb5eb079408..ee11918a2b35 100644
> > > --- a/drivers/tee/optee/ffa_abi.c
> > > +++ b/drivers/tee/optee/ffa_abi.c
> > > @@ -374,14 +374,14 @@ static int optee_ffa_shm_unregister_supp(struct tee_context *ctx,
> > >  static int pool_ffa_op_alloc(struct tee_shm_pool *pool,
> > >                              struct tee_shm *shm, size_t size, size_t align)
> > >  {
> > > -       return optee_pool_op_alloc_helper(pool, shm, size, align,
> > > -                                         optee_ffa_shm_register);
> > > +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> > > +                                           optee_ffa_shm_register);
> > >  }
> > >
> > >  static void pool_ffa_op_free(struct tee_shm_pool *pool,
> > >                              struct tee_shm *shm)
> > >  {
> > > -       optee_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
> > > +       tee_shm_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
> > >  }
> > >
> > >  static void pool_ffa_op_destroy_pool(struct tee_shm_pool *pool)
> > > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> > > index 7a5243c78b55..a153285a1919 100644
> > > --- a/drivers/tee/optee/optee_private.h
> > > +++ b/drivers/tee/optee/optee_private.h
> > > @@ -283,18 +283,6 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session);
> > >  int optee_enumerate_devices(u32 func);
> > >  void optee_unregister_devices(void);
> > >
> > > -int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > -                              size_t size, size_t align,
> > > -                              int (*shm_register)(struct tee_context *ctx,
> > > -                                                  struct tee_shm *shm,
> > > -                                                  struct page **pages,
> > > -                                                  size_t num_pages,
> > > -                                                  unsigned long start));
> > > -void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > -                              int (*shm_unregister)(struct tee_context *ctx,
> > > -                                                    struct tee_shm *shm));
> > > -
> > > -
> > >  void optee_remove_common(struct optee *optee);
> > >  int optee_open(struct tee_context *ctx, bool cap_memref_null);
> > >  void optee_release(struct tee_context *ctx);
> > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> > > index a37f87087e5c..b0c616b6870d 100644
> > > --- a/drivers/tee/optee/smc_abi.c
> > > +++ b/drivers/tee/optee/smc_abi.c
> > > @@ -592,19 +592,20 @@ static int pool_op_alloc(struct tee_shm_pool *pool,
> > >          * to be registered with OP-TEE.
> > >          */
> > >         if (shm->flags & TEE_SHM_PRIV)
> > > -               return optee_pool_op_alloc_helper(pool, shm, size, align, NULL);
> > > +               return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> > > +                                                   NULL);
> > >
> > > -       return optee_pool_op_alloc_helper(pool, shm, size, align,
> > > -                                         optee_shm_register);
> > > +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> > > +                                           optee_shm_register);
> > >  }
> > >
> > >  static void pool_op_free(struct tee_shm_pool *pool,
> > >                          struct tee_shm *shm)
> > >  {
> > >         if (!(shm->flags & TEE_SHM_PRIV))
> > > -               optee_pool_op_free_helper(pool, shm, optee_shm_unregister);
> > > +               tee_shm_pool_op_free_helper(pool, shm, optee_shm_unregister);
> > >         else
> > > -               optee_pool_op_free_helper(pool, shm, NULL);
> > > +               tee_shm_pool_op_free_helper(pool, shm, NULL);
> > >  }
> > >T
> > >  static void pool_op_destroy_pool(struct tee_shm_pool *pool)
> > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> > > index 731d9028b67f..641aad92ffe2 100644
> > > --- a/drivers/tee/tee_shm.c
> > > +++ b/drivers/tee/tee_shm.c
> > > @@ -202,6 +202,71 @@ struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size)
> > >  }
> > >  EXPORT_SYMBOL_GPL(tee_shm_alloc_priv_buf);
> > >
> > > +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> >
> > I don't see the first argument (struct tee_shm_pool *pool) being used,
> > so drop that. Also, we can just rename it as
> > tee_dyn_shm_alloc_helper().
> >
> > > +                                size_t size, size_t align,
> > > +                                int (*shm_register)(struct tee_context *ctx,
> > > +                                                    struct tee_shm *shm,
> > > +                                                    struct page **pages,
> > > +                                                    size_t num_pages,
> > > +                                                    unsigned long start))
> > > +{
> > > +       size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
> > > +       struct page **pages;
> > > +       unsigned int i;
> > > +       int rc = 0;
> > > +
> > > +       /*
> > > +        * Ignore alignment since this is already going to be page aligned
> > > +        * and there's no need for any larger alignment.
> > > +        */
> > > +       shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
> > > +                                      GFP_KERNEL | __GFP_ZERO);
> > > +       if (!shm->kaddr)
> > > +               return -ENOMEM;
> > > +
> > > +       shm->paddr = virt_to_phys(shm->kaddr);
> > > +       shm->size = nr_pages * PAGE_SIZE;
> > > +
> > > +       pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
> > > +       if (!pages) {
> > > +               rc = -ENOMEM;
> > > +               goto err;
> > > +       }
> > > +
> > > +       for (i = 0; i < nr_pages; i++)
> > > +               pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
> > > +
> > > +       shm->pages = pages;
> > > +       shm->num_pages = nr_pages;
> > > +
> > > +       if (shm_register) {
> > > +               rc = shm_register(shm->ctx, shm, pages, nr_pages,
> > > +                                 (unsigned long)shm->kaddr);
> > > +               if (rc)
> > > +                       goto err;
> > > +       }
> > > +
> > > +       return 0;
> > > +err:
> > > +       free_pages_exact(shm->kaddr, shm->size);
> > > +       shm->kaddr = NULL;
> > > +       return rc;
> > > +}
> > > +EXPORT_SYMBOL_GPL(tee_shm_pool_op_alloc_helper);
> > > +
> > > +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> >
> > Ditto tee_shm_pool_op_free_helper() -> tee_dyn_shm_free_helper()
> >
> > > +                                int (*shm_unregister)(struct tee_context *ctx,
> > > +                                                      struct tee_shm *shm))
> > > +{
> > > +       if (shm_unregister)
> > > +               shm_unregister(shm->ctx, shm);
> > > +       free_pages_exact(shm->kaddr, shm->size);
> > > +       shm->kaddr = NULL;
> > > +       kfree(shm->pages);
> > > +       shm->pages = NULL;
> > > +}
> > > +EXPORT_SYMBOL_GPL(tee_shm_pool_op_free_helper);
> > > +
> > >  static struct tee_shm *
> > >  register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
> > >                     int id)
> > > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> > > index 911ddf92dcee..4cf402424e71 100644
> > > --- a/include/linux/tee_drv.h
> > > +++ b/include/linux/tee_drv.h
> > > @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
> > >  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
> > >  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
> > >
> > > +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > +                                size_t size, size_t align,
> > > +                                int (*shm_register)(struct tee_context *ctx,
> > > +                                                    struct tee_shm *shm,
> > > +                                                    struct page **pages,
> > > +                                                    size_t num_pages,
> > > +                                                    unsigned long start));
> > > +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > +                                int (*shm_unregister)(struct tee_context *ctx,
> > > +                                                      struct tee_shm *shm));
> > > +
> >
> > These rather belong to drivers/tee/tee_private.h as we shouldn't
> > expose them to other kernel client drivers.
>
> This is the right place, this .h file is for TEE drivers too.
>

But this is shared with other kernel TEE client drivers and we
shouldn't expose internal APIs which aren't meant for them with a side
effect of API abuse too. Any particular reason to not use
drivers/tee/tee_private.h?

-Sumit

> Cheers,
> Jens
>
> >
> > -Sumit
> >
> > >  struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx,
> > >                                             void *addr, size_t length);
> > >
> > > --
> > > 2.34.1
> > >

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Jens Wiklander]

On Wed, Feb 28, 2024 at 6:58 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> On Tue, 27 Feb 2024 at 21:20, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > On Tue, Feb 27, 2024 at 7:06 AM Sumit Garg <sumit.garg@linaro.org> wrote:
[snip]
> > > > --- a/include/linux/tee_drv.h
> > > > +++ b/include/linux/tee_drv.h
> > > > @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
> > > >  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
> > > >  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
> > > >
> > > > +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > > +                                size_t size, size_t align,
> > > > +                                int (*shm_register)(struct tee_context *ctx,
> > > > +                                                    struct tee_shm *shm,
> > > > +                                                    struct page **pages,
> > > > +                                                    size_t num_pages,
> > > > +                                                    unsigned long start));
> > > > +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > > +                                int (*shm_unregister)(struct tee_context *ctx,
> > > > +                                                      struct tee_shm *shm));
> > > > +
> > >
> > > These rather belong to drivers/tee/tee_private.h as we shouldn't
> > > expose them to other kernel client drivers.
> >
> > This is the right place, this .h file is for TEE drivers too.
> >
>
> But this is shared with other kernel TEE client drivers and we
> shouldn't expose internal APIs which aren't meant for them with a side
> effect of API abuse too. Any particular reason to not use
> drivers/tee/tee_private.h?

drivers/tee/tee_private.h is supposed to be used internally by only
the files in drivers/tee. If you look in include/linux/tee_drv.h
you'll find a few functions and other definitions that a kernel TEE
client driver should not use, for instance, tee_device_register() and
tee_device_unregister(). This kernel TEE client interface was
introduced with commit 25559c22cef8 ("tee: add kernel internal client
interface"). include/linux/tee_drv.h existed before we even had any
kernel TEE client interface.

Cheers,
Jens

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Sumit Garg]

On Wed, 28 Feb 2024 at 14:11, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> On Wed, Feb 28, 2024 at 6:58 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > On Tue, 27 Feb 2024 at 21:20, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> > >
> > > On Tue, Feb 27, 2024 at 7:06 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> [snip]
> > > > > --- a/include/linux/tee_drv.h
> > > > > +++ b/include/linux/tee_drv.h
> > > > > @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
> > > > >  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
> > > > >  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
> > > > >
> > > > > +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > > > +                                size_t size, size_t align,
> > > > > +                                int (*shm_register)(struct tee_context *ctx,
> > > > > +                                                    struct tee_shm *shm,
> > > > > +                                                    struct page **pages,
> > > > > +                                                    size_t num_pages,
> > > > > +                                                    unsigned long start));
> > > > > +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > > > +                                int (*shm_unregister)(struct tee_context *ctx,
> > > > > +                                                      struct tee_shm *shm));
> > > > > +
> > > >
> > > > These rather belong to drivers/tee/tee_private.h as we shouldn't
> > > > expose them to other kernel client drivers.
> > >
> > > This is the right place, this .h file is for TEE drivers too.
> > >
> >
> > But this is shared with other kernel TEE client drivers and we
> > shouldn't expose internal APIs which aren't meant for them with a side
> > effect of API abuse too. Any particular reason to not use
> > drivers/tee/tee_private.h?
>
> drivers/tee/tee_private.h is supposed to be used internally by only
> the files in drivers/tee.

How about "struct tee_device" being in drivers/tee/tee_private.h?

> If you look in include/linux/tee_drv.h
> you'll find a few functions and other definitions that a kernel TEE
> client driver should not use, for instance, tee_device_register() and
> tee_device_unregister(). This kernel TEE client interface was
> introduced with commit 25559c22cef8 ("tee: add kernel internal client
> interface"). include/linux/tee_drv.h existed before we even had any
> kernel TEE client interface.

Anyhow, it looks like there is a chance for refactoring here. How
about splitting this header further in something like
include/linux/tee_core.h which will contain all the pieces relevant to
TEE drivers?

BTW, this patch series can keep using include/linux/tee_drv.h for the
time being.

-Sumit

>
> Cheers,
> Jens

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Jens Wiklander]

On Wed, Feb 28, 2024 at 12:20 PM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> On Wed, 28 Feb 2024 at 14:11, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > On Wed, Feb 28, 2024 at 6:58 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> > >
> > > On Tue, 27 Feb 2024 at 21:20, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> > > >
> > > > On Tue, Feb 27, 2024 at 7:06 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> > [snip]
> > > > > > --- a/include/linux/tee_drv.h
> > > > > > +++ b/include/linux/tee_drv.h
> > > > > > @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
> > > > > >  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
> > > > > >  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
> > > > > >
> > > > > > +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > > > > +                                size_t size, size_t align,
> > > > > > +                                int (*shm_register)(struct tee_context *ctx,
> > > > > > +                                                    struct tee_shm *shm,
> > > > > > +                                                    struct page **pages,
> > > > > > +                                                    size_t num_pages,
> > > > > > +                                                    unsigned long start));
> > > > > > +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> > > > > > +                                int (*shm_unregister)(struct tee_context *ctx,
> > > > > > +                                                      struct tee_shm *shm));
> > > > > > +
> > > > >
> > > > > These rather belong to drivers/tee/tee_private.h as we shouldn't
> > > > > expose them to other kernel client drivers.
> > > >
> > > > This is the right place, this .h file is for TEE drivers too.
> > > >
> > >
> > > But this is shared with other kernel TEE client drivers and we
> > > shouldn't expose internal APIs which aren't meant for them with a side
> > > effect of API abuse too. Any particular reason to not use
> > > drivers/tee/tee_private.h?
> >
> > drivers/tee/tee_private.h is supposed to be used internally by only
> > the files in drivers/tee.
>
> How about "struct tee_device" being in drivers/tee/tee_private.h?

What about it?

>
> > If you look in include/linux/tee_drv.h
> > you'll find a few functions and other definitions that a kernel TEE
> > client driver should not use, for instance, tee_device_register() and
> > tee_device_unregister(). This kernel TEE client interface was
> > introduced with commit 25559c22cef8 ("tee: add kernel internal client
> > interface"). include/linux/tee_drv.h existed before we even had any
> > kernel TEE client interface.
>
> Anyhow, it looks like there is a chance for refactoring here. How
> about splitting this header further in something like
> include/linux/tee_core.h which will contain all the pieces relevant to
> TEE drivers?

Sure.

>
> BTW, this patch series can keep using include/linux/tee_drv.h for the
> time being.

Agreed.

Cheers,
Jens

>
> -Sumit
>
> >
> > Cheers,
> > Jens

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Balint Dobszay]

Hi Sumit,

On 27 Feb 2024, at 7:06, Sumit Garg wrote:

> Hi Balint,
>
> On Fri, 23 Feb 2024 at 15:22, Balint Dobszay <balint.dobszay@arm.com> wrote:
>>
>> Move the pool alloc and free helper functions from the OP-TEE driver to
>> the TEE subsystem, since these could be reused in other TEE drivers.
>> This patch is not supposed to change behavior, it's only reorganizing
>> the code.
>>
>> Suggested-by: Jens Wiklander <jens.wiklander@linaro.org>
>> Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
>> ---
>>  drivers/tee/optee/core.c          | 64 ------------------------------
>>  drivers/tee/optee/ffa_abi.c       |  6 +--
>>  drivers/tee/optee/optee_private.h | 12 ------
>>  drivers/tee/optee/smc_abi.c       | 11 +++---
>>  drivers/tee/tee_shm.c             | 65 +++++++++++++++++++++++++++++++
>>  include/linux/tee_drv.h           | 11 ++++++
>>  6 files changed, 85 insertions(+), 84 deletions(-)
>>
>> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
>> index 3aed554bc8d8..9390f21f9902 100644
>> --- a/drivers/tee/optee/core.c
>> +++ b/drivers/tee/optee/core.c
>> @@ -9,7 +9,6 @@
>>  #include <linux/crash_dump.h>
>>  #include <linux/errno.h>
>>  #include <linux/io.h>
>> -#include <linux/mm.h>
>>  #include <linux/module.h>
>>  #include <linux/slab.h>
>>  #include <linux/string.h>
>> @@ -17,69 +16,6 @@
>>  #include <linux/types.h>
>>  #include "optee_private.h"
>>
>> -int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>> -                              size_t size, size_t align,
>> -                              int (*shm_register)(struct tee_context *ctx,
>> -                                                  struct tee_shm *shm,
>> -                                                  struct page **pages,
>> -                                                  size_t num_pages,
>> -                                                  unsigned long start))
>> -{
>> -       size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
>> -       struct page **pages;
>> -       unsigned int i;
>> -       int rc = 0;
>> -
>> -       /*
>> -        * Ignore alignment since this is already going to be page aligned
>> -        * and there's no need for any larger alignment.
>> -        */
>> -       shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
>> -                                      GFP_KERNEL | __GFP_ZERO);
>> -       if (!shm->kaddr)
>> -               return -ENOMEM;
>> -
>> -       shm->paddr = virt_to_phys(shm->kaddr);
>> -       shm->size = nr_pages * PAGE_SIZE;
>> -
>> -       pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
>> -       if (!pages) {
>> -               rc = -ENOMEM;
>> -               goto err;
>> -       }
>> -
>> -       for (i = 0; i < nr_pages; i++)
>> -               pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
>> -
>> -       shm->pages = pages;
>> -       shm->num_pages = nr_pages;
>> -
>> -       if (shm_register) {
>> -               rc = shm_register(shm->ctx, shm, pages, nr_pages,
>> -                                 (unsigned long)shm->kaddr);
>> -               if (rc)
>> -                       goto err;
>> -       }
>> -
>> -       return 0;
>> -err:
>> -       free_pages_exact(shm->kaddr, shm->size);
>> -       shm->kaddr = NULL;
>> -       return rc;
>> -}
>> -
>> -void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>> -                              int (*shm_unregister)(struct tee_context *ctx,
>> -                                                    struct tee_shm *shm))
>> -{
>> -       if (shm_unregister)
>> -               shm_unregister(shm->ctx, shm);
>> -       free_pages_exact(shm->kaddr, shm->size);
>> -       shm->kaddr = NULL;
>> -       kfree(shm->pages);
>> -       shm->pages = NULL;
>> -}
>> -
>>  static void optee_bus_scan(struct work_struct *work)
>>  {
>>         WARN_ON(optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP));
>> diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
>> index ecb5eb079408..ee11918a2b35 100644
>> --- a/drivers/tee/optee/ffa_abi.c
>> +++ b/drivers/tee/optee/ffa_abi.c
>> @@ -374,14 +374,14 @@ static int optee_ffa_shm_unregister_supp(struct tee_context *ctx,
>>  static int pool_ffa_op_alloc(struct tee_shm_pool *pool,
>>                              struct tee_shm *shm, size_t size, size_t align)
>>  {
>> -       return optee_pool_op_alloc_helper(pool, shm, size, align,
>> -                                         optee_ffa_shm_register);
>> +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
>> +                                           optee_ffa_shm_register);
>>  }
>>
>>  static void pool_ffa_op_free(struct tee_shm_pool *pool,
>>                              struct tee_shm *shm)
>>  {
>> -       optee_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
>> +       tee_shm_pool_op_free_helper(pool, shm, optee_ffa_shm_unregister);
>>  }
>>
>>  static void pool_ffa_op_destroy_pool(struct tee_shm_pool *pool)
>> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
>> index 7a5243c78b55..a153285a1919 100644
>> --- a/drivers/tee/optee/optee_private.h
>> +++ b/drivers/tee/optee/optee_private.h
>> @@ -283,18 +283,6 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session);
>>  int optee_enumerate_devices(u32 func);
>>  void optee_unregister_devices(void);
>>
>> -int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>> -                              size_t size, size_t align,
>> -                              int (*shm_register)(struct tee_context *ctx,
>> -                                                  struct tee_shm *shm,
>> -                                                  struct page **pages,
>> -                                                  size_t num_pages,
>> -                                                  unsigned long start));
>> -void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>> -                              int (*shm_unregister)(struct tee_context *ctx,
>> -                                                    struct tee_shm *shm));
>> -
>> -
>>  void optee_remove_common(struct optee *optee);
>>  int optee_open(struct tee_context *ctx, bool cap_memref_null);
>>  void optee_release(struct tee_context *ctx);
>> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
>> index a37f87087e5c..b0c616b6870d 100644
>> --- a/drivers/tee/optee/smc_abi.c
>> +++ b/drivers/tee/optee/smc_abi.c
>> @@ -592,19 +592,20 @@ static int pool_op_alloc(struct tee_shm_pool *pool,
>>          * to be registered with OP-TEE.
>>          */
>>         if (shm->flags & TEE_SHM_PRIV)
>> -               return optee_pool_op_alloc_helper(pool, shm, size, align, NULL);
>> +               return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
>> +                                                   NULL);
>>
>> -       return optee_pool_op_alloc_helper(pool, shm, size, align,
>> -                                         optee_shm_register);
>> +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
>> +                                           optee_shm_register);
>>  }
>>
>>  static void pool_op_free(struct tee_shm_pool *pool,
>>                          struct tee_shm *shm)
>>  {
>>         if (!(shm->flags & TEE_SHM_PRIV))
>> -               optee_pool_op_free_helper(pool, shm, optee_shm_unregister);
>> +               tee_shm_pool_op_free_helper(pool, shm, optee_shm_unregister);
>>         else
>> -               optee_pool_op_free_helper(pool, shm, NULL);
>> +               tee_shm_pool_op_free_helper(pool, shm, NULL);
>>  }
>>
>>  static void pool_op_destroy_pool(struct tee_shm_pool *pool)
>> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
>> index 731d9028b67f..641aad92ffe2 100644
>> --- a/drivers/tee/tee_shm.c
>> +++ b/drivers/tee/tee_shm.c
>> @@ -202,6 +202,71 @@ struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size)
>>  }
>>  EXPORT_SYMBOL_GPL(tee_shm_alloc_priv_buf);
>>
>> +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>
> I don't see the first argument (struct tee_shm_pool *pool) being used,
> so drop that. Also, we can just rename it as
> tee_dyn_shm_alloc_helper().

Okay, I'll refactor.

>> +                                size_t size, size_t align,
>> +                                int (*shm_register)(struct tee_context *ctx,
>> +                                                    struct tee_shm *shm,
>> +                                                    struct page **pages,
>> +                                                    size_t num_pages,
>> +                                                    unsigned long start))
>> +{
>> +       size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
>> +       struct page **pages;
>> +       unsigned int i;
>> +       int rc = 0;
>> +
>> +       /*
>> +        * Ignore alignment since this is already going to be page aligned
>> +        * and there's no need for any larger alignment.
>> +        */
>> +       shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
>> +                                      GFP_KERNEL | __GFP_ZERO);
>> +       if (!shm->kaddr)
>> +               return -ENOMEM;
>> +
>> +       shm->paddr = virt_to_phys(shm->kaddr);
>> +       shm->size = nr_pages * PAGE_SIZE;
>> +
>> +       pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
>> +       if (!pages) {
>> +               rc = -ENOMEM;
>> +               goto err;
>> +       }
>> +
>> +       for (i = 0; i < nr_pages; i++)
>> +               pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
>> +
>> +       shm->pages = pages;
>> +       shm->num_pages = nr_pages;
>> +
>> +       if (shm_register) {
>> +               rc = shm_register(shm->ctx, shm, pages, nr_pages,
>> +                                 (unsigned long)shm->kaddr);
>> +               if (rc)
>> +                       goto err;
>> +       }
>> +
>> +       return 0;
>> +err:
>> +       free_pages_exact(shm->kaddr, shm->size);
>> +       shm->kaddr = NULL;
>> +       return rc;
>> +}
>> +EXPORT_SYMBOL_GPL(tee_shm_pool_op_alloc_helper);
>> +
>> +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>
> Ditto tee_shm_pool_op_free_helper() -> tee_dyn_shm_free_helper()

Okay, actually the first argument is unused here too, I'll drop it.

>> +                                int (*shm_unregister)(struct tee_context *ctx,
>> +                                                      struct tee_shm *shm))
>> +{
>> +       if (shm_unregister)
>> +               shm_unregister(shm->ctx, shm);
>> +       free_pages_exact(shm->kaddr, shm->size);
>> +       shm->kaddr = NULL;
>> +       kfree(shm->pages);
>> +       shm->pages = NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(tee_shm_pool_op_free_helper);
>> +
>>  static struct tee_shm *
>>  register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
>>                     int id)
>> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
>> index 911ddf92dcee..4cf402424e71 100644
>> --- a/include/linux/tee_drv.h
>> +++ b/include/linux/tee_drv.h
>> @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
>>  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
>>  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
>>
>> +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>> +                                size_t size, size_t align,
>> +                                int (*shm_register)(struct tee_context *ctx,
>> +                                                    struct tee_shm *shm,
>> +                                                    struct page **pages,
>> +                                                    size_t num_pages,
>> +                                                    unsigned long start));
>> +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>> +                                int (*shm_unregister)(struct tee_context *ctx,
>> +                                                      struct tee_shm *shm));
>> +
>
> These rather belong to drivers/tee/tee_private.h as we shouldn't
> expose them to other kernel client drivers.

As per the discussion in the other thread I'll ignore this.

Regards,
Balint

>>  struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx,
>>                                             void *addr, size_t length);
>>
>> --
>> 2.34.1
>>

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Sumit Garg]

Hi Balint,

On Mon, 4 Mar 2024 at 14:33, Balint Dobszay <balint.dobszay@arm.com> wrote:
>
> Hi Sumit,
>

[snip]

> >> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> >> index 911ddf92dcee..4cf402424e71 100644
> >> --- a/include/linux/tee_drv.h
> >> +++ b/include/linux/tee_drv.h
> >> @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
> >>  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
> >>  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
> >>
> >> +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> >> +                                size_t size, size_t align,
> >> +                                int (*shm_register)(struct tee_context *ctx,
> >> +                                                    struct tee_shm *shm,
> >> +                                                    struct page **pages,
> >> +                                                    size_t num_pages,
> >> +                                                    unsigned long start));
> >> +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> >> +                                int (*shm_unregister)(struct tee_context *ctx,
> >> +                                                      struct tee_shm *shm));
> >> +
> >
> > These rather belong to drivers/tee/tee_private.h as we shouldn't
> > expose them to other kernel client drivers.
>
> As per the discussion in the other thread I'll ignore this.
>

Then it will have conflicts with this [1] patch. If you are fine to
incorporate [1] in your series then the right place for these function
declarations should be include/linux/tee_core.h.

[1] https://www.spinics.net/lists/kernel/msg5122983.html

-Sumit

> Regards,
> Balint
>
> >>  struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx,
> >>                                             void *addr, size_t length);
> >>
> >> --
> >> 2.34.1
> >>

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Balint Dobszay]

On 4 Mar 2024, at 10:17, Sumit Garg wrote:

> Hi Balint,
>
> On Mon, 4 Mar 2024 at 14:33, Balint Dobszay <balint.dobszay@arm.com> wrote:
>>
>> Hi Sumit,
>>
>
> [snip]
>
>>>> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
>>>> index 911ddf92dcee..4cf402424e71 100644
>>>> --- a/include/linux/tee_drv.h
>>>> +++ b/include/linux/tee_drv.h
>>>> @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
>>>>  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
>>>>  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
>>>>
>>>> +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>>>> +                                size_t size, size_t align,
>>>> +                                int (*shm_register)(struct tee_context *ctx,
>>>> +                                                    struct tee_shm *shm,
>>>> +                                                    struct page **pages,
>>>> +                                                    size_t num_pages,
>>>> +                                                    unsigned long start));
>>>> +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>>>> +                                int (*shm_unregister)(struct tee_context *ctx,
>>>> +                                                      struct tee_shm *shm));
>>>> +
>>>
>>> These rather belong to drivers/tee/tee_private.h as we shouldn't
>>> expose them to other kernel client drivers.
>>
>> As per the discussion in the other thread I'll ignore this.
>>
>
> Then it will have conflicts with this [1] patch. If you are fine to
> incorporate [1] in your series then the right place for these function
> declarations should be include/linux/tee_core.h.
>
> [1] https://www.spinics.net/lists/kernel/msg5122983.html

You're right, I'll rebase my patches on this.

By incorporating your patch in my series, do you mean that I should just
add it as the first patch in the series for the next version? Or keep my
series as is (do the rebase of course) and just mention that it's using
your patch as base?

Regards,
Balint

>>
>>>>  struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx,
>>>>                                             void *addr, size_t length);
>>>>
>>>> --
>>>> 2.34.1
>>>>

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Sumit Garg]

On Mon, 4 Mar 2024 at 19:57, Balint Dobszay <balint.dobszay@arm.com> wrote:
>
> On 4 Mar 2024, at 10:17, Sumit Garg wrote:
>
> > Hi Balint,
> >
> > On Mon, 4 Mar 2024 at 14:33, Balint Dobszay <balint.dobszay@arm.com> wrote:
> >>
> >> Hi Sumit,
> >>
> >
> > [snip]
> >
> >>>> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> >>>> index 911ddf92dcee..4cf402424e71 100644
> >>>> --- a/include/linux/tee_drv.h
> >>>> +++ b/include/linux/tee_drv.h
> >>>> @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
> >>>>  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
> >>>>  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
> >>>>
> >>>> +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> >>>> +                                size_t size, size_t align,
> >>>> +                                int (*shm_register)(struct tee_context *ctx,
> >>>> +                                                    struct tee_shm *shm,
> >>>> +                                                    struct page **pages,
> >>>> +                                                    size_t num_pages,
> >>>> +                                                    unsigned long start));
> >>>> +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> >>>> +                                int (*shm_unregister)(struct tee_context *ctx,
> >>>> +                                                      struct tee_shm *shm));
> >>>> +
> >>>
> >>> These rather belong to drivers/tee/tee_private.h as we shouldn't
> >>> expose them to other kernel client drivers.
> >>
> >> As per the discussion in the other thread I'll ignore this.
> >>
> >
> > Then it will have conflicts with this [1] patch. If you are fine to
> > incorporate [1] in your series then the right place for these function
> > declarations should be include/linux/tee_core.h.
> >
> > [1] https://www.spinics.net/lists/kernel/msg5122983.html
>
> You're right, I'll rebase my patches on this.
>
> By incorporating your patch in my series, do you mean that I should just
> add it as the first patch in the series for the next version?

If you are happy to carry it then I would be in favour of this to take
care of dependency.

-Sumit

> Or keep my
> series as is (do the rebase of course) and just mention that it's using
> your patch as base?
>
> Regards,
> Balint
>
> >>
> >>>>  struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx,
> >>>>                                             void *addr, size_t length);
> >>>>
> >>>> --
> >>>> 2.34.1
> >>>>

Re: [PATCH v2 1/3] tee: optee: Move pool_op helper functions

[Balint Dobszay]

On 5 Mar 2024, at 6:36, Sumit Garg wrote:

> On Mon, 4 Mar 2024 at 19:57, Balint Dobszay <balint.dobszay@arm.com> wrote:
>>
>> On 4 Mar 2024, at 10:17, Sumit Garg wrote:
>>
>>> Hi Balint,
>>>
>>> On Mon, 4 Mar 2024 at 14:33, Balint Dobszay <balint.dobszay@arm.com> wrote:
>>>>
>>>> Hi Sumit,
>>>>
>>>
>>> [snip]
>>>
>>>>>> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
>>>>>> index 911ddf92dcee..4cf402424e71 100644
>>>>>> --- a/include/linux/tee_drv.h
>>>>>> +++ b/include/linux/tee_drv.h
>>>>>> @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev);
>>>>>>  struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
>>>>>>  struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
>>>>>>
>>>>>> +int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>>>>>> +                                size_t size, size_t align,
>>>>>> +                                int (*shm_register)(struct tee_context *ctx,
>>>>>> +                                                    struct tee_shm *shm,
>>>>>> +                                                    struct page **pages,
>>>>>> +                                                    size_t num_pages,
>>>>>> +                                                    unsigned long start));
>>>>>> +void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>>>>>> +                                int (*shm_unregister)(struct tee_context *ctx,
>>>>>> +                                                      struct tee_shm *shm));
>>>>>> +
>>>>>
>>>>> These rather belong to drivers/tee/tee_private.h as we shouldn't
>>>>> expose them to other kernel client drivers.
>>>>
>>>> As per the discussion in the other thread I'll ignore this.
>>>>
>>>
>>> Then it will have conflicts with this [1] patch. If you are fine to
>>> incorporate [1] in your series then the right place for these function
>>> declarations should be include/linux/tee_core.h.
>>>
>>> [1] https://www.spinics.net/lists/kernel/msg5122983.html
>>
>> You're right, I'll rebase my patches on this.
>>
>> By incorporating your patch in my series, do you mean that I should just
>> add it as the first patch in the series for the next version?
>
> If you are happy to carry it then I would be in favour of this to take
> care of dependency.

Sure, I've added it to v3.

Regards,
Balint

>> Or keep my
>> series as is (do the rebase of course) and just mention that it's using
>> your patch as base?
>>
>> Regards,
>> Balint
>>
>>>>
>>>>>>  struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx,
>>>>>>                                             void *addr, size_t length);
>>>>>>
>>>>>> --
>>>>>> 2.34.1
>>>>>>

[PATCH v2 2/3] tee: tstee: Add Trusted Services TEE driver

[Balint Dobszay]

The Trusted Services project provides a framework for developing and
deploying device Root of Trust services in FF-A Secure Partitions. The
FF-A SPs are accessible through the FF-A driver, but this doesn't
provide a user space interface. The goal of this TEE driver is to make
Trusted Services SPs accessible for user space clients.

All TS SPs have the same FF-A UUID, it identifies the RPC protocol used
by TS. A TS SP can host one or more services, a service is identified by
its service UUID. The same type of service cannot be present twice in
the same SP. During SP boot each service in an SP is assigned an
interface ID, this is just a short ID to simplify message addressing.
There is 1:1 mapping between TS SPs and TEE devices, i.e. a separate TEE
device is registered for each TS SP. This is required since contrary to
the generic TEE design where memory is shared with the whole TEE
implementation, in case of FF-A, memory is shared with a specific SP. A
user space client has to be able to separately share memory with each SP
based on its endpoint ID.

Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
---
 drivers/tee/Kconfig               |   1 +
 drivers/tee/Makefile              |   1 +
 drivers/tee/tstee/Kconfig         |  11 +
 drivers/tee/tstee/Makefile        |   3 +
 drivers/tee/tstee/core.c          | 490 ++++++++++++++++++++++++++++++
 drivers/tee/tstee/tstee_private.h |  94 ++++++
 include/uapi/linux/tee.h          |   1 +
 7 files changed, 601 insertions(+)
 create mode 100644 drivers/tee/tstee/Kconfig
 create mode 100644 drivers/tee/tstee/Makefile
 create mode 100644 drivers/tee/tstee/core.c
 create mode 100644 drivers/tee/tstee/tstee_private.h

diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
index 73a147202e88..61b507c18780 100644
--- a/drivers/tee/Kconfig
+++ b/drivers/tee/Kconfig
@@ -15,5 +15,6 @@ if TEE
 
 source "drivers/tee/optee/Kconfig"
 source "drivers/tee/amdtee/Kconfig"
+source "drivers/tee/tstee/Kconfig"
 
 endif
diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
index 68da044afbfa..5488cba30bd2 100644
--- a/drivers/tee/Makefile
+++ b/drivers/tee/Makefile
@@ -5,3 +5,4 @@ tee-objs += tee_shm.o
 tee-objs += tee_shm_pool.o
 obj-$(CONFIG_OPTEE) += optee/
 obj-$(CONFIG_AMDTEE) += amdtee/
+obj-$(CONFIG_ARM_TSTEE) += tstee/
diff --git a/drivers/tee/tstee/Kconfig b/drivers/tee/tstee/Kconfig
new file mode 100644
index 000000000000..d32f91d47398
--- /dev/null
+++ b/drivers/tee/tstee/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config ARM_TSTEE
+	tristate "Arm Trusted Services TEE driver"
+	depends on ARM_FFA_TRANSPORT
+	default n
+	help
+	  The Trusted Services project provides a framework for developing and
+	  deploying device Root of Trust services in FF-A Secure Partitions.
+	  This driver provides an interface to make Trusted Services Secure
+	  Partitions accessible for user space clients, since the FF-A driver
+	  doesn't implement a user space interface directly.
diff --git a/drivers/tee/tstee/Makefile b/drivers/tee/tstee/Makefile
new file mode 100644
index 000000000000..5227020ebd30
--- /dev/null
+++ b/drivers/tee/tstee/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+arm-tstee-objs := core.o
+obj-$(CONFIG_ARM_TSTEE) = arm-tstee.o
diff --git a/drivers/tee/tstee/core.c b/drivers/tee/tstee/core.c
new file mode 100644
index 000000000000..2932be017dbe
--- /dev/null
+++ b/drivers/tee/tstee/core.c
@@ -0,0 +1,490 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, Arm Limited
+ */
+
+#define DRIVER_NAME "Arm TSTEE"
+#define pr_fmt(fmt) DRIVER_NAME ": " fmt
+
+#include <linux/arm_ffa.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/stat.h>
+#include <linux/tee_drv.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#include "tstee_private.h"
+
+#define FFA_DIRECT_REQ_ARG_NUM 5
+#define FFA_INVALID_MEM_HANDLE U64_MAX
+
+static void arg_list_to_ffa_data(const u32 *args,
+				 struct ffa_send_direct_data *data)
+{
+	data->data0 = args[0];
+	data->data1 = args[1];
+	data->data2 = args[2];
+	data->data3 = args[3];
+	data->data4 = args[4];
+}
+
+static void arg_list_from_ffa_data(const struct ffa_send_direct_data *data,
+				   u32 *args)
+{
+	args[0] = lower_32_bits(data->data0);
+	args[1] = lower_32_bits(data->data1);
+	args[2] = lower_32_bits(data->data2);
+	args[3] = lower_32_bits(data->data3);
+	args[4] = lower_32_bits(data->data4);
+}
+
+static void tstee_get_version(struct tee_device *teedev,
+			      struct tee_ioctl_version_data *vers)
+{
+	struct tstee *tstee = tee_get_drvdata(teedev);
+	struct tee_ioctl_version_data v = {
+		.impl_id = TEE_IMPL_ID_TSTEE,
+		/* FF-A endpoint ID only uses the lower 16 bits */
+		.impl_caps = lower_16_bits(tstee->ffa_dev->vm_id),
+		.gen_caps = 0,
+	};
+
+	*vers = v;
+}
+
+static int tstee_open(struct tee_context *ctx)
+{
+	struct ts_context_data *ctxdata;
+
+	ctxdata = kzalloc(sizeof(*ctxdata), GFP_KERNEL);
+	if (!ctxdata)
+		return -ENOMEM;
+
+	mutex_init(&ctxdata->mutex);
+	xa_init_flags(&ctxdata->sess_list, XA_FLAGS_ALLOC);
+
+	ctx->data = ctxdata;
+
+	return 0;
+}
+
+static void tstee_release(struct tee_context *ctx)
+{
+	struct ts_context_data *ctxdata = ctx->data;
+	struct ts_session *sess;
+	unsigned long idx;
+
+	if (!ctxdata)
+		return;
+
+	xa_for_each(&ctxdata->sess_list, idx, sess) {
+		xa_erase(&ctxdata->sess_list, idx);
+		kfree(sess);
+	}
+
+	xa_destroy(&ctxdata->sess_list);
+	mutex_destroy(&ctxdata->mutex);
+
+	kfree(ctxdata);
+	ctx->data = NULL;
+}
+
+static int tstee_open_session(struct tee_context *ctx,
+			      struct tee_ioctl_open_session_arg *arg,
+			      struct tee_param *param __always_unused)
+{
+	struct tstee *tstee = tee_get_drvdata(ctx->teedev);
+	struct ffa_device *ffa_dev = tstee->ffa_dev;
+	struct ts_context_data *ctxdata = ctx->data;
+	struct ffa_send_direct_data ffa_data;
+	struct ts_session *sess = NULL;
+	u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
+	u32 sess_id;
+	int rc;
+
+	ffa_args[TS_RPC_CTRL_REG] =
+		TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
+					      TS_RPC_OP_SERVICE_INFO);
+
+	memcpy(ffa_args + TS_RPC_SERVICE_INFO_UUID0, arg->uuid, UUID_SIZE);
+
+	arg_list_to_ffa_data(ffa_args, &ffa_data);
+	rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
+	if (rc)
+		return rc;
+
+	arg_list_from_ffa_data(&ffa_data, ffa_args);
+
+	if (ffa_args[TS_RPC_SERVICE_INFO_RPC_STATUS] != TS_RPC_OK)
+		return -ENODEV;
+
+	if (ffa_args[TS_RPC_SERVICE_INFO_IFACE] > U8_MAX)
+		return -EINVAL;
+
+	sess = kzalloc(sizeof(*sess), GFP_KERNEL);
+	if (!sess)
+		return -ENOMEM;
+
+	sess->iface_id = ffa_args[TS_RPC_SERVICE_INFO_IFACE];
+
+	rc = xa_alloc(&ctxdata->sess_list, &sess_id, sess, xa_limit_32b,
+		      GFP_KERNEL);
+	if (rc) {
+		kfree(sess);
+		return rc;
+	}
+
+	arg->session = sess_id;
+	arg->ret = 0;
+
+	return 0;
+}
+
+static int tstee_close_session(struct tee_context *ctx, u32 session)
+{
+	struct ts_context_data *ctxdata = ctx->data;
+	struct ts_session *sess;
+
+	mutex_lock(&ctxdata->mutex);
+	sess = xa_erase(&ctxdata->sess_list, session);
+	mutex_unlock(&ctxdata->mutex);
+	if (!sess)
+		return -EINVAL;
+
+	kfree(sess);
+
+	return 0;
+}
+
+static int tstee_invoke_func(struct tee_context *ctx,
+			     struct tee_ioctl_invoke_arg *arg,
+			     struct tee_param *param)
+{
+	struct tstee *tstee = tee_get_drvdata(ctx->teedev);
+	struct ffa_device *ffa_dev = tstee->ffa_dev;
+	struct ts_context_data *ctxdata = ctx->data;
+	struct ffa_send_direct_data ffa_data;
+	struct tee_shm *shm = NULL;
+	struct ts_session *sess;
+	u32 req_len, ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
+	int shm_id, rc;
+	u8 iface_id;
+	u64 handle;
+	u16 opcode;
+
+	mutex_lock(&ctxdata->mutex);
+	sess = xa_load(&ctxdata->sess_list, arg->session);
+
+	/*
+	 * Do this while holding the mutex to make sure that the session wasn't
+	 * closed meanwhile
+	 */
+	if (sess)
+		iface_id = sess->iface_id;
+
+	mutex_unlock(&ctxdata->mutex);
+	if (!sess)
+		return -EINVAL;
+
+	opcode = lower_16_bits(arg->func);
+	shm_id = lower_32_bits(param[0].u.value.a);
+	req_len = lower_32_bits(param[0].u.value.b);
+
+	if (shm_id != 0) {
+		shm = tee_shm_get_from_id(ctx, shm_id);
+		if (IS_ERR(shm))
+			return PTR_ERR(shm);
+
+		if (shm->size < req_len) {
+			pr_err("request doesn't fit into shared memory buffer\n");
+			rc = -EINVAL;
+			goto out;
+		}
+
+		handle = shm->sec_world_id;
+	} else {
+		handle = FFA_INVALID_MEM_HANDLE;
+	}
+
+	ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(iface_id,
+								  opcode);
+	ffa_args[TS_RPC_SERVICE_MEM_HANDLE_LSW] = lower_32_bits(handle);
+	ffa_args[TS_RPC_SERVICE_MEM_HANDLE_MSW] = upper_32_bits(handle);
+	ffa_args[TS_RPC_SERVICE_REQ_LEN] = req_len;
+	ffa_args[TS_RPC_SERVICE_CLIENT_ID] = 0;
+
+	arg_list_to_ffa_data(ffa_args, &ffa_data);
+	rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
+	if (rc)
+		goto out;
+
+	arg_list_from_ffa_data(&ffa_data, ffa_args);
+
+	if (ffa_args[TS_RPC_SERVICE_RPC_STATUS] != TS_RPC_OK) {
+		pr_err("invoke_func rpc status: %d\n",
+		       ffa_args[TS_RPC_SERVICE_RPC_STATUS]);
+		rc = -EINVAL;
+		goto out;
+	}
+
+	arg->ret = ffa_args[TS_RPC_SERVICE_STATUS];
+	if (shm && shm->size >= ffa_args[TS_RPC_SERVICE_RESP_LEN])
+		param[0].u.value.a = ffa_args[TS_RPC_SERVICE_RESP_LEN];
+
+out:
+	if (shm)
+		tee_shm_put(shm);
+
+	return rc;
+}
+
+static int tstee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
+			      struct page **pages, size_t num_pages,
+			      unsigned long start __always_unused)
+{
+	struct tstee *tstee = tee_get_drvdata(ctx->teedev);
+	struct ffa_device *ffa_dev = tstee->ffa_dev;
+	struct ffa_mem_region_attributes mem_attr = {
+		.receiver = tstee->ffa_dev->vm_id,
+		.attrs = FFA_MEM_RW,
+		.flag = 0,
+	};
+	struct ffa_mem_ops_args mem_args = {
+		.attrs = &mem_attr,
+		.use_txbuf = true,
+		.nattrs = 1,
+		.flags = 0,
+	};
+	struct ffa_send_direct_data ffa_data;
+	struct sg_table sgt;
+	u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
+	int rc;
+
+	rc = sg_alloc_table_from_pages(&sgt, pages, num_pages, 0,
+				       num_pages * PAGE_SIZE, GFP_KERNEL);
+	if (rc)
+		return rc;
+
+	mem_args.sg = sgt.sgl;
+	rc = ffa_dev->ops->mem_ops->memory_share(&mem_args);
+	sg_free_table(&sgt);
+	if (rc)
+		return rc;
+
+	shm->sec_world_id = mem_args.g_handle;
+
+	ffa_args[TS_RPC_CTRL_REG] =
+			TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
+						      TS_RPC_OP_RETRIEVE_MEM);
+	ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_LSW] =
+			lower_32_bits(shm->sec_world_id);
+	ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_MSW] =
+			upper_32_bits(shm->sec_world_id);
+	ffa_args[TS_RPC_RETRIEVE_MEM_TAG_LSW] = 0;
+	ffa_args[TS_RPC_RETRIEVE_MEM_TAG_MSW] = 0;
+
+	arg_list_to_ffa_data(ffa_args, &ffa_data);
+	rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
+	if (rc) {
+		(void)ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id,
+							    0);
+		return rc;
+	}
+
+	arg_list_from_ffa_data(&ffa_data, ffa_args);
+
+	if (ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS] != TS_RPC_OK) {
+		pr_err("shm_register rpc status: %d\n",
+		       ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS]);
+		ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int tstee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
+{
+	struct tstee *tstee = tee_get_drvdata(ctx->teedev);
+	struct ffa_device *ffa_dev = tstee->ffa_dev;
+	struct ffa_send_direct_data ffa_data;
+	u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
+	int rc;
+
+	ffa_args[TS_RPC_CTRL_REG] =
+			TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
+						      TS_RPC_OP_RELINQ_MEM);
+	ffa_args[TS_RPC_RELINQ_MEM_HANDLE_LSW] =
+			lower_32_bits(shm->sec_world_id);
+	ffa_args[TS_RPC_RELINQ_MEM_HANDLE_MSW] =
+			upper_32_bits(shm->sec_world_id);
+
+	arg_list_to_ffa_data(ffa_args, &ffa_data);
+	rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
+	if (rc)
+		return rc;
+	arg_list_from_ffa_data(&ffa_data, ffa_args);
+
+	if (ffa_args[TS_RPC_RELINQ_MEM_RPC_STATUS] != TS_RPC_OK) {
+		pr_err("shm_unregister rpc status: %d\n",
+		       ffa_args[TS_RPC_RELINQ_MEM_RPC_STATUS]);
+		return -EINVAL;
+	}
+
+	rc = ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
+
+	return rc;
+}
+
+static const struct tee_driver_ops tstee_ops = {
+	.get_version = tstee_get_version,
+	.open = tstee_open,
+	.release = tstee_release,
+	.open_session = tstee_open_session,
+	.close_session = tstee_close_session,
+	.invoke_func = tstee_invoke_func,
+	.shm_register = tstee_shm_register,
+	.shm_unregister = tstee_shm_unregister,
+};
+
+static const struct tee_desc tstee_desc = {
+	.name = "tstee-clnt",
+	.ops = &tstee_ops,
+	.owner = THIS_MODULE,
+};
+
+static int pool_op_alloc(struct tee_shm_pool *pool, struct tee_shm *shm,
+			 size_t size, size_t align)
+{
+	return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
+					    tstee_shm_register);
+}
+
+static void pool_op_free(struct tee_shm_pool *pool, struct tee_shm *shm)
+{
+	tee_shm_pool_op_free_helper(pool, shm, tstee_shm_unregister);
+}
+
+static void pool_op_destroy_pool(struct tee_shm_pool *pool)
+{
+	kfree(pool);
+}
+
+static const struct tee_shm_pool_ops pool_ops = {
+	.alloc = pool_op_alloc,
+	.free = pool_op_free,
+	.destroy_pool = pool_op_destroy_pool,
+};
+
+static struct tee_shm_pool *tstee_create_shm_pool(void)
+{
+	struct tee_shm_pool *pool = kzalloc(sizeof(*pool), GFP_KERNEL);
+
+	if (!pool)
+		return ERR_PTR(-ENOMEM);
+
+	pool->ops = &pool_ops;
+
+	return pool;
+}
+
+static bool tstee_check_rpc_compatible(struct ffa_device *ffa_dev)
+{
+	struct ffa_send_direct_data ffa_data;
+	u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
+
+	ffa_args[TS_RPC_CTRL_REG] =
+			TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
+						      TS_RPC_OP_GET_VERSION);
+
+	arg_list_to_ffa_data(ffa_args, &ffa_data);
+	if (ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data))
+		return false;
+
+	arg_list_from_ffa_data(&ffa_data, ffa_args);
+
+	return ffa_args[TS_RPC_GET_VERSION_RESP] == TS_RPC_PROTOCOL_VERSION;
+}
+
+static int tstee_probe(struct ffa_device *ffa_dev)
+{
+	struct tstee *tstee;
+	int rc;
+
+	ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
+
+	if (!tstee_check_rpc_compatible(ffa_dev))
+		return -EINVAL;
+
+	tstee = kzalloc(sizeof(*tstee), GFP_KERNEL);
+	if (!tstee)
+		return -ENOMEM;
+
+	tstee->ffa_dev = ffa_dev;
+
+	tstee->pool = tstee_create_shm_pool();
+	if (IS_ERR(tstee->pool)) {
+		rc = PTR_ERR(tstee->pool);
+		tstee->pool = NULL;
+		goto err_free_tstee;
+	}
+
+	tstee->teedev = tee_device_alloc(&tstee_desc, NULL, tstee->pool, tstee);
+	if (IS_ERR(tstee->teedev)) {
+		rc = PTR_ERR(tstee->teedev);
+		tstee->teedev = NULL;
+		goto err_free_pool;
+	}
+
+	rc = tee_device_register(tstee->teedev);
+	if (rc)
+		goto err_unreg_teedev;
+
+	ffa_dev_set_drvdata(ffa_dev, tstee);
+
+	return 0;
+
+err_unreg_teedev:
+	tee_device_unregister(tstee->teedev);
+err_free_pool:
+	tee_shm_pool_free(tstee->pool);
+err_free_tstee:
+	kfree(tstee);
+	return rc;
+}
+
+static void tstee_remove(struct ffa_device *ffa_dev)
+{
+	struct tstee *tstee = ffa_dev->dev.driver_data;
+
+	tee_device_unregister(tstee->teedev);
+	tee_shm_pool_free(tstee->pool);
+	kfree(tstee);
+}
+
+static const struct ffa_device_id tstee_device_ids[] = {
+	/* TS RPC protocol UUID: bdcd76d7-825e-4751-963b-86d4f84943ac */
+	{ TS_RPC_UUID },
+	{}
+};
+
+static struct ffa_driver tstee_driver = {
+	.name = "arm_tstee",
+	.probe = tstee_probe,
+	.remove = tstee_remove,
+	.id_table = tstee_device_ids,
+};
+
+module_ffa_driver(tstee_driver);
+
+MODULE_AUTHOR("Balint Dobszay <balint.dobszay@arm.com>");
+MODULE_DESCRIPTION("Arm Trusted Services TEE driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/tee/tstee/tstee_private.h b/drivers/tee/tstee/tstee_private.h
new file mode 100644
index 000000000000..6d24cf0dbbc2
--- /dev/null
+++ b/drivers/tee/tstee/tstee_private.h
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023, Arm Limited
+ */
+
+#ifndef TSTEE_PRIVATE_H
+#define TSTEE_PRIVATE_H
+
+#include <linux/arm_ffa.h>
+#include <linux/bitops.h>
+#include <linux/tee_drv.h>
+#include <linux/types.h>
+#include <linux/uuid.h>
+#include <linux/xarray.h>
+
+/*
+ * The description of the ABI implemented in this file is available at
+ * https://trusted-services.readthedocs.io/en/v1.0.0/developer/service-access-protocols.html#abi
+ */
+
+/* UUID of this protocol */
+#define TS_RPC_UUID UUID_INIT(0xbdcd76d7, 0x825e, 0x4751, \
+			      0x96, 0x3b, 0x86, 0xd4, 0xf8, 0x49, 0x43, 0xac)
+
+/* Protocol version*/
+#define TS_RPC_PROTOCOL_VERSION		(1)
+
+/* Status codes */
+#define TS_RPC_OK			(0)
+
+/* RPC control register */
+#define TS_RPC_CTRL_REG			(0)
+#define OPCODE_MASK			GENMASK(15, 0)
+#define IFACE_ID_MASK			GENMASK(23, 16)
+#define TS_RPC_CTRL_OPCODE(x)		((u16)(FIELD_GET(OPCODE_MASK, (x))))
+#define TS_RPC_CTRL_IFACE_ID(x)		((u8)(FIELD_GET(IFACE_ID_MASK, (x))))
+#define TS_RPC_CTRL_PACK_IFACE_OPCODE(i, o)	\
+	(FIELD_PREP(IFACE_ID_MASK, (i)) | FIELD_PREP(OPCODE_MASK, (o)))
+#define TS_RPC_CTRL_SAP_RC		BIT(30)
+#define TS_RPC_CTRL_SAP_ERR		BIT(31)
+
+/* Interface ID for RPC management operations */
+#define TS_RPC_MGMT_IFACE_ID		(0xff)
+
+/* Management calls */
+#define TS_RPC_OP_GET_VERSION		(0x0000)
+#define TS_RPC_GET_VERSION_RESP		(1)
+
+#define TS_RPC_OP_RETRIEVE_MEM		(0x0001)
+#define TS_RPC_RETRIEVE_MEM_HANDLE_LSW	(1)
+#define TS_RPC_RETRIEVE_MEM_HANDLE_MSW	(2)
+#define TS_RPC_RETRIEVE_MEM_TAG_LSW	(3)
+#define TS_RPC_RETRIEVE_MEM_TAG_MSW	(4)
+#define TS_RPC_RETRIEVE_MEM_RPC_STATUS	(1)
+
+#define TS_RPC_OP_RELINQ_MEM		(0x0002)
+#define TS_RPC_RELINQ_MEM_HANDLE_LSW	(1)
+#define TS_RPC_RELINQ_MEM_HANDLE_MSW	(2)
+#define TS_RPC_RELINQ_MEM_RPC_STATUS	(1)
+
+#define TS_RPC_OP_SERVICE_INFO		(0x0003)
+#define TS_RPC_SERVICE_INFO_UUID0	(1)
+#define TS_RPC_SERVICE_INFO_UUID1	(2)
+#define TS_RPC_SERVICE_INFO_UUID2	(3)
+#define TS_RPC_SERVICE_INFO_UUID3	(4)
+#define TS_RPC_SERVICE_INFO_RPC_STATUS	(1)
+#define TS_RPC_SERVICE_INFO_IFACE	(2)
+
+/* Service call */
+#define TS_RPC_SERVICE_MEM_HANDLE_LSW	(1)
+#define TS_RPC_SERVICE_MEM_HANDLE_MSW	(2)
+#define TS_RPC_SERVICE_REQ_LEN		(3)
+#define TS_RPC_SERVICE_CLIENT_ID	(4)
+#define TS_RPC_SERVICE_RPC_STATUS	(1)
+#define TS_RPC_SERVICE_STATUS		(2)
+#define TS_RPC_SERVICE_RESP_LEN		(3)
+
+struct tstee {
+	struct ffa_device *ffa_dev;
+	struct tee_device *teedev;
+	struct tee_shm_pool *pool;
+};
+
+struct ts_session {
+	u8 iface_id;
+};
+
+struct ts_context_data {
+	struct xarray sess_list;
+	/* Serializes access to the session list */
+	struct mutex mutex;
+};
+
+#endif /* TSTEE_PRIVATE_H */
diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
index 23e57164693c..d0430bee8292 100644
--- a/include/uapi/linux/tee.h
+++ b/include/uapi/linux/tee.h
@@ -56,6 +56,7 @@
  */
 #define TEE_IMPL_ID_OPTEE	1
 #define TEE_IMPL_ID_AMDTEE	2
+#define TEE_IMPL_ID_TSTEE	3
 
 /*
  * OP-TEE specific capabilities
-- 
2.34.1

Re: [PATCH v2 2/3] tee: tstee: Add Trusted Services TEE driver

[Sumit Garg]

On Fri, 23 Feb 2024 at 15:22, Balint Dobszay <balint.dobszay@arm.com> wrote:
>
> The Trusted Services project provides a framework for developing and
> deploying device Root of Trust services in FF-A Secure Partitions. The
> FF-A SPs are accessible through the FF-A driver, but this doesn't
> provide a user space interface. The goal of this TEE driver is to make
> Trusted Services SPs accessible for user space clients.
>
> All TS SPs have the same FF-A UUID, it identifies the RPC protocol used
> by TS. A TS SP can host one or more services, a service is identified by
> its service UUID. The same type of service cannot be present twice in
> the same SP. During SP boot each service in an SP is assigned an
> interface ID, this is just a short ID to simplify message addressing.
> There is 1:1 mapping between TS SPs and TEE devices, i.e. a separate TEE
> device is registered for each TS SP. This is required since contrary to
> the generic TEE design where memory is shared with the whole TEE
> implementation, in case of FF-A, memory is shared with a specific SP. A
> user space client has to be able to separately share memory with each SP
> based on its endpoint ID.
>
> Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
> ---
>  drivers/tee/Kconfig               |   1 +
>  drivers/tee/Makefile              |   1 +
>  drivers/tee/tstee/Kconfig         |  11 +
>  drivers/tee/tstee/Makefile        |   3 +
>  drivers/tee/tstee/core.c          | 490 ++++++++++++++++++++++++++++++
>  drivers/tee/tstee/tstee_private.h |  94 ++++++
>  include/uapi/linux/tee.h          |   1 +
>  7 files changed, 601 insertions(+)
>  create mode 100644 drivers/tee/tstee/Kconfig
>  create mode 100644 drivers/tee/tstee/Makefile
>  create mode 100644 drivers/tee/tstee/core.c
>  create mode 100644 drivers/tee/tstee/tstee_private.h
>
> diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
> index 73a147202e88..61b507c18780 100644
> --- a/drivers/tee/Kconfig
> +++ b/drivers/tee/Kconfig
> @@ -15,5 +15,6 @@ if TEE
>
>  source "drivers/tee/optee/Kconfig"
>  source "drivers/tee/amdtee/Kconfig"
> +source "drivers/tee/tstee/Kconfig"
>
>  endif
> diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
> index 68da044afbfa..5488cba30bd2 100644
> --- a/drivers/tee/Makefile
> +++ b/drivers/tee/Makefile
> @@ -5,3 +5,4 @@ tee-objs += tee_shm.o
>  tee-objs += tee_shm_pool.o
>  obj-$(CONFIG_OPTEE) += optee/
>  obj-$(CONFIG_AMDTEE) += amdtee/
> +obj-$(CONFIG_ARM_TSTEE) += tstee/
> diff --git a/drivers/tee/tstee/Kconfig b/drivers/tee/tstee/Kconfig
> new file mode 100644
> index 000000000000..d32f91d47398
> --- /dev/null
> +++ b/drivers/tee/tstee/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config ARM_TSTEE
> +       tristate "Arm Trusted Services TEE driver"
> +       depends on ARM_FFA_TRANSPORT
> +       default n
> +       help
> +         The Trusted Services project provides a framework for developing and
> +         deploying device Root of Trust services in FF-A Secure Partitions.
> +         This driver provides an interface to make Trusted Services Secure
> +         Partitions accessible for user space clients, since the FF-A driver
> +         doesn't implement a user space interface directly.
> diff --git a/drivers/tee/tstee/Makefile b/drivers/tee/tstee/Makefile
> new file mode 100644
> index 000000000000..5227020ebd30
> --- /dev/null
> +++ b/drivers/tee/tstee/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +arm-tstee-objs := core.o
> +obj-$(CONFIG_ARM_TSTEE) = arm-tstee.o
> diff --git a/drivers/tee/tstee/core.c b/drivers/tee/tstee/core.c
> new file mode 100644
> index 000000000000..2932be017dbe
> --- /dev/null
> +++ b/drivers/tee/tstee/core.c
> @@ -0,0 +1,490 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023, Arm Limited
> + */
> +
> +#define DRIVER_NAME "Arm TSTEE"
> +#define pr_fmt(fmt) DRIVER_NAME ": " fmt
> +
> +#include <linux/arm_ffa.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/limits.h>
> +#include <linux/list.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/scatterlist.h>
> +#include <linux/slab.h>
> +#include <linux/stat.h>
> +#include <linux/tee_drv.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +
> +#include "tstee_private.h"
> +
> +#define FFA_DIRECT_REQ_ARG_NUM 5
> +#define FFA_INVALID_MEM_HANDLE U64_MAX
> +
> +static void arg_list_to_ffa_data(const u32 *args,
> +                                struct ffa_send_direct_data *data)
> +{
> +       data->data0 = args[0];
> +       data->data1 = args[1];
> +       data->data2 = args[2];
> +       data->data3 = args[3];
> +       data->data4 = args[4];
> +}
> +
> +static void arg_list_from_ffa_data(const struct ffa_send_direct_data *data,
> +                                  u32 *args)
> +{
> +       args[0] = lower_32_bits(data->data0);
> +       args[1] = lower_32_bits(data->data1);
> +       args[2] = lower_32_bits(data->data2);
> +       args[3] = lower_32_bits(data->data3);
> +       args[4] = lower_32_bits(data->data4);
> +}
> +
> +static void tstee_get_version(struct tee_device *teedev,
> +                             struct tee_ioctl_version_data *vers)
> +{
> +       struct tstee *tstee = tee_get_drvdata(teedev);
> +       struct tee_ioctl_version_data v = {
> +               .impl_id = TEE_IMPL_ID_TSTEE,
> +               /* FF-A endpoint ID only uses the lower 16 bits */
> +               .impl_caps = lower_16_bits(tstee->ffa_dev->vm_id),
> +               .gen_caps = 0,
> +       };
> +
> +       *vers = v;
> +}
> +
> +static int tstee_open(struct tee_context *ctx)
> +{
> +       struct ts_context_data *ctxdata;
> +
> +       ctxdata = kzalloc(sizeof(*ctxdata), GFP_KERNEL);
> +       if (!ctxdata)
> +               return -ENOMEM;
> +
> +       mutex_init(&ctxdata->mutex);
> +       xa_init_flags(&ctxdata->sess_list, XA_FLAGS_ALLOC);
> +
> +       ctx->data = ctxdata;
> +
> +       return 0;
> +}
> +
> +static void tstee_release(struct tee_context *ctx)
> +{
> +       struct ts_context_data *ctxdata = ctx->data;
> +       struct ts_session *sess;
> +       unsigned long idx;
> +
> +       if (!ctxdata)
> +               return;
> +
> +       xa_for_each(&ctxdata->sess_list, idx, sess) {
> +               xa_erase(&ctxdata->sess_list, idx);
> +               kfree(sess);
> +       }
> +
> +       xa_destroy(&ctxdata->sess_list);
> +       mutex_destroy(&ctxdata->mutex);
> +
> +       kfree(ctxdata);
> +       ctx->data = NULL;
> +}
> +
> +static int tstee_open_session(struct tee_context *ctx,
> +                             struct tee_ioctl_open_session_arg *arg,
> +                             struct tee_param *param __always_unused)
> +{
> +       struct tstee *tstee = tee_get_drvdata(ctx->teedev);
> +       struct ffa_device *ffa_dev = tstee->ffa_dev;
> +       struct ts_context_data *ctxdata = ctx->data;
> +       struct ffa_send_direct_data ffa_data;
> +       struct ts_session *sess = NULL;
> +       u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
> +       u32 sess_id;
> +       int rc;
> +
> +       ffa_args[TS_RPC_CTRL_REG] =
> +               TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
> +                                             TS_RPC_OP_SERVICE_INFO);
> +
> +       memcpy(ffa_args + TS_RPC_SERVICE_INFO_UUID0, arg->uuid, UUID_SIZE);
> +
> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
> +       rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
> +       if (rc)
> +               return rc;
> +
> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
> +
> +       if (ffa_args[TS_RPC_SERVICE_INFO_RPC_STATUS] != TS_RPC_OK)
> +               return -ENODEV;
> +
> +       if (ffa_args[TS_RPC_SERVICE_INFO_IFACE] > U8_MAX)
> +               return -EINVAL;
> +
> +       sess = kzalloc(sizeof(*sess), GFP_KERNEL);
> +       if (!sess)
> +               return -ENOMEM;
> +
> +       sess->iface_id = ffa_args[TS_RPC_SERVICE_INFO_IFACE];
> +
> +       rc = xa_alloc(&ctxdata->sess_list, &sess_id, sess, xa_limit_32b,
> +                     GFP_KERNEL);
> +       if (rc) {
> +               kfree(sess);
> +               return rc;
> +       }
> +
> +       arg->session = sess_id;
> +       arg->ret = 0;
> +
> +       return 0;
> +}
> +
> +static int tstee_close_session(struct tee_context *ctx, u32 session)
> +{
> +       struct ts_context_data *ctxdata = ctx->data;
> +       struct ts_session *sess;
> +
> +       mutex_lock(&ctxdata->mutex);
> +       sess = xa_erase(&ctxdata->sess_list, session);
> +       mutex_unlock(&ctxdata->mutex);
> +       if (!sess)
> +               return -EINVAL;
> +
> +       kfree(sess);
> +
> +       return 0;
> +}
> +
> +static int tstee_invoke_func(struct tee_context *ctx,
> +                            struct tee_ioctl_invoke_arg *arg,
> +                            struct tee_param *param)
> +{
> +       struct tstee *tstee = tee_get_drvdata(ctx->teedev);
> +       struct ffa_device *ffa_dev = tstee->ffa_dev;
> +       struct ts_context_data *ctxdata = ctx->data;
> +       struct ffa_send_direct_data ffa_data;
> +       struct tee_shm *shm = NULL;
> +       struct ts_session *sess;
> +       u32 req_len, ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
> +       int shm_id, rc;
> +       u8 iface_id;
> +       u64 handle;
> +       u16 opcode;
> +
> +       mutex_lock(&ctxdata->mutex);
> +       sess = xa_load(&ctxdata->sess_list, arg->session);
> +
> +       /*
> +        * Do this while holding the mutex to make sure that the session wasn't
> +        * closed meanwhile
> +        */
> +       if (sess)
> +               iface_id = sess->iface_id;
> +
> +       mutex_unlock(&ctxdata->mutex);
> +       if (!sess)
> +               return -EINVAL;
> +

We can drop usage of mutex here and other places via reusing
xa_lock(), something like below should work:

        xa_lock(&ctxdata->sess_list);

        sess = xa_load(&ctxdata->sess_list, arg->session);
        if (sess)
                iface_id = sess->iface_id;

        xa_unlock(&ctxdata->sess_list);

-Sumit

> +       opcode = lower_16_bits(arg->func);
> +       shm_id = lower_32_bits(param[0].u.value.a);
> +       req_len = lower_32_bits(param[0].u.value.b);
> +
> +       if (shm_id != 0) {
> +               shm = tee_shm_get_from_id(ctx, shm_id);
> +               if (IS_ERR(shm))
> +                       return PTR_ERR(shm);
> +
> +               if (shm->size < req_len) {
> +                       pr_err("request doesn't fit into shared memory buffer\n");
> +                       rc = -EINVAL;
> +                       goto out;
> +               }
> +
> +               handle = shm->sec_world_id;
> +       } else {
> +               handle = FFA_INVALID_MEM_HANDLE;
> +       }
> +
> +       ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(iface_id,
> +                                                                 opcode);
> +       ffa_args[TS_RPC_SERVICE_MEM_HANDLE_LSW] = lower_32_bits(handle);
> +       ffa_args[TS_RPC_SERVICE_MEM_HANDLE_MSW] = upper_32_bits(handle);
> +       ffa_args[TS_RPC_SERVICE_REQ_LEN] = req_len;
> +       ffa_args[TS_RPC_SERVICE_CLIENT_ID] = 0;
> +
> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
> +       rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
> +       if (rc)
> +               goto out;
> +
> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
> +
> +       if (ffa_args[TS_RPC_SERVICE_RPC_STATUS] != TS_RPC_OK) {
> +               pr_err("invoke_func rpc status: %d\n",
> +                      ffa_args[TS_RPC_SERVICE_RPC_STATUS]);
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       arg->ret = ffa_args[TS_RPC_SERVICE_STATUS];
> +       if (shm && shm->size >= ffa_args[TS_RPC_SERVICE_RESP_LEN])
> +               param[0].u.value.a = ffa_args[TS_RPC_SERVICE_RESP_LEN];
> +
> +out:
> +       if (shm)
> +               tee_shm_put(shm);
> +
> +       return rc;
> +}
> +
> +static int tstee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
> +                             struct page **pages, size_t num_pages,
> +                             unsigned long start __always_unused)
> +{
> +       struct tstee *tstee = tee_get_drvdata(ctx->teedev);
> +       struct ffa_device *ffa_dev = tstee->ffa_dev;
> +       struct ffa_mem_region_attributes mem_attr = {
> +               .receiver = tstee->ffa_dev->vm_id,
> +               .attrs = FFA_MEM_RW,
> +               .flag = 0,
> +       };
> +       struct ffa_mem_ops_args mem_args = {
> +               .attrs = &mem_attr,
> +               .use_txbuf = true,
> +               .nattrs = 1,
> +               .flags = 0,
> +       };
> +       struct ffa_send_direct_data ffa_data;
> +       struct sg_table sgt;
> +       u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
> +       int rc;
> +
> +       rc = sg_alloc_table_from_pages(&sgt, pages, num_pages, 0,
> +                                      num_pages * PAGE_SIZE, GFP_KERNEL);
> +       if (rc)
> +               return rc;
> +
> +       mem_args.sg = sgt.sgl;
> +       rc = ffa_dev->ops->mem_ops->memory_share(&mem_args);
> +       sg_free_table(&sgt);
> +       if (rc)
> +               return rc;
> +
> +       shm->sec_world_id = mem_args.g_handle;
> +
> +       ffa_args[TS_RPC_CTRL_REG] =
> +                       TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
> +                                                     TS_RPC_OP_RETRIEVE_MEM);
> +       ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_LSW] =
> +                       lower_32_bits(shm->sec_world_id);
> +       ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_MSW] =
> +                       upper_32_bits(shm->sec_world_id);
> +       ffa_args[TS_RPC_RETRIEVE_MEM_TAG_LSW] = 0;
> +       ffa_args[TS_RPC_RETRIEVE_MEM_TAG_MSW] = 0;
> +
> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
> +       rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
> +       if (rc) {
> +               (void)ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id,
> +                                                           0);
> +               return rc;
> +       }
> +
> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
> +
> +       if (ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS] != TS_RPC_OK) {
> +               pr_err("shm_register rpc status: %d\n",
> +                      ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS]);
> +               ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static int tstee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
> +{
> +       struct tstee *tstee = tee_get_drvdata(ctx->teedev);
> +       struct ffa_device *ffa_dev = tstee->ffa_dev;
> +       struct ffa_send_direct_data ffa_data;
> +       u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
> +       int rc;
> +
> +       ffa_args[TS_RPC_CTRL_REG] =
> +                       TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
> +                                                     TS_RPC_OP_RELINQ_MEM);
> +       ffa_args[TS_RPC_RELINQ_MEM_HANDLE_LSW] =
> +                       lower_32_bits(shm->sec_world_id);
> +       ffa_args[TS_RPC_RELINQ_MEM_HANDLE_MSW] =
> +                       upper_32_bits(shm->sec_world_id);
> +
> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
> +       rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
> +       if (rc)
> +               return rc;
> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
> +
> +       if (ffa_args[TS_RPC_RELINQ_MEM_RPC_STATUS] != TS_RPC_OK) {
> +               pr_err("shm_unregister rpc status: %d\n",
> +                      ffa_args[TS_RPC_RELINQ_MEM_RPC_STATUS]);
> +               return -EINVAL;
> +       }
> +
> +       rc = ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
> +
> +       return rc;
> +}
> +
> +static const struct tee_driver_ops tstee_ops = {
> +       .get_version = tstee_get_version,
> +       .open = tstee_open,
> +       .release = tstee_release,
> +       .open_session = tstee_open_session,
> +       .close_session = tstee_close_session,
> +       .invoke_func = tstee_invoke_func,
> +       .shm_register = tstee_shm_register,
> +       .shm_unregister = tstee_shm_unregister,
> +};
> +
> +static const struct tee_desc tstee_desc = {
> +       .name = "tstee-clnt",
> +       .ops = &tstee_ops,
> +       .owner = THIS_MODULE,
> +};
> +
> +static int pool_op_alloc(struct tee_shm_pool *pool, struct tee_shm *shm,
> +                        size_t size, size_t align)
> +{
> +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
> +                                           tstee_shm_register);
> +}
> +
> +static void pool_op_free(struct tee_shm_pool *pool, struct tee_shm *shm)
> +{
> +       tee_shm_pool_op_free_helper(pool, shm, tstee_shm_unregister);
> +}
> +
> +static void pool_op_destroy_pool(struct tee_shm_pool *pool)
> +{
> +       kfree(pool);
> +}
> +
> +static const struct tee_shm_pool_ops pool_ops = {
> +       .alloc = pool_op_alloc,
> +       .free = pool_op_free,
> +       .destroy_pool = pool_op_destroy_pool,
> +};
> +
> +static struct tee_shm_pool *tstee_create_shm_pool(void)
> +{
> +       struct tee_shm_pool *pool = kzalloc(sizeof(*pool), GFP_KERNEL);
> +
> +       if (!pool)
> +               return ERR_PTR(-ENOMEM);
> +
> +       pool->ops = &pool_ops;
> +
> +       return pool;
> +}
> +
> +static bool tstee_check_rpc_compatible(struct ffa_device *ffa_dev)
> +{
> +       struct ffa_send_direct_data ffa_data;
> +       u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
> +
> +       ffa_args[TS_RPC_CTRL_REG] =
> +                       TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
> +                                                     TS_RPC_OP_GET_VERSION);
> +
> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
> +       if (ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data))
> +               return false;
> +
> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
> +
> +       return ffa_args[TS_RPC_GET_VERSION_RESP] == TS_RPC_PROTOCOL_VERSION;
> +}
> +
> +static int tstee_probe(struct ffa_device *ffa_dev)
> +{
> +       struct tstee *tstee;
> +       int rc;
> +
> +       ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
> +
> +       if (!tstee_check_rpc_compatible(ffa_dev))
> +               return -EINVAL;
> +
> +       tstee = kzalloc(sizeof(*tstee), GFP_KERNEL);
> +       if (!tstee)
> +               return -ENOMEM;
> +
> +       tstee->ffa_dev = ffa_dev;
> +
> +       tstee->pool = tstee_create_shm_pool();
> +       if (IS_ERR(tstee->pool)) {
> +               rc = PTR_ERR(tstee->pool);
> +               tstee->pool = NULL;
> +               goto err_free_tstee;
> +       }
> +
> +       tstee->teedev = tee_device_alloc(&tstee_desc, NULL, tstee->pool, tstee);
> +       if (IS_ERR(tstee->teedev)) {
> +               rc = PTR_ERR(tstee->teedev);
> +               tstee->teedev = NULL;
> +               goto err_free_pool;
> +       }
> +
> +       rc = tee_device_register(tstee->teedev);
> +       if (rc)
> +               goto err_unreg_teedev;
> +
> +       ffa_dev_set_drvdata(ffa_dev, tstee);
> +
> +       return 0;
> +
> +err_unreg_teedev:
> +       tee_device_unregister(tstee->teedev);
> +err_free_pool:
> +       tee_shm_pool_free(tstee->pool);
> +err_free_tstee:
> +       kfree(tstee);
> +       return rc;
> +}
> +
> +static void tstee_remove(struct ffa_device *ffa_dev)
> +{
> +       struct tstee *tstee = ffa_dev->dev.driver_data;
> +
> +       tee_device_unregister(tstee->teedev);
> +       tee_shm_pool_free(tstee->pool);
> +       kfree(tstee);
> +}
> +
> +static const struct ffa_device_id tstee_device_ids[] = {
> +       /* TS RPC protocol UUID: bdcd76d7-825e-4751-963b-86d4f84943ac */
> +       { TS_RPC_UUID },
> +       {}
> +};
> +
> +static struct ffa_driver tstee_driver = {
> +       .name = "arm_tstee",
> +       .probe = tstee_probe,
> +       .remove = tstee_remove,
> +       .id_table = tstee_device_ids,
> +};
> +
> +module_ffa_driver(tstee_driver);
> +
> +MODULE_AUTHOR("Balint Dobszay <balint.dobszay@arm.com>");
> +MODULE_DESCRIPTION("Arm Trusted Services TEE driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/tee/tstee/tstee_private.h b/drivers/tee/tstee/tstee_private.h
> new file mode 100644
> index 000000000000..6d24cf0dbbc2
> --- /dev/null
> +++ b/drivers/tee/tstee/tstee_private.h
> @@ -0,0 +1,94 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2023, Arm Limited
> + */
> +
> +#ifndef TSTEE_PRIVATE_H
> +#define TSTEE_PRIVATE_H
> +
> +#include <linux/arm_ffa.h>
> +#include <linux/bitops.h>
> +#include <linux/tee_drv.h>
> +#include <linux/types.h>
> +#include <linux/uuid.h>
> +#include <linux/xarray.h>
> +
> +/*
> + * The description of the ABI implemented in this file is available at
> + * https://trusted-services.readthedocs.io/en/v1.0.0/developer/service-access-protocols.html#abi
> + */
> +
> +/* UUID of this protocol */
> +#define TS_RPC_UUID UUID_INIT(0xbdcd76d7, 0x825e, 0x4751, \
> +                             0x96, 0x3b, 0x86, 0xd4, 0xf8, 0x49, 0x43, 0xac)
> +
> +/* Protocol version*/
> +#define TS_RPC_PROTOCOL_VERSION                (1)
> +
> +/* Status codes */
> +#define TS_RPC_OK                      (0)
> +
> +/* RPC control register */
> +#define TS_RPC_CTRL_REG                        (0)
> +#define OPCODE_MASK                    GENMASK(15, 0)
> +#define IFACE_ID_MASK                  GENMASK(23, 16)
> +#define TS_RPC_CTRL_OPCODE(x)          ((u16)(FIELD_GET(OPCODE_MASK, (x))))
> +#define TS_RPC_CTRL_IFACE_ID(x)                ((u8)(FIELD_GET(IFACE_ID_MASK, (x))))
> +#define TS_RPC_CTRL_PACK_IFACE_OPCODE(i, o)    \
> +       (FIELD_PREP(IFACE_ID_MASK, (i)) | FIELD_PREP(OPCODE_MASK, (o)))
> +#define TS_RPC_CTRL_SAP_RC             BIT(30)
> +#define TS_RPC_CTRL_SAP_ERR            BIT(31)
> +
> +/* Interface ID for RPC management operations */
> +#define TS_RPC_MGMT_IFACE_ID           (0xff)
> +
> +/* Management calls */
> +#define TS_RPC_OP_GET_VERSION          (0x0000)
> +#define TS_RPC_GET_VERSION_RESP                (1)
> +
> +#define TS_RPC_OP_RETRIEVE_MEM         (0x0001)
> +#define TS_RPC_RETRIEVE_MEM_HANDLE_LSW (1)
> +#define TS_RPC_RETRIEVE_MEM_HANDLE_MSW (2)
> +#define TS_RPC_RETRIEVE_MEM_TAG_LSW    (3)
> +#define TS_RPC_RETRIEVE_MEM_TAG_MSW    (4)
> +#define TS_RPC_RETRIEVE_MEM_RPC_STATUS (1)
> +
> +#define TS_RPC_OP_RELINQ_MEM           (0x0002)
> +#define TS_RPC_RELINQ_MEM_HANDLE_LSW   (1)
> +#define TS_RPC_RELINQ_MEM_HANDLE_MSW   (2)
> +#define TS_RPC_RELINQ_MEM_RPC_STATUS   (1)
> +
> +#define TS_RPC_OP_SERVICE_INFO         (0x0003)
> +#define TS_RPC_SERVICE_INFO_UUID0      (1)
> +#define TS_RPC_SERVICE_INFO_UUID1      (2)
> +#define TS_RPC_SERVICE_INFO_UUID2      (3)
> +#define TS_RPC_SERVICE_INFO_UUID3      (4)
> +#define TS_RPC_SERVICE_INFO_RPC_STATUS (1)
> +#define TS_RPC_SERVICE_INFO_IFACE      (2)
> +
> +/* Service call */
> +#define TS_RPC_SERVICE_MEM_HANDLE_LSW  (1)
> +#define TS_RPC_SERVICE_MEM_HANDLE_MSW  (2)
> +#define TS_RPC_SERVICE_REQ_LEN         (3)
> +#define TS_RPC_SERVICE_CLIENT_ID       (4)
> +#define TS_RPC_SERVICE_RPC_STATUS      (1)
> +#define TS_RPC_SERVICE_STATUS          (2)
> +#define TS_RPC_SERVICE_RESP_LEN                (3)
> +
> +struct tstee {
> +       struct ffa_device *ffa_dev;
> +       struct tee_device *teedev;
> +       struct tee_shm_pool *pool;
> +};
> +
> +struct ts_session {
> +       u8 iface_id;
> +};
> +
> +struct ts_context_data {
> +       struct xarray sess_list;
> +       /* Serializes access to the session list */
> +       struct mutex mutex;
> +};
> +
> +#endif /* TSTEE_PRIVATE_H */
> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
> index 23e57164693c..d0430bee8292 100644
> --- a/include/uapi/linux/tee.h
> +++ b/include/uapi/linux/tee.h
> @@ -56,6 +56,7 @@
>   */
>  #define TEE_IMPL_ID_OPTEE      1
>  #define TEE_IMPL_ID_AMDTEE     2
> +#define TEE_IMPL_ID_TSTEE      3
>
>  /*
>   * OP-TEE specific capabilities
> --
> 2.34.1
>

Re: [PATCH v2 2/3] tee: tstee: Add Trusted Services TEE driver

[Balint Dobszay]

On 27 Feb 2024, at 8:08, Sumit Garg wrote:

> On Fri, 23 Feb 2024 at 15:22, Balint Dobszay <balint.dobszay@arm.com> wrote:
>>
>> The Trusted Services project provides a framework for developing and
>> deploying device Root of Trust services in FF-A Secure Partitions. The
>> FF-A SPs are accessible through the FF-A driver, but this doesn't
>> provide a user space interface. The goal of this TEE driver is to make
>> Trusted Services SPs accessible for user space clients.
>>
>> All TS SPs have the same FF-A UUID, it identifies the RPC protocol used
>> by TS. A TS SP can host one or more services, a service is identified by
>> its service UUID. The same type of service cannot be present twice in
>> the same SP. During SP boot each service in an SP is assigned an
>> interface ID, this is just a short ID to simplify message addressing.
>> There is 1:1 mapping between TS SPs and TEE devices, i.e. a separate TEE
>> device is registered for each TS SP. This is required since contrary to
>> the generic TEE design where memory is shared with the whole TEE
>> implementation, in case of FF-A, memory is shared with a specific SP. A
>> user space client has to be able to separately share memory with each SP
>> based on its endpoint ID.
>>
>> Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
>> ---
>>  drivers/tee/Kconfig               |   1 +
>>  drivers/tee/Makefile              |   1 +
>>  drivers/tee/tstee/Kconfig         |  11 +
>>  drivers/tee/tstee/Makefile        |   3 +
>>  drivers/tee/tstee/core.c          | 490 ++++++++++++++++++++++++++++++
>>  drivers/tee/tstee/tstee_private.h |  94 ++++++
>>  include/uapi/linux/tee.h          |   1 +
>>  7 files changed, 601 insertions(+)
>>  create mode 100644 drivers/tee/tstee/Kconfig
>>  create mode 100644 drivers/tee/tstee/Makefile
>>  create mode 100644 drivers/tee/tstee/core.c
>>  create mode 100644 drivers/tee/tstee/tstee_private.h
>>
>> diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
>> index 73a147202e88..61b507c18780 100644
>> --- a/drivers/tee/Kconfig
>> +++ b/drivers/tee/Kconfig
>> @@ -15,5 +15,6 @@ if TEE
>>
>>  source "drivers/tee/optee/Kconfig"
>>  source "drivers/tee/amdtee/Kconfig"
>> +source "drivers/tee/tstee/Kconfig"
>>
>>  endif
>> diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
>> index 68da044afbfa..5488cba30bd2 100644
>> --- a/drivers/tee/Makefile
>> +++ b/drivers/tee/Makefile
>> @@ -5,3 +5,4 @@ tee-objs += tee_shm.o
>>  tee-objs += tee_shm_pool.o
>>  obj-$(CONFIG_OPTEE) += optee/
>>  obj-$(CONFIG_AMDTEE) += amdtee/
>> +obj-$(CONFIG_ARM_TSTEE) += tstee/
>> diff --git a/drivers/tee/tstee/Kconfig b/drivers/tee/tstee/Kconfig
>> new file mode 100644
>> index 000000000000..d32f91d47398
>> --- /dev/null
>> +++ b/drivers/tee/tstee/Kconfig
>> @@ -0,0 +1,11 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +config ARM_TSTEE
>> +       tristate "Arm Trusted Services TEE driver"
>> +       depends on ARM_FFA_TRANSPORT
>> +       default n
>> +       help
>> +         The Trusted Services project provides a framework for developing and
>> +         deploying device Root of Trust services in FF-A Secure Partitions.
>> +         This driver provides an interface to make Trusted Services Secure
>> +         Partitions accessible for user space clients, since the FF-A driver
>> +         doesn't implement a user space interface directly.
>> diff --git a/drivers/tee/tstee/Makefile b/drivers/tee/tstee/Makefile
>> new file mode 100644
>> index 000000000000..5227020ebd30
>> --- /dev/null
>> +++ b/drivers/tee/tstee/Makefile
>> @@ -0,0 +1,3 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +arm-tstee-objs := core.o
>> +obj-$(CONFIG_ARM_TSTEE) = arm-tstee.o
>> diff --git a/drivers/tee/tstee/core.c b/drivers/tee/tstee/core.c
>> new file mode 100644
>> index 000000000000..2932be017dbe
>> --- /dev/null
>> +++ b/drivers/tee/tstee/core.c
>> @@ -0,0 +1,490 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2023, Arm Limited
>> + */
>> +
>> +#define DRIVER_NAME "Arm TSTEE"
>> +#define pr_fmt(fmt) DRIVER_NAME ": " fmt
>> +
>> +#include <linux/arm_ffa.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/limits.h>
>> +#include <linux/list.h>
>> +#include <linux/mm.h>
>> +#include <linux/module.h>
>> +#include <linux/scatterlist.h>
>> +#include <linux/slab.h>
>> +#include <linux/stat.h>
>> +#include <linux/tee_drv.h>
>> +#include <linux/types.h>
>> +#include <linux/uaccess.h>
>> +
>> +#include "tstee_private.h"
>> +
>> +#define FFA_DIRECT_REQ_ARG_NUM 5
>> +#define FFA_INVALID_MEM_HANDLE U64_MAX
>> +
>> +static void arg_list_to_ffa_data(const u32 *args,
>> +                                struct ffa_send_direct_data *data)
>> +{
>> +       data->data0 = args[0];
>> +       data->data1 = args[1];
>> +       data->data2 = args[2];
>> +       data->data3 = args[3];
>> +       data->data4 = args[4];
>> +}
>> +
>> +static void arg_list_from_ffa_data(const struct ffa_send_direct_data *data,
>> +                                  u32 *args)
>> +{
>> +       args[0] = lower_32_bits(data->data0);
>> +       args[1] = lower_32_bits(data->data1);
>> +       args[2] = lower_32_bits(data->data2);
>> +       args[3] = lower_32_bits(data->data3);
>> +       args[4] = lower_32_bits(data->data4);
>> +}
>> +
>> +static void tstee_get_version(struct tee_device *teedev,
>> +                             struct tee_ioctl_version_data *vers)
>> +{
>> +       struct tstee *tstee = tee_get_drvdata(teedev);
>> +       struct tee_ioctl_version_data v = {
>> +               .impl_id = TEE_IMPL_ID_TSTEE,
>> +               /* FF-A endpoint ID only uses the lower 16 bits */
>> +               .impl_caps = lower_16_bits(tstee->ffa_dev->vm_id),
>> +               .gen_caps = 0,
>> +       };
>> +
>> +       *vers = v;
>> +}
>> +
>> +static int tstee_open(struct tee_context *ctx)
>> +{
>> +       struct ts_context_data *ctxdata;
>> +
>> +       ctxdata = kzalloc(sizeof(*ctxdata), GFP_KERNEL);
>> +       if (!ctxdata)
>> +               return -ENOMEM;
>> +
>> +       mutex_init(&ctxdata->mutex);
>> +       xa_init_flags(&ctxdata->sess_list, XA_FLAGS_ALLOC);
>> +
>> +       ctx->data = ctxdata;
>> +
>> +       return 0;
>> +}
>> +
>> +static void tstee_release(struct tee_context *ctx)
>> +{
>> +       struct ts_context_data *ctxdata = ctx->data;
>> +       struct ts_session *sess;
>> +       unsigned long idx;
>> +
>> +       if (!ctxdata)
>> +               return;
>> +
>> +       xa_for_each(&ctxdata->sess_list, idx, sess) {
>> +               xa_erase(&ctxdata->sess_list, idx);
>> +               kfree(sess);
>> +       }
>> +
>> +       xa_destroy(&ctxdata->sess_list);
>> +       mutex_destroy(&ctxdata->mutex);
>> +
>> +       kfree(ctxdata);
>> +       ctx->data = NULL;
>> +}
>> +
>> +static int tstee_open_session(struct tee_context *ctx,
>> +                             struct tee_ioctl_open_session_arg *arg,
>> +                             struct tee_param *param __always_unused)
>> +{
>> +       struct tstee *tstee = tee_get_drvdata(ctx->teedev);
>> +       struct ffa_device *ffa_dev = tstee->ffa_dev;
>> +       struct ts_context_data *ctxdata = ctx->data;
>> +       struct ffa_send_direct_data ffa_data;
>> +       struct ts_session *sess = NULL;
>> +       u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
>> +       u32 sess_id;
>> +       int rc;
>> +
>> +       ffa_args[TS_RPC_CTRL_REG] =
>> +               TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
>> +                                             TS_RPC_OP_SERVICE_INFO);
>> +
>> +       memcpy(ffa_args + TS_RPC_SERVICE_INFO_UUID0, arg->uuid, UUID_SIZE);
>> +
>> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
>> +       rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
>> +       if (rc)
>> +               return rc;
>> +
>> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
>> +
>> +       if (ffa_args[TS_RPC_SERVICE_INFO_RPC_STATUS] != TS_RPC_OK)
>> +               return -ENODEV;
>> +
>> +       if (ffa_args[TS_RPC_SERVICE_INFO_IFACE] > U8_MAX)
>> +               return -EINVAL;
>> +
>> +       sess = kzalloc(sizeof(*sess), GFP_KERNEL);
>> +       if (!sess)
>> +               return -ENOMEM;
>> +
>> +       sess->iface_id = ffa_args[TS_RPC_SERVICE_INFO_IFACE];
>> +
>> +       rc = xa_alloc(&ctxdata->sess_list, &sess_id, sess, xa_limit_32b,
>> +                     GFP_KERNEL);
>> +       if (rc) {
>> +               kfree(sess);
>> +               return rc;
>> +       }
>> +
>> +       arg->session = sess_id;
>> +       arg->ret = 0;
>> +
>> +       return 0;
>> +}
>> +
>> +static int tstee_close_session(struct tee_context *ctx, u32 session)
>> +{
>> +       struct ts_context_data *ctxdata = ctx->data;
>> +       struct ts_session *sess;
>> +
>> +       mutex_lock(&ctxdata->mutex);
>> +       sess = xa_erase(&ctxdata->sess_list, session);
>> +       mutex_unlock(&ctxdata->mutex);
>> +       if (!sess)
>> +               return -EINVAL;
>> +
>> +       kfree(sess);
>> +
>> +       return 0;
>> +}
>> +
>> +static int tstee_invoke_func(struct tee_context *ctx,
>> +                            struct tee_ioctl_invoke_arg *arg,
>> +                            struct tee_param *param)
>> +{
>> +       struct tstee *tstee = tee_get_drvdata(ctx->teedev);
>> +       struct ffa_device *ffa_dev = tstee->ffa_dev;
>> +       struct ts_context_data *ctxdata = ctx->data;
>> +       struct ffa_send_direct_data ffa_data;
>> +       struct tee_shm *shm = NULL;
>> +       struct ts_session *sess;
>> +       u32 req_len, ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
>> +       int shm_id, rc;
>> +       u8 iface_id;
>> +       u64 handle;
>> +       u16 opcode;
>> +
>> +       mutex_lock(&ctxdata->mutex);
>> +       sess = xa_load(&ctxdata->sess_list, arg->session);
>> +
>> +       /*
>> +        * Do this while holding the mutex to make sure that the session wasn't
>> +        * closed meanwhile
>> +        */
>> +       if (sess)
>> +               iface_id = sess->iface_id;
>> +
>> +       mutex_unlock(&ctxdata->mutex);
>> +       if (!sess)
>> +               return -EINVAL;
>> +
>
> We can drop usage of mutex here and other places via reusing
> xa_lock(), something like below should work:
>
>         xa_lock(&ctxdata->sess_list);
>
>         sess = xa_load(&ctxdata->sess_list, arg->session);
>         if (sess)
>                 iface_id = sess->iface_id;
>
>         xa_unlock(&ctxdata->sess_list);

Good point, I'll refactor according to your suggestion.

Regards,
Balint

>> +       opcode = lower_16_bits(arg->func);
>> +       shm_id = lower_32_bits(param[0].u.value.a);
>> +       req_len = lower_32_bits(param[0].u.value.b);
>> +
>> +       if (shm_id != 0) {
>> +               shm = tee_shm_get_from_id(ctx, shm_id);
>> +               if (IS_ERR(shm))
>> +                       return PTR_ERR(shm);
>> +
>> +               if (shm->size < req_len) {
>> +                       pr_err("request doesn't fit into shared memory buffer\n");
>> +                       rc = -EINVAL;
>> +                       goto out;
>> +               }
>> +
>> +               handle = shm->sec_world_id;
>> +       } else {
>> +               handle = FFA_INVALID_MEM_HANDLE;
>> +       }
>> +
>> +       ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(iface_id,
>> +                                                                 opcode);
>> +       ffa_args[TS_RPC_SERVICE_MEM_HANDLE_LSW] = lower_32_bits(handle);
>> +       ffa_args[TS_RPC_SERVICE_MEM_HANDLE_MSW] = upper_32_bits(handle);
>> +       ffa_args[TS_RPC_SERVICE_REQ_LEN] = req_len;
>> +       ffa_args[TS_RPC_SERVICE_CLIENT_ID] = 0;
>> +
>> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
>> +       rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
>> +       if (rc)
>> +               goto out;
>> +
>> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
>> +
>> +       if (ffa_args[TS_RPC_SERVICE_RPC_STATUS] != TS_RPC_OK) {
>> +               pr_err("invoke_func rpc status: %d\n",
>> +                      ffa_args[TS_RPC_SERVICE_RPC_STATUS]);
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       arg->ret = ffa_args[TS_RPC_SERVICE_STATUS];
>> +       if (shm && shm->size >= ffa_args[TS_RPC_SERVICE_RESP_LEN])
>> +               param[0].u.value.a = ffa_args[TS_RPC_SERVICE_RESP_LEN];
>> +
>> +out:
>> +       if (shm)
>> +               tee_shm_put(shm);
>> +
>> +       return rc;
>> +}
>> +
>> +static int tstee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
>> +                             struct page **pages, size_t num_pages,
>> +                             unsigned long start __always_unused)
>> +{
>> +       struct tstee *tstee = tee_get_drvdata(ctx->teedev);
>> +       struct ffa_device *ffa_dev = tstee->ffa_dev;
>> +       struct ffa_mem_region_attributes mem_attr = {
>> +               .receiver = tstee->ffa_dev->vm_id,
>> +               .attrs = FFA_MEM_RW,
>> +               .flag = 0,
>> +       };
>> +       struct ffa_mem_ops_args mem_args = {
>> +               .attrs = &mem_attr,
>> +               .use_txbuf = true,
>> +               .nattrs = 1,
>> +               .flags = 0,
>> +       };
>> +       struct ffa_send_direct_data ffa_data;
>> +       struct sg_table sgt;
>> +       u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
>> +       int rc;
>> +
>> +       rc = sg_alloc_table_from_pages(&sgt, pages, num_pages, 0,
>> +                                      num_pages * PAGE_SIZE, GFP_KERNEL);
>> +       if (rc)
>> +               return rc;
>> +
>> +       mem_args.sg = sgt.sgl;
>> +       rc = ffa_dev->ops->mem_ops->memory_share(&mem_args);
>> +       sg_free_table(&sgt);
>> +       if (rc)
>> +               return rc;
>> +
>> +       shm->sec_world_id = mem_args.g_handle;
>> +
>> +       ffa_args[TS_RPC_CTRL_REG] =
>> +                       TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
>> +                                                     TS_RPC_OP_RETRIEVE_MEM);
>> +       ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_LSW] =
>> +                       lower_32_bits(shm->sec_world_id);
>> +       ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_MSW] =
>> +                       upper_32_bits(shm->sec_world_id);
>> +       ffa_args[TS_RPC_RETRIEVE_MEM_TAG_LSW] = 0;
>> +       ffa_args[TS_RPC_RETRIEVE_MEM_TAG_MSW] = 0;
>> +
>> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
>> +       rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
>> +       if (rc) {
>> +               (void)ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id,
>> +                                                           0);
>> +               return rc;
>> +       }
>> +
>> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
>> +
>> +       if (ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS] != TS_RPC_OK) {
>> +               pr_err("shm_register rpc status: %d\n",
>> +                      ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS]);
>> +               ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int tstee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
>> +{
>> +       struct tstee *tstee = tee_get_drvdata(ctx->teedev);
>> +       struct ffa_device *ffa_dev = tstee->ffa_dev;
>> +       struct ffa_send_direct_data ffa_data;
>> +       u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
>> +       int rc;
>> +
>> +       ffa_args[TS_RPC_CTRL_REG] =
>> +                       TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
>> +                                                     TS_RPC_OP_RELINQ_MEM);
>> +       ffa_args[TS_RPC_RELINQ_MEM_HANDLE_LSW] =
>> +                       lower_32_bits(shm->sec_world_id);
>> +       ffa_args[TS_RPC_RELINQ_MEM_HANDLE_MSW] =
>> +                       upper_32_bits(shm->sec_world_id);
>> +
>> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
>> +       rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
>> +       if (rc)
>> +               return rc;
>> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
>> +
>> +       if (ffa_args[TS_RPC_RELINQ_MEM_RPC_STATUS] != TS_RPC_OK) {
>> +               pr_err("shm_unregister rpc status: %d\n",
>> +                      ffa_args[TS_RPC_RELINQ_MEM_RPC_STATUS]);
>> +               return -EINVAL;
>> +       }
>> +
>> +       rc = ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
>> +
>> +       return rc;
>> +}
>> +
>> +static const struct tee_driver_ops tstee_ops = {
>> +       .get_version = tstee_get_version,
>> +       .open = tstee_open,
>> +       .release = tstee_release,
>> +       .open_session = tstee_open_session,
>> +       .close_session = tstee_close_session,
>> +       .invoke_func = tstee_invoke_func,
>> +       .shm_register = tstee_shm_register,
>> +       .shm_unregister = tstee_shm_unregister,
>> +};
>> +
>> +static const struct tee_desc tstee_desc = {
>> +       .name = "tstee-clnt",
>> +       .ops = &tstee_ops,
>> +       .owner = THIS_MODULE,
>> +};
>> +
>> +static int pool_op_alloc(struct tee_shm_pool *pool, struct tee_shm *shm,
>> +                        size_t size, size_t align)
>> +{
>> +       return tee_shm_pool_op_alloc_helper(pool, shm, size, align,
>> +                                           tstee_shm_register);
>> +}
>> +
>> +static void pool_op_free(struct tee_shm_pool *pool, struct tee_shm *shm)
>> +{
>> +       tee_shm_pool_op_free_helper(pool, shm, tstee_shm_unregister);
>> +}
>> +
>> +static void pool_op_destroy_pool(struct tee_shm_pool *pool)
>> +{
>> +       kfree(pool);
>> +}
>> +
>> +static const struct tee_shm_pool_ops pool_ops = {
>> +       .alloc = pool_op_alloc,
>> +       .free = pool_op_free,
>> +       .destroy_pool = pool_op_destroy_pool,
>> +};
>> +
>> +static struct tee_shm_pool *tstee_create_shm_pool(void)
>> +{
>> +       struct tee_shm_pool *pool = kzalloc(sizeof(*pool), GFP_KERNEL);
>> +
>> +       if (!pool)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       pool->ops = &pool_ops;
>> +
>> +       return pool;
>> +}
>> +
>> +static bool tstee_check_rpc_compatible(struct ffa_device *ffa_dev)
>> +{
>> +       struct ffa_send_direct_data ffa_data;
>> +       u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
>> +
>> +       ffa_args[TS_RPC_CTRL_REG] =
>> +                       TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
>> +                                                     TS_RPC_OP_GET_VERSION);
>> +
>> +       arg_list_to_ffa_data(ffa_args, &ffa_data);
>> +       if (ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data))
>> +               return false;
>> +
>> +       arg_list_from_ffa_data(&ffa_data, ffa_args);
>> +
>> +       return ffa_args[TS_RPC_GET_VERSION_RESP] == TS_RPC_PROTOCOL_VERSION;
>> +}
>> +
>> +static int tstee_probe(struct ffa_device *ffa_dev)
>> +{
>> +       struct tstee *tstee;
>> +       int rc;
>> +
>> +       ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
>> +
>> +       if (!tstee_check_rpc_compatible(ffa_dev))
>> +               return -EINVAL;
>> +
>> +       tstee = kzalloc(sizeof(*tstee), GFP_KERNEL);
>> +       if (!tstee)
>> +               return -ENOMEM;
>> +
>> +       tstee->ffa_dev = ffa_dev;
>> +
>> +       tstee->pool = tstee_create_shm_pool();
>> +       if (IS_ERR(tstee->pool)) {
>> +               rc = PTR_ERR(tstee->pool);
>> +               tstee->pool = NULL;
>> +               goto err_free_tstee;
>> +       }
>> +
>> +       tstee->teedev = tee_device_alloc(&tstee_desc, NULL, tstee->pool, tstee);
>> +       if (IS_ERR(tstee->teedev)) {
>> +               rc = PTR_ERR(tstee->teedev);
>> +               tstee->teedev = NULL;
>> +               goto err_free_pool;
>> +       }
>> +
>> +       rc = tee_device_register(tstee->teedev);
>> +       if (rc)
>> +               goto err_unreg_teedev;
>> +
>> +       ffa_dev_set_drvdata(ffa_dev, tstee);
>> +
>> +       return 0;
>> +
>> +err_unreg_teedev:
>> +       tee_device_unregister(tstee->teedev);
>> +err_free_pool:
>> +       tee_shm_pool_free(tstee->pool);
>> +err_free_tstee:
>> +       kfree(tstee);
>> +       return rc;
>> +}
>> +
>> +static void tstee_remove(struct ffa_device *ffa_dev)
>> +{
>> +       struct tstee *tstee = ffa_dev->dev.driver_data;
>> +
>> +       tee_device_unregister(tstee->teedev);
>> +       tee_shm_pool_free(tstee->pool);
>> +       kfree(tstee);
>> +}
>> +
>> +static const struct ffa_device_id tstee_device_ids[] = {
>> +       /* TS RPC protocol UUID: bdcd76d7-825e-4751-963b-86d4f84943ac */
>> +       { TS_RPC_UUID },
>> +       {}
>> +};
>> +
>> +static struct ffa_driver tstee_driver = {
>> +       .name = "arm_tstee",
>> +       .probe = tstee_probe,
>> +       .remove = tstee_remove,
>> +       .id_table = tstee_device_ids,
>> +};
>> +
>> +module_ffa_driver(tstee_driver);
>> +
>> +MODULE_AUTHOR("Balint Dobszay <balint.dobszay@arm.com>");
>> +MODULE_DESCRIPTION("Arm Trusted Services TEE driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/tee/tstee/tstee_private.h b/drivers/tee/tstee/tstee_private.h
>> new file mode 100644
>> index 000000000000..6d24cf0dbbc2
>> --- /dev/null
>> +++ b/drivers/tee/tstee/tstee_private.h
>> @@ -0,0 +1,94 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2023, Arm Limited
>> + */
>> +
>> +#ifndef TSTEE_PRIVATE_H
>> +#define TSTEE_PRIVATE_H
>> +
>> +#include <linux/arm_ffa.h>
>> +#include <linux/bitops.h>
>> +#include <linux/tee_drv.h>
>> +#include <linux/types.h>
>> +#include <linux/uuid.h>
>> +#include <linux/xarray.h>
>> +
>> +/*
>> + * The description of the ABI implemented in this file is available at
>> + * https://trusted-services.readthedocs.io/en/v1.0.0/developer/service-access-protocols.html#abi
>> + */
>> +
>> +/* UUID of this protocol */
>> +#define TS_RPC_UUID UUID_INIT(0xbdcd76d7, 0x825e, 0x4751, \
>> +                             0x96, 0x3b, 0x86, 0xd4, 0xf8, 0x49, 0x43, 0xac)
>> +
>> +/* Protocol version*/
>> +#define TS_RPC_PROTOCOL_VERSION                (1)
>> +
>> +/* Status codes */
>> +#define TS_RPC_OK                      (0)
>> +
>> +/* RPC control register */
>> +#define TS_RPC_CTRL_REG                        (0)
>> +#define OPCODE_MASK                    GENMASK(15, 0)
>> +#define IFACE_ID_MASK                  GENMASK(23, 16)
>> +#define TS_RPC_CTRL_OPCODE(x)          ((u16)(FIELD_GET(OPCODE_MASK, (x))))
>> +#define TS_RPC_CTRL_IFACE_ID(x)                ((u8)(FIELD_GET(IFACE_ID_MASK, (x))))
>> +#define TS_RPC_CTRL_PACK_IFACE_OPCODE(i, o)    \
>> +       (FIELD_PREP(IFACE_ID_MASK, (i)) | FIELD_PREP(OPCODE_MASK, (o)))
>> +#define TS_RPC_CTRL_SAP_RC             BIT(30)
>> +#define TS_RPC_CTRL_SAP_ERR            BIT(31)
>> +
>> +/* Interface ID for RPC management operations */
>> +#define TS_RPC_MGMT_IFACE_ID           (0xff)
>> +
>> +/* Management calls */
>> +#define TS_RPC_OP_GET_VERSION          (0x0000)
>> +#define TS_RPC_GET_VERSION_RESP                (1)
>> +
>> +#define TS_RPC_OP_RETRIEVE_MEM         (0x0001)
>> +#define TS_RPC_RETRIEVE_MEM_HANDLE_LSW (1)
>> +#define TS_RPC_RETRIEVE_MEM_HANDLE_MSW (2)
>> +#define TS_RPC_RETRIEVE_MEM_TAG_LSW    (3)
>> +#define TS_RPC_RETRIEVE_MEM_TAG_MSW    (4)
>> +#define TS_RPC_RETRIEVE_MEM_RPC_STATUS (1)
>> +
>> +#define TS_RPC_OP_RELINQ_MEM           (0x0002)
>> +#define TS_RPC_RELINQ_MEM_HANDLE_LSW   (1)
>> +#define TS_RPC_RELINQ_MEM_HANDLE_MSW   (2)
>> +#define TS_RPC_RELINQ_MEM_RPC_STATUS   (1)
>> +
>> +#define TS_RPC_OP_SERVICE_INFO         (0x0003)
>> +#define TS_RPC_SERVICE_INFO_UUID0      (1)
>> +#define TS_RPC_SERVICE_INFO_UUID1      (2)
>> +#define TS_RPC_SERVICE_INFO_UUID2      (3)
>> +#define TS_RPC_SERVICE_INFO_UUID3      (4)
>> +#define TS_RPC_SERVICE_INFO_RPC_STATUS (1)
>> +#define TS_RPC_SERVICE_INFO_IFACE      (2)
>> +
>> +/* Service call */
>> +#define TS_RPC_SERVICE_MEM_HANDLE_LSW  (1)
>> +#define TS_RPC_SERVICE_MEM_HANDLE_MSW  (2)
>> +#define TS_RPC_SERVICE_REQ_LEN         (3)
>> +#define TS_RPC_SERVICE_CLIENT_ID       (4)
>> +#define TS_RPC_SERVICE_RPC_STATUS      (1)
>> +#define TS_RPC_SERVICE_STATUS          (2)
>> +#define TS_RPC_SERVICE_RESP_LEN                (3)
>> +
>> +struct tstee {
>> +       struct ffa_device *ffa_dev;
>> +       struct tee_device *teedev;
>> +       struct tee_shm_pool *pool;
>> +};
>> +
>> +struct ts_session {
>> +       u8 iface_id;
>> +};
>> +
>> +struct ts_context_data {
>> +       struct xarray sess_list;
>> +       /* Serializes access to the session list */
>> +       struct mutex mutex;
>> +};
>> +
>> +#endif /* TSTEE_PRIVATE_H */
>> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
>> index 23e57164693c..d0430bee8292 100644
>> --- a/include/uapi/linux/tee.h
>> +++ b/include/uapi/linux/tee.h
>> @@ -56,6 +56,7 @@
>>   */
>>  #define TEE_IMPL_ID_OPTEE      1
>>  #define TEE_IMPL_ID_AMDTEE     2
>> +#define TEE_IMPL_ID_TSTEE      3
>>
>>  /*
>>   * OP-TEE specific capabilities
>> --
>> 2.34.1
>>

[PATCH v2 3/3] Documentation: tee: Add TS-TEE driver

[Balint Dobszay]

Add documentation for the Trusted Services TEE driver.

Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
---
 Documentation/tee/index.rst  |  1 +
 Documentation/tee/ts-tee.rst | 71 ++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)
 create mode 100644 Documentation/tee/ts-tee.rst

diff --git a/Documentation/tee/index.rst b/Documentation/tee/index.rst
index a23bd08847e5..4be6e69d7837 100644
--- a/Documentation/tee/index.rst
+++ b/Documentation/tee/index.rst
@@ -10,6 +10,7 @@ TEE Subsystem
    tee
    op-tee
    amd-tee
+   ts-tee
 
 .. only::  subproject and html
 
diff --git a/Documentation/tee/ts-tee.rst b/Documentation/tee/ts-tee.rst
new file mode 100644
index 000000000000..843e34422648
--- /dev/null
+++ b/Documentation/tee/ts-tee.rst
@@ -0,0 +1,71 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================================
+TS-TEE (Trusted Services project)
+=================================
+
+This driver provides access to secure services implemented by Trusted Services.
+
+Trusted Services [1] is a TrustedFirmware.org project that provides a framework
+for developing and deploying device Root of Trust services in FF-A [2] S-EL0
+Secure Partitions. The project hosts the reference implementation of the Arm
+Platform Security Architecture [3] for Arm A-profile devices.
+
+The FF-A Secure Partitions (SP) are accessible through the FF-A driver [4] which
+provides the low level communication for this driver. On top of that the Trusted
+Services RPC protocol is used [5]. To use the driver from user space a reference
+implementation is provided at [6], which is part of the Trusted Services client
+library called libts [7].
+
+All Trusted Services (TS) SPs have the same FF-A UUID; it identifies the TS RPC
+protocol. A TS SP can host one or more services (e.g. PSA Crypto, PSA ITS, etc).
+A service is identified by its service UUID; the same type of service cannot be
+present twice in the same SP. During SP boot each service in the SP is assigned
+an "interface ID". This is just a short ID to simplify message addressing.
+
+The generic TEE design is to share memory at once with the Trusted OS, which can
+then be reused to communicate with multiple applications running on the Trusted
+OS. However, in case of FF-A, memory sharing works on an endpoint level, i.e.
+memory is shared with a specific SP. User space has to be able to separately
+share memory with each SP based on its endpoint ID; therefore a separate TEE
+device is registered for each discovered TS SP. Opening the SP corresponds to
+opening the TEE device and creating a TEE context. A TS SP hosts one or more
+services. Opening a service corresponds to opening a session in the given
+tee_context.
+
+Overview of a system with Trusted Services components::
+
+   User space                  Kernel space                   Secure world
+   ~~~~~~~~~~                  ~~~~~~~~~~~~                   ~~~~~~~~~~~~
+   +--------+                                               +-------------+
+   | Client |                                               | Trusted     |
+   +--------+                                               | Services SP |
+      /\                                                    +-------------+
+      ||                                                          /\
+      ||                                                          ||
+      ||                                                          ||
+      \/                                                          \/
+   +-------+                +----------+--------+           +-------------+
+   | libts |                |  TEE     | TS-TEE |           |  FF-A SPMC  |
+   |       |                |  subsys  | driver |           |   + SPMD    |
+   +-------+----------------+----+-----+--------+-----------+-------------+
+   |      Generic TEE API        |     |  FF-A  |     TS RPC protocol     |
+   |      IOCTL (TEE_IOC_*)      |     | driver |        over FF-A        |
+   +-----------------------------+     +--------+-------------------------+
+
+References
+==========
+
+[1] https://www.trustedfirmware.org/projects/trusted-services/
+
+[2] https://developer.arm.com/documentation/den0077/
+
+[3] https://www.arm.com/architecture/security-features/platform-security
+
+[4] drivers/firmware/arm_ffa/
+
+[5] https://trusted-services.readthedocs.io/en/v1.0.0/developer/service-access-protocols.html#abi
+
+[6] https://git.trustedfirmware.org/TS/trusted-services.git/tree/components/rpc/ts_rpc/caller/linux/ts_rpc_caller_linux.c?h=v1.0.0
+
+[7] https://git.trustedfirmware.org/TS/trusted-services.git/tree/deployments/libts/arm-linux/CMakeLists.txt?h=v1.0.0
-- 
2.34.1

Re: [PATCH v2 3/3] Documentation: tee: Add TS-TEE driver

[Sumit Garg]

On Fri, 23 Feb 2024 at 15:23, Balint Dobszay <balint.dobszay@arm.com> wrote:
>
> Add documentation for the Trusted Services TEE driver.
>
> Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
> ---
>  Documentation/tee/index.rst  |  1 +
>  Documentation/tee/ts-tee.rst | 71 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+)
>  create mode 100644 Documentation/tee/ts-tee.rst
>

Acked-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit

> diff --git a/Documentation/tee/index.rst b/Documentation/tee/index.rst
> index a23bd08847e5..4be6e69d7837 100644
> --- a/Documentation/tee/index.rst
> +++ b/Documentation/tee/index.rst
> @@ -10,6 +10,7 @@ TEE Subsystem
>     tee
>     op-tee
>     amd-tee
> +   ts-tee
>
>  .. only::  subproject and html
>
> diff --git a/Documentation/tee/ts-tee.rst b/Documentation/tee/ts-tee.rst
> new file mode 100644
> index 000000000000..843e34422648
> --- /dev/null
> +++ b/Documentation/tee/ts-tee.rst
> @@ -0,0 +1,71 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=================================
> +TS-TEE (Trusted Services project)
> +=================================
> +
> +This driver provides access to secure services implemented by Trusted Services.
> +
> +Trusted Services [1] is a TrustedFirmware.org project that provides a framework
> +for developing and deploying device Root of Trust services in FF-A [2] S-EL0
> +Secure Partitions. The project hosts the reference implementation of the Arm
> +Platform Security Architecture [3] for Arm A-profile devices.
> +
> +The FF-A Secure Partitions (SP) are accessible through the FF-A driver [4] which
> +provides the low level communication for this driver. On top of that the Trusted
> +Services RPC protocol is used [5]. To use the driver from user space a reference
> +implementation is provided at [6], which is part of the Trusted Services client
> +library called libts [7].
> +
> +All Trusted Services (TS) SPs have the same FF-A UUID; it identifies the TS RPC
> +protocol. A TS SP can host one or more services (e.g. PSA Crypto, PSA ITS, etc).
> +A service is identified by its service UUID; the same type of service cannot be
> +present twice in the same SP. During SP boot each service in the SP is assigned
> +an "interface ID". This is just a short ID to simplify message addressing.
> +
> +The generic TEE design is to share memory at once with the Trusted OS, which can
> +then be reused to communicate with multiple applications running on the Trusted
> +OS. However, in case of FF-A, memory sharing works on an endpoint level, i.e.
> +memory is shared with a specific SP. User space has to be able to separately
> +share memory with each SP based on its endpoint ID; therefore a separate TEE
> +device is registered for each discovered TS SP. Opening the SP corresponds to
> +opening the TEE device and creating a TEE context. A TS SP hosts one or more
> +services. Opening a service corresponds to opening a session in the given
> +tee_context.
> +
> +Overview of a system with Trusted Services components::
> +
> +   User space                  Kernel space                   Secure world
> +   ~~~~~~~~~~                  ~~~~~~~~~~~~                   ~~~~~~~~~~~~
> +   +--------+                                               +-------------+
> +   | Client |                                               | Trusted     |
> +   +--------+                                               | Services SP |
> +      /\                                                    +-------------+
> +      ||                                                          /\
> +      ||                                                          ||
> +      ||                                                          ||
> +      \/                                                          \/
> +   +-------+                +----------+--------+           +-------------+
> +   | libts |                |  TEE     | TS-TEE |           |  FF-A SPMC  |
> +   |       |                |  subsys  | driver |           |   + SPMD    |
> +   +-------+----------------+----+-----+--------+-----------+-------------+
> +   |      Generic TEE API        |     |  FF-A  |     TS RPC protocol     |
> +   |      IOCTL (TEE_IOC_*)      |     | driver |        over FF-A        |
> +   +-----------------------------+     +--------+-------------------------+
> +
> +References
> +==========
> +
> +[1] https://www.trustedfirmware.org/projects/trusted-services/
> +
> +[2] https://developer.arm.com/documentation/den0077/
> +
> +[3] https://www.arm.com/architecture/security-features/platform-security
> +
> +[4] drivers/firmware/arm_ffa/
> +
> +[5] https://trusted-services.readthedocs.io/en/v1.0.0/developer/service-access-protocols.html#abi
> +
> +[6] https://git.trustedfirmware.org/TS/trusted-services.git/tree/components/rpc/ts_rpc/caller/linux/ts_rpc_caller_linux.c?h=v1.0.0
> +
> +[7] https://git.trustedfirmware.org/TS/trusted-services.git/tree/deployments/libts/arm-linux/CMakeLists.txt?h=v1.0.0
> --
> 2.34.1
>

Re: [PATCH v2 3/3] Documentation: tee: Add TS-TEE driver

[Balint Dobszay]

On 27 Feb 2024, at 8:14, Sumit Garg wrote:

> On Fri, 23 Feb 2024 at 15:23, Balint Dobszay <balint.dobszay@arm.com> wrote:
>>
>> Add documentation for the Trusted Services TEE driver.
>>
>> Signed-off-by: Balint Dobszay <balint.dobszay@arm.com>
>> ---
>>  Documentation/tee/index.rst  |  1 +
>>  Documentation/tee/ts-tee.rst | 71 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 72 insertions(+)
>>  create mode 100644 Documentation/tee/ts-tee.rst
>>
>
> Acked-by: Sumit Garg <sumit.garg@linaro.org>
>
> -Sumit

Thanks, I'll apply the tag in the next version.

Regards,
Balint

>> diff --git a/Documentation/tee/index.rst b/Documentation/tee/index.rst
>> index a23bd08847e5..4be6e69d7837 100644
>> --- a/Documentation/tee/index.rst
>> +++ b/Documentation/tee/index.rst
>> @@ -10,6 +10,7 @@ TEE Subsystem
>>     tee
>>     op-tee
>>     amd-tee
>> +   ts-tee
>>
>>  .. only::  subproject and html
>>
>> diff --git a/Documentation/tee/ts-tee.rst b/Documentation/tee/ts-tee.rst
>> new file mode 100644
>> index 000000000000..843e34422648
>> --- /dev/null
>> +++ b/Documentation/tee/ts-tee.rst
>> @@ -0,0 +1,71 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +=================================
>> +TS-TEE (Trusted Services project)
>> +=================================
>> +
>> +This driver provides access to secure services implemented by Trusted Services.
>> +
>> +Trusted Services [1] is a TrustedFirmware.org project that provides a framework
>> +for developing and deploying device Root of Trust services in FF-A [2] S-EL0
>> +Secure Partitions. The project hosts the reference implementation of the Arm
>> +Platform Security Architecture [3] for Arm A-profile devices.
>> +
>> +The FF-A Secure Partitions (SP) are accessible through the FF-A driver [4] which
>> +provides the low level communication for this driver. On top of that the Trusted
>> +Services RPC protocol is used [5]. To use the driver from user space a reference
>> +implementation is provided at [6], which is part of the Trusted Services client
>> +library called libts [7].
>> +
>> +All Trusted Services (TS) SPs have the same FF-A UUID; it identifies the TS RPC
>> +protocol. A TS SP can host one or more services (e.g. PSA Crypto, PSA ITS, etc).
>> +A service is identified by its service UUID; the same type of service cannot be
>> +present twice in the same SP. During SP boot each service in the SP is assigned
>> +an "interface ID". This is just a short ID to simplify message addressing.
>> +
>> +The generic TEE design is to share memory at once with the Trusted OS, which can
>> +then be reused to communicate with multiple applications running on the Trusted
>> +OS. However, in case of FF-A, memory sharing works on an endpoint level, i.e.
>> +memory is shared with a specific SP. User space has to be able to separately
>> +share memory with each SP based on its endpoint ID; therefore a separate TEE
>> +device is registered for each discovered TS SP. Opening the SP corresponds to
>> +opening the TEE device and creating a TEE context. A TS SP hosts one or more
>> +services. Opening a service corresponds to opening a session in the given
>> +tee_context.
>> +
>> +Overview of a system with Trusted Services components::
>> +
>> +   User space                  Kernel space                   Secure world
>> +   ~~~~~~~~~~                  ~~~~~~~~~~~~                   ~~~~~~~~~~~~
>> +   +--------+                                               +-------------+
>> +   | Client |                                               | Trusted     |
>> +   +--------+                                               | Services SP |
>> +      /\                                                    +-------------+
>> +      ||                                                          /\
>> +      ||                                                          ||
>> +      ||                                                          ||
>> +      \/                                                          \/
>> +   +-------+                +----------+--------+           +-------------+
>> +   | libts |                |  TEE     | TS-TEE |           |  FF-A SPMC  |
>> +   |       |                |  subsys  | driver |           |   + SPMD    |
>> +   +-------+----------------+----+-----+--------+-----------+-------------+
>> +   |      Generic TEE API        |     |  FF-A  |     TS RPC protocol     |
>> +   |      IOCTL (TEE_IOC_*)      |     | driver |        over FF-A        |
>> +   +-----------------------------+     +--------+-------------------------+
>> +
>> +References
>> +==========
>> +
>> +[1] https://www.trustedfirmware.org/projects/trusted-services/
>> +
>> +[2] https://developer.arm.com/documentation/den0077/
>> +
>> +[3] https://www.arm.com/architecture/security-features/platform-security
>> +
>> +[4] drivers/firmware/arm_ffa/
>> +
>> +[5] https://trusted-services.readthedocs.io/en/v1.0.0/developer/service-access-protocols.html#abi
>> +
>> +[6] https://git.trustedfirmware.org/TS/trusted-services.git/tree/components/rpc/ts_rpc/caller/linux/ts_rpc_caller_linux.c?h=v1.0.0
>> +
>> +[7] https://git.trustedfirmware.org/TS/trusted-services.git/tree/deployments/libts/arm-linux/CMakeLists.txt?h=v1.0.0
>> --
>> 2.34.1
>>