> -----Original Message----- > From: Amit Singh Tomar <amitsinght@xxxxxxxxxxx> > Sent: Monday, July 22, 2024 5:08 PM > To: Pankaj Gupta <pankaj.gupta@xxxxxxx>; Jonathan Corbet > <corbet@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski > <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Shawn Guo > <shawnguo@xxxxxxxxxx>; Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; > Pengutronix Kernel Team <kernel@xxxxxxxxxxxxxx>; Fabio Estevam > <festevam@xxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx> > Cc: linux-doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx > Subject: [EXT] Re: [EXTERNAL] [PATCH v6 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 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > urldefense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps- > 3A__github.com_nxp-2D > > imx_imx-2Dsecure- > 2Denclave.git%26d%3DDwICaQ%26c%3DnKjWec2b6R0mOyPaz7xt > > fQ%26r%3DV_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A- > JKU%26m%3Dhqz6ztDhob0 > > jujItfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W- > WqLaM_utz%26s%3DC67hc24yMA > > > TzUglvGvywzpn0Efjurb6sOLm2V_9VpsI%26e%3D&data=05%7C02%7Cpanka > j.gupta%4 > > > 0nxp.com%7C5acd31c1cfc14661861c08dcaa42c232%7C686ea1d3bc2b4c6f > a92cd99c > > > 5c301635%7C0%7C0%7C638572450920546605%7CUnknown%7CTWFpbG > Zsb3d8eyJWIjoi > > > MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0 > %7C%7C%7 > > > C&sdata=cV67vBSDb5uPaABT8RDTmtNOtqePRAALqo7QuUaV4QQ%3D&rese > rved=0 > > <https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furl > > defense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps- > 3A__github.com_nxp-2Dimx > > _imx-2Dsecure- > 2Denclave.git%26d%3DDwICaQ%26c%3DnKjWec2b6R0mOyPaz7xtfQ% > > 26r%3DV_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A- > JKU%26m%3Dhqz6ztDhob0juj > > Itfaaf7PHhtqSHj4aoWie1-b4nAGXTUrSyBQtV9W- > WqLaM_utz%26s%3DC67hc24yMATzU > > > glvGvywzpn0Efjurb6sOLm2V_9VpsI%26e%3D&data=05%7C02%7Cpankaj.gu > pta%40nx > > > p.com%7C5acd31c1cfc14661861c08dcaa42c232%7C686ea1d3bc2b4c6fa92 > cd99c5c3 > > > 01635%7C0%7C0%7C638572450920558539%7CUnknown%7CTWFpbGZsb > 3d8eyJWIjoiMC4 > > > wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C > %7C%7C&s > > > data=yrSEmLSlxZrIZG%2Bk4J%2BbDyvzdEams5ux%2F8nKhQBLq74%3D&rese > rved=0>, > > - i.MX Secure Middle-Ware: > > -- > > > URL:https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > urldefense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps- > 3A__github.com_nxp-2D > > imx_imx- > 2Dsmw.git%26d%3DDwICaQ%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r%3D > V_GK > > 7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A- > JKU%26m%3Dhqz6ztDhob0jujItfaaf7PHh > > tqSHj4aoWie1-b4nAGXTUrSyBQtV9W- > WqLaM_utz%26s%3DNACAFfnEzGKFI7FlqdL4kxl > > > t8PtxeXRorc3IWanqgtY%26e%3D&data=05%7C02%7Cpankaj.gupta%40nxp. > com%7C5a > > > cd31c1cfc14661861c08dcaa42c232%7C686ea1d3bc2b4c6fa92cd99c5c3016 > 35%7C0% > > > 7C0%7C638572450920566218%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > C4wLjAwMDAiL > > > CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdat > a=vuTve > > lCrdOFlGPGGpwpx0YgA6So%2BRIPJQRSzOjfo2LM%3D&reserved=0 > > <https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furl > > defense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps- > 3A__github.com_nxp-2Dimx > > _imx- > 2Dsmw.git%26d%3DDwICaQ%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r%3D > V_GK7jR > > uCHDErm6txmgDK1-MbUihtnSQ3gPgB-A- > JKU%26m%3Dhqz6ztDhob0jujItfaaf7PHhtqS > > Hj4aoWie1-b4nAGXTUrSyBQtV9W- > WqLaM_utz%26s%3DNACAFfnEzGKFI7FlqdL4kxlt8P > > > txeXRorc3IWanqgtY%26e%3D&data=05%7C02%7Cpankaj.gupta%40nxp.co > m%7C5acd3 > > > 1c1cfc14661861c08dcaa42c232%7C686ea1d3bc2b4c6fa92cd99c5c301635 > %7C0%7C0 > > %7C638572450920572062%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC > 4wLjAwMDAiLCJQ > > > IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=E > DDL3DFE > > erSUqrBRGQchaAsN3L0H2nkkRw4AsoNBMqA%3D&reserved=0> > > > > 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://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%3DDwICaQ%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r% > 3DV_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A- > JKU%26m%3Dhqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1- > b4nAGXTUrSyBQtV9W- > WqLaM_utz%26s%3DC67hc24yMATzUglvGvywzpn0Efjurb6sOLm2V_9VpsI% > 26e%3D&data=05%7C02%7Cpankaj.gupta%40nxp.com%7C5acd31c1cfc146 > 61861c08dcaa42c232%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C > 0%7C638572450920577297%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4 > wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C > %7C%7C&sdata=FvSTP3MVF%2BSghK36fmJ8u%2FySJ80DP7VQjNYAytj9gws > %3D&reserved=0 > <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%3DDwICaQ%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r% > 3DV_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A- > JKU%26m%3Dhqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1- > b4nAGXTUrSyBQtV9W- > WqLaM_utz%26s%3DC67hc24yMATzUglvGvywzpn0Efjurb6sOLm2V_9VpsI% > 26e%3D&data=05%7C02%7Cpankaj.gupta%40nxp.com%7C5acd31c1cfc146 > 61861c08dcaa42c232%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C > 0%7C638572450920582198%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4 > wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C > %7C%7C&sdata=%2Fb342eydDBBpn451JY9h36udCBWaJmzMbMOQcJHWI% > 2BQ%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%3DDwICaQ%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r%3D > V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A- > JKU%26m%3Dhqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1- > b4nAGXTUrSyBQtV9W- > WqLaM_utz%26s%3DNACAFfnEzGKFI7FlqdL4kxlt8PtxeXRorc3IWanqgtY%26e > %3D&data=05%7C02%7Cpankaj.gupta%40nxp.com%7C5acd31c1cfc146618 > 61c08dcaa42c232%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > C638572450920586967%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C > %7C&sdata=sXtVRno2qOn6aGvIr2HpJrB0WbhexELpRMNQE8JPUxY%3D&res > erved=0 > <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%3DDwICaQ%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r%3D > V_GK7jRuCHDErm6txmgDK1-MbUihtnSQ3gPgB-A- > JKU%26m%3Dhqz6ztDhob0jujItfaaf7PHhtqSHj4aoWie1- > b4nAGXTUrSyBQtV9W- > WqLaM_utz%26s%3DNACAFfnEzGKFI7FlqdL4kxlt8PtxeXRorc3IWanqgtY%26e > %3D&data=05%7C02%7Cpankaj.gupta%40nxp.com%7C5acd31c1cfc146618 > 61c08dcaa42c232%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > C638572450920591699%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C > %7C&sdata=XVI6X7w5FBckyuDIklwBvWTvYQ69GJPvR%2FnIvgyWulU%3D&r > eserved=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 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(); > } Checkpatch.pl donot throw either warning or error, for this. Adding the braces to else, do not throw error or warning, either. Thus, will add this change. > > Also, made a similar comment on the earlier version (v5) as well: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch > work.kernel.org%2Fproject%2Fimx%2Fpatch%2F20240712-imx-se-if-v5-4- > 66a79903a872%40nxp.com%2F&data=05%7C02%7Cpankaj.gupta%40nxp.c > om%7C5acd31c1cfc14661861c08dcaa42c232%7C686ea1d3bc2b4c6fa92cd > 99c5c301635%7C0%7C0%7C638572450920596406%7CUnknown%7CTWF > pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX > VCI6Mn0%3D%7C0%7C%7C%7C&sdata=nnkWdELMd%2BaYqAcqFoeTaSOib > VnSoMBegxXsn8PMePc%3D&reserved=0 > > Thanks > -Amit >