Re: s390-iommu.c default domain conversion

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, May 20, 2022 at 03:05:46PM +0200, Niklas Schnelle wrote:

> I did some testing and created a prototype that gets rid of
> arch/s390/pci_dma.c and works soley via dma-iommu on top of our IOMMU
> driver. It looks like the existing dma-iommu code allows us to do this
> with relatively simple changes to the IOMMU driver only, mostly just
> implementing iotlb_sync(), iotlb_sync_map() and flush_iotlb_all() so
> that's great. They also do seem to map quite well to our RPCIT I/O TLB
> flush so that's great. For now the prototype still uses 4k pages only.

You are going to want to improve that page sizes in the iommu driver
anyhow for VFIO.
 
> With that the performance on the LPAR machine hypervisor (no paging) is
> on par with our existing code. On paging hypervisors (z/VM and KVM)
> i.e. with the hypervisor shadowing the I/O translation tables, it's
> still slower than our existing code and interestingly strict mode seems
> to be better than lazy here. One thing I haven't done yet is implement
> the map_pages() operation or adding larger page sizes. 

map_pages() speeds thiings up if there is contiguous memory, I'm not
sure what work load you are testing with so hard to guess if that is
interesting or not.

> Maybe you have some tips what you'd expect to be most beneficial?
> Either way we're optimistic this can be solved and this conversion
> will be a high ranking item on my backlog going forward.

I'm not really sure I understand the differences, do you have a sense
what is making it slower? Maybe there is some small feature that can
be added to the core code? It is very strange that strict is faster,
that should not be, strict requires synchronous flush in the unmap
cas, lazy does not. Are you sure you are getting the lazy flushes
enabled?

> I also stumbled over the following patch series which I think would
> also help our paging hypervisor cases a lot since it should alleviate
> the cost of shadowing short lived mappings:

This is quite different than what your current code does though?

Still, it seems encouraging

Jason