Re: [Tee-dev] [PATCH v3 2/2] tee: generic TEE subsystem

On Mon, Mar 23, 2015 at 04:07:53PM +0100, Joakim Bech wrote:

Hi,

A general comment for the files in this patch: s/dmabuf/dma_buf/ <-- since this is how dma_buf is spelled in the dma_buf files. s/teeshm/tee_shm/ <-- to be consistent with other "tee_xyz" names.

Take a look in drivers/dma-buf/dma-buf.c:dma_buf_release(), that pattern seem to repeat in other drivers too. I like the style so I'm trying to emulate it. But I can add a few extra _ here and there if that's desired.

On Thu, Mar 19, 2015 at 11:23:01AM +0100, Jens Wiklander wrote:

...

Signed-off-by: Jens Wiklander jens.wiklander@linaro.org

The generic TEE subsystem compiles and the devices are created when the kernel boots. Apart from that nothing is tested. I've added a stubbed OP-TEE driver which registers two devices where one is supposed to be used by tee-supplicant and the other by clients.

There's two kinds of shared memory, global dma-buf based, and driver private without dma-buf. The global dma-buf based shared memory is intended to be used when sharing memory from user space to Secure OS, typically what's passed in memrefs. The driver private shared memory is only shared between the driver and Secure OS, this is typically where pointers to the other arguments are stored to prevent user space from both changing pointers and also to not leak kernel pointers to user space. The driver private shared memory is expected to be only small buffers < 4KiB.

---

drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/sec-hw/Kconfig | 33 +++++ drivers/sec-hw/Makefile | 3 + drivers/sec-hw/optee/Kconfig | 22 ++++ drivers/sec-hw/optee/Makefile | 1 + drivers/sec-hw/optee/optee.c | 174 +++++++++++++++++++++++++ drivers/sec-hw/tee.c | 233 ++++++++++++++++++++++++++++++++++ drivers/sec-hw/tee_private.h | 39 ++++++ drivers/sec-hw/tee_shm.c | 279 +++++++++++++++++++++++++++++++++++++++++ include/linux/sec-hw/tee_drv.h | 155 +++++++++++++++++++++++ 11 files changed, 942 insertions(+) create mode 100644 drivers/sec-hw/Kconfig create mode 100644 drivers/sec-hw/Makefile create mode 100644 drivers/sec-hw/optee/Kconfig create mode 100644 drivers/sec-hw/optee/Makefile create mode 100644 drivers/sec-hw/optee/optee.c create mode 100644 drivers/sec-hw/tee.c create mode 100644 drivers/sec-hw/tee_private.h create mode 100644 drivers/sec-hw/tee_shm.c create mode 100644 include/linux/sec-hw/tee_drv.h

diff --git a/drivers/Kconfig b/drivers/Kconfig index c0cc96b..4dd892b 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -182,4 +182,6 @@ source "drivers/thunderbolt/Kconfig" source "drivers/android/Kconfig" +source "drivers/sec-hw/Kconfig"

endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 527a6da..f4403d1 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -165,3 +165,4 @@ obj-$(CONFIG_RAS) += ras/ obj-$(CONFIG_THUNDERBOLT) += thunderbolt/ obj-$(CONFIG_CORESIGHT) += coresight/ obj-$(CONFIG_ANDROID) += android/ +obj-y += sec-hw/ diff --git a/drivers/sec-hw/Kconfig b/drivers/sec-hw/Kconfig new file mode 100644 index 0000000..ad2e5db --- /dev/null +++ b/drivers/sec-hw/Kconfig @@ -0,0 +1,33 @@ +# +# Copyright (c) 2015, Linaro Limited +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License Version 2 as +# published by the Free Software Foundation. +# +# 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. +# +# You should have received a copy of the GNU General Public License along +# with this program. If not, see http://www.gnu.org/licenses/

+# Generic Trusted Execution Environment Configuration +config TEE_GENERIC

• bool "Trusted Execution Environment support"
• default n
• select DMA_SHARED_BUFFER
• help

•  This implements a generic interface towards a Trusted Execution
  

•  Environment (TEE).
  

+if TEE_GENERIC

+menu "TEE drivers"

+source "drivers/sec-hw/optee/Kconfig"

+endmenu

