Re: [PATCH v46 3/7] firmware: imx: add driver for NXP EdgeLock Enclave

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Deadlock in suspend/resume path due to GFP_KERNEL memory allocations in ele_get_info().
--

commit 05cadf4c7e4a0295c86129fe662754b87efdff7e
Author: Pankaj Gupta [off-list ref]

firmware: imx: add driver for NXP EdgeLock Enclave

Add MU-based communication interface for secure enclave. NXP hardware IP(s)
for secure-enclaves like Edgelock Enclave (ELE) are embedded in the SoC to
support features like HSM, SHE & V2X. The secure enclave FW communicates
with Linux over single or multiple dedicated messaging unit (MU) based
interface(s).

Closes: https://sashiko.dev/#/patchset/20260902-imx-se-if-v45-0-6e400879d8ec@nxp.com?part=3

[ ... ]

+int ele_get_info(struct se_if_priv *priv, struct ele_dev_info *s_info)
+{
+	dma_addr_t get_info_addr = 0;
+	void *get_info_data = NULL;
+	u32 get_info_len;
+	int ret;
+
+	if (!priv)
+		return -EINVAL;
+
+	memset(s_info, 0x0, sizeof(*s_info));
+
+	struct se_api_msg *tx_msg __free(kfree) =
+		kzalloc(ELE_GET_INFO_REQ_MSG_SZ, GFP_KERNEL);
+	if (!tx_msg)
+		return -ENOMEM;
+
+	struct se_api_msg *rx_msg __free(kfree) =
+		kzalloc(ELE_GET_INFO_RSP_MSG_SZ, GFP_KERNEL);
+	if (!rx_msg)
+		return -ENOMEM;
+
+	get_info_len = ELE_GET_INFO_BUFF_SZ;
+	if (priv->mem_pool)
+		get_info_data = gen_pool_dma_alloc(priv->mem_pool,
+						   get_info_len,
+						   &get_info_addr);
+	else
+		get_info_data = dma_alloc_coherent(priv->dev,
+						   get_info_len,
+						   &get_info_addr,
+						   GFP_KERNEL);

[Severity: High]
Can these GFP_KERNEL allocations cause a system deadlock during suspend?

The commit message notes that ele_service_swap() was updated to use GFP_NOIO 
because it runs in the suspend/resume path, and direct memory reclaim could 
trigger I/O to a suspended storage device. 

However, it appears ele_get_info() is also executed synchronously in this 
exact same suspend/resume path:

[ ... ]

+int se_save_imem_state(struct se_if_priv *priv, struct se_imem_buf *imem)
+{
+	struct ele_dev_info s_info = {0};
+	int ret;
+
+	ret = ele_get_info(priv, &s_info);
+	if (ret) {
+		dev_err(priv->dev, "Failed to get info from ELE.\n");
+		return ret;
+	}

Since se_suspend() unconditionally calls se_save_imem_state(), which in turn 
calls ele_get_info(), won't the GFP_KERNEL allocations for tx_msg, rx_msg, 
and get_info_data bypass the fix you made in ele_service_swap() and still 
risk the same I/O reclaim deadlock?

Should these allocations in ele_get_info() also be changed to use GFP_NOIO 
to ensure the entire suspend/resume chain avoids I/O triggers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-imx-se-if-v46-0-aefaab525034@nxp.com?part=3