Re: [PATCH 2/3] iommu/amd: Don't call report_iommu_fault()

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Oct 23, 2025 at 01:34:21PM +0100, Robin Murphy wrote:
> On 2025-10-22 6:12 pm, Jason Gunthorpe wrote:
> > This old style API is only used by drivers/gpu/drm/msm and
> > drivers/remoteproc, neither are used on x86 HW. Remove the dead code to
> > discourage new users.
> 
> I'd be almost certain there's somebody somewhere using remoteproc on x86
> with some FPGA/bespoke PCI device/on-board MCU/etc. - whether they're doing
> it on AMD *and* care about its fault reporting mechanism is really the
> question.

Hmm!

Looking more closely

static int rproc_enable_iommu(struct rproc *rproc)
{
        struct iommu_domain *domain;
        struct device *dev = rproc->dev.parent;
        int ret;

        if (!rproc->has_iommu) {
                 ^^^^^^^^^^^^^
                dev_dbg(dev, "iommu not present\n");
                return 0;
        }

        iommu_set_fault_handler(domain, rproc_iommu_fault, rproc);

And then:

drivers/remoteproc/omap_remoteproc.c:   rproc->has_iommu = true;
drivers/remoteproc/qcom_q6v5_adsp.c:    .has_iommu = true,

config OMAP_REMOTEPROC
        tristate "OMAP remoteproc support"
        depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
        depends on OMAP_IOMMU

config QCOM_Q6V5_ADSP
        tristate "Qualcomm Technology Inc ADSP Peripheral Image Loader"
        depends on OF && ARCH_QCOM
        depends on QCOM_SMEM

So, I think it is safe. I will revise the commit message.

Seems like these drivers are definately used:

 drivers/iommu/arm/arm-smmu/arm-smmu.c    |  1 +
 drivers/iommu/arm/arm-smmu/qcom_iommu.c  |  1 +
 drivers/iommu/omap-iommu.c               |  1 +

But I wonder if these are all dead code too? Any thoughts?

 drivers/iommu/ipmmu-vmsa.c               |  1 +
 drivers/iommu/mtk_iommu.c                |  1 +
 drivers/iommu/mtk_iommu_v1.c             |  1 +
 drivers/iommu/rockchip-iommu.c           |  1 +
 drivers/iommu/sun50i-iommu.c             |  1 +

> >   	if (dev_data) {
> > -		/*
> > -		 * If this is a DMA fault (for which the I(nterrupt)
> > -		 * bit will be unset), allow report_iommu_fault() to
> > -		 * prevent logging it.
> > -		 */
> > -		if (IS_IOMMU_MEM_TRANSACTION(flags)) {
> > -			/* Device not attached to domain properly */
> > -			if (dev_data->domain == NULL) {
> > -				pr_err_ratelimited("Event logged [Device not attached to domain properly]\n");
> > -				pr_err_ratelimited("  device=%04x:%02x:%02x.%x domain=0x%04x\n",
> > -						   iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid),
> > -						   PCI_FUNC(devid), domain_id);
> > -				goto out;
> > -			}
> This part is unrelated to the report_iommu_fault() call - in fact it was
> specifically added even more recently.

Yeah, I'll fix it

Thanks,
Jason