diff mbox

DMAR faults from unrelated device when vfio is used

Message ID 1360190737.11144.731.camel@bling.home (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Williamson Feb. 6, 2013, 10:45 p.m. UTC 
On Wed, 2013-02-06 at 21:25 +0100, Richard Weinberger wrote:
> Hi,
> 
> Am Wed, 06 Feb 2013 11:47:20 -0700
> schrieb Alex Williamson <alex.williamson@redhat.com>: 
> > Does the card work with pci-assign or are both broken?
> 
> It works with pci-assign. :-\

When you tested this, did you detach the group from vfio or use it as
is?  In your previous message I see this:

03:00.0 USB controller [0c03]: NEC Corporation uPD720200 USB 3.0 Host Controller [1033:0194] (rev ff)

/sys/kernel/iommu_groups/7/devices:
total 0
lrwxrwxrwx 1 root root 0 Feb  4 10:29 0000:00:1c.0 -> ../../../../devices/pci0000:00/0000:00:1c.0
lrwxrwxrwx 1 root root 0 Feb  4 10:29 0000:00:1c.6 -> ../../../../devices/pci0000:00/0000:00:1c.6
lrwxrwxrwx 1 root root 0 Feb  4 10:29 0000:03:00.0 -> ../../../../devices/pci0000:00/0000:00:1c.6/0000:03:00.0

This seemed like a good card to have in my test cache, so I went and got
one and it works fine for me... but I've been playing with pcieport
because I don't think we're handling them correctly in vfio.

Can you provide lspci -vvv -s 1c.6 while the guest is running?  I'm
going to bet that

Control: I/O+ Mem+ BusMaster+

is not set, which it would have been if pci-assign was tested without
the group bound to vfio.  I think the solution is going to be something
around white-listing pcieport, which you can easily test with a kernel
patch like this:


Then you won't need to bind 1c.0 or 1c.6 to vfio-pci and hopefully
things will work.  The other problem you might hit is that the pciehp
service driver may also be bound to these slots and somehow deletes the
pci device and re-adds it when a device reset happens.  This causes all
sorts of badness.  The solution here is to unbind the child device from
pciehp, ie:

echo 0000:00:1c.0:pcie04 | sudo \
    tee /sys/bus/pci_express/drivers/pciehp/unbind
echo 0000:00:1c.6:pcie04 | sudo \
    tee /sys/bus/pci_express/drivers/pciehp/unbind

Hopefully combined that will make things work, please let me know.
Another option is to move the device to a slot where it isn't grouped
with the root port above it, assuming it's a plugin card.  Also if we
could determine that these root ports support PCI ACS but just don't
report it, we could change the grouping and avoid root ports grouped
with devices.

I'm still trying to formulate how to fix this long term, whether we
should whitelist pcieport and require userspace to do this kind of set
(need a hotplug stub driver?) or if vfio-pci needs to gain some basic
pcieport functionality that can enable the device and bind service
drivers we want (aer) and avoid ones we don't (pciehp).  Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Richard Weinberger Feb. 7, 2013, 10:23 p.m. UTC | #1 
Hi,

Am Wed, 06 Feb 2013 15:45:37 -0700
schrieb Alex Williamson <alex.williamson@redhat.com>:

> On Wed, 2013-02-06 at 21:25 +0100, Richard Weinberger wrote:
> > Hi,
> > 
> > Am Wed, 06 Feb 2013 11:47:20 -0700
> > schrieb Alex Williamson <alex.williamson@redhat.com>: 
> > > Does the card work with pci-assign or are both broken?
> > 
> > It works with pci-assign. :-\
> 
> When you tested this, did you detach the group from vfio or use it as
> is?  In your previous message I see this:

I've detached it.

> 03:00.0 USB controller [0c03]: NEC Corporation uPD720200 USB 3.0 Host
> Controller [1033:0194] (rev ff)
> 
> /sys/kernel/iommu_groups/7/devices:
> total 0
> lrwxrwxrwx 1 root root 0 Feb  4 10:29 0000:00:1c.0
> -> ../../../../devices/pci0000:00/0000:00:1c.0 lrwxrwxrwx 1 root root
> 0 Feb  4 10:29 0000:00:1c.6
> -> ../../../../devices/pci0000:00/0000:00:1c.6 lrwxrwxrwx 1 root root
> 0 Feb  4 10:29 0000:03:00.0
> -> ../../../../devices/pci0000:00/0000:00:1c.6/0000:03:00.0
> 
> This seemed like a good card to have in my test cache, so I went and
> got one and it works fine for me... but I've been playing with
> pcieport because I don't think we're handling them correctly in vfio.
> 
> Can you provide lspci -vvv -s 1c.6 while the guest is running?  I'm
> going to bet that
> 
> Control: I/O+ Mem+ BusMaster+

Do you want "lspci -vvv -s 1c.6" after attaching 1c.6 to vfio and not
using pci-assign?

> is not set, which it would have been if pci-assign was tested without
> the group bound to vfio.  I think the solution is going to be
> something around white-listing pcieport, which you can easily test
> with a kernel patch like this:
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 12c264d..48a97fb 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -442,7 +442,7 @@ static struct vfio_device
> *vfio_group_get_device(struct vfio
>   * a device.  It's not always practical to leave a device within a
> group
>   * driverless as it could get re-bound to something unsafe.
>   */
> -static const char * const vfio_driver_whitelist[] = { "pci-stub" };
> +static const char * const vfio_driver_whitelist[] = { "pci-stub",
> "pcieport" }; 
>  static bool vfio_whitelisted_driver(struct device_driver *drv)
>  {

If I whitelist pcieport USB3 works within the guests. :-)
Binding 1c.0 and 1c.6 is no longer needed.
Next week I'll run some more tests with USB3 devices.
 
Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Williamson Feb. 7, 2013, 10:49 p.m. UTC | #2 
On Thu, 2013-02-07 at 23:23 +0100, Richard Weinberger wrote:
> Hi,
> 
> Am Wed, 06 Feb 2013 15:45:37 -0700
> schrieb Alex Williamson <alex.williamson@redhat.com>:
> 
> > On Wed, 2013-02-06 at 21:25 +0100, Richard Weinberger wrote:
> > > Hi,
> > > 
> > > Am Wed, 06 Feb 2013 11:47:20 -0700
> > > schrieb Alex Williamson <alex.williamson@redhat.com>: 
> > > > Does the card work with pci-assign or are both broken?
> > > 
> > > It works with pci-assign. :-\
> > 
> > When you tested this, did you detach the group from vfio or use it as
> > is?  In your previous message I see this:
> 
> I've detached it.
> 
> > 03:00.0 USB controller [0c03]: NEC Corporation uPD720200 USB 3.0 Host
> > Controller [1033:0194] (rev ff)
> > 
> > /sys/kernel/iommu_groups/7/devices:
> > total 0
> > lrwxrwxrwx 1 root root 0 Feb  4 10:29 0000:00:1c.0
> > -> ../../../../devices/pci0000:00/0000:00:1c.0 lrwxrwxrwx 1 root root
> > 0 Feb  4 10:29 0000:00:1c.6
> > -> ../../../../devices/pci0000:00/0000:00:1c.6 lrwxrwxrwx 1 root root
> > 0 Feb  4 10:29 0000:03:00.0
> > -> ../../../../devices/pci0000:00/0000:00:1c.6/0000:03:00.0
> > 
> > This seemed like a good card to have in my test cache, so I went and
> > got one and it works fine for me... but I've been playing with
> > pcieport because I don't think we're handling them correctly in vfio.
> > 
> > Can you provide lspci -vvv -s 1c.6 while the guest is running?  I'm
> > going to bet that
> > 
> > Control: I/O+ Mem+ BusMaster+
> 
> Do you want "lspci -vvv -s 1c.6" after attaching 1c.6 to vfio and not
> using pci-assign?

Was looking for while attached to vfio with the guest running after xhci
has failed to attach to it, but it's not really necessary, I'm pretty
sure of the result given that it work when the root port is left alone.


> > is not set, which it would have been if pci-assign was tested without
> > the group bound to vfio.  I think the solution is going to be
> > something around white-listing pcieport, which you can easily test
> > with a kernel patch like this:
> > 
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > index 12c264d..48a97fb 100644
> > --- a/drivers/vfio/vfio.c
> > +++ b/drivers/vfio/vfio.c
> > @@ -442,7 +442,7 @@ static struct vfio_device
> > *vfio_group_get_device(struct vfio
> >   * a device.  It's not always practical to leave a device within a
> > group
> >   * driverless as it could get re-bound to something unsafe.
> >   */
> > -static const char * const vfio_driver_whitelist[] = { "pci-stub" };
> > +static const char * const vfio_driver_whitelist[] = { "pci-stub",
> > "pcieport" }; 
> >  static bool vfio_whitelisted_driver(struct device_driver *drv)
> >  {
> 
> If I whitelist pcieport USB3 works within the guests. :-)
> Binding 1c.0 and 1c.6 is no longer needed.
> Next week I'll run some more tests with USB3 devices.

Great!  Thanks for the test.  I assume you didn't need to do anything
with unbinding pciehp?

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Weinberger Feb. 7, 2013, 11:26 p.m. UTC | #3 
Am Thu, 07 Feb 2013 15:49:58 -0700
schrieb Alex Williamson <alex.williamson@redhat.com>:
> > If I whitelist pcieport USB3 works within the guests. :-)
> > Binding 1c.0 and 1c.6 is no longer needed.
> > Next week I'll run some more tests with USB3 devices.
> 
> Great!  Thanks for the test.  I assume you didn't need to do anything
> with unbinding pciehp?

Yeah, unbinding from pciehp was not needed.
Next week I'll have physical access to that box and be able run more
tests.
So far everything looks fine.

Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 12c264d..48a97fb 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -442,7 +442,7 @@  static struct vfio_device *vfio_group_get_device(struct vfio
  * a device.  It's not always practical to leave a device within a group
  * driverless as it could get re-bound to something unsafe.
  */
-static const char * const vfio_driver_whitelist[] = { "pci-stub" };
+static const char * const vfio_driver_whitelist[] = { "pci-stub", "pcieport" };
 
 static bool vfio_whitelisted_driver(struct device_driver *drv)
 {