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

[David Gibson]

On Wed, May 25, 2016 at 07:59:26AM -0600, Alex Williamson wrote:
> 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.

*thinks*  ah, yes, you're right of course.  So instead we need some
hook that's triggered on transition of number of notifier listeners
from zero<->non-zero.

> > 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
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature