Re: [Qemu-devel] [PATCH qemu v16 01/19] vfio: Delay DMA address space listener release

[Alex Williamson]

On Wed, 25 May 2016 16:34:37 +1000
David Gibson <address@hidden> wrote:

> On Fri, May 13, 2016 at 04:24:53PM -0600, Alex Williamson wrote:
> > On Fri, 13 May 2016 17:16:48 +1000
> > Alexey Kardashevskiy <address@hidden> wrote:
> >   
> > > On 05/06/2016 08:39 AM, Alex Williamson wrote:  
> > > > On Wed,  4 May 2016 16:52:13 +1000
> > > > Alexey Kardashevskiy <address@hidden> wrote:
> > > >    
> > > >> This postpones VFIO container deinitialization to let region_del()
> > > >> callbacks (called via vfio_listener_release) do proper clean up
> > > >> while the group is still attached to the container.    
> > > >
> > > > Any mappings within the container should clean themselves up when the
> > > > container is deprivleged by removing the last group in the kernel. Is
> > > > the issue that that doesn't happen, which would be a spapr vfio kernel
> > > > bug, or that our QEMU side structures get all out of whack if we let
> > > > that happen?    
> > > 
> > > My mailbase got corrupted, missed that.
> > > 
> > > This is mostly for "[PATCH qemu v16 17/19] spapr_iommu, vfio, memory: 
> > > Notify IOMMU about starting/stopping being used by VFIO", I should have 
> > > put 
> > > 01/19 and 02/19 right before 17/19, sorry about that.  
> > 
> > Which I object to, it's just ridiculous to have vfio start/stop
> > callbacks in a set of generic iommu region ops.  
> 
> It's ugly, but I don't actually see a better way to do this (the
> general concept of having vfio start/stop callbacks, that is, not the
> specifics of the patches).
> 
> The fact is that how we implement the guest side IOMMU *does* need to
> change depending on whether VFIO devices are present or not. 

No, how the guest side iommu is implemented needs to change depending
on whether there's someone, anyone, in QEMU that cares about the iommu,
which can be determined by whether the iommu notifier has any clients.
Alexey has posted another patch that does this.

> That's
> due essentially to incompatibilities between a couple of kernel
> mechanisms.  Which in itself is ugly, but nonetheless real.
> 
> A (usually blank) vfio on/off callback in the guest side IOMMU ops
> seems like the least-bad way to handle this.

I disagree, we already call memory_region_register_iommu_notifier() to
indicate we care about the guest iommu, so the abstraction is already
there, there's absolutely no reason to make a vfio specific interface.
Thanks,

Alex