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

Jason Gunthorpe
2025-10-23T14:27:01Z

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