Re: [PATCH 1/3] libxl: attach xen-pciback only to PV domains

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4151 bytes --]

On Tue, Oct 25, 2016 at 09:10:02AM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Oct 20, 2016 at 12:42:33AM +0200, Marek Marczykowski-Górecki wrote:
> > On Wed, Oct 19, 2016 at 04:46:26PM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Wed, Oct 19, 2016 at 10:37:52AM +0100, Wei Liu wrote:
> > > > On Tue, Oct 18, 2016 at 03:53:31AM +0200, Marek Marczykowski-Górecki wrote:
> > > > > HVM domains use IOMMU and device model assistance for communicating with
> > > > > PCI devices, xen-pcifront/pciback is used only in PV domains.
> > > > 
> > > > This bit of description is in line with my understanding of how PCI
> > > > passthrough works.
> > > 
> > > Kind of. Pciback is also used to "own" the PCI devices. And in fact
> > > they do an important job of resetting the PCI device when the
> > > device is "bind" to pciback:
> > > 
> > > echo <Bdf> > bind
> > 
> > This part is still done.
> > 
> > > And .. this is the important part - when device changes ownership.
> > > That is when you disconnect it from one guest and assign to another.
> > > You need to reset the device in between. The code that calls
> > > the pci_reset_function is called by:
> > > 
> > > }                                                                               
> > >                                                                                 
> > > /*                                                                              
> > >  * Called when:                                                                 
> > >  *  - XenBus state has been reconfigure (pci unplug). See xen_pcibk_remove_device
> > >  *  - XenBus state has been disconnected (guest shutdown). See xen_pcibk_xenbus_remove
> > 
> > But this, in case of HVM without stubdomain, is not.
> > 
> > >  *  - 'echo BDF > unbind' on pciback module with no guest attached. See pcistub_remove
> > >  *  - 'echo BDF > unbind' with a guest still using it. See pcistub_remove       
> > >  *                                                                              
> > >  *  As such we have to be careful.                                              
> > >  *                                                                              
> > >  *  To make this easier, the caller has to hold the device lock.                
> > >  */                                                                             
> > > void pcistub_put_pci_dev(struct pci_dev *dev)
> > > 
> > > The first two are done when XenStore 'pci' entries are active - which
> > > this patch will remove and introduce a potential security problem.
> > > 
> > > Unless libxl does an 'unbind' followed by an 'bind'?
> > 
> > What about libxl__device_pci_reset, which is called (at least) before
> > attaching device to some domain, even after my patch and even if the
> > device is already bound to pciback. It tries to reset the device using
> > 'reset' entry in sysfs. I see this isn't available for some devices -
> > can pci_reset_function do any better?
> 
> My vague recollection was that it tried to do it but it aborted
> earlier due to holding locks (dev_lock is held when you do any
> operation on the SysFS). But I may be forgetting the details.
> 
> I need to look in the Linux code to confirm what the tricky part was.

Thanks. This is the last thing holding me from sending v2.

Anyway, if attaching xen-pciback to /something/ is needed, how should it
look? We have 3 cases:
1. PV - without qemu
2. HVM - with qemu in dom0
3. HVM - with qemu in stubdomain
And soon there will be 4th: PVH - without qemu

For 1 and 4 the device should be attached (in terms of xenstore) to the
target domain, as xen-pcifront (or equivalent) running there will be
used. BTW is that true for PVHv2?
For 3 - it should be attached to stubdomain (which is the case).
The question is what about 2 - should it be attached to the target domain,
even though it will not be used?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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