Re: [PATCH v4 07/15] iommu/arm-smmu-v3: Submit CMDQ_OP_PRI_RESP for IOPF event

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] arm_smmu_page_response() incorrectly hardcodes streams[0].id, causing RESUME and PRI_RESP commands to target the wrong SID for multi-stream devices during error fallback paths, leading to permanent hardware stalls.
--

commit 7f733f885611e1969870b8cc107b40c240d5c27d
Author: Malak Marrid <mmarrid@nvidia.com>

iommu/arm-smmu-v3: Submit CMDQ_OP_PRI_RESP for IOPF event
    
This patch updates arm_smmu_page_response() to issue a CMDQ_OP_PRI_RESP for
IOMMU_FAULT_PAGE_REQ events from the PRI queue. It also extends
arm_smmu_enable_iopf() to allow PRI-enabled masters to attach fault-capable
domains.

> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 8b7d88d469198..815847c2d7b43 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

[ ... ]

> -static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused,
> +static void arm_smmu_page_response(struct device *dev, struct iopf_fault *evt,
>  				   struct iommu_page_response *resp)
>  {
>  	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> -	u8 resume_resp;
> +	struct arm_smmu_cmd cmd;
> +	int sid;
>  
> -	if (WARN_ON(!master->stall_enabled))
> +	if (WARN_ON_ONCE(evt->fault.type != IOMMU_FAULT_PAGE_REQ))
>  		return;
>  
> -	switch (resp->code) {
> -	case IOMMU_PAGE_RESP_INVALID:
> -	case IOMMU_PAGE_RESP_FAILURE:
> -		resume_resp = CMDQ_RESUME_0_RESP_ABORT;
> -		break;
> -	case IOMMU_PAGE_RESP_SUCCESS:
> -		resume_resp = CMDQ_RESUME_0_RESP_RETRY;
> -		break;
> -	default:
> -		resume_resp = CMDQ_RESUME_0_RESP_TERM;
> -		break;
> +	/* IOPF is gated to num_streams == 1 in arm_smmu_enable_iopf() */
> +	sid = master->streams[0].id;

[Severity: High]
This is a pre-existing issue, but does hardcoding streams[0].id here cause a
permanent hardware stall for multi-stream devices during error fallback paths?

Even though arm_smmu_enable_iopf() rejects enabling IOPF for devices with
num_streams != 1, a multi-stream device with dma-can-stall can still generate
a stall event (EVTQ).

When this happens, arm_smmu_handle_event() converts it to a fault and calls
iommu_report_device_fault(). Since there is no IOPF handler, the fallback
iopf_error_response() will call arm_smmu_page_response() with
IOMMU_PAGE_RESP_INVALID.

Because streams[0].id is unconditionally used here, the RESUME command will
target the wrong SID if the fault originated from a different stream on the
master. The stalled transaction will never receive the RESUME command, leaving
the hardware permanently stalled.

While PRI is disabled for multi-stream devices elsewhere, the STALL path
remains fully vulnerable, and this newly added comment propagates the false
assumption that IOPF gating prevents this scenario.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789081084.git.nicolinc@nvidia.com?part=7