+endif diff --git a/drivers/sec-hw/Makefile b/drivers/sec-hw/Makefile new file mode 100644 index 0000000..1c3d990 --- /dev/null +++ b/drivers/sec-hw/Makefile @@ -0,0 +1,3 @@ +obj-$(CONFIG_TEE_GENERIC) += tee.o +obj-$(CONFIG_TEE_GENERIC) += tee_shm.o +obj-$(CONFIG_TEE_OPTEE) += optee/ diff --git a/drivers/sec-hw/optee/Kconfig b/drivers/sec-hw/optee/Kconfig new file mode 100644 index 0000000..e7d639e --- /dev/null +++ b/drivers/sec-hw/optee/Kconfig @@ -0,0 +1,22 @@ +# +# Copyright (c) 2015, Linaro Limited +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License Version 2 as +# published by the Free Software Foundation. +# +# 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. +# +# You should have received a copy of the GNU General Public License along +# with this program. If not, see http://www.gnu.org/licenses/

+# OP-TEE Trusted Execution Environment Configuration +config TEE_OPTEE

• tristate "OP-TEE"
• default n
• depends on TEE_GENERIC
• help

•  This implements the OP-TEE Trusted Execution Environment (TEE) driver.
  

diff --git a/drivers/sec-hw/optee/Makefile b/drivers/sec-hw/optee/Makefile new file mode 100644 index 0000000..11a3dfc --- /dev/null +++ b/drivers/sec-hw/optee/Makefile @@ -0,0 +1 @@ +obj-y += optee.o diff --git a/drivers/sec-hw/optee/optee.c b/drivers/sec-hw/optee/optee.c new file mode 100644 index 0000000..4a5ce23 --- /dev/null +++ b/drivers/sec-hw/optee/optee.c @@ -0,0 +1,174 @@ +/*

• • 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/module.h> +#include <linux/platform_device.h> +#include <linux/sec-hw/tee_drv.h>

+#define DRIVER_NAME "tee-optee"

+struct optee {

• struct tee_device *supp_teedev;
• struct tee_device *teedev;
• struct device *dev;

+};

+static void optee_get_version(struct tee_filp *teefilp,

• struct tee_version *version)
  

Since we haven't done any "copy_from_user" when entering this function, it would probably be wise to add "__user" here also (the same applies for the functions below).

There's no need for a copy from user since we're storing it in a temporary struct on the stack. When this function returns the content is copied out with copy_to_user() in tee_ioctl_version().

...

+{ +}

+static int optee_open(struct tee_filp *teefilp) +{

• return -EINVAL;

+}

+static int optee_release(struct tee_filp *teefilp) +{

• return -EINVAL;

+}

+static int optee_cmd(struct tee_filp *teefilp, void __user *buf, size_t len) +{

• return -EINVAL;

+}

+static int optee_shm_share(struct tee_filp *teefilp, struct tee_shm *shm) +{

• return -ENOENT;

+}

+static void optee_shm_unshare(struct tee_filp *teefilp, 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,
• .ops = &optee_ops,
• .owner = THIS_MODULE,

+};

+static int optee_supp_cmd(struct tee_filp *teefilp, 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,

Should both supplicant and the "regular" part have the same driver name?

I don't know, it depends what we mean by the name of the driver. I guess we could add a "-supp" suffix to the supplicant descriptor.

...

• .ops = &optee_supp_ops,
• .owner = THIS_MODULE,
• .flags = TEE_DESC_PRIVILEDGED,

+};

+static int optee_probe(struct platform_device *pdev) +{

• struct optee *optee;
• int ret;
• optee = devm_kzalloc(&pdev->dev, sizeof(*optee), GFP_KERNEL);
• if (!optee)

• return -ENOMEM;
  

• optee->dev = &pdev->dev;
• optee->teedev = tee_register(&optee_desc, &pdev->dev, optee);
• if (!optee->teedev) {

• dev_err(&pdev->dev, "could not register OP-TEE TEE driver\n");
  

Remember that dev_err will also print the name. So the line above will say: tee-optee: could not register OP-TEE TEE driver I.e, there a lot of tee and op-tee in once sentence.

OK, I guess "could not register driver\n" should be enough.

...

• ret = -EINVAL;
  

• goto err;
  

• }
• optee->supp_teedev = tee_register(&optee_supp_desc, &pdev->dev, optee);
• if (!optee->teedev) {

• dev_err(&pdev->dev,
  

• 	"could not register supplicant OP-TEE TEE driver\n");
  

• ret = -EINVAL;
  

• goto err;
  

• }
• platform_set_drvdata(pdev, optee);
• dev_info(&pdev->dev, "initialized OP-TEE TEE driver\n");
• return 0;

+err:

• if (optee->teedev)

• tee_unregister(optee->teedev);
  

• devm_kfree(&pdev->dev, optee);
• 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);
• return 0;

+}

+static const struct of_device_id optee_match[] = {

• { .compatible = "tee-optee" },
• {},

+};

+static struct platform_driver optee_driver = {

• .driver = {

• .name = DRIVER_NAME,
  

• .of_match_table = optee_match,
  

• },
• .probe = optee_probe,
• .remove = optee_remove,

+};

+static int __init optee_init(void) +{

• pr_info("%s", __func__);
• return platform_driver_register(&optee_driver);

+}

+static void __exit optee_exit(void) +{

• pr_info("%s", __func__);
• platform_driver_unregister(&optee_driver);

+}

+module_init(optee_init); +module_exit(optee_exit);

+MODULE_AUTHOR("Linaro"); +MODULE_DESCRIPTION("OP-TEE TEE driver"); +MODULE_SUPPORTED_DEVICE(""); +MODULE_VERSION("1.0"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/sec-hw/tee.c b/drivers/sec-hw/tee.c new file mode 100644 index 0000000..a361e84 --- /dev/null +++ b/drivers/sec-hw/tee.c @@ -0,0 +1,233 @@ +/*

• • 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/device.h> +#include <linux/fs.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/sec-hw/tee_drv.h> +#include <asm/uaccess.h> +#include "tee_private.h"

+static int tee_open(struct inode *inode, struct file *filp) +{

• int ret;
• struct tee_device *teedev;
• struct tee_filp *teefilp;
• teedev = container_of(filp->private_data, struct tee_device, miscdev);

Is this working? Isn't here something wrong on this line? I.e. when calling open, then filp->private_data is undefined or NULL? Am I wrong here?

Yes, take a look at the macro "container_of", it's like oposite of the "offsetof" macro, it has nothing to do with the value of filp->private_data.

...

• teefilp = kzalloc(sizeof(*teefilp), GFP_KERNEL);
• if (!teefilp)

• return -ENOMEM;
  

• teefilp->teedev = teedev;
• filp->private_data = teefilp;

Also, what is stored in filp->private_data is unique to every process opening this device. I.e, this is more some data structure used as some kind of session. I think that we even allocated and called it tee_session or something in the past. What is the key concept in this function?

I'd say that it corresponds to TEEC_Context.

...

• ret = teedev->desc->ops->open(teefilp);
• if (ret)

• kfree(teefilp);
  

• return ret;

+}

+static int tee_release(struct inode *inode, struct file *filp) +{

• struct tee_filp *teefilp = filp->private_data;

Casting?

Why? filp->private_data is already a void *.

...

• return teefilp->teedev->desc->ops->release(teefilp);

+}

+static long tee_ioctl_version(struct tee_filp *teefilp,

• struct tee_version __user *uvers)
  

+{

• struct tee_version vers;
• memset(&vers, 0, sizeof(vers));
• vers.gen_version = TEE_VERSION;
• teefilp->teedev->desc->ops->get_version(teefilp, &vers);
• return copy_to_user(uvers, &vers, sizeof(vers));

+}

+static long tee_ioctl_cmd(struct tee_filp *teefilp,

• struct tee_cmd_data __user *ucmd)
  

+{

• long ret;
• struct tee_cmd_data cmd;
• void __user *buf_ptr;
• ret = copy_from_user(&cmd, ucmd, sizeof(cmd));
• if (ret)

• return ret;
  

• buf_ptr = (void __user *)(uintptr_t)cmd.buf_ptr;
• return teefilp->teedev->desc->ops->cmd(teefilp, buf_ptr, cmd.buf_len);

+}

+static long tee_ioctl_shm_alloc(struct tee_filp *teefilp,

• struct tee_shm_alloc_data __user *udata)
  

+{

• long ret;
• struct tee_shm_alloc_data data;
• struct tee_shm *shm;
• if (copy_from_user(&data, udata, sizeof(data)))

• return -EFAULT;
  

• /* These are implicit in a user space request */
• data.flags |= TEE_SHM_MAPPED | TEE_SHM_GLOBAL_DMA_BUF;
• shm = tee_shm_alloc(teefilp->teedev, data.flags, data.size);
• if (IS_ERR(shm)) {

• ret = PTR_ERR(shm);
  

• goto err;
  

• }
• data.flags = shm->flags;
• data.fd = tee_shm_fd(shm);
• if (data.fd < 0) {

• ret = data.fd;
  

• goto err;
  

• }
• if (copy_to_user(udata, &data, sizeof(data))) {

• ret = -EFAULT;
  

• goto err;
  

• }
• return 0;

