[v2,5/5] firmware: imx: adds miscdev

Commit Message

Pankaj Gupta May 23, 2024, 10:49 a.m. UTC

Adds the driver for communication interface to secure-enclave,
for exchanging messages with NXP secure enclave HW IP(s) like
EdgeLock Enclave from:
- User-Space Applications via character driver.

ABI documentation for the NXP secure-enclave driver.

User-space library using this driver:
- i.MX Secure Enclave library:
  -- URL: https://github.com/nxp-imx/imx-secure-enclave.git,
- i.MX Secure Middle-Ware:
  -- URL: https://github.com/nxp-imx/imx-smw.git

Signed-off-by: Pankaj Gupta <[e-mail]>
---
 Documentation/ABI/testing/se-cdev |  42 +++
 drivers/firmware/imx/ele_common.c | 108 +++++-
 drivers/firmware/imx/ele_common.h |   3 +
 drivers/firmware/imx/se_ctrl.c    | 689 ++++++++++++++++++++++++++++++++++++++
 drivers/firmware/imx/se_ctrl.h    |  46 +++
 include/uapi/linux/se_ioctl.h     |  88 +++++
 6 files changed, 974 insertions(+), 2 deletions(-)

Comments

Sascha Hauer May 24, 2024, 8:23 a.m. UTC | #1

On Thu, May 23, 2024 at 04:19:36PM +0530, Pankaj Gupta wrote:
> +int imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
> +			     void *tx_msg, int tx_msg_sz)
> +{
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	struct se_msg_hdr *header;
> +	int err;
> +
> +	header = (struct se_msg_hdr *) tx_msg;
> +
> +	/*
> +	 * Check that the size passed as argument matches the size
> +	 * carried in the message.
> +	 */
> +	err = header->size << 2;
> +
> +	if (err != tx_msg_sz) {
> +		err = -EINVAL;
> +		dev_err(priv->dev,
> +			"%s: User buffer too small\n",
> +				dev_ctx->miscdev.name);
> +		goto exit;
> +	}
> +	/* Check the message is valid according to tags */
> +	if (header->tag == priv->cmd_tag) {
> +		mutex_lock(&priv->se_if_cmd_lock);

Grabbing a mutex in a character devices write fop and releasing it in the
read fop is really calling for undesired race conditions.

If sending a command and receiving the response shall be an atomic
operation then you should really consider turning this into an ioctl
and just not implement read/write on the character device. With this
you'll be able to get rid of several oddities in this drivers locking.

> +		priv->waiting_rsp_dev = dev_ctx;
> +	} else if (header->tag == priv->rsp_tag) {
> +		/* Check the device context can send the command */
> +		if (dev_ctx != priv->cmd_receiver_dev) {
> +			dev_err(priv->dev,
> +				"%s: Channel not configured to send resp to FW.",
> +				dev_ctx->miscdev.name);
> +			err = -EPERM;
> +			goto exit;
> +		}
> +	} else {
> +		dev_err(priv->dev,
> +			"%s: The message does not have a valid TAG\n",
> +				dev_ctx->miscdev.name);
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +	err = imx_ele_msg_send(priv, tx_msg);
> +	if (err < 0) {
> +		if (header->tag == priv->cmd_tag) {
> +			priv->waiting_rsp_dev = NULL;
> +			mutex_unlock(&dev_ctx->priv->se_if_cmd_lock);
> +		}
> +	} else
> +		err = header->size << 2;
> +exit:
> +	return err;
> +}
> +

[...]

> +static ssize_t se_if_fops_write(struct file *fp, const char __user *buf,
> +				size_t size, loff_t *ppos)
> +{
> +	struct se_api_msg *tx_msg __free(kfree);
> +	struct se_if_device_ctx *dev_ctx;
> +	struct se_if_priv *priv;
> +	int err;
> +
> +	dev_ctx = container_of(fp->private_data,
> +			       struct se_if_device_ctx,
> +			       miscdev);
> +	priv = dev_ctx->priv;
> +	dev_dbg(priv->dev,
> +		"%s: write from buf (%p)%zu, ppos=%lld\n",
> +			dev_ctx->miscdev.name,
> +			buf, size, ((ppos) ? *ppos : 0));
> +
> +	if (down_interruptible(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	if (dev_ctx->status != SE_IF_CTX_OPENED) {
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +
> +	if (size < SE_MU_HDR_SZ) {
> +		dev_err(priv->dev,
> +			"%s: User buffer too small(%zu < %d)\n",
> +				dev_ctx->miscdev.name,
> +				size, SE_MU_HDR_SZ);
> +		err = -ENOSPC;
> +		goto exit;
> +	}
> +	tx_msg = memdup_user(buf, size);
> +	if (!tx_msg) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}

memdup_user() returns an error pointer, not NULL. Also you are using
tx_msg uninitialized.

> +
> +	print_hex_dump_debug("from user ", DUMP_PREFIX_OFFSET, 4, 4,
> +			     tx_msg, size, false);
> +
> +	err = imx_ele_miscdev_msg_send(dev_ctx, tx_msg, size);
> +
> +exit:
> +	up(&dev_ctx->fops_lock);
> +	return err;
> +}
> +
> +/*
> + * Read a message from the MU.
> + * Blocking until a message is available.
> + */
> +static ssize_t se_if_fops_read(struct file *fp, char __user *buf,
> +			       size_t size, loff_t *ppos)
> +{
> +	struct se_if_device_ctx *dev_ctx;
> +	struct se_buf_desc *b_desc;
> +	struct se_if_priv *priv;
> +	u32 size_to_copy;
> +	int err;
> +
> +	dev_ctx = container_of(fp->private_data,
> +			       struct se_if_device_ctx,
> +			       miscdev);
> +	priv = dev_ctx->priv;
> +	dev_dbg(priv->dev,
> +		"%s: read to buf %p(%zu), ppos=%lld\n",
> +			dev_ctx->miscdev.name,
> +			buf, size, ((ppos) ? *ppos : 0));
> +
> +	if (down_interruptible(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	if (dev_ctx->status != SE_IF_CTX_OPENED) {
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +
> +	err = imx_ele_miscdev_msg_rcv(dev_ctx);
> +	if (err)
> +		goto exit;
> +
> +	/* Buffer containing the message from FW, is
> +	 * allocated in callback function.
> +	 * Check if buffer allocation failed.
> +	 */
> +	if (!dev_ctx->temp_resp) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	dev_dbg(priv->dev,
> +			"%s: %s %s\n",
> +			dev_ctx->miscdev.name,
> +			__func__,
> +			"message received, start transmit to user");
> +
> +	/*
> +	 * Check that the size passed as argument is larger than
> +	 * the one carried in the message.
> +	 */
> +	size_to_copy = dev_ctx->temp_resp_size << 2;
> +	if (size_to_copy > size) {
> +		dev_dbg(priv->dev,
> +			"%s: User buffer too small (%zu < %d)\n",
> +				dev_ctx->miscdev.name,
> +				size, size_to_copy);
> +		size_to_copy = size;
> +	}
> +
> +	/*
> +	 * We may need to copy the output data to user before
> +	 * delivering the completion message.
> +	 */
> +	while (!list_empty(&dev_ctx->pending_out)) {
> +		b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> +						  struct se_buf_desc,
> +						  link);
> +		if (!b_desc)
> +			continue;

b_desc will never be NULL because otherwise you wouldn't be in the loop
anymore. The usual way to iterate over a list is to use list_for_each_entry()
or list_for_each_entry_safe() in case you delete entries in the loop
body.

> +
> +		if (b_desc->usr_buf_ptr && b_desc->shared_buf_ptr) {
> +
> +			dev_dbg(priv->dev,
> +				"%s: Copy output data to user\n",
> +				dev_ctx->miscdev.name);
> +			if (copy_to_user(b_desc->usr_buf_ptr,
> +					 b_desc->shared_buf_ptr,
> +					 b_desc->size)) {
> +				dev_err(priv->dev,
> +					"%s: Failure copying output data to user.",
> +					dev_ctx->miscdev.name);
> +				err = -EFAULT;
> +				goto exit;
> +			}
> +		}
> +
> +		if (b_desc->shared_buf_ptr)
> +			memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> +
> +		__list_del_entry(&b_desc->link);

list_del()

> +		kfree(b_desc);
> +	}
> +
> +	/* Copy data from the buffer */
> +	print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
> +			     dev_ctx->temp_resp, size_to_copy, false);
> +	if (copy_to_user(buf, dev_ctx->temp_resp, size_to_copy)) {
> +		dev_err(priv->dev,
> +			"%s: Failed to copy to user\n",
> +				dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +	err = size_to_copy;
> +	kfree(dev_ctx->temp_resp);
> +
> +	/* free memory allocated on the shared buffers. */
> +	dev_ctx->secure_mem.pos = 0;
> +	dev_ctx->non_secure_mem.pos = 0;
> +
> +	dev_ctx->pending_hdr = 0;
> +
> +exit:
> +	/*
> +	 * Clean the used Shared Memory space,
> +	 * whether its Input Data copied from user buffers, or
> +	 * Data received from FW.
> +	 */
> +	while (!list_empty(&dev_ctx->pending_in) ||
> +	       !list_empty(&dev_ctx->pending_out)) {
> +		if (!list_empty(&dev_ctx->pending_in))
> +			b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
> +							  struct se_buf_desc,
> +							  link);
> +		else
> +			b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> +							  struct se_buf_desc,
> +							  link);
> +
> +		if (!b_desc)
> +			continue;
> +
> +		if (b_desc->shared_buf_ptr)
> +			memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> +
> +		__list_del_entry(&b_desc->link);
> +		kfree(b_desc);
> +	}
> +
> +	up(&dev_ctx->fops_lock);
> +	return err;
> +}
> +
> +static int se_ioctl_get_mu_info(struct se_if_device_ctx *dev_ctx,
> +				u64 arg)
> +{
> +	struct se_if_priv *priv = dev_get_drvdata(dev_ctx->dev);
> +	struct imx_se_node_info *if_node_info;
> +	struct se_ioctl_get_if_info info;
> +	int err = 0;
> +
> +	if_node_info = (struct imx_se_node_info *)priv->info;

priv->info is of type const void *. You are casting away the the 'const'
here. Either it is const, then it should stay const, or not, in which
case it shouldn't be declared const. Also why isn't priv->info of type
struct imx_se_node_info * in the first place?

> +
> +	info.se_if_id = if_node_info->se_if_id;
> +	info.interrupt_idx = 0;
> +	info.tz = 0;
> +	info.did = if_node_info->se_if_did;
> +	info.cmd_tag = if_node_info->cmd_tag;
> +	info.rsp_tag = if_node_info->rsp_tag;
> +	info.success_tag = if_node_info->success_tag;
> +	info.base_api_ver = if_node_info->base_api_ver;
> +	info.fw_api_ver = if_node_info->fw_api_ver;
> +
> +	dev_dbg(priv->dev,
> +		"%s: info [se_if_id: %d, irq_idx: %d, tz: 0x%x, did: 0x%x]\n",
> +			dev_ctx->miscdev.name,
> +			info.se_if_id, info.interrupt_idx, info.tz, info.did);
> +
> +	if (copy_to_user((u8 *)arg, &info, sizeof(info))) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed to copy mu info to user\n",
> +				dev_ctx->miscdev.name);

Just drop these error messages for failed copy_to_user(). They don't
give any valuable information and just bloat the code.

> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +exit:
> +	return err;
> +}
> +
> +/*
> + * Copy a buffer of data to/from the user and return the address to use in
> + * messages
> + */
> +static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
> +					    u64 arg)
> +{
> +	struct se_shared_mem *shared_mem = NULL;
> +	struct se_ioctl_setup_iobuf io = {0};
> +	struct se_buf_desc *b_desc = NULL;
> +	int err = 0;
> +	u32 pos;
> +
> +	if (copy_from_user(&io, (u8 *)arg, sizeof(io))) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed copy iobuf config from user\n",
> +				dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +	dev_dbg(dev_ctx->priv->dev,
> +			"%s: io [buf: %p(%d) flag: %x]\n",
> +			dev_ctx->miscdev.name,
> +			io.user_buf, io.length, io.flags);
> +
> +	if (io.length == 0 || !io.user_buf) {
> +		/*
> +		 * Accept NULL pointers since some buffers are optional
> +		 * in FW commands. In this case we should return 0 as
> +		 * pointer to be embedded into the message.
> +		 * Skip all data copy part of code below.
> +		 */
> +		io.ele_addr = 0;
> +		goto copy;
> +	}
> +
> +	/* Select the shared memory to be used for this buffer. */
> +	if (io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) {

If you don't support this flag (yet), just don't include it in your
submission.

> +		/* App requires to use secure memory for this buffer.*/
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed allocate SEC MEM memory\n",
> +				dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	} else {
> +		/* No specific requirement for this buffer. */
> +		shared_mem = &dev_ctx->non_secure_mem;
> +	}
> +
> +	/* Check there is enough space in the shared memory. */
> +	if (shared_mem->size < shared_mem->pos
> +			|| io.length >= shared_mem->size - shared_mem->pos) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Not enough space in shared memory\n",
> +				dev_ctx->miscdev.name);
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	/* Allocate space in shared memory. 8 bytes aligned. */
> +	pos = shared_mem->pos;
> +	shared_mem->pos += round_up(io.length, 8u);

You are checking if there's enough space in the shared memory without
taking this round_up into account.

> +	io.ele_addr = (u64)shared_mem->dma_addr + pos;
> +
> +	if ((io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) &&
> +	    !(io.flags & SE_MU_IO_FLAGS_USE_SHORT_ADDR)) {
> +		/*Add base address to get full address.*/
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed allocate SEC MEM memory\n",
> +				dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +	memset(shared_mem->ptr + pos, 0, io.length);
> +	if ((io.flags & SE_IO_BUF_FLAGS_IS_INPUT) ||
> +	    (io.flags & SE_IO_BUF_FLAGS_IS_IN_OUT)) {
> +		/*
> +		 * buffer is input:
> +		 * copy data from user space to this allocated buffer.
> +		 */
> +		if (copy_from_user(shared_mem->ptr + pos, io.user_buf,
> +				   io.length)) {
> +			dev_err(dev_ctx->priv->dev,
> +				"%s: Failed copy data to shared memory\n",
> +				dev_ctx->miscdev.name);
> +			err = -EFAULT;
> +			goto exit;
> +		}
> +	}
> +
> +	b_desc = kzalloc(sizeof(*b_desc), GFP_KERNEL);
> +	if (!b_desc) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +copy:
> +	/* Provide the EdgeLock Enclave address to user space only if success.*/
> +	if (copy_to_user((u8 *)arg, &io, sizeof(io))) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed to copy iobuff setup to user\n",
> +				dev_ctx->miscdev.name);
> +		kfree(b_desc);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +	if (b_desc) {
> +		b_desc->shared_buf_ptr = shared_mem->ptr + pos;
> +		b_desc->usr_buf_ptr = io.user_buf;
> +		b_desc->size = io.length;
> +
> +		if (io.flags & SE_IO_BUF_FLAGS_IS_INPUT) {
> +			/*
> +			 * buffer is input:
> +			 * add an entry in the "pending input buffers" list so
> +			 * that copied data can be cleaned from shared memory
> +			 * later.
> +			 */
> +			list_add_tail(&b_desc->link, &dev_ctx->pending_in);
> +		} else {
> +			/*
> +			 * buffer is output:
> +			 * add an entry in the "pending out buffers" list so data
> +			 * can be copied to user space when receiving Secure-Enclave
> +			 * response.
> +			 */
> +			list_add_tail(&b_desc->link, &dev_ctx->pending_out);
> +		}
> +	}
> +
> +exit:
> +	return err;
> +}
> +
> +/* IOCTL to provide SoC information */
> +static int se_ioctl_get_se_soc_info_handler(struct se_if_device_ctx *dev_ctx,
> +					     u64 arg)
> +{
> +	struct imx_se_node_info_list *info_list;
> +	struct se_ioctl_get_soc_info soc_info;
> +	int err = -EINVAL;
> +
> +	info_list = (struct imx_se_node_info_list *)
> +			device_get_match_data(dev_ctx->priv->dev);

device_get_match_data() returns a const void *. It is generally not necessary
to explicitly cast void * to another pointer type. By explicitly casting
it you just cast away the 'const' without noticing.

> +	if (!info_list)
> +		goto exit;
> +
> +	soc_info.soc_id = info_list->soc_id;
> +	soc_info.soc_rev = dev_ctx->priv->soc_rev;
> +
> +	err = (int)copy_to_user((u8 *)arg, (u8 *)(&soc_info), sizeof(soc_info));

First argument of copy_to_user() is a void *, then why do you cast it to
something entirely unrelated (u8)? Second argument is a void * as well,
so no need to explicitly cast.

> +	if (err) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed to copy soc info to user\n",
> +			dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;

This goto is unnecessary.

> +	}
> +
> +exit:
> +	return err;
> +}
> +
> +/* Open a character device. */
> +static int se_if_fops_open(struct inode *nd, struct file *fp)
> +{
> +	struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> +							struct se_if_device_ctx,
> +							miscdev);
> +	int err = 0;
> +
> +	/* Avoid race if opened at the same time */
> +	if (down_trylock(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	/* Authorize only 1 instance. */
> +	if (dev_ctx->status != SE_IF_CTX_FREE) {
> +		err = -EBUSY;
> +		goto exit;
> +	}
> +
> +	/*
> +	 * Allocate some memory for data exchanges with S40x.
> +	 * This will be used for data not requiring secure memory.
> +	 */
> +	dev_ctx->non_secure_mem.ptr = dmam_alloc_coherent(dev_ctx->dev,
> +					MAX_DATA_SIZE_PER_USER,
> +					&dev_ctx->non_secure_mem.dma_addr,
> +					GFP_KERNEL);

As Marc already mentioned: There is no point in using the managed
version on dma_alloc_coherent() here.

> +	if (!dev_ctx->non_secure_mem.ptr) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	dev_ctx->non_secure_mem.size = MAX_DATA_SIZE_PER_USER;
> +	dev_ctx->non_secure_mem.pos = 0;
> +	dev_ctx->status = SE_IF_CTX_OPENED;
> +
> +	dev_ctx->pending_hdr = 0;
> +
> +	goto exit;
> +
> +	dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
> +			   dev_ctx->non_secure_mem.ptr,
> +			   dev_ctx->non_secure_mem.dma_addr);

This code is unreachable.

> +
> +exit:
> +	up(&dev_ctx->fops_lock);
> +	return err;
> +}
> +
> +/* Close a character device. */
> +static int se_if_fops_close(struct inode *nd, struct file *fp)
> +{
> +	struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> +							struct se_if_device_ctx,
> +							miscdev);
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	struct se_buf_desc *b_desc;
> +
> +	/* Avoid race if closed at the same time */
> +	if (down_trylock(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	/* The device context has not been opened */
> +	if (dev_ctx->status != SE_IF_CTX_OPENED)
> +		goto exit;
> +
> +	/* check if this device was registered as command receiver. */
> +	if (priv->cmd_receiver_dev == dev_ctx)
> +		priv->cmd_receiver_dev = NULL;
> +
> +	/* check if this device was registered as waiting response. */
> +	if (priv->waiting_rsp_dev == dev_ctx) {
> +		priv->waiting_rsp_dev = NULL;
> +		mutex_unlock(&priv->se_if_cmd_lock);
> +	}
> +
> +	/* Unmap secure memory shared buffer. */
> +	if (dev_ctx->secure_mem.ptr)
> +		devm_iounmap(dev_ctx->dev, dev_ctx->secure_mem.ptr);

secure_mem is unused in this patch set. Drop this and re-add it later
when you want to add support for this.

> +
> +	dev_ctx->secure_mem.ptr = NULL;
> +	dev_ctx->secure_mem.dma_addr = 0;
> +	dev_ctx->secure_mem.size = 0;
> +	dev_ctx->secure_mem.pos = 0;
> +
> +	/* Free non-secure shared buffer. */
> +	dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
> +			   dev_ctx->non_secure_mem.ptr,
> +			   dev_ctx->non_secure_mem.dma_addr);
> +
> +	dev_ctx->non_secure_mem.ptr = NULL;
> +	dev_ctx->non_secure_mem.dma_addr = 0;
> +	dev_ctx->non_secure_mem.size = 0;
> +	dev_ctx->non_secure_mem.pos = 0;
> +
> +	while (!list_empty(&dev_ctx->pending_in) ||
> +	       !list_empty(&dev_ctx->pending_out)) {
> +		if (!list_empty(&dev_ctx->pending_in))
> +			b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
> +							  struct se_buf_desc,
> +							  link);
> +		else
> +			b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> +							  struct se_buf_desc,
> +							  link);
> +
> +		if (!b_desc)
> +			continue;
> +
> +		if (b_desc->shared_buf_ptr)
> +			memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> +
> +		__list_del_entry(&b_desc->link);
> +		kfree(b_desc);
> +	}
> +
> +	dev_ctx->status = SE_IF_CTX_FREE;
> +
> +exit:
> +	up(&dev_ctx->fops_lock);
> +	return 0;
> +}
> +
> +/* IOCTL entry point of a character device */
> +static long se_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +{
> +	struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> +							struct se_if_device_ctx,
> +							miscdev);
> +	struct se_if_priv *se_if_priv = dev_ctx->priv;
> +	int err = -EINVAL;
> +
> +	/* Prevent race during change of device context */
> +	if (down_interruptible(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	switch (cmd) {
> +	case SE_IOCTL_ENABLE_CMD_RCV:
> +		if (!se_if_priv->cmd_receiver_dev) {
> +			se_if_priv->cmd_receiver_dev = dev_ctx;
> +			err = 0;
> +		}
> +		break;
> +	case SE_IOCTL_GET_MU_INFO:
> +		err = se_ioctl_get_mu_info(dev_ctx, arg);
> +		break;
> +	case SE_IOCTL_SETUP_IOBUF:
> +		err = se_ioctl_setup_iobuf_handler(dev_ctx, arg);
> +		break;
> +	case SE_IOCTL_GET_SOC_INFO:
> +		err = se_ioctl_get_se_soc_info_handler(dev_ctx, arg);
> +		break;
> +
> +	default:
> +		err = -EINVAL;
> +		dev_dbg(se_if_priv->dev,
> +			"%s: IOCTL %.8x not supported\n",
> +				dev_ctx->miscdev.name,
> +				cmd);
> +	}
> +
> +	up(&dev_ctx->fops_lock);
> +	return (long)err;
> +}
> +
> +/* Char driver setup */
> +static const struct file_operations se_if_fops = {
> +	.open		= se_if_fops_open,
> +	.owner		= THIS_MODULE,
> +	.release	= se_if_fops_close,
> +	.unlocked_ioctl = se_ioctl,
> +	.read		= se_if_fops_read,
> +	.write		= se_if_fops_write,
> +};
> +
> +/* interface for managed res to unregister a character device */
> +static void if_misc_deregister(void *miscdevice)
> +{
> +	misc_deregister(miscdevice);
> +}
> +
>  /* interface for managed res to free a mailbox channel */
>  static void if_mbox_free_channel(void *mbox_chan)
>  {
> @@ -270,6 +855,7 @@ static int se_probe_if_cleanup(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct se_if_priv *priv;
>  	int ret = 0;
> +	int i;
>  
>  	priv = dev_get_drvdata(dev);
>  	if (!priv) {
> @@ -294,6 +880,17 @@ static int se_probe_if_cleanup(struct platform_device *pdev)
>  		priv->imem.buf = NULL;
>  	}
>  
> +	if (priv->ctxs) {
> +		for (i = 0; i < priv->max_dev_ctx; i++) {
> +			if (priv->ctxs[i]) {
> +				devm_remove_action(dev,
> +						   if_misc_deregister,
> +						   &priv->ctxs[i]->miscdev);
> +				misc_deregister(&priv->ctxs[i]->miscdev);
> +			}
> +		}
> +	}
> +
>  	if (priv->flags & RESERVED_DMA_POOL) {
>  		of_reserved_mem_device_release(dev);
>  		priv->flags &= (~RESERVED_DMA_POOL);
> @@ -302,6 +899,84 @@ static int se_probe_if_cleanup(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +static int init_device_context(struct device *dev)
> +{
> +	const struct imx_se_node_info *info;
> +	struct se_if_device_ctx *dev_ctx;
> +	struct se_if_priv *priv;
> +	u8 *devname;
> +	int ret = 0;
> +	int i;
> +
> +	priv = dev_get_drvdata(dev);
> +
> +	if (!priv) {
> +		ret = -EINVAL;
> +		dev_err(dev, "Invalid SE-MU Priv data");
> +		return ret;
> +	}

You won't hit this as you already have called dev_set_drvdata(). Just
drop this check. Also you should pass a struct se_if_priv * directly
to this function instead of taking the detour of passing a struct device
*.

> +	info = priv->info;
> +
> +	priv->ctxs = devm_kzalloc(dev, sizeof(dev_ctx) * priv->max_dev_ctx,
> +				  GFP_KERNEL);
> +
> +	if (!priv->ctxs) {
> +		ret = -ENOMEM;
> +		return ret;
> +	}
> +
> +	/* Create users */
> +	for (i = 0; i < priv->max_dev_ctx; i++) {
> +		dev_ctx = devm_kzalloc(dev, sizeof(*dev_ctx), GFP_KERNEL);
> +		if (!dev_ctx) {
> +			ret = -ENOMEM;
> +			return ret;
> +		}
> +
> +		dev_ctx->dev = dev;
> +		dev_ctx->status = SE_IF_CTX_FREE;
> +		dev_ctx->priv = priv;
> +
> +		priv->ctxs[i] = dev_ctx;
> +
> +		/* Default value invalid for an header. */
> +		init_waitqueue_head(&dev_ctx->wq);
> +
> +		INIT_LIST_HEAD(&dev_ctx->pending_out);
> +		INIT_LIST_HEAD(&dev_ctx->pending_in);
> +		sema_init(&dev_ctx->fops_lock, 1);
> +
> +		devname = devm_kasprintf(dev, GFP_KERNEL, "%s_ch%d",
> +					 info->se_name, i);
> +		if (!devname) {
> +			ret = -ENOMEM;
> +			return ret;
> +		}
> +
> +		dev_ctx->miscdev.name = devname;
> +		dev_ctx->miscdev.minor = MISC_DYNAMIC_MINOR;
> +		dev_ctx->miscdev.fops = &se_if_fops;
> +		dev_ctx->miscdev.parent = dev;
> +		ret = misc_register(&dev_ctx->miscdev);
> +		if (ret) {
> +			dev_err(dev, "failed to register misc device %d\n",
> +				ret);
> +			return ret;
> +		}
> +
> +		ret = devm_add_action(dev, if_misc_deregister,
> +				      &dev_ctx->miscdev);
> +		if (ret) {
> +			dev_err(dev,
> +				"failed[%d] to add action to the misc-dev\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  static void se_load_firmware(const struct firmware *fw, void *context)
>  {
>  	struct se_if_priv *priv = (struct se_if_priv *) context;
> @@ -461,6 +1136,16 @@ static int se_if_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	if (info->max_dev_ctx) {
> +		ret = init_device_context(dev);
> +		if (ret) {
> +			dev_err(dev,
> +				"Failed[0x%x] to create device contexts.\n",
> +				ret);
> +			goto exit;
> +		}
> +	}
> +
>  	dev_info(dev, "i.MX secure-enclave: %s interface to firmware, configured.\n",
>  		 info->se_name);
>  	return devm_of_platform_populate(dev);
> @@ -502,6 +1187,10 @@ static int se_resume(struct device *dev)
>  	struct se_if_priv *priv = dev_get_drvdata(dev);
>  	const struct imx_se_node_info *info
>  					= priv->info;
> +	int i;
> +
> +	for (i = 0; i < priv->max_dev_ctx; i++)
> +		wake_up_interruptible(&priv->ctxs[i]->wq);
>  
>  	if (info && info->imem_mgmt)
>  		se_restore_imem_state(dev);

Sascha

Pankaj Gupta May 24, 2024, 12:03 p.m. UTC | #2

> -----Original Message-----
> From: Sascha Hauer <[e-mail]>
> Sent: Friday, May 24, 2024 1:53 PM
> To: Pankaj Gupta <[e-mail]>
> Cc: Jonathan Corbet <[e-mail]>; Rob Herring <[e-mail]>;
> Krzysztof Kozlowski <[e-mail]>; Conor Dooley
> <[e-mail]>; Shawn Guo <[e-mail]>; Pengutronix
> Kernel Team <[e-mail]>; Fabio Estevam
> <[e-mail]>; Rob Herring <[e-mail]>; Krzysztof Kozlowski
> <[e-mail]>; [e-mail]; linux-
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]
> Subject: [EXT] Re: [PATCH v2 5/5] firmware: imx: adds miscdev
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> On Thu, May 23, 2024 at 04:19:36PM +0530, Pankaj Gupta wrote:
> > +int imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
> > +                          void *tx_msg, int tx_msg_sz) {
> > +     struct se_if_priv *priv = dev_ctx->priv;
> > +     struct se_msg_hdr *header;
> > +     int err;
> > +
> > +     header = (struct se_msg_hdr *) tx_msg;
> > +
> > +     /*
> > +      * Check that the size passed as argument matches the size
> > +      * carried in the message.
> > +      */
> > +     err = header->size << 2;
> > +
> > +     if (err != tx_msg_sz) {
> > +             err = -EINVAL;
> > +             dev_err(priv->dev,
> > +                     "%s: User buffer too small\n",
> > +                             dev_ctx->miscdev.name);
> > +             goto exit;
> > +     }
> > +     /* Check the message is valid according to tags */
> > +     if (header->tag == priv->cmd_tag) {
> > +             mutex_lock(&priv->se_if_cmd_lock);
>
> Grabbing a mutex in a character devices write fop and releasing it in the read
> fop is really calling for undesired race conditions.

Condition is:
- Only one command is allowed to be in flight, at a time per interface.
   -- Second command is not allowed, when one command is in flight.
- Duration of the flight is till the time the response is not received from the FW.

Command lock is grabbed and then released in process context only.

>
> If sending a command and receiving the response shall be an atomic operation
> then you should really consider turning this into an ioctl and just not
> implement read/write on the character device. With this you'll be able to get
> rid of several oddities in this drivers locking.
>

It is not an atomic operation. It can be pre-empted.
But it cannot be pre-empted to send another command on the same interface.

As only one command is allowed to be executed at one point in time, through an interface.


> > +             priv->waiting_rsp_dev = dev_ctx;
> > +     } else if (header->tag == priv->rsp_tag) {
> > +             /* Check the device context can send the command */
> > +             if (dev_ctx != priv->cmd_receiver_dev) {
> > +                     dev_err(priv->dev,
> > +                             "%s: Channel not configured to send resp to FW.",
> > +                             dev_ctx->miscdev.name);
> > +                     err = -EPERM;
> > +                     goto exit;
> > +             }
> > +     } else {
> > +             dev_err(priv->dev,
> > +                     "%s: The message does not have a valid TAG\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EINVAL;
> > +             goto exit;
> > +     }
> > +     err = imx_ele_msg_send(priv, tx_msg);
> > +     if (err < 0) {
> > +             if (header->tag == priv->cmd_tag) {
> > +                     priv->waiting_rsp_dev = NULL;
> > +                     mutex_unlock(&dev_ctx->priv->se_if_cmd_lock);
> > +             }
> > +     } else
> > +             err = header->size << 2;
> > +exit:
> > +     return err;
> > +}
> > +
>
> [...]
>
> > +static ssize_t se_if_fops_write(struct file *fp, const char __user *buf,
> > +                             size_t size, loff_t *ppos) {
> > +     struct se_api_msg *tx_msg __free(kfree);

Accepted. Will correct in V3.
Initialize tx_msg with NULL.

> > +     struct se_if_device_ctx *dev_ctx;
> > +     struct se_if_priv *priv;
> > +     int err;
> > +
> > +     dev_ctx = container_of(fp->private_data,
> > +                            struct se_if_device_ctx,
> > +                            miscdev);
> > +     priv = dev_ctx->priv;
> > +     dev_dbg(priv->dev,
> > +             "%s: write from buf (%p)%zu, ppos=%lld\n",
> > +                     dev_ctx->miscdev.name,
> > +                     buf, size, ((ppos) ? *ppos : 0));
> > +
> > +     if (down_interruptible(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     if (dev_ctx->status != SE_IF_CTX_OPENED) {
> > +             err = -EINVAL;
> > +             goto exit;
> > +     }
> > +
> > +     if (size < SE_MU_HDR_SZ) {
> > +             dev_err(priv->dev,
> > +                     "%s: User buffer too small(%zu < %d)\n",
> > +                             dev_ctx->miscdev.name,
> > +                             size, SE_MU_HDR_SZ);
> > +             err = -ENOSPC;
> > +             goto exit;
> > +     }
> > +     tx_msg = memdup_user(buf, size);
> > +     if (!tx_msg) {
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
>
> memdup_user() returns an error pointer, not NULL. Also you are using tx_msg
> uninitialized.
>
Accepted. Will correct in V3.

> > +
> > +     print_hex_dump_debug("from user ", DUMP_PREFIX_OFFSET, 4, 4,
> > +                          tx_msg, size, false);
> > +
> > +     err = imx_ele_miscdev_msg_send(dev_ctx, tx_msg, size);
> > +
> > +exit:
> > +     up(&dev_ctx->fops_lock);
> > +     return err;
> > +}
> > +
> > +/*
> > + * Read a message from the MU.
> > + * Blocking until a message is available.
> > + */
> > +static ssize_t se_if_fops_read(struct file *fp, char __user *buf,
> > +                            size_t size, loff_t *ppos) {
> > +     struct se_if_device_ctx *dev_ctx;
> > +     struct se_buf_desc *b_desc;
> > +     struct se_if_priv *priv;
> > +     u32 size_to_copy;
> > +     int err;
> > +
> > +     dev_ctx = container_of(fp->private_data,
> > +                            struct se_if_device_ctx,
> > +                            miscdev);
> > +     priv = dev_ctx->priv;
> > +     dev_dbg(priv->dev,
> > +             "%s: read to buf %p(%zu), ppos=%lld\n",
> > +                     dev_ctx->miscdev.name,
> > +                     buf, size, ((ppos) ? *ppos : 0));
> > +
> > +     if (down_interruptible(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     if (dev_ctx->status != SE_IF_CTX_OPENED) {
> > +             err = -EINVAL;
> > +             goto exit;
> > +     }
> > +
> > +     err = imx_ele_miscdev_msg_rcv(dev_ctx);
> > +     if (err)
> > +             goto exit;
> > +
> > +     /* Buffer containing the message from FW, is
> > +      * allocated in callback function.
> > +      * Check if buffer allocation failed.
> > +      */
> > +     if (!dev_ctx->temp_resp) {
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
> > +
> > +     dev_dbg(priv->dev,
> > +                     "%s: %s %s\n",
> > +                     dev_ctx->miscdev.name,
> > +                     __func__,
> > +                     "message received, start transmit to user");
> > +
> > +     /*
> > +      * Check that the size passed as argument is larger than
> > +      * the one carried in the message.
> > +      */
> > +     size_to_copy = dev_ctx->temp_resp_size << 2;
> > +     if (size_to_copy > size) {
> > +             dev_dbg(priv->dev,
> > +                     "%s: User buffer too small (%zu < %d)\n",
> > +                             dev_ctx->miscdev.name,
> > +                             size, size_to_copy);
> > +             size_to_copy = size;
> > +     }
> > +
> > +     /*
> > +      * We may need to copy the output data to user before
> > +      * delivering the completion message.
> > +      */
> > +     while (!list_empty(&dev_ctx->pending_out)) {
> > +             b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> > +                                               struct se_buf_desc,
> > +                                               link);
> > +             if (!b_desc)
> > +                     continue;
>
> b_desc will never be NULL because otherwise you wouldn't be in the loop
> anymore. The usual way to iterate over a list is to use list_for_each_entry() or
> list_for_each_entry_safe() in case you delete entries in the loop body.
>

Will remove the NULL check.
        if (!b_desc)
               continue;


> > +
> > +             if (b_desc->usr_buf_ptr && b_desc->shared_buf_ptr) {
> > +
> > +                     dev_dbg(priv->dev,
> > +                             "%s: Copy output data to user\n",
> > +                             dev_ctx->miscdev.name);
> > +                     if (copy_to_user(b_desc->usr_buf_ptr,
> > +                                      b_desc->shared_buf_ptr,
> > +                                      b_desc->size)) {
> > +                             dev_err(priv->dev,
> > +                                     "%s: Failure copying output data to user.",
> > +                                     dev_ctx->miscdev.name);
> > +                             err = -EFAULT;
> > +                             goto exit;
> > +                     }
> > +             }
> > +
> > +             if (b_desc->shared_buf_ptr)
> > +                     memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> > +
> > +             __list_del_entry(&b_desc->link);
>
> list_del()


>
> > +             kfree(b_desc);
> > +     }
> > +
> > +     /* Copy data from the buffer */
> > +     print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
> > +                          dev_ctx->temp_resp, size_to_copy, false);
> > +     if (copy_to_user(buf, dev_ctx->temp_resp, size_to_copy)) {
> > +             dev_err(priv->dev,
> > +                     "%s: Failed to copy to user\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +     err = size_to_copy;
> > +     kfree(dev_ctx->temp_resp);
> > +
> > +     /* free memory allocated on the shared buffers. */
> > +     dev_ctx->secure_mem.pos = 0;
> > +     dev_ctx->non_secure_mem.pos = 0;
> > +
> > +     dev_ctx->pending_hdr = 0;
> > +
> > +exit:
> > +     /*
> > +      * Clean the used Shared Memory space,
> > +      * whether its Input Data copied from user buffers, or
> > +      * Data received from FW.
> > +      */
> > +     while (!list_empty(&dev_ctx->pending_in) ||
> > +            !list_empty(&dev_ctx->pending_out)) {
> > +             if (!list_empty(&dev_ctx->pending_in))
> > +                     b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
> > +                                                       struct se_buf_desc,
> > +                                                       link);
> > +             else
> > +                     b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> > +                                                       struct se_buf_desc,
> > +                                                       link);
> > +
> > +             if (!b_desc)
> > +                     continue;
> > +
> > +             if (b_desc->shared_buf_ptr)
> > +                     memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> > +
> > +             __list_del_entry(&b_desc->link);
> > +             kfree(b_desc);
> > +     }
> > +
> > +     up(&dev_ctx->fops_lock);
> > +     return err;
> > +}
> > +
> > +static int se_ioctl_get_mu_info(struct se_if_device_ctx *dev_ctx,
> > +                             u64 arg) {
> > +     struct se_if_priv *priv = dev_get_drvdata(dev_ctx->dev);
> > +     struct imx_se_node_info *if_node_info;
> > +     struct se_ioctl_get_if_info info;
> > +     int err = 0;
> > +
> > +     if_node_info = (struct imx_se_node_info *)priv->info;
>
> priv->info is of type const void *. You are casting away the the 'const'
> here. Either it is const, then it should stay const, or not, in which case it
> shouldn't be declared const. Also why isn't priv->info of type struct
> imx_se_node_info * in the first place?

This struct definition is local to the file se_ctrl.c.
Declaration of imx_se_node_info, is fixed by adding const in the whole file.

>
> > +
> > +     info.se_if_id = if_node_info->se_if_id;
> > +     info.interrupt_idx = 0;
> > +     info.tz = 0;
> > +     info.did = if_node_info->se_if_did;
> > +     info.cmd_tag = if_node_info->cmd_tag;
> > +     info.rsp_tag = if_node_info->rsp_tag;
> > +     info.success_tag = if_node_info->success_tag;
> > +     info.base_api_ver = if_node_info->base_api_ver;
> > +     info.fw_api_ver = if_node_info->fw_api_ver;
> > +
> > +     dev_dbg(priv->dev,
> > +             "%s: info [se_if_id: %d, irq_idx: %d, tz: 0x%x, did: 0x%x]\n",
> > +                     dev_ctx->miscdev.name,
> > +                     info.se_if_id, info.interrupt_idx, info.tz,
> > + info.did);
> > +
> > +     if (copy_to_user((u8 *)arg, &info, sizeof(info))) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed to copy mu info to user\n",
> > +                             dev_ctx->miscdev.name);
>
> Just drop these error messages for failed copy_to_user(). They don't give any
> valuable information and just bloat the code.
Accepted. Will correct in v3.

>
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +exit:
> > +     return err;
> > +}
> > +
> > +/*
> > + * Copy a buffer of data to/from the user and return the address to
> > +use in
> > + * messages
> > + */
> > +static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
> > +                                         u64 arg) {
> > +     struct se_shared_mem *shared_mem = NULL;
> > +     struct se_ioctl_setup_iobuf io = {0};
> > +     struct se_buf_desc *b_desc = NULL;
> > +     int err = 0;
> > +     u32 pos;
> > +
> > +     if (copy_from_user(&io, (u8 *)arg, sizeof(io))) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed copy iobuf config from user\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +     dev_dbg(dev_ctx->priv->dev,
> > +                     "%s: io [buf: %p(%d) flag: %x]\n",
> > +                     dev_ctx->miscdev.name,
> > +                     io.user_buf, io.length, io.flags);
> > +
> > +     if (io.length == 0 || !io.user_buf) {
> > +             /*
> > +              * Accept NULL pointers since some buffers are optional
> > +              * in FW commands. In this case we should return 0 as
> > +              * pointer to be embedded into the message.
> > +              * Skip all data copy part of code below.
> > +              */
> > +             io.ele_addr = 0;
> > +             goto copy;
> > +     }
> > +
> > +     /* Select the shared memory to be used for this buffer. */
> > +     if (io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) {
>
> If you don't support this flag (yet), just don't include it in your submission.

Accepted. Will remove it in V3.
>
> > +             /* App requires to use secure memory for this buffer.*/
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed allocate SEC MEM memory\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     } else {
> > +             /* No specific requirement for this buffer. */
> > +             shared_mem = &dev_ctx->non_secure_mem;
> > +     }
> > +
> > +     /* Check there is enough space in the shared memory. */
> > +     if (shared_mem->size < shared_mem->pos
> > +                     || io.length >= shared_mem->size - shared_mem->pos) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Not enough space in shared memory\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
> > +
> > +     /* Allocate space in shared memory. 8 bytes aligned. */
> > +     pos = shared_mem->pos;
> > +     shared_mem->pos += round_up(io.length, 8u);
>
> You are checking if there's enough space in the shared memory without taking
> this round_up into account.

Yes. It is initializing the local variable 'pos', with last store value of shared_mem->pos.

>
> > +     io.ele_addr = (u64)shared_mem->dma_addr + pos;
> > +
> > +     if ((io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) &&
> > +         !(io.flags & SE_MU_IO_FLAGS_USE_SHORT_ADDR)) {
> > +             /*Add base address to get full address.*/
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed allocate SEC MEM memory\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +     memset(shared_mem->ptr + pos, 0, io.length);
> > +     if ((io.flags & SE_IO_BUF_FLAGS_IS_INPUT) ||
> > +         (io.flags & SE_IO_BUF_FLAGS_IS_IN_OUT)) {
> > +             /*
> > +              * buffer is input:
> > +              * copy data from user space to this allocated buffer.
> > +              */
> > +             if (copy_from_user(shared_mem->ptr + pos, io.user_buf,
> > +                                io.length)) {
> > +                     dev_err(dev_ctx->priv->dev,
> > +                             "%s: Failed copy data to shared memory\n",
> > +                             dev_ctx->miscdev.name);
> > +                     err = -EFAULT;
> > +                     goto exit;
> > +             }
> > +     }
> > +
> > +     b_desc = kzalloc(sizeof(*b_desc), GFP_KERNEL);
> > +     if (!b_desc) {
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
> > +
> > +copy:
> > +     /* Provide the EdgeLock Enclave address to user space only if success.*/
> > +     if (copy_to_user((u8 *)arg, &io, sizeof(io))) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed to copy iobuff setup to user\n",
> > +                             dev_ctx->miscdev.name);
> > +             kfree(b_desc);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +     if (b_desc) {
> > +             b_desc->shared_buf_ptr = shared_mem->ptr + pos;
> > +             b_desc->usr_buf_ptr = io.user_buf;
> > +             b_desc->size = io.length;
> > +
> > +             if (io.flags & SE_IO_BUF_FLAGS_IS_INPUT) {
> > +                     /*
> > +                      * buffer is input:
> > +                      * add an entry in the "pending input buffers" list so
> > +                      * that copied data can be cleaned from shared memory
> > +                      * later.
> > +                      */
> > +                     list_add_tail(&b_desc->link, &dev_ctx->pending_in);
> > +             } else {
> > +                     /*
> > +                      * buffer is output:
> > +                      * add an entry in the "pending out buffers" list so data
> > +                      * can be copied to user space when receiving Secure-Enclave
> > +                      * response.
> > +                      */
> > +                     list_add_tail(&b_desc->link, &dev_ctx->pending_out);
> > +             }
> > +     }
> > +
> > +exit:
> > +     return err;
> > +}
> > +
> > +/* IOCTL to provide SoC information */ static int
> > +se_ioctl_get_se_soc_info_handler(struct se_if_device_ctx *dev_ctx,
> > +                                          u64 arg) {
> > +     struct imx_se_node_info_list *info_list;
> > +     struct se_ioctl_get_soc_info soc_info;
> > +     int err = -EINVAL;
> > +
> > +     info_list = (struct imx_se_node_info_list *)
> > +                     device_get_match_data(dev_ctx->priv->dev);
>
> device_get_match_data() returns a const void *. It is generally not necessary
> to explicitly cast void * to another pointer type. By explicitly casting it you just
> cast away the 'const' without noticing.


Accepted. Will correct it v3.

>
> > +     if (!info_list)
> > +             goto exit;
> > +
> > +     soc_info.soc_id = info_list->soc_id;
> > +     soc_info.soc_rev = dev_ctx->priv->soc_rev;
> > +
> > +     err = (int)copy_to_user((u8 *)arg, (u8 *)(&soc_info),
> > + sizeof(soc_info));
>
> First argument of copy_to_user() is a void *, then why do you cast it to
> something entirely unrelated (u8)? Second argument is a void * as well, so no
> need to explicitly cast.

Accepted, will correct in v3.

>
> > +     if (err) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed to copy soc info to user\n",
> > +                     dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
>
> This goto is unnecessary.
Accepted will corrected in v3.

>
> > +     }
> > +
> > +exit:
> > +     return err;
> > +}
> > +
> > +/* Open a character device. */
> > +static int se_if_fops_open(struct inode *nd, struct file *fp) {
> > +     struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> > +                                                     struct se_if_device_ctx,
> > +                                                     miscdev);
> > +     int err = 0;
> > +
> > +     /* Avoid race if opened at the same time */
> > +     if (down_trylock(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     /* Authorize only 1 instance. */
> > +     if (dev_ctx->status != SE_IF_CTX_FREE) {
> > +             err = -EBUSY;
> > +             goto exit;
> > +     }
> > +
> > +     /*
> > +      * Allocate some memory for data exchanges with S40x.
> > +      * This will be used for data not requiring secure memory.
> > +      */
> > +     dev_ctx->non_secure_mem.ptr = dmam_alloc_coherent(dev_ctx->dev,
> > +                                     MAX_DATA_SIZE_PER_USER,
> > +                                     &dev_ctx->non_secure_mem.dma_addr,
> > +                                     GFP_KERNEL);
>
> As Marc already mentioned: There is no point in using the managed version on
> dma_alloc_coherent() here.

Since it associated with device context and will remain associated till the device ctx is closed.
It was left to be part of managed version.
Accepted. Will correct it.
Will check other instances too.

>
> > +     if (!dev_ctx->non_secure_mem.ptr) {
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
> > +
> > +     dev_ctx->non_secure_mem.size = MAX_DATA_SIZE_PER_USER;
> > +     dev_ctx->non_secure_mem.pos = 0;
> > +     dev_ctx->status = SE_IF_CTX_OPENED;
> > +
> > +     dev_ctx->pending_hdr = 0;
> > +
> > +     goto exit;
> > +
> > +     dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
> > +                        dev_ctx->non_secure_mem.ptr,
> > +                        dev_ctx->non_secure_mem.dma_addr);
>
> This code is unreachable.

Accepted. Will correct in v3.

>
> > +
> > +exit:
> > +     up(&dev_ctx->fops_lock);
> > +     return err;
> > +}
> > +
> > +/* Close a character device. */
> > +static int se_if_fops_close(struct inode *nd, struct file *fp) {
> > +     struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> > +                                                     struct se_if_device_ctx,
> > +                                                     miscdev);
> > +     struct se_if_priv *priv = dev_ctx->priv;
> > +     struct se_buf_desc *b_desc;
> > +
> > +     /* Avoid race if closed at the same time */
> > +     if (down_trylock(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     /* The device context has not been opened */
> > +     if (dev_ctx->status != SE_IF_CTX_OPENED)
> > +             goto exit;
> > +
> > +     /* check if this device was registered as command receiver. */
> > +     if (priv->cmd_receiver_dev == dev_ctx)
> > +             priv->cmd_receiver_dev = NULL;
> > +
> > +     /* check if this device was registered as waiting response. */
> > +     if (priv->waiting_rsp_dev == dev_ctx) {
> > +             priv->waiting_rsp_dev = NULL;
> > +             mutex_unlock(&priv->se_if_cmd_lock);
> > +     }
> > +
> > +     /* Unmap secure memory shared buffer. */
> > +     if (dev_ctx->secure_mem.ptr)
> > +             devm_iounmap(dev_ctx->dev, dev_ctx->secure_mem.ptr);
>
> secure_mem is unused in this patch set. Drop this and re-add it later when
> you want to add support for this.
Accepted, will correct in v3.

>
> > +
> > +     dev_ctx->secure_mem.ptr = NULL;
> > +     dev_ctx->secure_mem.dma_addr = 0;
> > +     dev_ctx->secure_mem.size = 0;
> > +     dev_ctx->secure_mem.pos = 0;
> > +
> > +     /* Free non-secure shared buffer. */
> > +     dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
> > +                        dev_ctx->non_secure_mem.ptr,
> > +                        dev_ctx->non_secure_mem.dma_addr);
> > +
> > +     dev_ctx->non_secure_mem.ptr = NULL;
> > +     dev_ctx->non_secure_mem.dma_addr = 0;
> > +     dev_ctx->non_secure_mem.size = 0;
> > +     dev_ctx->non_secure_mem.pos = 0;
> > +
> > +     while (!list_empty(&dev_ctx->pending_in) ||
> > +            !list_empty(&dev_ctx->pending_out)) {
> > +             if (!list_empty(&dev_ctx->pending_in))
> > +                     b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
> > +                                                       struct se_buf_desc,
> > +                                                       link);
> > +             else
> > +                     b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> > +                                                       struct se_buf_desc,
> > +                                                       link);
> > +
> > +             if (!b_desc)
> > +                     continue;
> > +
> > +             if (b_desc->shared_buf_ptr)
> > +                     memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> > +
> > +             __list_del_entry(&b_desc->link);
> > +             kfree(b_desc);
> > +     }
> > +
> > +     dev_ctx->status = SE_IF_CTX_FREE;
> > +
> > +exit:
> > +     up(&dev_ctx->fops_lock);
> > +     return 0;
> > +}
> > +
> > +/* IOCTL entry point of a character device */ static long
> > +se_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) {
> > +     struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> > +                                                     struct se_if_device_ctx,
> > +                                                     miscdev);
> > +     struct se_if_priv *se_if_priv = dev_ctx->priv;
> > +     int err = -EINVAL;
> > +
> > +     /* Prevent race during change of device context */
> > +     if (down_interruptible(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     switch (cmd) {
> > +     case SE_IOCTL_ENABLE_CMD_RCV:
> > +             if (!se_if_priv->cmd_receiver_dev) {
> > +                     se_if_priv->cmd_receiver_dev = dev_ctx;
> > +                     err = 0;
> > +             }
> > +             break;
> > +     case SE_IOCTL_GET_MU_INFO:
> > +             err = se_ioctl_get_mu_info(dev_ctx, arg);
> > +             break;
> > +     case SE_IOCTL_SETUP_IOBUF:
> > +             err = se_ioctl_setup_iobuf_handler(dev_ctx, arg);
> > +             break;
> > +     case SE_IOCTL_GET_SOC_INFO:
> > +             err = se_ioctl_get_se_soc_info_handler(dev_ctx, arg);
> > +             break;
> > +
> > +     default:
> > +             err = -EINVAL;
> > +             dev_dbg(se_if_priv->dev,
> > +                     "%s: IOCTL %.8x not supported\n",
> > +                             dev_ctx->miscdev.name,
> > +                             cmd);
> > +     }
> > +
> > +     up(&dev_ctx->fops_lock);
> > +     return (long)err;
> > +}
> > +
> > +/* Char driver setup */
> > +static const struct file_operations se_if_fops = {
> > +     .open           = se_if_fops_open,
> > +     .owner          = THIS_MODULE,
> > +     .release        = se_if_fops_close,
> > +     .unlocked_ioctl = se_ioctl,
> > +     .read           = se_if_fops_read,
> > +     .write          = se_if_fops_write,
> > +};
> > +
> > +/* interface for managed res to unregister a character device */
> > +static void if_misc_deregister(void *miscdevice) {
> > +     misc_deregister(miscdevice);
> > +}
> > +
> >  /* interface for managed res to free a mailbox channel */  static
> > void if_mbox_free_channel(void *mbox_chan)  { @@ -270,6 +855,7 @@
> > static int se_probe_if_cleanup(struct platform_device *pdev)
> >       struct device *dev = &pdev->dev;
> >       struct se_if_priv *priv;
> >       int ret = 0;
> > +     int i;
> >
> >       priv = dev_get_drvdata(dev);
> >       if (!priv) {
> > @@ -294,6 +880,17 @@ static int se_probe_if_cleanup(struct
> platform_device *pdev)
> >               priv->imem.buf = NULL;
> >       }
> >
> > +     if (priv->ctxs) {
> > +             for (i = 0; i < priv->max_dev_ctx; i++) {
> > +                     if (priv->ctxs[i]) {
> > +                             devm_remove_action(dev,
> > +                                                if_misc_deregister,
> > +                                                &priv->ctxs[i]->miscdev);
> > +                             misc_deregister(&priv->ctxs[i]->miscdev);
> > +                     }
> > +             }
> > +     }
> > +
> >       if (priv->flags & RESERVED_DMA_POOL) {
> >               of_reserved_mem_device_release(dev);
> >               priv->flags &= (~RESERVED_DMA_POOL); @@ -302,6 +899,84
> > @@ static int se_probe_if_cleanup(struct platform_device *pdev)
> >       return ret;
> >  }
> >
> > +static int init_device_context(struct device *dev) {
> > +     const struct imx_se_node_info *info;
> > +     struct se_if_device_ctx *dev_ctx;
> > +     struct se_if_priv *priv;
> > +     u8 *devname;
> > +     int ret = 0;
> > +     int i;
> > +
> > +     priv = dev_get_drvdata(dev);
> > +
> > +     if (!priv) {
> > +             ret = -EINVAL;
> > +             dev_err(dev, "Invalid SE-MU Priv data");
> > +             return ret;
> > +     }
>
> You won't hit this as you already have called dev_set_drvdata(). Just drop this
> check. Also you should pass a struct se_if_priv * directly to this function
> instead of taking the detour of passing a struct device *.

Ok. Accepted.

>
> > +     info = priv->info;
> > +
> > +     priv->ctxs = devm_kzalloc(dev, sizeof(dev_ctx) * priv->max_dev_ctx,
> > +                               GFP_KERNEL);
> > +
> > +     if (!priv->ctxs) {
> > +             ret = -ENOMEM;
> > +             return ret;
> > +     }
> > +
> > +     /* Create users */
> > +     for (i = 0; i < priv->max_dev_ctx; i++) {
> > +             dev_ctx = devm_kzalloc(dev, sizeof(*dev_ctx), GFP_KERNEL);
> > +             if (!dev_ctx) {
> > +                     ret = -ENOMEM;
> > +                     return ret;
> > +             }
> > +
> > +             dev_ctx->dev = dev;
> > +             dev_ctx->status = SE_IF_CTX_FREE;
> > +             dev_ctx->priv = priv;
> > +
> > +             priv->ctxs[i] = dev_ctx;
> > +
> > +             /* Default value invalid for an header. */
> > +             init_waitqueue_head(&dev_ctx->wq);
> > +
> > +             INIT_LIST_HEAD(&dev_ctx->pending_out);
> > +             INIT_LIST_HEAD(&dev_ctx->pending_in);
> > +             sema_init(&dev_ctx->fops_lock, 1);
> > +
> > +             devname = devm_kasprintf(dev, GFP_KERNEL, "%s_ch%d",
> > +                                      info->se_name, i);
> > +             if (!devname) {
> > +                     ret = -ENOMEM;
> > +                     return ret;
> > +             }
> > +
> > +             dev_ctx->miscdev.name = devname;
> > +             dev_ctx->miscdev.minor = MISC_DYNAMIC_MINOR;
> > +             dev_ctx->miscdev.fops = &se_if_fops;
> > +             dev_ctx->miscdev.parent = dev;
> > +             ret = misc_register(&dev_ctx->miscdev);
> > +             if (ret) {
> > +                     dev_err(dev, "failed to register misc device %d\n",
> > +                             ret);
> > +                     return ret;
> > +             }
> > +
> > +             ret = devm_add_action(dev, if_misc_deregister,
> > +                                   &dev_ctx->miscdev);
> > +             if (ret) {
> > +                     dev_err(dev,
> > +                             "failed[%d] to add action to the misc-dev\n",
> > +                             ret);
> > +                     return ret;
> > +             }
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> >  static void se_load_firmware(const struct firmware *fw, void
> > *context)  {
> >       struct se_if_priv *priv = (struct se_if_priv *) context; @@
> > -461,6 +1136,16 @@ static int se_if_probe(struct platform_device *pdev)
> >               }
> >       }
> >
> > +     if (info->max_dev_ctx) {
> > +             ret = init_device_context(dev);
> > +             if (ret) {
> > +                     dev_err(dev,
> > +                             "Failed[0x%x] to create device contexts.\n",
> > +                             ret);
> > +                     goto exit;
> > +             }
> > +     }
> > +
> >       dev_info(dev, "i.MX secure-enclave: %s interface to firmware,
> configured.\n",
> >                info->se_name);
> >       return devm_of_platform_populate(dev); @@ -502,6 +1187,10 @@
> > static int se_resume(struct device *dev)
> >       struct se_if_priv *priv = dev_get_drvdata(dev);
> >       const struct imx_se_node_info *info
> >                                       = priv->info;
> > +     int i;
> > +
> > +     for (i = 0; i < priv->max_dev_ctx; i++)
> > +             wake_up_interruptible(&priv->ctxs[i]->wq);
> >
> >       if (info && info->imem_mgmt)
> >               se_restore_imem_state(dev);
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> http://www.pe/
> ngutronix.de%2F&data=05%7C02%7Cpankaj.gupta%40nxp.com%7C5351b96c
> b80a4eefa99d08dc7bcac8b3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C638521358079376269%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7
> C%7C%7C&sdata=rI4tOYhDyFr7rHCBqFUJ%2FNBzVGf2mha4d20F2QM%2BN6
> Y%3D&reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: [phone]    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |

Sascha Hauer May 24, 2024, 12:43 p.m. UTC | #3

On Fri, May 24, 2024 at 12:03:35PM +0000, Pankaj Gupta wrote:
> 
> 
> > -----Original Message-----
> > From: Sascha Hauer <[e-mail]>
> > Sent: Friday, May 24, 2024 1:53 PM
> > To: Pankaj Gupta <[e-mail]>
> > Cc: Jonathan Corbet <[e-mail]>; Rob Herring <[e-mail]>;
> > Krzysztof Kozlowski <[e-mail]>; Conor Dooley
> > <[e-mail]>; Shawn Guo <[e-mail]>; Pengutronix
> > Kernel Team <[e-mail]>; Fabio Estevam
> > <[e-mail]>; Rob Herring <[e-mail]>; Krzysztof Kozlowski
> > <[e-mail]>; [e-mail]; linux-
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]
> > Subject: [EXT] Re: [PATCH v2 5/5] firmware: imx: adds miscdev
> >
> > Caution: This is an external email. Please take care when clicking links or
> > opening attachments. When in doubt, report the message using the 'Report
> > this email' button
> >
> >
> > On Thu, May 23, 2024 at 04:19:36PM +0530, Pankaj Gupta wrote:
> > > +int imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
> > > +                          void *tx_msg, int tx_msg_sz) {
> > > +     struct se_if_priv *priv = dev_ctx->priv;
> > > +     struct se_msg_hdr *header;
> > > +     int err;
> > > +
> > > +     header = (struct se_msg_hdr *) tx_msg;
> > > +
> > > +     /*
> > > +      * Check that the size passed as argument matches the size
> > > +      * carried in the message.
> > > +      */
> > > +     err = header->size << 2;
> > > +
> > > +     if (err != tx_msg_sz) {
> > > +             err = -EINVAL;
> > > +             dev_err(priv->dev,
> > > +                     "%s: User buffer too small\n",
> > > +                             dev_ctx->miscdev.name);
> > > +             goto exit;
> > > +     }
> > > +     /* Check the message is valid according to tags */
> > > +     if (header->tag == priv->cmd_tag) {
> > > +             mutex_lock(&priv->se_if_cmd_lock);
> >
> > Grabbing a mutex in a character devices write fop and releasing it in the read
> > fop is really calling for undesired race conditions.
> 
> Condition is:
> - Only one command is allowed to be in flight, at a time per interface.
>    -- Second command is not allowed, when one command is in flight.
> - Duration of the flight is till the time the response is not received from the FW.
> 
> Command lock is grabbed and then released in process context only.
> 
> >
> > If sending a command and receiving the response shall be an atomic operation
> > then you should really consider turning this into an ioctl and just not
> > implement read/write on the character device. With this you'll be able to get
> > rid of several oddities in this drivers locking.
> >
> 
> It is not an atomic operation. It can be pre-empted.

I didn't mean atomic in the sense of being non preemptable.

> But it cannot be pre-empted to send another command on the same interface.
> 
> As only one command is allowed to be executed at one point in time, through an interface.

I meant atomic in the sense that only one command may be in flight: Send
a message and do not allow to send another message until the answer to
the first one is received.

Using an ioctl you can just use imx_ele_msg_send_rcv() which takes a
mutex during the whole send/receive process and have no need for such a
strange locking construct.

> > > +     /*
> > > +      * We may need to copy the output data to user before
> > > +      * delivering the completion message.
> > > +      */
> > > +     while (!list_empty(&dev_ctx->pending_out)) {
> > > +             b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> > > +                                               struct se_buf_desc,
> > > +                                               link);
> > > +             if (!b_desc)
> > > +                     continue;
> >
> > b_desc will never be NULL because otherwise you wouldn't be in the loop
> > anymore. The usual way to iterate over a list is to use list_for_each_entry() or
> > list_for_each_entry_safe() in case you delete entries in the loop body.
> >
> 
> Will remove the NULL check.
>         if (!b_desc)
>                continue;

Please don't. Use list_for_each_entry_safe() which is the normal way to
iterate over a list.

> > > +static int se_ioctl_get_mu_info(struct se_if_device_ctx *dev_ctx,
> > > +                             u64 arg) {
> > > +     struct se_if_priv *priv = dev_get_drvdata(dev_ctx->dev);
> > > +     struct imx_se_node_info *if_node_info;
> > > +     struct se_ioctl_get_if_info info;
> > > +     int err = 0;
> > > +
> > > +     if_node_info = (struct imx_se_node_info *)priv->info;
> >
> > priv->info is of type const void *. You are casting away the the 'const'
> > here. Either it is const, then it should stay const, or not, in which case it
> > shouldn't be declared const. Also why isn't priv->info of type struct
> > imx_se_node_info * in the first place?
> 
> This struct definition is local to the file se_ctrl.c.
> Declaration of imx_se_node_info, is fixed by adding const in the whole file.

Add a

struct imx_se_node_info;

to se_ctrl.h and you're done.

> 
> > > +             err = -EFAULT;
> > > +             goto exit;
> > > +     } else {
> > > +             /* No specific requirement for this buffer. */
> > > +             shared_mem = &dev_ctx->non_secure_mem;
> > > +     }
> > > +
> > > +     /* Check there is enough space in the shared memory. */
> > > +     if (shared_mem->size < shared_mem->pos
> > > +                     || io.length >= shared_mem->size - shared_mem->pos) {
> > > +             dev_err(dev_ctx->priv->dev,
> > > +                     "%s: Not enough space in shared memory\n",
> > > +                             dev_ctx->miscdev.name);
> > > +             err = -ENOMEM;
> > > +             goto exit;
> > > +     }
> > > +
> > > +     /* Allocate space in shared memory. 8 bytes aligned. */
> > > +     pos = shared_mem->pos;
> > > +     shared_mem->pos += round_up(io.length, 8u);
> >
> > You are checking if there's enough space in the shared memory without taking
> > this round_up into account.
> 
> Yes. It is initializing the local variable 'pos', with last store value of shared_mem->pos.

Your check is:

	if (shared_mem->size < shared_mem->pos || io.length >= shared_mem->size - shared_mem->pos)

Afterwards you do a:

	shared_mem->pos += round_up(io.length, 8u);

This invalidates the check. You have to honor the potential padding in
your check as well.

Sascha

Pankaj Gupta June 1, 2024, 4:38 a.m. UTC | #4

> -----Original Message-----
> From: Sascha Hauer <[e-mail]>
> Sent: Friday, May 24, 2024 6:14 PM
> To: Pankaj Gupta <[e-mail]>
> Cc: Jonathan Corbet <[e-mail]>; Rob Herring <[e-mail]>;
> Krzysztof Kozlowski <[e-mail]>; Conor Dooley
> <[e-mail]>; Shawn Guo <[e-mail]>; Pengutronix
> Kernel Team <[e-mail]>; Fabio Estevam
> <[e-mail]>; Rob Herring <[e-mail]>; Krzysztof Kozlowski
> <[e-mail]>; [e-mail]; linux-
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]
> Subject: Re: [EXT] Re: [PATCH v2 5/5] firmware: imx: adds miscdev
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> On Fri, May 24, 2024 at 12:03:35PM +0000, Pankaj Gupta wrote:
> >
> >
> > > -----Original Message-----
> > > From: Sascha Hauer <[e-mail]>
> > > Sent: Friday, May 24, 2024 1:53 PM
> > > To: Pankaj Gupta <[e-mail]>
> > > Cc: Jonathan Corbet <[e-mail]>; Rob Herring
> > > <[e-mail]>; Krzysztof Kozlowski
> > > <[e-mail]>; Conor Dooley
> > > <[e-mail]>; Shawn Guo <[e-mail]>;
> Pengutronix
> > > Kernel Team <[e-mail]>; Fabio Estevam
> > > <[e-mail]>; Rob Herring <[e-mail]>; Krzysztof
> > > Kozlowski <[e-mail]>; [e-mail]; linux-
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]
> > > Subject: [EXT] Re: [PATCH v2 5/5] firmware: imx: adds miscdev
> > >
> > > Caution: This is an external email. Please take care when clicking
> > > links or opening attachments. When in doubt, report the message
> > > using the 'Report this email' button
> > >
> > >
> > > On Thu, May 23, 2024 at 04:19:36PM +0530, Pankaj Gupta wrote:
> > > > +int imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
> > > > +                          void *tx_msg, int tx_msg_sz) {
> > > > +     struct se_if_priv *priv = dev_ctx->priv;
> > > > +     struct se_msg_hdr *header;
> > > > +     int err;
> > > > +
> > > > +     header = (struct se_msg_hdr *) tx_msg;
> > > > +
> > > > +     /*
> > > > +      * Check that the size passed as argument matches the size
> > > > +      * carried in the message.
> > > > +      */
> > > > +     err = header->size << 2;
> > > > +
> > > > +     if (err != tx_msg_sz) {
> > > > +             err = -EINVAL;
> > > > +             dev_err(priv->dev,
> > > > +                     "%s: User buffer too small\n",
> > > > +                             dev_ctx->miscdev.name);
> > > > +             goto exit;
> > > > +     }
> > > > +     /* Check the message is valid according to tags */
> > > > +     if (header->tag == priv->cmd_tag) {
> > > > +             mutex_lock(&priv->se_if_cmd_lock);
> > >
> > > Grabbing a mutex in a character devices write fop and releasing it
> > > in the read fop is really calling for undesired race conditions.
> >
> > Condition is:
> > - Only one command is allowed to be in flight, at a time per interface.
> >    -- Second command is not allowed, when one command is in flight.
> > - Duration of the flight is till the time the response is not received from the
> FW.
> >
> > Command lock is grabbed and then released in process context only.
> >
> > >
> > > If sending a command and receiving the response shall be an atomic
> > > operation then you should really consider turning this into an ioctl
> > > and just not implement read/write on the character device. With this
> > > you'll be able to get rid of several oddities in this drivers locking.
> > >
> >
> > It is not an atomic operation. It can be pre-empted.
>
> I didn't mean atomic in the sense of being non preemptable.
>
> > But it cannot be pre-empted to send another command on the same
> interface.
> >
> > As only one command is allowed to be executed at one point in time,
> through an interface.
>
> I meant atomic in the sense that only one command may be in flight: Send a
> message and do not allow to send another message until the answer to the
> first one is received.
>
> Using an ioctl you can just use imx_ele_msg_send_rcv() which takes a mutex
> during the whole send/receive process and have no need for such a strange
> locking construct.
>
> > > > +     /*
> > > > +      * We may need to copy the output data to user before
> > > > +      * delivering the completion message.
> > > > +      */
> > > > +     while (!list_empty(&dev_ctx->pending_out)) {
> > > > +             b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> > > > +                                               struct se_buf_desc,
> > > > +                                               link);
> > > > +             if (!b_desc)
> > > > +                     continue;
> > >
> > > b_desc will never be NULL because otherwise you wouldn't be in the
> > > loop anymore. The usual way to iterate over a list is to use
> > > list_for_each_entry() or
> > > list_for_each_entry_safe() in case you delete entries in the loop body.
> > >
> >
> > Will remove the NULL check.
> >         if (!b_desc)
> >                continue;
>
> Please don't. Use list_for_each_entry_safe() which is the normal way to
> iterate over a list.

This change will add few more cpu cycles.
Will add this in v3.

>
> > > > +static int se_ioctl_get_mu_info(struct se_if_device_ctx *dev_ctx,
> > > > +                             u64 arg) {
> > > > +     struct se_if_priv *priv = dev_get_drvdata(dev_ctx->dev);
> > > > +     struct imx_se_node_info *if_node_info;
> > > > +     struct se_ioctl_get_if_info info;
> > > > +     int err = 0;
> > > > +
> > > > +     if_node_info = (struct imx_se_node_info *)priv->info;
> > >
> > > priv->info is of type const void *. You are casting away the the 'const'
> > > here. Either it is const, then it should stay const, or not, in
> > > which case it shouldn't be declared const. Also why isn't priv->info
> > > of type struct imx_se_node_info * in the first place?
> >
> > This struct definition is local to the file se_ctrl.c.
> > Declaration of imx_se_node_info, is fixed by adding const in the whole file.
>
> Add a
>
> struct imx_se_node_info;
>
> to se_ctrl.h and you're done.

The scope of this structure will remain to se_ctrl.c only.
If you suggest, will replace the info structure with secure-enclave interface index(u8 if_idx).

>
> >
> > > > +             err = -EFAULT;
> > > > +             goto exit;
> > > > +     } else {
> > > > +             /* No specific requirement for this buffer. */
> > > > +             shared_mem = &dev_ctx->non_secure_mem;
> > > > +     }
> > > > +
> > > > +     /* Check there is enough space in the shared memory. */
> > > > +     if (shared_mem->size < shared_mem->pos
> > > > +                     || io.length >= shared_mem->size - shared_mem->pos) {

Will update this check to replace io.length with round_up(io.length).
Will correct in v3.

> > > > +             dev_err(dev_ctx->priv->dev,
> > > > +                     "%s: Not enough space in shared memory\n",
> > > > +                             dev_ctx->miscdev.name);
> > > > +             err = -ENOMEM;
> > > > +             goto exit;
> > > > +     }
> > > > +
> > > > +     /* Allocate space in shared memory. 8 bytes aligned. */
> > > > +     pos = shared_mem->pos;
> > > > +     shared_mem->pos += round_up(io.length, 8u);
> > >
> > > You are checking if there's enough space in the shared memory
> > > without taking this round_up into account.
> >
> > Yes. It is initializing the local variable 'pos', with last store value of
> shared_mem->pos.
>
> Your check is:
>
>         if (shared_mem->size < shared_mem->pos || io.length >= shared_mem-
> >size - shared_mem->pos)
>
> Afterwards you do a:
>
>         shared_mem->pos += round_up(io.length, 8u);
>
> This invalidates the check. You have to honor the potential padding in your
> check as well.
>
See the comment above.

> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> http://www.pe/
> ngutronix.de%2F&data=05%7C02%7Cpankaj.gupta%40nxp.com%7Cbd91341
> 92f06423be17c08dc7bef290e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C638521514310084478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%
> 7C%7C%7C&sdata=iVTkihKZ0F9ATk%2FTXpXMqmj8tJq8ufKijglcPWegjA4%3D&
> reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: [phone]    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |

Amit Singh Tomar June 3, 2024, 4:22 p.m. UTC | #5

Hi,

> 
> ----------------------------------------------------------------------
> Adds the driver for communication interface to secure-enclave,
> for exchanging messages with NXP secure enclave HW IP(s) like
> EdgeLock Enclave from:
> - User-Space Applications via character driver.
> 
> ABI documentation for the NXP secure-enclave driver.
> 
> User-space library using this driver:
> - i.MX Secure Enclave library:
>    -- URL: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsecure-2Denclave.git&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=8xuiz3OEyshbkzLQ6-G2afLkh7sVv9b_HkFaPJVBu4wjmgAowHGi90q5RTroZ2oy&s=xi3fO_c9z_t-zdk3LdTGgqJ6M-5OjRD6oj-ECiVQ40Q&e= ,
> - i.MX Secure Middle-Ware:
>    -- URL: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsmw.git&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=8xuiz3OEyshbkzLQ6-G2afLkh7sVv9b_HkFaPJVBu4wjmgAowHGi90q5RTroZ2oy&s=jPOlKXqt_GIZGMvMboOdjkwu3UTpZ7fwEFm8Ki5z0LE&e=
> 
> Signed-off-by: Pankaj Gupta <[e-mail]>
> ---
>   Documentation/ABI/testing/se-cdev |  42 +++
>   drivers/firmware/imx/ele_common.c | 108 +++++-
>   drivers/firmware/imx/ele_common.h |   3 +
>   drivers/firmware/imx/se_ctrl.c    | 689 ++++++++++++++++++++++++++++++++++++++
>   drivers/firmware/imx/se_ctrl.h    |  46 +++
>   include/uapi/linux/se_ioctl.h     |  88 +++++
>   6 files changed, 974 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/se-cdev b/Documentation/ABI/testing/se-cdev
> new file mode 100644
> index 000000000000..699525af6b86
> --- /dev/null
> +++ b/Documentation/ABI/testing/se-cdev
> @@ -0,0 +1,42 @@
> +What:		/dev/<se>_mu[0-9]+_ch[0-9]+
> +Date:		May 2024
> +KernelVersion:	6.8
> +Contact:	[e-mail], [e-mail]
> +Description:
> +		NXP offers multiple hardware IP(s) for  secure-enclaves like EdgeLock-
> +		Enclave(ELE), SECO. The character device file-descriptors
> +		/dev/<se>_mu*_ch* are the interface between user-space NXP's secure-
> +		enclave shared-library and the kernel driver.
> +
> +		The ioctl(2)-based ABI is defined and documented in
> +		[include]<linux/firmware/imx/ele_mu_ioctl.h>
> +		 ioctl(s) are used primarily for:
> +			- shared memory management
> +			- allocation of I/O buffers
> +			- get mu info
> +			- setting a dev-ctx as receiver that is slave to fw
> +			- get SoC info
> +
> +		The following file operations are supported:
> +
> +		open(2)
> +		  Currently the only useful flags are O_RDWR.
> +
> +		read(2)
> +		  Every read() from the opened character device context is waiting on
> +		  wakeup_intruptible, that gets set by the registered mailbox callback
> +		  function; indicating a message received from the firmware on message-
> +		  unit.
> +
> +		write(2)
> +		  Every write() to the opened character device context needs to acquire
> +		  mailbox_lock, before sending message on to the message unit.
> +
> +		close(2)
> +		  Stops and free up the I/O contexts that was associated
> +		  with the file descriptor.
> +
> +Users:		https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsecure-2Denclave.git&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=8xuiz3OEyshbkzLQ6-G2afLkh7sVv9b_HkFaPJVBu4wjmgAowHGi90q5RTroZ2oy&s=xi3fO_c9z_t-zdk3LdTGgqJ6M-5OjRD6oj-ECiVQ40Q&e= ,
> +		https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsmw.git&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=8xuiz3OEyshbkzLQ6-G2afLkh7sVv9b_HkFaPJVBu4wjmgAowHGi90q5RTroZ2oy&s=jPOlKXqt_GIZGMvMboOdjkwu3UTpZ7fwEFm8Ki5z0LE&e=
> +		crypto/skcipher,
> +		drivers/nvmem/imx-ocotp-ele.c
> diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
> index c286c3d84d82..15fabc369b21 100644
> --- a/drivers/firmware/imx/ele_common.c
> +++ b/drivers/firmware/imx/ele_common.c
> @@ -78,12 +78,98 @@ int imx_ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, void *rx_msg)
>   	return err;
>   }
>   
> +int imx_ele_miscdev_msg_rcv(struct se_if_device_ctx *dev_ctx)
> +{
> +	struct se_msg_hdr *header = {0};
> +	int err;
> +
> +	if (dev_ctx->priv->waiting_rsp_dev == dev_ctx)
> +		lockdep_assert_held(&dev_ctx->priv->se_if_cmd_lock);
> +
> +	err = wait_event_interruptible(dev_ctx->wq, dev_ctx->pending_hdr != 0);
> +	if (err)
> +		dev_err(dev_ctx->dev,
> +			"%s: Err[0x%x]:Interrupted by signal.\n",
> +			dev_ctx->miscdev.name, err);
> +
> +	header = (struct se_msg_hdr *) dev_ctx->temp_resp;
> +
> +	if (header->tag == dev_ctx->priv->rsp_tag) {
> +		if (dev_ctx->priv->waiting_rsp_dev != dev_ctx)
> +			dev_warn(dev_ctx->dev,
> +			"%s: Device context waiting for response mismatch.\n",
> +			dev_ctx->miscdev.name);
> +		else
> +			dev_ctx->priv->waiting_rsp_dev = NULL;
> +
> +		mutex_unlock(&dev_ctx->priv->se_if_cmd_lock);
> +	}
> +
> +	return err;
> +}
> +
> +int imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
> +			     void *tx_msg, int tx_msg_sz)
> +{
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	struct se_msg_hdr *header;
> +	int err;
> +
> +	header = (struct se_msg_hdr *) tx_msg;
> +
> +	/*
> +	 * Check that the size passed as argument matches the size
> +	 * carried in the message.
> +	 */
> +	err = header->size << 2;
> +
> +	if (err != tx_msg_sz) {
> +		err = -EINVAL;
> +		dev_err(priv->dev,
> +			"%s: User buffer too small\n",
> +				dev_ctx->miscdev.name);
> +		goto exit;
> +	}
> +	/* Check the message is valid according to tags */
> +	if (header->tag == priv->cmd_tag) {
> +		mutex_lock(&priv->se_if_cmd_lock);

In the previous patch (4/5), you used 'guard' locks. Wouldn't it be 
better to use them here as well, considering the lock handling seems a 
bit dodgy (I mean, the way lock is released only under certain condition)?

> +		priv->waiting_rsp_dev = dev_ctx;
> +	} else if (header->tag == priv->rsp_tag) {
> +		/* Check the device context can send the command */
> +		if (dev_ctx != priv->cmd_receiver_dev) {
> +			dev_err(priv->dev,
> +				"%s: Channel not configured to send resp to FW.",
> +				dev_ctx->miscdev.name);
> +			err = -EPERM;
> +			goto exit;
> +		}
> +	} else {
> +		dev_err(priv->dev,
> +			"%s: The message does not have a valid TAG\n",
> +				dev_ctx->miscdev.name);
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +	err = imx_ele_msg_send(priv, tx_msg);
> +	if (err < 0) {
> +		if (header->tag == priv->cmd_tag) {
> +			priv->waiting_rsp_dev = NULL;
> +			mutex_unlock(&dev_ctx->priv->se_if_cmd_lock);
> +		}
> +	} else
> +		err = header->size << 2;
> +exit:
> +	return err;
> +}
> +
>   /*
>    * Callback called by mailbox FW, when data is received.
>    */
>   void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
>   {
>   	struct device *dev = mbox_cl->dev;
> +	struct se_if_device_ctx *dev_ctx;
> +	struct se_api_msg *rx_msg;
>   	struct se_if_priv *priv;
>   	struct se_msg_hdr *header;
>   
> @@ -97,8 +183,15 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
>   
>   	header = (struct se_msg_hdr *) msg;
>   
> -	if (header->tag == priv->rsp_tag) {
> -		if (!priv->waiting_rsp_dev) {
> +	/* Incoming command: wake up the receiver if any. */
> +	if (header->tag == priv->cmd_tag) {
> +		dev_dbg(dev, "Selecting cmd receiver\n");
> +		dev_ctx = priv->cmd_receiver_dev;
> +	} else if (header->tag == priv->rsp_tag) {
> +		if (priv->waiting_rsp_dev) {
> +			dev_dbg(dev, "Selecting rsp waiter\n");
> +			dev_ctx = priv->waiting_rsp_dev;
> +		} else {
>   			/*
>   			 * Reading the EdgeLock Enclave response
>   			 * to the command, sent by other
> @@ -116,6 +209,17 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
>   				*((u32 *) header));
>   		return;
>   	}
> +	/* Init reception */
> +	rx_msg = kzalloc(header->size << 2, GFP_KERNEL);
> +	if (rx_msg)
> +		memcpy(rx_msg, msg, header->size << 2);
> +
> +	dev_ctx->temp_resp = (u32 *)rx_msg;
> +	dev_ctx->temp_resp_size = header->size;
> +
> +	/* Allow user to read */
> +	dev_ctx->pending_hdr = 1;
> +	wake_up_interruptible(&dev_ctx->wq);
>   }
>   
>   int validate_rsp_hdr(struct se_if_priv *priv,
> diff --git a/drivers/firmware/imx/ele_common.h b/drivers/firmware/imx/ele_common.h
> index 76777ac629d6..11b9b36d4fda 100644
> --- a/drivers/firmware/imx/ele_common.h
> +++ b/drivers/firmware/imx/ele_common.h
> @@ -12,6 +12,9 @@
>   #define IMX_ELE_FW_DIR                 "imx/ele/"
>   
>   uint32_t plat_add_msg_crc(uint32_t *msg, uint32_t msg_len);
> +int imx_ele_miscdev_msg_rcv(struct se_if_device_ctx *priv);
> +int imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
> +			     void *tx_msg, int tx_msg_sz);
>   int imx_ele_msg_rcv(struct se_if_priv *priv);
>   int imx_ele_msg_send(struct se_if_priv *priv, void *tx_msg);
>   int imx_ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, void *rx_msg);
> diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
> index 0642d349b3d3..3acaecd8f3bc 100644
> --- a/drivers/firmware/imx/se_ctrl.c
> +++ b/drivers/firmware/imx/se_ctrl.c
> @@ -23,6 +23,7 @@
>   #include <linux/slab.h>
>   #include <linux/string.h>
>   #include <linux/sys_soc.h>
> +#include <uapi/linux/se_ioctl.h>
>   
>   #include "ele_base_msg.h"
>   #include "ele_common.h"
> @@ -232,6 +233,590 @@ static int imx_fetch_se_soc_info(struct device *dev)
>   	return 0;
>   }
>   
> +/*
> + * File operations for user-space
> + */
> +
> +/* Write a message to the MU. */
> +static ssize_t se_if_fops_write(struct file *fp, const char __user *buf,
> +				size_t size, loff_t *ppos)
> +{
> +	struct se_api_msg *tx_msg __free(kfree);
> +	struct se_if_device_ctx *dev_ctx;
> +	struct se_if_priv *priv;
> +	int err;
> +
> +	dev_ctx = container_of(fp->private_data,
> +			       struct se_if_device_ctx,
> +			       miscdev);
> +	priv = dev_ctx->priv;
> +	dev_dbg(priv->dev,
> +		"%s: write from buf (%p)%zu, ppos=%lld\n",
> +			dev_ctx->miscdev.name,
> +			buf, size, ((ppos) ? *ppos : 0));
> +
> +	if (down_interruptible(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	if (dev_ctx->status != SE_IF_CTX_OPENED) {
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +
> +	if (size < SE_MU_HDR_SZ) {
> +		dev_err(priv->dev,
> +			"%s: User buffer too small(%zu < %d)\n",
> +				dev_ctx->miscdev.name,
> +				size, SE_MU_HDR_SZ);
> +		err = -ENOSPC;
> +		goto exit;
> +	}
> +	tx_msg = memdup_user(buf, size);
> +	if (!tx_msg) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	print_hex_dump_debug("from user ", DUMP_PREFIX_OFFSET, 4, 4,
> +			     tx_msg, size, false);
> +
> +	err = imx_ele_miscdev_msg_send(dev_ctx, tx_msg, size);
> +
> +exit:
> +	up(&dev_ctx->fops_lock);
> +	return err;
> +}
> +
> +/*
> + * Read a message from the MU.
> + * Blocking until a message is available.
> + */
> +static ssize_t se_if_fops_read(struct file *fp, char __user *buf,
> +			       size_t size, loff_t *ppos)
> +{
> +	struct se_if_device_ctx *dev_ctx;
> +	struct se_buf_desc *b_desc;
> +	struct se_if_priv *priv;
> +	u32 size_to_copy;
> +	int err;
> +
> +	dev_ctx = container_of(fp->private_data,
> +			       struct se_if_device_ctx,
> +			       miscdev);
> +	priv = dev_ctx->priv;
> +	dev_dbg(priv->dev,
> +		"%s: read to buf %p(%zu), ppos=%lld\n",
> +			dev_ctx->miscdev.name,
> +			buf, size, ((ppos) ? *ppos : 0));
> +
> +	if (down_interruptible(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	if (dev_ctx->status != SE_IF_CTX_OPENED) {
> +		err = -EINVAL;
> +		goto exit;
> +	}
> +
> +	err = imx_ele_miscdev_msg_rcv(dev_ctx);
> +	if (err)
> +		goto exit;
> +
> +	/* Buffer containing the message from FW, is
> +	 * allocated in callback function.
> +	 * Check if buffer allocation failed.
> +	 */
> +	if (!dev_ctx->temp_resp) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	dev_dbg(priv->dev,
> +			"%s: %s %s\n",
> +			dev_ctx->miscdev.name,
> +			__func__,
> +			"message received, start transmit to user");
> +
> +	/*
> +	 * Check that the size passed as argument is larger than
> +	 * the one carried in the message.
> +	 */
> +	size_to_copy = dev_ctx->temp_resp_size << 2;
> +	if (size_to_copy > size) {
> +		dev_dbg(priv->dev,
> +			"%s: User buffer too small (%zu < %d)\n",
> +				dev_ctx->miscdev.name,
> +				size, size_to_copy);
> +		size_to_copy = size;
> +	}
> +
> +	/*
> +	 * We may need to copy the output data to user before
> +	 * delivering the completion message.
> +	 */
> +	while (!list_empty(&dev_ctx->pending_out)) {
> +		b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> +						  struct se_buf_desc,
> +						  link);
> +		if (!b_desc)
> +			continue;
> +
> +		if (b_desc->usr_buf_ptr && b_desc->shared_buf_ptr) {
> +
> +			dev_dbg(priv->dev,
> +				"%s: Copy output data to user\n",
> +				dev_ctx->miscdev.name);
> +			if (copy_to_user(b_desc->usr_buf_ptr,
> +					 b_desc->shared_buf_ptr,
> +					 b_desc->size)) {
> +				dev_err(priv->dev,
> +					"%s: Failure copying output data to user.",
> +					dev_ctx->miscdev.name);
> +				err = -EFAULT;
> +				goto exit;
> +			}
> +		}
> +
> +		if (b_desc->shared_buf_ptr)
> +			memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> +
> +		__list_del_entry(&b_desc->link);
> +		kfree(b_desc);
> +	}
> +
> +	/* Copy data from the buffer */
> +	print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
> +			     dev_ctx->temp_resp, size_to_copy, false);
> +	if (copy_to_user(buf, dev_ctx->temp_resp, size_to_copy)) {
> +		dev_err(priv->dev,
> +			"%s: Failed to copy to user\n",
> +				dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +	err = size_to_copy;
> +	kfree(dev_ctx->temp_resp);
> +
> +	/* free memory allocated on the shared buffers. */
> +	dev_ctx->secure_mem.pos = 0;
> +	dev_ctx->non_secure_mem.pos = 0;
> +
> +	dev_ctx->pending_hdr = 0;
> +
> +exit:
> +	/*
> +	 * Clean the used Shared Memory space,
> +	 * whether its Input Data copied from user buffers, or
> +	 * Data received from FW.
> +	 */
> +	while (!list_empty(&dev_ctx->pending_in) ||
> +	       !list_empty(&dev_ctx->pending_out)) {
> +		if (!list_empty(&dev_ctx->pending_in))
> +			b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
> +							  struct se_buf_desc,
> +							  link);
> +		else
> +			b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> +							  struct se_buf_desc,
> +							  link);
> +
> +		if (!b_desc)
> +			continue;
> +
> +		if (b_desc->shared_buf_ptr)
> +			memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> +
> +		__list_del_entry(&b_desc->link);
> +		kfree(b_desc);
> +	}
> +
> +	up(&dev_ctx->fops_lock);
> +	return err;
> +}
> +
> +static int se_ioctl_get_mu_info(struct se_if_device_ctx *dev_ctx,
> +				u64 arg)
> +{
> +	struct se_if_priv *priv = dev_get_drvdata(dev_ctx->dev);
> +	struct imx_se_node_info *if_node_info;
> +	struct se_ioctl_get_if_info info;
> +	int err = 0;
> +
> +	if_node_info = (struct imx_se_node_info *)priv->info;
> +
> +	info.se_if_id = if_node_info->se_if_id;
> +	info.interrupt_idx = 0;
> +	info.tz = 0;
> +	info.did = if_node_info->se_if_did;
> +	info.cmd_tag = if_node_info->cmd_tag;
> +	info.rsp_tag = if_node_info->rsp_tag;
> +	info.success_tag = if_node_info->success_tag;
> +	info.base_api_ver = if_node_info->base_api_ver;
> +	info.fw_api_ver = if_node_info->fw_api_ver;
> +
> +	dev_dbg(priv->dev,
> +		"%s: info [se_if_id: %d, irq_idx: %d, tz: 0x%x, did: 0x%x]\n",
> +			dev_ctx->miscdev.name,
> +			info.se_if_id, info.interrupt_idx, info.tz, info.did);
> +
> +	if (copy_to_user((u8 *)arg, &info, sizeof(info))) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed to copy mu info to user\n",
> +				dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +exit:
> +	return err;
> +}
> +
> +/*
> + * Copy a buffer of data to/from the user and return the address to use in
> + * messages
> + */
> +static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
> +					    u64 arg)
> +{
> +	struct se_shared_mem *shared_mem = NULL;
> +	struct se_ioctl_setup_iobuf io = {0};
> +	struct se_buf_desc *b_desc = NULL;
> +	int err = 0;
> +	u32 pos;
> +
> +	if (copy_from_user(&io, (u8 *)arg, sizeof(io))) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed copy iobuf config from user\n",
> +				dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +	dev_dbg(dev_ctx->priv->dev,
> +			"%s: io [buf: %p(%d) flag: %x]\n",
> +			dev_ctx->miscdev.name,
> +			io.user_buf, io.length, io.flags);
> +
> +	if (io.length == 0 || !io.user_buf) {
> +		/*
> +		 * Accept NULL pointers since some buffers are optional
> +		 * in FW commands. In this case we should return 0 as
> +		 * pointer to be embedded into the message.
> +		 * Skip all data copy part of code below.
> +		 */
> +		io.ele_addr = 0;
> +		goto copy;
> +	}
> +
> +	/* Select the shared memory to be used for this buffer. */
> +	if (io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) {
> +		/* App requires to use secure memory for this buffer.*/
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed allocate SEC MEM memory\n",
> +				dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	} else {
> +		/* No specific requirement for this buffer. */
> +		shared_mem = &dev_ctx->non_secure_mem;
> +	}
> +
> +	/* Check there is enough space in the shared memory. */
> +	if (shared_mem->size < shared_mem->pos
> +			|| io.length >= shared_mem->size - shared_mem->pos) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Not enough space in shared memory\n",
> +				dev_ctx->miscdev.name);
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	/* Allocate space in shared memory. 8 bytes aligned. */
> +	pos = shared_mem->pos;
> +	shared_mem->pos += round_up(io.length, 8u);
> +	io.ele_addr = (u64)shared_mem->dma_addr + pos;
> +
> +	if ((io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) &&
> +	    !(io.flags & SE_MU_IO_FLAGS_USE_SHORT_ADDR)) {
> +		/*Add base address to get full address.*/
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed allocate SEC MEM memory\n",
> +				dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +	memset(shared_mem->ptr + pos, 0, io.length);
> +	if ((io.flags & SE_IO_BUF_FLAGS_IS_INPUT) ||
> +	    (io.flags & SE_IO_BUF_FLAGS_IS_IN_OUT)) {
> +		/*
> +		 * buffer is input:
> +		 * copy data from user space to this allocated buffer.
> +		 */
> +		if (copy_from_user(shared_mem->ptr + pos, io.user_buf,
> +				   io.length)) {
> +			dev_err(dev_ctx->priv->dev,
> +				"%s: Failed copy data to shared memory\n",
> +				dev_ctx->miscdev.name);
> +			err = -EFAULT;
> +			goto exit;
> +		}
> +	}
> +
> +	b_desc = kzalloc(sizeof(*b_desc), GFP_KERNEL);
> +	if (!b_desc) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +copy:
> +	/* Provide the EdgeLock Enclave address to user space only if success.*/
> +	if (copy_to_user((u8 *)arg, &io, sizeof(io))) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed to copy iobuff setup to user\n",
> +				dev_ctx->miscdev.name);
> +		kfree(b_desc);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +	if (b_desc) {
> +		b_desc->shared_buf_ptr = shared_mem->ptr + pos;
> +		b_desc->usr_buf_ptr = io.user_buf;
> +		b_desc->size = io.length;
> +
> +		if (io.flags & SE_IO_BUF_FLAGS_IS_INPUT) {
> +			/*
> +			 * buffer is input:
> +			 * add an entry in the "pending input buffers" list so
> +			 * that copied data can be cleaned from shared memory
> +			 * later.
> +			 */
> +			list_add_tail(&b_desc->link, &dev_ctx->pending_in);
> +		} else {
> +			/*
> +			 * buffer is output:
> +			 * add an entry in the "pending out buffers" list so data
> +			 * can be copied to user space when receiving Secure-Enclave
> +			 * response.
> +			 */
> +			list_add_tail(&b_desc->link, &dev_ctx->pending_out);
> +		}
> +	}
> +
> +exit:
> +	return err;
> +}
> +
> +/* IOCTL to provide SoC information */
> +static int se_ioctl_get_se_soc_info_handler(struct se_if_device_ctx *dev_ctx,
> +					     u64 arg)
> +{
> +	struct imx_se_node_info_list *info_list;
> +	struct se_ioctl_get_soc_info soc_info;
> +	int err = -EINVAL;
> +
> +	info_list = (struct imx_se_node_info_list *)
> +			device_get_match_data(dev_ctx->priv->dev);
> +	if (!info_list)
> +		goto exit;
> +
> +	soc_info.soc_id = info_list->soc_id;
> +	soc_info.soc_rev = dev_ctx->priv->soc_rev;
> +
> +	err = (int)copy_to_user((u8 *)arg, (u8 *)(&soc_info), sizeof(soc_info));
> +	if (err) {
> +		dev_err(dev_ctx->priv->dev,
> +			"%s: Failed to copy soc info to user\n",
> +			dev_ctx->miscdev.name);
> +		err = -EFAULT;
> +		goto exit;
> +	}
> +
> +exit:
> +	return err;
> +}
> +
> +/* Open a character device. */
> +static int se_if_fops_open(struct inode *nd, struct file *fp)
> +{
> +	struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> +							struct se_if_device_ctx,
> +							miscdev);
> +	int err = 0;
> +
> +	/* Avoid race if opened at the same time */
> +	if (down_trylock(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	/* Authorize only 1 instance. */
> +	if (dev_ctx->status != SE_IF_CTX_FREE) {
> +		err = -EBUSY;
> +		goto exit;
> +	}
> +
> +	/*
> +	 * Allocate some memory for data exchanges with S40x.
> +	 * This will be used for data not requiring secure memory.
> +	 */
> +	dev_ctx->non_secure_mem.ptr = dmam_alloc_coherent(dev_ctx->dev,
> +					MAX_DATA_SIZE_PER_USER,
> +					&dev_ctx->non_secure_mem.dma_addr,
> +					GFP_KERNEL);
> +	if (!dev_ctx->non_secure_mem.ptr) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	dev_ctx->non_secure_mem.size = MAX_DATA_SIZE_PER_USER;
> +	dev_ctx->non_secure_mem.pos = 0;
> +	dev_ctx->status = SE_IF_CTX_OPENED;
> +
> +	dev_ctx->pending_hdr = 0;
> +
> +	goto exit;
> +
> +	dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
> +			   dev_ctx->non_secure_mem.ptr,
> +			   dev_ctx->non_secure_mem.dma_addr);
> +
> +exit:
> +	up(&dev_ctx->fops_lock);
> +	return err;
> +}
> +
> +/* Close a character device. */
> +static int se_if_fops_close(struct inode *nd, struct file *fp)
> +{
> +	struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> +							struct se_if_device_ctx,
> +							miscdev);
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	struct se_buf_desc *b_desc;
> +
> +	/* Avoid race if closed at the same time */
> +	if (down_trylock(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	/* The device context has not been opened */
> +	if (dev_ctx->status != SE_IF_CTX_OPENED)
> +		goto exit;
> +
> +	/* check if this device was registered as command receiver. */
> +	if (priv->cmd_receiver_dev == dev_ctx)
> +		priv->cmd_receiver_dev = NULL;
> +
> +	/* check if this device was registered as waiting response. */
> +	if (priv->waiting_rsp_dev == dev_ctx) {
> +		priv->waiting_rsp_dev = NULL;
> +		mutex_unlock(&priv->se_if_cmd_lock);
> +	}
> +
> +	/* Unmap secure memory shared buffer. */
> +	if (dev_ctx->secure_mem.ptr)
> +		devm_iounmap(dev_ctx->dev, dev_ctx->secure_mem.ptr);
> +
> +	dev_ctx->secure_mem.ptr = NULL;
> +	dev_ctx->secure_mem.dma_addr = 0;
> +	dev_ctx->secure_mem.size = 0;
> +	dev_ctx->secure_mem.pos = 0;
> +
> +	/* Free non-secure shared buffer. */
> +	dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
> +			   dev_ctx->non_secure_mem.ptr,
> +			   dev_ctx->non_secure_mem.dma_addr);
> +
> +	dev_ctx->non_secure_mem.ptr = NULL;
> +	dev_ctx->non_secure_mem.dma_addr = 0;
> +	dev_ctx->non_secure_mem.size = 0;
> +	dev_ctx->non_secure_mem.pos = 0;
> +
> +	while (!list_empty(&dev_ctx->pending_in) ||
> +	       !list_empty(&dev_ctx->pending_out)) {
> +		if (!list_empty(&dev_ctx->pending_in))
> +			b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
> +							  struct se_buf_desc,
> +							  link);
> +		else
> +			b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> +							  struct se_buf_desc,
> +							  link);
> +
> +		if (!b_desc)
> +			continue;
> +
> +		if (b_desc->shared_buf_ptr)
> +			memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> +
> +		__list_del_entry(&b_desc->link);
> +		kfree(b_desc);
> +	}
> +
> +	dev_ctx->status = SE_IF_CTX_FREE;
> +
> +exit:
> +	up(&dev_ctx->fops_lock);
> +	return 0;
> +}
> +
> +/* IOCTL entry point of a character device */
> +static long se_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +{
> +	struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> +							struct se_if_device_ctx,
> +							miscdev);
> +	struct se_if_priv *se_if_priv = dev_ctx->priv;
> +	int err = -EINVAL;
> +
> +	/* Prevent race during change of device context */
> +	if (down_interruptible(&dev_ctx->fops_lock))
> +		return -EBUSY;
> +
> +	switch (cmd) {
> +	case SE_IOCTL_ENABLE_CMD_RCV:
> +		if (!se_if_priv->cmd_receiver_dev) {
> +			se_if_priv->cmd_receiver_dev = dev_ctx;
> +			err = 0;
> +		}
> +		break;
> +	case SE_IOCTL_GET_MU_INFO:
> +		err = se_ioctl_get_mu_info(dev_ctx, arg);
> +		break;
> +	case SE_IOCTL_SETUP_IOBUF:
> +		err = se_ioctl_setup_iobuf_handler(dev_ctx, arg);
> +		break;
> +	case SE_IOCTL_GET_SOC_INFO:
> +		err = se_ioctl_get_se_soc_info_handler(dev_ctx, arg);
> +		break;
> +
> +	default:
> +		err = -EINVAL;
> +		dev_dbg(se_if_priv->dev,
> +			"%s: IOCTL %.8x not supported\n",
> +				dev_ctx->miscdev.name,
> +				cmd);
> +	}
> +
> +	up(&dev_ctx->fops_lock);
> +	return (long)err;
> +}
> +
> +/* Char driver setup */
> +static const struct file_operations se_if_fops = {
> +	.open		= se_if_fops_open,
> +	.owner		= THIS_MODULE,
> +	.release	= se_if_fops_close,
> +	.unlocked_ioctl = se_ioctl,
> +	.read		= se_if_fops_read,
> +	.write		= se_if_fops_write,
> +};
> +
> +/* interface for managed res to unregister a character device */
> +static void if_misc_deregister(void *miscdevice)
> +{
> +	misc_deregister(miscdevice);
> +}
> +
>   /* interface for managed res to free a mailbox channel */
>   static void if_mbox_free_channel(void *mbox_chan)
>   {
> @@ -270,6 +855,7 @@ static int se_probe_if_cleanup(struct platform_device *pdev)
>   	struct device *dev = &pdev->dev;
>   	struct se_if_priv *priv;
>   	int ret = 0;
> +	int i;
>   
>   	priv = dev_get_drvdata(dev);
>   	if (!priv) {
> @@ -294,6 +880,17 @@ static int se_probe_if_cleanup(struct platform_device *pdev)
>   		priv->imem.buf = NULL;
>   	}
>   
> +	if (priv->ctxs) {
> +		for (i = 0; i < priv->max_dev_ctx; i++) {
> +			if (priv->ctxs[i]) {
> +				devm_remove_action(dev,
> +						   if_misc_deregister,
> +						   &priv->ctxs[i]->miscdev);
> +				misc_deregister(&priv->ctxs[i]->miscdev);
> +			}
> +		}
> +	}
> +
>   	if (priv->flags & RESERVED_DMA_POOL) {
>   		of_reserved_mem_device_release(dev);
>   		priv->flags &= (~RESERVED_DMA_POOL);
> @@ -302,6 +899,84 @@ static int se_probe_if_cleanup(struct platform_device *pdev)
>   	return ret;
>   }
>   
> +static int init_device_context(struct device *dev)
> +{
> +	const struct imx_se_node_info *info;
> +	struct se_if_device_ctx *dev_ctx;
> +	struct se_if_priv *priv;
> +	u8 *devname;
> +	int ret = 0;
> +	int i;
> +
> +	priv = dev_get_drvdata(dev);
> +
> +	if (!priv) {
> +		ret = -EINVAL;
> +		dev_err(dev, "Invalid SE-MU Priv data");
> +		return ret;
> +	}
> +	info = priv->info;
> +
> +	priv->ctxs = devm_kzalloc(dev, sizeof(dev_ctx) * priv->max_dev_ctx,
> +				  GFP_KERNEL);
> +
> +	if (!priv->ctxs) {
> +		ret = -ENOMEM;
> +		return ret;
> +	}
> +
> +	/* Create users */
> +	for (i = 0; i < priv->max_dev_ctx; i++) {
> +		dev_ctx = devm_kzalloc(dev, sizeof(*dev_ctx), GFP_KERNEL);
> +		if (!dev_ctx) {
> +			ret = -ENOMEM;
> +			return ret;
> +		}
> +
> +		dev_ctx->dev = dev;
> +		dev_ctx->status = SE_IF_CTX_FREE;
> +		dev_ctx->priv = priv;
> +
> +		priv->ctxs[i] = dev_ctx;
> +
> +		/* Default value invalid for an header. */
> +		init_waitqueue_head(&dev_ctx->wq);
> +
> +		INIT_LIST_HEAD(&dev_ctx->pending_out);
> +		INIT_LIST_HEAD(&dev_ctx->pending_in);
> +		sema_init(&dev_ctx->fops_lock, 1);
> +
> +		devname = devm_kasprintf(dev, GFP_KERNEL, "%s_ch%d",
> +					 info->se_name, i);
> +		if (!devname) {
> +			ret = -ENOMEM;
> +			return ret;
> +		}
> +
> +		dev_ctx->miscdev.name = devname;
> +		dev_ctx->miscdev.minor = MISC_DYNAMIC_MINOR;
> +		dev_ctx->miscdev.fops = &se_if_fops;
> +		dev_ctx->miscdev.parent = dev;
> +		ret = misc_register(&dev_ctx->miscdev);
> +		if (ret) {
> +			dev_err(dev, "failed to register misc device %d\n",
> +				ret);
> +			return ret;
> +		}
> +
> +		ret = devm_add_action(dev, if_misc_deregister,
> +				      &dev_ctx->miscdev);
> +		if (ret) {
> +			dev_err(dev,
> +				"failed[%d] to add action to the misc-dev\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>   static void se_load_firmware(const struct firmware *fw, void *context)
>   {
>   	struct se_if_priv *priv = (struct se_if_priv *) context;
> @@ -461,6 +1136,16 @@ static int se_if_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> +	if (info->max_dev_ctx) {
> +		ret = init_device_context(dev);
> +		if (ret) {
> +			dev_err(dev,
> +				"Failed[0x%x] to create device contexts.\n",
> +				ret);
> +			goto exit;
> +		}
> +	}
> +
>   	dev_info(dev, "i.MX secure-enclave: %s interface to firmware, configured.\n",
>   		 info->se_name);
>   	return devm_of_platform_populate(dev);
> @@ -502,6 +1187,10 @@ static int se_resume(struct device *dev)
>   	struct se_if_priv *priv = dev_get_drvdata(dev);
>   	const struct imx_se_node_info *info
>   					= priv->info;
> +	int i;
> +
> +	for (i = 0; i < priv->max_dev_ctx; i++)
> +		wake_up_interruptible(&priv->ctxs[i]->wq);
>   
>   	if (info && info->imem_mgmt)
>   		se_restore_imem_state(dev);
> diff --git a/drivers/firmware/imx/se_ctrl.h b/drivers/firmware/imx/se_ctrl.h
> index 7d4f439a6158..41d9cedb05d7 100644
> --- a/drivers/firmware/imx/se_ctrl.h
> +++ b/drivers/firmware/imx/se_ctrl.h
> @@ -13,15 +13,61 @@
>   #define MAX_FW_LOAD_RETRIES		50
>   
>   #define RES_STATUS(x)			FIELD_GET(0x000000ff, x)
> +#define MAX_DATA_SIZE_PER_USER		(65 * 1024)
>   #define MESSAGING_VERSION_6		0x6
>   #define MESSAGING_VERSION_7		0x7
>   
> +#define SE_MU_IO_FLAGS_USE_SEC_MEM	(0x02u)
> +#define SE_MU_IO_FLAGS_USE_SHORT_ADDR	(0x04u)
> +
>   struct se_imem_buf {
>   	u8 *buf;
>   	phys_addr_t phyaddr;
>   	u32 size;
>   };
>   
> +struct se_buf_desc {
> +	u8 *shared_buf_ptr;
> +	u8 *usr_buf_ptr;
> +	u32 size;
> +	struct list_head link;
> +};
> +
> +/* Status of a char device */
> +enum se_if_dev_ctx_status_t {
> +	SE_IF_CTX_FREE,
> +	SE_IF_CTX_OPENED
> +};
> +
> +struct se_shared_mem {
> +	dma_addr_t dma_addr;
> +	u32 size;
> +	u32 pos;
> +	u8 *ptr;
> +};
> +
> +/* Private struct for each char device instance. */
> +struct se_if_device_ctx {
> +	struct device *dev;
> +	struct se_if_priv *priv;
> +	struct miscdevice miscdev;
> +
> +	enum se_if_dev_ctx_status_t status;
> +	wait_queue_head_t wq;
> +	struct semaphore fops_lock;
> +
> +	u32 pending_hdr;
> +	struct list_head pending_in;
> +	struct list_head pending_out;
> +
> +	struct se_shared_mem secure_mem;
> +	struct se_shared_mem non_secure_mem;
> +
> +	u32 *temp_resp;
> +	u32 temp_resp_size;
> +	struct notifier_block se_notify;
> +};
> +
>   /* Header of the messages exchange with the EdgeLock Enclave */
>   struct se_msg_hdr {
>   	u8 ver;
> diff --git a/include/uapi/linux/se_ioctl.h b/include/uapi/linux/se_ioctl.h
> new file mode 100644
> index 000000000000..f68a36e9da2c
> --- /dev/null
> +++ b/include/uapi/linux/se_ioctl.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: (GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause*/
> +/*
> + * Copyright 2024 NXP
> + */
> +
> +#ifndef SE_IOCTL_H
> +#define SE_IOCTL_H
> +
> +/* IOCTL definitions. */
> +
> +struct se_ioctl_setup_iobuf {
> +	u8 *user_buf;
> +	u32 length;
> +	u32 flags;
> +	u64 ele_addr;
> +};
> +
> +struct se_ioctl_shared_mem_cfg {
> +	u32 base_offset;
> +	u32 size;
> +};
> +
> +struct se_ioctl_get_if_info {
> +	u8 se_if_id;
> +	u8 interrupt_idx;
> +	u8 tz;
> +	u8 did;
> +	u8 cmd_tag;
> +	u8 rsp_tag;
> +	u8 success_tag;
> +	u8 base_api_ver;
> +	u8 fw_api_ver;
> +};
> +
> +struct se_ioctl_signed_message {
> +	u8 *message;
> +	u32 msg_size;
> +	u32 error_code;
> +};
> +
> +struct se_ioctl_get_soc_info {
> +	u16 soc_id;
> +	u16 soc_rev;
> +};
> +
> +/* IO Buffer Flags */
> +#define SE_IO_BUF_FLAGS_IS_OUTPUT	(0x00u)
> +#define SE_IO_BUF_FLAGS_IS_INPUT	(0x01u)
> +#define SE_IO_BUF_FLAGS_USE_SEC_MEM	(0x02u)
> +#define SE_IO_BUF_FLAGS_USE_SHORT_ADDR	(0x04u)
> +#define SE_IO_BUF_FLAGS_IS_IN_OUT	(0x10u)
> +
> +/* IOCTLS */
> +#define SE_IOCTL			0x0A /* like MISC_MAJOR. */
> +
> +/*
> + * ioctl to designated the current fd as logical-reciever.
> + * This is ioctl is send when the nvm-daemon, a slave to the
> + * firmware is started by the user.
> + */
> +#define SE_IOCTL_ENABLE_CMD_RCV	_IO(SE_IOCTL, 0x01)
> +
> +/*
> + * ioctl to get the buffer allocated from the memory, which is shared
> + * between kernel and FW.
> + * Post allocation, the kernel tagged the allocated memory with:
> + *  Output
> + *  Input
> + *  Input-Output
> + *  Short address
> + *  Secure-memory
> + */
> +#define SE_IOCTL_SETUP_IOBUF	_IOWR(SE_IOCTL, 0x03, \
> +					struct se_ioctl_setup_iobuf)
> +
> +/*
> + * ioctl to get the mu information, that is used to exchange message
> + * with FW, from user-spaced.
> + */
> +#define SE_IOCTL_GET_MU_INFO	_IOR(SE_IOCTL, 0x04, \
> +					struct se_ioctl_get_if_info)
> +/*
> + * ioctl to get SoC Info from user-space.
> + */
> +#define SE_IOCTL_GET_SOC_INFO      _IOR(SE_IOCTL, 0x06, \
> +					struct se_ioctl_get_soc_info)
> +
> +#endif
>

Pankaj Gupta June 5, 2024, 9:15 a.m. UTC | #6

> -----Original Message-----
> From: Amit Singh Tomar <[e-mail]>
> Sent: Monday, June 3, 2024 9:53 PM
> To: Pankaj Gupta <[e-mail]>; Jonathan Corbet
> <[e-mail]>; Rob Herring <[e-mail]>; Krzysztof Kozlowski
> <[e-mail]>; Conor Dooley <[e-mail]>;
> Shawn Guo <[e-mail]>; Sascha Hauer
> <[e-mail]>; Pengutronix Kernel Team
> <[e-mail]>; Fabio Estevam <[e-mail]>; Rob
> Herring <[e-mail]>; Krzysztof Kozlowski <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail]; linux-arm-
> [e-mail]
> Subject: [EXT] [PATCH v2 5/5] firmware: imx: adds miscdev
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hi,
> 
> >
> > ----------------------------------------------------------------------
> > Adds the driver for communication interface to secure-enclave, for
> > exchanging messages with NXP secure enclave HW IP(s) like EdgeLock
> > Enclave from:
> > - User-Space Applications via character driver.
> >
> > ABI documentation for the NXP secure-enclave driver.
> >
> > User-space library using this driver:
> > - i.MX Secure Enclave library:
> >    -- URL:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furld
> > efense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-3A__github.com_nxp-
> 2Dimx_
> > imx-2Dsecure-
> 2Denclave.git%26d%3DDwICAg%26c%3DnKjWec2b6R0mOyPaz7xtfQ%2
> > 6r%3DV_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-
> JKU%26m%3D8xuiz3OEyshbkzL
> > Q6-
> G2afLkh7sVv9b_HkFaPJVBu4wjmgAowHGi90q5RTroZ2oy%26s%3Dxi3fO_c9z_t
> -zd
> > k3LdTGgqJ6M-5OjRD6oj-
> ECiVQ40Q%26e%3D&data=05%7C02%7Cpankaj.gupta%40nxp
> > .com%7Cf63cdafaf0fe4a191a3108dc83e97a8a%7C686ea1d3bc2b4c6fa92cd
> 99c5c30
> >
> 1635%7C0%7C0%7C638530286027736635%7CUnknown%7CTWFpbGZsb3d8e
> yJWIjoiMC4w
> >
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C
> %7C&sd
> >
> ata=KHyEla5aGRaS8tMOBYAbsYWxUPO%2BxBploTHqvGwMSuA%3D&reserve
> d=0 ,
> > - i.MX Secure Middle-Ware:
> >    -- URL:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furld
> > efense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-3A__github.com_nxp-
> 2Dimx_
> > imx-
> 2Dsmw.git%26d%3DDwICAg%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r%3DV_
> GK7jRu
> > CHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU%26m%3D8xuiz3OEyshbkzLQ6-
> G2afLkh7sV
> >
> v9b_HkFaPJVBu4wjmgAowHGi90q5RTroZ2oy%26s%3DjPOlKXqt_GIZGMvMbo
> Odjkwu3UT
> >
> pZ7fwEFm8Ki5z0LE%26e%3D&data=05%7C02%7Cpankaj.gupta%40nxp.com%
> 7Cf63cda
> >
> faf0fe4a191a3108dc83e97a8a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%
> >
> 7C638530286027747831%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQI
> >
> joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=Ew
> UWiVl5q
> > fKjdEgOm6J0cvg%2FmeLKo%2FQxetXPWJlNUDY%3D&reserved=0
> >
> > Signed-off-by: Pankaj Gupta <[e-mail]>
> > ---
> >   Documentation/ABI/testing/se-cdev |  42 +++
> >   drivers/firmware/imx/ele_common.c | 108 +++++-
> >   drivers/firmware/imx/ele_common.h |   3 +
> >   drivers/firmware/imx/se_ctrl.c    | 689
> ++++++++++++++++++++++++++++++++++++++
> >   drivers/firmware/imx/se_ctrl.h    |  46 +++
> >   include/uapi/linux/se_ioctl.h     |  88 +++++
> >   6 files changed, 974 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/se-cdev
> > b/Documentation/ABI/testing/se-cdev
> > new file mode 100644
> > index 000000000000..699525af6b86
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/se-cdev
> > @@ -0,0 +1,42 @@
> > +What:                /dev/<se>_mu[0-9]+_ch[0-9]+
> > +Date:                May 2024
> > +KernelVersion:       6.8
> > +Contact:     [e-mail], [e-mail]
> > +Description:
> > +             NXP offers multiple hardware IP(s) for  secure-enclaves like
> EdgeLock-
> > +             Enclave(ELE), SECO. The character device file-descriptors
> > +             /dev/<se>_mu*_ch* are the interface between user-space NXP's
> secure-
> > +             enclave shared-library and the kernel driver.
> > +
> > +             The ioctl(2)-based ABI is defined and documented in
> > +             [include]<linux/firmware/imx/ele_mu_ioctl.h>
> > +              ioctl(s) are used primarily for:
> > +                     - shared memory management
> > +                     - allocation of I/O buffers
> > +                     - get mu info
> > +                     - setting a dev-ctx as receiver that is slave to fw
> > +                     - get SoC info
> > +
> > +             The following file operations are supported:
> > +
> > +             open(2)
> > +               Currently the only useful flags are O_RDWR.
> > +
> > +             read(2)
> > +               Every read() from the opened character device context is waiting
> on
> > +               wakeup_intruptible, that gets set by the registered mailbox
> callback
> > +               function; indicating a message received from the firmware on
> message-
> > +               unit.
> > +
> > +             write(2)
> > +               Every write() to the opened character device context needs to
> acquire
> > +               mailbox_lock, before sending message on to the message unit.
> > +
> > +             close(2)
> > +               Stops and free up the I/O contexts that was associated
> > +               with the file descriptor.
> > +
> > +Users:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furldef
> ense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-3A__github.com_nxp-
> 2Dimx_imx-2Dsecure-
> 2Denclave.git%26d%3DDwICAg%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r%3D
> V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-
> JKU%26m%3D8xuiz3OEyshbkzLQ6-
> G2afLkh7sVv9b_HkFaPJVBu4wjmgAowHGi90q5RTroZ2oy%26s%3Dxi3fO_c9z_t
> -zdk3LdTGgqJ6M-5OjRD6oj-
> ECiVQ40Q%26e%3D&data=05%7C02%7Cpankaj.gupta%40nxp.com%7Cf63cda
> faf0fe4a191a3108dc83e97a8a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C638530286027754085%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%
> 7C%7C&sdata=S20ZeHvRTa8ByxkD7znd5qbLitdcyuQOOmtdnak%2F9VQ%3D&
> reserved=0 ,
> > +
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furldef
> ense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-3A__github.com_nxp-
> 2Dimx_imx-
> 2Dsmw.git%26d%3DDwICAg%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r%3DV_
> GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-
> JKU%26m%3D8xuiz3OEyshbkzLQ6-
> G2afLkh7sVv9b_HkFaPJVBu4wjmgAowHGi90q5RTroZ2oy%26s%3DjPOlKXqt_GI
> ZGMvMboOdjkwu3UTpZ7fwEFm8Ki5z0LE%26e%3D&data=05%7C02%7Cpank
> aj.gupta%40nxp.com%7Cf63cdafaf0fe4a191a3108dc83e97a8a%7C686ea1d3b
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C638530286027758865%7CUnknown
> %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=qusMAsOp5fGyER76rQNwsaFq
> WgOPhY755l42krUF%2Bok%3D&reserved=0
> > +             crypto/skcipher,
> > +             drivers/nvmem/imx-ocotp-ele.c
> > diff --git a/drivers/firmware/imx/ele_common.c
> > b/drivers/firmware/imx/ele_common.c
> > index c286c3d84d82..15fabc369b21 100644
> > --- a/drivers/firmware/imx/ele_common.c
> > +++ b/drivers/firmware/imx/ele_common.c
> > @@ -78,12 +78,98 @@ int imx_ele_msg_send_rcv(struct se_if_priv *priv,
> void *tx_msg, void *rx_msg)
> >       return err;
> >   }
> >
> > +int imx_ele_miscdev_msg_rcv(struct se_if_device_ctx *dev_ctx) {
> > +     struct se_msg_hdr *header = {0};
> > +     int err;
> > +
> > +     if (dev_ctx->priv->waiting_rsp_dev == dev_ctx)
> > +             lockdep_assert_held(&dev_ctx->priv->se_if_cmd_lock);
> > +
> > +     err = wait_event_interruptible(dev_ctx->wq, dev_ctx->pending_hdr !=
> 0);
> > +     if (err)
> > +             dev_err(dev_ctx->dev,
> > +                     "%s: Err[0x%x]:Interrupted by signal.\n",
> > +                     dev_ctx->miscdev.name, err);
> > +
> > +     header = (struct se_msg_hdr *) dev_ctx->temp_resp;
> > +
> > +     if (header->tag == dev_ctx->priv->rsp_tag) {
> > +             if (dev_ctx->priv->waiting_rsp_dev != dev_ctx)
> > +                     dev_warn(dev_ctx->dev,
> > +                     "%s: Device context waiting for response mismatch.\n",
> > +                     dev_ctx->miscdev.name);
> > +             else
> > +                     dev_ctx->priv->waiting_rsp_dev = NULL;
> > +
> > +             mutex_unlock(&dev_ctx->priv->se_if_cmd_lock);
> > +     }
> > +
> > +     return err;
> > +}
> > +
> > +int imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
> > +                          void *tx_msg, int tx_msg_sz) {
> > +     struct se_if_priv *priv = dev_ctx->priv;
> > +     struct se_msg_hdr *header;
> > +     int err;
> > +
> > +     header = (struct se_msg_hdr *) tx_msg;
> > +
> > +     /*
> > +      * Check that the size passed as argument matches the size
> > +      * carried in the message.
> > +      */
> > +     err = header->size << 2;
> > +
> > +     if (err != tx_msg_sz) {
> > +             err = -EINVAL;
> > +             dev_err(priv->dev,
> > +                     "%s: User buffer too small\n",
> > +                             dev_ctx->miscdev.name);
> > +             goto exit;
> > +     }
> > +     /* Check the message is valid according to tags */
> > +     if (header->tag == priv->cmd_tag) {
> > +             mutex_lock(&priv->se_if_cmd_lock);
> 
> In the previous patch (4/5), you used 'guard' locks. Wouldn't it be better to
> use them here as well, considering the lock handling seems a bit dodgy (I
> mean, the way lock is released only under certain condition)?

There is a comment from Pengutronix.
I have created an IOCTL for send/receive, where I am locking and unlocking the command lock.

> 
> > +             priv->waiting_rsp_dev = dev_ctx;
> > +     } else if (header->tag == priv->rsp_tag) {
> > +             /* Check the device context can send the command */
> > +             if (dev_ctx != priv->cmd_receiver_dev) {
> > +                     dev_err(priv->dev,
> > +                             "%s: Channel not configured to send resp to FW.",
> > +                             dev_ctx->miscdev.name);
> > +                     err = -EPERM;
> > +                     goto exit;
> > +             }
> > +     } else {
> > +             dev_err(priv->dev,
> > +                     "%s: The message does not have a valid TAG\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EINVAL;
> > +             goto exit;
> > +     }
> > +     err = imx_ele_msg_send(priv, tx_msg);
> > +     if (err < 0) {
> > +             if (header->tag == priv->cmd_tag) {
> > +                     priv->waiting_rsp_dev = NULL;
> > +                     mutex_unlock(&dev_ctx->priv->se_if_cmd_lock);
> > +             }
> > +     } else
> > +             err = header->size << 2;
> > +exit:
> > +     return err;
> > +}
> > +
> >   /*
> >    * Callback called by mailbox FW, when data is received.
> >    */
> >   void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
> >   {
> >       struct device *dev = mbox_cl->dev;
> > +     struct se_if_device_ctx *dev_ctx;
> > +     struct se_api_msg *rx_msg;
> >       struct se_if_priv *priv;
> >       struct se_msg_hdr *header;
> >
> > @@ -97,8 +183,15 @@ void se_if_rx_callback(struct mbox_client
> > *mbox_cl, void *msg)
> >
> >       header = (struct se_msg_hdr *) msg;
> >
> > -     if (header->tag == priv->rsp_tag) {
> > -             if (!priv->waiting_rsp_dev) {
> > +     /* Incoming command: wake up the receiver if any. */
> > +     if (header->tag == priv->cmd_tag) {
> > +             dev_dbg(dev, "Selecting cmd receiver\n");
> > +             dev_ctx = priv->cmd_receiver_dev;
> > +     } else if (header->tag == priv->rsp_tag) {
> > +             if (priv->waiting_rsp_dev) {
> > +                     dev_dbg(dev, "Selecting rsp waiter\n");
> > +                     dev_ctx = priv->waiting_rsp_dev;
> > +             } else {
> >                       /*
> >                        * Reading the EdgeLock Enclave response
> >                        * to the command, sent by other @@ -116,6
> > +209,17 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void
> *msg)
> >                               *((u32 *) header));
> >               return;
> >       }
> > +     /* Init reception */
> > +     rx_msg = kzalloc(header->size << 2, GFP_KERNEL);
> > +     if (rx_msg)
> > +             memcpy(rx_msg, msg, header->size << 2);
> > +
> > +     dev_ctx->temp_resp = (u32 *)rx_msg;
> > +     dev_ctx->temp_resp_size = header->size;
> > +
> > +     /* Allow user to read */
> > +     dev_ctx->pending_hdr = 1;
> > +     wake_up_interruptible(&dev_ctx->wq);
> >   }
> >
> >   int validate_rsp_hdr(struct se_if_priv *priv, diff --git
> > a/drivers/firmware/imx/ele_common.h
> > b/drivers/firmware/imx/ele_common.h
> > index 76777ac629d6..11b9b36d4fda 100644
> > --- a/drivers/firmware/imx/ele_common.h
> > +++ b/drivers/firmware/imx/ele_common.h
> > @@ -12,6 +12,9 @@
> >   #define IMX_ELE_FW_DIR                 "imx/ele/"
> >
> >   uint32_t plat_add_msg_crc(uint32_t *msg, uint32_t msg_len);
> > +int imx_ele_miscdev_msg_rcv(struct se_if_device_ctx *priv); int
> > +imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
> > +                          void *tx_msg, int tx_msg_sz);
> >   int imx_ele_msg_rcv(struct se_if_priv *priv);
> >   int imx_ele_msg_send(struct se_if_priv *priv, void *tx_msg);
> >   int imx_ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, void
> > *rx_msg); diff --git a/drivers/firmware/imx/se_ctrl.c
> > b/drivers/firmware/imx/se_ctrl.c index 0642d349b3d3..3acaecd8f3bc
> > 100644
> > --- a/drivers/firmware/imx/se_ctrl.c
> > +++ b/drivers/firmware/imx/se_ctrl.c
> > @@ -23,6 +23,7 @@
> >   #include <linux/slab.h>
> >   #include <linux/string.h>
> >   #include <linux/sys_soc.h>
> > +#include <uapi/linux/se_ioctl.h>
> >
> >   #include "ele_base_msg.h"
> >   #include "ele_common.h"
> > @@ -232,6 +233,590 @@ static int imx_fetch_se_soc_info(struct device
> *dev)
> >       return 0;
> >   }
> >
> > +/*
> > + * File operations for user-space
> > + */
> > +
> > +/* Write a message to the MU. */
> > +static ssize_t se_if_fops_write(struct file *fp, const char __user *buf,
> > +                             size_t size, loff_t *ppos) {
> > +     struct se_api_msg *tx_msg __free(kfree);
> > +     struct se_if_device_ctx *dev_ctx;
> > +     struct se_if_priv *priv;
> > +     int err;
> > +
> > +     dev_ctx = container_of(fp->private_data,
> > +                            struct se_if_device_ctx,
> > +                            miscdev);
> > +     priv = dev_ctx->priv;
> > +     dev_dbg(priv->dev,
> > +             "%s: write from buf (%p)%zu, ppos=%lld\n",
> > +                     dev_ctx->miscdev.name,
> > +                     buf, size, ((ppos) ? *ppos : 0));
> > +
> > +     if (down_interruptible(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     if (dev_ctx->status != SE_IF_CTX_OPENED) {
> > +             err = -EINVAL;
> > +             goto exit;
> > +     }
> > +
> > +     if (size < SE_MU_HDR_SZ) {
> > +             dev_err(priv->dev,
> > +                     "%s: User buffer too small(%zu < %d)\n",
> > +                             dev_ctx->miscdev.name,
> > +                             size, SE_MU_HDR_SZ);
> > +             err = -ENOSPC;
> > +             goto exit;
> > +     }
> > +     tx_msg = memdup_user(buf, size);
> > +     if (!tx_msg) {
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
> > +
> > +     print_hex_dump_debug("from user ", DUMP_PREFIX_OFFSET, 4, 4,
> > +                          tx_msg, size, false);
> > +
> > +     err = imx_ele_miscdev_msg_send(dev_ctx, tx_msg, size);
> > +
> > +exit:
> > +     up(&dev_ctx->fops_lock);
> > +     return err;
> > +}
> > +
> > +/*
> > + * Read a message from the MU.
> > + * Blocking until a message is available.
> > + */
> > +static ssize_t se_if_fops_read(struct file *fp, char __user *buf,
> > +                            size_t size, loff_t *ppos) {
> > +     struct se_if_device_ctx *dev_ctx;
> > +     struct se_buf_desc *b_desc;
> > +     struct se_if_priv *priv;
> > +     u32 size_to_copy;
> > +     int err;
> > +
> > +     dev_ctx = container_of(fp->private_data,
> > +                            struct se_if_device_ctx,
> > +                            miscdev);
> > +     priv = dev_ctx->priv;
> > +     dev_dbg(priv->dev,
> > +             "%s: read to buf %p(%zu), ppos=%lld\n",
> > +                     dev_ctx->miscdev.name,
> > +                     buf, size, ((ppos) ? *ppos : 0));
> > +
> > +     if (down_interruptible(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     if (dev_ctx->status != SE_IF_CTX_OPENED) {
> > +             err = -EINVAL;
> > +             goto exit;
> > +     }
> > +
> > +     err = imx_ele_miscdev_msg_rcv(dev_ctx);
> > +     if (err)
> > +             goto exit;
> > +
> > +     /* Buffer containing the message from FW, is
> > +      * allocated in callback function.
> > +      * Check if buffer allocation failed.
> > +      */
> > +     if (!dev_ctx->temp_resp) {
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
> > +
> > +     dev_dbg(priv->dev,
> > +                     "%s: %s %s\n",
> > +                     dev_ctx->miscdev.name,
> > +                     __func__,
> > +                     "message received, start transmit to user");
> > +
> > +     /*
> > +      * Check that the size passed as argument is larger than
> > +      * the one carried in the message.
> > +      */
> > +     size_to_copy = dev_ctx->temp_resp_size << 2;
> > +     if (size_to_copy > size) {
> > +             dev_dbg(priv->dev,
> > +                     "%s: User buffer too small (%zu < %d)\n",
> > +                             dev_ctx->miscdev.name,
> > +                             size, size_to_copy);
> > +             size_to_copy = size;
> > +     }
> > +
> > +     /*
> > +      * We may need to copy the output data to user before
> > +      * delivering the completion message.
> > +      */
> > +     while (!list_empty(&dev_ctx->pending_out)) {
> > +             b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> > +                                               struct se_buf_desc,
> > +                                               link);
> > +             if (!b_desc)
> > +                     continue;
> > +
> > +             if (b_desc->usr_buf_ptr && b_desc->shared_buf_ptr) {
> > +
> > +                     dev_dbg(priv->dev,
> > +                             "%s: Copy output data to user\n",
> > +                             dev_ctx->miscdev.name);
> > +                     if (copy_to_user(b_desc->usr_buf_ptr,
> > +                                      b_desc->shared_buf_ptr,
> > +                                      b_desc->size)) {
> > +                             dev_err(priv->dev,
> > +                                     "%s: Failure copying output data to user.",
> > +                                     dev_ctx->miscdev.name);
> > +                             err = -EFAULT;
> > +                             goto exit;
> > +                     }
> > +             }
> > +
> > +             if (b_desc->shared_buf_ptr)
> > +                     memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> > +
> > +             __list_del_entry(&b_desc->link);
> > +             kfree(b_desc);
> > +     }
> > +
> > +     /* Copy data from the buffer */
> > +     print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
> > +                          dev_ctx->temp_resp, size_to_copy, false);
> > +     if (copy_to_user(buf, dev_ctx->temp_resp, size_to_copy)) {
> > +             dev_err(priv->dev,
> > +                     "%s: Failed to copy to user\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +     err = size_to_copy;
> > +     kfree(dev_ctx->temp_resp);
> > +
> > +     /* free memory allocated on the shared buffers. */
> > +     dev_ctx->secure_mem.pos = 0;
> > +     dev_ctx->non_secure_mem.pos = 0;
> > +
> > +     dev_ctx->pending_hdr = 0;
> > +
> > +exit:
> > +     /*
> > +      * Clean the used Shared Memory space,
> > +      * whether its Input Data copied from user buffers, or
> > +      * Data received from FW.
> > +      */
> > +     while (!list_empty(&dev_ctx->pending_in) ||
> > +            !list_empty(&dev_ctx->pending_out)) {
> > +             if (!list_empty(&dev_ctx->pending_in))
> > +                     b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
> > +                                                       struct se_buf_desc,
> > +                                                       link);
> > +             else
> > +                     b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> > +                                                       struct se_buf_desc,
> > +                                                       link);
> > +
> > +             if (!b_desc)
> > +                     continue;
> > +
> > +             if (b_desc->shared_buf_ptr)
> > +                     memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> > +
> > +             __list_del_entry(&b_desc->link);
> > +             kfree(b_desc);
> > +     }
> > +
> > +     up(&dev_ctx->fops_lock);
> > +     return err;
> > +}
> > +
> > +static int se_ioctl_get_mu_info(struct se_if_device_ctx *dev_ctx,
> > +                             u64 arg) {
> > +     struct se_if_priv *priv = dev_get_drvdata(dev_ctx->dev);
> > +     struct imx_se_node_info *if_node_info;
> > +     struct se_ioctl_get_if_info info;
> > +     int err = 0;
> > +
> > +     if_node_info = (struct imx_se_node_info *)priv->info;
> > +
> > +     info.se_if_id = if_node_info->se_if_id;
> > +     info.interrupt_idx = 0;
> > +     info.tz = 0;
> > +     info.did = if_node_info->se_if_did;
> > +     info.cmd_tag = if_node_info->cmd_tag;
> > +     info.rsp_tag = if_node_info->rsp_tag;
> > +     info.success_tag = if_node_info->success_tag;
> > +     info.base_api_ver = if_node_info->base_api_ver;
> > +     info.fw_api_ver = if_node_info->fw_api_ver;
> > +
> > +     dev_dbg(priv->dev,
> > +             "%s: info [se_if_id: %d, irq_idx: %d, tz: 0x%x, did: 0x%x]\n",
> > +                     dev_ctx->miscdev.name,
> > +                     info.se_if_id, info.interrupt_idx, info.tz,
> > + info.did);
> > +
> > +     if (copy_to_user((u8 *)arg, &info, sizeof(info))) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed to copy mu info to user\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +exit:
> > +     return err;
> > +}
> > +
> > +/*
> > + * Copy a buffer of data to/from the user and return the address to
> > +use in
> > + * messages
> > + */
> > +static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
> > +                                         u64 arg) {
> > +     struct se_shared_mem *shared_mem = NULL;
> > +     struct se_ioctl_setup_iobuf io = {0};
> > +     struct se_buf_desc *b_desc = NULL;
> > +     int err = 0;
> > +     u32 pos;
> > +
> > +     if (copy_from_user(&io, (u8 *)arg, sizeof(io))) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed copy iobuf config from user\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +     dev_dbg(dev_ctx->priv->dev,
> > +                     "%s: io [buf: %p(%d) flag: %x]\n",
> > +                     dev_ctx->miscdev.name,
> > +                     io.user_buf, io.length, io.flags);
> > +
> > +     if (io.length == 0 || !io.user_buf) {
> > +             /*
> > +              * Accept NULL pointers since some buffers are optional
> > +              * in FW commands. In this case we should return 0 as
> > +              * pointer to be embedded into the message.
> > +              * Skip all data copy part of code below.
> > +              */
> > +             io.ele_addr = 0;
> > +             goto copy;
> > +     }
> > +
> > +     /* Select the shared memory to be used for this buffer. */
> > +     if (io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) {
> > +             /* App requires to use secure memory for this buffer.*/
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed allocate SEC MEM memory\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     } else {
> > +             /* No specific requirement for this buffer. */
> > +             shared_mem = &dev_ctx->non_secure_mem;
> > +     }
> > +
> > +     /* Check there is enough space in the shared memory. */
> > +     if (shared_mem->size < shared_mem->pos
> > +                     || io.length >= shared_mem->size - shared_mem->pos) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Not enough space in shared memory\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
> > +
> > +     /* Allocate space in shared memory. 8 bytes aligned. */
> > +     pos = shared_mem->pos;
> > +     shared_mem->pos += round_up(io.length, 8u);
> > +     io.ele_addr = (u64)shared_mem->dma_addr + pos;
> > +
> > +     if ((io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) &&
> > +         !(io.flags & SE_MU_IO_FLAGS_USE_SHORT_ADDR)) {
> > +             /*Add base address to get full address.*/
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed allocate SEC MEM memory\n",
> > +                             dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +     memset(shared_mem->ptr + pos, 0, io.length);
> > +     if ((io.flags & SE_IO_BUF_FLAGS_IS_INPUT) ||
> > +         (io.flags & SE_IO_BUF_FLAGS_IS_IN_OUT)) {
> > +             /*
> > +              * buffer is input:
> > +              * copy data from user space to this allocated buffer.
> > +              */
> > +             if (copy_from_user(shared_mem->ptr + pos, io.user_buf,
> > +                                io.length)) {
> > +                     dev_err(dev_ctx->priv->dev,
> > +                             "%s: Failed copy data to shared memory\n",
> > +                             dev_ctx->miscdev.name);
> > +                     err = -EFAULT;
> > +                     goto exit;
> > +             }
> > +     }
> > +
> > +     b_desc = kzalloc(sizeof(*b_desc), GFP_KERNEL);
> > +     if (!b_desc) {
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
> > +
> > +copy:
> > +     /* Provide the EdgeLock Enclave address to user space only if
> success.*/
> > +     if (copy_to_user((u8 *)arg, &io, sizeof(io))) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed to copy iobuff setup to user\n",
> > +                             dev_ctx->miscdev.name);
> > +             kfree(b_desc);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +     if (b_desc) {
> > +             b_desc->shared_buf_ptr = shared_mem->ptr + pos;
> > +             b_desc->usr_buf_ptr = io.user_buf;
> > +             b_desc->size = io.length;
> > +
> > +             if (io.flags & SE_IO_BUF_FLAGS_IS_INPUT) {
> > +                     /*
> > +                      * buffer is input:
> > +                      * add an entry in the "pending input buffers" list so
> > +                      * that copied data can be cleaned from shared memory
> > +                      * later.
> > +                      */
> > +                     list_add_tail(&b_desc->link, &dev_ctx->pending_in);
> > +             } else {
> > +                     /*
> > +                      * buffer is output:
> > +                      * add an entry in the "pending out buffers" list so data
> > +                      * can be copied to user space when receiving Secure-
> Enclave
> > +                      * response.
> > +                      */
> > +                     list_add_tail(&b_desc->link, &dev_ctx->pending_out);
> > +             }
> > +     }
> > +
> > +exit:
> > +     return err;
> > +}
> > +
> > +/* IOCTL to provide SoC information */ static int
> > +se_ioctl_get_se_soc_info_handler(struct se_if_device_ctx *dev_ctx,
> > +                                          u64 arg) {
> > +     struct imx_se_node_info_list *info_list;
> > +     struct se_ioctl_get_soc_info soc_info;
> > +     int err = -EINVAL;
> > +
> > +     info_list = (struct imx_se_node_info_list *)
> > +                     device_get_match_data(dev_ctx->priv->dev);
> > +     if (!info_list)
> > +             goto exit;
> > +
> > +     soc_info.soc_id = info_list->soc_id;
> > +     soc_info.soc_rev = dev_ctx->priv->soc_rev;
> > +
> > +     err = (int)copy_to_user((u8 *)arg, (u8 *)(&soc_info), sizeof(soc_info));
> > +     if (err) {
> > +             dev_err(dev_ctx->priv->dev,
> > +                     "%s: Failed to copy soc info to user\n",
> > +                     dev_ctx->miscdev.name);
> > +             err = -EFAULT;
> > +             goto exit;
> > +     }
> > +
> > +exit:
> > +     return err;
> > +}
> > +
> > +/* Open a character device. */
> > +static int se_if_fops_open(struct inode *nd, struct file *fp) {
> > +     struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> > +                                                     struct se_if_device_ctx,
> > +                                                     miscdev);
> > +     int err = 0;
> > +
> > +     /* Avoid race if opened at the same time */
> > +     if (down_trylock(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     /* Authorize only 1 instance. */
> > +     if (dev_ctx->status != SE_IF_CTX_FREE) {
> > +             err = -EBUSY;
> > +             goto exit;
> > +     }
> > +
> > +     /*
> > +      * Allocate some memory for data exchanges with S40x.
> > +      * This will be used for data not requiring secure memory.
> > +      */
> > +     dev_ctx->non_secure_mem.ptr = dmam_alloc_coherent(dev_ctx->dev,
> > +                                     MAX_DATA_SIZE_PER_USER,
> > +                                     &dev_ctx->non_secure_mem.dma_addr,
> > +                                     GFP_KERNEL);
> > +     if (!dev_ctx->non_secure_mem.ptr) {
> > +             err = -ENOMEM;
> > +             goto exit;
> > +     }
> > +
> > +     dev_ctx->non_secure_mem.size = MAX_DATA_SIZE_PER_USER;
> > +     dev_ctx->non_secure_mem.pos = 0;
> > +     dev_ctx->status = SE_IF_CTX_OPENED;
> > +
> > +     dev_ctx->pending_hdr = 0;
> > +
> > +     goto exit;
> > +
> > +     dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
> > +                        dev_ctx->non_secure_mem.ptr,
> > +                        dev_ctx->non_secure_mem.dma_addr);
> > +
> > +exit:
> > +     up(&dev_ctx->fops_lock);
> > +     return err;
> > +}
> > +
> > +/* Close a character device. */
> > +static int se_if_fops_close(struct inode *nd, struct file *fp) {
> > +     struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> > +                                                     struct se_if_device_ctx,
> > +                                                     miscdev);
> > +     struct se_if_priv *priv = dev_ctx->priv;
> > +     struct se_buf_desc *b_desc;
> > +
> > +     /* Avoid race if closed at the same time */
> > +     if (down_trylock(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     /* The device context has not been opened */
> > +     if (dev_ctx->status != SE_IF_CTX_OPENED)
> > +             goto exit;
> > +
> > +     /* check if this device was registered as command receiver. */
> > +     if (priv->cmd_receiver_dev == dev_ctx)
> > +             priv->cmd_receiver_dev = NULL;
> > +
> > +     /* check if this device was registered as waiting response. */
> > +     if (priv->waiting_rsp_dev == dev_ctx) {
> > +             priv->waiting_rsp_dev = NULL;
> > +             mutex_unlock(&priv->se_if_cmd_lock);
> > +     }
> > +
> > +     /* Unmap secure memory shared buffer. */
> > +     if (dev_ctx->secure_mem.ptr)
> > +             devm_iounmap(dev_ctx->dev, dev_ctx->secure_mem.ptr);
> > +
> > +     dev_ctx->secure_mem.ptr = NULL;
> > +     dev_ctx->secure_mem.dma_addr = 0;
> > +     dev_ctx->secure_mem.size = 0;
> > +     dev_ctx->secure_mem.pos = 0;
> > +
> > +     /* Free non-secure shared buffer. */
> > +     dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
> > +                        dev_ctx->non_secure_mem.ptr,
> > +                        dev_ctx->non_secure_mem.dma_addr);
> > +
> > +     dev_ctx->non_secure_mem.ptr = NULL;
> > +     dev_ctx->non_secure_mem.dma_addr = 0;
> > +     dev_ctx->non_secure_mem.size = 0;
> > +     dev_ctx->non_secure_mem.pos = 0;
> > +
> > +     while (!list_empty(&dev_ctx->pending_in) ||
> > +            !list_empty(&dev_ctx->pending_out)) {
> > +             if (!list_empty(&dev_ctx->pending_in))
> > +                     b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
> > +                                                       struct se_buf_desc,
> > +                                                       link);
> > +             else
> > +                     b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
> > +                                                       struct se_buf_desc,
> > +                                                       link);
> > +
> > +             if (!b_desc)
> > +                     continue;
> > +
> > +             if (b_desc->shared_buf_ptr)
> > +                     memset(b_desc->shared_buf_ptr, 0, b_desc->size);
> > +
> > +             __list_del_entry(&b_desc->link);
> > +             kfree(b_desc);
> > +     }
> > +
> > +     dev_ctx->status = SE_IF_CTX_FREE;
> > +
> > +exit:
> > +     up(&dev_ctx->fops_lock);
> > +     return 0;
> > +}
> > +
> > +/* IOCTL entry point of a character device */ static long
> > +se_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) {
> > +     struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
> > +                                                     struct se_if_device_ctx,
> > +                                                     miscdev);
> > +     struct se_if_priv *se_if_priv = dev_ctx->priv;
> > +     int err = -EINVAL;
> > +
> > +     /* Prevent race during change of device context */
> > +     if (down_interruptible(&dev_ctx->fops_lock))
> > +             return -EBUSY;
> > +
> > +     switch (cmd) {
> > +     case SE_IOCTL_ENABLE_CMD_RCV:
> > +             if (!se_if_priv->cmd_receiver_dev) {
> > +                     se_if_priv->cmd_receiver_dev = dev_ctx;
> > +                     err = 0;
> > +             }
> > +             break;
> > +     case SE_IOCTL_GET_MU_INFO:
> > +             err = se_ioctl_get_mu_info(dev_ctx, arg);
> > +             break;
> > +     case SE_IOCTL_SETUP_IOBUF:
> > +             err = se_ioctl_setup_iobuf_handler(dev_ctx, arg);
> > +             break;
> > +     case SE_IOCTL_GET_SOC_INFO:
> > +             err = se_ioctl_get_se_soc_info_handler(dev_ctx, arg);
> > +             break;
> > +
> > +     default:
> > +             err = -EINVAL;
> > +             dev_dbg(se_if_priv->dev,
> > +                     "%s: IOCTL %.8x not supported\n",
> > +                             dev_ctx->miscdev.name,
> > +                             cmd);
> > +     }
> > +
> > +     up(&dev_ctx->fops_lock);
> > +     return (long)err;
> > +}
> > +
> > +/* Char driver setup */
> > +static const struct file_operations se_if_fops = {
> > +     .open           = se_if_fops_open,
> > +     .owner          = THIS_MODULE,
> > +     .release        = se_if_fops_close,
> > +     .unlocked_ioctl = se_ioctl,
> > +     .read           = se_if_fops_read,
> > +     .write          = se_if_fops_write,
> > +};
> > +
> > +/* interface for managed res to unregister a character device */
> > +static void if_misc_deregister(void *miscdevice) {
> > +     misc_deregister(miscdevice);
> > +}
> > +
> >   /* interface for managed res to free a mailbox channel */
> >   static void if_mbox_free_channel(void *mbox_chan)
> >   {
> > @@ -270,6 +855,7 @@ static int se_probe_if_cleanup(struct
> platform_device *pdev)
> >       struct device *dev = &pdev->dev;
> >       struct se_if_priv *priv;
> >       int ret = 0;
> > +     int i;
> >
> >       priv = dev_get_drvdata(dev);
> >       if (!priv) {
> > @@ -294,6 +880,17 @@ static int se_probe_if_cleanup(struct
> platform_device *pdev)
> >               priv->imem.buf = NULL;
> >       }
> >
> > +     if (priv->ctxs) {
> > +             for (i = 0; i < priv->max_dev_ctx; i++) {
> > +                     if (priv->ctxs[i]) {
> > +                             devm_remove_action(dev,
> > +                                                if_misc_deregister,
> > +                                                &priv->ctxs[i]->miscdev);
> > +                             misc_deregister(&priv->ctxs[i]->miscdev);
> > +                     }
> > +             }
> > +     }
> > +
> >       if (priv->flags & RESERVED_DMA_POOL) {
> >               of_reserved_mem_device_release(dev);
> >               priv->flags &= (~RESERVED_DMA_POOL); @@ -302,6 +899,84
> > @@ static int se_probe_if_cleanup(struct platform_device *pdev)
> >       return ret;
> >   }
> >
> > +static int init_device_context(struct device *dev) {
> > +     const struct imx_se_node_info *info;
> > +     struct se_if_device_ctx *dev_ctx;
> > +     struct se_if_priv *priv;
> > +     u8 *devname;
> > +     int ret = 0;
> > +     int i;
> > +
> > +     priv = dev_get_drvdata(dev);
> > +
> > +     if (!priv) {
> > +             ret = -EINVAL;
> > +             dev_err(dev, "Invalid SE-MU Priv data");
> > +             return ret;
> > +     }
> > +     info = priv->info;
> > +
> > +     priv->ctxs = devm_kzalloc(dev, sizeof(dev_ctx) * priv->max_dev_ctx,
> > +                               GFP_KERNEL);
> > +
> > +     if (!priv->ctxs) {
> > +             ret = -ENOMEM;
> > +             return ret;
> > +     }
> > +
> > +     /* Create users */
> > +     for (i = 0; i < priv->max_dev_ctx; i++) {
> > +             dev_ctx = devm_kzalloc(dev, sizeof(*dev_ctx), GFP_KERNEL);
> > +             if (!dev_ctx) {
> > +                     ret = -ENOMEM;
> > +                     return ret;
> > +             }
> > +
> > +             dev_ctx->dev = dev;
> > +             dev_ctx->status = SE_IF_CTX_FREE;
> > +             dev_ctx->priv = priv;
> > +
> > +             priv->ctxs[i] = dev_ctx;
> > +
> > +             /* Default value invalid for an header. */
> > +             init_waitqueue_head(&dev_ctx->wq);
> > +
> > +             INIT_LIST_HEAD(&dev_ctx->pending_out);
> > +             INIT_LIST_HEAD(&dev_ctx->pending_in);
> > +             sema_init(&dev_ctx->fops_lock, 1);
> > +
> > +             devname = devm_kasprintf(dev, GFP_KERNEL, "%s_ch%d",
> > +                                      info->se_name, i);
> > +             if (!devname) {
> > +                     ret = -ENOMEM;
> > +                     return ret;
> > +             }
> > +
> > +             dev_ctx->miscdev.name = devname;
> > +             dev_ctx->miscdev.minor = MISC_DYNAMIC_MINOR;
> > +             dev_ctx->miscdev.fops = &se_if_fops;
> > +             dev_ctx->miscdev.parent = dev;
> > +             ret = misc_register(&dev_ctx->miscdev);
> > +             if (ret) {
> > +                     dev_err(dev, "failed to register misc device %d\n",
> > +                             ret);
> > +                     return ret;
> > +             }
> > +
> > +             ret = devm_add_action(dev, if_misc_deregister,
> > +                                   &dev_ctx->miscdev);
> > +             if (ret) {
> > +                     dev_err(dev,
> > +                             "failed[%d] to add action to the misc-dev\n",
> > +                             ret);
> > +                     return ret;
> > +             }
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> >   static void se_load_firmware(const struct firmware *fw, void *context)
> >   {
> >       struct se_if_priv *priv = (struct se_if_priv *) context; @@
> > -461,6 +1136,16 @@ static int se_if_probe(struct platform_device *pdev)
> >               }
> >       }
> >
> > +     if (info->max_dev_ctx) {
> > +             ret = init_device_context(dev);
> > +             if (ret) {
> > +                     dev_err(dev,
> > +                             "Failed[0x%x] to create device contexts.\n",
> > +                             ret);
> > +                     goto exit;
> > +             }
> > +     }
> > +
> >       dev_info(dev, "i.MX secure-enclave: %s interface to firmware,
> configured.\n",
> >                info->se_name);
> >       return devm_of_platform_populate(dev); @@ -502,6 +1187,10 @@
> > static int se_resume(struct device *dev)
> >       struct se_if_priv *priv = dev_get_drvdata(dev);
> >       const struct imx_se_node_info *info
> >                                       = priv->info;
> > +     int i;
> > +
> > +     for (i = 0; i < priv->max_dev_ctx; i++)
> > +             wake_up_interruptible(&priv->ctxs[i]->wq);
> >
> >       if (info && info->imem_mgmt)
> >               se_restore_imem_state(dev); diff --git
> > a/drivers/firmware/imx/se_ctrl.h b/drivers/firmware/imx/se_ctrl.h
> > index 7d4f439a6158..41d9cedb05d7 100644
> > --- a/drivers/firmware/imx/se_ctrl.h
> > +++ b/drivers/firmware/imx/se_ctrl.h
> > @@ -13,15 +13,61 @@
> >   #define MAX_FW_LOAD_RETRIES         50
> >
> >   #define RES_STATUS(x)                       FIELD_GET(0x000000ff, x)
> > +#define MAX_DATA_SIZE_PER_USER               (65 * 1024)
> >   #define MESSAGING_VERSION_6         0x6
> >   #define MESSAGING_VERSION_7         0x7
> >
> > +#define SE_MU_IO_FLAGS_USE_SEC_MEM   (0x02u)
> > +#define SE_MU_IO_FLAGS_USE_SHORT_ADDR        (0x04u)
> > +
> >   struct se_imem_buf {
> >       u8 *buf;
> >       phys_addr_t phyaddr;
> >       u32 size;
> >   };
> >
> > +struct se_buf_desc {
> > +     u8 *shared_buf_ptr;
> > +     u8 *usr_buf_ptr;
> > +     u32 size;
> > +     struct list_head link;
> > +};
> > +
> > +/* Status of a char device */
> > +enum se_if_dev_ctx_status_t {
> > +     SE_IF_CTX_FREE,
> > +     SE_IF_CTX_OPENED
> > +};
> > +
> > +struct se_shared_mem {
> > +     dma_addr_t dma_addr;
> > +     u32 size;
> > +     u32 pos;
> > +     u8 *ptr;
> > +};
> > +
> > +/* Private struct for each char device instance. */ struct
> > +se_if_device_ctx {
> > +     struct device *dev;
> > +     struct se_if_priv *priv;
> > +     struct miscdevice miscdev;
> > +
> > +     enum se_if_dev_ctx_status_t status;
> > +     wait_queue_head_t wq;
> > +     struct semaphore fops_lock;
> > +
> > +     u32 pending_hdr;
> > +     struct list_head pending_in;
> > +     struct list_head pending_out;
> > +
> > +     struct se_shared_mem secure_mem;
> > +     struct se_shared_mem non_secure_mem;
> > +
> > +     u32 *temp_resp;
> > +     u32 temp_resp_size;
> > +     struct notifier_block se_notify; };
> > +
> >   /* Header of the messages exchange with the EdgeLock Enclave */
> >   struct se_msg_hdr {
> >       u8 ver;
> > diff --git a/include/uapi/linux/se_ioctl.h
> > b/include/uapi/linux/se_ioctl.h new file mode 100644 index
> > 000000000000..f68a36e9da2c
> > --- /dev/null
> > +++ b/include/uapi/linux/se_ioctl.h
> > @@ -0,0 +1,88 @@
> > +/* SPDX-License-Identifier: (GPL-2.0 WITH Linux-syscall-note) OR
> > +BSD-3-Clause*/
> > +/*
> > + * Copyright 2024 NXP
> > + */
> > +
> > +#ifndef SE_IOCTL_H
> > +#define SE_IOCTL_H
> > +
> > +/* IOCTL definitions. */
> > +
> > +struct se_ioctl_setup_iobuf {
> > +     u8 *user_buf;
> > +     u32 length;
> > +     u32 flags;
> > +     u64 ele_addr;
> > +};
> > +
> > +struct se_ioctl_shared_mem_cfg {
> > +     u32 base_offset;
> > +     u32 size;
> > +};
> > +
> > +struct se_ioctl_get_if_info {
> > +     u8 se_if_id;
> > +     u8 interrupt_idx;
> > +     u8 tz;
> > +     u8 did;
> > +     u8 cmd_tag;
> > +     u8 rsp_tag;
> > +     u8 success_tag;
> > +     u8 base_api_ver;
> > +     u8 fw_api_ver;
> > +};
> > +
> > +struct se_ioctl_signed_message {
> > +     u8 *message;
> > +     u32 msg_size;
> > +     u32 error_code;
> > +};
> > +
> > +struct se_ioctl_get_soc_info {
> > +     u16 soc_id;
> > +     u16 soc_rev;
> > +};
> > +
> > +/* IO Buffer Flags */
> > +#define SE_IO_BUF_FLAGS_IS_OUTPUT    (0x00u)
> > +#define SE_IO_BUF_FLAGS_IS_INPUT     (0x01u)
> > +#define SE_IO_BUF_FLAGS_USE_SEC_MEM  (0x02u)
> > +#define SE_IO_BUF_FLAGS_USE_SHORT_ADDR       (0x04u)
> > +#define SE_IO_BUF_FLAGS_IS_IN_OUT    (0x10u)
> > +
> > +/* IOCTLS */
> > +#define SE_IOCTL                     0x0A /* like MISC_MAJOR. */
> > +
> > +/*
> > + * ioctl to designated the current fd as logical-reciever.
> > + * This is ioctl is send when the nvm-daemon, a slave to the
> > + * firmware is started by the user.
> > + */
> > +#define SE_IOCTL_ENABLE_CMD_RCV      _IO(SE_IOCTL, 0x01)
> > +
> > +/*
> > + * ioctl to get the buffer allocated from the memory, which is shared
> > + * between kernel and FW.
> > + * Post allocation, the kernel tagged the allocated memory with:
> > + *  Output
> > + *  Input
> > + *  Input-Output
> > + *  Short address
> > + *  Secure-memory
> > + */
> > +#define SE_IOCTL_SETUP_IOBUF _IOWR(SE_IOCTL, 0x03, \
> > +                                     struct se_ioctl_setup_iobuf)
> > +
> > +/*
> > + * ioctl to get the mu information, that is used to exchange message
> > + * with FW, from user-spaced.
> > + */
> > +#define SE_IOCTL_GET_MU_INFO _IOR(SE_IOCTL, 0x04, \
> > +                                     struct se_ioctl_get_if_info)
> > +/*
> > + * ioctl to get SoC Info from user-space.
> > + */
> > +#define SE_IOCTL_GET_SOC_INFO      _IOR(SE_IOCTL, 0x06, \
> > +                                     struct se_ioctl_get_soc_info)
> > +
> > +#endif
> >

Patch

diff --git a/Documentation/ABI/testing/se-cdev b/Documentation/ABI/testing/se-cdev
new file mode 100644
index 000000000000..699525af6b86
--- /dev/null
+++ b/Documentation/ABI/testing/se-cdev
@@ -0,0 +1,42 @@ 
+What:		/dev/<se>_mu[0-9]+_ch[0-9]+
+Date:		May 2024
+KernelVersion:	6.8
+Contact:	[e-mail], [e-mail]
+Description:
+		NXP offers multiple hardware IP(s) for  secure-enclaves like EdgeLock-
+		Enclave(ELE), SECO. The character device file-descriptors
+		/dev/<se>_mu*_ch* are the interface between user-space NXP's secure-
+		enclave shared-library and the kernel driver.
+
+		The ioctl(2)-based ABI is defined and documented in
+		[include]<linux/firmware/imx/ele_mu_ioctl.h>
+		 ioctl(s) are used primarily for:
+			- shared memory management
+			- allocation of I/O buffers
+			- get mu info
+			- setting a dev-ctx as receiver that is slave to fw
+			- get SoC info
+
+		The following file operations are supported:
+
+		open(2)
+		  Currently the only useful flags are O_RDWR.
+
+		read(2)
+		  Every read() from the opened character device context is waiting on
+		  wakeup_intruptible, that gets set by the registered mailbox callback
+		  function; indicating a message received from the firmware on message-
+		  unit.
+
+		write(2)
+		  Every write() to the opened character device context needs to acquire
+		  mailbox_lock, before sending message on to the message unit.
+
+		close(2)
+		  Stops and free up the I/O contexts that was associated
+		  with the file descriptor.
+
+Users:		https://github.com/nxp-imx/imx-secure-enclave.git,
+		https://github.com/nxp-imx/imx-smw.git
+		crypto/skcipher,
+		drivers/nvmem/imx-ocotp-ele.c
diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
index c286c3d84d82..15fabc369b21 100644
--- a/drivers/firmware/imx/ele_common.c
+++ b/drivers/firmware/imx/ele_common.c
@@ -78,12 +78,98 @@  int imx_ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, void *rx_msg)
 	return err;
 }
 
+int imx_ele_miscdev_msg_rcv(struct se_if_device_ctx *dev_ctx)
+{
+	struct se_msg_hdr *header = {0};
+	int err;
+
+	if (dev_ctx->priv->waiting_rsp_dev == dev_ctx)
+		lockdep_assert_held(&dev_ctx->priv->se_if_cmd_lock);
+
+	err = wait_event_interruptible(dev_ctx->wq, dev_ctx->pending_hdr != 0);
+	if (err)
+		dev_err(dev_ctx->dev,
+			"%s: Err[0x%x]:Interrupted by signal.\n",
+			dev_ctx->miscdev.name, err);
+
+	header = (struct se_msg_hdr *) dev_ctx->temp_resp;
+
+	if (header->tag == dev_ctx->priv->rsp_tag) {
+		if (dev_ctx->priv->waiting_rsp_dev != dev_ctx)
+			dev_warn(dev_ctx->dev,
+			"%s: Device context waiting for response mismatch.\n",
+			dev_ctx->miscdev.name);
+		else
+			dev_ctx->priv->waiting_rsp_dev = NULL;
+
+		mutex_unlock(&dev_ctx->priv->se_if_cmd_lock);
+	}
+
+	return err;
+}
+
+int imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
+			     void *tx_msg, int tx_msg_sz)
+{
+	struct se_if_priv *priv = dev_ctx->priv;
+	struct se_msg_hdr *header;
+	int err;
+
+	header = (struct se_msg_hdr *) tx_msg;
+
+	/*
+	 * Check that the size passed as argument matches the size
+	 * carried in the message.
+	 */
+	err = header->size << 2;
+
+	if (err != tx_msg_sz) {
+		err = -EINVAL;
+		dev_err(priv->dev,
+			"%s: User buffer too small\n",
+				dev_ctx->miscdev.name);
+		goto exit;
+	}
+	/* Check the message is valid according to tags */
+	if (header->tag == priv->cmd_tag) {
+		mutex_lock(&priv->se_if_cmd_lock);
+		priv->waiting_rsp_dev = dev_ctx;
+	} else if (header->tag == priv->rsp_tag) {
+		/* Check the device context can send the command */
+		if (dev_ctx != priv->cmd_receiver_dev) {
+			dev_err(priv->dev,
+				"%s: Channel not configured to send resp to FW.",
+				dev_ctx->miscdev.name);
+			err = -EPERM;
+			goto exit;
+		}
+	} else {
+		dev_err(priv->dev,
+			"%s: The message does not have a valid TAG\n",
+				dev_ctx->miscdev.name);
+		err = -EINVAL;
+		goto exit;
+	}
+	err = imx_ele_msg_send(priv, tx_msg);
+	if (err < 0) {
+		if (header->tag == priv->cmd_tag) {
+			priv->waiting_rsp_dev = NULL;
+			mutex_unlock(&dev_ctx->priv->se_if_cmd_lock);
+		}
+	} else
+		err = header->size << 2;
+exit:
+	return err;
+}
+
 /*
  * Callback called by mailbox FW, when data is received.
  */
 void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
 {
 	struct device *dev = mbox_cl->dev;
+	struct se_if_device_ctx *dev_ctx;
+	struct se_api_msg *rx_msg;
 	struct se_if_priv *priv;
 	struct se_msg_hdr *header;
 
@@ -97,8 +183,15 @@  void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
 
 	header = (struct se_msg_hdr *) msg;
 
-	if (header->tag == priv->rsp_tag) {
-		if (!priv->waiting_rsp_dev) {
+	/* Incoming command: wake up the receiver if any. */
+	if (header->tag == priv->cmd_tag) {
+		dev_dbg(dev, "Selecting cmd receiver\n");
+		dev_ctx = priv->cmd_receiver_dev;
+	} else if (header->tag == priv->rsp_tag) {
+		if (priv->waiting_rsp_dev) {
+			dev_dbg(dev, "Selecting rsp waiter\n");
+			dev_ctx = priv->waiting_rsp_dev;
+		} else {
 			/*
 			 * Reading the EdgeLock Enclave response
 			 * to the command, sent by other
@@ -116,6 +209,17 @@  void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
 				*((u32 *) header));
 		return;
 	}
+	/* Init reception */
+	rx_msg = kzalloc(header->size << 2, GFP_KERNEL);
+	if (rx_msg)
+		memcpy(rx_msg, msg, header->size << 2);
+
+	dev_ctx->temp_resp = (u32 *)rx_msg;
+	dev_ctx->temp_resp_size = header->size;
+
+	/* Allow user to read */
+	dev_ctx->pending_hdr = 1;
+	wake_up_interruptible(&dev_ctx->wq);
 }
 
 int validate_rsp_hdr(struct se_if_priv *priv,
diff --git a/drivers/firmware/imx/ele_common.h b/drivers/firmware/imx/ele_common.h
index 76777ac629d6..11b9b36d4fda 100644
--- a/drivers/firmware/imx/ele_common.h
+++ b/drivers/firmware/imx/ele_common.h
@@ -12,6 +12,9 @@ 
 #define IMX_ELE_FW_DIR                 "imx/ele/"
 
 uint32_t plat_add_msg_crc(uint32_t *msg, uint32_t msg_len);
+int imx_ele_miscdev_msg_rcv(struct se_if_device_ctx *priv);
+int imx_ele_miscdev_msg_send(struct se_if_device_ctx *dev_ctx,
+			     void *tx_msg, int tx_msg_sz);
 int imx_ele_msg_rcv(struct se_if_priv *priv);
 int imx_ele_msg_send(struct se_if_priv *priv, void *tx_msg);
 int imx_ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, void *rx_msg);
diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
index 0642d349b3d3..3acaecd8f3bc 100644
--- a/drivers/firmware/imx/se_ctrl.c
+++ b/drivers/firmware/imx/se_ctrl.c
@@ -23,6 +23,7 @@ 
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/sys_soc.h>
+#include <uapi/linux/se_ioctl.h>
 
 #include "ele_base_msg.h"
 #include "ele_common.h"
@@ -232,6 +233,590 @@  static int imx_fetch_se_soc_info(struct device *dev)
 	return 0;
 }
 
+/*
+ * File operations for user-space
+ */
+
+/* Write a message to the MU. */
+static ssize_t se_if_fops_write(struct file *fp, const char __user *buf,
+				size_t size, loff_t *ppos)
+{
+	struct se_api_msg *tx_msg __free(kfree);
+	struct se_if_device_ctx *dev_ctx;
+	struct se_if_priv *priv;
+	int err;
+
+	dev_ctx = container_of(fp->private_data,
+			       struct se_if_device_ctx,
+			       miscdev);
+	priv = dev_ctx->priv;
+	dev_dbg(priv->dev,
+		"%s: write from buf (%p)%zu, ppos=%lld\n",
+			dev_ctx->miscdev.name,
+			buf, size, ((ppos) ? *ppos : 0));
+
+	if (down_interruptible(&dev_ctx->fops_lock))
+		return -EBUSY;
+
+	if (dev_ctx->status != SE_IF_CTX_OPENED) {
+		err = -EINVAL;
+		goto exit;
+	}
+
+	if (size < SE_MU_HDR_SZ) {
+		dev_err(priv->dev,
+			"%s: User buffer too small(%zu < %d)\n",
+				dev_ctx->miscdev.name,
+				size, SE_MU_HDR_SZ);
+		err = -ENOSPC;
+		goto exit;
+	}
+	tx_msg = memdup_user(buf, size);
+	if (!tx_msg) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	print_hex_dump_debug("from user ", DUMP_PREFIX_OFFSET, 4, 4,
+			     tx_msg, size, false);
+
+	err = imx_ele_miscdev_msg_send(dev_ctx, tx_msg, size);
+
+exit:
+	up(&dev_ctx->fops_lock);
+	return err;
+}
+
+/*
+ * Read a message from the MU.
+ * Blocking until a message is available.
+ */
+static ssize_t se_if_fops_read(struct file *fp, char __user *buf,
+			       size_t size, loff_t *ppos)
+{
+	struct se_if_device_ctx *dev_ctx;
+	struct se_buf_desc *b_desc;
+	struct se_if_priv *priv;
+	u32 size_to_copy;
+	int err;
+
+	dev_ctx = container_of(fp->private_data,
+			       struct se_if_device_ctx,
+			       miscdev);
+	priv = dev_ctx->priv;
+	dev_dbg(priv->dev,
+		"%s: read to buf %p(%zu), ppos=%lld\n",
+			dev_ctx->miscdev.name,
+			buf, size, ((ppos) ? *ppos : 0));
+
+	if (down_interruptible(&dev_ctx->fops_lock))
+		return -EBUSY;
+
+	if (dev_ctx->status != SE_IF_CTX_OPENED) {
+		err = -EINVAL;
+		goto exit;
+	}
+
+	err = imx_ele_miscdev_msg_rcv(dev_ctx);
+	if (err)
+		goto exit;
+
+	/* Buffer containing the message from FW, is
+	 * allocated in callback function.
+	 * Check if buffer allocation failed.
+	 */
+	if (!dev_ctx->temp_resp) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	dev_dbg(priv->dev,
+			"%s: %s %s\n",
+			dev_ctx->miscdev.name,
+			__func__,
+			"message received, start transmit to user");
+
+	/*
+	 * Check that the size passed as argument is larger than
+	 * the one carried in the message.
+	 */
+	size_to_copy = dev_ctx->temp_resp_size << 2;
+	if (size_to_copy > size) {
+		dev_dbg(priv->dev,
+			"%s: User buffer too small (%zu < %d)\n",
+				dev_ctx->miscdev.name,
+				size, size_to_copy);
+		size_to_copy = size;
+	}
+
+	/*
+	 * We may need to copy the output data to user before
+	 * delivering the completion message.
+	 */
+	while (!list_empty(&dev_ctx->pending_out)) {
+		b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
+						  struct se_buf_desc,
+						  link);
+		if (!b_desc)
+			continue;
+
+		if (b_desc->usr_buf_ptr && b_desc->shared_buf_ptr) {
+
+			dev_dbg(priv->dev,
+				"%s: Copy output data to user\n",
+				dev_ctx->miscdev.name);
+			if (copy_to_user(b_desc->usr_buf_ptr,
+					 b_desc->shared_buf_ptr,
+					 b_desc->size)) {
+				dev_err(priv->dev,
+					"%s: Failure copying output data to user.",
+					dev_ctx->miscdev.name);
+				err = -EFAULT;
+				goto exit;
+			}
+		}
+
+		if (b_desc->shared_buf_ptr)
+			memset(b_desc->shared_buf_ptr, 0, b_desc->size);
+
+		__list_del_entry(&b_desc->link);
+		kfree(b_desc);
+	}
+
+	/* Copy data from the buffer */
+	print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
+			     dev_ctx->temp_resp, size_to_copy, false);
+	if (copy_to_user(buf, dev_ctx->temp_resp, size_to_copy)) {
+		dev_err(priv->dev,
+			"%s: Failed to copy to user\n",
+				dev_ctx->miscdev.name);
+		err = -EFAULT;
+		goto exit;
+	}
+
+	err = size_to_copy;
+	kfree(dev_ctx->temp_resp);
+
+	/* free memory allocated on the shared buffers. */
+	dev_ctx->secure_mem.pos = 0;
+	dev_ctx->non_secure_mem.pos = 0;
+
+	dev_ctx->pending_hdr = 0;
+
+exit:
+	/*
+	 * Clean the used Shared Memory space,
+	 * whether its Input Data copied from user buffers, or
+	 * Data received from FW.
+	 */
+	while (!list_empty(&dev_ctx->pending_in) ||
+	       !list_empty(&dev_ctx->pending_out)) {
+		if (!list_empty(&dev_ctx->pending_in))
+			b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
+							  struct se_buf_desc,
+							  link);
+		else
+			b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
+							  struct se_buf_desc,
+							  link);
+
+		if (!b_desc)
+			continue;
+
+		if (b_desc->shared_buf_ptr)
+			memset(b_desc->shared_buf_ptr, 0, b_desc->size);
+
+		__list_del_entry(&b_desc->link);
+		kfree(b_desc);
+	}
+
+	up(&dev_ctx->fops_lock);
+	return err;
+}
+
+static int se_ioctl_get_mu_info(struct se_if_device_ctx *dev_ctx,
+				u64 arg)
+{
+	struct se_if_priv *priv = dev_get_drvdata(dev_ctx->dev);
+	struct imx_se_node_info *if_node_info;
+	struct se_ioctl_get_if_info info;
+	int err = 0;
+
+	if_node_info = (struct imx_se_node_info *)priv->info;
+
+	info.se_if_id = if_node_info->se_if_id;
+	info.interrupt_idx = 0;
+	info.tz = 0;
+	info.did = if_node_info->se_if_did;
+	info.cmd_tag = if_node_info->cmd_tag;
+	info.rsp_tag = if_node_info->rsp_tag;
+	info.success_tag = if_node_info->success_tag;
+	info.base_api_ver = if_node_info->base_api_ver;
+	info.fw_api_ver = if_node_info->fw_api_ver;
+
+	dev_dbg(priv->dev,
+		"%s: info [se_if_id: %d, irq_idx: %d, tz: 0x%x, did: 0x%x]\n",
+			dev_ctx->miscdev.name,
+			info.se_if_id, info.interrupt_idx, info.tz, info.did);
+
+	if (copy_to_user((u8 *)arg, &info, sizeof(info))) {
+		dev_err(dev_ctx->priv->dev,
+			"%s: Failed to copy mu info to user\n",
+				dev_ctx->miscdev.name);
+		err = -EFAULT;
+		goto exit;
+	}
+
+exit:
+	return err;
+}
+
+/*
+ * Copy a buffer of data to/from the user and return the address to use in
+ * messages
+ */
+static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
+					    u64 arg)
+{
+	struct se_shared_mem *shared_mem = NULL;
+	struct se_ioctl_setup_iobuf io = {0};
+	struct se_buf_desc *b_desc = NULL;
+	int err = 0;
+	u32 pos;
+
+	if (copy_from_user(&io, (u8 *)arg, sizeof(io))) {
+		dev_err(dev_ctx->priv->dev,
+			"%s: Failed copy iobuf config from user\n",
+				dev_ctx->miscdev.name);
+		err = -EFAULT;
+		goto exit;
+	}
+
+	dev_dbg(dev_ctx->priv->dev,
+			"%s: io [buf: %p(%d) flag: %x]\n",
+			dev_ctx->miscdev.name,
+			io.user_buf, io.length, io.flags);
+
+	if (io.length == 0 || !io.user_buf) {
+		/*
+		 * Accept NULL pointers since some buffers are optional
+		 * in FW commands. In this case we should return 0 as
+		 * pointer to be embedded into the message.
+		 * Skip all data copy part of code below.
+		 */
+		io.ele_addr = 0;
+		goto copy;
+	}
+
+	/* Select the shared memory to be used for this buffer. */
+	if (io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) {
+		/* App requires to use secure memory for this buffer.*/
+		dev_err(dev_ctx->priv->dev,
+			"%s: Failed allocate SEC MEM memory\n",
+				dev_ctx->miscdev.name);
+		err = -EFAULT;
+		goto exit;
+	} else {
+		/* No specific requirement for this buffer. */
+		shared_mem = &dev_ctx->non_secure_mem;
+	}
+
+	/* Check there is enough space in the shared memory. */
+	if (shared_mem->size < shared_mem->pos
+			|| io.length >= shared_mem->size - shared_mem->pos) {
+		dev_err(dev_ctx->priv->dev,
+			"%s: Not enough space in shared memory\n",
+				dev_ctx->miscdev.name);
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	/* Allocate space in shared memory. 8 bytes aligned. */
+	pos = shared_mem->pos;
+	shared_mem->pos += round_up(io.length, 8u);
+	io.ele_addr = (u64)shared_mem->dma_addr + pos;
+
+	if ((io.flags & SE_MU_IO_FLAGS_USE_SEC_MEM) &&
+	    !(io.flags & SE_MU_IO_FLAGS_USE_SHORT_ADDR)) {
+		/*Add base address to get full address.*/
+		dev_err(dev_ctx->priv->dev,
+			"%s: Failed allocate SEC MEM memory\n",
+				dev_ctx->miscdev.name);
+		err = -EFAULT;
+		goto exit;
+	}
+
+	memset(shared_mem->ptr + pos, 0, io.length);
+	if ((io.flags & SE_IO_BUF_FLAGS_IS_INPUT) ||
+	    (io.flags & SE_IO_BUF_FLAGS_IS_IN_OUT)) {
+		/*
+		 * buffer is input:
+		 * copy data from user space to this allocated buffer.
+		 */
+		if (copy_from_user(shared_mem->ptr + pos, io.user_buf,
+				   io.length)) {
+			dev_err(dev_ctx->priv->dev,
+				"%s: Failed copy data to shared memory\n",
+				dev_ctx->miscdev.name);
+			err = -EFAULT;
+			goto exit;
+		}
+	}
+
+	b_desc = kzalloc(sizeof(*b_desc), GFP_KERNEL);
+	if (!b_desc) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+copy:
+	/* Provide the EdgeLock Enclave address to user space only if success.*/
+	if (copy_to_user((u8 *)arg, &io, sizeof(io))) {
+		dev_err(dev_ctx->priv->dev,
+			"%s: Failed to copy iobuff setup to user\n",
+				dev_ctx->miscdev.name);
+		kfree(b_desc);
+		err = -EFAULT;
+		goto exit;
+	}
+
+	if (b_desc) {
+		b_desc->shared_buf_ptr = shared_mem->ptr + pos;
+		b_desc->usr_buf_ptr = io.user_buf;
+		b_desc->size = io.length;
+
+		if (io.flags & SE_IO_BUF_FLAGS_IS_INPUT) {
+			/*
+			 * buffer is input:
+			 * add an entry in the "pending input buffers" list so
+			 * that copied data can be cleaned from shared memory
+			 * later.
+			 */
+			list_add_tail(&b_desc->link, &dev_ctx->pending_in);
+		} else {
+			/*
+			 * buffer is output:
+			 * add an entry in the "pending out buffers" list so data
+			 * can be copied to user space when receiving Secure-Enclave
+			 * response.
+			 */
+			list_add_tail(&b_desc->link, &dev_ctx->pending_out);
+		}
+	}
+
+exit:
+	return err;
+}
+
+/* IOCTL to provide SoC information */
+static int se_ioctl_get_se_soc_info_handler(struct se_if_device_ctx *dev_ctx,
+					     u64 arg)
+{
+	struct imx_se_node_info_list *info_list;
+	struct se_ioctl_get_soc_info soc_info;
+	int err = -EINVAL;
+
+	info_list = (struct imx_se_node_info_list *)
+			device_get_match_data(dev_ctx->priv->dev);
+	if (!info_list)
+		goto exit;
+
+	soc_info.soc_id = info_list->soc_id;
+	soc_info.soc_rev = dev_ctx->priv->soc_rev;
+
+	err = (int)copy_to_user((u8 *)arg, (u8 *)(&soc_info), sizeof(soc_info));
+	if (err) {
+		dev_err(dev_ctx->priv->dev,
+			"%s: Failed to copy soc info to user\n",
+			dev_ctx->miscdev.name);
+		err = -EFAULT;
+		goto exit;
+	}
+
+exit:
+	return err;
+}
+
+/* Open a character device. */
+static int se_if_fops_open(struct inode *nd, struct file *fp)
+{
+	struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
+							struct se_if_device_ctx,
+							miscdev);
+	int err = 0;
+
+	/* Avoid race if opened at the same time */
+	if (down_trylock(&dev_ctx->fops_lock))
+		return -EBUSY;
+
+	/* Authorize only 1 instance. */
+	if (dev_ctx->status != SE_IF_CTX_FREE) {
+		err = -EBUSY;
+		goto exit;
+	}
+
+	/*
+	 * Allocate some memory for data exchanges with S40x.
+	 * This will be used for data not requiring secure memory.
+	 */
+	dev_ctx->non_secure_mem.ptr = dmam_alloc_coherent(dev_ctx->dev,
+					MAX_DATA_SIZE_PER_USER,
+					&dev_ctx->non_secure_mem.dma_addr,
+					GFP_KERNEL);
+	if (!dev_ctx->non_secure_mem.ptr) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	dev_ctx->non_secure_mem.size = MAX_DATA_SIZE_PER_USER;
+	dev_ctx->non_secure_mem.pos = 0;
+	dev_ctx->status = SE_IF_CTX_OPENED;
+
+	dev_ctx->pending_hdr = 0;
+
+	goto exit;
+
+	dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
+			   dev_ctx->non_secure_mem.ptr,
+			   dev_ctx->non_secure_mem.dma_addr);
+
+exit:
+	up(&dev_ctx->fops_lock);
+	return err;
+}
+
+/* Close a character device. */
+static int se_if_fops_close(struct inode *nd, struct file *fp)
+{
+	struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
+							struct se_if_device_ctx,
+							miscdev);
+	struct se_if_priv *priv = dev_ctx->priv;
+	struct se_buf_desc *b_desc;
+
+	/* Avoid race if closed at the same time */
+	if (down_trylock(&dev_ctx->fops_lock))
+		return -EBUSY;
+
+	/* The device context has not been opened */
+	if (dev_ctx->status != SE_IF_CTX_OPENED)
+		goto exit;
+
+	/* check if this device was registered as command receiver. */
+	if (priv->cmd_receiver_dev == dev_ctx)
+		priv->cmd_receiver_dev = NULL;
+
+	/* check if this device was registered as waiting response. */
+	if (priv->waiting_rsp_dev == dev_ctx) {
+		priv->waiting_rsp_dev = NULL;
+		mutex_unlock(&priv->se_if_cmd_lock);
+	}
+
+	/* Unmap secure memory shared buffer. */
+	if (dev_ctx->secure_mem.ptr)
+		devm_iounmap(dev_ctx->dev, dev_ctx->secure_mem.ptr);
+
+	dev_ctx->secure_mem.ptr = NULL;
+	dev_ctx->secure_mem.dma_addr = 0;
+	dev_ctx->secure_mem.size = 0;
+	dev_ctx->secure_mem.pos = 0;
+
+	/* Free non-secure shared buffer. */
+	dmam_free_coherent(dev_ctx->priv->dev, MAX_DATA_SIZE_PER_USER,
+			   dev_ctx->non_secure_mem.ptr,
+			   dev_ctx->non_secure_mem.dma_addr);
+
+	dev_ctx->non_secure_mem.ptr = NULL;
+	dev_ctx->non_secure_mem.dma_addr = 0;
+	dev_ctx->non_secure_mem.size = 0;
+	dev_ctx->non_secure_mem.pos = 0;
+
+	while (!list_empty(&dev_ctx->pending_in) ||
+	       !list_empty(&dev_ctx->pending_out)) {
+		if (!list_empty(&dev_ctx->pending_in))
+			b_desc = list_first_entry_or_null(&dev_ctx->pending_in,
+							  struct se_buf_desc,
+							  link);
+		else
+			b_desc = list_first_entry_or_null(&dev_ctx->pending_out,
+							  struct se_buf_desc,
+							  link);
+
+		if (!b_desc)
+			continue;
+
+		if (b_desc->shared_buf_ptr)
+			memset(b_desc->shared_buf_ptr, 0, b_desc->size);
+
+		__list_del_entry(&b_desc->link);
+		kfree(b_desc);
+	}
+
+	dev_ctx->status = SE_IF_CTX_FREE;
+
+exit:
+	up(&dev_ctx->fops_lock);
+	return 0;
+}
+
+/* IOCTL entry point of a character device */
+static long se_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+{
+	struct se_if_device_ctx *dev_ctx = container_of(fp->private_data,
+							struct se_if_device_ctx,
+							miscdev);
+	struct se_if_priv *se_if_priv = dev_ctx->priv;
+	int err = -EINVAL;
+
+	/* Prevent race during change of device context */
+	if (down_interruptible(&dev_ctx->fops_lock))
+		return -EBUSY;
+
+	switch (cmd) {
+	case SE_IOCTL_ENABLE_CMD_RCV:
+		if (!se_if_priv->cmd_receiver_dev) {
+			se_if_priv->cmd_receiver_dev = dev_ctx;
+			err = 0;
+		}
+		break;
+	case SE_IOCTL_GET_MU_INFO:
+		err = se_ioctl_get_mu_info(dev_ctx, arg);
+		break;
+	case SE_IOCTL_SETUP_IOBUF:
+		err = se_ioctl_setup_iobuf_handler(dev_ctx, arg);
+		break;
+	case SE_IOCTL_GET_SOC_INFO:
+		err = se_ioctl_get_se_soc_info_handler(dev_ctx, arg);
+		break;
+
+	default:
+		err = -EINVAL;
+		dev_dbg(se_if_priv->dev,
+			"%s: IOCTL %.8x not supported\n",
+				dev_ctx->miscdev.name,
+				cmd);
+	}
+
+	up(&dev_ctx->fops_lock);
+	return (long)err;
+}
+
+/* Char driver setup */
+static const struct file_operations se_if_fops = {
+	.open		= se_if_fops_open,
+	.owner		= THIS_MODULE,
+	.release	= se_if_fops_close,
+	.unlocked_ioctl = se_ioctl,
+	.read		= se_if_fops_read,
+	.write		= se_if_fops_write,
+};
+
+/* interface for managed res to unregister a character device */
+static void if_misc_deregister(void *miscdevice)
+{
+	misc_deregister(miscdevice);
+}
+
 /* interface for managed res to free a mailbox channel */
 static void if_mbox_free_channel(void *mbox_chan)
 {
@@ -270,6 +855,7 @@  static int se_probe_if_cleanup(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct se_if_priv *priv;
 	int ret = 0;
+	int i;
 
 	priv = dev_get_drvdata(dev);
 	if (!priv) {
@@ -294,6 +880,17 @@  static int se_probe_if_cleanup(struct platform_device *pdev)
 		priv->imem.buf = NULL;
 	}
 
+	if (priv->ctxs) {
+		for (i = 0; i < priv->max_dev_ctx; i++) {
+			if (priv->ctxs[i]) {
+				devm_remove_action(dev,
+						   if_misc_deregister,
+						   &priv->ctxs[i]->miscdev);
+				misc_deregister(&priv->ctxs[i]->miscdev);
+			}
+		}
+	}
+
 	if (priv->flags & RESERVED_DMA_POOL) {
 		of_reserved_mem_device_release(dev);
 		priv->flags &= (~RESERVED_DMA_POOL);
@@ -302,6 +899,84 @@  static int se_probe_if_cleanup(struct platform_device *pdev)
 	return ret;
 }
 
+static int init_device_context(struct device *dev)
+{
+	const struct imx_se_node_info *info;
+	struct se_if_device_ctx *dev_ctx;
+	struct se_if_priv *priv;
+	u8 *devname;
+	int ret = 0;
+	int i;
+
+	priv = dev_get_drvdata(dev);
+
+	if (!priv) {
+		ret = -EINVAL;
+		dev_err(dev, "Invalid SE-MU Priv data");
+		return ret;
+	}
+	info = priv->info;
+
+	priv->ctxs = devm_kzalloc(dev, sizeof(dev_ctx) * priv->max_dev_ctx,
+				  GFP_KERNEL);
+
+	if (!priv->ctxs) {
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	/* Create users */
+	for (i = 0; i < priv->max_dev_ctx; i++) {
+		dev_ctx = devm_kzalloc(dev, sizeof(*dev_ctx), GFP_KERNEL);
+		if (!dev_ctx) {
+			ret = -ENOMEM;
+			return ret;
+		}
+
+		dev_ctx->dev = dev;
+		dev_ctx->status = SE_IF_CTX_FREE;
+		dev_ctx->priv = priv;
+
+		priv->ctxs[i] = dev_ctx;
+
+		/* Default value invalid for an header. */
+		init_waitqueue_head(&dev_ctx->wq);
+
+		INIT_LIST_HEAD(&dev_ctx->pending_out);
+		INIT_LIST_HEAD(&dev_ctx->pending_in);
+		sema_init(&dev_ctx->fops_lock, 1);
+
+		devname = devm_kasprintf(dev, GFP_KERNEL, "%s_ch%d",
+					 info->se_name, i);
+		if (!devname) {
+			ret = -ENOMEM;
+			return ret;
+		}
+
+		dev_ctx->miscdev.name = devname;
+		dev_ctx->miscdev.minor = MISC_DYNAMIC_MINOR;
+		dev_ctx->miscdev.fops = &se_if_fops;
+		dev_ctx->miscdev.parent = dev;
+		ret = misc_register(&dev_ctx->miscdev);
+		if (ret) {
+			dev_err(dev, "failed to register misc device %d\n",
+				ret);
+			return ret;
+		}
+
+		ret = devm_add_action(dev, if_misc_deregister,
+				      &dev_ctx->miscdev);
+		if (ret) {
+			dev_err(dev,
+				"failed[%d] to add action to the misc-dev\n",
+				ret);
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
 static void se_load_firmware(const struct firmware *fw, void *context)
 {
 	struct se_if_priv *priv = (struct se_if_priv *) context;
@@ -461,6 +1136,16 @@  static int se_if_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (info->max_dev_ctx) {
+		ret = init_device_context(dev);
+		if (ret) {
+			dev_err(dev,
+				"Failed[0x%x] to create device contexts.\n",
+				ret);
+			goto exit;
+		}
+	}
+
 	dev_info(dev, "i.MX secure-enclave: %s interface to firmware, configured.\n",
 		 info->se_name);
 	return devm_of_platform_populate(dev);
@@ -502,6 +1187,10 @@  static int se_resume(struct device *dev)
 	struct se_if_priv *priv = dev_get_drvdata(dev);
 	const struct imx_se_node_info *info
 					= priv->info;
+	int i;
+
+	for (i = 0; i < priv->max_dev_ctx; i++)
+		wake_up_interruptible(&priv->ctxs[i]->wq);
 
 	if (info && info->imem_mgmt)
 		se_restore_imem_state(dev);
diff --git a/drivers/firmware/imx/se_ctrl.h b/drivers/firmware/imx/se_ctrl.h
index 7d4f439a6158..41d9cedb05d7 100644
--- a/drivers/firmware/imx/se_ctrl.h
+++ b/drivers/firmware/imx/se_ctrl.h
@@ -13,15 +13,61 @@ 
 #define MAX_FW_LOAD_RETRIES		50
 
 #define RES_STATUS(x)			FIELD_GET(0x000000ff, x)
+#define MAX_DATA_SIZE_PER_USER		(65 * 1024)
 #define MESSAGING_VERSION_6		0x6
 #define MESSAGING_VERSION_7		0x7
 
+#define SE_MU_IO_FLAGS_USE_SEC_MEM	(0x02u)
+#define SE_MU_IO_FLAGS_USE_SHORT_ADDR	(0x04u)
+
 struct se_imem_buf {
 	u8 *buf;
 	phys_addr_t phyaddr;
 	u32 size;
 };
 
+struct se_buf_desc {
+	u8 *shared_buf_ptr;
+	u8 *usr_buf_ptr;
+	u32 size;
+	struct list_head link;
+};
+
+/* Status of a char device */
+enum se_if_dev_ctx_status_t {
+	SE_IF_CTX_FREE,
+	SE_IF_CTX_OPENED
+};
+
+struct se_shared_mem {
+	dma_addr_t dma_addr;
+	u32 size;
+	u32 pos;
+	u8 *ptr;
+};
+
+/* Private struct for each char device instance. */
+struct se_if_device_ctx {
+	struct device *dev;
+	struct se_if_priv *priv;
+	struct miscdevice miscdev;
+
+	enum se_if_dev_ctx_status_t status;
+	wait_queue_head_t wq;
+	struct semaphore fops_lock;
+
+	u32 pending_hdr;
+	struct list_head pending_in;
+	struct list_head pending_out;
+
+	struct se_shared_mem secure_mem;
+	struct se_shared_mem non_secure_mem;
+
+	u32 *temp_resp;
+	u32 temp_resp_size;
+	struct notifier_block se_notify;
+};
+
 /* Header of the messages exchange with the EdgeLock Enclave */
 struct se_msg_hdr {
 	u8 ver;
diff --git a/include/uapi/linux/se_ioctl.h b/include/uapi/linux/se_ioctl.h
new file mode 100644
index 000000000000..f68a36e9da2c
--- /dev/null
+++ b/include/uapi/linux/se_ioctl.h
@@ -0,0 +1,88 @@ 
+/* SPDX-License-Identifier: (GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause*/
+/*
+ * Copyright 2024 NXP
+ */
+
+#ifndef SE_IOCTL_H
+#define SE_IOCTL_H
+
+/* IOCTL definitions. */
+
+struct se_ioctl_setup_iobuf {
+	u8 *user_buf;
+	u32 length;
+	u32 flags;
+	u64 ele_addr;
+};
+
+struct se_ioctl_shared_mem_cfg {
+	u32 base_offset;
+	u32 size;
+};
+
+struct se_ioctl_get_if_info {
+	u8 se_if_id;
+	u8 interrupt_idx;
+	u8 tz;
+	u8 did;
+	u8 cmd_tag;
+	u8 rsp_tag;
+	u8 success_tag;
+	u8 base_api_ver;
+	u8 fw_api_ver;
+};
+
+struct se_ioctl_signed_message {
+	u8 *message;
+	u32 msg_size;
+	u32 error_code;
+};
+
+struct se_ioctl_get_soc_info {
+	u16 soc_id;
+	u16 soc_rev;
+};
+
+/* IO Buffer Flags */
+#define SE_IO_BUF_FLAGS_IS_OUTPUT	(0x00u)
+#define SE_IO_BUF_FLAGS_IS_INPUT	(0x01u)
+#define SE_IO_BUF_FLAGS_USE_SEC_MEM	(0x02u)
+#define SE_IO_BUF_FLAGS_USE_SHORT_ADDR	(0x04u)
+#define SE_IO_BUF_FLAGS_IS_IN_OUT	(0x10u)
+
+/* IOCTLS */
+#define SE_IOCTL			0x0A /* like MISC_MAJOR. */
+
+/*
+ * ioctl to designated the current fd as logical-reciever.
+ * This is ioctl is send when the nvm-daemon, a slave to the
+ * firmware is started by the user.
+ */
+#define SE_IOCTL_ENABLE_CMD_RCV	_IO(SE_IOCTL, 0x01)
+
+/*
+ * ioctl to get the buffer allocated from the memory, which is shared
+ * between kernel and FW.
+ * Post allocation, the kernel tagged the allocated memory with:
+ *  Output
+ *  Input
+ *  Input-Output
+ *  Short address
+ *  Secure-memory
+ */
+#define SE_IOCTL_SETUP_IOBUF	_IOWR(SE_IOCTL, 0x03, \
+					struct se_ioctl_setup_iobuf)
+
+/*
+ * ioctl to get the mu information, that is used to exchange message
+ * with FW, from user-spaced.
+ */
+#define SE_IOCTL_GET_MU_INFO	_IOR(SE_IOCTL, 0x04, \
+					struct se_ioctl_get_if_info)
+/*
+ * ioctl to get SoC Info from user-space.
+ */
+#define SE_IOCTL_GET_SOC_INFO      _IOR(SE_IOCTL, 0x06, \
+					struct se_ioctl_get_soc_info)
+
+#endif