Re: [PATCH v5 09/18] iommu/arm-smmu-v3: Introduce per-cmdq cmdq_err_handler callback

[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_device_disable()` sleeps while called from hardirq context in `arm_smmu_gerror_handler()`.
--

commit d1df4f7525ea4b8d14160727c6a78de7c62b7cbb
Author: Nicolin Chen <nicolinc@nvidia.com>

iommu/arm-smmu-v3: Introduce per-cmdq cmdq_err_handler callback

This patch adds a cmdq_err_handler and a paired cmdq_err_lock to struct
arm_smmu_cmdq. It updates arm_smmu_gerror_handler() to take the per-cmdq
cmdq_err_lock when acking CMDQ_ERR.

> 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 8a4edefeec770..c6e3d1be23403 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
[ ... ]
> @@ -2317,6 +2322,8 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
>  	if (active & GERROR_SFM_ERR) {
>  		/* SMMU is being disabled, so other errors don't matter */
>  		writel(gerror, smmu->base + ARM_SMMU_GERRORN);
> +		/* Release before arm_smmu_device_disable() that sleeps */
> +		raw_spin_unlock_irqrestore(&smmu->cmdq.cmdq_err_lock, flags);
>  		dev_err(smmu->dev, "device has entered Service Failure Mode!\n");
>  		arm_smmu_device_disable(smmu);

[Severity: High]
This is a pre-existing issue, but since the added comment explicitly mentions
that arm_smmu_device_disable() sleeps, can this cause a scheduling while
atomic bug?

Looking at the call chain, arm_smmu_gerror_handler() is registered as a
hardirq handler, and arm_smmu_device_disable() eventually calls:

arm_smmu_device_disable()
  arm_smmu_write_reg_sync()
    readl_relaxed_poll_timeout(..., 1, ARM_SMMU_POLL_TIMEOUT_US)

Because the polling timeout is called with a non-zero delay of 1 microsecond,
it expands to might_sleep_if() and usleep_range(), forcing the kernel to sleep.

While releasing the spinlock avoids a deadlock, does sleeping directly inside
the hardirq context still risk a panic when handling a Service Failure Mode
error?

>  		return IRQ_HANDLED;
>  	}

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