Re: [PATCH v2 27/30] iommu/io-pgtable-arm: remove deprecated iova_to_phys wrapper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] In `arm_smmu_iova_to_phys_hard`, a hardware translation failure returns `0` instead of `PHYS_ADDR_MAX`. This breaks callers of `iommu_iova_to_phys_length` which expect `PHYS_ADDR_MAX` to detect failures, causing them to treat physical address `0` as a valid translation result.
--

commit 17d24f53e1c566320f87a73674b736d336e36024
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>

iommu/io-pgtable-arm: remove deprecated iova_to_phys wrapper

Remove the iova_to_phys wrapper function and .iova_to_phys assignment
from ARM LPAE io-pgtable, as all callers now use iova_to_phys_length.

> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 2b94e0563e214..ceea8da8d1fd7 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
[ ... ]
> @@ -999,7 +987,6 @@ arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg)
>  	data->iop.ops = (struct io_pgtable_ops) {
>  		.map_pages	= arm_lpae_map_pages,
>  		.unmap_pages	= arm_lpae_unmap_pages,
> -		.iova_to_phys	= arm_lpae_iova_to_phys,
>  		.iova_to_phys_length	= arm_lpae_iova_to_phys_length,

[Severity: Critical]
With the API migration to iova_to_phys_length now complete, does this introduce
a regression in error handling for drivers like arm-smmu?

The old iova_to_phys interface expected 0 on failure, but iova_to_phys_length
expects PHYS_ADDR_MAX. Looking at drivers/iommu/arm/arm-smmu/arm-smmu.c,
arm_smmu_iova_to_phys_length() can call arm_smmu_iova_to_phys_hard() which
still returns 0 instead of PHYS_ADDR_MAX on runtime PM get failure:

	ret = arm_smmu_rpm_get(smmu);
	if (ret < 0)
		return 0;

It also returns 0 on a hardware translation fault:

	if (phys & ARM_SMMU_CB_PAR_F) {
		dev_err(dev, "translation fault!\n");
		dev_err(dev, "PAR = 0x%llx\n", phys);
		goto out;
	}

	addr = (phys & GENMASK_ULL(39, 12)) | (iova & 0xfff);
out:
	arm_smmu_rpm_put(smmu);

	return addr;

Could callers like vfio_iommu_type1 now treat 0 as a valid physical address,
bypassing the WARN_ON(phys == PHYS_ADDR_MAX) check?

If iommu_unmap() unmaps the region, would VFIO use this incorrect physical
address 0 to unpin pages?

This appears to lead to an arbitrary page reference count decrement on PFN 0,
and could leak the reference count of the actual unmapped physical memory.

>  		.read_and_clear_dirty = arm_lpae_read_and_clear_dirty,
>  		.pgtable_walk	= arm_lpae_pgtable_walk,
>  	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602104637.1219810-1-guanghuifeng@linux.alibaba.com?part=27