| Message ID | 1429257057-7935-3-git-send-email-jens.wiklander@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 17, 2015 at 09:50:57AM +0200, Jens Wiklander wrote: > Adds mostly stubbed OP-TEE driver which also can be compiled as a > loadable module. Please provide a "real" driver, so that we can see how this is all going to work. Otherwise it's not really that useful to create some framework that nothing in-kernel is going to use. thanks, greg k-h
Hi, We have discussed and implemented an in-kernel interface for the driver. However, we need to agree on that interface with the kernel submodules that can be interested in using it (e.g., IMA, keyring). We though it was easier to have a framework in place before taking this space. This makes sense since a TEE driver will be, as for today, mostly used by user space. applications. Best, Javier > On 18 Apr 2015, at 10:57, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Fri, Apr 17, 2015 at 09:50:57AM +0200, Jens Wiklander wrote: >> Adds mostly stubbed OP-TEE driver which also can be compiled as a >> loadable module. > > Please provide a "real" driver, so that we can see how this is all going > to work. Otherwise it's not really that useful to create some framework > that nothing in-kernel is going to use. > > thanks, > > greg k-h
On Sat, Apr 18, 2015 at 11:36:53AM +0200, Javier González wrote: > Hi, A: No. Q: Should I include quotations after my reply? http://daringfireball.net/2007/07/on_top > We have discussed and implemented an in-kernel interface for the driver. > However, we need to agree on that interface with the kernel submodules that > can be interested in using it (e.g., IMA, keyring). We though it was easier > to have a framework in place before taking this space. This makes sense > since a TEE driver will be, as for today, mostly used by user space. > applications. No, please provide a "real" solution, just providing a framework that no one uses means that I get to delete it from the kernel tree the next release, and I doubt you want that :) Please do all of the work here, as odds are, what you need in the end will be different from what you have proposed here. thanks, greg k-h
On Saturday 18 April 2015 20:49:03 Greg Kroah-Hartman wrote: > On Sat, Apr 18, 2015 at 11:36:53AM +0200, Javier González wrote: > > Hi, > > A: No. > Q: Should I include quotations after my reply? > > http://daringfireball.net/2007/07/on_top > > > We have discussed and implemented an in-kernel interface for the driver. > > However, we need to agree on that interface with the kernel submodules that > > can be interested in using it (e.g., IMA, keyring). We though it was easier > > to have a framework in place before taking this space. This makes sense > > since a TEE driver will be, as for today, mostly used by user space. > > applications. > > No, please provide a "real" solution, just providing a framework that no > one uses means that I get to delete it from the kernel tree the next > release, and I doubt you want that > > Please do all of the work here, as odds are, what you need in the end > will be different from what you have proposed here. I guess an alternative would be to remove all the unused infrastructure code and only provide a user space interface for the features that op_tee requires, but no optional user interfaces or in-kernel interfaces. Arnd
> On 18 Apr 2015, at 21:01, Arnd Bergmann <arnd@arndb.de> wrote: > > On Saturday 18 April 2015 20:49:03 Greg Kroah-Hartman wrote: >> On Sat, Apr 18, 2015 at 11:36:53AM +0200, Javier González wrote: >>> Hi, >> >> A: No. >> Q: Should I include quotations after my reply? >> >> http://daringfireball.net/2007/07/on_top >> >>> We have discussed and implemented an in-kernel interface for the driver. >>> However, we need to agree on that interface with the kernel submodules that >>> can be interested in using it (e.g., IMA, keyring). We though it was easier >>> to have a framework in place before taking this space. This makes sense >>> since a TEE driver will be, as for today, mostly used by user space. >>> applications. >> >> No, please provide a "real" solution, just providing a framework that no >> one uses means that I get to delete it from the kernel tree the next >> release, and I doubt you want that >> >> Please do all of the work here, as odds are, what you need in the end >> will be different from what you have proposed here. > > I guess an alternative would be to remove all the unused infrastructure > code and only provide a user space interface for the features that > op_tee requires, but no optional user interfaces or in-kernel interfaces. > > Arnd Only providing user space support would defeat one of the main purposes of the driver. We could better organize the patches and divide them into user space support and in-kernel support if that is what you mean. In the end the interfaces are orthogonal, even though the functionality should be very similar. Javier.
On Sunday 19 April 2015 13:17:20 Javier González wrote: > > Only providing user space support would defeat one of the main purposes > of the driver. We could better organize the patches and divide them into user > space support and in-kernel support if that is what you mean. In the end > the interfaces are orthogonal, even though the functionality should be very > similar. Splitting up the patches to separate the user interface from the in-kernel interface is certainly a good idea, but aside from that, I also agree with Greg on this point: if you want to establish an in-kernel interface, don't add any dead code at the beginning, but add it together with the users of that interface. Arnd
On Sat, Apr 18, 2015 at 10:57:47AM +0200, Greg Kroah-Hartman wrote: > On Fri, Apr 17, 2015 at 09:50:57AM +0200, Jens Wiklander wrote: > > Adds mostly stubbed OP-TEE driver which also can be compiled as a > > loadable module. > > Please provide a "real" driver, so that we can see how this is all going > to work. Otherwise it's not really that useful to create some framework > that nothing in-kernel is going to use. OK, I'll provide that in the next round. Regards, Jens
> On 19 Apr 2015, at 21:47, Arnd Bergmann <arnd@arndb.de> wrote: > > On Sunday 19 April 2015 13:17:20 Javier González wrote: >> >> Only providing user space support would defeat one of the main purposes >> of the driver. We could better organize the patches and divide them into user >> space support and in-kernel support if that is what you mean. In the end >> the interfaces are orthogonal, even though the functionality should be very >> similar. > > Splitting up the patches to separate the user interface from the in-kernel > interface is certainly a good idea, but aside from that, I also agree with > Greg on this point: if you want to establish an in-kernel interface, don't > add any dead code at the beginning, but add it together with the users > of that interface. > > Arnd Thanks for the feedback. We will do so. Javier.
diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig index 64a8cd7..b269276 100644 --- a/drivers/tee/Kconfig +++ b/drivers/tee/Kconfig @@ -6,3 +6,13 @@ config TEE help This implements a generic interface towards a Trusted Execution Environment (TEE). + +if TEE + +menu "TEE drivers" + +source "drivers/tee/optee/Kconfig" + +endmenu + +endif diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile index 60d2dab..53f3c76 100644 --- a/drivers/tee/Makefile +++ b/drivers/tee/Makefile @@ -1,3 +1,4 @@ obj-y += tee.o obj-y += tee_shm.o obj-y += tee_shm_pool.o +obj-$(CONFIG_OPTEE) += optee/ diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig new file mode 100644 index 0000000..f43af5e --- /dev/null +++ b/drivers/tee/optee/Kconfig @@ -0,0 +1,7 @@ +# OP-TEE Trusted Execution Environment Configuration +config OPTEE + tristate "OP-TEE" + default n + select DMA_CMA + help + This implements the OP-TEE Trusted Execution Environment (TEE) driver. diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile new file mode 100644 index 0000000..124d22c --- /dev/null +++ b/drivers/tee/optee/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_OPTEE) += optee.o +optee-objs += core.o diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c new file mode 100644 index 0000000..e0f0755 --- /dev/null +++ b/drivers/tee/optee/core.c @@ -0,0 +1,192 @@ +/* + * Copyright (c) 2015, Linaro Limited + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ +#include <linux/types.h> +#include <linux/string.h> +#include <linux/errno.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/dma-contiguous.h> +#include <linux/cma.h> +#include <linux/tee/tee_drv.h> + +#define DRIVER_NAME "tee-optee" +#define OPTEE_VERSION 1 + +struct optee { + struct tee_device *supp_teedev; + struct tee_device *teedev; + struct device *dev; + struct tee_shm_pool *pool; +}; + +struct optee_context_data { + int dummy; +}; + +static void optee_get_version(struct tee_context *ctx, + u32 *version, u8 *uuid) +{ + *version = OPTEE_VERSION; + memset(uuid, 0, TEE_UUID_SIZE); +} + +static int optee_open(struct tee_context *ctx) +{ + ctx->data = kzalloc(sizeof(struct optee_context_data), GFP_KERNEL); + if (!ctx->data) + return -ENOMEM; + return 0; +} + +static void optee_release(struct tee_context *ctx) +{ + kfree(ctx->data); + ctx->data = NULL; +} + +static int optee_cmd(struct tee_context *ctx, void __user *buf, size_t len) +{ + return -EINVAL; +} + +static int optee_shm_share(struct tee_shm *shm) +{ + /* No special action needed to share memory with OP-TEE */ + return 0; +} + +static void optee_shm_unshare(struct tee_shm *shm) +{ +} + +static struct tee_driver_ops optee_ops = { + .get_version = optee_get_version, + .open = optee_open, + .release = optee_release, + .cmd = optee_cmd, + .shm_share = optee_shm_share, + .shm_unshare = optee_shm_unshare, +}; + +static struct tee_desc optee_desc = { + .name = DRIVER_NAME "-clnt", + .ops = &optee_ops, + .owner = THIS_MODULE, +}; + +static int optee_supp_cmd(struct tee_context *teectx, void __user *buf, + size_t len) +{ + return -EINVAL; +} + +static struct tee_driver_ops optee_supp_ops = { + .get_version = optee_get_version, + .open = optee_open, + .release = optee_release, + .cmd = optee_supp_cmd, + .shm_share = optee_shm_share, + .shm_unshare = optee_shm_unshare, +}; + +static struct tee_desc optee_supp_desc = { + .name = DRIVER_NAME "-supp", + .ops = &optee_supp_ops, + .owner = THIS_MODULE, + .flags = TEE_DESC_PRIVILEGED, +}; + +static int optee_probe(struct platform_device *pdev) +{ + struct tee_shm_pool *pool; + struct optee *optee; + u_long vaddr; + phys_addr_t paddr; + size_t size; + int ret; + + pool = tee_shm_pool_alloc_cma(&pdev->dev, &vaddr, &paddr, &size); + if (IS_ERR(pool)) + return PTR_ERR(pool); + + dev_info(&pdev->dev, "pool: va 0x%lx pa 0x%lx size %zx\n", + vaddr, (u_long)paddr, size); + + optee = devm_kzalloc(&pdev->dev, sizeof(*optee), GFP_KERNEL); + if (!optee) { + ret = -ENOMEM; + goto err; + } + + optee->dev = &pdev->dev; + + optee->teedev = tee_register(&optee_desc, &pdev->dev, pool, optee); + if (IS_ERR(optee->teedev)) { + ret = PTR_ERR(optee->teedev); + goto err; + } + + optee->supp_teedev = tee_register(&optee_supp_desc, &pdev->dev, pool, + optee); + if (!optee->teedev) { + ret = PTR_ERR(optee->teedev); + goto err; + } + + optee->pool = pool; + platform_set_drvdata(pdev, optee); + + dev_info(&pdev->dev, "initialized driver\n"); + return 0; +err: + if (optee && optee->teedev) + tee_unregister(optee->teedev); + if (pool) + tee_shm_pool_free(pool); + return ret; +} + +static int optee_remove(struct platform_device *pdev) +{ + struct optee *optee = platform_get_drvdata(pdev); + + tee_unregister(optee->teedev); + tee_unregister(optee->supp_teedev); + tee_shm_pool_free(optee->pool); + + return 0; +} + +static const struct of_device_id optee_match[] = { + { .compatible = "optee,optee-tz" }, + {}, +}; + +static struct platform_driver optee_driver = { + .driver = { + .name = DRIVER_NAME, + .of_match_table = optee_match, + }, + .probe = optee_probe, + .remove = optee_remove, +}; + +module_platform_driver(optee_driver); + +MODULE_AUTHOR("Linaro"); +MODULE_DESCRIPTION("OP-TEE TEE driver"); +MODULE_SUPPORTED_DEVICE(""); +MODULE_VERSION("1.0"); +MODULE_LICENSE("GPL v2");
Adds mostly stubbed OP-TEE driver which also can be compiled as a loadable module. Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> --- drivers/tee/Kconfig | 10 +++ drivers/tee/Makefile | 1 + drivers/tee/optee/Kconfig | 7 ++ drivers/tee/optee/Makefile | 2 + drivers/tee/optee/core.c | 192 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 212 insertions(+) create mode 100644 drivers/tee/optee/Kconfig create mode 100644 drivers/tee/optee/Makefile create mode 100644 drivers/tee/optee/core.c