+err:

• tee_shm_free(shm);
• return ret;

+}

+static long tee_ioctl_mem_share(struct tee_filp *teefilp,

• struct tee_mem_share_data __user *udata)
  

+{

• /* Not supported yet */
• return -ENOENT;

+}

+static long tee_ioctl_mem_unshare(struct tee_filp *teefilp,

• struct tee_mem_share_data __user *udata)
  

+{

• /* Not supported yet */
• return -ENOENT;

+}

+static long tee_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) +{

• struct tee_filp *teefilp = filp->private_data;

I guess we should cast this to the correct struct?

Same as above.

...

• void __user *uarg = (void __user *)arg;
• switch (cmd) {
• case TEE_IOC_VERSION:

• return tee_ioctl_version(teefilp, uarg);
  

• case TEE_IOC_CMD:

• return tee_ioctl_cmd(teefilp, uarg);
  

• case TEE_IOC_SHM_ALLOC:

• return tee_ioctl_shm_alloc(teefilp, uarg);
  

• case TEE_IOC_MEM_SHARE:

• return tee_ioctl_mem_share(teefilp, uarg);
  

• case TEE_IOC_MEM_UNSHARE:

• return tee_ioctl_mem_unshare(teefilp, uarg);
  

• default:

• return -EINVAL;
  

• }

+}

