Re: [EXTERNAL] [PATCH v6 5/5] firmware: imx: adds miscdev

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pankaj,

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=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=hqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W-WqLaM_utz&s=C67hc24yMATzUglvGvywzpn0Efjurb6sOLm2V_9VpsI&e=  <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsecure-2Denclave.git&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=hqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W-WqLaM_utz&s=C67hc24yMATzUglvGvywzpn0Efjurb6sOLm2V_9VpsI&e=>,
- i.MX Secure Middle-Ware:
    -- URL:https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsmw.git&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=hqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W-WqLaM_utz&s=NACAFfnEzGKFI7FlqdL4kxlt8PtxeXRorc3IWanqgtY&e=  <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsmw.git&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=hqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W-WqLaM_utz&s=NACAFfnEzGKFI7FlqdL4kxlt8PtxeXRorc3IWanqgtY&e=>

Signed-off-by: Pankaj Gupta <pankaj.gupta@xxxxxxx>
---
   Documentation/ABI/testing/se-cdev |  43 +++
   drivers/firmware/imx/ele_common.c | 192 ++++++++++-
   drivers/firmware/imx/ele_common.h |   4 +
   drivers/firmware/imx/se_ctrl.c    | 677 ++++++++++++++++++++++++++++++++++++++
   drivers/firmware/imx/se_ctrl.h    |  46 +++
   include/uapi/linux/se_ioctl.h     |  94 ++++++
   6 files changed, 1053 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/se-cdev b/Documentation/ABI/testing/se-cdev
new file mode 100644
index 000000000000..3451c909ccc4
--- /dev/null
+++ b/Documentation/ABI/testing/se-cdev
@@ -0,0 +1,43 @@
+What:		/dev/<se>_mu[0-9]+_ch[0-9]+
+Date:		May 2024
+KernelVersion:	6.8
+Contact:	linux-imx@xxxxxxx, pankaj.gupta@xxxxxxx
+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 userspace 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
+			- getting mu info
+			- setting a dev-ctx as receiver to receive all the commands from FW
+			- getting SoC info
+			- send command and receive command response
+
+		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
+		  wait_event_interruptible, 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 frees up the I/O contexts that were associated
+		  with the file descriptor.
+
+Users:		https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsecure-2Denclave.git&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=hqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W-WqLaM_utz&s=C67hc24yMATzUglvGvywzpn0Efjurb6sOLm2V_9VpsI&e=  <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsecure-2Denclave.git&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=hqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W-WqLaM_utz&s=C67hc24yMATzUglvGvywzpn0Efjurb6sOLm2V_9VpsI&e=>,
+		https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsmw.git&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=hqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W-WqLaM_utz&s=NACAFfnEzGKFI7FlqdL4kxlt8PtxeXRorc3IWanqgtY&e=  <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nxp-2Dimx_imx-2Dsmw.git&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A-JKU&m=hqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W-WqLaM_utz&s=NACAFfnEzGKFI7FlqdL4kxlt8PtxeXRorc3IWanqgtY&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 3a6584d6f6f2..8167ae201b83 100644
--- a/drivers/firmware/imx/ele_common.c
+++ b/drivers/firmware/imx/ele_common.c
@@ -78,6 +78,149 @@ int ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, void *rx_msg)
   	return err;
   }
