xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073947475


##########
include/nuttx/tee.h:
##########
@@ -198,6 +198,10 @@ struct tee_ioctl_shm_register_fd_data
 
 #define TEE_IOCTL_LOGIN_REE_KERNEL              0x80000000
 
+/* Macro to help calculate the total size of n params */
+
+#define TEE_IOCTL_PARAM_SIZE(x)        (sizeof(struct tee_ioctl_param) * (x))

Review Comment:
   align the 3rd column with other lines



##########
drivers/misc/optee.c:
##########
@@ -99,6 +104,51 @@ static const struct file_operations g_optee_ops =
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: optee_safe_va_range
+ *
+ * Description:
+ *   Check whether provided virtual address range belongs to user-owned
+ *   memory. If this function is called from a kernel thread, it returns
+ *   true. If this function is called in a build without CONFIG_ARCH_ADDRENV
+ *   it always returns true.
+ *
+ * Parameters:
+ *   va    - Beginning of address range to check.
+ *   size  - Size of memory to check.
+ *
+ * Returned Values:
+ *   True if the provided address range belongs to the user or the caller is
+ *   a kernel thread. False otherwise.
+ *
+ ****************************************************************************/
+
+ #ifdef CONFIG_ARCH_ADDRENV
+static bool optee_safe_va_range(FAR void *va, size_t size)
+{
+  FAR struct tcb_s *tcb;
+  uint8_t ttype;
+
+  tcb = nxsched_self();
+  ttype = tcb->flags & TCB_FLAG_TTYPE_MASK;
+
+  if (ttype == TCB_FLAG_TTYPE_KERNEL)
+    {
+      return true;
+    }
+
+  if (up_addrenv_user_vaddr((uintptr_t)va) &&
+      up_addrenv_user_vaddr((uintptr_t)va + size - 1))
+    {
+      return true;
+    }
+
+  return false;
+}
+#else
+#define optee_safe_va_range(addr, size) (true)

Review Comment:
   `#  define optee_safe_va_range(addr, size) (true)`



##########
drivers/misc/optee.c:
##########
@@ -24,16 +24,19 @@
  * Included Files
  ****************************************************************************/
 
-#include <nuttx/tee.h>
-
 #include <fcntl.h>
+#include <nuttx/tee.h>
+#include <nuttx/config.h>

Review Comment:
   remove nuttx/config.h



##########
drivers/misc/optee.h:
##########
@@ -0,0 +1,77 @@
+/****************************************************************************
+ * drivers/misc/optee.h
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __DRIVERS_MISC_OPTEE_H
+#define __DRIVERS_MISC_OPTEE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/compiler.h>
+#include <nuttx/tee.h>
+
+#include "optee_msg.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define OPTEE_SERVER_PATH              "optee"
+#define OPTEE_MAX_PARAM_NUM            6
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+struct optee_priv_data;
+
+struct optee_priv_data
+{
+  FAR void *transport;
+};
+
+/****************************************************************************
+ * Public Functions Definitions
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+int optee_transport_init(void);
+int optee_transport_open(FAR struct optee_priv_data *priv);
+int optee_transport_close(FAR struct optee_priv_data *priv);

Review Comment:
   let's change `int` to `void`
   ```
   void optee_transport_close(FAR void *priv);
   ```



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)
+{
+  return (uintptr_t)va;
+}
+
 #define optee_safe_va_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_shm_to_page_list
+ *
+ * Description:
+ *   Provide a page list of a shared memory buffer. Secure world doesn't
+ *   know about the address environment mapping of NuttX, so physical
+ *   pointers are needed when sharing memory. This implementation enables
+ *   sharing of physically non-contiguous buffers according to
+ *   optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG.
+ *   Each entry in the generated page list is an array of the physical,
+ *   potentially non-contiguous pages making up the actual buffer to
+ *   represent. Note that this representation does not account for the page
+ *   offset of the shared memory buffer. The offset is encoded in the
+ *   physical address returned in `list_pa_p`.
+ *
+ * Parameters:
+ *   shme        - Shared memory entry to create a page list for.
+ *   list_pa_p   - If not NULL, will be set to the page list's physical
+ *                 address (which is aligned to
+ *                 OPTEE_MSG_NONCONTIG_PAGE_SIZE) added with shared memory
+ *                 page offset.
+ *
+ * Returned Values:
+ *   A pointer to the kernel virtual address of the page list on success.
+ *   NULL on failure. Caller responsible to free the returned list using
+ *   `kmm_free()`.
+ *
+ ****************************************************************************/
+
+FAR void *optee_shm_to_page_list(FAR struct optee_shm_entry *shme,
+                                 FAR uintptr_t *list_pa_p)
+{
+  const size_t pgsize = OPTEE_MSG_NONCONTIG_PAGE_SIZE;

Review Comment:
   remove const



##########
drivers/misc/optee.c:
##########
@@ -1185,5 +1189,13 @@ static int optee_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
 
 int optee_register(void)
 {
+  int ret;
+
+  ret = optee_transport_init();

Review Comment:
   move to the first patch



##########
drivers/misc/optee_smc.c:
##########
@@ -0,0 +1,323 @@
+/****************************************************************************
+ * drivers/misc/optee_smc.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <errno.h>
+#include <syslog.h>
+#include <string.h>
+#include <arch/syscall.h>
+
+#include "optee.h"
+#include "optee_smc.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_ARCH_ARM)
+#  define smccc_smc_fn               arm_smccc_smc
+#  define smccc_hvc_fn               arm_smccc_hvc
+#  define smccc_res_t                arm_smccc_res_t
+#elif defined(CONFIG_ARCH_ARM64)
+#  define smccc_smc_fn               arm64_smccc_smc
+#  define smccc_hvc_fn               arm64_smccc_hvc
+#  define smccc_res_t                arm64_smccc_res_t
+#else
+#  error "CONFIG_DEV_OPTEE_SMC is only supported in arm and arm64"
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC_CONDUIT_SMC
+#  define smc_conduit_fn             smccc_smc_fn
+#else
+#  define smc_conduit_fn             smccc_hvc_fn
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef void (optee_smc_fn)(unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long,
+                            smccc_res_t *);
+
+union os_revision

Review Comment:
   let's add optee_ prefix



##########
drivers/misc/optee.h:
##########
@@ -0,0 +1,77 @@
+/****************************************************************************
+ * drivers/misc/optee.h
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __DRIVERS_MISC_OPTEE_H
+#define __DRIVERS_MISC_OPTEE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/compiler.h>
+#include <nuttx/tee.h>
+
+#include "optee_msg.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define OPTEE_SERVER_PATH              "optee"
+#define OPTEE_MAX_PARAM_NUM            6
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+struct optee_priv_data;
+
+struct optee_priv_data
+{
+  FAR void *transport;
+};
+
+/****************************************************************************
+ * Public Functions Definitions
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+int optee_transport_init(void);
+int optee_transport_open(FAR struct optee_priv_data *priv);
+int optee_transport_close(FAR struct optee_priv_data *priv);
+int optee_transport_call(FAR struct optee_priv_data *priv,

Review Comment:
   `int optee_transport_call(FAR void *priv, FAR struct optee_msg_arg *arg);`



##########
drivers/misc/optee.h:
##########
@@ -0,0 +1,77 @@
+/****************************************************************************
+ * drivers/misc/optee.h
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __DRIVERS_MISC_OPTEE_H
+#define __DRIVERS_MISC_OPTEE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/compiler.h>
+#include <nuttx/tee.h>
+
+#include "optee_msg.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define OPTEE_SERVER_PATH              "optee"
+#define OPTEE_MAX_PARAM_NUM            6
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+struct optee_priv_data;
+
+struct optee_priv_data

Review Comment:
   let's move the definition into transport file, so each transport could 
define the different content



##########
drivers/misc/optee.c:
##########
@@ -283,9 +338,20 @@ static int optee_ioctl_open_session(FAR struct 
optee_priv_data *priv,
 {
   char msg_buf[OPTEE_MSG_GET_ARG_SIZE(OPTEE_MAX_PARAM_NUM)];
   FAR struct tee_ioctl_open_session_arg *arg;
+  struct tee_ioctl_close_session_arg close_arg;
   FAR struct optee_msg_arg *msg;
   int ret;
 
+  if (buf == NULL)

Review Comment:
   move into optee_safe_va_range



##########
drivers/misc/optee.c:
##########
@@ -348,8 +419,8 @@ static int optee_ioctl_open_session(FAR struct 
optee_priv_data *priv,
                              msg->params + 2);
   if (ret < 0)
     {
-      close_args.session = msg->session;
-      optee_ioctl_close_session(priv, &close_args);
+      close_arg.session = msg->session;
+      optee_ioctl_close_session(priv, &close_arg);

Review Comment:
   the check in optee_ioctl_close_session will fail, let's add a new function 
and change the second parameter to session



##########
drivers/misc/optee.c:
##########
@@ -334,6 +348,8 @@ static int optee_ioctl_open_session(FAR struct 
optee_priv_data *priv,
                              msg->params + 2);
   if (ret < 0)
     {
+      close_args.session = msg->session;

Review Comment:
   where we define close_args? let' rename close_args to close_arg



##########
drivers/misc/optee.c:
##########
@@ -125,52 +115,24 @@ static const struct file_operations g_optee_ops =
 
 static int optee_open(FAR struct file *filep)
 {
-  FAR struct socket *psock;
-#ifdef CONFIG_DEV_OPTEE_LOCAL
-  struct sockaddr_un addr;
-#else
-  struct sockaddr_rpmsg addr;
-#endif
+  FAR struct optee_priv_data *priv;
   int ret;
 
-  psock = (FAR struct socket *)kmm_zalloc(sizeof(struct socket));
-  if (psock == NULL)
+  priv = (FAR struct optee_priv_data *)
+            kmm_zalloc(sizeof(struct optee_priv_data));
+  if (priv == NULL)
     {
       return -ENOMEM;
     }
 
-#ifdef CONFIG_DEV_OPTEE_LOCAL
-  ret = psock_socket(AF_UNIX, SOCK_STREAM, 0, psock);
-#else
-  ret = psock_socket(AF_RPMSG, SOCK_STREAM, 0, psock);
-#endif
+  ret = optee_transport_open(priv);
   if (ret < 0)
     {
-      kmm_free(psock);

Review Comment:
   why remove



##########
drivers/misc/optee.c:
##########
@@ -446,27 +537,41 @@ static int optee_ioctl_cancel(FAR struct optee_priv_data 
*priv,
 static int
 optee_ioctl_shm_alloc(FAR struct tee_ioctl_shm_alloc_data *data)
 {
-  int memfd = memfd_create(OPTEE_SERVER_PATH, O_CREAT | O_CLOEXEC);
+  int memfd;
+
+  if (data == NULL)

Review Comment:
   let's move NULL check into optee_safe_va_range



##########
drivers/misc/optee.c:
##########
@@ -125,52 +115,24 @@ static const struct file_operations g_optee_ops =
 
 static int optee_open(FAR struct file *filep)
 {
-  FAR struct socket *psock;
-#ifdef CONFIG_DEV_OPTEE_LOCAL
-  struct sockaddr_un addr;
-#else
-  struct sockaddr_rpmsg addr;
-#endif
+  FAR struct optee_priv_data *priv;
   int ret;
 
-  psock = (FAR struct socket *)kmm_zalloc(sizeof(struct socket));
-  if (psock == NULL)
+  priv = (FAR struct optee_priv_data *)

Review Comment:
   let's pass filep->f_priv to transport directly:
   ```
   ret = optee_transport_open(&filep->f_priv);
   ```
   to avoid calling kmm_malloc twice.



##########
drivers/misc/optee.h:
##########
@@ -0,0 +1,77 @@
+/****************************************************************************
+ * drivers/misc/optee.h
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __DRIVERS_MISC_OPTEE_H
+#define __DRIVERS_MISC_OPTEE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/compiler.h>
+#include <nuttx/tee.h>
+
+#include "optee_msg.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define OPTEE_SERVER_PATH              "optee"
+#define OPTEE_MAX_PARAM_NUM            6
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+struct optee_priv_data;
+
+struct optee_priv_data
+{
+  FAR void *transport;
+};
+
+/****************************************************************************
+ * Public Functions Definitions
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+int optee_transport_init(void);
+int optee_transport_open(FAR struct optee_priv_data *priv);

Review Comment:
   `int optee_transport_open(FAR void **priv);`



##########
drivers/misc/optee.c:
##########
@@ -368,6 +439,16 @@ static int optee_ioctl_invoke(FAR struct optee_priv_data 
*priv,
   FAR struct optee_msg_arg *msg;
   int ret;
 
+  if (buf == NULL)

Review Comment:
   move into optee_safe_va_range



##########
drivers/misc/optee_smc.c:
##########
@@ -0,0 +1,323 @@
+/****************************************************************************
+ * drivers/misc/optee_smc.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <errno.h>
+#include <syslog.h>
+#include <string.h>
+#include <arch/syscall.h>
+
+#include "optee.h"
+#include "optee_smc.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_ARCH_ARM)
+#  define smccc_smc_fn               arm_smccc_smc
+#  define smccc_hvc_fn               arm_smccc_hvc
+#  define smccc_res_t                arm_smccc_res_t
+#elif defined(CONFIG_ARCH_ARM64)
+#  define smccc_smc_fn               arm64_smccc_smc
+#  define smccc_hvc_fn               arm64_smccc_hvc
+#  define smccc_res_t                arm64_smccc_res_t
+#else
+#  error "CONFIG_DEV_OPTEE_SMC is only supported in arm and arm64"
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC_CONDUIT_SMC
+#  define smc_conduit_fn             smccc_smc_fn
+#else
+#  define smc_conduit_fn             smccc_hvc_fn
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef void (optee_smc_fn)(unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long,
+                            smccc_res_t *);
+
+union os_revision
+{
+  smccc_res_t smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+
+union calls_revision
+{
+  smccc_res_t smccc;
+  struct optee_smc_calls_revision_result result;
+};
+
+union exchg_caps
+{
+  smccc_res_t smccc;
+  struct optee_smc_exchange_capabilities_result result;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static bool optee_smc_is_compatible(optee_smc_fn *smc_fn)
+{
+  smccc_res_t callsuid;
+  union os_revision osrev;
+  union calls_revision callsrev;
+  union exchg_caps xchgcaps;
+
+  /* Print the OS revision and build ID (if reported) */
+
+  osrev.result.build_id = 0;
+
+  smc_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0, &osrev.smccc);
+
+  if (osrev.result.build_id)
+    {
+      syslog(LOG_INFO, "OP-TEE: OS revision %lu.%lu (%08lx)\n",
+                       osrev.result.major, osrev.result.minor,
+                       osrev.result.build_id);
+    }
+  else
+    {
+      syslog(LOG_INFO, "OP-TEE: OS revision %lu.%lu\n",
+                       osrev.result.major, osrev.result.minor);
+    }
+
+  /* Check the API UID */
+
+  smc_fn(OPTEE_SMC_CALLS_UID, 0, 0, 0, 0, 0, 0, 0, &callsuid);
+
+  if (callsuid.a0 != OPTEE_MSG_UID_0 || callsuid.a1 != OPTEE_MSG_UID_1 ||
+      callsuid.a2 != OPTEE_MSG_UID_2 || callsuid.a3 != OPTEE_MSG_UID_3)
+    {
+      syslog(LOG_ERR, "OP-TEE: API UID mismatch\n");
+      return false;
+    }
+
+  /* Check the API revision */
+
+  smc_fn(OPTEE_SMC_CALLS_REVISION, 0, 0, 0, 0, 0, 0, 0, &callsrev.smccc);
+
+  if (callsrev.result.major != OPTEE_MSG_REVISION_MAJOR ||
+      (int)callsrev.result.minor < OPTEE_MSG_REVISION_MINOR)
+    {
+      syslog(LOG_ERR, "OP-TEE: API revision incompatible\n");
+      return false;
+    }
+
+  /* Check the capabilities */
+
+  smc_fn(OPTEE_SMC_EXCHANGE_CAPABILITIES, OPTEE_SMC_NSEC_CAP_UNIPROCESSOR,
+         0, 0, 0, 0, 0, 0, &xchgcaps.smccc);
+
+  if (xchgcaps.result.status != OPTEE_SMC_RETURN_OK)
+    {
+      syslog(LOG_ERR, "OP-TEE: Failed to exchange capabilities\n");
+      return false;
+    }
+
+  if (!(xchgcaps.result.capabilities & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM))
+    {
+      syslog(LOG_ERR, "OP-TEE: Does not support dynamic shared mem\n");
+      return false;
+    }
+
+  return true;
+}
+
+static void optee_smc_handle_rpc(FAR struct optee_priv_data *priv,
+                                 FAR smccc_res_t *par)
+{
+  FAR struct optee_shm_entry *shme;
+  uintptr_t shme_pa;
+  uint32_t rpc_func;
+  size_t shm_size;
+
+  rpc_func = OPTEE_SMC_RETURN_GET_RPC_FUNC(par->a0);
+  par->a0 = OPTEE_SMC_CALL_RETURN_FROM_RPC;
+
+  switch (rpc_func)
+    {
+      case OPTEE_SMC_RPC_FUNC_ALLOC:
+        {
+          shm_size = par->a1;
+          par->a1 = 0;
+          par->a2 = 0;
+          par->a4 = 0;
+          par->a5 = 0;
+
+          if (!priv->shm_alloc(priv, OPTEE_MSG_NONCONTIG_PAGE_SIZE, NULL,
+                               shm_size, TEE_SHM_ALLOC | TEE_SHM_REGISTER,
+                               &shme))
+            {
+              shme_pa = priv->va_to_pa(
+                          (FAR void *)(uintptr_t)shme->shm.addr);
+              par->a1 = shme_pa >> 32;
+              par->a2 = shme_pa & UINT32_MAX;
+              par->a4 = (uintptr_t)shme >> 32;
+              par->a5 = (uintptr_t)shme & UINT32_MAX;
+            }
+          break;
+        }
+
+      case OPTEE_SMC_RPC_FUNC_FREE:
+        {
+          shme = (FAR struct optee_shm_entry *)(uintptr_t)
+                  ((par->a1 << 32) | (par->a2 & UINT32_MAX));
+          priv->shm_free(shme);
+          break;
+        }
+
+      case OPTEE_SMC_RPC_FUNC_FOREIGN_INTR:
+        {
+          break;
+        }
+
+      default:
+        {
+          syslog(LOG_ERR, "OP-TEE: RPC 0x%04x not implemented\n", rpc_func);
+          break;
+        }
+    }
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: optee_transport_init
+ *
+ * Description:
+ *   Perform any initialization actions specific to the transport used
+ *   right before the driver is registered.
+ *
+ * Returned Values:
+ *   0 on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int optee_transport_init(void)
+{
+  if (!optee_smc_is_compatible(smc_conduit_fn))
+    {
+      return -ENOENT;
+    }
+
+  syslog(LOG_INFO, "OP-TEE: compatibility check complete\n");
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: optee_transport_open
+ *
+ * Description:
+ *   Perform any transport-specific actions upon driver character device
+ *   open.
+ *
+ * Returned Values:
+ *   0 on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int optee_transport_open(FAR struct optee_priv_data *priv)
+{
+  priv->transport = smc_conduit_fn;
+  return 0;
+}
+
+/****************************************************************************
+ * Name: optee_transport_close
+ *
+ * Description:
+ *   Perform any transport-specific actions upon driver character device
+ *   close.
+ *
+ * Returned Values:
+ *   0 on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int optee_transport_close(FAR struct optee_priv_data *priv)
+{
+  return 0;
+}
+
+/****************************************************************************
+ * Name: optee_transport_call
+ *
+ * Description:
+ *   Call OP-TEE OS using SMCs.
+ *
+ * Returned Values:
+ *   0 on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int optee_transport_call(FAR struct optee_priv_data *priv,
+                         FAR struct optee_msg_arg *arg)
+{
+  smccc_res_t res;
+  smccc_res_t par;
+  uintptr_t arg_pa;
+  optee_smc_fn *smc_fn = (optee_smc_fn *)priv->transport;
+
+  memset(&par, 0, sizeof(smccc_res_t));
+
+  par.a0 = OPTEE_SMC_CALL_WITH_ARG;
+
+  arg_pa = priv->va_to_pa(arg);
+  par.a1 = arg_pa >> 32;
+  par.a2 = arg_pa & UINT32_MAX;
+
+  while (1)

Review Comment:
   `for (; ; )`



##########
drivers/misc/optee.c:
##########
@@ -99,6 +104,51 @@ static const struct file_operations g_optee_ops =
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: optee_safe_va_range
+ *
+ * Description:
+ *   Check whether provided virtual address range belongs to user-owned
+ *   memory. If this function is called from a kernel thread, it returns
+ *   true. If this function is called in a build without CONFIG_ARCH_ADDRENV
+ *   it always returns true.
+ *
+ * Parameters:
+ *   va    - Beginning of address range to check.
+ *   size  - Size of memory to check.
+ *
+ * Returned Values:
+ *   True if the provided address range belongs to the user or the caller is
+ *   a kernel thread. False otherwise.
+ *
+ ****************************************************************************/
+
+ #ifdef CONFIG_ARCH_ADDRENV

Review Comment:
   remove the first space



##########
drivers/misc/optee.c:
##########
@@ -283,9 +338,20 @@ static int optee_ioctl_open_session(FAR struct 
optee_priv_data *priv,
 {
   char msg_buf[OPTEE_MSG_GET_ARG_SIZE(OPTEE_MAX_PARAM_NUM)];
   FAR struct tee_ioctl_open_session_arg *arg;
+  struct tee_ioctl_close_session_arg close_arg;

Review Comment:
   move to previous patch



##########
drivers/misc/optee.c:
##########
@@ -476,62 +956,83 @@ static int optee_ioctl_invoke(FAR struct optee_priv_data 
*priv,
   arg->ret = TEE_ERROR_COMMUNICATION;
   arg->ret_origin = TEE_ORIGIN_COMMS;
 
-  memset(msg_buf, 0, sizeof(msg_buf));
-  msg = (FAR struct optee_msg_arg *)&msg_buf[0];
+  msg = optee_shm_alloc_msg(priv, arg->num_params, &shme);
+  if (msg == NULL)
+    {
+      return -ENOMEM;
+    }
 
   msg->cmd = OPTEE_MSG_CMD_INVOKE_COMMAND;
   msg->func = arg->func;
   msg->session = arg->session;
   msg->cancel_id = arg->cancel_id;
-  msg->num_params = arg->num_params;
 
   ret = optee_to_msg_param(msg->params, arg->num_params, arg->params);
   if (ret < 0)
     {
-      return ret;
+      goto errout_with_msg;
     }
 
   ret = optee_transport_call(priv, msg);
   if (ret < 0)
     {
-      return ret;
+      goto errout_with_msg;
     }
 
   ret = optee_from_msg_param(arg->params, arg->num_params, msg->params);
   if (ret < 0)
     {
-      return ret;
+      goto errout_with_msg;
     }
 
   arg->ret = msg->ret;
   arg->ret_origin = msg->ret_origin;
 
+errout_with_msg:
+  optee_shm_free(shme);
+
   return ret;
 }
 
-static int optee_ioctl_version(FAR struct tee_ioctl_version_data *vers)
+static int
+optee_ioctl_version(FAR struct tee_ioctl_version_data *vers)
 {
   vers->impl_id = TEE_IMPL_ID_OPTEE;
   vers->impl_caps = TEE_OPTEE_CAP_TZ;
   vers->gen_caps = TEE_GEN_CAP_GP | TEE_GEN_CAP_MEMREF_NULL;
+#if 0
+  vers->gen_caps |= TEE_GEN_CAP_REG_MEM;
+#endif
   return 0;
 }
 
-static int optee_ioctl_cancel(FAR struct optee_priv_data *priv,
-                              FAR struct tee_ioctl_cancel_arg *arg)
+static int
+optee_ioctl_cancel(FAR struct optee_priv_data *priv,
+                   FAR struct tee_ioctl_cancel_arg *arg)
 {
-  struct optee_msg_arg msg;
+  FAR struct optee_shm_entry *shme;
+  FAR struct optee_msg_arg *msg;
+  int ret;
 
   if (!optee_safe_va_range(arg, sizeof(*arg)))
     {
       return -EACCES;
     }
 
-  memset(&msg, 0, sizeof(struct optee_msg_arg));
-  msg.cmd = OPTEE_MSG_CMD_CANCEL;
-  msg.session = arg->session;
-  msg.cancel_id = arg->cancel_id;
-  return optee_transport_call(priv, &msg);
+  msg = optee_shm_alloc_msg(priv, 0, &shme);

Review Comment:
   why need allocate shared memory



##########
drivers/misc/optee.c:
##########
@@ -99,6 +104,51 @@ static const struct file_operations g_optee_ops =
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: optee_safe_va_range
+ *
+ * Description:
+ *   Check whether provided virtual address range belongs to user-owned
+ *   memory. If this function is called from a kernel thread, it returns
+ *   true. If this function is called in a build without CONFIG_ARCH_ADDRENV
+ *   it always returns true.
+ *
+ * Parameters:
+ *   va    - Beginning of address range to check.
+ *   size  - Size of memory to check.
+ *
+ * Returned Values:
+ *   True if the provided address range belongs to the user or the caller is
+ *   a kernel thread. False otherwise.
+ *
+ ****************************************************************************/
+
+ #ifdef CONFIG_ARCH_ADDRENV
+static bool optee_safe_va_range(FAR void *va, size_t size)

Review Comment:
   optee_is_valid_range



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)
+{
+  return (uintptr_t)va;
+}
+
 #define optee_safe_va_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_shm_to_page_list
+ *
+ * Description:
+ *   Provide a page list of a shared memory buffer. Secure world doesn't
+ *   know about the address environment mapping of NuttX, so physical
+ *   pointers are needed when sharing memory. This implementation enables
+ *   sharing of physically non-contiguous buffers according to
+ *   optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG.
+ *   Each entry in the generated page list is an array of the physical,
+ *   potentially non-contiguous pages making up the actual buffer to
+ *   represent. Note that this representation does not account for the page
+ *   offset of the shared memory buffer. The offset is encoded in the
+ *   physical address returned in `list_pa_p`.
+ *
+ * Parameters:
+ *   shme        - Shared memory entry to create a page list for.
+ *   list_pa_p   - If not NULL, will be set to the page list's physical
+ *                 address (which is aligned to
+ *                 OPTEE_MSG_NONCONTIG_PAGE_SIZE) added with shared memory
+ *                 page offset.
+ *
+ * Returned Values:
+ *   A pointer to the kernel virtual address of the page list on success.
+ *   NULL on failure. Caller responsible to free the returned list using
+ *   `kmm_free()`.
+ *
+ ****************************************************************************/
+
+FAR void *optee_shm_to_page_list(FAR struct optee_shm_entry *shme,
+                                 FAR uintptr_t *list_pa_p)
+{
+  const size_t pgsize = OPTEE_MSG_NONCONTIG_PAGE_SIZE;
+  uintptr_t shm_pgoff;
+  uintptr_t shm_page;
+  uint32_t shm_total_pages;
+  uint32_t list_size;
+  FAR void *list;
+  FAR struct page_list_entry *list_entry;
+  uint32_t i;
+
+  shm_pgoff = shme->shm.addr & (pgsize - 1);
+  shm_total_pages = (uint32_t)
+              (ALIGN_UP(shm_pgoff + shme->shm.length, pgsize) / pgsize);
+  list_size = div_round_up(shm_total_pages, PAGES_ARRAY_LEN) * pgsize;
+
+  /* Page list's address should be page aligned, conveniently leaving
+   * log2(<page size>) zero least-significant bits to use as the page
+   * offset of the shm buffer (added last before return below).
+   */
+
+  list = kmm_memalign(pgsize, list_size);
+  if (!list)
+    {
+      return NULL;
+    }
+
+  list_entry = (FAR struct page_list_entry *)list;
+  shm_page = ALIGN_DOWN(shme->shm.addr, pgsize);
+  i = 0;
+  while (shm_total_pages)
+    {
+      list_entry->pages_array[i++] = optee_va_to_pa((FAR void *)shm_page);
+      shm_page += pgsize;
+      shm_total_pages--;
+
+      if (i == PAGES_ARRAY_LEN)
+        {
+          list_entry->next_page_data = optee_va_to_pa(list_entry + 1);
+          list_entry++;
+          i = 0;
+        }
+    }
+
+  if (list_pa_p)
+    {
+      *list_pa_p = optee_va_to_pa(list) | shm_pgoff;
+    }
+
+  return list;
+}
+
+/****************************************************************************
+ * Name: optee_shm_sec_register
+ *
+ * Description:
+ *   Register specified shared memory entry with OP-TEE.
+ *
+ * Parameters:
+ *   priv  - The driver's private data structure
+ *   shme  - Pointer to shared memory entry to register. The entry, the
+ *           contained shared memory object, or the referenced shared buffer
+ *           cannot be NULL.
+ *
+ * Returned Values:
+ *   0 on success, negative error code otherwise.
+ *
+ ****************************************************************************/
+
+static int optee_shm_sec_register(FAR struct optee_priv_data *priv,
+                                  FAR struct optee_shm_entry *shme)
+{
+  FAR struct optee_shm_entry *msg_shme;
+  FAR struct optee_msg_arg *msg;
+  FAR void *page_list;
+  uintptr_t page_list_pa;
+  int ret = 0;
+
+  msg = optee_shm_alloc_msg(priv, 1, &msg_shme);
+  if (msg == NULL)
+    {
+      return -ENOMEM;
+    }
+
+  page_list = optee_shm_to_page_list(shme, &page_list_pa);
+  if (page_list == NULL)
+    {
+      ret = -ENOMEM;
+      goto errout_with_msg;
+    }
+
+  msg->cmd = OPTEE_MSG_CMD_REGISTER_SHM;
+  msg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT |
+                        OPTEE_MSG_ATTR_NONCONTIG;
+  msg->params[0].u.tmem.buf_ptr = page_list_pa;
+  msg->params[0].u.tmem.shm_ref = shme->shm.addr;
+  msg->params[0].u.tmem.size = shme->shm.length;
+
+  if (optee_transport_call(priv, msg) || msg->ret)
+    {
+      ret = -EINVAL;
+    }
+
+  kmm_free(page_list);
+
+errout_with_msg:
+  optee_shm_free(msg_shme);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: optee_shm_sec_unregister
+ *
+ * Description:
+ *   Unregister specified shared memory entry with OP-TEE.
+ *
+ * Parameters:
+ *   priv  - the driver's private data structure
+ *   shme  - Pointer to shared memory entry to unregister. The shared
+ *           memory entry must have been previously registered previously
+ *           with the OP-TEE and cannot be NULL.
+ *
+ * Returned Values:
+ *   0 on success, negative error code otherwise.
+ *
+ ****************************************************************************/
+
+static int optee_shm_sec_unregister(FAR struct optee_priv_data *priv,
+                                    FAR struct optee_shm_entry *shme)
+{
+  FAR struct optee_shm_entry *msg_shme;
+  FAR struct optee_msg_arg *msg;
+  int ret = 0;
+
+  msg = optee_shm_alloc_msg(priv, 1, &msg_shme);
+  if (msg == NULL)
+    {
+      return -ENOMEM;
+    }
+
+  msg->cmd = OPTEE_MSG_CMD_UNREGISTER_SHM;
+  msg->params[0].attr = OPTEE_MSG_ATTR_TYPE_RMEM_INPUT;
+  msg->params[0].u.rmem.shm_ref = shme->shm.addr;
+
+  if (optee_transport_call(priv, msg) || msg->ret)

Review Comment:
   ```
   ret = optee_transport_call(priv, msg);
   if (ret >= 0)
     {
       ret = optee_convert_error(msg->ret);
     }
   ```
   let's add optee_convert_error to convert the optee error message to posix 
error code



##########
drivers/misc/optee.h:
##########
@@ -46,9 +47,29 @@
 
 struct optee_priv_data;
 
+struct optee_shm_entry
+{
+  struct optee_priv_data *priv;

Review Comment:
   add FAR for ALL pointers



##########
drivers/misc/optee.c:
##########
@@ -66,10 +69,23 @@
 
 #define OPTEE_DEV_PATH                  "/dev/tee0"
 
+/* According to optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG */
+
+#define PAGES_ARRAY_LEN \
+        ((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(uint64_t)) - 1)
+
 /****************************************************************************
  * Private Types
  ****************************************************************************/
 
+/* According to optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG documentation */
+
+struct page_list_entry

Review Comment:
   optee_page_list_entry



##########
drivers/misc/optee.c:
##########
@@ -66,10 +69,23 @@
 
 #define OPTEE_DEV_PATH                  "/dev/tee0"
 
+/* According to optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG */
+
+#define PAGES_ARRAY_LEN \

Review Comment:
   add OPTEE_ prefix



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)

Review Comment:
   `#  define optee_va_to_pa(va) ((uintptr_t)va)`



##########
drivers/misc/optee_smc.c:
##########
@@ -0,0 +1,323 @@
+/****************************************************************************
+ * drivers/misc/optee_smc.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <errno.h>
+#include <syslog.h>
+#include <string.h>
+#include <arch/syscall.h>
+
+#include "optee.h"
+#include "optee_smc.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_ARCH_ARM)
+#  define smccc_smc_fn               arm_smccc_smc
+#  define smccc_hvc_fn               arm_smccc_hvc
+#  define smccc_res_t                arm_smccc_res_t
+#elif defined(CONFIG_ARCH_ARM64)
+#  define smccc_smc_fn               arm64_smccc_smc
+#  define smccc_hvc_fn               arm64_smccc_hvc
+#  define smccc_res_t                arm64_smccc_res_t
+#else
+#  error "CONFIG_DEV_OPTEE_SMC is only supported in arm and arm64"
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC_CONDUIT_SMC
+#  define smc_conduit_fn             smccc_smc_fn
+#else
+#  define smc_conduit_fn             smccc_hvc_fn
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef void (optee_smc_fn)(unsigned long, unsigned long, unsigned long,

Review Comment:
   ypedef void (*optee_smc_fn)(unsigned long, unsigned long, unsigned long,



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)
+{
+  return (uintptr_t)va;
+}
+
 #define optee_safe_va_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_shm_to_page_list
+ *
+ * Description:
+ *   Provide a page list of a shared memory buffer. Secure world doesn't
+ *   know about the address environment mapping of NuttX, so physical
+ *   pointers are needed when sharing memory. This implementation enables
+ *   sharing of physically non-contiguous buffers according to
+ *   optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG.
+ *   Each entry in the generated page list is an array of the physical,
+ *   potentially non-contiguous pages making up the actual buffer to
+ *   represent. Note that this representation does not account for the page
+ *   offset of the shared memory buffer. The offset is encoded in the
+ *   physical address returned in `list_pa_p`.
+ *
+ * Parameters:
+ *   shme        - Shared memory entry to create a page list for.
+ *   list_pa_p   - If not NULL, will be set to the page list's physical
+ *                 address (which is aligned to
+ *                 OPTEE_MSG_NONCONTIG_PAGE_SIZE) added with shared memory
+ *                 page offset.
+ *
+ * Returned Values:
+ *   A pointer to the kernel virtual address of the page list on success.
+ *   NULL on failure. Caller responsible to free the returned list using
+ *   `kmm_free()`.
+ *
+ ****************************************************************************/
+
+FAR void *optee_shm_to_page_list(FAR struct optee_shm_entry *shme,
+                                 FAR uintptr_t *list_pa_p)
+{
+  const size_t pgsize = OPTEE_MSG_NONCONTIG_PAGE_SIZE;
+  uintptr_t shm_pgoff;
+  uintptr_t shm_page;
+  uint32_t shm_total_pages;

Review Comment:
   remove shm_ prefix



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)
+{
+  return (uintptr_t)va;
+}
+
 #define optee_safe_va_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_shm_to_page_list
+ *
+ * Description:
+ *   Provide a page list of a shared memory buffer. Secure world doesn't
+ *   know about the address environment mapping of NuttX, so physical
+ *   pointers are needed when sharing memory. This implementation enables
+ *   sharing of physically non-contiguous buffers according to
+ *   optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG.
+ *   Each entry in the generated page list is an array of the physical,
+ *   potentially non-contiguous pages making up the actual buffer to
+ *   represent. Note that this representation does not account for the page
+ *   offset of the shared memory buffer. The offset is encoded in the
+ *   physical address returned in `list_pa_p`.
+ *
+ * Parameters:
+ *   shme        - Shared memory entry to create a page list for.
+ *   list_pa_p   - If not NULL, will be set to the page list's physical
+ *                 address (which is aligned to
+ *                 OPTEE_MSG_NONCONTIG_PAGE_SIZE) added with shared memory
+ *                 page offset.
+ *
+ * Returned Values:
+ *   A pointer to the kernel virtual address of the page list on success.
+ *   NULL on failure. Caller responsible to free the returned list using
+ *   `kmm_free()`.
+ *
+ ****************************************************************************/
+
+FAR void *optee_shm_to_page_list(FAR struct optee_shm_entry *shme,
+                                 FAR uintptr_t *list_pa_p)
+{
+  const size_t pgsize = OPTEE_MSG_NONCONTIG_PAGE_SIZE;
+  uintptr_t shm_pgoff;
+  uintptr_t shm_page;
+  uint32_t shm_total_pages;
+  uint32_t list_size;
+  FAR void *list;
+  FAR struct page_list_entry *list_entry;
+  uint32_t i;
+
+  shm_pgoff = shme->shm.addr & (pgsize - 1);
+  shm_total_pages = (uint32_t)
+              (ALIGN_UP(shm_pgoff + shme->shm.length, pgsize) / pgsize);
+  list_size = div_round_up(shm_total_pages, PAGES_ARRAY_LEN) * pgsize;
+
+  /* Page list's address should be page aligned, conveniently leaving
+   * log2(<page size>) zero least-significant bits to use as the page
+   * offset of the shm buffer (added last before return below).
+   */
+
+  list = kmm_memalign(pgsize, list_size);
+  if (!list)
+    {
+      return NULL;
+    }
+
+  list_entry = (FAR struct page_list_entry *)list;
+  shm_page = ALIGN_DOWN(shme->shm.addr, pgsize);
+  i = 0;
+  while (shm_total_pages)
+    {
+      list_entry->pages_array[i++] = optee_va_to_pa((FAR void *)shm_page);
+      shm_page += pgsize;
+      shm_total_pages--;
+
+      if (i == PAGES_ARRAY_LEN)
+        {
+          list_entry->next_page_data = optee_va_to_pa(list_entry + 1);
+          list_entry++;
+          i = 0;
+        }
+    }
+
+  if (list_pa_p)
+    {
+      *list_pa_p = optee_va_to_pa(list) | shm_pgoff;
+    }
+
+  return list;
+}
+
+/****************************************************************************
+ * Name: optee_shm_sec_register
+ *
+ * Description:
+ *   Register specified shared memory entry with OP-TEE.
+ *
+ * Parameters:
+ *   priv  - The driver's private data structure
+ *   shme  - Pointer to shared memory entry to register. The entry, the
+ *           contained shared memory object, or the referenced shared buffer
+ *           cannot be NULL.
+ *
+ * Returned Values:
+ *   0 on success, negative error code otherwise.
+ *
+ ****************************************************************************/
+
+static int optee_shm_sec_register(FAR struct optee_priv_data *priv,
+                                  FAR struct optee_shm_entry *shme)
+{
+  FAR struct optee_shm_entry *msg_shme;
+  FAR struct optee_msg_arg *msg;
+  FAR void *page_list;
+  uintptr_t page_list_pa;
+  int ret = 0;
+
+  msg = optee_shm_alloc_msg(priv, 1, &msg_shme);
+  if (msg == NULL)
+    {
+      return -ENOMEM;
+    }
+
+  page_list = optee_shm_to_page_list(shme, &page_list_pa);
+  if (page_list == NULL)
+    {
+      ret = -ENOMEM;
+      goto errout_with_msg;
+    }
+
+  msg->cmd = OPTEE_MSG_CMD_REGISTER_SHM;
+  msg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT |
+                        OPTEE_MSG_ATTR_NONCONTIG;
+  msg->params[0].u.tmem.buf_ptr = page_list_pa;
+  msg->params[0].u.tmem.shm_ref = shme->shm.addr;
+  msg->params[0].u.tmem.size = shme->shm.length;
+
+  if (optee_transport_call(priv, msg) || msg->ret)
+    {
+      ret = -EINVAL;
+    }
+
+  kmm_free(page_list);
+
+errout_with_msg:
+  optee_shm_free(msg_shme);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: optee_shm_sec_unregister
+ *
+ * Description:
+ *   Unregister specified shared memory entry with OP-TEE.
+ *
+ * Parameters:
+ *   priv  - the driver's private data structure
+ *   shme  - Pointer to shared memory entry to unregister. The shared
+ *           memory entry must have been previously registered previously
+ *           with the OP-TEE and cannot be NULL.
+ *
+ * Returned Values:
+ *   0 on success, negative error code otherwise.
+ *
+ ****************************************************************************/
+
+static int optee_shm_sec_unregister(FAR struct optee_priv_data *priv,
+                                    FAR struct optee_shm_entry *shme)
+{
+  FAR struct optee_shm_entry *msg_shme;
+  FAR struct optee_msg_arg *msg;
+  int ret = 0;
+
+  msg = optee_shm_alloc_msg(priv, 1, &msg_shme);
+  if (msg == NULL)
+    {
+      return -ENOMEM;
+    }
+
+  msg->cmd = OPTEE_MSG_CMD_UNREGISTER_SHM;
+  msg->params[0].attr = OPTEE_MSG_ATTR_TYPE_RMEM_INPUT;
+  msg->params[0].u.rmem.shm_ref = shme->shm.addr;
+
+  if (optee_transport_call(priv, msg) || msg->ret)
+    {
+      ret = -EINVAL;
+    }
+
+  optee_shm_free(msg_shme);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: optee_shm_alloc
+ *
+ * Description:
+ *   Allocate, register and/or add shared memory for use with OP-TEE calls.
+ *   Shared memory entry suitable for use in shared memory linked list
+ *   returned in pass-by-reference `shmep` pointer. This function always
+ *   allocates a shared memory entry, regardless of flags. The rest of this
+ *   function's behaviour is largely determined by `flags`:
+ *   - If `TEE_SHM_ALLOC` is specified, then a buffer of length `size` and
+ *     alignment `align` will be allocated. In this case `addr` will be
+ *     ignored. This flag is reserved for kernel use only.
+ *   - If `TEE_SHM_SEC_REGISTER` is specified, then the memory specified by
+ *     `addr` or allocated through `TEE_SHM_ALLOC`, will be registered with
+ *     OP-TEE as dynamic shared memory.
+ *   - If `TEE_SHM_REGISTER` is specified, then the memory specified by
+ *     `addr` or allocated through `TEE_SHM_ALLOC`, will be added to the
+ *     driver's private data structure linked list of shared memory chunks.
+ *
+ *   Use `optee_shm_free()` to undo this operation, i.e. to free,
+ *   unregister, and/or remove from the list the entry returned in `shmep`
+ *   and the contained buffer.
+ *
+ * Parameters:
+ *   priv  - the driver's private data structure
+ *   align - the desired alignment for the shared memory to allocate.
+ *           Ignored when `flags` do not specify `TEE_SHM_ALLOC`.
+ *   addr  - the address of the shared memory to register with OP-TEE and/or
+ *           add to the driver's linked list of shared memory chunks.
+ *   size  - the size of the shared memory buffer to allocate/add/register.
+ *   flags - flags specifying the behaviour of this function. Supports
+ *           combinations of `TEE_SHM_{ALLOC,REGISTER,SEC_REGISTER}`.
+ *   shmep - Pass-by-reference pointer to return the shared memory entry
+ *           allocated. Cannot be NULL.
+ *
+ * Returned Values:
+ *   0 on success, negative error code otherwise.
+ *
+ ****************************************************************************/
+
+static int optee_shm_alloc(FAR struct optee_priv_data *priv, uintptr_t align,
+                           FAR void *addr, size_t size, uint32_t flags,
+                           FAR struct optee_shm_entry **shmep)
+{
+  FAR struct optee_shm_entry *shme;
+  FAR void *ptr;
+  int ret;
+
+  shme = (FAR struct optee_shm_entry *)
+            kmm_zalloc(sizeof(struct optee_shm_entry));
+  if (shme == NULL)
+    {
+      return -ENOMEM;
+    }
+
+  if (flags & TEE_SHM_ALLOC)
+    {
+      if (align)
+        {
+          ptr = kmm_memalign(align, size);
+        }
+      else
+        {
+          ptr = kmm_malloc(size);
+        }
+    }
+  else
+    {
+      ptr = addr;
+    }
+
+  if (ptr == NULL)
+    {
+      ret = -ENOMEM;
+      goto err;
+    }
+
+  shme->priv = priv;

Review Comment:
   why need save priv?



##########
drivers/misc/optee.h:
##########
@@ -46,9 +47,29 @@
 
 struct optee_priv_data;
 
+struct optee_shm_entry
+{
+  struct optee_priv_data *priv;
+  struct tee_ioctl_shm_register_data shm;
+  SLIST_ENTRY(optee_shm_entry) link;

Review Comment:
   could we use list from nuttx/list.h



##########
drivers/misc/optee.c:
##########
@@ -1000,7 +1000,7 @@ optee_ioctl_version(FAR struct tee_ioctl_version_data 
*vers)
   vers->impl_id = TEE_IMPL_ID_OPTEE;
   vers->impl_caps = TEE_OPTEE_CAP_TZ;
   vers->gen_caps = TEE_GEN_CAP_GP | TEE_GEN_CAP_MEMREF_NULL;
-#if 0
+#ifdef CONFIG_DEV_OPTEE_SMC

Review Comment:
   move the previous change here



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)
+{
+  return (uintptr_t)va;
+}
+
 #define optee_safe_va_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_shm_to_page_list
+ *
+ * Description:
+ *   Provide a page list of a shared memory buffer. Secure world doesn't
+ *   know about the address environment mapping of NuttX, so physical
+ *   pointers are needed when sharing memory. This implementation enables
+ *   sharing of physically non-contiguous buffers according to
+ *   optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG.
+ *   Each entry in the generated page list is an array of the physical,
+ *   potentially non-contiguous pages making up the actual buffer to
+ *   represent. Note that this representation does not account for the page
+ *   offset of the shared memory buffer. The offset is encoded in the
+ *   physical address returned in `list_pa_p`.
+ *
+ * Parameters:
+ *   shme        - Shared memory entry to create a page list for.
+ *   list_pa_p   - If not NULL, will be set to the page list's physical
+ *                 address (which is aligned to
+ *                 OPTEE_MSG_NONCONTIG_PAGE_SIZE) added with shared memory
+ *                 page offset.
+ *
+ * Returned Values:
+ *   A pointer to the kernel virtual address of the page list on success.
+ *   NULL on failure. Caller responsible to free the returned list using
+ *   `kmm_free()`.
+ *
+ ****************************************************************************/
+
+FAR void *optee_shm_to_page_list(FAR struct optee_shm_entry *shme,
+                                 FAR uintptr_t *list_pa_p)

Review Comment:
   list_pa_p->list_pa



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)
+{
+  return (uintptr_t)va;
+}
+
 #define optee_safe_va_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_shm_to_page_list
+ *
+ * Description:
+ *   Provide a page list of a shared memory buffer. Secure world doesn't
+ *   know about the address environment mapping of NuttX, so physical
+ *   pointers are needed when sharing memory. This implementation enables
+ *   sharing of physically non-contiguous buffers according to
+ *   optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG.
+ *   Each entry in the generated page list is an array of the physical,
+ *   potentially non-contiguous pages making up the actual buffer to
+ *   represent. Note that this representation does not account for the page
+ *   offset of the shared memory buffer. The offset is encoded in the
+ *   physical address returned in `list_pa_p`.
+ *
+ * Parameters:
+ *   shme        - Shared memory entry to create a page list for.
+ *   list_pa_p   - If not NULL, will be set to the page list's physical
+ *                 address (which is aligned to
+ *                 OPTEE_MSG_NONCONTIG_PAGE_SIZE) added with shared memory
+ *                 page offset.
+ *
+ * Returned Values:
+ *   A pointer to the kernel virtual address of the page list on success.
+ *   NULL on failure. Caller responsible to free the returned list using
+ *   `kmm_free()`.
+ *
+ ****************************************************************************/
+
+FAR void *optee_shm_to_page_list(FAR struct optee_shm_entry *shme,
+                                 FAR uintptr_t *list_pa_p)
+{
+  const size_t pgsize = OPTEE_MSG_NONCONTIG_PAGE_SIZE;
+  uintptr_t shm_pgoff;
+  uintptr_t shm_page;
+  uint32_t shm_total_pages;
+  uint32_t list_size;
+  FAR void *list;
+  FAR struct page_list_entry *list_entry;
+  uint32_t i;
+
+  shm_pgoff = shme->shm.addr & (pgsize - 1);
+  shm_total_pages = (uint32_t)
+              (ALIGN_UP(shm_pgoff + shme->shm.length, pgsize) / pgsize);
+  list_size = div_round_up(shm_total_pages, PAGES_ARRAY_LEN) * pgsize;
+
+  /* Page list's address should be page aligned, conveniently leaving
+   * log2(<page size>) zero least-significant bits to use as the page
+   * offset of the shm buffer (added last before return below).
+   */
+
+  list = kmm_memalign(pgsize, list_size);
+  if (!list)
+    {
+      return NULL;
+    }
+
+  list_entry = (FAR struct page_list_entry *)list;
+  shm_page = ALIGN_DOWN(shme->shm.addr, pgsize);
+  i = 0;

Review Comment:
   merge to line 258



##########
drivers/misc/Make.defs:
##########
@@ -77,8 +77,12 @@ endif
 
 ifneq ($(CONFIG_DEV_OPTEE_NONE),y)
   CSRCS += optee.c
+ifeq ($(CONFIG_DEV_OPTEE_SMC),y)

Review Comment:
   add indent



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)
+{
+  return (uintptr_t)va;
+}
+
 #define optee_safe_va_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_shm_to_page_list
+ *
+ * Description:
+ *   Provide a page list of a shared memory buffer. Secure world doesn't
+ *   know about the address environment mapping of NuttX, so physical
+ *   pointers are needed when sharing memory. This implementation enables
+ *   sharing of physically non-contiguous buffers according to
+ *   optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG.
+ *   Each entry in the generated page list is an array of the physical,
+ *   potentially non-contiguous pages making up the actual buffer to
+ *   represent. Note that this representation does not account for the page
+ *   offset of the shared memory buffer. The offset is encoded in the
+ *   physical address returned in `list_pa_p`.
+ *
+ * Parameters:
+ *   shme        - Shared memory entry to create a page list for.
+ *   list_pa_p   - If not NULL, will be set to the page list's physical
+ *                 address (which is aligned to
+ *                 OPTEE_MSG_NONCONTIG_PAGE_SIZE) added with shared memory
+ *                 page offset.
+ *
+ * Returned Values:
+ *   A pointer to the kernel virtual address of the page list on success.
+ *   NULL on failure. Caller responsible to free the returned list using
+ *   `kmm_free()`.
+ *
+ ****************************************************************************/
+
+FAR void *optee_shm_to_page_list(FAR struct optee_shm_entry *shme,
+                                 FAR uintptr_t *list_pa_p)
+{
+  const size_t pgsize = OPTEE_MSG_NONCONTIG_PAGE_SIZE;
+  uintptr_t shm_pgoff;
+  uintptr_t shm_page;
+  uint32_t shm_total_pages;
+  uint32_t list_size;
+  FAR void *list;
+  FAR struct page_list_entry *list_entry;
+  uint32_t i;
+
+  shm_pgoff = shme->shm.addr & (pgsize - 1);
+  shm_total_pages = (uint32_t)
+              (ALIGN_UP(shm_pgoff + shme->shm.length, pgsize) / pgsize);

Review Comment:
   `div_round_up(shm_pgoff + shme->shm.length, pgsize)`



##########
drivers/misc/optee_smc.c:
##########
@@ -0,0 +1,323 @@
+/****************************************************************************
+ * drivers/misc/optee_smc.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <errno.h>
+#include <syslog.h>
+#include <string.h>
+#include <arch/syscall.h>
+
+#include "optee.h"
+#include "optee_smc.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_ARCH_ARM)
+#  define smccc_smc_fn               arm_smccc_smc
+#  define smccc_hvc_fn               arm_smccc_hvc
+#  define smccc_res_t                arm_smccc_res_t
+#elif defined(CONFIG_ARCH_ARM64)
+#  define smccc_smc_fn               arm64_smccc_smc
+#  define smccc_hvc_fn               arm64_smccc_hvc
+#  define smccc_res_t                arm64_smccc_res_t
+#else
+#  error "CONFIG_DEV_OPTEE_SMC is only supported in arm and arm64"
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC_CONDUIT_SMC
+#  define smc_conduit_fn             smccc_smc_fn
+#else
+#  define smc_conduit_fn             smccc_hvc_fn
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef void (optee_smc_fn)(unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long,
+                            smccc_res_t *);

Review Comment:
   add FAR



##########
drivers/misc/optee.c:
##########
@@ -1075,6 +1075,7 @@ optee_ioctl_shm_alloc(FAR struct tee_ioctl_shm_alloc_data 
*data)
   return get_errno();
 }
 
+#ifdef CONFIG_DEV_OPTEE_SMC

Review Comment:
   let's remove the check, we are planing to support the shared memory for 
rpmsg/local socket too



##########
drivers/misc/optee_smc.c:
##########
@@ -0,0 +1,323 @@
+/****************************************************************************
+ * drivers/misc/optee_smc.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <errno.h>
+#include <syslog.h>
+#include <string.h>
+#include <arch/syscall.h>
+
+#include "optee.h"
+#include "optee_smc.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_ARCH_ARM)
+#  define smccc_smc_fn               arm_smccc_smc

Review Comment:
   remove ALL _fn suffix



##########
drivers/misc/optee_smc.c:
##########
@@ -0,0 +1,323 @@
+/****************************************************************************
+ * drivers/misc/optee_smc.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <errno.h>
+#include <syslog.h>
+#include <string.h>
+#include <arch/syscall.h>
+
+#include "optee.h"
+#include "optee_smc.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_ARCH_ARM)
+#  define smccc_smc_fn               arm_smccc_smc
+#  define smccc_hvc_fn               arm_smccc_hvc
+#  define smccc_res_t                arm_smccc_res_t
+#elif defined(CONFIG_ARCH_ARM64)
+#  define smccc_smc_fn               arm64_smccc_smc
+#  define smccc_hvc_fn               arm64_smccc_hvc
+#  define smccc_res_t                arm64_smccc_res_t
+#else
+#  error "CONFIG_DEV_OPTEE_SMC is only supported in arm and arm64"
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC_CONDUIT_SMC
+#  define smc_conduit_fn             smccc_smc_fn
+#else
+#  define smc_conduit_fn             smccc_hvc_fn
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef void (optee_smc_fn)(unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long,
+                            smccc_res_t *);
+
+union os_revision
+{
+  smccc_res_t smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+
+union calls_revision
+{
+  smccc_res_t smccc;
+  struct optee_smc_calls_revision_result result;
+};
+
+union exchg_caps
+{
+  smccc_res_t smccc;
+  struct optee_smc_exchange_capabilities_result result;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static bool optee_smc_is_compatible(optee_smc_fn *smc_fn)
+{
+  smccc_res_t callsuid;
+  union os_revision osrev;
+  union calls_revision callsrev;
+  union exchg_caps xchgcaps;
+
+  /* Print the OS revision and build ID (if reported) */
+
+  osrev.result.build_id = 0;
+
+  smc_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0, &osrev.smccc);
+
+  if (osrev.result.build_id)
+    {
+      syslog(LOG_INFO, "OP-TEE: OS revision %lu.%lu (%08lx)\n",
+                       osrev.result.major, osrev.result.minor,
+                       osrev.result.build_id);
+    }
+  else
+    {
+      syslog(LOG_INFO, "OP-TEE: OS revision %lu.%lu\n",
+                       osrev.result.major, osrev.result.minor);
+    }
+
+  /* Check the API UID */
+
+  smc_fn(OPTEE_SMC_CALLS_UID, 0, 0, 0, 0, 0, 0, 0, &callsuid);
+
+  if (callsuid.a0 != OPTEE_MSG_UID_0 || callsuid.a1 != OPTEE_MSG_UID_1 ||
+      callsuid.a2 != OPTEE_MSG_UID_2 || callsuid.a3 != OPTEE_MSG_UID_3)
+    {
+      syslog(LOG_ERR, "OP-TEE: API UID mismatch\n");
+      return false;
+    }
+
+  /* Check the API revision */
+
+  smc_fn(OPTEE_SMC_CALLS_REVISION, 0, 0, 0, 0, 0, 0, 0, &callsrev.smccc);
+
+  if (callsrev.result.major != OPTEE_MSG_REVISION_MAJOR ||
+      (int)callsrev.result.minor < OPTEE_MSG_REVISION_MINOR)
+    {
+      syslog(LOG_ERR, "OP-TEE: API revision incompatible\n");
+      return false;
+    }
+
+  /* Check the capabilities */
+
+  smc_fn(OPTEE_SMC_EXCHANGE_CAPABILITIES, OPTEE_SMC_NSEC_CAP_UNIPROCESSOR,
+         0, 0, 0, 0, 0, 0, &xchgcaps.smccc);
+
+  if (xchgcaps.result.status != OPTEE_SMC_RETURN_OK)
+    {
+      syslog(LOG_ERR, "OP-TEE: Failed to exchange capabilities\n");
+      return false;
+    }
+
+  if (!(xchgcaps.result.capabilities & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM))
+    {
+      syslog(LOG_ERR, "OP-TEE: Does not support dynamic shared mem\n");
+      return false;
+    }
+
+  return true;
+}
+
+static void optee_smc_handle_rpc(FAR struct optee_priv_data *priv,
+                                 FAR smccc_res_t *par)
+{
+  FAR struct optee_shm_entry *shme;
+  uintptr_t shme_pa;
+  uint32_t rpc_func;
+  size_t shm_size;
+
+  rpc_func = OPTEE_SMC_RETURN_GET_RPC_FUNC(par->a0);
+  par->a0 = OPTEE_SMC_CALL_RETURN_FROM_RPC;
+
+  switch (rpc_func)
+    {
+      case OPTEE_SMC_RPC_FUNC_ALLOC:
+        {
+          shm_size = par->a1;
+          par->a1 = 0;
+          par->a2 = 0;
+          par->a4 = 0;
+          par->a5 = 0;
+
+          if (!priv->shm_alloc(priv, OPTEE_MSG_NONCONTIG_PAGE_SIZE, NULL,
+                               shm_size, TEE_SHM_ALLOC | TEE_SHM_REGISTER,
+                               &shme))
+            {
+              shme_pa = priv->va_to_pa(
+                          (FAR void *)(uintptr_t)shme->shm.addr);
+              par->a1 = shme_pa >> 32;
+              par->a2 = shme_pa & UINT32_MAX;
+              par->a4 = (uintptr_t)shme >> 32;
+              par->a5 = (uintptr_t)shme & UINT32_MAX;
+            }
+          break;
+        }
+
+      case OPTEE_SMC_RPC_FUNC_FREE:
+        {
+          shme = (FAR struct optee_shm_entry *)(uintptr_t)
+                  ((par->a1 << 32) | (par->a2 & UINT32_MAX));
+          priv->shm_free(shme);
+          break;
+        }
+
+      case OPTEE_SMC_RPC_FUNC_FOREIGN_INTR:
+        {
+          break;
+        }
+
+      default:
+        {
+          syslog(LOG_ERR, "OP-TEE: RPC 0x%04x not implemented\n", rpc_func);
+          break;
+        }
+    }
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: optee_transport_init
+ *
+ * Description:
+ *   Perform any initialization actions specific to the transport used
+ *   right before the driver is registered.
+ *
+ * Returned Values:
+ *   0 on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int optee_transport_init(void)
+{
+  if (!optee_smc_is_compatible(smc_conduit_fn))
+    {
+      return -ENOENT;
+    }
+
+  syslog(LOG_INFO, "OP-TEE: compatibility check complete\n");
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: optee_transport_open
+ *
+ * Description:
+ *   Perform any transport-specific actions upon driver character device
+ *   open.
+ *
+ * Returned Values:
+ *   0 on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int optee_transport_open(FAR struct optee_priv_data *priv)
+{
+  priv->transport = smc_conduit_fn;
+  return 0;
+}
+
+/****************************************************************************
+ * Name: optee_transport_close
+ *
+ * Description:
+ *   Perform any transport-specific actions upon driver character device
+ *   close.
+ *
+ * Returned Values:
+ *   0 on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int optee_transport_close(FAR struct optee_priv_data *priv)
+{
+  return 0;
+}
+
+/****************************************************************************
+ * Name: optee_transport_call
+ *
+ * Description:
+ *   Call OP-TEE OS using SMCs.
+ *
+ * Returned Values:
+ *   0 on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int optee_transport_call(FAR struct optee_priv_data *priv,
+                         FAR struct optee_msg_arg *arg)
+{
+  smccc_res_t res;
+  smccc_res_t par;
+  uintptr_t arg_pa;
+  optee_smc_fn *smc_fn = (optee_smc_fn *)priv->transport;
+
+  memset(&par, 0, sizeof(smccc_res_t));
+
+  par.a0 = OPTEE_SMC_CALL_WITH_ARG;
+
+  arg_pa = priv->va_to_pa(arg);

Review Comment:
   let's call the function directly instead through pointer



##########
drivers/misc/optee.c:
##########
@@ -175,6 +625,12 @@ static int optee_open(FAR struct file *filep)
       return -ENOMEM;
     }
 
+  SLIST_INIT(&priv->list_shm);
+
+  priv->shm_alloc = optee_shm_alloc;

Review Comment:
   why not call optee_shm_alloc directly, but through function pointer



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)
+{
+  return (uintptr_t)va;
+}
+
 #define optee_safe_va_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_shm_to_page_list
+ *
+ * Description:
+ *   Provide a page list of a shared memory buffer. Secure world doesn't
+ *   know about the address environment mapping of NuttX, so physical
+ *   pointers are needed when sharing memory. This implementation enables
+ *   sharing of physically non-contiguous buffers according to
+ *   optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG.
+ *   Each entry in the generated page list is an array of the physical,
+ *   potentially non-contiguous pages making up the actual buffer to
+ *   represent. Note that this representation does not account for the page
+ *   offset of the shared memory buffer. The offset is encoded in the
+ *   physical address returned in `list_pa_p`.
+ *
+ * Parameters:
+ *   shme        - Shared memory entry to create a page list for.
+ *   list_pa_p   - If not NULL, will be set to the page list's physical
+ *                 address (which is aligned to
+ *                 OPTEE_MSG_NONCONTIG_PAGE_SIZE) added with shared memory
+ *                 page offset.
+ *
+ * Returned Values:
+ *   A pointer to the kernel virtual address of the page list on success.
+ *   NULL on failure. Caller responsible to free the returned list using
+ *   `kmm_free()`.
+ *
+ ****************************************************************************/
+
+FAR void *optee_shm_to_page_list(FAR struct optee_shm_entry *shme,
+                                 FAR uintptr_t *list_pa_p)
+{
+  const size_t pgsize = OPTEE_MSG_NONCONTIG_PAGE_SIZE;
+  uintptr_t shm_pgoff;
+  uintptr_t shm_page;
+  uint32_t shm_total_pages;
+  uint32_t list_size;
+  FAR void *list;
+  FAR struct page_list_entry *list_entry;
+  uint32_t i;
+
+  shm_pgoff = shme->shm.addr & (pgsize - 1);
+  shm_total_pages = (uint32_t)
+              (ALIGN_UP(shm_pgoff + shme->shm.length, pgsize) / pgsize);
+  list_size = div_round_up(shm_total_pages, PAGES_ARRAY_LEN) * pgsize;

Review Comment:
   `ALIGN_UP(shm_total_pages, PAGES_ARRAY_LEN)`



##########
drivers/misc/optee.c:
##########
@@ -145,10 +207,398 @@ static bool optee_safe_va_range(FAR void *va, size_t 
size)
 
   return false;
 }
-#else
+#else /* !CONFIG_ARCH_ADDRENV */
+static uintptr_t optee_va_to_pa(FAR void *va)
+{
+  return (uintptr_t)va;
+}
+
 #define optee_safe_va_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_shm_to_page_list
+ *
+ * Description:
+ *   Provide a page list of a shared memory buffer. Secure world doesn't
+ *   know about the address environment mapping of NuttX, so physical
+ *   pointers are needed when sharing memory. This implementation enables
+ *   sharing of physically non-contiguous buffers according to
+ *   optee_msg.h#OPTEE_MSG_ATTR_NONCONTIG.
+ *   Each entry in the generated page list is an array of the physical,
+ *   potentially non-contiguous pages making up the actual buffer to
+ *   represent. Note that this representation does not account for the page
+ *   offset of the shared memory buffer. The offset is encoded in the
+ *   physical address returned in `list_pa_p`.
+ *
+ * Parameters:
+ *   shme        - Shared memory entry to create a page list for.
+ *   list_pa_p   - If not NULL, will be set to the page list's physical
+ *                 address (which is aligned to
+ *                 OPTEE_MSG_NONCONTIG_PAGE_SIZE) added with shared memory
+ *                 page offset.
+ *
+ * Returned Values:
+ *   A pointer to the kernel virtual address of the page list on success.
+ *   NULL on failure. Caller responsible to free the returned list using
+ *   `kmm_free()`.
+ *
+ ****************************************************************************/
+
+FAR void *optee_shm_to_page_list(FAR struct optee_shm_entry *shme,
+                                 FAR uintptr_t *list_pa_p)
+{
+  const size_t pgsize = OPTEE_MSG_NONCONTIG_PAGE_SIZE;
+  uintptr_t shm_pgoff;
+  uintptr_t shm_page;
+  uint32_t shm_total_pages;
+  uint32_t list_size;
+  FAR void *list;
+  FAR struct page_list_entry *list_entry;
+  uint32_t i;
+
+  shm_pgoff = shme->shm.addr & (pgsize - 1);
+  shm_total_pages = (uint32_t)
+              (ALIGN_UP(shm_pgoff + shme->shm.length, pgsize) / pgsize);
+  list_size = div_round_up(shm_total_pages, PAGES_ARRAY_LEN) * pgsize;
+
+  /* Page list's address should be page aligned, conveniently leaving
+   * log2(<page size>) zero least-significant bits to use as the page
+   * offset of the shm buffer (added last before return below).
+   */
+
+  list = kmm_memalign(pgsize, list_size);
+  if (!list)
+    {
+      return NULL;
+    }
+
+  list_entry = (FAR struct page_list_entry *)list;
+  shm_page = ALIGN_DOWN(shme->shm.addr, pgsize);
+  i = 0;
+  while (shm_total_pages)
+    {
+      list_entry->pages_array[i++] = optee_va_to_pa((FAR void *)shm_page);
+      shm_page += pgsize;
+      shm_total_pages--;
+
+      if (i == PAGES_ARRAY_LEN)
+        {
+          list_entry->next_page_data = optee_va_to_pa(list_entry + 1);
+          list_entry++;
+          i = 0;
+        }
+    }
+
+  if (list_pa_p)
+    {
+      *list_pa_p = optee_va_to_pa(list) | shm_pgoff;
+    }
+
+  return list;
+}
+
+/****************************************************************************
+ * Name: optee_shm_sec_register
+ *
+ * Description:
+ *   Register specified shared memory entry with OP-TEE.
+ *
+ * Parameters:
+ *   priv  - The driver's private data structure
+ *   shme  - Pointer to shared memory entry to register. The entry, the
+ *           contained shared memory object, or the referenced shared buffer
+ *           cannot be NULL.
+ *
+ * Returned Values:
+ *   0 on success, negative error code otherwise.
+ *
+ ****************************************************************************/
+
+static int optee_shm_sec_register(FAR struct optee_priv_data *priv,
+                                  FAR struct optee_shm_entry *shme)
+{
+  FAR struct optee_shm_entry *msg_shme;
+  FAR struct optee_msg_arg *msg;
+  FAR void *page_list;
+  uintptr_t page_list_pa;
+  int ret = 0;
+
+  msg = optee_shm_alloc_msg(priv, 1, &msg_shme);
+  if (msg == NULL)
+    {
+      return -ENOMEM;
+    }
+
+  page_list = optee_shm_to_page_list(shme, &page_list_pa);
+  if (page_list == NULL)
+    {
+      ret = -ENOMEM;
+      goto errout_with_msg;
+    }
+
+  msg->cmd = OPTEE_MSG_CMD_REGISTER_SHM;
+  msg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT |
+                        OPTEE_MSG_ATTR_NONCONTIG;
+  msg->params[0].u.tmem.buf_ptr = page_list_pa;
+  msg->params[0].u.tmem.shm_ref = shme->shm.addr;
+  msg->params[0].u.tmem.size = shme->shm.length;
+
+  if (optee_transport_call(priv, msg) || msg->ret)
+    {
+      ret = -EINVAL;
+    }
+
+  kmm_free(page_list);
+
+errout_with_msg:
+  optee_shm_free(msg_shme);
+

Review Comment:
   remove blank line



##########
drivers/misc/optee_smc.c:
##########
@@ -0,0 +1,323 @@
+/****************************************************************************
+ * drivers/misc/optee_smc.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <errno.h>
+#include <syslog.h>
+#include <string.h>
+#include <arch/syscall.h>
+
+#include "optee.h"
+#include "optee_smc.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_ARCH_ARM)
+#  define smccc_smc_fn               arm_smccc_smc
+#  define smccc_hvc_fn               arm_smccc_hvc
+#  define smccc_res_t                arm_smccc_res_t
+#elif defined(CONFIG_ARCH_ARM64)
+#  define smccc_smc_fn               arm64_smccc_smc
+#  define smccc_hvc_fn               arm64_smccc_hvc
+#  define smccc_res_t                arm64_smccc_res_t
+#else
+#  error "CONFIG_DEV_OPTEE_SMC is only supported in arm and arm64"
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC_CONDUIT_SMC
+#  define smc_conduit_fn             smccc_smc_fn
+#else
+#  define smc_conduit_fn             smccc_hvc_fn
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef void (optee_smc_fn)(unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long, unsigned long,
+                            unsigned long, unsigned long,
+                            smccc_res_t *);
+
+union os_revision
+{
+  smccc_res_t smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+
+union calls_revision
+{
+  smccc_res_t smccc;
+  struct optee_smc_calls_revision_result result;
+};
+
+union exchg_caps
+{
+  smccc_res_t smccc;
+  struct optee_smc_exchange_capabilities_result result;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static bool optee_smc_is_compatible(optee_smc_fn *smc_fn)

Review Comment:
   remove `*` from all optee_smc_fn 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to