+static const struct file_operations tee_fops = {

• .owner = THIS_MODULE,
• .open = tee_open,
• .release = tee_release,
• .unlocked_ioctl = tee_ioctl

+};

+struct tee_device *tee_register(const struct tee_desc *teedesc,

• 	struct device *dev, void *driver_data)
  

+{

• static atomic_t device_no = ATOMIC_INIT(-1);
• static atomic_t priv_device_no = ATOMIC_INIT(-1);
• struct tee_device *teedev;
• int ret;
• if (!teedesc || !teedesc->name || !dev)

• return NULL;
  

• teedev = kzalloc(sizeof(*teedev), GFP_KERNEL);
• if (!teedev) {

• dev_err(dev, "failed to alloc struct tee_device\n");
  

• return NULL;
  

• }
• teedev->dev = dev;
• teedev->desc = teedesc;
• teedev->driver_data = driver_data;
• if (teedesc->flags & TEE_DESC_PRIVILEDGED)

• snprintf(teedev->name, sizeof(teedev->name),
  

• 	 "teepriv%d", atomic_inc_return(&priv_device_no));
  

• else

• snprintf(teedev->name, sizeof(teedev->name),
  

• 	 "tee%d", atomic_inc_return(&device_no));
  

• teedev->miscdev.parent = dev;
• teedev->miscdev.minor = MISC_DYNAMIC_MINOR;
• teedev->miscdev.name = teedev->name;
• teedev->miscdev.fops = &tee_fops;
• ret = misc_register(&teedev->miscdev);
• if (ret) {

• dev_err(dev, "misc_register() failed name=\"%s\"\n",
  

• 	teedev->name);
  

• goto err;
  

• }
• INIT_LIST_HEAD(&teedev->list_shm);
• mutex_init(&teedev->mutex);
• dev_set_drvdata(teedev->miscdev.this_device, teedev);
• dev_info(dev, "register misc device "%s" (minor=%d)\n",

•  dev_name(teedev->miscdev.this_device), teedev->miscdev.minor);
  

• return teedev;

+err:

• kfree(teedev);
• return NULL;

+} +EXPORT_SYMBOL_GPL(tee_register);

+void tee_unregister(struct tee_device *teedev) +{

• if (!teedev)

• return;
  

• dev_info(teedev->dev, "unregister misc device "%s" (minor=%d)\n",

•  dev_name(teedev->miscdev.this_device), teedev->miscdev.minor);
  

• misc_deregister(&teedev->miscdev);
• /* TODO finish this function */

+} +EXPORT_SYMBOL_GPL(tee_unregister);

