Re: [Xen-devel] PCI passthrough will cause unknown device appearance in device manager of Windows OS via xenstore keys

[Sander Eikelenboom <linux@xxxxxxxxxxxxxx>]

Monday, April 10, 2017, 4:44:47 PM, you wrote:

> On Mon, Apr 10, 2017 at 03:21:41PM +0100, Andrew Cooper wrote:
>> On 10/04/17 15:12, Konrad Rzeszutek Wilk wrote:
>> > On Mon, Apr 10, 2017 at 02:28:58PM +0100, Wei Liu wrote:
>> >> On Mon, Apr 10, 2017 at 01:46:20PM +0100, Andrew Cooper wrote:
>> >>> On 10/04/17 12:24, lidonglin wrote:
>> >>>> Hi all:
>> >>>>
>> >>>>                 I have one question about PCI passthrough. I found
>> >>>> that if I created a VM with host pci device(cfg file as below), there
>> >>>> were some xenstore keys exsiting in /local/domain/0 and
>> >>>> /local/domain/DomID/. Besides I found one unknown device in device
>> >>>> manager of Windows OS with Class ID FF80 from vendor ID 5853. My
>> >>>> questions  are as below:
>> >>>>
>> >>>>  
>> >>>>
>> >>>> 1.       Why do we create frontend and backend key pairs in xenstore?
>> >>>>  I think  Passthrough is not PV,  was it possible that we just used
>> >>>> these keys to record something?
>> >>>>
>> >>>> 2.        After review the code, I think backend keys are used to
>> >>>> record state of hostdev, some codes will query something using these
>> >>>> keys. But for frontend keys, can we delete them?
>> >>>>
>> >>>> 3.       if frontend keys exist in xenstore, then unknown device will
>> >>>> appear in device manager. Can we fix this?  For  an obsessive,  he
>> >>>> can’t stand for this.
>> >>>>
>> >>> This is a libxl bug which has been reported before, but nothing happened.
>> >>>
>> >>> libxl must not start a PV pci-back when doing HVM PCI passthrough using
>> >>> qemu, because absolutely nothing good can come of having two different
>> >>> ways of poking at PCI state.
>> >>>
>> >>> The patch, unchanged from before, is
>> >>> https://github.com/xenserver/xen-4.7.pg/blob/master/master/0001-libxl-Don-t-insert-PCI-device-into-xenstore-for-HVM-.patch
>> >>>
>> >> ISTR Konrad made some comments about it. CC Konrad.
>> > Aye. My primary beef was that you need some FLR mechanism and pciback
>> > does that. The 'do_flr' was the solution to that - as you could
>> > effectively do it via an ioctl and all would be good.
>> >
>> > Except I am probably missing some edge case (like guest is killed
>> > and we need to do an FLR again, or there are AERs to deal with, etc) so
>> > some pciback -> xenstore -> libxl needs to be in place even for HVM guests.
>> >
>> > Just disabling pciback for HVM does not solve the problem that:
>> >
>> >  - We need FLR
>> >  - We need AER support (CC-ing Venu who is working on this for 
>> > libxl/pciback)
>> 
>> The problem is that pciback is two distinct pieces of functionality.  It
>> should be split in half, so the "steal a device from its real driver and

> Yes sure.

>> provide reset functionality" is purposefully separate from "be the
>> reverse half the PV interface".

>> 
>> Qemu has no problems with resetting the device when necessary, though,

> It does? The sysfs 'reset' is does not do everything you expect.

I can concur, i use this "do_flr" patchset from Konrad for quite some time now.
Without it i can't shutdown and restart HVM guest which use (secondary) GPU 
passthrough (without rebooting the host), with this patchset that works fine!

With other PCI devices there seems to be less of a problem, but for what i have 
as a test case these all function with this patchset as well.

In the past the patchset was blocked from upstreaming to the kernel 
by David Vrabel on the naming.
Since "FLR" is an actual existing (but not widely spread i guess) hardware 
reset function 
from the PCI spec. This patchset does however do something that is more like a 
"bus 
reset" and is not limited to devices having the hardware FLR option. 

Since the libxl code that runs the sysfs do_flr thing is kind of dead code now,
perhaps it's best to rename it to something generic, so it could be extended in 
the future if support for more reset-methods will be bolted on here ?
(so writing some other value to the ioctl could specify which reset methods 
should be tried on this device) 

--
Sander

>> which is why this patch functions perfectly well in a real system.
>> 
>> ~Andrew




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel