Hi Jens,
Good idea putting it into code. Thanks for doing it :)
On 12 Mar 2015, at 10:00, Joakim Bech joakim.bech@linaro.org wrote:
Hi Jens and Jerome,
On Thu, Mar 12, 2015 at 09:50:43AM +0100, Jerome Forissier wrote:
Hi Jens,
On 03/12/2015 08:14 AM, Jens Wiklander wrote:
Hi,
In this patch I've tried to summarize the recent discussion. I've defined the needed ioctls and a brief description of how the other relevant syscalls are used.
Good idea to make this a patch.
Regards, Jens
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org
Documentation/ioctl/ioctl-number.txt | 1 + include/linux/sechw/tee.h | 154 +++++++++++++++++++++++++++++++++++
'sechw' looks a bit weird to me; 'sec' or 'sec_hw' maybe?
It's possible to change, I think this suggestion was coming from Javier initially.
I am good with sec_hw if that makes more sense to you. Normally in /drivers/ we do not find “_” in names, that is why I proposed sechw. The idea is to have a “secure hawrdware” submodule where we can eventually move TPM and other secure coprocessors.
2 files changed, 155 insertions(+) create mode 100644 include/linux/sechw/tee.h
diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt index 8136e1f..a04c139 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/sechw/tee.h Generic TEE driver 0xAB 00-1F linux/nbd.h 0xAC 00-1F linux/raw.h 0xAD 00 Netfilter device in development: diff --git a/include/linux/sechw/tee.h b/include/linux/sechw/tee.h new file mode 100644 index 0000000..0c44d5d --- /dev/null +++ b/include/linux/sechw/tee.h @@ -0,0 +1,154 @@ +/*
- 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_GENDRV_MAJOR_VERSION 1 +#define TEE_GENDRV_MINOR_VERSION 0
+/**
- struct tee_version - TEE versions
- @gendrv_major_version: Generic TEE driver major version
- @gendrv_minor_version: Generic TEE driver minor version
- @specdrv_major_version: Specific TEE driver major version
- @specdrv_minor_version: Specific TEE driver minor version
- @tee_api_major_version: Specific TEE API major version
- @tee_api_minor_version: Specific TEE API minor version
- @tee_os_major_version: Secure OS major version
- @tee_os_minor_version: Secure OS minor version
- @tee_api_uuid: Specific TEE API uuid
- @tee_os_uuid: Secure OS uuid
- Identifies the generic TEE driver, the specific TEE driver, which API
- is used to communicate with the Secure OS and the Secure OS itself.
- Unused fields are zeroed.
- */
+struct tee_version {
- uint32_t gendrv_major_version;
- uint32_t gendrv_minor_version;
- uint32_t specdrv_major_version;
- uint32_t specdrv_minor_version;
- uint32_t tee_api_major_version;
- uint32_t tee_api_minor_version;
- uint32_t tee_os_major_version;
- uint32_t tee_os_minor_version;
- uint8_t tee_api_uuid[16];
- uint8_t tee_os_uuid[16];
+};
Seems like a lot of different version numbers here. I wonder if we want to expose much more than the TEE API version and eventually the Generic driver version? Can't the other versions be something that the specific driver etc provides themselves?
Agreed.
Also, even though it is convenient to get all this information in one go maybe we should consider reducing the struct to only contain one major and one minor value and then instead add a flag/type that tells which kind of version you are asking for.
struct tee_version { uint32_t major_version; uint32_t minor_version; uint64_t type; /* This is huge, but it's 64bits to make alignment of the struct good */ };
+1
In type, a certain range could be defined (by us when creating this generic driver) and another range could be implementation defined. This would of course require the caller to do several calls instead of a single call to figure out misc versions, that is the drawback.
Version and UUID are related, but it would like to separate them apart. UUID is nothing TEE specific, that is something generic stated in RFC4122, which means that the struct we have been using so far is a good choice. I.e,
struct tee_uuid { uint32_t time_low; uint16_t time_mid; uint16_t time_hi_and_version; uint8_t clockseq_and_node[8]; };
Maybe we should consider splitting up clockseq_and_node into clk_seq_hi_res, clk_seq_low and node as in RFC4122?
+/**
- 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, currently zero
- @fd: File descriptor of the shared memory
"dma_buf file descriptor" maybe?
- */
+struct tee_shm_alloc_data {
- uint64_t size;
- uint32_t flags;
- int32_t fd;
+};
+/**
- struct tee_mem_share_data - share user space memory with Secure OS
- @ptr: A __user pointer to memory to share
- @size: Size of the memory to share
- @flags: Flags to/from sharing, currently set to zero by caller
- @pad: Padding, set to zero by caller
- */
+struct tee_mem_share_data {
- uint64_t ptr;
- uint64_t size;
- uint32_t flags;
- uint32_t pad;
+};
Here we may want to also support registration of a foreign dma_buf. I.e., user app obtains a file descriptor associated with a dma_buf (from another driver typically), then it registers the buffer for use on the trusted side. So, I would make it:
struct tee_mem_buf { uint64_t ptr; uint64_t size; };
struct tee_mem_dma_buf { uint32_t fd; uint32_t pad; };
struct tee_mem_share_data { union { struct tee_mem_buf buf; struct tee_mem_dma_buf dma_buf; }; uint32_t flags; uint32_t pad; };
flags would indicate whether .buf or .dmabuf is to be used.
Sounds good.
Good idea.
I was just thinking about naming. I know that this isn't the most important thing to talk about initially. But still I'd like to comment on this. Already now in this proposal we have "shm", "mem", "mem_share". Do we actually need to mention "mem"? Isn't that implicitly understood? I.e, what about changing: from -> to tee_mem_buf -> tee_buf tee_mem_dma_buf -> tee_dma_buf tee_mem_share_data -> tee_shared_buf
However for "tee_shm_alloc_data" I still think it's good to have the "shm" in the name.
-- Jerome
+#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 and secure OS
- 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)
Here, I would like to add TEE_IOC_TASK for stateless calls to the TEE.
If you all agree that TEE_IOC_LOAD_TS and TEE_IOC_GET_TS_LIST should be left for future revisions, it is OK. I think though that having the discussion about how OP_TEE could support this is necessary.
- 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)
+/**
- 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
-- Regards, Joakim
Best, Javier
Tee-dev mailing list Tee-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/tee-dev