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


##########
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:
   What we could perhaps do is replace the call to `optee_shm_alloc()` with a 
plain call to `kmm_memalign()`, and save one allocation for the shm entry.
   
   I'll do that.



##########
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:
   What we could perhaps do is replace the call to `optee_shm_alloc()` with a 
direct call to `kmm_memalign()`, and save one allocation for the shm entry.
   
   I'll do that.



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