Re: [PATCH 9/9] iommu: remove deprecated iova_to_phys from domain_ops and io_pgtable_ops

[sashiko-bot@kernel.org]

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

New issues:
- [High] Unmigrated driver vsi-iommu.c causes a compilation error because `.iova_to_phys` was removed from `struct iommu_domain_ops`.

Pre-existing issues:
- [High] The hardirq handler `vsi_iommu_irq` calls `pm_runtime_resume_and_get()`, which sleeps waiting for the device to resume, causing a kernel panic.
--

commit 6b44c3a91e8a25d542a9e22178e54c5a5807b633
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>

iommu: remove deprecated iova_to_phys from domain_ops and io_pgtable_ops

This commit removes the deprecated iova_to_phys interface from both
struct iommu_domain_ops and struct io_pgtable_ops. It cleans up the fallback
logic in iommu_iova_to_phys_length() and removes wrapper functions across
various io-pgtable implementations.

[ ... ]

> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 29c7c042d400d..e95b7491b5aac 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -805,8 +804,6 @@ struct iommu_domain_ops {
>  	int (*cache_invalidate_user)(struct iommu_domain *domain,
>  				     struct iommu_user_data_array *array);
>  
> -	phys_addr_t (*iova_to_phys)(struct iommu_domain *domain,
> -				    dma_addr_t iova);

[Severity: High]
The commit message states that all drivers implement iova_to_phys_length and
have migrated, but was drivers/iommu/vsi-iommu.c missed?

It looks like this driver still initializes this member:

drivers/iommu/vsi-iommu.c:vsi_iommu_ops {
    .iova_to_phys = vsi_iommu_iova_to_phys,
}

Will removing this member cause a build failure for that driver?


[Severity: High]
This is a pre-existing issue, but while looking at drivers/iommu/vsi-iommu.c,
can the hardirq handler sleep?

The interrupt is registered as a shared hardirq:

drivers/iommu/vsi-iommu.c:vsi_iommu_probe() {
    ...
    err = devm_request_irq(iommu->dev, iommu->irq, vsi_iommu_irq,
                           IRQF_SHARED, dev_name(dev), iommu);
    ...
}

But the handler calls a sleeping function:

drivers/iommu/vsi-iommu.c:vsi_iommu_irq() {
    ...
    if (pm_runtime_resume_and_get(iommu->dev) < 0)
        return IRQ_NONE;
    ...
}

Since pm_runtime_resume_and_get() can block waiting for the device to
resume, will this trigger a system panic or might_sleep() warning when
another device sharing the IRQ line fires?

>  	phys_addr_t (*iova_to_phys_length)(struct iommu_domain *domain,
>  					    dma_addr_t iova,
>  					    size_t *mapped_length);

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