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

From: Jason Gunthorpe <jgg@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Christian Benvenuti <benve@cisco.com>,
	Heiko Stuebner <heiko@sntech.de>,
	iommu@lists.linux.dev, Jernej Skrabec <jernej.skrabec@gmail.com>,
	Joerg Roedel <joro@8bytes.org>, Leon Romanovsky <leon@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-arm-msm@vger.kernel.org,
	linux-mediatek@lists.infradead.org, linux-rdma@vger.kernel.org,
	linux-rockchip@lists.infradead.org, linux-sunxi@lists.linux.dev,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Nelson Escobar <neescoba@cisco.com>,
	Rob Clark <robin.clark@oss.qualcomm.com>,
	Samuel Holland <samuel@sholland.org>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	Chen-Yu Tsai <wens@csie.org>, Will Deacon <will@kernel.org>,
	Yong Wu <yong.wu@mediatek.com>,
	patches@lists.linux.dev
Subject: Re: [PATCH 2/3] iommu/amd: Don't call report_iommu_fault()
Date: Thu, 23 Oct 2025 11:26:57 -0300	[thread overview]
Message-ID: <20251023142657.GH262900@nvidia.com> (raw)
In-Reply-To: <579bdc4e-ab71-4120-8991-34400d4bbf8d@arm.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