On 06/12/17 12:30, Alex Williamson wrote: > On Wed, 6 Dec 2017 12:02:01 +1100 > Alexey Kardashevskiy <a...@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-sta...@nongnu.org # qemu-2.10+ >>> Signed-off-by: Alex Williamson <alex.william...@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 <a...@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 > > Laptops don't have many devices and we're not good about keeping up > with ACS quirks on laptop chipsets, so it might be difficult to find > the prerequisite setup there. Thanks,
Tried the laptop, these worked: 03:00.0 Network controller: Intel Corporation Centrino Advanced-N 6205 [Taylor Peak] (rev 34) 00:1a.0 USB controller: Intel Corporation 7 Series/C216 Chipset Family USB Enhanced Host Controller #2 (rev 04) However VGA did not. $ lspci -nns 00:02.0 00:02.0 VGA compatible controller [0300]: Intel Corporation 3rd Gen Core processor Graphics Controller [8086:0166] (rev 09) I run like this: pbuild/qemu-localhost-x86_64/x86_64-softmmu/qemu-system-x86_64 \ -enable-kvm -m 2G \ -netdev "tap,id=TAP0,helper=/home/aik/qemu-bridge-helper --br=br0" \ -device "virtio-net-pci,id=vnet0,mac=C0:41:49:4b:00:32,netdev=TAP0" \ virtimg/fc27-32GB.qcow2 -nodefaults \ -chardev stdio,id=STDIO0,signal=off,mux=on \ -device isa-serial,id=isa-serial0,chardev=STDIO0 \ -mon id=MON0,chardev=STDIO0,mode=readline -nographic -vga none \ -snapshot \ -device "vfio-pci,id=vfio0000_00_02_0,host=0000:00:02.0" and it crashes pretty soon, I suppose, as @pc does not change: (qemu) info cpus * CPU #0: pc=0x00000000000c5afa thread_id=4024 (qemu) info cpus * CPU #0: pc=0x00000000000c5afa thread_id=4024 and it does not seem to reach seabios or it does and seabios is initializing VGA - hard to tell, without any VGA - seabios prints messages to the console and shows grub. Is there any trick to try? Not big deal if none, just curious. Thanks. > > Alex > >>> hw/vfio/common.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >>> index 7b2924c0ef19..7007878e345e 100644 >>> --- a/hw/vfio/common.c >>> +++ b/hw/vfio/common.c >>> @@ -968,6 +968,7 @@ static int vfio_connect_container(VFIOGroup *group, >>> AddressSpace *as, >>> if (!ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &container->fd)) { >>> group->container = container; >>> QLIST_INSERT_HEAD(&container->group_list, group, >>> container_next); >>> + vfio_kvm_device_add_group(group); >>> return 0; >>> } >>> } >>> >> >> > -- Alexey