Re: [PATCH v6 4/6] iommu/arm-smmu-v3: Standardize ATS enablement failure reporting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The error message drops the specific device context.
- [Low] State-mutating function call placed inside WARN() macro condition.
--

commit f858943c568bf1c4936c2371054b39b080e8ae6a
Author: Pranjal Shrivastava <praan@google.com>

iommu/arm-smmu-v3: Standardize ATS enablement failure reporting

This commit moves failable ATS configuration to the probe phase and
updates the commit phase to use WARN() for enablement failures. Since
hardware capabilities are verified earlier, failures here are considered
kernel bugs.

Link: https://lore.kernel.org/all/cover.1779392420.git.nicolinc@nvidia.com/

> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
[ ... ]
> @@ -2956,8 +2956,14 @@ static void arm_smmu_enable_ats(struct arm_smmu_master *master)
>  	 * ATC invalidation of PASID 0 causes the entire ATC to be flushed.
>  	 */
>  	arm_smmu_atc_inv_master(master, IOMMU_NO_PASID);
> -	if (pci_enable_ats(pdev, stu))
> -		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n", stu);
> +
> +	/*
> +	 * Any failure at this point is a kernel bug. pci_ats_supported()
> +	 * and pci_prepare_ats() have already verified the hardware capability
> +	 * and programmed the STU. Thus, pci_enable_ats() should not fail here.
> +	 */
> +	WARN(pci_enable_ats(pdev, stu),
> +	     "Failed to enable ATS (STU %zu)\n", stu);

[Severity: Low]
Does placing a state-mutating function like pci_enable_ats() directly inside
the WARN() macro condition hide essential hardware configuration logic?

While Linux's generic WARN() macro currently evaluates its condition exactly
once, placing this logic inside an error-reporting macro can degrade code
readability and break developer expectations. Could the function call be
extracted and its return value evaluated instead?

[Severity: Medium]
Does this change drop the specific device context from the error message?

The previous dev_err() call automatically prefixed the log message with the
specific device's identifier (like its PCI BDF address), which is crucial
for determining exactly which hardware component failed. By switching to a
generic WARN(), this device context is lost.

Would it be better to use dev_WARN(master->dev, ...) or explicitly include
dev_name(master->dev) in the format string to preserve observability for
debugging specific hardware failures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529111208.387412-1-praan@google.com?part=4