Re: [PATCH v4 1/5] iommu/s390: Fix duplicate domain attachments

From: Jason Gunthorpe
Date: Thu Oct 06 2022 - 08:02:31 EST


On Thu, Oct 06, 2022 at 01:52:44PM +0200, Niklas Schnelle wrote:

> One option I see would be to ignore the error return from
> zpci_register_ioat() if it indicates case 2b. Then we would still add
> the device to the IOMMU's devices list and return success despite
> knowing that the device is inaccessible (DMA and MMIO blocked).
>
> Then the recovery/reset code will register the new domain once the
> device comes out of the error state. At least from an IOMMU API point
> of view that would make the attachment always succeed for all
> zpci_register_ioat() error cases that aren't programming bugs and can
> conceivably be recovered from.

This is what I was thinking..

> If you agree I would propose adding this as a robustness improvement as
> part of my upcoming series of IOMMU improvements needed for the DMA API
> conversion. As stated above before the DMA API conversion any error
> that would cause zpci_register_ioat() to fail while the IOMMU API is
> being used will need a "power cycle" anyway so postponing this doesn't
> hurt.

Yes, I think this series is fine as is

Patch 4 mostly deletes all these error cases, and the one hunk that is left:

+ if (domain->geometry.aperture_start > zdev->end_dma ||
+ domain->geometry.aperture_end < zdev->start_dma)
+ return -EINVAL;

Is misplaced. If a device cannot be supported by the IOMMU, which is
what that is really saying since it only s390 creates one aperture
size, then it should fail to probe, not fail at attach.

So I'd change the above to a WARN_ON() for future safety and add a
similar test to probe and then all that is left is the
zpci_register_ioat() which you have a plan for.

Jason