xiaoxiang781216 commented on code in PR #16734:
URL: https://github.com/apache/nuttx/pull/16734#discussion_r2211068328
##########
drivers/misc/optee.c:
##########
@@ -1225,7 +1225,8 @@ int optee_shm_alloc(FAR struct optee_priv_data *priv, FAR
void *addr,
shm->fd = -1;
shm->priv = priv;
- shm->addr = (uintptr_t)ptr;
+ shm->vaddr = (uintptr_t)ptr;
+ shm->paddr = optee_va_to_pa((void *)shm->vaddr);
Review Comment:
add FAR to `void *`
##########
drivers/misc/optee.c:
##########
@@ -34,6 +34,8 @@
#include <nuttx/lib/math32.h>
#include <sys/mman.h>
#include <sys/param.h>
Review Comment:
add blank line
##########
drivers/misc/Kconfig:
##########
@@ -70,6 +70,11 @@ config DEV_OPTEE_NONE
endchoice
+config DEV_OPTEE_SUPPLICANT
+ bool "Enables kernel driver for TEE supplicant"
+ default n
+ depends on DEV_OPTEE_SMC
Review Comment:
why depends on SMC backend?
##########
drivers/misc/optee.c:
##########
@@ -1218,17 +1241,21 @@ int optee_shm_alloc(FAR struct optee_priv_data *priv,
FAR void *addr,
ptr = addr;
}
- if (ptr == NULL)
+ if (!(flags & TEE_SHM_USER_MAP))
{
- goto err;
+ if (ptr == NULL)
+ {
+ return -EINVAL;
+ }
}
shm->fd = -1;
shm->priv = priv;
shm->vaddr = (uintptr_t)ptr;
- shm->paddr = optee_va_to_pa((void *)shm->vaddr);
+ shm->paddr = shm->vaddr? optee_va_to_pa((void *)shm->vaddr): 0;
shm->length = size;
shm->flags = flags;
+
shm->id = idr_alloc(priv->shms, shm, 0, 0);
Review Comment:
remove the blank line
##########
drivers/misc/optee.h:
##########
@@ -38,26 +38,50 @@
* Pre-processor Definitions
****************************************************************************/
+/* Some GlobalPlatform error codes used in this driver */
+
+#define TEE_SUCCESS 0x00000000
+#define TEE_ERROR_ACCESS_DENIED 0xFFFF0001
+#define TEE_ERROR_BAD_FORMAT 0xFFFF0005
+#define TEE_ERROR_BAD_PARAMETERS 0xFFFF0006
+#define TEE_ERROR_GENERIC 0xFFFF0000
+#define TEE_ERROR_NOT_SUPPORTED 0xFFFF000A
+#define TEE_ERROR_OUT_OF_MEMORY 0xFFFF000C
+#define TEE_ERROR_BUSY 0xFFFF000D
+#define TEE_ERROR_COMMUNICATION 0xFFFF000E
+#define TEE_ERROR_SECURITY 0xFFFF000F
+#define TEE_ERROR_SHORT_BUFFER 0xFFFF0010
+#define TEE_ERROR_TIMEOUT 0xFFFF3001
+
+#define TEE_ORIGIN_COMMS 0x00000002
+
#define OPTEE_SERVER_PATH "optee"
#define OPTEE_MAX_PARAM_NUM 6
/****************************************************************************
* Public Types
****************************************************************************/
+enum optee_role_e
+{
+ OPTEE_ROLE_CA, /* /dev/tee0 */
+ OPTEE_ROLE_SUPPLICANT, /* /dev/tee-supp0 */
Review Comment:
align the comment to the same column of the comment at line 73
##########
drivers/misc/optee_msg.h:
##########
@@ -352,4 +354,122 @@ struct optee_msg_arg
#define OPTEE_MSG_CMD_STOP_ASYNC_NOTIF 7
#define OPTEE_MSG_FUNCID_CALL_WITH_ARG 0x0004
+/****************************************************************************
+ * Part 3 - Requests from secure world, RPC
+ ****************************************************************************/
+
+/* All RPC is done with a struct optee_msg_arg as bearer of information,
+ * struct optee_msg_arg::arg holds values defined by OPTEE_MSG_RPC_CMD_*
+ * below
+ *
+ * RPC communication with tee-supplicant is reversed compared to normal
+ * client communication described above. The supplicant receives requests
+ * and sends responses.
+ */
+
+/* Load a TA into memory, defined in tee-supplicant
+ */
+#define OPTEE_MSG_RPC_CMD_LOAD_TA 0
+
+/* Reserved
+ */
Review Comment:
add blank line after ALL comment per style requirement.
##########
drivers/misc/optee.c:
##########
@@ -980,49 +1018,34 @@ optee_ioctl_shm_alloc(FAR struct optee_priv_data *priv,
FAR struct tee_ioctl_shm_alloc_data *data)
{
FAR struct optee_shm *shm;
- FAR void *addr;
- int memfd;
int ret;
if (!optee_is_valid_range(data, sizeof(*data)))
{
return -EFAULT;
}
- memfd = memfd_create(OPTEE_SERVER_PATH, O_CREAT | O_CLOEXEC);
- if (memfd < 0)
+ ret = optee_shm_alloc(priv, 0, data->size, TEE_SHM_USER_MAP, &shm);
Review Comment:
0->NULL
##########
drivers/misc/optee_msg.h:
##########
@@ -352,4 +354,122 @@ struct optee_msg_arg
#define OPTEE_MSG_CMD_STOP_ASYNC_NOTIF 7
#define OPTEE_MSG_FUNCID_CALL_WITH_ARG 0x0004
+/****************************************************************************
+ * Part 3 - Requests from secure world, RPC
+ ****************************************************************************/
+
+/* All RPC is done with a struct optee_msg_arg as bearer of information,
+ * struct optee_msg_arg::arg holds values defined by OPTEE_MSG_RPC_CMD_*
+ * below
+ *
+ * RPC communication with tee-supplicant is reversed compared to normal
+ * client communication described above. The supplicant receives requests
+ * and sends responses.
+ */
+
+/* Load a TA into memory, defined in tee-supplicant
+ */
+#define OPTEE_MSG_RPC_CMD_LOAD_TA 0
Review Comment:
align ALL number to the same column as line 355
##########
Documentation/guides/optee.rst:
##########
@@ -14,8 +14,16 @@ not officially supported by NuttX, and is out of the scope
of this guide.
The driver supports opening and closing sessions, allocating and registering
shared memory, and invoking functions on OP-TEE Trusted Applications (TAs).
-It does not (yet) support reverse direction commands (TA -> Normal World)
-to something like a TEE supplicant.
+The driver also supports, reverse direction commands called RPCs
+(TA -> Normal World). Some of the RPCs are handled completely by the kernel
+driver while others require the TEE supplicant userspace process to be running
+by having opened (``/dev/teepriv#``). Similarly to libteec, the supplicant
Review Comment:
could you add the userspace supplicant code to
https://github.com/apache/nuttx-apps/tree/master/tee
##########
drivers/misc/optee.c:
##########
@@ -34,6 +34,8 @@
#include <nuttx/lib/math32.h>
#include <sys/mman.h>
#include <sys/param.h>
+#include "fs_anonmap.h"
+#include "fs_rammap.h"
Review Comment:
should we move the internal api to include/nuttx/fs/fs.h to avoid adding
fs/mmap to search path and include the private fs headers
##########
drivers/misc/optee.c:
##########
@@ -1218,17 +1241,21 @@ int optee_shm_alloc(FAR struct optee_priv_data *priv,
FAR void *addr,
ptr = addr;
}
- if (ptr == NULL)
+ if (!(flags & TEE_SHM_USER_MAP))
{
- goto err;
+ if (ptr == NULL)
+ {
+ return -EINVAL;
+ }
}
shm->fd = -1;
shm->priv = priv;
shm->vaddr = (uintptr_t)ptr;
- shm->paddr = optee_va_to_pa((void *)shm->vaddr);
+ shm->paddr = shm->vaddr? optee_va_to_pa((void *)shm->vaddr): 0;
Review Comment:
`shm->paddr = shm->vaddr ? optee_va_to_pa((FAR void *)shm->vaddr) : 0;`
##########
drivers/misc/optee_smc.c:
##########
@@ -318,6 +323,7 @@ int optee_transport_call(FAR struct optee_priv_data *priv_,
up_clean_dcache((uintptr_t)arg, (uintptr_t)arg + arg_size);
#endif
+ void *last_page_list = 0;
Review Comment:
0->NULL
##########
drivers/misc/optee_msg.h:
##########
@@ -352,4 +354,122 @@ struct optee_msg_arg
#define OPTEE_MSG_CMD_STOP_ASYNC_NOTIF 7
#define OPTEE_MSG_FUNCID_CALL_WITH_ARG 0x0004
+/****************************************************************************
+ * Part 3 - Requests from secure world, RPC
+ ****************************************************************************/
+
+/* All RPC is done with a struct optee_msg_arg as bearer of information,
+ * struct optee_msg_arg::arg holds values defined by OPTEE_MSG_RPC_CMD_*
+ * below
+ *
+ * RPC communication with tee-supplicant is reversed compared to normal
+ * client communication described above. The supplicant receives requests
+ * and sends responses.
+ */
+
+/* Load a TA into memory, defined in tee-supplicant
+ */
Review Comment:
merge `*/` to ALL one line comment
##########
drivers/misc/optee.h:
##########
@@ -38,26 +38,50 @@
* Pre-processor Definitions
****************************************************************************/
+/* Some GlobalPlatform error codes used in this driver */
+
+#define TEE_SUCCESS 0x00000000
+#define TEE_ERROR_ACCESS_DENIED 0xFFFF0001
+#define TEE_ERROR_BAD_FORMAT 0xFFFF0005
+#define TEE_ERROR_BAD_PARAMETERS 0xFFFF0006
+#define TEE_ERROR_GENERIC 0xFFFF0000
+#define TEE_ERROR_NOT_SUPPORTED 0xFFFF000A
+#define TEE_ERROR_OUT_OF_MEMORY 0xFFFF000C
+#define TEE_ERROR_BUSY 0xFFFF000D
+#define TEE_ERROR_COMMUNICATION 0xFFFF000E
+#define TEE_ERROR_SECURITY 0xFFFF000F
+#define TEE_ERROR_SHORT_BUFFER 0xFFFF0010
+#define TEE_ERROR_TIMEOUT 0xFFFF3001
+
+#define TEE_ORIGIN_COMMS 0x00000002
+
#define OPTEE_SERVER_PATH "optee"
#define OPTEE_MAX_PARAM_NUM 6
/****************************************************************************
* Public Types
****************************************************************************/
+enum optee_role_e
+{
+ OPTEE_ROLE_CA, /* /dev/tee0 */
+ OPTEE_ROLE_SUPPLICANT, /* /dev/tee-supp0 */
+};
+
struct optee_priv_data
{
uintptr_t alignment; /* Transport-specified message alignment */
- FAR struct idr_s *shms; /* An RB tree of all shm entries */
+ FAR struct idr_s *shms; /* An RB tree of process local shm entries */
+ enum optee_role_e role;
};
struct optee_shm
{
FAR struct optee_priv_data *priv;
int fd;
int32_t id;
- uint64_t paddr;
uint64_t vaddr;
+ uint64_t paddr;
Review Comment:
move the change to first patch
##########
drivers/misc/optee_supplicant.h:
##########
@@ -0,0 +1,73 @@
+/****************************************************************************
+ * drivers/misc/optee_supplicant.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_SUPPLICANT_H
+#define __DRIVERS_MISC_OPTEE_SUPPLICANT_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/types.h>
+#include "optee.h"
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+void optee_supplicant_init(void);
+void optee_supplicant_uninit(void);
+bool optee_supplicant_running(void);
+
+FAR struct idr_s *optee_supplicant_get_shm_idr(void);
+FAR struct idr_s *optee_supplicant_init_shm_idr(void);
+
+uint32_t optee_supplicant_request(uint32_t func, size_t num_params,
+ FAR struct tee_ioctl_param *param);
+
+uint32_t optee_supplicant_cmd_free(int32_t shm_id);
+int32_t optee_supplicant_cmd_alloc(FAR struct optee_priv_data *priv,
+ size_t sz, struct optee_shm **shm);
+
+void optee_supplicant_cmd(FAR struct optee_priv_data *priv,
+ struct optee_msg_arg *arg);
Review Comment:
FAR too
##########
drivers/misc/optee_rpc.h:
##########
@@ -0,0 +1,52 @@
+/****************************************************************************
+ * drivers/misc/optee_rpc.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_RPC_H
+#define __DRIVERS_MISC_OPTEE_RPC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include "optee.h"
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+void optee_rpc_handle_cmd(FAR struct optee_priv_data *priv,
+ struct optee_shm *shm, void **last_page_list);
Review Comment:
add FAR for ALL pointers
##########
drivers/misc/optee_supplicant.h:
##########
@@ -0,0 +1,73 @@
+/****************************************************************************
+ * drivers/misc/optee_supplicant.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_SUPPLICANT_H
+#define __DRIVERS_MISC_OPTEE_SUPPLICANT_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <sys/types.h>
+#include "optee.h"
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+void optee_supplicant_init(void);
+void optee_supplicant_uninit(void);
+bool optee_supplicant_running(void);
+
+FAR struct idr_s *optee_supplicant_get_shm_idr(void);
+FAR struct idr_s *optee_supplicant_init_shm_idr(void);
+
+uint32_t optee_supplicant_request(uint32_t func, size_t num_params,
+ FAR struct tee_ioctl_param *param);
+
+uint32_t optee_supplicant_cmd_free(int32_t shm_id);
+int32_t optee_supplicant_cmd_alloc(FAR struct optee_priv_data *priv,
+ size_t sz, struct optee_shm **shm);
Review Comment:
FAR
--
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
