Re: [PATCH] vfio/type1: Cleanup remaining vaddr removal/update fragments

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 12 Dec 2022 19:08:57 -0400
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Mon, Dec 12, 2022 at 02:26:51PM -0700, Alex Williamson wrote:
> > On Mon, 12 Dec 2022 15:59:11 -0500
> > Steven Sistare <steven.sistare@oracle.com> wrote:
> >   
> > > On 12/12/2022 10:58 AM, Alex Williamson wrote:  
> > > > On Mon, 12 Dec 2022 09:17:54 -0400
> > > > Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > >     
> > > >> On Sat, Dec 10, 2022 at 09:14:06AM -0500, Steven Sistare wrote:
> > > >>    
> > > >>> Thank you for your thoughtful response.  Rather than debate the degree of
> > > >>> of vulnerability, I propose an alternate solution.  The technical crux of
> > > >>> the matter is support for mediated devices.        
> > > >>
> > > >> I'm not sure I'm convinced about that. It is easy to make problematic
> > > >> situations with mdevs, but that doesn't mean other cases don't exist
> > > >> too eg what happens if userspace suspends and then immediately does
> > > >> something to trigger a domain attachment? Doesn't it still deadlock
> > > >> the kernel?    
> > > > 
> > > > The opportunity for that to deadlock isn't obvious to me, a replay
> > > > would be stalled waiting for invalid vaddrs, but this is essentially
> > > > the user deadlocking themselves.  There's also code there to handle the
> > > > process getting killed while waiting, making it interruptible.  Thanks,    
> > > 
> > > I will submit new patches tomorrow to exclude mdevs.  Almost done.  
> > 
> > I've dropped the removal commits from my next branch in the interim.  
> 
> Woah, please don't do that - I already built and sent pull requests
> assuming this, there are conflicts.

I've done merges both ways with your iommufd pull request and don't see
any conflicts relative to these changes.  Kconfig, Makefile, and
vfio_main.c related to virq integration and group extraction are the
only conflicts.  Besides, it's already pushed and I don't have any
references to the old head, so someone would need to provide it if we
wanted to keep the old hashes.
 
> Why would we not revert everything from 6.2 - that is what we agreed
> to do?

The decision to revert was based on the current interface being buggy,
abandoned, and re-implemented.  It doesn't seem that there's much future
for the current interface, but Steve has stepped up to restrict the
current implementation to non-mdev devices, which resolves your concern
regarding unlimited user blocking of kernel threads afaict, and we'll
see what he does with locked memory.  If it looks ok, then I think it
reduces our urgency to remove it, and in particular, I think negates
our need to remove it from stable when we eventually do so anyway.
Thanks,

Alex