+void *tee_get_drvdata(struct tee_device *teedev) +{

• return teedev->driver_data;

+} +EXPORT_SYMBOL_GPL(tee_get_drvdata);

Is this function needed? I mean you provide the parameter that are used to get the driver data. tee_get_drvdata(teedev); or teedev->driver_data ...

The content of "struct tee_device" is not visible outside the generic driver.

...

+static int __init tee_init(void) +{

• pr_info("initialized tee subsystem\n");
• return 0;

+}

+core_initcall(tee_init); diff --git a/drivers/sec-hw/tee_private.h b/drivers/sec-hw/tee_private.h new file mode 100644 index 0000000..2879b12 --- /dev/null +++ b/drivers/sec-hw/tee_private.h @@ -0,0 +1,39 @@ +/* * 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.
• */

+#ifndef TEE_PRIVATE_H +#define TEE_PRIVATE_H

+#define TEE_MAX_DEV_NAME_LEN 32 +struct tee_device {

• char name[TEE_MAX_DEV_NAME_LEN];

I think this is not needed, since we have a device name in dev already.

name holds something like "tee0" or "teepriv0".

...

• const struct tee_desc *desc;
• struct device *dev;
• struct miscdevice miscdev;
• struct list_head list_shm;
• struct mutex mutex;
• void *driver_data;

+};

+struct tee_shm {

• struct list_head list_node;
• struct tee_device *teedev;
• dma_addr_t paddr;
• void *kaddr;
• size_t size;
• struct dma_buf *dmabuf;
• u32 flags;

+};

+int tee_shm_fd(struct tee_shm *shm);

+#endif /*TEE_PRIVATE_H*/ diff --git a/drivers/sec-hw/tee_shm.c b/drivers/sec-hw/tee_shm.c new file mode 100644 index 0000000..894d7e8 --- /dev/null +++ b/drivers/sec-hw/tee_shm.c @@ -0,0 +1,279 @@ +/*

• • 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/device.h> +#include <linux/dma-buf.h> +#include <linux/slab.h> +#include <linux/sec-hw/tee_drv.h> +#include "tee_private.h"

+static DEFINE_MUTEX(teeshm_list_mutex); +static LIST_HEAD(teeshm_list);

+static void tee_shm_release(struct tee_shm *shm);

+static struct sg_table *tee_shm_dmabuf_map_dma_buf(struct dma_buf_attachment

• 	*attach, enum dma_data_direction dir)
  

Do we really need to have the "dma_buf" twice in the function name? The same for a few more below.

"tee_shm_dmabuf_" is the prefix that ties the functions to tee_shm_dma_buf_ops, the rest matches the member name it's assigned. As far as I can tell this the same pattern as is used elsewhere.

...

+{

•    return NULL;
  

+}

+static void tee_shm_dmabuf_unmap_dma_buf(struct dma_buf_attachment *attach,

• 	struct sg_table *table, enum dma_data_direction dir)
  

+{ +}

+static void tee_shm_dmabuf_release(struct dma_buf *dmabuf) +{

• struct tee_shm *shm = dmabuf->priv;
• tee_shm_release(shm);

+}

+static void *tee_shm_dmabuf_kmap_atomic(struct dma_buf *dmabuf,

• 	unsigned long pgnum)
  

+{

• return NULL;

+}

+static void *tee_shm_dmabuf_kmap(struct dma_buf *dmabuf, unsigned long pgnum) +{

• return NULL;

+}

+static int tee_shm_dmabuf_mmap(struct dma_buf *dmabuf,

• 	struct vm_area_struct *vma)
  

+{

• struct tee_shm *shm = dmabuf->priv;
• size_t size = vma->vm_end - vma->vm_start;
• return remap_pfn_range(vma, vma->vm_start, shm->paddr >> PAGE_SHIFT,

• 	       size, vma->vm_page_prot);
  

+}

+static struct dma_buf_ops tee_shm_dma_buf_ops = {

• .map_dma_buf = tee_shm_dmabuf_map_dma_buf,
• .unmap_dma_buf = tee_shm_dmabuf_unmap_dma_buf,
• .release = tee_shm_dmabuf_release,
• .kmap_atomic = tee_shm_dmabuf_kmap_atomic,
• .kmap = tee_shm_dmabuf_kmap,
• .mmap = tee_shm_dmabuf_mmap,

+};

