Re: [PATCH v6 0/6] iommu/s390: Fixes related to attach and aperture handling

[Niklas Schnelle]

On Mon, 2022-10-10 at 15:45 -0300, Jason Gunthorpe wrote:
> On Mon, Oct 10, 2022 at 04:54:07PM +0200, Niklas Schnelle wrote:
> > On Fri, 2022-10-07 at 11:49 +0200, Niklas Schnelle wrote:
> > > Hi All,
> > > 
> > > This is v5 of a follow up to Matt's recent series[0] where he tackled
> > > a race that turned out to be outside of the s390 IOMMU driver itself as
> > > well as duplicate device attachments. After an internal discussion we came
> > > up with what I believe is a cleaner fix. Instead of actively checking for
> > > duplicates we instead detach from any previous domain on attach. From my
> > > cursory reading of the code this seems to be what the Intel IOMMU driver is
> > > doing as well.
> > > 
> > > Moreover we drop the attempt to re-attach the device to its previous IOMMU
> > > domain on failure. This was fragile, unlikely to help and unexpected for
> > > calling code. Thanks Jason for the suggestion.
> > > 
> > > We can also get rid of struct s390_domain_device entirely if we instead
> > > thread the list through the attached struct zpci_devs. This saves us from
> > > having to allocate during attach and gets rid of one level of indirection
> > > during IOMMU operations.
> > > 
> > > Additionally 3 more fixes have been added in v3 that weren't in v2 of this
> > > series. One is for a potential situation where the aperture of a domain
> > > could shrink and leave invalid translations. The next one fixes an off by
> > > one in checking validity of an IOVA and the last one fixes a wrong value
> > > for pgsize_bitmap.
> > > 
> > > In v4 we also add a patch changing to the map_pages()/unmap_pages()
> > > interface in order to prevent a performance regression due to the
> > > pgsize_bitmap change.
> > > 
> > > *Note*:
> > > This series is against the s390 features branch[1] which already contains
> > > the bus_next field removal that was part of v2.
> > > 
> > > It is also available as branch iommu_fixes_v6 with the GPG signed tag
> > > s390_iommu_fixes_v5 on my niks/linux.git on git.kernel.org[2].
> > > 
> > > *Open Question*:
> > > Which tree should this go via?
> > 
> > The conflicting commit that removed the bus_next field from struct
> > zpci_dev has now made it into Linus' tree via the s390 pull. So this
> > series now applies cleanly on mainline master. Still not sure though
> > which tree this would best go into.
> 
> Arguably it should go through Joerg's iommu tree since it is only in
> the iommu driver..
> 
> If you need it on a branch to share with the s390 tree then send Joerg
> a PR.
> 
> Jason

Ok makes sense, I don't think I need it on an extra branch and whatever
is easiest for Joerg is fine. I hope that since all but patch 6 are
fixes and that one is quite simple that this could maybe still go into
v6.1.

Not sure if Joerg is still waiting on some Acks or R-bs though. I did
remove yours on patches 1, 2,  and 3 as there were some changes since
you gave it. I don't think you gave one for patch 4 and patch 6 is new.

I plan on sending further IOMMU improvements and the DMA conversion
based on this but will just reference it and provide private branches
on git.kernel.org. I think those will target the next merge window at
the earliest so that should be fine.

Thanks,
Niklas