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.

> +
> +
> +#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?

> +
> +/**
> + * 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 zero

Is 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;
> +};

> +
> +/**
> + * 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?

> +
> +/**
> + * 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;
> +};

> +
> +#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)

> +
> +/**
> + * 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