Hello Jens,
On 19 March 2015 at 14:48, Jérôme Forissier jerome.forissier@linaro.org wrote:
Hi Jens,
On 03/19/2015 11:23 AM, Jens Wiklander wrote:
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org
Changes for V3:
- Defines the flags for struct tee_shm_alloc_data flags field
- Changed line in ioctl-number to "Generic TEE subsystem" instead
Documentation/ioctl/ioctl-number.txt | 1 + include/linux/sec-hw/tee.h | 172
+++++++++++++++++++++++++++++++++++
2 files changed, 173 insertions(+) create mode 100644 include/linux/sec-hw/tee.h
diff --git a/Documentation/ioctl/ioctl-number.txt
b/Documentation/ioctl/ioctl-number.txt
index 8136e1f..6e9bd04 100644 --- a/Documentation/ioctl/ioctl-number.txt +++ b/Documentation/ioctl/ioctl-number.txt @@ -301,6 +301,7 @@ Code Seq#(hex) Include File Comments 0xA3 80-8F Port ACL in development: mailto:tlewis@mindspring.com 0xA3 90-9F linux/dtlk.h +0xA4 00-1F linux/sec-hw/tee.h Generic TEE subsystem 0xAB 00-1F linux/nbd.h 0xAC 00-1F linux/raw.h 0xAD 00 Netfilter device in development: diff --git a/include/linux/sec-hw/tee.h b/include/linux/sec-hw/tee.h new file mode 100644 index 0000000..cbaa346 --- /dev/null +++ b/include/linux/sec-hw/tee.h @@ -0,0 +1,172 @@ +/*
- 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_H +#define __TEE_H
+#include <linux/ioctl.h> +#include <linux/types.h>
+/*
- This file describes the API provided by the generic TEE driver to
user
- space
- */
+#define TEE_VERSION 1
About TEE_* names: is it OK to use them, or should we use a different prefix to avoid confusions or even clashes with GP APIs? I'd say: keep them, since we're not tied to GP, and I don't think it can be a major problem.
I would say it can create confusion. In a specific backend driver which implements GP, we may have a mixed of defines coming from the generic driver and from the GP-TEE Internal API. All will look the same.
+#define TEE_SHM_MAPPED 0x1 +#define TEE_SHM_GLOBAL_DMA_BUF 0x2
What do these flags mean/where can they be used? (add comments). I'm not sure 'GLOBAL' is useful -> TEE_SHM_DMA_BUF?
+1 for TEE_SHM_DMA_BUF
Also, some comments on the flag would not hurt.
+/**
- struct tee_version - TEE versions
- @gen_version: Generic TEE driver version
- @spec_version: Specific TEE driver version
- @uuid: Specific TEE driver uuid, zero if not used
To make the API easier to understand, I suggest adding the direction for each parameter, like this: /**
- struct tee_version - TEE versions
- @gen_version: [out] Generic TEE driver version
- @spec_version: [out] Specific TEE driver version
- @uuid: [out] Specific TEE driver uuid, zero if not used
*/
- Identifies the generic TEE driver, and the specific TEE driver.
- */
+struct tee_version {
uint32_t gen_version;uint32_t spec_version;uint8_t uuid[16];+};
+/**
- struct tee_cmd_data - Opaque command argument
- @buf_ptr: A __user pointer to a command buffer
- @buf_len: Length of the buffer above
- Opaque command data which is passed on to the specific driver. The
command
- buffer doesn't have to reside in shared memory.
- */
+struct tee_cmd_data {
uint64_t buf_ptr;uint64_t buf_len;+};
+/**
- struct tee_shm_alloc_data - Shared memory allocate argument
- @size: Size of shared memory to allocate
- @flags: Flags to/from allocation, only bits flags defined by
TEE_SHM_*
above are valid, the rest should be zeroIs that TEE_SHM_MAPPED and TEE_SHM_GLOBAL_DMA_BUF?
- @fd: dma_buf file descriptor of the shared memory
- */
+struct tee_shm_alloc_data {
uint64_t size;uint32_t flags;int32_t fd;+};
Should we have more explicit name to indicate tee_shm_alloc_data will be returned to the user normal world, whereas tee_shm is for internal use? It may help the readability of the generic driver source
+/**
- struct tee_mem_buf - share user space memory with Secure OS
- @ptr: A __user pointer to memory to share
- @size: Size of the memory to share
- */
+struct tee_mem_buf {
uint64_t ptr;uint64_t size;+};
+/**
- struct tee_mem_dma_buf - share foreign dma_buf memory
- @fd: dma_buf file descriptor
- @pad: padding, set to zero by caller
- */
+struct tee_mem_dma_buf {
int32_t fd;uint32_t pad;+};
+/*
- Bits in struct tee_mem_share_data.flags
- */
+#define TEE_MEM_SHARE_FLAG_FOREIGN_BUFFER 0x1 /* use dma_buf
field */
Here I would use DMA_BUF in the flag name, let's avoid having too many names for the same thing. TEE_MEM_SHARE_DMA_BUF maybe?
+1
+/**
- struct tee_mem_share_data - share memory with Secure OS
- @buf: share user space memory
- @dma_buf: share foreign dma_buf memory
- @flags: Flags to/from sharing, unused bits set to zero by caller
- @pad: Padding, set to zero by caller
- If TEE_MEM_SHARE_FLAG_FOREIGN_BUFFER is set use the dma_buf field,
else
- the buf field in the union.
- */
+struct tee_mem_share_data {
union {struct tee_mem_buf buf;struct tee_mem_dma_buf dma_buf;};uint32_t flags;uint32_t pad;+};
Yet another Shared Memory structure ;) How could we make more explicit the name "tee_mem_share_data" (YASM will not be accepted ;) )
+#define TEE_IOC_MAGIC 0xa4 +#define TEE_IOC_BASE 0
+#define _TEE_IOR(nr, size) _IOR(TEE_IOC_MAGIC, TEE_IOC_BASE + (nr),
size)
+#define _TEE_IOWR(nr, size) _IOWR(TEE_IOC_MAGIC, TEE_IOC_BASE + (nr),
size)
+/**
- TEE_IOC_VERSION - query version of drivers
- Takes a tee_version struct and returns with the version numbers
filled in.
- */
+#define TEE_IOC_VERSION _TEE_IOR(0, struct tee_version)
+/**
- TEE_IOC_CMD - pass a command to the specific TEE driver
- Takes tee_cmd_data struct which is passed to the specific TEE driver.
- */
+#define TEE_IOC_CMD _TEE_IOR(1, struct tee_cmd_data)
+/**
- TEE_IOC_SHM_ALLOC - allocate shared memory
- Allocates shared memory between the user space process and secure OS.
- The returned file descriptor is used to map the shared memory into
user
- space. The shared memory is freed when the descriptor is closed and
the
- memory is unmapped.
- */
+#define TEE_IOC_SHM_ALLOC _TEE_IOWR(2, struct tee_shm_alloc_data)
Worth to indicate which members of udata are in, out, inout. This is not obvious size and flag are inout.
+/**
- TEE_IOC_MEM_SHARE - share a portion of user space memory with secure
OS
- Shares a portion of user space memory with secure OS.
- */
+#define TEE_IOC_MEM_SHARE _TEE_IOWR(3, struct tee_mem_share_data)
+/**
- TEE_IOC_MEM_UNSHARE - unshares a portion shared user space memory
- Unshares a portion of previously shared user space memory.
- */
+#define TEE_IOC_MEM_UNSHARE _TEE_IOWR(4, struct tee_mem_share_data)
+/*
- Five syscalls are used when communicating with the generic TEE
driver.
- open(): opens the device associated with the driver
- ioctl(): as described above operating on the file descripto from
open()
- close(): two cases
- closes the device file descriptor
- closes a file descriptor connected to allocated shared memory
- mmap(): maps shared memory into user space
- munmap(): unmaps previously shared memory
- */
+#endif /*__TEE_H*/
Tee-dev mailing list Tee-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/tee-dev