On Thu, 2022-10-06 at 09:02 -0300, Jason Gunthorpe wrote: > 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 > Sounds good will do a v5 anyway to add the map_pages()/unmap_pages().
