Hello,
This patchset fixes issues with probing() tee, optee and optee driver if they were compiled into kernel, built as modules or any mixed combination. These changes require optee-os changes which already were merged. Main corresponding commits are: https://github.com/OP-TEE/optee_os/commit/9389d8030ef198c9d7b8ab7ea8e877e0ac... https://github.com/OP-TEE/optee_os/commit/bc5921cdab538c8ae48422f5ffd600f1cb...
optee_enumerate_devices() which discovers Trusted Applications on tee bus is split up on 2 changes. Do probe of drivers which do not require userspace support of tee-supplicant and stage two to run drivers with support of tee-supplicant only after tee supplicant run.
Best regards, Maxim.
Maxim Uvarov (2): optee: do drivers initialization before and after tee-supplicant run tpm_ftpm_tee: register driver on tee bus
drivers/char/tpm/tpm_ftpm_tee.c | 69 ++++++++++++++++++++++++++----- drivers/tee/optee/core.c | 25 +++++++++-- drivers/tee/optee/device.c | 17 +++++--- drivers/tee/optee/optee_private.h | 8 +++- 4 files changed, 99 insertions(+), 20 deletions(-)
Some drivers (like ftpm) can operate only after tee-supplicant runs becase of tee-supplicant provides things like storage services. This patch splits probe of non tee-supplicant dependable drivers to early stage, and after tee-supplicant run probe other drivers.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org Suggested-by: Sumit Garg sumit.garg@linaro.org Suggested-by: Arnd Bergmann arnd@linaro.org --- drivers/tee/optee/core.c | 25 ++++++++++++++++++++++--- drivers/tee/optee/device.c | 17 +++++++++++------ drivers/tee/optee/optee_private.h | 8 +++++++- 3 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 99698b8a3a74..dd2265c44907 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -17,6 +17,7 @@ #include <linux/tee_drv.h> #include <linux/types.h> #include <linux/uaccess.h> +#include <linux/workqueue.h> #include "optee_private.h" #include "optee_smc.h" #include "shm_pool.h" @@ -218,6 +219,15 @@ static void optee_get_version(struct tee_device *teedev, *vers = v; }
+static void optee_bus_scan(struct work_struct *work) +{ + int rc; + + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP); + if (rc) + pr_err("optee_enumerate_devices failed %d\n", rc); +} + static int optee_open(struct tee_context *ctx) { struct optee_context_data *ctxdata; @@ -241,8 +251,15 @@ static int optee_open(struct tee_context *ctx) kfree(ctxdata); return -EBUSY; } - }
+ INIT_WORK(&optee->scan_bus_work, optee_bus_scan); + optee->scan_bus_wq = create_workqueue("optee_bus_scan"); + if (!optee->scan_bus_wq) { + pr_err("optee: couldn't create workqueue\n"); + return -ECHILD; + } + queue_work(optee->scan_bus_wq, &optee->scan_bus_work); + } mutex_init(&ctxdata->mutex); INIT_LIST_HEAD(&ctxdata->sess_list);
@@ -296,8 +313,10 @@ static void optee_release(struct tee_context *ctx)
ctx->data = NULL;
- if (teedev == optee->supp_teedev) + if (teedev == optee->supp_teedev) { + destroy_workqueue(optee->scan_bus_wq); optee_supp_release(&optee->supp); + } }
static const struct tee_driver_ops optee_ops = { @@ -675,7 +694,7 @@ static int optee_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, optee);
- rc = optee_enumerate_devices(); + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES); if (rc) { optee_remove(pdev); return rc; diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index e3a148521ec1..8263b308efd5 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -21,7 +21,6 @@ * TEE_ERROR_BAD_PARAMETERS - Incorrect input param * TEE_ERROR_SHORT_BUFFER - Output buffer size less than required */ -#define PTA_CMD_GET_DEVICES 0x0
static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) { @@ -32,7 +31,8 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) }
static int get_devices(struct tee_context *ctx, u32 session, - struct tee_shm *device_shm, u32 *shm_size) + struct tee_shm *device_shm, u32 *shm_size, + u32 func) { int ret = 0; struct tee_ioctl_invoke_arg inv_arg; @@ -42,7 +42,7 @@ static int get_devices(struct tee_context *ctx, u32 session, memset(¶m, 0, sizeof(param));
/* Invoke PTA_CMD_GET_DEVICES function */ - inv_arg.func = PTA_CMD_GET_DEVICES; + inv_arg.func = func; inv_arg.session = session; inv_arg.num_params = 4;
@@ -87,7 +87,7 @@ static int optee_register_device(const uuid_t *device_uuid, u32 device_id) return rc; }
-int optee_enumerate_devices(void) +int __optee_enumerate_devices(u32 func) { const uuid_t pta_uuid = UUID_INIT(0x7011a688, 0xddde, 0x4053, @@ -118,7 +118,7 @@ int optee_enumerate_devices(void) goto out_ctx; }
- rc = get_devices(ctx, sess_arg.session, NULL, &shm_size); + rc = get_devices(ctx, sess_arg.session, NULL, &shm_size, func); if (rc < 0 || !shm_size) goto out_sess;
@@ -130,7 +130,7 @@ int optee_enumerate_devices(void) goto out_sess; }
- rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size); + rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size, func); if (rc < 0) goto out_shm;
@@ -158,3 +158,8 @@ int optee_enumerate_devices(void)
return rc; } + +int optee_enumerate_devices(u32 func) +{ + return __optee_enumerate_devices(func); +} diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index d9c5037b4e03..6cdac4bb7253 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -78,6 +78,8 @@ struct optee_supp { * @memremaped_shm virtual address of memory in shared memory pool * @sec_caps: secure world capabilities defined by * OPTEE_SMC_SEC_CAP_* in optee_smc.h + * @scan_bus_wq workqueue to scan optee bus and register optee drivers + * @scan_bus_work workq to scan optee bus and register optee drivers */ struct optee { struct tee_device *supp_teedev; @@ -89,6 +91,8 @@ struct optee { struct tee_shm_pool *pool; void *memremaped_shm; u32 sec_caps; + struct workqueue_struct *scan_bus_wq; + struct work_struct scan_bus_work; };
struct optee_session { @@ -173,7 +177,9 @@ void optee_free_pages_list(void *array, size_t num_entries); void optee_fill_pages_list(u64 *dst, struct page **pages, int num_pages, size_t page_offset);
-int optee_enumerate_devices(void); +#define PTA_CMD_GET_DEVICES 0x0 +#define PTA_CMD_GET_DEVICES_SUPP 0x1 +int optee_enumerate_devices(u32 func);
/* * Small helpers
Hi Maxim,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on char-misc/char-misc-testing] [also build test WARNING on linus/master v5.7-rc6 next-20200519] [cannot apply to linux/master] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Maxim-Uvarov/optee-register-drivers... base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 57c76221d5af648c8355a55c09b050c5d8d38189 config: arm64-allyesconfig (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot lkp@intel.com
All warnings (new ones prefixed by >>, old ones prefixed by <<):
drivers/tee/optee/device.c:90:5: warning: no previous prototype for '__optee_enumerate_devices' [-Wmissing-prototypes]
90 | int __optee_enumerate_devices(u32 func) | ^~~~~~~~~~~~~~~~~~~~~~~~~
vim +/__optee_enumerate_devices +90 drivers/tee/optee/device.c
89
90 int __optee_enumerate_devices(u32 func)
91 { 92 const uuid_t pta_uuid = 93 UUID_INIT(0x7011a688, 0xddde, 0x4053, 94 0xa5, 0xa9, 0x7b, 0x3c, 0x4d, 0xdf, 0x13, 0xb8); 95 struct tee_ioctl_open_session_arg sess_arg; 96 struct tee_shm *device_shm = NULL; 97 const uuid_t *device_uuid = NULL; 98 struct tee_context *ctx = NULL; 99 u32 shm_size = 0, idx, num_devices = 0; 100 int rc; 101 102 memset(&sess_arg, 0, sizeof(sess_arg)); 103 104 /* Open context with OP-TEE driver */ 105 ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL); 106 if (IS_ERR(ctx)) 107 return -ENODEV; 108 109 /* Open session with device enumeration pseudo TA */ 110 memcpy(sess_arg.uuid, pta_uuid.b, TEE_IOCTL_UUID_LEN); 111 sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC; 112 sess_arg.num_params = 0; 113 114 rc = tee_client_open_session(ctx, &sess_arg, NULL); 115 if ((rc < 0) || (sess_arg.ret != TEEC_SUCCESS)) { 116 /* Device enumeration pseudo TA not found */ 117 rc = 0; 118 goto out_ctx; 119 } 120 121 rc = get_devices(ctx, sess_arg.session, NULL, &shm_size, func); 122 if (rc < 0 || !shm_size) 123 goto out_sess; 124 125 device_shm = tee_shm_alloc(ctx, shm_size, 126 TEE_SHM_MAPPED | TEE_SHM_DMA_BUF); 127 if (IS_ERR(device_shm)) { 128 pr_err("tee_shm_alloc failed\n"); 129 rc = PTR_ERR(device_shm); 130 goto out_sess; 131 } 132 133 rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size, func); 134 if (rc < 0) 135 goto out_shm; 136 137 device_uuid = tee_shm_get_va(device_shm, 0); 138 if (IS_ERR(device_uuid)) { 139 pr_err("tee_shm_get_va failed\n"); 140 rc = PTR_ERR(device_uuid); 141 goto out_shm; 142 } 143 144 num_devices = shm_size / sizeof(uuid_t); 145 146 for (idx = 0; idx < num_devices; idx++) { 147 rc = optee_register_device(&device_uuid[idx], idx); 148 if (rc) 149 goto out_shm; 150 } 151 152 out_shm: 153 tee_shm_free(device_shm); 154 out_sess: 155 tee_client_close_session(ctx, sess_arg.session); 156 out_ctx: 157 tee_client_close_context(ctx); 158 159 return rc; 160 } 161
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Maxim,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on char-misc/char-misc-testing] [also build test WARNING on linus/master v5.7-rc6 next-20200519] [cannot apply to linux/master] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Maxim-Uvarov/optee-register-drivers... base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 57c76221d5af648c8355a55c09b050c5d8d38189 config: arm64-randconfig-r026-20200519 (attached as .config) compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 135b877874fae96b4372c8a3fbfaa8ff44ff86e3) reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm64 cross compiling tool for clang build # apt-get install binutils-aarch64-linux-gnu # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot lkp@intel.com
All warnings (new ones prefixed by >>, old ones prefixed by <<):
drivers/tee/optee/device.c:90:5: warning: no previous prototype for function '__optee_enumerate_devices' [-Wmissing-prototypes]
int __optee_enumerate_devices(u32 func) ^ drivers/tee/optee/device.c:90:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int __optee_enumerate_devices(u32 func) ^ static 1 warning generated.
vim +/__optee_enumerate_devices +90 drivers/tee/optee/device.c
89
90 int __optee_enumerate_devices(u32 func)
91 { 92 const uuid_t pta_uuid = 93 UUID_INIT(0x7011a688, 0xddde, 0x4053, 94 0xa5, 0xa9, 0x7b, 0x3c, 0x4d, 0xdf, 0x13, 0xb8); 95 struct tee_ioctl_open_session_arg sess_arg; 96 struct tee_shm *device_shm = NULL; 97 const uuid_t *device_uuid = NULL; 98 struct tee_context *ctx = NULL; 99 u32 shm_size = 0, idx, num_devices = 0; 100 int rc; 101 102 memset(&sess_arg, 0, sizeof(sess_arg)); 103 104 /* Open context with OP-TEE driver */ 105 ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL); 106 if (IS_ERR(ctx)) 107 return -ENODEV; 108 109 /* Open session with device enumeration pseudo TA */ 110 memcpy(sess_arg.uuid, pta_uuid.b, TEE_IOCTL_UUID_LEN); 111 sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC; 112 sess_arg.num_params = 0; 113 114 rc = tee_client_open_session(ctx, &sess_arg, NULL); 115 if ((rc < 0) || (sess_arg.ret != TEEC_SUCCESS)) { 116 /* Device enumeration pseudo TA not found */ 117 rc = 0; 118 goto out_ctx; 119 } 120 121 rc = get_devices(ctx, sess_arg.session, NULL, &shm_size, func); 122 if (rc < 0 || !shm_size) 123 goto out_sess; 124 125 device_shm = tee_shm_alloc(ctx, shm_size, 126 TEE_SHM_MAPPED | TEE_SHM_DMA_BUF); 127 if (IS_ERR(device_shm)) { 128 pr_err("tee_shm_alloc failed\n"); 129 rc = PTR_ERR(device_shm); 130 goto out_sess; 131 } 132 133 rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size, func); 134 if (rc < 0) 135 goto out_shm; 136 137 device_uuid = tee_shm_get_va(device_shm, 0); 138 if (IS_ERR(device_uuid)) { 139 pr_err("tee_shm_get_va failed\n"); 140 rc = PTR_ERR(device_uuid); 141 goto out_shm; 142 } 143 144 num_devices = shm_size / sizeof(uuid_t); 145 146 for (idx = 0; idx < num_devices; idx++) { 147 rc = optee_register_device(&device_uuid[idx], idx); 148 if (rc) 149 goto out_shm; 150 } 151 152 out_shm: 153 tee_shm_free(device_shm); 154 out_sess: 155 tee_client_close_session(ctx, sess_arg.session); 156 out_ctx: 157 tee_client_close_context(ctx); 158 159 return rc; 160 } 161
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Signed-off-by: kbuild test robot lkp@intel.com --- device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index 8263b308efd56..d4931dad07aaa 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -87,7 +87,7 @@ static int optee_register_device(const uuid_t *device_uuid, u32 device_id) return rc; }
-int __optee_enumerate_devices(u32 func) +static int __optee_enumerate_devices(u32 func) { const uuid_t pta_uuid = UUID_INIT(0x7011a688, 0xddde, 0x4053,
Register driver on tee bus. module tee registers bus, and module optee calls optee_enumerate_devices() to scan all devices on the bus. This TA can be Early TA's ( can be compiled into optee-os). In that case it will be on optee bus before linux booting. Also optee-suplicant application is needed to be loaded between optee module and ftpm module to to maintain functionality for ftpm driver.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org Suggested-by: Sumit Garg sumit.garg@linaro.org Suggested-by: Arnd Bergmann arnd@linaro.org --- drivers/char/tpm/tpm_ftpm_tee.c | 69 ++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 10 deletions(-)
diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c index 22bf553ccf9d..7bb4ce281050 100644 --- a/drivers/char/tpm/tpm_ftpm_tee.c +++ b/drivers/char/tpm/tpm_ftpm_tee.c @@ -214,11 +214,10 @@ static int ftpm_tee_match(struct tee_ioctl_version_data *ver, const void *data) * Return: * On success, 0. On failure, -errno. */ -static int ftpm_tee_probe(struct platform_device *pdev) +static int ftpm_tee_probe(struct device *dev) { int rc; struct tpm_chip *chip; - struct device *dev = &pdev->dev; struct ftpm_tee_private *pvt_data = NULL; struct tee_ioctl_open_session_arg sess_arg;
@@ -297,6 +296,13 @@ static int ftpm_tee_probe(struct platform_device *pdev) return rc; }
+static int ftpm_plat_tee_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + + return ftpm_tee_probe(dev); +} + /** * ftpm_tee_remove() - remove the TPM device * @pdev: the platform_device description. @@ -304,9 +310,9 @@ static int ftpm_tee_probe(struct platform_device *pdev) * Return: * 0 always. */ -static int ftpm_tee_remove(struct platform_device *pdev) +static int ftpm_tee_remove(struct device *dev) { - struct ftpm_tee_private *pvt_data = dev_get_drvdata(&pdev->dev); + struct ftpm_tee_private *pvt_data = dev_get_drvdata(dev);
/* Release the chip */ tpm_chip_unregister(pvt_data->chip); @@ -328,11 +334,18 @@ static int ftpm_tee_remove(struct platform_device *pdev) return 0; }
+static int ftpm_plat_tee_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + + return ftpm_tee_remove(dev); +} + /** * ftpm_tee_shutdown() - shutdown the TPM device * @pdev: the platform_device description. */ -static void ftpm_tee_shutdown(struct platform_device *pdev) +static void ftpm_plat_tee_shutdown(struct platform_device *pdev) { struct ftpm_tee_private *pvt_data = dev_get_drvdata(&pdev->dev);
@@ -347,17 +360,53 @@ static const struct of_device_id of_ftpm_tee_ids[] = { }; MODULE_DEVICE_TABLE(of, of_ftpm_tee_ids);
-static struct platform_driver ftpm_tee_driver = { +static struct platform_driver ftpm_tee_plat_driver = { .driver = { .name = "ftpm-tee", .of_match_table = of_match_ptr(of_ftpm_tee_ids), }, - .probe = ftpm_tee_probe, - .remove = ftpm_tee_remove, - .shutdown = ftpm_tee_shutdown, + .shutdown = ftpm_plat_tee_shutdown, + .probe = ftpm_plat_tee_probe, + .remove = ftpm_plat_tee_remove, +}; + +static const struct tee_client_device_id optee_ftpm_id_table[] = { + {UUID_INIT(0xbc50d971, 0xd4c9, 0x42c4, + 0x82, 0xcb, 0x34, 0x3f, 0xb7, 0xf3, 0x78, 0x96)}, + {} };
-module_platform_driver(ftpm_tee_driver); +MODULE_DEVICE_TABLE(tee, optee_ftpm_id_table); + +static struct tee_client_driver ftpm_tee_driver = { + .id_table = optee_ftpm_id_table, + .driver = { + .name = "optee-ftpm", + .bus = &tee_bus_type, + .probe = ftpm_tee_probe, + .remove = ftpm_tee_remove, + }, +}; + +static int __init ftpm_mod_init(void) +{ + int rc; + + rc = platform_driver_register(&ftpm_tee_plat_driver); + if (rc) + return rc; + + return driver_register(&ftpm_tee_driver.driver); +} + +static void __exit ftpm_mod_exit(void) +{ + platform_driver_unregister(&ftpm_tee_plat_driver); + driver_unregister(&ftpm_tee_driver.driver); +} + +module_init(ftpm_mod_init); +module_exit(ftpm_mod_exit);
MODULE_AUTHOR("Thirupathaiah Annapureddy thiruan@microsoft.com"); MODULE_DESCRIPTION("TPM Driver for fTPM TA in TEE");
On Mon, 2020-05-18 at 16:34 +0300, Maxim Uvarov wrote:
Register driver on tee bus. module tee registers bus, and module optee calls optee_enumerate_devices() to scan all devices on the bus. This TA can be Early TA's ( can be compiled into optee-os). In that case it will be on optee bus before linux booting. Also optee-suplicant application is needed to be loaded between optee module and ftpm module to to maintain functionality for ftpm driver.
Please use proper casing in the commit message e.g. tee to TEE and so forth. What is TA?
/Jarkko
On Mon, 18 May 2020 at 22:38, Jarkko Sakkinen jarkko.sakkinen@linux.intel.com wrote:
On Mon, 2020-05-18 at 16:34 +0300, Maxim Uvarov wrote:
Register driver on tee bus. module tee registers bus, and module optee calls optee_enumerate_devices() to scan all devices on the bus. This TA can be Early TA's ( can be compiled into optee-os). In that case it will be on optee bus before linux booting. Also optee-suplicant application is needed to be loaded between optee module and ftpm module to to maintain functionality for ftpm driver.
Please use proper casing in the commit message e.g. tee to TEE and so forth.
Ok. Will do in v2.
What is TA?
TA there is Trusted Application. I.e. application which runs in a secure world (arm trust zone.). This also fixes the Microsoft ftpm driver which has a corresponding application for secure world.
Maxim.
/Jarkko
-
Jarkko Sakkinen -
kbuild test robot -
Maxim Uvarov