+struct tee_shm *tee_shm_alloc(struct tee_device *teedev, size_t size,

• 	u32 flags)
  

+{

• struct tee_shm *shm;
• void *ret;
• struct mutex *mutex;
• struct list_head *list_shm;
• if (!(flags & TEE_SHM_MAPPED)) {

• dev_err(teedev->dev, "only mapped allocations supported\n");
  

• return ERR_PTR(-EINVAL);
  

• }
• if ((flags & ~(TEE_SHM_MAPPED|TEE_SHM_GLOBAL_DMA_BUF))) {

• dev_err(teedev->dev, "invalid shm flags 0x%x", flags);
  

• return ERR_PTR(-EINVAL);
  

• }
• shm = kzalloc(sizeof(struct tee_shm), GFP_KERNEL);
• if (!shm) {

• dev_err(teedev->dev, "failed to allocate struct tee_shm\n");
  

• return ERR_PTR(-ENOMEM);
  

• }
• shm->flags = flags;
• if (flags & TEE_SHM_GLOBAL_DMA_BUF) {

• int order = get_order(size);
  

• shm->size = (1 << order) << PAGE_SHIFT;
  

• shm->kaddr = (void *)__get_free_pages(GFP_KERNEL, order);
  

• if (!shm->kaddr) {
  

• 	dev_err(teedev->dev,
  

• 		"failed to get order %d pages for shared memory\n",
  

• 		order);
  

• 	ret = ERR_PTR(-ENOMEM);
  

• 	goto err;
  

• }
  

• /* Clear it to not leak information */
  

• memset(shm->kaddr, 0, shm->size);
  

I was thinking about using "get_zeroed_page" instead of "__get_free_pages" above? But apparently we need the "order"?

We can or in __GFP_ZERO together with GFP_KERNEL and the pages will be zeroed, I was held back by the __ first, but it seems like that how it's done elsewhere so I'll change it.

...

• shm->dmabuf = dma_buf_export(shm, &tee_shm_dma_buf_ops,
  

• 			     shm->size, O_RDWR, NULL);
  

• if (IS_ERR(shm->dmabuf)) {
  

• 	ret = ERR_CAST(shm->dmabuf);
  

• 	goto err;
  

• }
  

• mutex = &teeshm_list_mutex;
  

• list_shm = &teeshm_list;
  

• } else {

• shm->size = size;
  

• shm->kaddr = kzalloc(size, GFP_KERNEL);
  

• if (!shm->kaddr) {
  

• 	dev_err(teedev->dev,
  

• 		"failed to allocate %zu bytes of shared memory\n",
  

• 		size);
  

• 	ret = ERR_PTR(-ENOMEM);
  

• 	goto err;
  

• }
  

• mutex = &teedev->mutex;
  

• list_shm = &teedev->list_shm;
  

• }
• mutex_lock(mutex);
• list_add_tail(&shm->list_node, list_shm);
• mutex_unlock(mutex);
• shm->paddr = virt_to_phys(shm->kaddr);
• return shm;

+err:

• if (shm->kaddr) {

• if (shm->flags & TEE_SHM_GLOBAL_DMA_BUF)
  

• 	free_pages((unsigned long)shm->kaddr, get_order(size));
  

• else
  

• 	kfree(shm->kaddr);
  

• }
• kfree(shm);
• return ret;

+} +EXPORT_SYMBOL_GPL(tee_shm_alloc);

+int tee_shm_fd(struct tee_shm *shm) +{

• u32 req_flags = TEE_SHM_MAPPED | TEE_SHM_GLOBAL_DMA_BUF;
• if ((shm->flags & req_flags) != req_flags)

• return -EINVAL;
  

• return dma_buf_fd(shm->dmabuf, O_CLOEXEC);

+}

+static void tee_shm_release(struct tee_shm *shm) +{

• struct tee_device *teedev = shm->teedev;
• if (shm->flags & TEE_SHM_GLOBAL_DMA_BUF) {

• free_pages((unsigned long)shm->kaddr, get_order(shm->size));
  

• mutex_lock(&teeshm_list_mutex);
  

• list_del(&shm->list_node);
  

• mutex_unlock(&teeshm_list_mutex);
  

• } else {

• kfree(shm->kaddr);
  

• mutex_lock(&teedev->mutex);
  

• list_del(&shm->list_node);
  

• mutex_unlock(&teedev->mutex);
  

• }
• kfree(shm);

+}

