On Wed, Sep 04, 2024 at 04:21:16PM +0530, Pankaj Gupta wrote: Can you fix subject "[PATCH v7 0/5] Changes in v7"? I remember someone already complain this in previous version. Frank > 5/5 > - struct se_clbk_handle, is added with a member struct se_if_device_ctx *dev_ctx. > - func call to ele_miscdev_msg_rcv() & ele_miscdev_msg_send(), are removed. > - func se_ioctl_cmd_snd_rcv_rsp_handler(), is modified to remove the func call to ele_miscdev_msg_rcv() & ele_miscdev_msg_send() > - func se_ioctl_cmd_snd_rcv_rsp_handler is callig func ele_msg_send_rcv(), instead. > - Mutext "se_cmd_if_lock", handling is removed from this patch. > - func ele_miscdev_msg_send() is replaced with func ele_msg_send(), in fops_write. > - func ele_miscdev_msg_rcv() is replaced with func ele_msg_rcv(), in fops_read. > - fops_open is modified to create the new dev_ctx instance (using func init_device_context()), which is not registered as miscdev. > - Only one dev_ctx is registered as miscdev and its reference is stored in the struct se_if_priv, as priv_dev_ctx. > - Separate func cleanup_se_shared_mem() & func init_se_shared_mem(), for shared memory handling part of struct dev_ctx. > - Input param for func(s) ele_msg_rcv(), ele_msg_send() & ele_msg_send_rcv(), is replaced from struct se_if_priv to struct se_if_device_ctx. > > 4/5 > - A new structure is defined name struct "se_clbk_handle", to contain members processed in mailbox call-back function. > - "struct se_if_priv" is modified to contain the two structures of "se_clbk_handle" - waiting_rsp_clbk_hdl & cmd_receiver_clbk_hdl. > - func ele_msg_rcv() is modified to take a new additional input reference param "struct se_clbk_handle *se_clbk_hdl". > - func ele_msg_send() is modified to take a new additional input tx_msg_sz. > - func ele_msg_send_rcv(), is modified to take 2 more inputs - tx_msg_sz & exp_rx_msg_sz. > - func se_val_rsp_hdr_n_status(), is modified to take input of rx_msg buffer, instead of header value, as input param. > - each caller of the func ele_msg_send_rcv(), is sending these two additional input params. > - func se_if_callback(), is modified to work on two structures of "se_clbk_handle" - waiting_rsp_clbk_hdl & cmd_receiver_clbk_hdl. > - Variable "max_dev_ctx", is removed from info & priv struture, as well its usage. > - New member variable "se_img_file_to_load", is added to structure "priv". > - Other member variables - rx_msg(ptr), rx_msg_sz, completion done & list of dev_ctxs, is removed from priv struture, along with their usage. > - func se_resume(), updated to wakeup the two "wq", part of "struct se_clbk_handle": priv->waiting_rsp_clbk_hdl & priv->cmd_receiver_clbk_hdl. > > 3/5 > - Node name is changed from senclave-firmware@0 to "secure-enclave" > > 2/5 > - Node name is changed to "secure-enclave". > > - Link to v6: https://lore.kernel.org/r/20240722-imx-se-if-v6-0-ee26a87b824a@xxxxxxx > > v6: firmware: imx: driver for NXP secure-enclave > > 5/5 > - replaced scope_gaurd with gaurd. > > 4/5 > - replaced scope_gaurd with gaurd. > - remove reading the regs property from dtb. > - Added NULL check for priv data fetched from device, as a sanity check, for ele_base_msg apis) > > 3/5 > - replace firmware with senclave-firmware. > > 2/5 > - replace firmware with senclave-firmware. > - drop description for mbox > - Replaced "items:" with maxItems:1 for "memory-region" > - Replaced "items:" with maxItems:1 for "sram" > - remove regs property. > - remove "$nodename" > > - Link to v5: https://lore.kernel.org/r/20240712-imx-se-if-v5-0-66a79903a872@xxxxxxx > > Changes in v5: > > 2/5 > - updated the description of mboxes > - updated the description & items for mbox-names. > - updated the description of memory-region > - move "additional properties: false" after allOf block. > - removed other example except one. > > 4/5 > - Corrected the indentation in Kconfig. > - info members:mbox_tx_name & mbox_rx_name, are replaced with macros. > > 5/5 > - Replaced "for secure enclaves", with "for secure enclaves" > - Replaced "user space" with "userspace". > - End the line "[include]<linux/firmware/imx/ele_mu_ioctl.h>" with a period. > > - Link to v4: https://lore.kernel.org/r/20240705-imx-se-if-v4-0-52d000e18a1d@xxxxxxx > > Changes in v4: > > 1/5 > a. Removed - from EdgeLock Enclave. > > b. Removed , after "Each of the above feature," > > c. replace "can exists" with "can exist". > > d. > -messaging units(MU) per SE. Each co-existing 'se' can have one or multiple exclusive > -MU(s), dedicated to itself. None of the MU is shared between two SEs. > +messaging units(MU) per SE. Each co-existing SE can have one or multiple exclusive > +MUs, dedicated to itself. None of the MU is shared between two SEs. > Communication of the MU is realized using the Linux mailbox driver. > > e. > -All those SE interfaces 'se-if' that is/are dedicated to a particular SE, will be > -enumerated and provisioned under the very single 'SE' node. > +Although MU(s) is/are not shared between SE(s). But for SoC like i.MX95 which has > +multiple SE(s) like HSM, V2X-HSM, V2X-SHE; all the SE(s) and their interfaces 'se-if' > +that is/are dedicated to a particular SE will be enumerated and provisioned using the > +single compatible node("fsl,imx95-se"). > > f. Removed ",". Replaced for "Each 'se-if'," with "Each se-if'. > > g. removed "," > - This layer is responsible for ensuring the communication protocol, that is defined > + This layer is responsible for ensuring the communication protocol that is defined > > h. removed "-" > - - FW can handle one command-message at a time. > + - FW can handle one command message at a time. > > i. > - Using these multiple device contexts, that are getting multiplexed over a single MU, > - user-space application(s) can call fops like write/read to send the command-message, > - and read back the command-response-message to/from Firmware. > - fops like read & write uses the above defined service layer API(s) to communicate with > + Using these multiple device contexts that are getting multiplexed over a single MU, > + userspace application(s) can call fops like write/read to send the command message, > + and read back the command response message to/from Firmware. > + fops like read & write use the above defined service layer API(s) to communicate with > Firmware. > > j. Uppercase for word "Linux". > > 2/5 > a. Rephrased the description to remove list of phandles. > > b. Moved required before allOf: > +required: > + - compatible > + - reg > + - mboxes > + - mbox-names > + > +additionalProperties: false > + > allOf: > > c. replaced not: required: with properties: <property-name>: false. > # memory-region > - not: > - required: > - - memory-region > + properties: > + memory-region: false > > # sram > - else: > - not: > - required: > - - sram > > d. Reduced examples. keeping example of i.MX95. > e. node-name is changed to "firmware@<hex>" > > 3/5 > - node name changed to "firmware@<hex>". > > 4/5 > - used sizeof(*s_info) > - return early, rather than doing goto exit, in ele_get_info(). > - Use upper_32_bits() and lower_32_bits() > - use rx_msg here instead of priv->rx_msg > - Moved the status check to validate_rsp_hdr. Rename the function to "se_val_rsp_hdr_n_status" > - typecasting removed header = (struct se_msg_hdr *) msg; > - Converted the API name with prefix imx_ele_* or imx_se_*, to ele_* and se_*, respectively. > - Removed the functions definition & declaration for: free_phybuf_mem_pool() & get_phybuf_mem_pool() > - removed the mbox_free_channel() calls from clean-up. > - Flag "priv->flags" is removed. > - Converted the int se_if_probe_cleanup() to void se_if_probe_cleanup(). > - Replaced NULL initialization of structure members: priv->cmd_receiver_dev & priv->waiting_rsp_dev , with comments. > - Removed the function's declaration get_phy_buf_mem_pool1 > > 5/5 > Changes to Documentation/ABI/testing/se-cdev. > a. Removed "-" from "secure-enclave" and "file-descriptor". > > b. Removed "-" from "shared-library" > > c. Replaced "get" with "getting". > > d. Added description for the new IOCTL "send command and receive command response" > > e. Replaced "wakeup_intruptible" with "wait_event_interruptible" > > f. Removed ";" > > g. Removd "," from "mailbox_lock," > > h. Replaced "free" with "frees" > > i. In mailbox callback function, checking the buffer size before > copying. > > - Link to v3: https://lore.kernel.org/r/20240617-imx-se-if-v3-0-a7d28dea5c4a@xxxxxxx > > Communication Interface to NXP secure-enclave HW IP like Edgelock Enclave > > Hardware interface of the NXP Secure Enclave HW IP(s) like EdgeLock Enclave, > V2X, SHE etc, is based on the Messaging Unit module that enables processing > elements like ARMv8 core, RISC V core, within the SoC to communicate and > coordinate by passing messages (e.g., data, status and control) through > these interfaces. > > The NXP i.MX secure enclaves hardware interface kernel driver, is specifically > targeted for use between application core and NXP secure-enclave(s) HW. It allows > to send/receive messages to/from the secure-enclave. > > Patch-set adds the kernel 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. > > To: Jonathan Corbet <corbet@xxxxxxx> > To: Rob Herring <robh+dt@xxxxxxxxxx> > To: Conor Dooley <conor+dt@xxxxxxxxxx> > To: Shawn Guo <shawnguo@xxxxxxxxxx> > To: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > To: Pengutronix Kernel Team <kernel@xxxxxxxxxxxxxx> > To: Fabio Estevam <festevam@xxxxxxxxx> > To: Rob Herring <robh@xxxxxxxxxx> > Cc: linux-doc@xxxxxxxxxxxxxxx > Cc: linux-kernel@xxxxxxxxxxxxxxx > Cc: devicetree@xxxxxxxxxxxxxxx > Cc: imx@xxxxxxxxxxxxxxx > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Signed-off-by: Pankaj Gupta <pankaj.gupta@xxxxxxx> > > Changes in v3: > 5/5: > - Initialize tx_msg with NULL. > - memdup_user() returns an error pointer, not NULL. correct it by adding check for err_ptr. > - new IOCTL is added to send & recieve the message. > - replaced the while loop till list is empty, with list_for_each_entry. > - replaced __list_del_entry, with list_del. > - Removed the dev_err message from copy to user. > - Removed the casting of void *. > - corrected the typcasting in copy to user. > - removed un-necessary goto statement. > - Removed dead code for clean-up of memory. > - Removed un-mapping of secured memory > - Passing se_if_priv structure to init_device_context. > - Updated the below check to replace io.length with round_up(io.length). > if (shared_mem->size < shared_mem->pos|| io.length >= shared_mem->size - shared_mem->pos) > - Created a function to cleanup the list of shared memory buffers. > - Used list_for_each_entry_safe(). created a separate functions: se_dev_ctx_cpy_out_data() & se_dev_ctx_shared_mem_cleanup() > > 4/5 > - Changed the compatible string to replace "-ele", to "-se". > - Declaration of imx_se_node_info, is done as const in the whole file > - Remove the unused macros from ele_base_msg.h > - Remove the function declaration get_phy_buf_mem_pool1, from the header file. > - Replace the use of dmam_alloc_coherent to dma_alloc_coherent > - Check for function pointer, before calling the fucntion pointer in imx_fetch_se_soc_info > - Removed the unused flag for SE_MU_IO_FLAGS_USE_SEC_MEM. > - Removed the unused macros WORD_SZ > - instead of struct device *dev, struct se_if_priv *priv, is used as argument to the funtions:se_save_imem_state, se_restore_imem_state, imx_fetch_se_soc_info > - Removed ret from validate_rsp_hdr. > - changed the prefix of the funtion: plat_add_msg_crc and plat_fill_cmd_msg_hdr. > - indentation correction for info structures. > - remove the check for priv not null from se_if_probe_cleanup > - Removed the casting of void *. > - se_load_firmware function is corrected for not freeing the buffer when allocation fails. > - Checking if get_imx_se_node_info() can return NULL, in se_if_probe() > - imem.size has type u32. return value from se_save_imem_state() will be assigned to imem.size in case of success only. > - removed the flag un-setting in case of failure. priv->flags &= (~RESERVED_DMA_POOL); > - removed the function call for devm_of_platform_populate(dev); > - Checking for not-NULL, before calling the funtion pointer se_fetch_soc_info. > - Removed the checking for reserved memory flag, before freeing up the reserved memory, in se_probe_if_cleanup. > > 3/5 > - Changed the compatible string to replace "-ele", to "-se". > > 2/5 > - to fix the warning error, replaced the "-ele" & "-v2x" in compatible string, to "-se". > - Added an example for ele@0 for compatible string "fsl,imx95-se" > > Changes in v2: > > 4/4 > - Split this patch into two: 1. base driver & 2. Miscdev > - Initialize the return variable "err" as 0, before calling 'return err', in the file ele_common.c > - Fix the usage of un-iniitialized pointer variable, by initializing them with NULL, in ele_base_msg.c. > - Fix initializing the ret variable, to return the correct error code in case of issue. > - replaced dmam_alloc_coherent with dma_alloc_coherent. > - Replace the use of ELE_GET_INFO_READ_SZ, with sizeof(soc_info). > - Replaced -1 with -EPERM > - Removed the safety check on func-input param, in ele_get_info(). > - fix the assigning data[1] with lower 32 address, rather than zero, for ele_fw_authenticate API. > - Correctly initializing the function's return error code, for file ele_base_msg.c. > - replaced 'return' with 'goto'. > - Use length in bytes. > - Corrected the structure se_msg_hdr. > - Moved setting of rx_msg to priv, into the function imx_ele_msg_send_rcv > - Will add lockdep_assert_held, to receive path, in v2. > - corrected the spacing at "ret = validate_rsp_hdr" > - FIELD_GET() used for RES_STATUS > - Re-write the structure soc_info, matching the information provided in response to this api. > - The "|" goes to the end of the previous line. > - Moved the locking and unlocking of the command lock to the caller of the function. > - removed the safety check for device private data. > - Structure memory reference, used to read message header. > - In the interrupt call back function, remove assigning waiting_rsp_dev to NULL, in case of response message rcv from FW. > - do while removed. > - replaced BIT(1) for RESERVED_DMA_POOL, to BIT(0) > - The backslash is removed while assigning the file name with absolute path to structure variable.fw_name_in_rfs =. > - Update the 'if' condition by removing "idx < 0". > - mbox_request_channel_byname() uses a "char" for the name not a u8. Corrected. > - devm managed resources, are not cleaned now, in function se_probe_if_cleanup > - Used dev_err_probe(). > - Used %pe to print error string. > - remove "__maybe_unused" for "struct platform_device *enum_plat_dev __maybe_unused;" > - used FIELD_GET(), for RES_STATUS. Removed the use of MSG_TAG, MSG_COMMAND, MSG_SIZE, MSG_VER. > - Depricated the used of member of struct se_if_priv, bool no_dev_ctx_used; > - Moved the text explaing the synchronization logic via mutexes, from patch 1/4 to se_ctrl.h. > - removed the type casting of info_list = (struct imx_se_node_info_list *) device_get_match_data(dev->parent); > - Used static variable priv->soc_rev in the se_ctrl.c, replaced the following condition: if (info_list->soc_rev) to if (priv->soc_rev) for checking if this flow is already executed or not. > - imx_fetch_soc_info will return failure if the get_info function fails. > - Removed devm_free from imx_fetch_soc_info too. > > 3/3 > - Made changes to move all the properties to parent node, without any child node. > > 2/4 > - Use Hex pattern string. > - Move the properties to parent node, with no child node. > - Add i.MX95-ele to compatible nodes to fix the warning "/example-2/v2x: failed to match any schema with compatible: ['fsl,imx95-v2x']" > > 1/1 > - Corrected the spelling from creats to creates. > - drop the braces around the plural 's' for interfaces > - written se in upper case SE. > - Replace "multiple message(s)" with messages. > - Removed too much details about locks. > > Testing > - make CHECK_DTBS=y freescale/imx8ulp-evk.dtb; > - make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 dt_binding_check DT_SCHEMA_FILES=fsl,imx-se.yaml > - make C=1 CHECK=scripts/coccicheck drivers/firmware/imx/*.* W=1 > r.txt > - ./scripts/checkpatch.pl --git <>..HEAD > - Tested the Image and .dtb, on the i.MX8ULP. > > Reference > - Link to v1: https://lore.kernel.org/r/20240510-imx-se-if-v1-0-27c5a674916d@xxxxxxx > - Link to v2: https://lore.kernel.org/r/20240523-imx-se-if-v2-0-5a6fd189a539@xxxxxxx > > --- > Pankaj Gupta (5): > Documentation/firmware: add imx/se to other_interfaces > dt-bindings: arm: fsl: add imx-se-fw binding doc > arm64: dts: imx8ulp-evk: add nxp secure enclave firmware > firmware: imx: add driver for NXP EdgeLock Enclave > firmware: imx: adds miscdev > > Documentation/ABI/testing/se-cdev | 43 + > .../devicetree/bindings/firmware/fsl,imx-se.yaml | 91 ++ > .../driver-api/firmware/other_interfaces.rst | 121 ++ > arch/arm64/boot/dts/freescale/imx8ulp-evk.dts | 17 +- > arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 13 +- > drivers/firmware/imx/Kconfig | 12 + > drivers/firmware/imx/Makefile | 2 + > drivers/firmware/imx/ele_base_msg.c | 286 +++++ > drivers/firmware/imx/ele_base_msg.h | 95 ++ > drivers/firmware/imx/ele_common.c | 318 +++++ > drivers/firmware/imx/ele_common.h | 51 + > drivers/firmware/imx/se_ctrl.c | 1305 ++++++++++++++++++++ > drivers/firmware/imx/se_ctrl.h | 151 +++ > include/linux/firmware/imx/se_api.h | 14 + > include/uapi/linux/se_ioctl.h | 94 ++ > 15 files changed, 2610 insertions(+), 3 deletions(-) > --- > base-commit: b63ff26648537a5600cf79bd62f916792c53e015 > change-id: 20240507-imx-se-if-a40055093dc6 > > Best regards, > -- > Pankaj Gupta <pankaj.gupta@xxxxxxx> >
