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:
v3[7] -> v4:
- Remove unnecessary callbacks from tstee_ops
- Add maintainers entry for the new driver
v2[6] -> v3:
- Add patch "tee: Refactor TEE subsystem header files" from Sumit
- Remove unnecessary includes from core.c
- Remove the mutex from "struct ts_context_data" since the same
mechanism could be implemented by reusing the XArray's internal lock
- Rename tee_shm_pool_op_*_helper functions as suggested by Sumit
- Replace pr_* with dev_* as previously suggested by Krzysztof
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/[email protected]/
[5] https://lore.kernel.org/lkml/[email protected]/
[6] https://lore.kernel.org/lkml/[email protected]/
[7] https://lore.kernel.org/lkml/[email protected]/
Balint Dobszay (4):
tee: optee: Move pool_op helper functions
tee: tstee: Add Trusted Services TEE driver
Documentation: tee: Add TS-TEE driver
MAINTAINERS: tee: tstee: Add entry
Sumit Garg (1):
tee: Refactor TEE subsystem header files
Documentation/tee/index.rst | 1 +
Documentation/tee/ts-tee.rst | 71 ++++
MAINTAINERS | 10 +
drivers/tee/Kconfig | 1 +
drivers/tee/Makefile | 1 +
drivers/tee/amdtee/amdtee_private.h | 2 +-
drivers/tee/amdtee/call.c | 2 +-
drivers/tee/amdtee/core.c | 3 +-
drivers/tee/amdtee/shm_pool.c | 2 +-
drivers/tee/optee/call.c | 2 +-
drivers/tee/optee/core.c | 66 +---
drivers/tee/optee/device.c | 2 +-
drivers/tee/optee/ffa_abi.c | 8 +-
drivers/tee/optee/notif.c | 2 +-
drivers/tee/optee/optee_private.h | 14 +-
drivers/tee/optee/rpc.c | 2 +-
drivers/tee/optee/smc_abi.c | 11 +-
drivers/tee/tee_core.c | 2 +-
drivers/tee/tee_private.h | 35 --
drivers/tee/tee_shm.c | 66 +++-
drivers/tee/tee_shm_pool.c | 2 +-
drivers/tee/tstee/Kconfig | 11 +
drivers/tee/tstee/Makefile | 3 +
drivers/tee/tstee/core.c | 480 ++++++++++++++++++++++++++++
drivers/tee/tstee/tstee_private.h | 92 ++++++
include/linux/tee_core.h | 306 ++++++++++++++++++
include/linux/tee_drv.h | 285 ++---------------
include/uapi/linux/tee.h | 1 +
28 files changed, 1094 insertions(+), 389 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
create mode 100644 include/linux/tee_core.h
--
2.34.1
Create an entry for the newly added Trusted Services TEE driver, with
Sudeep and myself as maintainers.
Signed-off-by: Balint Dobszay <[email protected]>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index fd221a7d4d1c..eaa89feabd25 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22470,6 +22470,15 @@ F: Documentation/ABI/testing/configfs-tsm
F: drivers/virt/coco/tsm.c
F: include/linux/tsm.h
+TRUSTED SERVICES TEE DRIVER
+M: Balint Dobszay <[email protected]>
+M: Sudeep Holla <[email protected]>
+L: [email protected] (moderated for non-subscribers)
+L: [email protected]
+S: Maintained
+F: Documentation/tee/ts-tee.rst
+F: drivers/tee/tstee/
+
TTY LAYER AND SERIAL DRIVERS
M: Greg Kroah-Hartman <[email protected]>
M: Jiri Slaby <[email protected]>
--
2.34.1
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.
Reviewed-by: Sumit Garg <[email protected]>
Suggested-by: Jens Wiklander <[email protected]>
Signed-off-by: Balint Dobszay <[email protected]>
---
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 | 9 ++---
drivers/tee/tee_shm.c | 64 +++++++++++++++++++++++++++++++
include/linux/tee_core.h | 10 +++++
6 files changed, 81 insertions(+), 84 deletions(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index f762e3a25119..39e688d4e974 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 cee8ccb84cb8..3235e1c719e8 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_dyn_shm_alloc_helper(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_dyn_shm_free_helper(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 a0698ac18993..429cc20be5cc 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 9c296b887dc1..734c484ed0f6 100644
--- a/drivers/tee/optee/smc_abi.c
+++ b/drivers/tee/optee/smc_abi.c
@@ -592,19 +592,18 @@ 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_dyn_shm_alloc_helper(shm, size, align, NULL);
- return optee_pool_op_alloc_helper(pool, shm, size, align,
- optee_shm_register);
+ return tee_dyn_shm_alloc_helper(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_dyn_shm_free_helper(shm, optee_shm_unregister);
else
- optee_pool_op_free_helper(pool, shm, NULL);
+ tee_dyn_shm_free_helper(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 96a45c817427..5bf76c35cd9e 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -202,6 +202,70 @@ 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_dyn_shm_alloc_helper(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_dyn_shm_alloc_helper);
+
+void tee_dyn_shm_free_helper(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_dyn_shm_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_core.h b/include/linux/tee_core.h
index d9b3ba8e8fa9..efd16ed52315 100644
--- a/include/linux/tee_core.h
+++ b/include/linux/tee_core.h
@@ -232,6 +232,16 @@ void *tee_get_drvdata(struct tee_device *teedev);
*/
struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
+int tee_dyn_shm_alloc_helper(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_dyn_shm_free_helper(struct tee_shm *shm,
+ int (*shm_unregister)(struct tee_context *ctx,
+ struct tee_shm *shm));
+
/**
* tee_shm_is_dynamic() - Check if shared memory object is of the dynamic kind
* @shm: Shared memory handle
--
2.34.1
On Mon, 25 Mar 2024 at 20:42, Balint Dobszay <[email protected]> wrote:
>
> Create an entry for the newly added Trusted Services TEE driver, with
> Sudeep and myself as maintainers.
>
> Signed-off-by: Balint Dobszay <[email protected]>
> ---
> MAINTAINERS | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
Reviewed-by: Sumit Garg <[email protected]>
-Sumit
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fd221a7d4d1c..eaa89feabd25 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22470,6 +22470,15 @@ F: Documentation/ABI/testing/configfs-tsm
> F: drivers/virt/coco/tsm.c
> F: include/linux/tsm.h
>
> +TRUSTED SERVICES TEE DRIVER
> +M: Balint Dobszay <[email protected]>
> +M: Sudeep Holla <[email protected]>
> +L: [email protected] (moderated for non-subscribers)
> +L: [email protected]
> +S: Maintained
> +F: Documentation/tee/ts-tee.rst
> +F: drivers/tee/tstee/
> +
> TTY LAYER AND SERIAL DRIVERS
> M: Greg Kroah-Hartman <[email protected]>
> M: Jiri Slaby <[email protected]>
> --
> 2.34.1
>
On Mon, Mar 25, 2024 at 04:11:05PM +0100, Balint Dobszay wrote:
> Create an entry for the newly added Trusted Services TEE driver, with
> Sudeep and myself as maintainers.
>
Acked-by: Sudeep Holla <[email protected]>
> Signed-off-by: Balint Dobszay <[email protected]>
> ---
> MAINTAINERS | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fd221a7d4d1c..eaa89feabd25 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22470,6 +22470,15 @@ F: Documentation/ABI/testing/configfs-tsm
> F: drivers/virt/coco/tsm.c
> F: include/linux/tsm.h
>
> +TRUSTED SERVICES TEE DRIVER
> +M: Balint Dobszay <[email protected]>
> +M: Sudeep Holla <[email protected]>
> +L: [email protected] (moderated for non-subscribers)
> +L: [email protected]
> +S: Maintained
> +F: Documentation/tee/ts-tee.rst
> +F: drivers/tee/tstee/
> +
> TTY LAYER AND SERIAL DRIVERS
> M: Greg Kroah-Hartman <[email protected]>
> M: Jiri Slaby <[email protected]>
> --
> 2.34.1
>
--
Regards,
Sudeep
Hi Balint,
On Mon, Mar 25, 2024 at 4:11 PM Balint Dobszay <[email protected]> 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.
>
> Reviewed-by: Sumit Garg <[email protected]>
> Suggested-by: Jens Wiklander <[email protected]>
> Signed-off-by: Balint Dobszay <[email protected]>
> ---
> 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 | 9 ++---
> drivers/tee/tee_shm.c | 64 +++++++++++++++++++++++++++++++
> include/linux/tee_core.h | 10 +++++
> 6 files changed, 81 insertions(+), 84 deletions(-)
This patch fails to build on x86_64:
CC [M] drivers/tee/tee_shm.o
drivers/tee/tee_shm.c: In function ‘tee_dyn_shm_alloc_helper’:
linux/drivers/tee/tee_shm.c:226:22: error: implicit declaration of
function ‘virt_to_phys’; did you mean ‘virt_to_page’?
[-Werror=implicit-function-declaration]
226 | shm->paddr = virt_to_phys(shm->kaddr);
| ^~~~~~~~~~~~
| virt_to_page
It's fixed by adding
#include <linux/io.h>
I'll fix up the patch if you agree with the fix.
Cheers,
Jens
>
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index f762e3a25119..39e688d4e974 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 cee8ccb84cb8..3235e1c719e8 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_dyn_shm_alloc_helper(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_dyn_shm_free_helper(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 a0698ac18993..429cc20be5cc 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 9c296b887dc1..734c484ed0f6 100644
> --- a/drivers/tee/optee/smc_abi.c
> +++ b/drivers/tee/optee/smc_abi.c
> @@ -592,19 +592,18 @@ 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_dyn_shm_alloc_helper(shm, size, align, NULL);
>
> - return optee_pool_op_alloc_helper(pool, shm, size, align,
> - optee_shm_register);
> + return tee_dyn_shm_alloc_helper(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_dyn_shm_free_helper(shm, optee_shm_unregister);
> else
> - optee_pool_op_free_helper(pool, shm, NULL);
> + tee_dyn_shm_free_helper(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 96a45c817427..5bf76c35cd9e 100644
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -202,6 +202,70 @@ 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_dyn_shm_alloc_helper(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_dyn_shm_alloc_helper);
> +
> +void tee_dyn_shm_free_helper(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_dyn_shm_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_core.h b/include/linux/tee_core.h
> index d9b3ba8e8fa9..efd16ed52315 100644
> --- a/include/linux/tee_core.h
> +++ b/include/linux/tee_core.h
> @@ -232,6 +232,16 @@ void *tee_get_drvdata(struct tee_device *teedev);
> */
> struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
>
> +int tee_dyn_shm_alloc_helper(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_dyn_shm_free_helper(struct tee_shm *shm,
> + int (*shm_unregister)(struct tee_context *ctx,
> + struct tee_shm *shm));
> +
> /**
> * tee_shm_is_dynamic() - Check if shared memory object is of the dynamic kind
> * @shm: Shared memory handle
> --
> 2.34.1
>
Hi Jens,
On 3 Apr 2024, at 11:43, Jens Wiklander wrote:
> Hi Balint,
>
> On Mon, Mar 25, 2024 at 4:11 PM Balint Dobszay <[email protected]> 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.
>>
>> Reviewed-by: Sumit Garg <[email protected]>
>> Suggested-by: Jens Wiklander <[email protected]>
>> Signed-off-by: Balint Dobszay <[email protected]>
>> ---
>> 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 | 9 ++---
>> drivers/tee/tee_shm.c | 64 +++++++++++++++++++++++++++++++
>> include/linux/tee_core.h | 10 +++++
>> 6 files changed, 81 insertions(+), 84 deletions(-)
>
> This patch fails to build on x86_64:
> CC [M] drivers/tee/tee_shm.o
> drivers/tee/tee_shm.c: In function ‘tee_dyn_shm_alloc_helper’:
> linux/drivers/tee/tee_shm.c:226:22: error: implicit declaration of
> function ‘virt_to_phys’; did you mean ‘virt_to_page’?
> [-Werror=implicit-function-declaration]
> 226 | shm->paddr = virt_to_phys(shm->kaddr);
> | ^~~~~~~~~~~~
> | virt_to_page
>
> It's fixed by adding
> #include <linux/io.h>
>
> I'll fix up the patch if you agree with the fix.
Thanks for catching this. I agree with the fix, please apply it.
Regards,
Balint
>>
>> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
>> index f762e3a25119..39e688d4e974 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 cee8ccb84cb8..3235e1c719e8 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_dyn_shm_alloc_helper(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_dyn_shm_free_helper(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 a0698ac18993..429cc20be5cc 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 9c296b887dc1..734c484ed0f6 100644
>> --- a/drivers/tee/optee/smc_abi.c
>> +++ b/drivers/tee/optee/smc_abi.c
>> @@ -592,19 +592,18 @@ 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_dyn_shm_alloc_helper(shm, size, align, NULL);
>>
>> - return optee_pool_op_alloc_helper(pool, shm, size, align,
>> - optee_shm_register);
>> + return tee_dyn_shm_alloc_helper(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_dyn_shm_free_helper(shm, optee_shm_unregister);
>> else
>> - optee_pool_op_free_helper(pool, shm, NULL);
>> + tee_dyn_shm_free_helper(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 96a45c817427..5bf76c35cd9e 100644
>> --- a/drivers/tee/tee_shm.c
>> +++ b/drivers/tee/tee_shm.c
>> @@ -202,6 +202,70 @@ 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_dyn_shm_alloc_helper(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_dyn_shm_alloc_helper);
>> +
>> +void tee_dyn_shm_free_helper(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_dyn_shm_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_core.h b/include/linux/tee_core.h
>> index d9b3ba8e8fa9..efd16ed52315 100644
>> --- a/include/linux/tee_core.h
>> +++ b/include/linux/tee_core.h
>> @@ -232,6 +232,16 @@ void *tee_get_drvdata(struct tee_device *teedev);
>> */
>> struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size);
>>
>> +int tee_dyn_shm_alloc_helper(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_dyn_shm_free_helper(struct tee_shm *shm,
>> + int (*shm_unregister)(struct tee_context *ctx,
>> + struct tee_shm *shm));
>> +
>> /**
>> * tee_shm_is_dynamic() - Check if shared memory object is of the dynamic kind
>> * @shm: Shared memory handle
>> --
>> 2.34.1
>>
On Mon, Mar 25, 2024 at 4:11 PM Balint Dobszay <[email protected]> wrote:
>
> 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:
> v3[7] -> v4:
> - Remove unnecessary callbacks from tstee_ops
> - Add maintainers entry for the new driver
>
> v2[6] -> v3:
> - Add patch "tee: Refactor TEE subsystem header files" from Sumit
> - Remove unnecessary includes from core.c
> - Remove the mutex from "struct ts_context_data" since the same
> mechanism could be implemented by reusing the XArray's internal lock
> - Rename tee_shm_pool_op_*_helper functions as suggested by Sumit
> - Replace pr_* with dev_* as previously suggested by Krzysztof
>
> 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/[email protected]/
> [5] https://lore.kernel.org/lkml/[email protected]/
> [6] https://lore.kernel.org/lkml/[email protected]/
> [7] https://lore.kernel.org/lkml/[email protected]/
>
>
> Balint Dobszay (4):
> tee: optee: Move pool_op helper functions
> tee: tstee: Add Trusted Services TEE driver
> Documentation: tee: Add TS-TEE driver
> MAINTAINERS: tee: tstee: Add entry
>
> Sumit Garg (1):
> tee: Refactor TEE subsystem header files
>
> Documentation/tee/index.rst | 1 +
> Documentation/tee/ts-tee.rst | 71 ++++
> MAINTAINERS | 10 +
> drivers/tee/Kconfig | 1 +
> drivers/tee/Makefile | 1 +
> drivers/tee/amdtee/amdtee_private.h | 2 +-
> drivers/tee/amdtee/call.c | 2 +-
> drivers/tee/amdtee/core.c | 3 +-
> drivers/tee/amdtee/shm_pool.c | 2 +-
> drivers/tee/optee/call.c | 2 +-
> drivers/tee/optee/core.c | 66 +---
> drivers/tee/optee/device.c | 2 +-
> drivers/tee/optee/ffa_abi.c | 8 +-
> drivers/tee/optee/notif.c | 2 +-
> drivers/tee/optee/optee_private.h | 14 +-
> drivers/tee/optee/rpc.c | 2 +-
> drivers/tee/optee/smc_abi.c | 11 +-
> drivers/tee/tee_core.c | 2 +-
> drivers/tee/tee_private.h | 35 --
> drivers/tee/tee_shm.c | 66 +++-
> drivers/tee/tee_shm_pool.c | 2 +-
> drivers/tee/tstee/Kconfig | 11 +
> drivers/tee/tstee/Makefile | 3 +
> drivers/tee/tstee/core.c | 480 ++++++++++++++++++++++++++++
> drivers/tee/tstee/tstee_private.h | 92 ++++++
> include/linux/tee_core.h | 306 ++++++++++++++++++
> include/linux/tee_drv.h | 285 ++---------------
> include/uapi/linux/tee.h | 1 +
> 28 files changed, 1094 insertions(+), 389 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
> create mode 100644 include/linux/tee_core.h
>
> --
> 2.34.1
>
I'm picking up this patch set.
Thanks,
Jens