Re: [PATCH v6 4/6] s390/pci: Use dma-iommu layer

Niklas Schnelle
2023-02-17T16:35:03Z

On Fri, 2023-02-17 at 09:56 -0500, Matthew Rosato wrote:
> On 2/17/23 3:51 AM, Niklas Schnelle wrote:
> > On Wed, 2023-02-15 at 13:00 -0500, Matthew Rosato wrote:
> > > On 2/15/23 7:03 AM, Niklas Schnelle wrote:
> > > > While s390 already has a standard IOMMU driver and previous changes have
> > > > added I/O TLB flushing operations this driver is currently only used for
> > > > user-space PCI access such as vfio-pci. For the DMA API s390 instead
> > > > utilizes its own implementation in arch/s390/pci/pci_dma.c which drives
> > > > the same hardware and shares some code but requires a complex and
> > > > fragile hand over between DMA API and IOMMU API use of a device and
> > > > despite code sharing still leads to significant duplication and
> > > > maintenance effort. Let's utilize the common code DMAP API
> > > > implementation from drivers/iommu/dma-iommu.c instead allowing us to
> > > > get rid of arch/s390/pci/pci_dma.c.
> > > >
> > > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > > ---
> > >
> > > FYI, this patch doesn't fit on top of iommu-next, I'd guess at least due to baolu's 'Retire detach_dev callback' series, which removed .detach_dev and added .set_platform_dma_ops for s390-iommu. That's relevant here, because now that this patch enables dma-iommu for s390 and removes the platform DMA ops it must now remove .set_platform_dma_ops/s390_iommu_set_platform_dma for s390-iommu.
> > >
> > > Matt
> >
> >
> > Ok, yes this series is currently against v6.2-rc8. Should I rebase
> > against iommu-next and send a v7 before further review or after?
> >
>
> So, overall I'm fine with the code in this patch (and this series) at this point; however I'd like to do one more pass of testing rebased on top of iommu-next / with the set_platform_dma collision handled, so I'm going to hold off on tagging my review of this one until v7. That really only impacts this patch so if you want to give others a chance to review the rest of the series before rolling out v7 that's OK by me.
>
> Thanks,
> Matt

Ok, I've got a version on top of iommu-next running locally and will
send that as v7 next week. Thankfully the conflict resolution is pretty
simple. We get rid of set_platform_dma() completely and the detach is
only used internally when attaching to a different domain. Also got rid
of a missed superfluous zdev->dma_table assignment that I missed as
feedback.

Thanks,
Niklas