Re: [Xen-devel] [PATCH 2/2] libxl_pci: Fix guest shutdown with PCI PT attached

[Anthony PERARD <anthony.perard@citrix.com>]

On Mon, Oct 14, 2019 at 11:03:43PM +0800, Chao Gao wrote:
> On Thu, Oct 10, 2019 at 06:13:43PM +0200, Sander Eikelenboom wrote:
> >Hi Anthony / Chao,
> >
> >I have to come back to this, a bit because perhaps there is an underlying issue.
> >While it earlier occurred to me that the VM to which I passed through most pci-devices 
> >(8 to be exact) became very slow to shutdown, but I  didn't investigate it further.
> >
> >But after you commit messages from this patch it kept nagging, so today I did some testing
> >and bisecting.
> >
> >The difference in tear-down time at least from what the IOMMU code logs is quite large:
> >
> >xen-4.12.0
> >	Setup: 	    7.452 s
> >	Tear-down:  7.626 s
> >
> >xen-unstable-ee7170822f1fc209f33feb47b268bab35541351d
> >	Setup:      7.468 s
> >	Tear-down: 50.239 s
> >
> >Bisection turned up:
> >	commit c4b1ef0f89aa6a74faa4618ce3efed1de246ec40
> >	Author: Chao Gao <chao.gao@intel.com>
> >	Date:   Fri Jul 19 10:24:08 2019 +0100
> >	libxl_qmp: wait for completion of device removal
> >
> >Which makes me wonder if there is something going wrong in Qemu ?
> 
> Hi Sander,
> 
> Thanks for your testing and the bisection.
> 
> I tried on my machine, the destruction time of a guest with 8 pass-thru
> devices increased from 4s to 12s after applied the commit above. In my
> understanding, I guess you might get the error message "timed out
> waiting for DM to remove...". There might be some issues on your assigned
> devices' drivers. You can first unbind the devices with their drivers in
> VM and then tear down the VM, and check whether the VM teardown gets
> much faster.

Hi,

Chao, I think you've tested `xl destroy`, and Sander, I think your are
speaking about `xl shutdown` or simply power off of a guest. Well, these
two operations are a bit different, on destroy the guest kernel is
still running when the pci devices are been removed, but on shutdown the
guest kernel is gone.

I don't think there's anything wrong with QEMU or with the devices, it
just that when the toolstack ask QEMU to unplug the pci device, QEMU
will ask the guest kernel first. So the guest may never acknowledge the
removal and QEMU will not let go of the pci device. There is actually an
old Xen commit about that:
77fea72b068d25afb7e63947aba32b487d7124a2, and a comment in the code:
    /* This depends on guest operating system acknowledging the
     * SCI, if it doesn't respond in time then we may wish to
     * force the removal. */


> Anthony & Wei,
> 
> The commit above basically serializes and synchronizes detaching
> assigned devices and thus increases VM teardown time significantly if
> there are multiple assigned devices. The commit aimed to avoid qemu's
> access to PCI configuration space coinciding with the device reset
> initiated by xl (which is not desired and is exactly the case which
> triggers the assertion in Xen [1]). I personally insist that xl should
> wait for DM's completion of device detaching. Otherwise, besides Xen
> panic (which can be fixed in another way), in theory, such sudden
> unawared device reset might cause a disaster (e.g. data loss for a
> storage device).
> 
> [1]: https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg03287.html
> 
> But considering fast creation and teardown is an important benefit of
> virtualization, I am not sure how to deal with the situation. Anyway,
> you can make the decision. To fix the regression on VM teardown, we can
> revert the commit by removing the timeout logic.
> 
> What's your opinion?

It probably a good idea to wait a bit until QEMU has detach the device.
For cases where QEMU will never detach the device (the guest kernel is
shutdown), we could reduce the timeout. Following my changes to pci
passthrough handling in libxl, the timeout is 10s for one device (and
probably 10s for many; I don't think libxl will even ask qemu to remove
the other devices if the first one timeout).

So, maybe we could wait for 5s for QEMU to detach the pci device? As
past that time, it will probably never happen. This still mean about +5s
to tear-down compare to previous releases. (Or maybe +5s per device if
we have to do one device at a time.)

There are other issues with handling multiple pci passthrough devices,
so I don't have patches yet.

Cheers,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel