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

[Sander Eikelenboom <linux@eikelenboom.it>]

On 18/10/2019 18:11, Anthony PERARD wrote:
> 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. */

Hi Anthony,

Correct I was referring to the behavior with "xl shutdown".

The above is interesting, my follow up question would be:
Does Qemu know / keep a state when the guest kernel is gone ?
Because if it does, we could make the need to ackknowledge removal be
conditional on that ?

--
Sander


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


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