drivers/misc/optee: Fix GP API compatibility

Previous implementation was not compatible with GlobalPlatform
API in the following ways:
 - Registered mem IDs would begin from negatives when it should
   have been greater than or equal to 0
 - Register IOCTL would return 0 on success, when it should have
   been returning a file descriptor.
 - Register IOCTL would expect the user-space client to specify
   TEE_SHM_* flags dictating its behaviour when in fact, libteec
   never specifies flags.

This commit fixes all those issues. It uses nuttx/idr.h instead
of a linked list, and it uses `file_allocate` to provide file
descriptors for registered shared memory. Upon close(fd), the
memory is de-registered and freed accordingly. It also updates
the documentation accordingly.

Signed-off-by: George Poulios <gpoulios@census-labs.com>

Documentation/guides/optee.rst

@@ -100,8 +100,8 @@ on success unless otherwise specified (see ``TEE_IOC_SHM_ALLOC``).
     future, but until then, please refer to ``include/nuttx/tee.h`` for details.
     In short, for shared memory references, ``.a`` is the offset into the
     shared memory buffer, ``.b`` is the size of the buffer, and ``.c`` is the
-    the shared memory identifier (``.addr`` field used in
+    the shared memory identifier (``.id`` field returned by
-    ``TEE_IOC_SHM_REGISTER``).
+    ``TEE_IOC_SHM_ALLOC`` or ``TEE_IOC_SHM_REGISTER``).
 
 - ``TEE_IOC_CANCEL`` : Cancel a currently invoked command.
 
@@ -113,8 +113,8 @@ on success unless otherwise specified (see ``TEE_IOC_SHM_ALLOC``).
   - Expects a ``struct tee_ioctl_shm_alloc_data`` pointer with the ``.size``
     field set, and ignoring the ``.flags`` field. Upon successful return,
     it returns the memory file descriptor one can use ``mmap()`` on (with
-    ``MAP_SHARED``). It also returns an identifier for the allocation
+    ``MAP_SHARED``). It also returns an identifier for use in memory references
-    in ``.id``.
+    (``tee_ioctl_param.c`` field) in ``.id``.
 
 - ``TEE_IOC_SHM_REGISTER`` : Register a shared memory reference with the secure OS.
 
@@ -126,10 +126,9 @@ on success unless otherwise specified (see ``TEE_IOC_SHM_ALLOC``).
     ``TEE_SHM_SEC_REGISTER`` registers the memory with the secure OS for later
     use in memrefs and is automatically de-registered during driver close if
     ``TEE_SHM_REGISTER`` is also specified. ``.addr`` shall point to the (user)
-    memory to register and ``.size`` shall indicate its size. The identifier
+    memory to register and ``.size`` shall indicate its size. One may use the
-    used to register shared memory with the secure OS is the value of the
+    returned ``.id`` field when specifying shared memory references
-    ``.addr`` field (what one needs to specify in ``tee_ioctl_param.c``
+    (``tee_ioctl_param.c`` field).
-    during a ``TEE_IOC_INVOKE``).
 
 Typical usage
 =============
@@ -167,7 +166,7 @@ Typical usage
          return ret;
        }
 
-     [...check ioc_ver accordingly...]
+     /* check ioc_ver accordingly */
 
 #. Open a session with a Trusted Application
 
@@ -188,7 +187,7 @@ Typical usage
          return ret;
        }
 
-     [...use ioc_opn.session returned...]
+     /* use ioc_opn.session returned */
 
 #. Invoke a function of the Trusted Application
 
@@ -222,7 +221,7 @@ Typical usage
          goto err_with_args;
        }
 
-     [...use result (if any) in ioc_args->params...]
+     /* use result (if any) in ioc_args->params */
 
 #. Allocate shared memory through the driver
 
@@ -253,22 +252,25 @@ Typical usage
    .. code-block:: c
 
      /* The following will fail if TEE_GEN_CAP_REG_MEM is not reported in
-      * the returned `ioc_ver.gen_caps` in step 1 above */
+      * the returned `ioc_ver.gen_caps` in step 1 above
+      * Note: user memory used does not have to be allocated through IOCTL
+      */
 
      struct tee_ioctl_shm_register_data ioc_reg = { 0 };
 
-     ioc_reg.addr = (uintptr_t)ptr;
+     ioc_reg.addr = (uintptr_t)<some user memory ptr>;
-     ioc_reg.length = ioc_alloc.size;
+     ioc_reg.length = <user memory size>;
-     ioc_reg.flags = TEE_SHM_REGISTER | TEE_SHM_SEC_REGISTER;
 
-     ret = ioctl(fd, TEE_IOC_SHM_REGISTER, (unsigned long)&ioc_reg);
+     memfd = ioctl(fd, TEE_IOC_SHM_REGISTER, (unsigned long)&ioc_reg);
-     if (ret < 0)
+     if (memfd < 0)
        {
-         munmap(shm, ioc_alloc.size);
-         close(memfd);
          return ret;
        }
 