+void tee_shm_free(struct tee_shm *shm) +{

• /*

• * dma_buf_put() decreases the dmabuf reference counter and will
  

• * call tee_shm_release() when the last reference is gone.
  

• *
  

• * In the case of anonymous memory we call tee_shm_release directly
  

• * instead at it doesn't have a reference counter.
  

• */
  

• if (shm->flags & TEE_SHM_GLOBAL_DMA_BUF)

• dma_buf_put(shm->dmabuf);
  

• else

• tee_shm_release(shm);
  

+} +EXPORT_SYMBOL_GPL(tee_shm_free);

+static bool cmp_key_va(struct tee_shm *shm, uintptr_t va) +{

• uintptr_t shm_va = (uintptr_t)shm->kaddr;
• return (va >= shm_va) && (va < (shm_va + shm->size));

+}

+static bool cmp_key_pa(struct tee_shm *shm, uintptr_t pa) +{

• return (pa >= shm->paddr) && (pa < (shm->paddr + shm->size));

+}

+static struct tee_shm *tee_shm_find_by_key(struct tee_device *teedev, u32 flags,

• 	bool (*cmp)(struct tee_shm *shm, uintptr_t key),
  

• 	uintptr_t key)
  

So in principle this function will return a piece of shared memory in case you provide an address that are withing a certain interval? Is my understanding correct?

Yes

...

+{

• struct tee_shm *ret = NULL;
• struct tee_shm *shm;
• struct mutex *mutex;
• struct list_head *list_shm;
• if (flags & TEE_SHM_GLOBAL_DMA_BUF) {

• mutex = &teeshm_list_mutex;
  

• list_shm = &teeshm_list;
  

• } else {

• mutex = &teedev->mutex;
  

• list_shm = &teedev->list_shm;
  

• }
• mutex_lock(mutex);
• list_for_each_entry(shm, list_shm, list_node) {

• if (cmp(shm, key)) {
  

• 	ret = shm;
  

• 	break;
  

• }
  

• }
• mutex_unlock(mutex);
• return ret;

+}

+struct tee_shm *tee_shm_find_by_va(struct tee_device *teedev, u32 flags,

• 	void *va)
  

+{

• return tee_shm_find_by_key(teedev, flags, cmp_key_va, (uintptr_t)va);

+} +EXPORT_SYMBOL_GPL(tee_shm_find_by_va);

+struct tee_shm *tee_shm_find_by_pa(struct tee_device *teedev, u32 flags,

• 	dma_addr_t pa)
  

+{

• return tee_shm_find_by_key(teedev, flags, cmp_key_pa, pa);

+} +EXPORT_SYMBOL_GPL(tee_shm_find_by_pa);

+int tee_shm_va2pa(struct tee_shm *smh, void *va, dma_addr_t *pa) +{

• /*

• * Since we only have TEE_SHM_MAPPED shared memory using
  

• * __get_free_pages() we don't need "shm" for the moment, but with
  

• * other allocators for the shared memory this may change.
  

• */
  

• *pa = virt_to_phys(va);
• return 0;

+} +EXPORT_SYMBOL_GPL(tee_shm_va2pa);

+int tee_shm_pa2va(struct tee_shm *smh, dma_addr_t pa, void **va) +{

• /*

• * Since we only have TEE_SHM_MAPPED shared memory using
  

• * __get_free_pages() we don't need "shm" for the moment, but with
  

• * other allocators for the shared memory this may change.
  

• */
  

• *va = phys_to_virt(pa);
• return 0;

+} +EXPORT_SYMBOL_GPL(tee_shm_pa2va); diff --git a/include/linux/sec-hw/tee_drv.h b/include/linux/sec-hw/tee_drv.h new file mode 100644 index 0000000..16ca13b --- /dev/null +++ b/include/linux/sec-hw/tee_drv.h @@ -0,0 +1,155 @@ +/* * 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.
• */

+#ifndef __TEE_DRV_H +#define __TEE_DRV_H

+#include <linux/types.h> +#include <linux/list.h> +#include <linux/miscdevice.h> +#include <linux/sec-hw/tee.h>

+/*

• • The file describes the API provided by the generic TEE driver to the
• • specific TEE driver.
• */

+struct tee_device; +struct tee_shm;

