Re: [PATCH v42 5/7] firmware: imx: adds miscdev

[Frank Li]

On Tue, Aug 25, 2026 at 04:33:37AM +0530, pankaj.gupta@xxxxxxxxxxx wrote:
> From: Pankaj Gupta <pankaj.gupta@xxxxxxx>
>
> Adds the driver for communication interface to secure-enclave, that
> enables 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://github.com/nxp-imx/imx-secure-enclave.git,
> - i.MX Secure Middle-Ware:
>   -- URL: https://github.com/nxp-imx/imx-smw.git
>
> Following checks are performed on the incoming msg-header,
> to block exchanging invalid arbitrary commands:
> - maximum allowed words,
> - check if command-tag & response-tag are valid
> - version,
> - command id validation check, to allow limited base-line API(s)
>   and restrict following:
>   - exchanging power management commands.
>   - reset requests.
>   - BBSM configuration requests.
>   - re-initializing the FW.
>   - RNG init
>   - CAAM resource release management
>   - SE's internal memory management.
> from user-space.
>
> Signed-off-by: Pankaj Gupta <pankaj.gupta@xxxxxxx>
> ---
...
> +
> +int ele_uapi_allowed_fw_rsp(struct se_if_device_ctx *dev_ctx, struct se_msg_hdr *header,
> +			    u32 tx_msg_sz)
> +{
> +	struct se_api_msg *msg = container_of(header, struct se_api_msg, header);
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	bool is_cmd_receiver = false;
> +
> +	scoped_guard(mutex, &priv->modify_lock)
> +		if (dev_ctx == priv->cmd_receiver_clbk_hdl.dev_ctx)
> +			is_cmd_receiver = true;
> +
> +	if (!is_cmd_receiver)
> +		return -EINVAL;

is_cmd_receiver only use once

	if (dev_ctx != priv->cmd_receiver_clbk_hdl.dev_ctx)
		return -EINVAL;

so is_cmd_receiver can be removed.

> +
> +	return se_cmd_receiver_allowed_rsp(dev_ctx, msg, tx_msg_sz);
> +}
> +
...
> +int se_close_session(struct se_if_device_ctx *dev_ctx, u32 session_hdl)
> +{
> +	struct se_api_msg *tx_msg __free(kfree) = NULL;
> +	struct se_api_msg *rx_msg __free(kfree) = NULL;
> +	struct se_if_priv *priv;
> +	int ret;
> +
> +	if (!dev_ctx || !dev_ctx->priv)
> +		return -EINVAL;
> +
> +	priv = dev_ctx->priv;
> +
> +	tx_msg = kzalloc(ELE_SESSION_CLOSE_REQ_SZ, GFP_KERNEL);

cleanup prefer declear varible here

	struct se_api_msg *tx_msg __free(kfree)  =
		kzalloc(ELE_SESSION_CLOSE_REQ_SZ, GFP_KERNEL);

> +	if (!tx_msg)
> +		return -ENOMEM;
> +
> +	rx_msg = kzalloc(ELE_SESSION_CLOSE_RSP_SZ, GFP_KERNEL);
> +	if (!rx_msg)
> +		return -ENOMEM;

the same here. Check other place.

> +
> +	/*
> +	 * Session close is a FW-API command; format it with the FW API version
> +	 * so se_val_rsp_hdr_n_status() below (called with is_base_api = false,
> +	 * i.e. expecting fw_api_ver) does not reject the matching response and
> +	 * wrongly report the close as failed, which would leak the handle.
> +	 */
> +	se_fill_cmd_msg_hdr(priv, (struct se_msg_hdr *)&tx_msg->header,
> +			    ELE_SESSION_CLOSE_REQ, ELE_SESSION_CLOSE_REQ_SZ, false);
> +
> +	tx_msg->data[0] = session_hdl;
> +
> +	/*
> +	 * Transmit on the caller's own context. Using dev_ctx (rather than
> +	 * hardcoding priv->priv_dev_ctx) keeps a userspace close() subject to
> +	 * the going_away check in ele_msg_send_rcv(): if unbind has begun and
> +	 * freed priv->tx_chan, the send is rejected with -ENODEV instead of
> +	 * touching the freed mailbox channel. The teardown path passes
> +	 * priv_dev_ctx so its resync closes are still let through.
> +	 */
> +	ret = ele_msg_send_rcv(dev_ctx,
> +			       tx_msg,
> +			       ELE_SESSION_CLOSE_REQ_SZ,
> +			       rx_msg,
> +			       ELE_SESSION_CLOSE_RSP_SZ);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = se_val_rsp_hdr_n_status(priv,
> +				      rx_msg,
> +				      ELE_SESSION_CLOSE_REQ,
> +				      ELE_SESSION_CLOSE_RSP_SZ,
> +				      false);
> +	return ret;
> +}
> +
...
> +int se_get_mem_pool_buf(struct se_if_device_ctx *dev_ctx, void **buf,
> +			dma_addr_t *daddr, u32 len)
> +{
> +	struct se_shared_mem_mgmt_info *se_shared_mem_mgmt = &dev_ctx->se_shared_mem_mgmt;
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	struct se_buf_desc *b_desc = NULL;
> +
> +	lockdep_assert_held(&dev_ctx->fops_lock);
> +
> +	if (se_is_fw_busy_ctx(dev_ctx))
> +		return -EBUSY;
> +
> +	b_desc = kzalloc_obj(*b_desc, GFP_KERNEL);

GPF_KERNEL is default for *_obj alloc macro. Needn't set it

kzalloc_obj(*b_desc), check others.

Frank