> > 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. Thanks for pointing it out. Will ensure not to repeat it. > > 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. > > > > 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> > >
