Re: [Xen-devel] [PATCH v4] PCI back fixes for 3.17.

[Sander Eikelenboom]

Monday, July 14, 2014, 10:18:17 PM, you wrote:

> On Mon, Jul 14, 2014 at 10:16:18PM +0200, Sander Eikelenboom wrote:
>> 
>> Monday, July 14, 2014, 9:54:05 PM, you wrote:
>> 
>> > On Mon, Jul 14, 2014 at 09:01:29PM +0200, Sander Eikelenboom wrote:
>> >> 
>> >> Monday, July 14, 2014, 8:45:47 PM, you wrote:
>> >> 
>> >> > On Mon, Jul 14, 2014 at 08:24:33PM +0200, Sander Eikelenboom wrote:
>> >> >> 
>> >> >> Monday, July 14, 2014, 7:45:25 PM, you wrote:
>> >> >> 
>> >> >> > On Mon, Jul 14, 2014 at 07:43:04PM +0200, Sander Eikelenboom wrote:
>> >> >> >> 
>> >> >> >> Monday, July 14, 2014, 7:37:53 PM, you wrote:
>> >> >> >> 
>> >> >> >> >> >> Ad B)
>> >> >> >> >> >> 
>> >> >> >> >> >> root@dom0:~# xl pci-list router
>> >> >> >> >> >> Vdev Device
>> >> >> >> >> >> 05.0 0000:00:1b.0
>> >> >> >> >> >> 
>> >> >> >> >> >> root@dom0:~# xl pci-assignable-list
>> >> >> >> >> >> 0000:02:00.0
>> >> >> >> >> >> 
>> >> >> >> >> >> root@dom0:~# xl pci-detach router 00:1b.0
>> >> >> >> >> >> dmesg shows:
>> >> >> >> >> >> [  199.742668] pciback 0000:00:1b.0: restoring config space at offset 0x10 (was 0x4, writing 0xf7d30004)
>> >> >> >> >> >> [  199.743527] pciback 0000:00:1b.0: restoring config space at offset 0xc (was 0x0, writing 0x10)
>> >> >> >> >> >> [  199.744321] pciback 0000:00:1b.0: restoring config space at offset 0x4 (was 0x100000, writing 0x100006)
>> >> >> >> >> >> [  199.757184] xen-pciback pci-1-0: xen_pcibk_xenbus_remove freeing pdev @ 0xffff8800589fce40
>> >> >> >> >> >> [  199.758139] xen-pciback pci-1-0: xen_pcibk_disconnect pdev @ 0xffff8800589fce40
>> >> >> >> >> >> [  199.862595] xen: xen_unregister_device_domain_owner
>> >> >> >> >> >> 
>> >> >> >> >> >> xl dmesg shows:
>> >> >> >> >> >> (XEN) [2014-07-14 16:28:29] memory_map:remove: dom1 gfn=f3070 mfn=f7d30 nr=4
>> >> >> >> >> >> (XEN) [2014-07-14 16:28:29] io.c:322: d1: unbind: m_gsi=22 g_gsi=36 dev=00:00.5 intx=0
>> >> >> >> >> >> (XEN) [2014-07-14 16:28:29] io.c:390: d1 unmap: m_irq=22 dev=00:00.5 intx=0
>> >> >> >> >> >> (XEN) [2014-07-14 16:28:29] [VT-D]iommu.c:1579: d1:PCIe: unmap 0000:00:1b.0
>> >> >> >> >> >> (XEN) [2014-07-14 16:28:29] [VT-D]iommu.c:1440: d0:PCIe: map 0000:00:1b.0
>> >> >> >> >> >> 
>> >> >> >> >> >> root@dom0:~# xl pci-list router
>> >> >> >> >> >> root@dom0:~# xl pci-assignable-list
>> >> >> >> >> >> 0000:00:1b.0
>> >> >> >> >> >> 0000:02:00.0
>> >> >> >> >> >> 
>> >> >> >> >> >> root@dom0:~# xl pci-assignable-remove 00:1b.0
>> >> >> >> >> >> dmesg shows:
>> >> >> >> >> >> [  318.827415] xen: xen_unregister_device_domain_owner
>> >> >> >> >> >> [  318.828771] xen: xen_unregister_device_domain_owner: ENODEV
>> >> >> >> >> >> [  318.930869] pciback 0000:00:1b.0: restoring config space at offset 0x10 (was 0x4, writing 0xf7d30004)
>> >> >> >> >> >> [  318.933435] pciback 0000:00:1b.0: restoring config space at offset 0xc (was 0x0, writing 0x10)
>> >> >> >> >> >> [  318.935877] pciback 0000:00:1b.0: restoring config space at offset 0x4 (was 0x100000, writing 0x100006)
>> >> >> >> >> >> 
>> >> >> >> >> >> root@dom0:~# xl pci-list router
>> >> >> >> >> >> root@dom0:~# xl pci-assignable-list
>> >> >> >> >> >> 0000:02:00.0
>> >> >> >> >> >> 
>> >> >> >> >> >> 
>> >> >> >> >> 
>> >> >> >> >> > And if you do:
>> >> >> >> >> 
>> >> >> >> >> > # xl pci-detach router 02:00.0
>> >> >> >> >> 
>> >> >> >> 
>> >> >> >> > Err, I meant
>> >> >> >> > # xl pci-assignable-remove 02:00.0
>> >> >> >> 
>> >> >> >> Ah ok .. so that is:
>> >> >> >> remove a device from pciback that has never been assigned to any guest .. 
>> >> >> >> 
>> >> >> >> will also give that a go .. although that probably won't be a problem. 
>> >> >> 
>> >> >> > Right. So I think it all works as expected? That is if you have
>> >> >> > two PCI devices assigned to a guest and want to re-use them you
>> >> >> > have to do the 'pci-detach' twice and then follow that with
>> >> >> >  'pci-assignable-remove' twice as well?
>> >> >> 
>> >> >> Well except that's not what i want :-) .. that's about the same as shutting down 
>> >> >> the guest .. and indeed that works :-)
>> >> >> 
>> >> >> What i want to do is to remove just *one* device and leave the other one in the guest.
>> >> >> 
>> >> >> The one that i remove .. i would like to be able to remove it from being 
>> >> >> assignable .. (rebind it in dom0) and/or assign it to another guest.
>> >> 
>> >> > Aha
>> >> >> 
>> >> >> And that fails because pciback still thinks the guest owns the device ..
>> >> >> while libxl thinks it doesn't (outcome of xl pci-assignable-list).
>> >> 
>> >> > Oh
>> >> >> 
>> >> >> When doing the:
>> >> >> 
>> >> >> # xl pci-assignable-list
>> >> >> 0000:02:00.0
>> >> >> # xl pci-assignable-remove 02:00.0
>> >> >> dmesg shows:               
>> >> >> [  443.292951] xen: xen_unregister_device_domain_owner
>> >> >> [  443.294308] xen: xen_unregister_device_domain_owner: ENODEV
>> >> >> [  443.398827] pciback 0000:02:00.0: restoring config space at offset 0x3c (was 0x100, writing 0x104)
>> >> >> [  443.400797] pciback 0000:02:00.0: restoring config space at offset 0x10 (was 0x4, writing 0xf7c00004)
>> >> >> [  443.401713] pciback 0000:02:00.0: restoring config space at offset 0x4 (was 0x100000, writing 0x100006)
>> >> >> 
>> >> >> which i think is what is expected ..
>> >> 
>> >> > Right.
>> >> 
>> >> >> 
>> >> >> 
>> >> >> Hmm  /sys/bus/pci/drivers/pciback  doesn't shed much light, because in the state 
>> >> >> just after a xl pci-detach, the device is still registred by pciback (from dom0's 
>> >> >> view) which is correct, what is wrong is that pciback still seems to think that 
>> >> >> it is in turn still owned by the guest. That doesn't get reflected in the info 
>> >> >> available in the /sys dir.
>> >> 
>> >> > Oh
>> >> >> 
>> >> >> Ok over to xen-store ... 
>> >> 
>> >> > I think I had a patch for that in libxl. It was the order of operations it was
>> >> > doing - it did the 'unbind' first and then it did the XenBus operations.
>> >> 
>> >> >> 
>> >> >> first a diff between the booted guest with two devices passed through and the 
>> >> >> state after the "xl pci-detach router 00:1b.0"
>> >> >> 
>> >> >> root@creanuc:~# diff -U10 xs-before-detach-first.txt xs-after-detach-first.txt
>> >> >> --- xs-before-detach-first.txt  2014-07-14 19:56:23.233576000 +0200
>> >> >> +++ xs-after-detach-first.txt   2014-07-14 20:00:00.053576000 +0200
>> >> >> @@ -185,35 +185,29 @@
>> >> >>         feature-rx-flip = "0"
>> >> >>         feature-split-event-channels = "1"
>> >> >>         multi-queue-max-queues = "4"
>> >> >>         hotplug-status = "connected"
>> >> >>      pci = ""
>> >> >>       1 = ""
>> >> >>        0 = ""
>> >> >>         frontend = "/local/domain/1/device/pci/0"
>> >> >>         frontend-id = "1"
>> >> >>         online = "1"
>> >> >> -       state = "3"
>> >> >> +       state = "7"
>> >> >>         domain = "router"
>> >> >>         key-0 = "0000:02:00.0"
>> >> >>         dev-0 = "0000:02:00.0"
>> >> >>         vdevfn-0 = "28"
>> >> >>         opts-0 = "msitranslate=0,power_mgmt=0,permissive=0"
>> >> >>         state-0 = "3"
>> >> >> -       key-1 = "0000:00:1b.0"
>> >> >> -       dev-1 = "0000:00:1b.0"
>> >> >> -       vdevfn-1 = "30"
>> >> >> -       opts-1 = "msitranslate=0,power_mgmt=0,permissive=0"
>> >> >> -       state-1 = "3"
>> >> >> -       num_devs = "2"
>> >> >> +       num_devs = "1"
>> >> >>         vdev-0 = "0000:00:00.00"
>> >> >> -       vdev-1 = "0000:00:01.00"
>> >> >>         root-0 = "0000:00"
>> >> >>         root_num = "1"
>> >> >>    1 = ""
>> >> >>     vm = "/vm/a102d97e-8388-414e-97e5-cc394656c47d"
>> >> >>     name = "router"
>> >> >>     cpu = ""
>> >> >>      0 = ""
>> >> >>       availability = "online"
>> >> >>      1 = ""
>> >> >>       availability = "online"
>> >> >> 
>> >> >> It seems ok, perhaps apart from the general pci state going from 3 to 7 ?
>> >> >> Haven't looked up the exact states, but since only one domain is removes i would 
>> >> >> still expect it to be 3 ?
>> >> 
>> >> > 7 is Reconfiguring which is what it should move to (to update the XenBus).
>> >> 
>> >> Ok, but that should than be acknowledged somewhere by someone i suppose .. and 
>> >> not stay in that state ?
>> >> 
>> >> >> 
>> >> >> 
>> >> >> Now a diff before and after the "xl pci-assignable-remove 00:1b.0"
>> >> >> diff xs-after-detach-first.txt xs-after-pci-assignable-remove-first.txt is 
>> >> >> empty, as expected since this is a pciback only thing now involving any guest.
>> >> >> 
>> >> >> 
>> >> >> And now the last diff, between the state before and after the "xl pci-detach 
>> >> >> 02:00.0" removing the last pci device from the guest.
>> >> >> 
>> >> >> root@creanuc:~# diff -U5 xs-after-pci-assignable-remove-first.txt xs-after-pci-detach-second.txt
>> >> >> --- xs-after-pci-assignable-remove-first.txt    2014-07-14 20:00:45.161576000 +0200
>> >> >> +++ xs-after-pci-detach-second.txt      2014-07-14 20:01:20.177576000 +0200
>> >> >> @@ -184,27 +184,10 @@
>> >> >>         feature-rx-copy = "1"
>> >> >>         feature-rx-flip = "0"
>> >> >>         feature-split-event-channels = "1"
>> >> >>         multi-queue-max-queues = "4"
>> >> >>         hotplug-status = "connected"
>> >> >> -    pci = ""
>> >> >> -     1 = ""
>> >> >> -      0 = ""
>> >> >> -       frontend = "/local/domain/1/device/pci/0"
>> >> >> -       frontend-id = "1"
>> >> >> -       online = "1"
>> >> >> -       state = "7"
>> >> >> -       domain = "router"
>> >> >> -       key-0 = "0000:02:00.0"
>> >> >> -       dev-0 = "0000:02:00.0"
>> >> >> -       vdevfn-0 = "28"
>> >> >> -       opts-0 = "msitranslate=0,power_mgmt=0,permissive=0"
>> >> >> -       state-0 = "3"
>> >> >> -       num_devs = "1"
>> >> >> -       vdev-0 = "0000:00:00.00"
>> >> >> -       root-0 = "0000:00"
>> >> >> -       root_num = "1"
>> >> >>    1 = ""
>> >> >>     vm = "/vm/a102d97e-8388-414e-97e5-cc394656c47d"
>> >> >>     name = "router"
>> >> >>     cpu = ""
>> >> >>      0 = ""
>> >> >> @@ -249,15 +232,10 @@
>> >> >>        event-channel-rx = "25"
>> >> >>        request-rx-copy = "1"
>> >> >>        feature-rx-notify = "1"
>> >> >>        feature-sg = "1"
>> >> >>        feature-gso-tcpv4 = "1"
>> >> >> -    pci = ""
>> >> >> -     0 = ""
>> >> >> -      backend = "/local/domain/0/backend/pci/1/0"
>> >> >> -      backend-id = "0"
>> >> >> -      state = "1"
>> >> >>     control = ""
>> >> >>      shutdown = ""
>> >> >>      platform-feature-multiprocessor-suspend = "1"
>> >> >>      platform-feature-xs_reset_watches = "1"
>> >> >>     hvmloader = ""
>> >> >> 
>> >> >> That seems ok.
>> >> >> 
>> >> >> So apart from that perhaps incorrect general pci state change, the info in 
>> >> >> xenstore seems to be OK.
>> >> >> 
>> >> >> So it does seem to be a lack of pciback receiving a confirm from libxl / qemu that the 
>> >> >> device is indeed removed from the guest for the final deregistring of the 
>> >> >> ownership.
>> >> >> 
>> >> >> Because after a pci-detach the pci device is:
>> >> >> - removed from xen-store as attached to the guest
>> >> >> - the iommu has assigned/mapped it back to dom0
>> >> >> - xl pci-assignable-list shows it as assignable again
>> >> >> 
>> >> >> So only pciback is the only one left out of the party :-)
>> >> >> Couldn't it be somehow tied to the iommu events, since it should be in sync with that i suppose ?
>> >> 
>> >> 
>> >> > Let me find out what is up with this. Once the device
>> >> > is de-assigned from a guest (even if forcefully) it should
>> >> > be possible to assign it to another.
>> >> 
>> >> Yes .. at least for the rebinding to dom0 case, because the 
>> >> pci-assignable-remove seems to do it forcefully now. But i haven't tried what it 
>> >> does when reassigning to another guest (if it forcefully swaps ownership then, 
>> >> since the pci-assignable-remove isn't involved then).
>> >> 
>> >> Will give the xen patch a try.
>> 
>> > I found out what the problem is. It has been in the code for eons and 
>> > I think we never tested for it because the older toolstack (xend) would
>> > do things differently.
>> 
>> That .. and probably not many people testing .. 
>> Until now i used the automatic assign and deassign on guest creation
>> and shutdown which doesn't exhibit this problem.
>> 
>> > The issue here is:
>> 
>> >  1) xl pci-detach <dom-id> <BDF>
>> 
>> > 'xl' removes the whole device from the XenStore, so from:
>> >        domain = "USB"
>> 
>> >        vdevfn-0 = "28" 
>> >        opts-0 = "msitranslate=0,power_mgmt=0,permissive=0"
>> >        state-0 = "3"
>> >        vdev-0 = "0000:00:00.00"
>> >        root-0 = "0000:00"
>> >        key-0 = "0000:07:00.0"
>> >        dev-0 = "0000:07:00.0"
>> 
>> >        key-1 = "0000:04:00.0"
>> >        dev-1 = "0000:04:00.0"
>> >        vdevfn-1 = "30" 
>> >        opts-1 = "msitranslate=0,power_mgmt=0,permissive=0"
>> >        state-1 = "3"
>> >        num_devs = "2"
>> >        vdev-1 = "0000:00:01.00"
>> >        root_num = "1"
>> 
>> > it just yanks out the '*-1' values, so we have:
>> 
>> >        domain = "USB"
>> 
>> >        vdevfn-0 = "28" 
>> >        opts-0 = "msitranslate=0,power_mgmt=0,permissive=0"
>> >        state-0 = "3"
>> >        vdev-0 = "0000:00:00.00"
>> >        root-0 = "0000:00"
>> >        key-0 = "0000:07:00.0"
>> >        dev-0 = "0000:07:00.0"
>> 
>> >         num_devs = "1"
>> 
>> > While if the frontend was PV (so xen-pcifront) it would first but
>> > the state-1 to 5 (Closed) which would let the Xen pciback deallocate this.
>> 
>> > Anyhow, Xen pciback should be able to deal with this, but the patch
>> > is going to take a bit of time to figure out.
>> 
>> > Thank you for reporting it and helping me with the XenStore output to
>> > narrow down the culprit!
>> 
>> The thing i don't understand then is:
>> How is it different for the last (or all ) device(s) ?
>> (since the last device (or removing all devices) before doing the 
>> assignable-remove doesn't trigger the warning)
>> 
>> Or does the complete removal of the pci-part of the tree in xenstore trigger something ?

> Yup. It removes the whole 'pci' directory in XenStore which triggers
> in Xen pciback the code to deal with that.
So that essentially needs to be changed to watch the individual devices in the 
xenstore tree instead of the whole pci stuff.
Then it all makes sense :-)  

>> You know if you got a patch cooked up, i'm more than happy to test !

> Hehe!
>> 
>> --
>> Sander
>> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/