On Fri, May 10, 2024 at 06:57:30PM +0530, Pankaj Gupta wrote: > NXP hardware IP(s) for secure-enclaves like Edgelock Enclave(ELE), > are embedded in the SoC to support the features like HSM, SHE & V2X, > using message based communication interface. > > The secure enclave FW communicates on a dedicated messaging unit(MU) > based interface(s) with application core, where kernel is running. > It exists on specific i.MX processors. e.g. i.MX8ULP, i.MX93. > > This patch adds the driver for communication interface to secure-enclave, > for exchanging messages with NXP secure enclave HW IP(s) like EdgeLock Enclave, > both from: > - User-Space Applications via character driver. > - Kernel-space, used by kernel management layers like DM-Crypt. > > ABI documentation for the NXP secure-enclave driver. Several review comments inside, but stopping here. I just found v7 of this series. Could it be that you have resent an older version of this series instead of the new one?? Sascha > > 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 <pankaj.gupta@xxxxxxx> > --- > Documentation/ABI/testing/se-cdev | 42 ++ > drivers/firmware/imx/Kconfig | 12 + > drivers/firmware/imx/Makefile | 2 + > drivers/firmware/imx/ele_base_msg.c | 287 ++++++++ > drivers/firmware/imx/ele_base_msg.h | 70 ++ > drivers/firmware/imx/ele_common.c | 341 +++++++++ > drivers/firmware/imx/ele_common.h | 43 ++ > drivers/firmware/imx/se_ctrl.c | 1339 +++++++++++++++++++++++++++++++++++ > drivers/firmware/imx/se_ctrl.h | 151 ++++ > include/linux/firmware/imx/se_api.h | 14 + > include/uapi/linux/se_ioctl.h | 88 +++ > 11 files changed, 2389 insertions(+) > > 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: 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 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/Kconfig b/drivers/firmware/imx/Kconfig > index 183613f82a11..56bdca9bd917 100644 > --- a/drivers/firmware/imx/Kconfig > +++ b/drivers/firmware/imx/Kconfig > @@ -22,3 +22,15 @@ config IMX_SCU > > This driver manages the IPC interface between host CPU and the > SCU firmware running on M4. > + > +config IMX_SEC_ENCLAVE > + tristate "i.MX Embedded Secure Enclave - EdgeLock Enclave Firmware driver." > + depends on IMX_MBOX && ARCH_MXC && ARM64 > + default m if ARCH_MXC > + > + help > + It is possible to use APIs exposed by the iMX Secure Enclave HW IP called: > + - EdgeLock Enclave Firmware (for i.MX8ULP, i.MX93), > + like base, HSM, V2X & SHE using the SAB protocol via the shared Messaging > + Unit. This driver exposes these interfaces via a set of file descriptors > + allowing to configure shared memory, send and receive messages. > diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile > index 8f9f04a513a8..aa9033e0e9e3 100644 > --- a/drivers/firmware/imx/Makefile > +++ b/drivers/firmware/imx/Makefile > @@ -1,3 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-$(CONFIG_IMX_DSP) += imx-dsp.o > obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o > +sec_enclave-objs = se_ctrl.o ele_common.o ele_base_msg.o > +obj-${CONFIG_IMX_SEC_ENCLAVE} += sec_enclave.o > diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c > new file mode 100644 > index 000000000000..0463f26d93c7 > --- /dev/null > +++ b/drivers/firmware/imx/ele_base_msg.c > @@ -0,0 +1,287 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2024 NXP > + */ > + > +#include <linux/types.h> > +#include <linux/completion.h> > +#include <linux/dma-mapping.h> > + > +#include "ele_base_msg.h" > +#include "ele_common.h" > + > +int ele_get_info(struct device *dev, struct soc_info *s_info) > +{ > + struct se_if_priv *priv = dev_get_drvdata(dev); > + struct se_api_msg *tx_msg __free(kfree); > + struct se_api_msg *rx_msg __free(kfree); > + phys_addr_t get_info_addr; > + u32 *get_info_data; > + u32 status; > + int ret; > + > + if (!priv || !s_info) > + goto exit; ret and get_info_addr are used uninitialized when jumping to the exit label from here. > + > + memset(s_info, 0x0, sizeof(*s_info)); > + > + if (priv->mem_pool_name) > + get_info_data = get_phy_buf_mem_pool(dev, > + priv->mem_pool_name, > + &get_info_addr, > + ELE_GET_INFO_BUFF_SZ); > + else > + get_info_data = dmam_alloc_coherent(dev, > + ELE_GET_INFO_BUFF_SZ, > + &get_info_addr, > + GFP_KERNEL); > + if (!get_info_data) { > + ret = -ENOMEM; > + dev_err(dev, > + "%s: Failed to allocate get_info_addr.\n", > + __func__); > + goto exit; > + } > + > + tx_msg = kzalloc(ELE_GET_INFO_REQ_MSG_SZ << 2, GFP_KERNEL); > + if (!tx_msg) { > + ret = -ENOMEM; > + return ret; > + } > + > + rx_msg = kzalloc(ELE_GET_INFO_RSP_MSG_SZ << 2, GFP_KERNEL); > + if (!rx_msg) { > + ret = -ENOMEM; > + return ret; > + } > + > + ret = plat_fill_cmd_msg_hdr(priv, > + (struct se_msg_hdr *)&tx_msg->header, > + ELE_GET_INFO_REQ, > + ELE_GET_INFO_REQ_MSG_SZ, > + true); > + if (ret) > + goto exit; > + > + tx_msg->data[0] = upper_32_bits(get_info_addr); How can this work without triggering a NULL pointer exception? struct se_api_msg is declared as: struct se_api_msg { u32 header; /* u8 Tag; u8 Command; u8 Size; u8 Ver; */ u32 *data; }; The memory for tx_msg is kzalloced above, so *data is a NULL pointer. > + tx_msg->data[1] = lower_32_bits(get_info_addr); > + tx_msg->data[2] = ELE_GET_INFO_READ_SZ; > + priv->rx_msg = rx_msg; > + ret = imx_ele_msg_send_rcv(priv, tx_msg); > + if (ret < 0) > + goto exit; > + > + ret = validate_rsp_hdr(priv, > + priv->rx_msg->header, > + ELE_GET_INFO_REQ, > + ELE_GET_INFO_RSP_MSG_SZ, > + true); > + if (ret) > + goto exit; > + > + status = RES_STATUS(priv->rx_msg->data[0]); > + if (status != priv->success_tag) { > + dev_err(dev, "Command Id[%d], Response Failure = 0x%x", > + ELE_GET_INFO_REQ, status); > + ret = -1; Callers seem to expect an error code. Do you intend to return -EPERM here? > + } > + > + s_info->imem_state = (get_info_data[ELE_IMEM_STATE_WORD] > + & ELE_IMEM_STATE_MASK) >> 16; > + s_info->major_ver = (get_info_data[GET_INFO_SOC_INFO_WORD_OFFSET] > + & SOC_VER_MASK) >> 24; > + s_info->minor_ver = ((get_info_data[GET_INFO_SOC_INFO_WORD_OFFSET] > + & SOC_VER_MASK) >> 16) & 0xFF; > + s_info->soc_rev = (get_info_data[GET_INFO_SOC_INFO_WORD_OFFSET] > + & SOC_VER_MASK) >> 16; > + s_info->soc_id = get_info_data[GET_INFO_SOC_INFO_WORD_OFFSET] > + & SOC_ID_MASK; > + s_info->serial_num > + = (u64)get_info_data[GET_INFO_SL_NUM_MSB_WORD_OFF] << 32 > + | get_info_data[GET_INFO_SL_NUM_LSB_WORD_OFF]; > +exit: > + if (get_info_addr) { > + if (priv->mem_pool_name) > + free_phybuf_mem_pool(dev, priv->mem_pool_name, > + get_info_data, ELE_GET_INFO_BUFF_SZ); > + else > + dmam_free_coherent(dev, > + ELE_GET_INFO_BUFF_SZ, > + get_info_data, > + get_info_addr); > + } > + > + return ret; > +} > + > +int ele_ping(struct device *dev) > +{ > + struct se_if_priv *priv = dev_get_drvdata(dev); > + struct se_api_msg *tx_msg __free(kfree); > + struct se_api_msg *rx_msg __free(kfree); > + u32 status; > + int ret; > + > + tx_msg = kzalloc(ELE_PING_REQ_SZ << 2, GFP_KERNEL); > + if (!tx_msg) { > + ret = -ENOMEM; > + return ret; > + } > + > + rx_msg = kzalloc(ELE_PING_RSP_SZ << 2, GFP_KERNEL); > + if (!rx_msg) { > + ret = -ENOMEM; > + return ret; > + } > + > + ret = plat_fill_cmd_msg_hdr(priv, > + (struct se_msg_hdr *)&tx_msg->header, > + ELE_PING_REQ, ELE_PING_REQ_SZ, > + true); > + if (ret) { > + dev_err(dev, "Error: plat_fill_cmd_msg_hdr failed.\n"); > + goto exit; > + } > + > + priv->rx_msg = rx_msg; > + ret = imx_ele_msg_send_rcv(priv, tx_msg); > + if (ret) > + goto exit; > + > + ret = validate_rsp_hdr(priv, > + priv->rx_msg->header, > + ELE_PING_REQ, > + ELE_PING_RSP_SZ, > + true); > + if (ret) > + goto exit; > + > + status = RES_STATUS(priv->rx_msg->data[0]); > + if (status != priv->success_tag) { > + dev_err(dev, "Command Id[%d], Response Failure = 0x%x", > + ELE_PING_REQ, status); > + ret = -1; > + } > +exit: > + return ret; > +} > + > +int ele_service_swap(struct device *dev, > + phys_addr_t addr, > + u32 addr_size, u16 flag) > +{ > + struct se_if_priv *priv = dev_get_drvdata(dev); > + struct se_api_msg *tx_msg __free(kfree); > + struct se_api_msg *rx_msg __free(kfree); > + u32 status; > + int ret; > + > + tx_msg = kzalloc(ELE_SERVICE_SWAP_REQ_MSG_SZ << 2, GFP_KERNEL); > + if (!tx_msg) { > + ret = -ENOMEM; > + return ret; > + } > + > + rx_msg = kzalloc(ELE_SERVICE_SWAP_RSP_MSG_SZ << 2, GFP_KERNEL); > + if (!rx_msg) { > + ret = -ENOMEM; > + return ret; > + } > + > + ret = plat_fill_cmd_msg_hdr(priv, > + (struct se_msg_hdr *)&tx_msg->header, > + ELE_SERVICE_SWAP_REQ, > + ELE_SERVICE_SWAP_REQ_MSG_SZ, > + true); > + if (ret) > + goto exit; > + > + tx_msg->data[0] = flag; > + tx_msg->data[1] = addr_size; > + tx_msg->data[2] = ELE_NONE_VAL; > + tx_msg->data[3] = lower_32_bits(addr); > + tx_msg->data[4] = plat_add_msg_crc((uint32_t *)&tx_msg[0], > + ELE_SERVICE_SWAP_REQ_MSG_SZ); > + priv->rx_msg = rx_msg; > + ret = imx_ele_msg_send_rcv(priv, tx_msg); > + if (ret < 0) > + goto exit; > + > + ret = validate_rsp_hdr(priv, > + priv->rx_msg->header, > + ELE_SERVICE_SWAP_REQ, > + ELE_SERVICE_SWAP_RSP_MSG_SZ, > + true); > + if (ret) > + goto exit; > + > + status = RES_STATUS(priv->rx_msg->data[0]); > + if (status != priv->success_tag) { > + dev_err(dev, "Command Id[%d], Response Failure = 0x%x", > + ELE_SERVICE_SWAP_REQ, status); > + ret = -1; > + } else { > + if (flag == ELE_IMEM_EXPORT) > + ret = priv->rx_msg->data[1]; > + else > + ret = 0; > + } > +exit: > + > + return ret; > +} > + > +int ele_fw_authenticate(struct device *dev, phys_addr_t addr) > +{ > + struct se_if_priv *priv = dev_get_drvdata(dev); > + struct se_api_msg *tx_msg __free(kfree); > + struct se_api_msg *rx_msg __free(kfree); > + u32 status; > + int ret; > + > + tx_msg = kzalloc(ELE_FW_AUTH_REQ_SZ << 2, GFP_KERNEL); > + if (!tx_msg) { > + ret = -ENOMEM; > + return ret; > + } > + > + rx_msg = kzalloc(ELE_FW_AUTH_RSP_MSG_SZ << 2, GFP_KERNEL); > + if (!rx_msg) { > + ret = -ENOMEM; > + return ret; > + } > + ret = plat_fill_cmd_msg_hdr(priv, > + (struct se_msg_hdr *)&tx_msg->header, > + ELE_FW_AUTH_REQ, > + ELE_FW_AUTH_REQ_SZ, > + true); > + if (ret) > + goto exit; > + > + tx_msg->data[0] = addr; > + tx_msg->data[1] = 0x0; > + tx_msg->data[2] = addr; Has this been tested? According to the documentation data[0] shall contain the upper 32bit of the address and data[1] shall contain the lower 32bit of the address. There is no data[2] for this call. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