+int ele_miscdev_msg_rcv(struct se_if_device_ctx *dev_ctx,
+			void *rx_buf,
+			int rx_buf_sz)
+{
+	struct se_msg_hdr *header;
+	int err;
+
+	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);
+		goto exit;
+	}
+
+	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->priv->waiting_rsp_dev != dev_ctx) {
+			dev_warn(dev_ctx->dev,
+			"Dev-ctx waiting for response mismatch (%s != %s).\n",
+			dev_ctx->miscdev.name, dev_ctx->priv->waiting_rsp_dev->miscdev.name);
+			err = -EPERM;
+			goto exit;
+		}
+	}
+
+	dev_dbg(dev_ctx->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.
+	 *
+	 * In case of US-command/response, the dev_ctx->temp_resp_size
+	 * is set before sending the command.
+	 *
+	 * In case of NVM Slave-command/response, the dev_ctx->temp_resp_size
+	 * is set after receing the message from mailbox.
+	 */
+	if (dev_ctx->temp_resp_size > rx_buf_sz) {
+		dev_err(dev_ctx->dev,
+			"%s: User buffer too small (%d < %d)\n",
+			dev_ctx->miscdev.name,
+			rx_buf_sz, dev_ctx->temp_resp_size);
+		 dev_ctx->temp_resp_size = rx_buf_sz;
+	}
+
+	/* We may need to copy the output data to user before
+	 * delivering the completion message.
+	 */
+	err = se_dev_ctx_cpy_out_data(dev_ctx, true);
+	if (err < 0)
+		goto exit;
+
+	/* Copy data from the buffer */
+	print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
+			     dev_ctx->temp_resp, dev_ctx->temp_resp_size, false);
+	if (copy_to_user(rx_buf, dev_ctx->temp_resp, dev_ctx->temp_resp_size)) {
+		dev_err(dev_ctx->dev,
+			"%s: Failed to copy to user\n",
+			dev_ctx->miscdev.name);
+		err = -EFAULT;
+		goto exit;
+	}
+
+	err = dev_ctx->temp_resp_size;
+exit:
+	if (err < 0)
+		se_dev_ctx_cpy_out_data(dev_ctx, false);
+
+	/* 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;
+	se_dev_ctx_shared_mem_cleanup(dev_ctx);
+
+	return err;
+}
+
+int 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;
+	u32 size_to_send;
+	int err;
+
+	header = (struct se_msg_hdr *) tx_msg;
+
+	/*
+	 * Check that the size passed as argument matches the size
+	 * carried in the message.
+	 */
+	size_to_send = header->size << 2;
+
+	if (size_to_send != tx_msg_sz) {
+		err = -EINVAL;
+		dev_err(priv->dev,
+			"%s: User buf hdr(0x%x) sz mismatced with input-sz (%d != %d).\n",
+			dev_ctx->miscdev.name, *(u32 *)header, size_to_send, tx_msg_sz);
+		goto exit;
+	}
+
+	/* Check the message is valid according to tags */
+	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 if (header->tag == priv->cmd_tag) {
+		if (priv->waiting_rsp_dev != dev_ctx) {
+			dev_err(priv->dev,
+				"%s: Channel not configured to send cmd to FW.",
+				dev_ctx->miscdev.name);
+			err = -EPERM;
+			goto exit;
+		}
+		lockdep_assert_held(&priv->se_if_cmd_lock);
+	} else {
+		dev_err(priv->dev,
+			"%s: The message does not have a valid TAG\n",
+			dev_ctx->miscdev.name);
+		err = -EINVAL;
+		goto exit;
+	}
+	err = ele_msg_send(priv, tx_msg);
+	if (err < 0)
+		goto exit;
+
+	err = size_to_send;
+exit:
+	return err;
+}
+
   static bool exception_for_size(struct se_if_priv *priv,
   				struct se_msg_hdr *header)
   {
@@ -99,6 +242,7 @@ static bool exception_for_size(struct se_if_priv *priv,
   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_if_priv *priv;
   	struct se_msg_hdr *header;
   	u32 rx_msg_sz;
@@ -114,8 +258,50 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
   	header = msg;
   	rx_msg_sz = header->size << 2;
- 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;
+		/* Pre-allocated buffer of MAX_NVM_MSG_LEN
+		 * as the NVM command are initiated by FW.
+		 * Size is revealed as part of this call function.
+		 */
+		if (rx_msg_sz > MAX_NVM_MSG_LEN) {
+			dev_err(dev,
+				"%s: Msg recvd hdr(0x%x) with greater[%d] than allocated buf-sz.\n",
+				dev_ctx->miscdev.name,
+				*(u32 *) header,
+				rx_msg_sz);
+		} else
+			memcpy(dev_ctx->temp_resp, msg, rx_msg_sz);

It is categorically stated (in the Linux kernel coding style guide) that this rule does not apply if only one branch of a conditional statement consists of a single statement. In such cases, you should categorically use braces for both branches of the conditional statement:

if (condition) {
        do_this();
        do_that();
} else {
        otherwise();
}

Also, made a similar comment on the earlier version (v5) as well:
https://patchwork.kernel.org/project/imx/patch/20240712-imx-se-if-v5-4-66a79903a872@xxxxxxx/

Thanks
-Amit






[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux