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


##########
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:
   > OP-TEE rejects it if it's not 4KB-aligned 
(`OPTEE_MSG_NONCONTIG_PAGE_SIZE`) and `optee_shm_alloc()` aligns it. The same 
goes for all calls.
   > 
   
   Ok, this may because MMU require 4KB page size, and OP-TEE access the 
parameter pointer directly in SMC case. But for socket ,OP-TEE side can't 
access the buffer directly, but need call recv to copy the parameter into it's 
internal buffer, so the alignment isn't required at all.
   Could you move it to SMC backend.
   
   > `OPTEE_MSG_MAX_NUM_PARAMS()` operates on this assumption, too. See 
[here](https://github.com/OP-TEE/optee_os/blob/76d920d354df0940181bfbd0d7f4a71883e0a2bf/core/include/optee_msg.h#L234C9-L234C33).
   > 
   > This must be something new compared to when the initial contribution to 
NuttX was made, cause I don't see it in our optee_msg.h
   
   Yes, we run OP-TEE in Cortex-M TrustZone which is totally different from 
Cortex-A environment.
   
   > 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.
   
   Yes, I think so.



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