Hi,
On 08 Jun 2015, at 15:05, Jens Wiklander jens.wiklander@linaro.org wrote:
Hi,
There's been a massive resistance to the opaque TEE_IOC_CMD interface on the kernel mailing lists. The last is from Mark Rutland: https://lkml.org/lkml/2015/6/5/244
Jason Gunthorpe made some quite hard remarks in https://lkml.org/lkml/2015/4/20/513
There hasn't been that many different people (3 or something) criticizing the interface, but everyone has more or less asked us to reconsider.
I think it's time to take one step back and reevaluate the consequences of having a well defined user space API.
I believe that this is in a way the same problem that we discussed regarding an in-kernel API. Then it would be immediate to map to a user-space API (plus, we have a strong argument to put the driver in the kernel in the first place).
Consider dropping TEE_IOC_CMD and replace it with: struct tee_open_session_arg { __u8 uuid[UUID_LEN]; __u8 clnt_uuid[UUID_LEN]; __u32 clnt_login; __u32 cookie; /* some unique value set by user space */ __u32 session; __u32 ret; __u32 ret_origin; __u32 num_params; }; TEE_IOC_OPEN_SESSION
struct tee_invoke_command_arg { __u32 func; __u32 cookie; /* some unique value set by user space */ __u32 session; __u32 ret; __u32 ret_origin; __u32 num_params; }; TEE_IOC_INVOKE_COMMAND
struct tee_cancel_command_arg { __u32 cookie; /* matches the command or open session to cancel */ __u32 session; __u32 ret; __u32 ret_origin; }; TEE_IOC_CANCEL_COMMAND
struct tee_close_session_arg { __u32 session; __u32 ret; __u32 ret_origin; }; TEE_IOC_CLOSE_SESSION
I would not use TEE naming (i.e., _session); open/close/send(invoke) should be enough. Also, we have seen that shared memory operations are common to the TEEs we have worked with, so I would suggest to add them to the interface too.
The tee_open_session_arg and tee_invoke_command_arg structs would be followed by a struct msg_param as defined in optee_msg.h in patch v3 https://lkml.org/lkml/2015/5/15/48 .
TEE_IOC_VERSION would be changed to: #define TEE_IMPL_ID_OPTEE 1 #define TEE_OPTEE_CAP_TZ (1 << 0) struct tee_ioctl_version_data { __u32 impl_id; __u32 capabilities; }; or equivalent.
Advantages:
- User space gets a well defined interface
- It will be easier to create a uniform API for the coming kernel internal
API 3. The subsystem could offload more from the driver.
+1
Disadvantages:
- Risk that we'll need to extend the API if a field is missing for some
TEE.
We should be able to define read/write/send pretty good to start with - they are not that complicated. Then we can add more functionality to the interface as we define new trusted services that we want to expose to kernel submodules and user space.
I'd like some feedback on providing a well defined user space API. Note that the user space API shouldn't enforce a certain protocol between normal and secure world.
As I mentioned I think that treating user and kernel space separately is an error, since we risk to end up with a fragmented interface (this is indeed one of the comments we got regarding kernel subsystems). I would send a revision of the driver where the kernel interface is explicit and ioctls map to them (after converting to kernel memory space, etc.).
-- Thanks, Jens _______________________________________________ Tee-dev mailing list Tee-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/tee-dev
Best, Javier.
Javier