xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2079713761
##########
drivers/misc/optee.c:
##########
@@ -200,7 +522,19 @@ static int optee_open(FAR struct file *filep)
static int optee_close(FAR struct file *filep)
{
FAR struct optee_priv_data *priv = filep->f_priv;
+ FAR struct optee_shm_entry *shme;
+ FAR struct optee_shm_entry *tmp;
+ while (nxrmutex_lock(&priv->lock) < 0);
Review Comment:
don't need hold lock at all since no other party can issue request to this
driver in optee_close
##########
drivers/misc/optee.c:
##########
@@ -594,6 +993,204 @@ static int optee_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: optee_va_to_pa
+ *
+ * Description:
+ * Convert the specified virtual address to a physical address. If the
+ * virtual address does not belong to the user, it is assumed to be a
+ * kernel virtual address with a 1-1 mapping and the VA is returned as-is.
+ * The VA is also returned as-is if this is a build without
+ * CONFIG_ARCH_ADDRENV.
+ *
+ * Parameters:
+ * va - The virtual address to convert.
+ *
+ * Returned Values:
+ * The physical address corresponding to the specified VA.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_ADDRENV
+uintptr_t optee_va_to_pa(FAR const void *va)
+{
+ FAR arch_addrenv_t *addrenv;
+ FAR struct tcb_s *tcb;
+ uintptr_t page;
+
+ tcb = nxsched_self();
+ addrenv = &tcb->addrenv_curr->addrenv;
+
+ page = up_addrenv_find_page(addrenv, (uintptr_t)va);
+ if (page == 0)
+ {
+ return (uintptr_t)va;
+ }
+
+ return page | ((uintptr_t)va & MM_PGMASK);
+}
+#endif
+
+/****************************************************************************
+ * 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` will
+ * be allocated. In this case `addr` will be ignored. The allocated
+ * buffer will be aligned to `priv->alignment`. `TEE_SHM_ALLOC` 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
+ * 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.
+ *
+ ****************************************************************************/
+
+int optee_shm_alloc(FAR struct optee_priv_data *priv, 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 = kmm_zalloc(sizeof(struct optee_shm_entry));
+ if (shme == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ if (flags & TEE_SHM_ALLOC)
+ {
+ if (priv->alignment)
+ {
+ ptr = kmm_memalign(priv->alignment, size);
+ }
+ else
+ {
+ ptr = kmm_malloc(size);
+ }
+ }
+ else
+ {
+ ptr = addr;
+ }
+
+ if (ptr == NULL)
+ {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ shme->shm.addr = (uintptr_t)ptr;
+ shme->shm.length = size;
+ shme->shm.flags = flags;
+
+ if (flags & TEE_SHM_SEC_REGISTER)
+ {
+ ret = optee_shm_sec_register(priv, shme);
+ if (ret < 0)
+ {
+ goto err;
+ }
+ }
+
+ if (flags & TEE_SHM_REGISTER)
+ {
+ while (nxrmutex_lock(&priv->lock) < 0);
Review Comment:
why need the recursive lock?
##########
drivers/misc/optee.h:
##########
@@ -44,8 +46,17 @@
* Public Types
****************************************************************************/
+struct optee_shm_entry
+{
+ struct list_node node;
+ struct tee_ioctl_shm_register_data shm;
+};
+
struct optee_priv_data
{
+ uintptr_t alignment; /* Transport-specified message alignment */
+ struct list_node shm_list; /* A list of all shm registered */
+ rmutex_t lock; /* Mutex used to guard list accesses */
Review Comment:
let's change to spinlock_t which is fast then mutex
--
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