+/**

• • struct tee_filp - driver specific file pointer data
• • @teedev: pointer to this drivers struct tee_device
• • @filp_data: driver specific file pointer data, managed by the driver
• */

+struct tee_filp {

• struct tee_device *teedev;
• void *filp_data;

+};

+/**

• • struct tee_driver_ops - driver operations vtable
• • @get_version: fills in spec_version and uuid of supplied

Nitpick: uuid -> UUID

The member is spelled 'uuid' in 'struct tee_version'.

...

• • 	struct tee_version
    

• • @open: called when the device file is opened
• • @release: release this open file
• • @cmd: process a command from user space
• • @shm_share: share some memory with Secure OS
• • @shm_unshare: unshare some memory with Secure OS
• */

+struct tee_driver_ops {

• void (*get_version)(struct tee_filp *teefilp,

• 	    struct tee_version *version);
  

• int (*open)(struct tee_filp *teefilp);
• int (*release)(struct tee_filp *teefilp);
• int (*cmd)(struct tee_filp *teefilp, void __user *buf, size_t len);
• int (*shm_share)(struct tee_filp *teefilp, struct tee_shm *shm);
• void (*shm_unshare)(struct tee_filp *teefilp, struct tee_shm *shm);

+};

+/**

• • struct tee_desc - Describes the TEE driver to the subsystem
• • @name: name of driver
• • @ops: driver operations vtable
• • @owner: module providing the driver
• • @flags: Extra properties of driver, defined by TEE_DESC_* below
• */

+#define TEE_DESC_PRIVILEDGED 0x1 +struct tee_desc {

• const char *name;

I've already touched this above when assigning the both drivers. I guess this will be used differently compared to the name of the device? I.e, here we are talking about OP-TEE for example and tee-supplicant?

This is the name that is assigned the miscdev which is registered with the /dev/tee{,priv}*

...

• const struct tee_driver_ops *ops;
• struct module *owner;
• u32 flags;

+};

+/**

• • tee_register() - Register a specific TEE driver
• • @dev:
• • @ops: Operations on a specific TEE device
• • Once the specific driver has been probed it registers in the generic
• • driver with this function.
• • @returns a pointer to struct tee_device
• */

+struct tee_device *tee_register(const struct tee_desc *teedesc,

• 	struct device *dev, void *driver_data);
  

+/**

• • tee_unregister() - Unregister a specific TEE driver
• • @teedev:
• */

+void tee_unregister(struct tee_device *teedev);

+/**

• • tee_get_drvdata() - Return driver_data pointer
• • @returns the driver_data pointer supplied to tee_register().
• */

+void *tee_get_drvdata(struct tee_device *teedev);

+/**

• • tee_shm_alloc() - Allocate shared memory
• • @teedev:
• • @size:
• • @flags:
• • @returns a pointer to struct tee_shm
• */

+struct tee_shm *tee_shm_alloc(struct tee_device *teedev, size_t size,

• 	u32 flags);
  

+/**

• • tee_shm_free() - Free shared memory
• • @shm:
• */

+void tee_shm_free(struct tee_shm *shm);

+/**

• • tee_shm_find_by_va() - Find a shared memory handle by a virtual address
• • @teedev:
• • @flags:
• • @va:
• • @returns a pointer to struct tee_shm
• */

+struct tee_shm *tee_shm_find_by_va(struct tee_device *teedev, u32 flags,

• 	void *va);
  

+/**

• • tee_shm_find_by_pa() - Find a shared memory handle by a physical address
• • @teedev:
• • @flags:
• • @pa:
• • @returns a pointer to struct tee_shm
• */

+struct tee_shm *tee_shm_find_by_pa(struct tee_device *teedev, u32 flags,

• 	dma_addr_t pa);
  

+/**

• • tee_shm_va2pa() - Get physical address of a virtual address
• • @shm:
• • @va:
• • @pa:
• • @returns 0 on success and < 0 on failure
• */

+int tee_shm_va2pa(struct tee_shm *smh, void *va, dma_addr_t *pa);

+/**

• • tee_shm_pa2va() - Get virtual address of a physical address
• • @shm:
• • @pa:
• • @va:
• • @returns 0 on success and < 0 on failure
• */

+int tee_shm_pa2va(struct tee_shm *smh, dma_addr_t pa, void **va);

+#endif /*__TEE_DRV_H*/

1.9.1

---

Tee-dev mailing list Tee-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/tee-dev

-- Regards, Joakim B