+     /* use ioc_reg.id returned in OP-TEE parameters (e.g. open, invoke) */
+
+     close(memfd);
+
 #. Use the registered shared memory in an invocation
 
    .. code-block:: c
@@ -292,8 +294,8 @@ Typical usage
      ioc_args->num_params = num_params;
      ioc_args->params[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
      ioc_args->params[0].a = 0;
-     ioc_args->params[0].b = ioc_alloc.size;
+     ioc_args->params[0].b = ioc_reg.length;
-     ioc_args->params[0].a = (uintptr_t)shm;
+     ioc_args->params[0].c = ioc_reg.id;
 
      ioc_buf.buf_ptr = (uintptr_t)ioc_args;
      ioc_buf.buf_len = ioc_args_len;
@@ -304,4 +306,4 @@ Typical usage
          goto err_with_args;
        }
 
-     [...use result (if any) in ioc_args->params...]
+     /* use result (if any) in ioc_args->params */

drivers/misc/optee.h

@@ -30,8 +30,7 @@
 #include <nuttx/config.h>
 #include <nuttx/compiler.h>
 #include <nuttx/tee.h>
-#include <nuttx/list.h>
+#include <nuttx/idr.h>
-#include <nuttx/spinlock.h>
 
 #include "optee_msg.h"
 
@@ -46,17 +45,19 @@
  * Public Types
  ****************************************************************************/
 
-struct optee_shm_entry
-{
-  struct list_node node;
-  struct tee_ioctl_shm_register_data shm;
-};
-
 struct optee_priv_data
 {
   uintptr_t alignment;        /* Transport-specified message alignment */
-  struct list_node shm_list;  /* A list of all shm registered */
+  FAR struct idr_s *shms;     /* An RB tree of all shm entries */
-  spinlock_t lock;            /* Lock used to guard list accesses */
+};
+
+struct optee_shm_entry
+{
+  FAR struct optee_priv_data *priv;
+  int32_t id;
+  uint64_t addr;
+  uint64_t length;
+  uint32_t flags;
 };
 
 /****************************************************************************
@@ -80,8 +81,7 @@ uintptr_t optee_va_to_pa(FAR const void *va);
 int optee_shm_alloc(FAR struct optee_priv_data *priv, FAR void *addr,
                     size_t size, uint32_t flags,
                     FAR struct optee_shm_entry **shmep);
-void optee_shm_free(FAR struct optee_priv_data *priv,
+void optee_shm_free(FAR struct optee_shm_entry *shme);
-                    FAR struct optee_shm_entry *shme);
 int optee_transport_init(void);
 int optee_transport_open(FAR struct optee_priv_data **priv);
 void optee_transport_close(FAR struct optee_priv_data *priv);

drivers/misc/optee_smc.c

@@ -187,10 +187,9 @@ static void optee_smc_handle_rpc(FAR struct optee_priv_data *priv_,
   switch (rpc_func)
     {
       case OPTEE_SMC_RPC_FUNC_ALLOC:
-        if (!optee_shm_alloc(priv_, NULL, par->a1,
+        if (!optee_shm_alloc(priv_, NULL, par->a1, TEE_SHM_ALLOC, &shme))
-                             TEE_SHM_ALLOC | TEE_SHM_REGISTER, &shme))
           {
-            shme_pa = optee_va_to_pa((FAR void *)(uintptr_t)shme->shm.addr);
+            shme_pa = optee_va_to_pa((FAR void *)(uintptr_t)shme->addr);
             reg_pair_from_64(shme_pa, &par->a1, &par->a2);
             reg_pair_from_64((uintptr_t)shme, &par->a4, &par->a5);
           }
@@ -204,7 +203,7 @@ static void optee_smc_handle_rpc(FAR struct optee_priv_data *priv_,
       case OPTEE_SMC_RPC_FUNC_FREE:
         shme = (FAR struct optee_shm_entry *)
                   reg_pair_to_uintptr(par->a1, par->a2);
-        optee_shm_free(priv_, shme);
+        optee_shm_free(shme);
         break;
 
       case OPTEE_SMC_RPC_FUNC_FOREIGN_INTR:

include/nuttx/tee.h

@@ -378,12 +378,10 @@ struct tee_iocl_supp_send_arg
 
 #define TEE_IOC_SUPPL_SEND _IOC(TEE_IOC_MAGIC << 8, TEE_IOC_BASE + 7)
 
-/* Shared memory specific defines */
+/* Shared memory-specific defines */
 
-#define TEE_SHM_REGISTER     (1 << 0) /* In list of shared memory */
+#define TEE_SHM_ALLOC    (1 << 0) /* Kernel-malloced and must freed */
-#define TEE_SHM_SEC_REGISTER (1 << 1) /* TEE notified of this memory */
+#define TEE_SHM_REGISTER (1 << 1) /* Registered with TEE OS */
-#define TEE_SHM_ALLOC        (1 << 2) /* The memory is malloced() and must
-                                       * be freed() */
 
 /* struct tee_ioctl_shm_register_data - Shared memory register argument
  * addr:      [in] Start address of shared memory to register