Re: [Qemu-devel] [PATCH for-2.11] vfio: Fix vfio-kvm group registration

[Peter Xu <peterx@redhat.com>]

On Tue, Dec 05, 2017 at 06:30:39PM -0700, Alex Williamson wrote:
> On Wed, 6 Dec 2017 12:02:01 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
> > On 06/12/17 08:09, Alex Williamson wrote:
> > > Commit 8c37faa475f3 ("vfio-pci, ppc64/spapr: Reorder group-to-container
> > > attaching") moved registration of groups with the vfio-kvm device from
> > > vfio_get_group() to vfio_connect_container(), but it missed the case
> > > where a group is attached to an existing container and takes an early
> > > exit.  Perhaps this is a less common case on ppc64/spapr, but on x86
> > > (without viommu) all groups are connected to the same container and
> > > thus only the first group gets registered with the vfio-kvm device.
> > > This becomes a problem if we then hot-unplug the devices associated
> > > with that first group and we end up with KVM being misinformed about
> > > any vfio connections that might remain.  Fix by including the call to
> > > vfio_kvm_device_add_group() in this early exit path.
> > > 
> > > Fixes: 8c37faa475f3 ("vfio-pci, ppc64/spapr: Reorder group-to-container attaching")
> > > Cc: qemu-stable@nongnu.org # qemu-2.10+
> > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > ---
> > > 
> > > This bug also existed in QEMU 2.10, but I think the fix is sufficiently
> > > obvious (famous last words) to propose for 2.11 at this late date.  If
> > > the first group is hot unplugged then KVM may revert to code emulation
> > > that assumes no non-coherent DMA is present on some systems.  Also for
> > > KVMGT, if the vGPU is not the first device registered, then the
> > > notifier to enable linkages to KVM would not be called.  Please review.  
> > 
> > For what it is worth
> > 
> > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Thanks!
> 
> > Sorry for the breakage...
> > 
> > One question - how was this discovered? I'd love to set up a test
> > environment on my old thinkpad x230 if possible.
> 
> Assign two devices from separate iommu groups, hot unplug the first
> device, followed by the second device.  The second unplug will trigger:
> 
> qemu-kvm: Failed to remove group ## from KVM VFIO device: No such file or directory

I reproduced this with command line:

bin=x86_64-softmmu/qemu-system-x86_64  
$bin -machine q35,kernel-irqchip=split \                                       
     -enable-kvm -m 4G -nographic \    
     -monitor telnet::6666,server,nowait \                                     
     -device ioh3420,multifunction=on,bus=pcie.0,id=port0,chassis=0 \          
     -device ioh3420,bus=pcie.0,id=port1,chassis=1 \                           
     -netdev user,id=user.0,hostfwd=tcp::5555-:22 \                            
     -device e1000,netdev=user.0 \     
     -device vfio-pci,host=05:00.0,id=vfio0,bus=port0 \                        
     -device vfio-pci,host=05:00.1,id=vfio1,bus=port1 \                        
     /home/images/fedora-25.qcow2      

The patch fixes it, so:

Reviewed-by: Peter Xu <peterx@redhat.com>
Tested-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu