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


##########
drivers/misc/optee.c:
##########
@@ -70,7 +70,7 @@
 
 #define TEE_ORIGIN_COMMS               0x00000002
 
-#define OPTEE_DEV_PATH                  "/dev/tee0"
+#define OPTEE_DEV_PATH                 "/dev/tee0"

Review Comment:
   let's move to the first patch too



##########
drivers/misc/optee.h:
##########
@@ -27,9 +27,11 @@
  * Included Files
  ****************************************************************************/
 
+#include <stdbool.h>

Review Comment:
   remove, don't need now



##########
drivers/misc/optee.c:
##########
@@ -154,6 +202,245 @@ static bool optee_is_valid_range(FAR const void *va, 
size_t size)
 #  define optee_is_valid_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_msg_alloc
+ *
+ * Description:
+ *   Allocate OP-TEE page-aligned memory for use as arguments to OP-TEE
+ *   calls, large enough to fit `num_params` parameters. Initialize the
+ *   buffer to 0, and set the `.num_params` field to the specified value.
+ *
+ *   Use `kmm_free()` to free the memory returned.
+ *
+ * Parameters:
+ *   priv       - the driver's private data structure
+ *   num_params - the number of arguments to allocate shared memory for.
+ *                Can be zero.
+ *
+ * Returned Values:
+ *   On success, pointer to OP-TEE message arguments struct initialized
+ *   to 0 except for `.num_params`, which is initialized to the specified.
+ *   On failure, NULL.
+ *
+ ****************************************************************************/
+
+static FAR struct optee_msg_arg *
+optee_msg_alloc(FAR struct optee_priv_data *priv, uint32_t num_params)
+{
+  FAR struct optee_msg_arg *msg;
+
+  if (priv->alignment)
+    {
+      msg = kmm_memalign(priv->alignment,
+                         OPTEE_MSG_GET_ARG_SIZE(num_params));
+    }
+  else
+    {
+      msg = kmm_malloc(OPTEE_MSG_GET_ARG_SIZE(num_params));

Review Comment:
   could we change to alloca here to avoid the dynamic allocation in the simple 
case? of course, we may need change optee_msg_alloc to macro and add a simple 
optee_msg_free macro to skip free in the simple case.



##########
drivers/misc/optee.c:
##########
@@ -154,6 +202,245 @@ static bool optee_is_valid_range(FAR const void *va, 
size_t size)
 #  define optee_is_valid_range(addr, size) (true)
 #endif
 
+/****************************************************************************
+ * Name: optee_msg_alloc
+ *
+ * Description:
+ *   Allocate OP-TEE page-aligned memory for use as arguments to OP-TEE
+ *   calls, large enough to fit `num_params` parameters. Initialize the
+ *   buffer to 0, and set the `.num_params` field to the specified value.
+ *
+ *   Use `kmm_free()` to free the memory returned.
+ *
+ * Parameters:
+ *   priv       - the driver's private data structure
+ *   num_params - the number of arguments to allocate shared memory for.
+ *                Can be zero.
+ *
+ * Returned Values:
+ *   On success, pointer to OP-TEE message arguments struct initialized
+ *   to 0 except for `.num_params`, which is initialized to the specified.
+ *   On failure, NULL.
+ *
+ ****************************************************************************/
+
+static FAR struct optee_msg_arg *
+optee_msg_alloc(FAR struct optee_priv_data *priv, uint32_t num_params)
+{
+  FAR struct optee_msg_arg *msg;
+
+  if (priv->alignment)

Review Comment:
   we can optimize to:
   ```
   if (priv->alignment > sizeof(uintptr_t))
   ```



##########
drivers/misc/optee_smc.c:
##########
@@ -0,0 +1,338 @@
+/****************************************************************************
+ * 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/kmalloc.h>
+#include <errno.h>
+#include <syslog.h>
+#include <string.h>
+
+#include "optee.h"
+#include "optee_smc.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_ARCH_ARM)
+#  define smccc_smc                  arm_smccc_smc
+#  define smccc_hvc                  arm_smccc_hvc
+#  define smccc_res_t                arm_smccc_res_t
+#elif defined(CONFIG_ARCH_ARM64)
+#  define smccc_smc                  arm64_smccc_smc
+#  define smccc_hvc                  arm64_smccc_hvc
+#  define smccc_res_t                arm64_smccc_res_t
+#else
+#  error "CONFIG_DEV_OPTEE_SMC is only supported on arm and arm64"
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC_CONDUIT_SMC
+#  define smc_conduit                smccc_smc
+#else
+#  define smc_conduit                smccc_hvc
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef void (*optee_smc_fn)(unsigned long, unsigned long, unsigned long,
+                             unsigned long, unsigned long, unsigned long,
+                             unsigned long, unsigned long,
+                             FAR smccc_res_t *);
+
+struct optee_smc_priv_data
+{
+  struct optee_priv_data base;
+  optee_smc_fn smc_fn;
+};
+
+union optee_smc_os_revision
+{
+  smccc_res_t smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+
+union optee_smc_calls_revision
+{
+  smccc_res_t smccc;
+  struct optee_smc_calls_revision_result result;
+};
+
+union optee_smc_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)
+{
+  union optee_smc_os_revision osrev;
+  union optee_smc_calls_revision callsrev;
+  union optee_smc_exchg_caps xchgcaps;
+  smccc_res_t callsuid;
+
+  /* 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 ||
+      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 (!optee_shm_alloc(priv_, NULL, shm_size,
+                              TEE_SHM_ALLOC | TEE_SHM_REGISTER, &shme))

Review Comment:
   align



-- 
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