All of lore.kernel.org
 help / color / mirror / Atom feed
* xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
@ 2013-06-07 15:45 Konrad Rzeszutek Wilk
  2013-06-10 11:12 ` George Dunlap
  0 siblings, 1 reply; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-07 15:45 UTC (permalink / raw)
  To: xen-devel, jbeulich, george.dunlap

Something is odd. If I do the pci-detach in Xend 
(xm pci-detach latest 0000:01:00.0) the guest tells me:

[   20.866562] pcifront pci-0: Rescanning PCI Frontend Bus 0000:00
[   20.996602] pcifront pci-0: backend going away!
[   20.996824] pci_bus 0000:00: busn_res: [bus 00-ff] is released
[   20.997089] pcifront pci-0: Disconnecting PCI Frontend Buses
[   21.006171] pcifront pci-0: 22 freeing event channel 17

and I want to attach it back (xm pci-attach latest 0000:01:00.0):

# [   78.823076] pcifront pci-0: Installing PCI frontend
[   78.823332] pcifront pci-0: Creating PCI Frontend Bus 0000:00
[   78.823674] pcifront pci-0: PCI host bridge to bus 0000:00
[   78.823686] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
[   78.823697] pci_bus 0000:00: root bus resource [mem 0x00000000-0xfffffffff]
[   78.823703] pci_bus 0000:00: root bus resource [bus 00-ff]
[   78.823911] pci 0000:00:00.0: [8086:105e] type 00 class 0x020000
[   78.824234] pci 0000:00:00.0: reg 10: [mem 0xfe4a0000-0xfe4bffff]
[   78.824378] pci 0000:00:00.0: reg 14: [mem 0xfe480000-0xfe49ffff]
[   78.824489] pci 0000:00:00.0: reg 18: [io  0xe020-0xe03f]
[   78.827837] pcifront pci-0: claiming resource 0000:00:00.0/0
[   78.827845] pcifront pci-0: claiming resource 0000:00:00.0/1
[   78.827850] pcifront pci-0: claiming resource 0000:00:00.0/2
[   78.831779] e1000e: Intel(R) PRO/1000 Network Driver - 2.3.2-k
[   78.831790] e1000e: Copyright(c) 1999 - 2013 Intel Corporation.
[   78.831842] e1000e 0000:00:00.0: Disabling ASPM  L1
[   78.831937] e1000e 0000:00:00.0: enabling device (0000 -> 0002)
[   78.834698] xen_map_pirq_gsi: returning irq 34 for gsi 16
[   78.834707] e1000e 0000:00:00.0: Xen PCI mapped GSI16 to IRQ34
[   78.835330] e1000e 0000:00:00.0: Interrupt Throttling Rate (ints/sec) set to dynamic conservative mode
[   79.002430] e1000e 0000:00:00.0 eth0: (PCI Express:2.5GT/s:Width x4) 00:15:17:8f:18:a2
[   79.002441] e1000e 0000:00:00.0 eth0: Intel(R) PRO/1000 Network Connection
[   79.002521] e1000e 0000:00:00.0 eth0: MAC: 0, PHY: 4, PBA No: D50868-003
[   79.310912] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[   79.313442] device eth0 entered promiscuous mode
[   82.230982] e1000e: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: Rx/Tx
[   82.231133] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[   82.231225] switch: port 1(eth0) entered forwarding state
[   82.231242] switch: port 1(eth0) entered forwarding state

Doing this with xl I get:

15:42:23 # 57 :~/
> xl -f pci-detach latest 0000:01:00.0
libxl: error: libxl_pci.c:1231:do_pci_remove: xc_physdev_unmap_pirq irq=82
libxl: error: libxl_pci.c:1235:do_pci_remove: xc_domain_irq_permission irq=82


and in the guest:
# kill -1 1
# [   28.516427] switch: port 1(eth0) entered disabled state
[   28.516590] device eth0 left promiscuous mode
[   28.516599] switch: port 1(eth0) entered disabled state
[   28.547664] pcifront pci-0: Rescanning PCI Frontend Bus 0000:00
[   28.654770] pci_bus 0000:00: busn_res: [bus 00-ff] is released
[   28.654982] pcifront pci-0: 22 freeing event channel 17
[   28.655228] pcifront pci-0: failed to write error node for device/pci/0 (22 freeing event channel 17)


but this with Xen 4.3 that has at its tip:

commit 365c95f7de789e1dca03f119eab7dc61fe0f77c9
Author: Jan Beulich <jbeulich@suse.com>
Date:   Tue Jun 4 09:29:07 2013 +0200

    x86/xsave: properly check guest input to XSETBV

any thoughts? I don't know if this is a regression or not, but the
libxl errors are pointing me to the recent XSA issue - which I thought
was fixed?

Guest config:
extra="console=hvc0 debug "
kernel="/mnt/lab/latest/vmlinuz"
ramdisk="/mnt/lab/latest/initramfs.cpio.gz"
memory=1024
vcpus=2
name="latest"
on_crash="preserve"
vnc=1
vnclisten="0.0.0.0"
pci=["01:00.0"]
e820_host=1
disk=['phy:/dev/sdb2,xvda,w']

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-06-07 15:45 xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not) Konrad Rzeszutek Wilk
@ 2013-06-10 11:12 ` George Dunlap
  2013-06-10 11:15   ` Processed: " xen
  2013-06-10 13:20   ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-10 11:12 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Jan Beulich, xenbugs

create ^
title it xl pci-detach failure
thanks

On Fri, Jun 7, 2013 at 4:45 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> any thoughts? I don't know if this is a regression or not, but the
> libxl errors are pointing me to the recent XSA issue - which I thought
> was fixed?

I think the recent issue was with xend not working.  Did we fix send
and break xl?

In any case, this is a pretty important feature; I think we need to
sort it out before release, so I'm giving it a bug id to track.

Is "pci permissive" set in the global xl.conf file?

Have you tried this with say, a stock Debian Wheezy kernel?

 -George

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-06-10 11:12 ` George Dunlap
@ 2013-06-10 11:15   ` xen
  2013-06-10 13:20   ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 36+ messages in thread
From: xen @ 2013-06-10 11:15 UTC (permalink / raw)
  To: George Dunlap, xen-devel

Processing commands for xen@bugs.xenproject.org:

> create ^
Created new bug #12 rooted at `<20130607154553.GC24882@phenom.dumpdata.com>'
Title: `Re: [Xen-devel] xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)'
> title it xl pci-detach failure
Set title for #12 to `xl pci-detach failure'
> thanks
Finished processing.

Modified/created Bugs:
 - 12: http://bugs.xenproject.org/xen/bug/12 (new)

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-06-10 11:12 ` George Dunlap
  2013-06-10 11:15   ` Processed: " xen
@ 2013-06-10 13:20   ` Konrad Rzeszutek Wilk
  2013-06-10 13:28     ` George Dunlap
  2013-06-10 13:30     ` Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not) xen
  1 sibling, 2 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-10 13:20 UTC (permalink / raw)
  To: George Dunlap; +Cc: xen-devel, Jan Beulich, xenbugs

On Mon, Jun 10, 2013 at 12:12:47PM +0100, George Dunlap wrote:
> create ^
> title it xl pci-detach failure
> thanks
> 
> On Fri, Jun 7, 2013 at 4:45 PM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> > any thoughts? I don't know if this is a regression or not, but the
> > libxl errors are pointing me to the recent XSA issue - which I thought
> > was fixed?
> 
> I think the recent issue was with xend not working.  Did we fix send
> and break xl?

Could be. It might also be that that 'xl' never did it the same way
as 'xend' (meaning this might be a XenBus teardown change).

Note also that this is PV guests - and I think most of the testing
had been with the HVM guests with PCI passthrough. So it might be a
seperate issue altogether. Or that nobody tried doing PCI plug/unplug
in the past :-(

> 
> In any case, this is a pretty important feature; I think we need to
> sort it out before release, so I'm giving it a bug id to track.
> 
> Is "pci permissive" set in the global xl.conf file?

No. Let me of course try that.
> 
> Have you tried this with say, a stock Debian Wheezy kernel?

No. A v3.10-rc4 with Xen 4.3 latest.

Is Debian Wheezy a 2.6.32 kernel?

> 
>  -George
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-06-10 13:20   ` Konrad Rzeszutek Wilk
@ 2013-06-10 13:28     ` George Dunlap
  2013-06-10 13:30       ` Processed: " xen
  2013-06-10 20:24       ` Konrad Rzeszutek Wilk
  2013-06-10 13:30     ` Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not) xen
  1 sibling, 2 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-10 13:28 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Jan Beulich, xenbugs

On 10/06/13 14:20, Konrad Rzeszutek Wilk wrote:
> On Mon, Jun 10, 2013 at 12:12:47PM +0100, George Dunlap wrote:
>> create ^
>> title it xl pci-detach failure
>> thanks
>>
>> On Fri, Jun 7, 2013 at 4:45 PM, Konrad Rzeszutek Wilk
>> <konrad.wilk@oracle.com> wrote:
>>> any thoughts? I don't know if this is a regression or not, but the
>>> libxl errors are pointing me to the recent XSA issue - which I thought
>>> was fixed?
>> I think the recent issue was with xend not working.  Did we fix send
>> and break xl?
> Could be. It might also be that that 'xl' never did it the same way
> as 'xend' (meaning this might be a XenBus teardown change).
>
> Note also that this is PV guests - and I think most of the testing
> had been with the HVM guests with PCI passthrough. So it might be a
> seperate issue altogether. Or that nobody tried doing PCI plug/unplug
> in the past :-(

I certainly did before my February FOSDEM talk that included driver domains.

>
>> In any case, this is a pretty important feature; I think we need to
>> sort it out before release, so I'm giving it a bug id to track.
>>
>> Is "pci permissive" set in the global xl.conf file?
> No. Let me of course try that.

That really should have to do with getting it working in the first 
place, not detaching it; but still...

>> Have you tried this with say, a stock Debian Wheezy kernel?
> No. A v3.10-rc4 with Xen 4.3 latest.
>
> Is Debian Wheezy a 2.6.32 kernel?

No, 3.2.

  -George

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-06-10 13:20   ` Konrad Rzeszutek Wilk
  2013-06-10 13:28     ` George Dunlap
@ 2013-06-10 13:30     ` xen
  1 sibling, 0 replies; 36+ messages in thread
From: xen @ 2013-06-10 13:30 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel

Processing commands for xen@bugs.xenproject.org:

> On Mon, Jun 10, 2013 at 12:12:47PM +0100, George Dunlap wrote:
Command failed: Unknown command `On'. at /srv/xen-devel-bugs/lib/emesinae/control.pl line 408, <M> line 41.
Stop processing here.

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-06-10 13:28     ` George Dunlap
@ 2013-06-10 13:30       ` xen
  2013-06-10 20:24       ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 36+ messages in thread
From: xen @ 2013-06-10 13:30 UTC (permalink / raw)
  To: George Dunlap, xen-devel

Processing commands for xen@bugs.xenproject.org:

> On 10/06/13 14:20, Konrad Rzeszutek Wilk wrote:
Command failed: Unknown command `On'. at /srv/xen-devel-bugs/lib/emesinae/control.pl line 408, <M> line 34.
Stop processing here.

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-06-10 13:28     ` George Dunlap
  2013-06-10 13:30       ` Processed: " xen
@ 2013-06-10 20:24       ` Konrad Rzeszutek Wilk
  2013-06-10 20:30         ` Processed: " xen
  2013-06-10 21:06           ` (unknown), Konrad Rzeszutek Wilk
  1 sibling, 2 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-10 20:24 UTC (permalink / raw)
  To: George Dunlap; +Cc: xen-devel, Jan Beulich, xenbugs

On Mon, Jun 10, 2013 at 02:28:11PM +0100, George Dunlap wrote:
> On 10/06/13 14:20, Konrad Rzeszutek Wilk wrote:
> >On Mon, Jun 10, 2013 at 12:12:47PM +0100, George Dunlap wrote:
> >>create ^
> >>title it xl pci-detach failure
> >>thanks
> >>
> >>On Fri, Jun 7, 2013 at 4:45 PM, Konrad Rzeszutek Wilk
> >><konrad.wilk@oracle.com> wrote:
> >>>any thoughts? I don't know if this is a regression or not, but the
> >>>libxl errors are pointing me to the recent XSA issue - which I thought
> >>>was fixed?
> >>I think the recent issue was with xend not working.  Did we fix send
> >>and break xl?
> >Could be. It might also be that that 'xl' never did it the same way
> >as 'xend' (meaning this might be a XenBus teardown change).
> >
> >Note also that this is PV guests - and I think most of the testing
> >had been with the HVM guests with PCI passthrough. So it might be a
> >seperate issue altogether. Or that nobody tried doing PCI plug/unplug
> >in the past :-(
> 
> I certainly did before my February FOSDEM talk that included driver domains.
> 
> >
> >>In any case, this is a pretty important feature; I think we need to
> >>sort it out before release, so I'm giving it a bug id to track.
> >>
> >>Is "pci permissive" set in the global xl.conf file?
> >No. Let me of course try that.
> 
> That really should have to do with getting it working in the first
> place, not detaching it; but still...
> 
> >>Have you tried this with say, a stock Debian Wheezy kernel?
> >No. A v3.10-rc4 with Xen 4.3 latest.
> >
> >Is Debian Wheezy a 2.6.32 kernel?
> 
> No, 3.2.

I figured it out. It is the XenBus states.


The 'xm' for pci-detach would do:
 4(Connected)->7(Reconfiguring)-> 8(Reconfigured)-> 4(Connected)->5(Closing).

While 'xl' does:
 4(Connected)->7(Reconfiguring)-> 8(Reconfigured)-> 4(Connected)

That means the xen-pcifront never gets told that the connection is going to
be removed and can do its cleanup.

Without the cleanup it fails at the 3->4(Connected) change state:

[  151.403112] pcifront pci-0: publishing successful!
[  151.404313] pcifront pci-0: backend new state: 2 (old state:3)!
[  151.406160] pcifront pci-0: backend new state: 3 (old state:3)!
[  151.407287] pcifront pci-0: backend new state: 4 (old state:3)!
[  151.407463] pcifront pci-0: PCI frontend already installed!
===> [  151.407474] pcifront pci-0: 17 Error setting up PCI Frontend <=====
[  151.407680] pcifront pci-0: failed to write error node for device/pci/0 (17 Error setting up PCI Frontend) ret: -13
[  151.410502] pcifront pci-0: backend new state: 5 (old state:5)!
[  151.410509] pcifront pci-0: backend going away!

B/c the 'PCI frontend already installed' check has been hit (new in 3.7)
and would never progress further (see git commit
3d925320e9e2de162bd138bf97816bda8c3f71be - xen/pcifront: Use Xen-SWIOTLB when initting if required.)


This looks like an OK protocol change so I am inclined to say the Xen pcifront
needs a bit more checking. Sending patches for that shortly.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-06-10 20:24       ` Konrad Rzeszutek Wilk
@ 2013-06-10 20:30         ` xen
       [not found]           ` <20131104202224.GA18449@phenom.dumpdata.com>
  2013-06-10 21:06           ` (unknown), Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 36+ messages in thread
From: xen @ 2013-06-10 20:30 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel

Processing commands for xen@bugs.xenproject.org:

> On Mon, Jun 10, 2013 at 02:28:11PM +0100, George Dunlap wrote:
Command failed: Unknown command `On'. at /srv/xen-devel-bugs/lib/emesinae/control.pl line 408, <M> line 43.
Stop processing here.

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

^ permalink raw reply	[flat|nested] 36+ messages in thread

* (no subject)
  2013-06-10 20:24       ` Konrad Rzeszutek Wilk
@ 2013-06-10 21:06           ` Konrad Rzeszutek Wilk
  2013-06-10 21:06           ` (unknown), Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-10 21:06 UTC (permalink / raw)
  To: george.dunlap, xen-devel, linux-kernel

Please see attached patch. It fixes it for me.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* (unknown), 
@ 2013-06-10 21:06           ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-10 21:06 UTC (permalink / raw)
  To: george.dunlap, xen-devel, linux-kernel

Please see attached patch. It fixes it for me.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-10 21:06           ` (unknown), Konrad Rzeszutek Wilk
  (?)
@ 2013-06-10 21:06           ` Konrad Rzeszutek Wilk
  2013-06-11  7:29             ` Jan Beulich
                               ` (2 more replies)
  -1 siblings, 3 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-10 21:06 UTC (permalink / raw)
  To: george.dunlap, xen-devel, linux-kernel
  Cc: Konrad Rzeszutek Wilk, Bjorn Helgaas, linux-pci, stable

There are two tool-stack that can instruct the Xen PCI frontend
and backend to change states: 'xm' (Python code with a daemon),
and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach
<guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend
state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

When a PCI device is plugged in (xm pci-attach <guest> <BDF>)
both of them follow the same pattern:
2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).

[xen-pcifront ignores the 2,3 state changes and only acts when
4 (Connected) has been reached]

The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be
("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced
a mechanism to initialize the SWIOTLB when the Xen PCI front moves to
Connected state. It also had some aggressive seatbelt code check that
would warn the user if one tried to change to Connected state without
hitting first the Closing state:

 pcifront pci-0: PCI frontend already installed!

However, that code can be relaxed and we can continue on working
even if the frontend is instructed to be the 'Connected' state with
no devices and then gets tickled to be in 'Connected' state again.

In other words, this 4(Connected)->5(Closing)->4(Connected) state
was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected)
was not. This patch removes that aggressive check and allows
Xen pcifront to work with the 'xl' toolstack.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/pci/xen-pcifront.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index ac99515..cc46e253 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -675,10 +675,9 @@ static int pcifront_connect_and_init_dma(struct pcifront_device *pdev)
 	if (!pcifront_dev) {
 		dev_info(&pdev->xdev->dev, "Installing PCI frontend\n");
 		pcifront_dev = pdev;
-	} else {
-		dev_err(&pdev->xdev->dev, "PCI frontend already installed!\n");
+	} else
 		err = -EEXIST;
-	}
+
 	spin_unlock(&pcifront_dev_lock);
 
 	if (!err && !swiotlb_nr_tbl()) {
@@ -846,7 +845,7 @@ static int pcifront_try_connect(struct pcifront_device *pdev)
 		goto out;
 
 	err = pcifront_connect_and_init_dma(pdev);
-	if (err) {
+	if (err && err != -EEXIST) {
 		xenbus_dev_fatal(pdev->xdev, err,
 				 "Error setting up PCI Frontend");
 		goto out;
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [Xen-devel] [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-10 21:06           ` [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing' Konrad Rzeszutek Wilk
  2013-06-11  7:29             ` Jan Beulich
@ 2013-06-11  7:29             ` Jan Beulich
  2013-06-11  9:00               ` George Dunlap
  2013-06-11  9:00               ` [Xen-devel] " George Dunlap
  2013-06-11 15:36               ` George Dunlap
  2 siblings, 2 replies; 36+ messages in thread
From: Jan Beulich @ 2013-06-11  7:29 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: george.dunlap, Bjorn Helgaas, xen-devel, linux-kernel, linux-pci, stable

>>> On 10.06.13 at 23:06, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> There are two tool-stack that can instruct the Xen PCI frontend
> and backend to change states: 'xm' (Python code with a daemon),
> and 'xl' (C library - does not keep state changes).
> 
> With the 'xm', the path to disconnect a PCI device (xm pci-detach
> <guest> <BDF>)is:
> 
> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).
> 
> The * is for states that the tool-stack sets. For 'xl', it is similar:
> 
> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
> 
> Both of them also tear down the XenBus structure, so the backend
> state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.
> 
> When a PCI device is plugged in (xm pci-attach <guest> <BDF>)
> both of them follow the same pattern:
> 2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).
> 
> [xen-pcifront ignores the 2,3 state changes and only acts when
> 4 (Connected) has been reached]
> 
> The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be
> ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced
> a mechanism to initialize the SWIOTLB when the Xen PCI front moves to
> Connected state. It also had some aggressive seatbelt code check that
> would warn the user if one tried to change to Connected state without
> hitting first the Closing state:
> 
>  pcifront pci-0: PCI frontend already installed!
> 
> However, that code can be relaxed and we can continue on working
> even if the frontend is instructed to be the 'Connected' state with
> no devices and then gets tickled to be in 'Connected' state again.
> 
> In other words, this 4(Connected)->5(Closing)->4(Connected) state
> was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected)
> was not. This patch removes that aggressive check and allows
> Xen pcifront to work with the 'xl' toolstack.

I actually think this shouldn't be worked around here, but fixed in
xl. Any device removed from a guest should be driven towards
the "Closed" state.

Jan

> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org 
> Cc: stable@vger.kernel.org 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  drivers/pci/xen-pcifront.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index ac99515..cc46e253 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -675,10 +675,9 @@ static int pcifront_connect_and_init_dma(struct 
> pcifront_device *pdev)
>  	if (!pcifront_dev) {
>  		dev_info(&pdev->xdev->dev, "Installing PCI frontend\n");
>  		pcifront_dev = pdev;
> -	} else {
> -		dev_err(&pdev->xdev->dev, "PCI frontend already installed!\n");
> +	} else
>  		err = -EEXIST;
> -	}
> +
>  	spin_unlock(&pcifront_dev_lock);
>  
>  	if (!err && !swiotlb_nr_tbl()) {
> @@ -846,7 +845,7 @@ static int pcifront_try_connect(struct pcifront_device 
> *pdev)
>  		goto out;
>  
>  	err = pcifront_connect_and_init_dma(pdev);
> -	if (err) {
> +	if (err && err != -EEXIST) {
>  		xenbus_dev_fatal(pdev->xdev, err,
>  				 "Error setting up PCI Frontend");
>  		goto out;
> -- 
> 1.8.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-10 21:06           ` [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing' Konrad Rzeszutek Wilk
@ 2013-06-11  7:29             ` Jan Beulich
  2013-06-11  7:29             ` [Xen-devel] " Jan Beulich
  2013-06-11 15:36               ` George Dunlap
  2 siblings, 0 replies; 36+ messages in thread
From: Jan Beulich @ 2013-06-11  7:29 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: george.dunlap, linux-pci, linux-kernel, stable, xen-devel, Bjorn Helgaas

>>> On 10.06.13 at 23:06, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> There are two tool-stack that can instruct the Xen PCI frontend
> and backend to change states: 'xm' (Python code with a daemon),
> and 'xl' (C library - does not keep state changes).
> 
> With the 'xm', the path to disconnect a PCI device (xm pci-detach
> <guest> <BDF>)is:
> 
> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).
> 
> The * is for states that the tool-stack sets. For 'xl', it is similar:
> 
> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
> 
> Both of them also tear down the XenBus structure, so the backend
> state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.
> 
> When a PCI device is plugged in (xm pci-attach <guest> <BDF>)
> both of them follow the same pattern:
> 2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).
> 
> [xen-pcifront ignores the 2,3 state changes and only acts when
> 4 (Connected) has been reached]
> 
> The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be
> ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced
> a mechanism to initialize the SWIOTLB when the Xen PCI front moves to
> Connected state. It also had some aggressive seatbelt code check that
> would warn the user if one tried to change to Connected state without
> hitting first the Closing state:
> 
>  pcifront pci-0: PCI frontend already installed!
> 
> However, that code can be relaxed and we can continue on working
> even if the frontend is instructed to be the 'Connected' state with
> no devices and then gets tickled to be in 'Connected' state again.
> 
> In other words, this 4(Connected)->5(Closing)->4(Connected) state
> was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected)
> was not. This patch removes that aggressive check and allows
> Xen pcifront to work with the 'xl' toolstack.

I actually think this shouldn't be worked around here, but fixed in
xl. Any device removed from a guest should be driven towards
the "Closed" state.

Jan

> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org 
> Cc: stable@vger.kernel.org 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  drivers/pci/xen-pcifront.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index ac99515..cc46e253 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -675,10 +675,9 @@ static int pcifront_connect_and_init_dma(struct 
> pcifront_device *pdev)
>  	if (!pcifront_dev) {
>  		dev_info(&pdev->xdev->dev, "Installing PCI frontend\n");
>  		pcifront_dev = pdev;
> -	} else {
> -		dev_err(&pdev->xdev->dev, "PCI frontend already installed!\n");
> +	} else
>  		err = -EEXIST;
> -	}
> +
>  	spin_unlock(&pcifront_dev_lock);
>  
>  	if (!err && !swiotlb_nr_tbl()) {
> @@ -846,7 +845,7 @@ static int pcifront_try_connect(struct pcifront_device 
> *pdev)
>  		goto out;
>  
>  	err = pcifront_connect_and_init_dma(pdev);
> -	if (err) {
> +	if (err && err != -EEXIST) {
>  		xenbus_dev_fatal(pdev->xdev, err,
>  				 "Error setting up PCI Frontend");
>  		goto out;
> -- 
> 1.8.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Xen-devel] [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-11  7:29             ` [Xen-devel] " Jan Beulich
  2013-06-11  9:00               ` George Dunlap
@ 2013-06-11  9:00               ` George Dunlap
  2013-06-11 13:03                 ` konrad wilk
  2013-06-11 13:03                 ` [Xen-devel] " konrad wilk
  1 sibling, 2 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-11  9:00 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Konrad Rzeszutek Wilk, Bjorn Helgaas, xen-devel, linux-kernel,
	linux-pci, stable

On 06/11/2013 08:29 AM, Jan Beulich wrote:
>>>> On 10.06.13 at 23:06, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> There are two tool-stack that can instruct the Xen PCI frontend
>> and backend to change states: 'xm' (Python code with a daemon),
>> and 'xl' (C library - does not keep state changes).
>>
>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>> <guest> <BDF>)is:
>>
>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).
>>
>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>
>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>
>> Both of them also tear down the XenBus structure, so the backend
>> state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.
>>
>> When a PCI device is plugged in (xm pci-attach <guest> <BDF>)
>> both of them follow the same pattern:
>> 2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).
>>
>> [xen-pcifront ignores the 2,3 state changes and only acts when
>> 4 (Connected) has been reached]
>>
>> The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be
>> ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced
>> a mechanism to initialize the SWIOTLB when the Xen PCI front moves to
>> Connected state. It also had some aggressive seatbelt code check that
>> would warn the user if one tried to change to Connected state without
>> hitting first the Closing state:
>>
>>   pcifront pci-0: PCI frontend already installed!
>>
>> However, that code can be relaxed and we can continue on working
>> even if the frontend is instructed to be the 'Connected' state with
>> no devices and then gets tickled to be in 'Connected' state again.
>>
>> In other words, this 4(Connected)->5(Closing)->4(Connected) state
>> was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected)
>> was not. This patch removes that aggressive check and allows
>> Xen pcifront to work with the 'xl' toolstack.
>
> I actually think this shouldn't be worked around here, but fixed in
> xl. Any device removed from a guest should be driven towards
> the "Closed" state.

Yeah, that seems pretty obvious to me.  The weird thing is that this 
wasn't noticed before -- does this work in 4.2?  Have you been doing 
this test all along, or has it only broken recently?

I've reproduced it on one of my test boxes; let me see if I can sort it out.

  -George


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-11  7:29             ` [Xen-devel] " Jan Beulich
@ 2013-06-11  9:00               ` George Dunlap
  2013-06-11  9:00               ` [Xen-devel] " George Dunlap
  1 sibling, 0 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-11  9:00 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Konrad Rzeszutek Wilk, linux-pci, linux-kernel, stable,
	xen-devel, Bjorn Helgaas

On 06/11/2013 08:29 AM, Jan Beulich wrote:
>>>> On 10.06.13 at 23:06, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> There are two tool-stack that can instruct the Xen PCI frontend
>> and backend to change states: 'xm' (Python code with a daemon),
>> and 'xl' (C library - does not keep state changes).
>>
>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>> <guest> <BDF>)is:
>>
>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).
>>
>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>
>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>
>> Both of them also tear down the XenBus structure, so the backend
>> state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.
>>
>> When a PCI device is plugged in (xm pci-attach <guest> <BDF>)
>> both of them follow the same pattern:
>> 2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).
>>
>> [xen-pcifront ignores the 2,3 state changes and only acts when
>> 4 (Connected) has been reached]
>>
>> The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be
>> ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced
>> a mechanism to initialize the SWIOTLB when the Xen PCI front moves to
>> Connected state. It also had some aggressive seatbelt code check that
>> would warn the user if one tried to change to Connected state without
>> hitting first the Closing state:
>>
>>   pcifront pci-0: PCI frontend already installed!
>>
>> However, that code can be relaxed and we can continue on working
>> even if the frontend is instructed to be the 'Connected' state with
>> no devices and then gets tickled to be in 'Connected' state again.
>>
>> In other words, this 4(Connected)->5(Closing)->4(Connected) state
>> was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected)
>> was not. This patch removes that aggressive check and allows
>> Xen pcifront to work with the 'xl' toolstack.
>
> I actually think this shouldn't be worked around here, but fixed in
> xl. Any device removed from a guest should be driven towards
> the "Closed" state.

Yeah, that seems pretty obvious to me.  The weird thing is that this 
wasn't noticed before -- does this work in 4.2?  Have you been doing 
this test all along, or has it only broken recently?

I've reproduced it on one of my test boxes; let me see if I can sort it out.

  -George

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Xen-devel] [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-11  9:00               ` [Xen-devel] " George Dunlap
  2013-06-11 13:03                 ` konrad wilk
@ 2013-06-11 13:03                 ` konrad wilk
  1 sibling, 0 replies; 36+ messages in thread
From: konrad wilk @ 2013-06-11 13:03 UTC (permalink / raw)
  To: George Dunlap
  Cc: Jan Beulich, Bjorn Helgaas, xen-devel, linux-kernel, linux-pci, stable


On 6/11/2013 5:00 AM, George Dunlap wrote:
> On 06/11/2013 08:29 AM, Jan Beulich wrote:
>>>>> On 10.06.13 at 23:06, Konrad Rzeszutek Wilk 
>>>>> <konrad.wilk@oracle.com> wrote:
>>> There are two tool-stack that can instruct the Xen PCI frontend
>>> and backend to change states: 'xm' (Python code with a daemon),
>>> and 'xl' (C library - does not keep state changes).
>>>
>>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>>> <guest> <BDF>)is:
>>>
>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 
>>> 4(Connected)->5(Closing*).
>>>
>>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>>
>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>>
>>> Both of them also tear down the XenBus structure, so the backend
>>> state ends up going in the 3(Initialised) and calls 
>>> pcifront_xenbus_remove.
>>>
>>> When a PCI device is plugged in (xm pci-attach <guest> <BDF>)
>>> both of them follow the same pattern:
>>> 2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).
>>>
>>> [xen-pcifront ignores the 2,3 state changes and only acts when
>>> 4 (Connected) has been reached]
>>>
>>> The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be
>>> ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced
>>> a mechanism to initialize the SWIOTLB when the Xen PCI front moves to
>>> Connected state. It also had some aggressive seatbelt code check that
>>> would warn the user if one tried to change to Connected state without
>>> hitting first the Closing state:
>>>
>>>   pcifront pci-0: PCI frontend already installed!
>>>
>>> However, that code can be relaxed and we can continue on working
>>> even if the frontend is instructed to be the 'Connected' state with
>>> no devices and then gets tickled to be in 'Connected' state again.
>>>
>>> In other words, this 4(Connected)->5(Closing)->4(Connected) state
>>> was expected, while 4(Connected)->.... anything but 
>>> 5(Closing)->4(Connected)
>>> was not. This patch removes that aggressive check and allows
>>> Xen pcifront to work with the 'xl' toolstack.
>>
>> I actually think this shouldn't be worked around here, but fixed in
>> xl. Any device removed from a guest should be driven towards
>> the "Closed" state.

There is also the per-device state. Those are moved to the 5 (Closing), 
while the
whole connection is still in the 4(Connected) state. In essence all of 
the per-device states
are closed, it is just that the global state is still Connected.


>
> Yeah, that seems pretty obvious to me.  The weird thing is that this 
> wasn't noticed before -- does this work in 4.2?  Have you been doing 
> this test all along, or has it only broken recently?

I just reproduced this in Xen 4.2. I believe that the reason I did not 
see this before was b/c I was using 'xm'
primarily.
>
> I've reproduced it on one of my test boxes; let me see if I can sort 
> it out.

OK.
>
>  -George
>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-11  9:00               ` [Xen-devel] " George Dunlap
@ 2013-06-11 13:03                 ` konrad wilk
  2013-06-11 13:03                 ` [Xen-devel] " konrad wilk
  1 sibling, 0 replies; 36+ messages in thread
From: konrad wilk @ 2013-06-11 13:03 UTC (permalink / raw)
  To: George Dunlap
  Cc: linux-pci, linux-kernel, stable, xen-devel, Jan Beulich, Bjorn Helgaas


On 6/11/2013 5:00 AM, George Dunlap wrote:
> On 06/11/2013 08:29 AM, Jan Beulich wrote:
>>>>> On 10.06.13 at 23:06, Konrad Rzeszutek Wilk 
>>>>> <konrad.wilk@oracle.com> wrote:
>>> There are two tool-stack that can instruct the Xen PCI frontend
>>> and backend to change states: 'xm' (Python code with a daemon),
>>> and 'xl' (C library - does not keep state changes).
>>>
>>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>>> <guest> <BDF>)is:
>>>
>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 
>>> 4(Connected)->5(Closing*).
>>>
>>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>>
>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>>
>>> Both of them also tear down the XenBus structure, so the backend
>>> state ends up going in the 3(Initialised) and calls 
>>> pcifront_xenbus_remove.
>>>
>>> When a PCI device is plugged in (xm pci-attach <guest> <BDF>)
>>> both of them follow the same pattern:
>>> 2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).
>>>
>>> [xen-pcifront ignores the 2,3 state changes and only acts when
>>> 4 (Connected) has been reached]
>>>
>>> The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be
>>> ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced
>>> a mechanism to initialize the SWIOTLB when the Xen PCI front moves to
>>> Connected state. It also had some aggressive seatbelt code check that
>>> would warn the user if one tried to change to Connected state without
>>> hitting first the Closing state:
>>>
>>>   pcifront pci-0: PCI frontend already installed!
>>>
>>> However, that code can be relaxed and we can continue on working
>>> even if the frontend is instructed to be the 'Connected' state with
>>> no devices and then gets tickled to be in 'Connected' state again.
>>>
>>> In other words, this 4(Connected)->5(Closing)->4(Connected) state
>>> was expected, while 4(Connected)->.... anything but 
>>> 5(Closing)->4(Connected)
>>> was not. This patch removes that aggressive check and allows
>>> Xen pcifront to work with the 'xl' toolstack.
>>
>> I actually think this shouldn't be worked around here, but fixed in
>> xl. Any device removed from a guest should be driven towards
>> the "Closed" state.

There is also the per-device state. Those are moved to the 5 (Closing), 
while the
whole connection is still in the 4(Connected) state. In essence all of 
the per-device states
are closed, it is just that the global state is still Connected.


>
> Yeah, that seems pretty obvious to me.  The weird thing is that this 
> wasn't noticed before -- does this work in 4.2?  Have you been doing 
> this test all along, or has it only broken recently?

I just reproduced this in Xen 4.2. I believe that the reason I did not 
see this before was b/c I was using 'xm'
primarily.
>
> I've reproduced it on one of my test boxes; let me see if I can sort 
> it out.

OK.
>
>  -George
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-10 21:06           ` [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing' Konrad Rzeszutek Wilk
@ 2013-06-11 15:36               ` George Dunlap
  2013-06-11  7:29             ` [Xen-devel] " Jan Beulich
  2013-06-11 15:36               ` George Dunlap
  2 siblings, 0 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-11 15:36 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable

On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
> There are two tool-stack that can instruct the Xen PCI frontend
> and backend to change states: 'xm' (Python code with a daemon),
> and 'xl' (C library - does not keep state changes).
>
> With the 'xm', the path to disconnect a PCI device (xm pci-detach
> <guest> <BDF>)is:
>
> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).
>
> The * is for states that the tool-stack sets. For 'xl', it is similar:
>
> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>
> Both of them also tear down the XenBus structure, so the backend
> state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

So I looked a little bit into this; there are actually two different 
states that happen as part of this handshake.  In order to disonnect a 
*device*, xl signals using the *bus* state, like this:
* Wait for the *bus* to be in state 4(Connected)
* Set the *device* state to 5(Closing)
* Set the *bus* state to 7(Reconfiguring)
* Wait for the *bus* state to return to 4(Connected)

So are all of these states you see the *bus* state?  And why would you 
disconnect the whole pci bus if you're only removing one device?

  -George

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
@ 2013-06-11 15:36               ` George Dunlap
  0 siblings, 0 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-11 15:36 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable

On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
> There are two tool-stack that can instruct the Xen PCI frontend
> and backend to change states: 'xm' (Python code with a daemon),
> and 'xl' (C library - does not keep state changes).
>
> With the 'xm', the path to disconnect a PCI device (xm pci-detach
> <guest> <BDF>)is:
>
> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).
>
> The * is for states that the tool-stack sets. For 'xl', it is similar:
>
> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>
> Both of them also tear down the XenBus structure, so the backend
> state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

So I looked a little bit into this; there are actually two different 
states that happen as part of this handshake.  In order to disonnect a 
*device*, xl signals using the *bus* state, like this:
* Wait for the *bus* to be in state 4(Connected)
* Set the *device* state to 5(Closing)
* Set the *bus* state to 7(Reconfiguring)
* Wait for the *bus* state to return to 4(Connected)

So are all of these states you see the *bus* state?  And why would you 
disconnect the whole pci bus if you're only removing one device?

  -George

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-11 15:36               ` George Dunlap
  (?)
@ 2013-06-11 16:08               ` konrad wilk
  2013-06-11 16:17                   ` George Dunlap
  -1 siblings, 1 reply; 36+ messages in thread
From: konrad wilk @ 2013-06-11 16:08 UTC (permalink / raw)
  To: George Dunlap; +Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable


On 6/11/2013 11:36 AM, George Dunlap wrote:
> On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
>> There are two tool-stack that can instruct the Xen PCI frontend
>> and backend to change states: 'xm' (Python code with a daemon),
>> and 'xl' (C library - does not keep state changes).
>>
>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>> <guest> <BDF>)is:
>>
>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 
>> 4(Connected)->5(Closing*).
>>
>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>
>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>
>> Both of them also tear down the XenBus structure, so the backend
>> state ends up going in the 3(Initialised) and calls 
>> pcifront_xenbus_remove.
>
> So I looked a little bit into this; there are actually two different 
> states that happen as part of this handshake.  In order to disonnect a 
> *device*, xl signals using the *bus* state, like this:
> * Wait for the *bus* to be in state 4(Connected)
> * Set the *device* state to 5(Closing)
> * Set the *bus* state to 7(Reconfiguring)
> * Wait for the *bus* state to return to 4(Connected)
>
> So are all of these states you see the *bus* state?  And why would you 
> disconnect the whole pci bus if you're only removing one device?

Correct. The stats I enumerated are *bus* states. Not per-device states.
I presume (and I hadn't checked xm) that Xend has some logic to only 
disconnect the bus if all of the PCI devices have been disconnected. In 
'xl' it does not do that.

The testing I did was just with one PCI device.
>
>  -George


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-11 16:08               ` konrad wilk
@ 2013-06-11 16:17                   ` George Dunlap
  0 siblings, 0 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-11 16:17 UTC (permalink / raw)
  To: konrad wilk; +Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable

On 06/11/2013 05:08 PM, konrad wilk wrote:
>
> On 6/11/2013 11:36 AM, George Dunlap wrote:
>> On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
>>> There are two tool-stack that can instruct the Xen PCI frontend
>>> and backend to change states: 'xm' (Python code with a daemon),
>>> and 'xl' (C library - does not keep state changes).
>>>
>>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>>> <guest> <BDF>)is:
>>>
>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)->
>>> 4(Connected)->5(Closing*).
>>>
>>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>>
>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>>
>>> Both of them also tear down the XenBus structure, so the backend
>>> state ends up going in the 3(Initialised) and calls
>>> pcifront_xenbus_remove.
>>
>> So I looked a little bit into this; there are actually two different
>> states that happen as part of this handshake.  In order to disonnect a
>> *device*, xl signals using the *bus* state, like this:
>> * Wait for the *bus* to be in state 4(Connected)
>> * Set the *device* state to 5(Closing)
>> * Set the *bus* state to 7(Reconfiguring)
>> * Wait for the *bus* state to return to 4(Connected)
>>
>> So are all of these states you see the *bus* state?  And why would you
>> disconnect the whole pci bus if you're only removing one device?
>
> Correct. The stats I enumerated are *bus* states. Not per-device states.
> I presume (and I hadn't checked xm) that Xend has some logic to only
> disconnect the bus if all of the PCI devices have been disconnected. In
> 'xl' it does not do that.
>
> The testing I did was just with one PCI device.

Ah, OK -- I see now.  The problem is that the code in the Linux side 
didn't know about the whole "4->7->8->4" thing to unplug a device.  In 
all likelihood, if you had used xm with two devices (so that the bus 
didn't get disconnected), then you would have run across the same error.

So at least part of the problem *is* a bug in Linux.

That doesn't explain why I have problems doing this on Debian's version 
of 3.2 -- unless the "fix" you mentoned above was backported to the 
stable kernel, perhaps?

  -George

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
@ 2013-06-11 16:17                   ` George Dunlap
  0 siblings, 0 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-11 16:17 UTC (permalink / raw)
  To: konrad wilk; +Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable

On 06/11/2013 05:08 PM, konrad wilk wrote:
>
> On 6/11/2013 11:36 AM, George Dunlap wrote:
>> On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
>>> There are two tool-stack that can instruct the Xen PCI frontend
>>> and backend to change states: 'xm' (Python code with a daemon),
>>> and 'xl' (C library - does not keep state changes).
>>>
>>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>>> <guest> <BDF>)is:
>>>
>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)->
>>> 4(Connected)->5(Closing*).
>>>
>>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>>
>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>>
>>> Both of them also tear down the XenBus structure, so the backend
>>> state ends up going in the 3(Initialised) and calls
>>> pcifront_xenbus_remove.
>>
>> So I looked a little bit into this; there are actually two different
>> states that happen as part of this handshake.  In order to disonnect a
>> *device*, xl signals using the *bus* state, like this:
>> * Wait for the *bus* to be in state 4(Connected)
>> * Set the *device* state to 5(Closing)
>> * Set the *bus* state to 7(Reconfiguring)
>> * Wait for the *bus* state to return to 4(Connected)
>>
>> So are all of these states you see the *bus* state?  And why would you
>> disconnect the whole pci bus if you're only removing one device?
>
> Correct. The stats I enumerated are *bus* states. Not per-device states.
> I presume (and I hadn't checked xm) that Xend has some logic to only
> disconnect the bus if all of the PCI devices have been disconnected. In
> 'xl' it does not do that.
>
> The testing I did was just with one PCI device.

Ah, OK -- I see now.  The problem is that the code in the Linux side 
didn't know about the whole "4->7->8->4" thing to unplug a device.  In 
all likelihood, if you had used xm with two devices (so that the bus 
didn't get disconnected), then you would have run across the same error.

So at least part of the problem *is* a bug in Linux.

That doesn't explain why I have problems doing this on Debian's version 
of 3.2 -- unless the "fix" you mentoned above was backported to the 
stable kernel, perhaps?

  -George

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-11 16:17                   ` George Dunlap
  (?)
@ 2013-06-11 16:24                   ` konrad wilk
  -1 siblings, 0 replies; 36+ messages in thread
From: konrad wilk @ 2013-06-11 16:24 UTC (permalink / raw)
  To: George Dunlap; +Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable


On 6/11/2013 12:17 PM, George Dunlap wrote:
> On 06/11/2013 05:08 PM, konrad wilk wrote:
>>
>> On 6/11/2013 11:36 AM, George Dunlap wrote:
>>> On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
>>>> There are two tool-stack that can instruct the Xen PCI frontend
>>>> and backend to change states: 'xm' (Python code with a daemon),
>>>> and 'xl' (C library - does not keep state changes).
>>>>
>>>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>>>> <guest> <BDF>)is:
>>>>
>>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)->
>>>> 4(Connected)->5(Closing*).
>>>>
>>>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>>>
>>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>>>
>>>> Both of them also tear down the XenBus structure, so the backend
>>>> state ends up going in the 3(Initialised) and calls
>>>> pcifront_xenbus_remove.
>>>
>>> So I looked a little bit into this; there are actually two different
>>> states that happen as part of this handshake.  In order to disonnect a
>>> *device*, xl signals using the *bus* state, like this:
>>> * Wait for the *bus* to be in state 4(Connected)
>>> * Set the *device* state to 5(Closing)
>>> * Set the *bus* state to 7(Reconfiguring)
>>> * Wait for the *bus* state to return to 4(Connected)
>>>
>>> So are all of these states you see the *bus* state?  And why would you
>>> disconnect the whole pci bus if you're only removing one device?
>>
>> Correct. The stats I enumerated are *bus* states. Not per-device states.
>> I presume (and I hadn't checked xm) that Xend has some logic to only
>> disconnect the bus if all of the PCI devices have been disconnected. In
>> 'xl' it does not do that.
>>
>> The testing I did was just with one PCI device.
>
> Ah, OK -- I see now.  The problem is that the code in the Linux side 
> didn't know about the whole "4->7->8->4" thing to unplug a device.  In 
> all likelihood, if you had used xm with two devices (so that the bus 
> didn't get disconnected), then you would have run across the same error.
>
> So at least part of the problem *is* a bug in Linux.

Right.
>
> That doesn't explain why I have problems doing this on Debian's 
> version of 3.2 -- unless the "fix" you mentoned above was backported 
> to the stable kernel, perhaps?
No. It was a feature.
>
>  -George


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-11 16:17                   ` George Dunlap
  (?)
  (?)
@ 2013-06-12 13:45                   ` Konrad Rzeszutek Wilk
  2013-06-12 13:47                       ` George Dunlap
  2013-06-12 17:28                     ` Bjorn Helgaas
  -1 siblings, 2 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-12 13:45 UTC (permalink / raw)
  To: George Dunlap; +Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

On Tue, Jun 11, 2013 at 05:17:45PM +0100, George Dunlap wrote:
> On 06/11/2013 05:08 PM, konrad wilk wrote:
> >
> >On 6/11/2013 11:36 AM, George Dunlap wrote:
> >>On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
> >>>There are two tool-stack that can instruct the Xen PCI frontend
> >>>and backend to change states: 'xm' (Python code with a daemon),
> >>>and 'xl' (C library - does not keep state changes).
> >>>
> >>>With the 'xm', the path to disconnect a PCI device (xm pci-detach
> >>><guest> <BDF>)is:
> >>>
> >>>4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)->
> >>>4(Connected)->5(Closing*).
> >>>
> >>>The * is for states that the tool-stack sets. For 'xl', it is similar:
> >>>
> >>>4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
> >>>
> >>>Both of them also tear down the XenBus structure, so the backend
> >>>state ends up going in the 3(Initialised) and calls
> >>>pcifront_xenbus_remove.
> >>
> >>So I looked a little bit into this; there are actually two different
> >>states that happen as part of this handshake.  In order to disonnect a
> >>*device*, xl signals using the *bus* state, like this:
> >>* Wait for the *bus* to be in state 4(Connected)
> >>* Set the *device* state to 5(Closing)
> >>* Set the *bus* state to 7(Reconfiguring)
> >>* Wait for the *bus* state to return to 4(Connected)
> >>
> >>So are all of these states you see the *bus* state?  And why would you
> >>disconnect the whole pci bus if you're only removing one device?
> >
> >Correct. The stats I enumerated are *bus* states. Not per-device states.
> >I presume (and I hadn't checked xm) that Xend has some logic to only
> >disconnect the bus if all of the PCI devices have been disconnected. In
> >'xl' it does not do that.
> >
> >The testing I did was just with one PCI device.
> 
> Ah, OK -- I see now.  The problem is that the code in the Linux side
> didn't know about the whole "4->7->8->4" thing to unplug a device.
> In all likelihood, if you had used xm with two devices (so that the
> bus didn't get disconnected), then you would have run across the
> same error.
> 
> So at least part of the problem *is* a bug in Linux.

Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here
for reference) or putting it in your queue for Linus?

My plan would be to send it to Linus in the 3.11 merge window.

[-- Attachment #2: 0001-xen-pci-Deal-with-toolstack-missing-an-XenbusStateCl.patch --]
[-- Type: text/plain, Size: 3105 bytes --]

>From efdfbd66b4f0ff6f005f9d30891adb8bd3f3eefa Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 10 Jun 2013 16:48:09 -0400
Subject: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.

There are two tool-stack that can instruct the Xen PCI frontend
and backend to change states: 'xm' (Python code with a daemon),
and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach
<guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend
state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

When a PCI device is plugged in (xm pci-attach <guest> <BDF>)
both of them follow the same pattern:
2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).

[xen-pcifront ignores the 2,3 state changes and only acts when
4 (Connected) has been reached]

The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be
("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced
a mechanism to initialize the SWIOTLB when the Xen PCI front moves to
Connected state. It also had some aggressive seatbelt code check that
would warn the user if one tried to change to Connected state without
hitting first the Closing state:

 pcifront pci-0: PCI frontend already installed!

However, that code can be relaxed and we can continue on working
even if the frontend is instructed to be the 'Connected' state with
no devices and then gets tickled to be in 'Connected' state again.

In other words, this 4(Connected)->5(Closing)->4(Connected) state
was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected)
was not. This patch removes that aggressive check and allows
Xen pcifront to work with the 'xl' toolstack.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/pci/xen-pcifront.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index ac99515..cc46e253 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -675,10 +675,9 @@ static int pcifront_connect_and_init_dma(struct pcifront_device *pdev)
 	if (!pcifront_dev) {
 		dev_info(&pdev->xdev->dev, "Installing PCI frontend\n");
 		pcifront_dev = pdev;
-	} else {
-		dev_err(&pdev->xdev->dev, "PCI frontend already installed!\n");
+	} else
 		err = -EEXIST;
-	}
+
 	spin_unlock(&pcifront_dev_lock);
 
 	if (!err && !swiotlb_nr_tbl()) {
@@ -846,7 +845,7 @@ static int pcifront_try_connect(struct pcifront_device *pdev)
 		goto out;
 
 	err = pcifront_connect_and_init_dma(pdev);
-	if (err) {
+	if (err && err != -EEXIST) {
 		xenbus_dev_fatal(pdev->xdev, err,
 				 "Error setting up PCI Frontend");
 		goto out;
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-12 13:45                   ` Konrad Rzeszutek Wilk
@ 2013-06-12 13:47                       ` George Dunlap
  2013-06-12 17:28                     ` Bjorn Helgaas
  1 sibling, 0 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-12 13:47 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable

On 12/06/13 14:45, Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 11, 2013 at 05:17:45PM +0100, George Dunlap wrote:
>> On 06/11/2013 05:08 PM, konrad wilk wrote:
>>> On 6/11/2013 11:36 AM, George Dunlap wrote:
>>>> On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
>>>>> There are two tool-stack that can instruct the Xen PCI frontend
>>>>> and backend to change states: 'xm' (Python code with a daemon),
>>>>> and 'xl' (C library - does not keep state changes).
>>>>>
>>>>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>>>>> <guest> <BDF>)is:
>>>>>
>>>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)->
>>>>> 4(Connected)->5(Closing*).
>>>>>
>>>>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>>>>
>>>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>>>>
>>>>> Both of them also tear down the XenBus structure, so the backend
>>>>> state ends up going in the 3(Initialised) and calls
>>>>> pcifront_xenbus_remove.
>>>> So I looked a little bit into this; there are actually two different
>>>> states that happen as part of this handshake.  In order to disonnect a
>>>> *device*, xl signals using the *bus* state, like this:
>>>> * Wait for the *bus* to be in state 4(Connected)
>>>> * Set the *device* state to 5(Closing)
>>>> * Set the *bus* state to 7(Reconfiguring)
>>>> * Wait for the *bus* state to return to 4(Connected)
>>>>
>>>> So are all of these states you see the *bus* state?  And why would you
>>>> disconnect the whole pci bus if you're only removing one device?
>>> Correct. The stats I enumerated are *bus* states. Not per-device states.
>>> I presume (and I hadn't checked xm) that Xend has some logic to only
>>> disconnect the bus if all of the PCI devices have been disconnected. In
>>> 'xl' it does not do that.
>>>
>>> The testing I did was just with one PCI device.
>> Ah, OK -- I see now.  The problem is that the code in the Linux side
>> didn't know about the whole "4->7->8->4" thing to unplug a device.
>> In all likelihood, if you had used xm with two devices (so that the
>> bus didn't get disconnected), then you would have run across the
>> same error.
>>
>> So at least part of the problem *is* a bug in Linux.
> Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here
> for reference) or putting it in your queue for Linus?
>
> My plan would be to send it to Linus in the 3.11 merge window.

One nit -- "to work with the 'xl' toolstack" -- didn't we theorize this 
would also be broken with xm if you had two devices passed through?

  -George

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
@ 2013-06-12 13:47                       ` George Dunlap
  0 siblings, 0 replies; 36+ messages in thread
From: George Dunlap @ 2013-06-12 13:47 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable

On 12/06/13 14:45, Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 11, 2013 at 05:17:45PM +0100, George Dunlap wrote:
>> On 06/11/2013 05:08 PM, konrad wilk wrote:
>>> On 6/11/2013 11:36 AM, George Dunlap wrote:
>>>> On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
>>>>> There are two tool-stack that can instruct the Xen PCI frontend
>>>>> and backend to change states: 'xm' (Python code with a daemon),
>>>>> and 'xl' (C library - does not keep state changes).
>>>>>
>>>>> With the 'xm', the path to disconnect a PCI device (xm pci-detach
>>>>> <guest> <BDF>)is:
>>>>>
>>>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)->
>>>>> 4(Connected)->5(Closing*).
>>>>>
>>>>> The * is for states that the tool-stack sets. For 'xl', it is similar:
>>>>>
>>>>> 4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>>>>>
>>>>> Both of them also tear down the XenBus structure, so the backend
>>>>> state ends up going in the 3(Initialised) and calls
>>>>> pcifront_xenbus_remove.
>>>> So I looked a little bit into this; there are actually two different
>>>> states that happen as part of this handshake.  In order to disonnect a
>>>> *device*, xl signals using the *bus* state, like this:
>>>> * Wait for the *bus* to be in state 4(Connected)
>>>> * Set the *device* state to 5(Closing)
>>>> * Set the *bus* state to 7(Reconfiguring)
>>>> * Wait for the *bus* state to return to 4(Connected)
>>>>
>>>> So are all of these states you see the *bus* state?  And why would you
>>>> disconnect the whole pci bus if you're only removing one device?
>>> Correct. The stats I enumerated are *bus* states. Not per-device states.
>>> I presume (and I hadn't checked xm) that Xend has some logic to only
>>> disconnect the bus if all of the PCI devices have been disconnected. In
>>> 'xl' it does not do that.
>>>
>>> The testing I did was just with one PCI device.
>> Ah, OK -- I see now.  The problem is that the code in the Linux side
>> didn't know about the whole "4->7->8->4" thing to unplug a device.
>> In all likelihood, if you had used xm with two devices (so that the
>> bus didn't get disconnected), then you would have run across the
>> same error.
>>
>> So at least part of the problem *is* a bug in Linux.
> Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here
> for reference) or putting it in your queue for Linus?
>
> My plan would be to send it to Linus in the 3.11 merge window.

One nit -- "to work with the 'xl' toolstack" -- didn't we theorize this 
would also be broken with xm if you had two devices passed through?

  -George

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-12 13:47                       ` George Dunlap
  (?)
@ 2013-06-12 14:27                       ` Konrad Rzeszutek Wilk
  -1 siblings, 0 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-12 14:27 UTC (permalink / raw)
  To: George Dunlap; +Cc: xen-devel, linux-kernel, Bjorn Helgaas, linux-pci, stable

On Wed, Jun 12, 2013 at 02:47:11PM +0100, George Dunlap wrote:
> On 12/06/13 14:45, Konrad Rzeszutek Wilk wrote:
> >On Tue, Jun 11, 2013 at 05:17:45PM +0100, George Dunlap wrote:
> >>On 06/11/2013 05:08 PM, konrad wilk wrote:
> >>>On 6/11/2013 11:36 AM, George Dunlap wrote:
> >>>>On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
> >>>>>There are two tool-stack that can instruct the Xen PCI frontend
> >>>>>and backend to change states: 'xm' (Python code with a daemon),
> >>>>>and 'xl' (C library - does not keep state changes).
> >>>>>
> >>>>>With the 'xm', the path to disconnect a PCI device (xm pci-detach
> >>>>><guest> <BDF>)is:
> >>>>>
> >>>>>4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)->
> >>>>>4(Connected)->5(Closing*).
> >>>>>
> >>>>>The * is for states that the tool-stack sets. For 'xl', it is similar:
> >>>>>
> >>>>>4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
> >>>>>
> >>>>>Both of them also tear down the XenBus structure, so the backend
> >>>>>state ends up going in the 3(Initialised) and calls
> >>>>>pcifront_xenbus_remove.
> >>>>So I looked a little bit into this; there are actually two different
> >>>>states that happen as part of this handshake.  In order to disonnect a
> >>>>*device*, xl signals using the *bus* state, like this:
> >>>>* Wait for the *bus* to be in state 4(Connected)
> >>>>* Set the *device* state to 5(Closing)
> >>>>* Set the *bus* state to 7(Reconfiguring)
> >>>>* Wait for the *bus* state to return to 4(Connected)
> >>>>
> >>>>So are all of these states you see the *bus* state?  And why would you
> >>>>disconnect the whole pci bus if you're only removing one device?
> >>>Correct. The stats I enumerated are *bus* states. Not per-device states.
> >>>I presume (and I hadn't checked xm) that Xend has some logic to only
> >>>disconnect the bus if all of the PCI devices have been disconnected. In
> >>>'xl' it does not do that.
> >>>
> >>>The testing I did was just with one PCI device.
> >>Ah, OK -- I see now.  The problem is that the code in the Linux side
> >>didn't know about the whole "4->7->8->4" thing to unplug a device.
> >>In all likelihood, if you had used xm with two devices (so that the
> >>bus didn't get disconnected), then you would have run across the
> >>same error.
> >>
> >>So at least part of the problem *is* a bug in Linux.
> >Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here
> >for reference) or putting it in your queue for Linus?
> >
> >My plan would be to send it to Linus in the 3.11 merge window.
> 
> One nit -- "to work with the 'xl' toolstack" -- didn't we theorize
> this would also be broken with xm if you had two devices passed
> through?

Yes. I will fix up the title to reflect that shortly (say Friday?)

Thanks for your sharp eyes.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-12 13:45                   ` Konrad Rzeszutek Wilk
  2013-06-12 13:47                       ` George Dunlap
@ 2013-06-12 17:28                     ` Bjorn Helgaas
  2013-06-14 16:28                       ` Konrad Rzeszutek Wilk
  2013-11-04 20:43                       ` Konrad Rzeszutek Wilk
  1 sibling, 2 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2013-06-12 17:28 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: George Dunlap, xen-devel, linux-kernel, linux-pci, stable

On Wed, Jun 12, 2013 at 7:45 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Tue, Jun 11, 2013 at 05:17:45PM +0100, George Dunlap wrote:
>> On 06/11/2013 05:08 PM, konrad wilk wrote:
>> >
>> >On 6/11/2013 11:36 AM, George Dunlap wrote:
>> >>On 06/10/2013 10:06 PM, Konrad Rzeszutek Wilk wrote:
>> >>>There are two tool-stack that can instruct the Xen PCI frontend
>> >>>and backend to change states: 'xm' (Python code with a daemon),
>> >>>and 'xl' (C library - does not keep state changes).
>> >>>
>> >>>With the 'xm', the path to disconnect a PCI device (xm pci-detach
>> >>><guest> <BDF>)is:
>> >>>
>> >>>4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)->
>> >>>4(Connected)->5(Closing*).
>> >>>
>> >>>The * is for states that the tool-stack sets. For 'xl', it is similar:
>> >>>
>> >>>4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)
>> >>>
>> >>>Both of them also tear down the XenBus structure, so the backend
>> >>>state ends up going in the 3(Initialised) and calls
>> >>>pcifront_xenbus_remove.
>> >>
>> >>So I looked a little bit into this; there are actually two different
>> >>states that happen as part of this handshake.  In order to disonnect a
>> >>*device*, xl signals using the *bus* state, like this:
>> >>* Wait for the *bus* to be in state 4(Connected)
>> >>* Set the *device* state to 5(Closing)
>> >>* Set the *bus* state to 7(Reconfiguring)
>> >>* Wait for the *bus* state to return to 4(Connected)
>> >>
>> >>So are all of these states you see the *bus* state?  And why would you
>> >>disconnect the whole pci bus if you're only removing one device?
>> >
>> >Correct. The stats I enumerated are *bus* states. Not per-device states.
>> >I presume (and I hadn't checked xm) that Xend has some logic to only
>> >disconnect the bus if all of the PCI devices have been disconnected. In
>> >'xl' it does not do that.
>> >
>> >The testing I did was just with one PCI device.
>>
>> Ah, OK -- I see now.  The problem is that the code in the Linux side
>> didn't know about the whole "4->7->8->4" thing to unplug a device.
>> In all likelihood, if you had used xm with two devices (so that the
>> bus didn't get disconnected), then you would have run across the
>> same error.
>>
>> So at least part of the problem *is* a bug in Linux.
>
> Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here
> for reference) or putting it in your queue for Linus?
>
> My plan would be to send it to Linus in the 3.11 merge window.

Sure; this is your baby :)  Why don't you handle it via your tree,
since it's more related to xen than any PCI core stuff.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-12 17:28                     ` Bjorn Helgaas
@ 2013-06-14 16:28                       ` Konrad Rzeszutek Wilk
  2013-11-04 20:43                       ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-14 16:28 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: George Dunlap, xen-devel, linux-kernel, linux-pci, stable

> >> So at least part of the problem *is* a bug in Linux.
> >
> > Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here
> > for reference) or putting it in your queue for Linus?
> >
> > My plan would be to send it to Linus in the 3.11 merge window.
> 
> Sure; this is your baby :)  Why don't you handle it via your tree,
> since it's more related to xen than any PCI core stuff.

OK. Thanks!

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Processed: Re: Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
       [not found]           ` <20131104202224.GA18449@phenom.dumpdata.com>
@ 2013-11-04 20:30             ` xen
  2013-11-04 20:39               ` Wei Liu
  0 siblings, 1 reply; 36+ messages in thread
From: xen @ 2013-11-04 20:30 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel

Processing commands for xen@bugs.xenproject.org:

> help
Command failed: Unknown command `help'. at /srv/xen-devel-bugs/lib/emesinae/control.pl line 437, <M> line 43.
Stop processing here.

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Processed: Re: Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-11-04 20:30             ` Processed: " xen
@ 2013-11-04 20:39               ` Wei Liu
  2013-11-04 20:45                 ` Processed: " xen
  0 siblings, 1 reply; 36+ messages in thread
From: Wei Liu @ 2013-11-04 20:39 UTC (permalink / raw)
  To: xen; +Cc: xen-devel, wei.liu2

On Mon, Nov 04, 2013 at 08:30:01PM +0000, xen@bugs.xenproject.org wrote:
> Processing commands for xen@bugs.xenproject.org:
> 
> > help
> Command failed: Unknown command `help'. at /srv/xen-devel-bugs/lib/emesinae/control.pl line 437, <M> line 43.
> Stop processing here.
> 

http://bugs.xenproject.org/xen/static/control.txt

However you might not have permission to do things. :-)

Wei.

> ---
> Xen Hypervisor Bug Tracker
> See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
> Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-06-12 17:28                     ` Bjorn Helgaas
  2013-06-14 16:28                       ` Konrad Rzeszutek Wilk
@ 2013-11-04 20:43                       ` Konrad Rzeszutek Wilk
  2013-11-04 20:56                         ` Ben Guthro
  1 sibling, 1 reply; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-11-04 20:43 UTC (permalink / raw)
  To: Bjorn Helgaas, ian.campbell
  Cc: George Dunlap, xen-devel, linux-kernel, linux-pci, stable


> Sure; this is your baby :)  Why don't you handle it via your tree,
> since it's more related to xen than any PCI core stuff.
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Definitly fixed in v3.12. Just tested it and it works.

George, Ian, how do I "close" a bug in http://bugs.xenproject.org/xen/bug/12 ?

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Processed: Re: Processed: Re: Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)
  2013-11-04 20:39               ` Wei Liu
@ 2013-11-04 20:45                 ` xen
  0 siblings, 0 replies; 36+ messages in thread
From: xen @ 2013-11-04 20:45 UTC (permalink / raw)
  To: Wei Liu, xen-devel

Processing commands for xen@bugs.xenproject.org:

> On Mon, Nov 04, 2013 at 08:30:01PM +0000, xen@bugs.xenproject.org wrote:
Command failed: Unknown command `On'. at /srv/xen-devel-bugs/lib/emesinae/control.pl line 437, <M> line 41.
Stop processing here.

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
  2013-11-04 20:43                       ` Konrad Rzeszutek Wilk
@ 2013-11-04 20:56                         ` Ben Guthro
       [not found]                           ` <20131104205917.GA18696@phenom.dumpdata.com>
  0 siblings, 1 reply; 36+ messages in thread
From: Ben Guthro @ 2013-11-04 20:56 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: xen-devel, Ian Campbell, George Dunlap, linux-pci, linux-kernel,
	stable, Bjorn Helgaas


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

On Mon, Nov 4, 2013 at 3:43 PM, Konrad Rzeszutek Wilk <
konrad.wilk@oracle.com> wrote:

>
> > Sure; this is your baby :)  Why don't you handle it via your tree,
> > since it's more related to xen than any PCI core stuff.
> >
> > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Definitly fixed in v3.12. Just tested it and it works.
>
> George, Ian, how do I "close" a bug in
> http://bugs.xenproject.org/xen/bug/12 ?
>

See doc here, for a HOWTO:
http://wiki.xen.org/wiki/Xen_Bug_Management_Interface#Closing_a_bug




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

[-- Attachment #1.2: Type: text/html, Size: 1948 bytes --]

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

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Processed: Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
       [not found]                           ` <20131104205917.GA18696@phenom.dumpdata.com>
@ 2013-11-04 21:15                             ` xen
  0 siblings, 0 replies; 36+ messages in thread
From: xen @ 2013-11-04 21:15 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel

Processing commands for xen@bugs.xenproject.org:

> close 12
Closing bug #12
> 

Modified/created Bugs:
 - 12: http://bugs.xenproject.org/xen/bug/12

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2013-11-04 21:15 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-07 15:45 xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not) Konrad Rzeszutek Wilk
2013-06-10 11:12 ` George Dunlap
2013-06-10 11:15   ` Processed: " xen
2013-06-10 13:20   ` Konrad Rzeszutek Wilk
2013-06-10 13:28     ` George Dunlap
2013-06-10 13:30       ` Processed: " xen
2013-06-10 20:24       ` Konrad Rzeszutek Wilk
2013-06-10 20:30         ` Processed: " xen
     [not found]           ` <20131104202224.GA18449@phenom.dumpdata.com>
2013-11-04 20:30             ` Processed: " xen
2013-11-04 20:39               ` Wei Liu
2013-11-04 20:45                 ` Processed: " xen
2013-06-10 21:06         ` Konrad Rzeszutek Wilk
2013-06-10 21:06           ` (unknown), Konrad Rzeszutek Wilk
2013-06-10 21:06           ` [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing' Konrad Rzeszutek Wilk
2013-06-11  7:29             ` Jan Beulich
2013-06-11  7:29             ` [Xen-devel] " Jan Beulich
2013-06-11  9:00               ` George Dunlap
2013-06-11  9:00               ` [Xen-devel] " George Dunlap
2013-06-11 13:03                 ` konrad wilk
2013-06-11 13:03                 ` [Xen-devel] " konrad wilk
2013-06-11 15:36             ` George Dunlap
2013-06-11 15:36               ` George Dunlap
2013-06-11 16:08               ` konrad wilk
2013-06-11 16:17                 ` George Dunlap
2013-06-11 16:17                   ` George Dunlap
2013-06-11 16:24                   ` konrad wilk
2013-06-12 13:45                   ` Konrad Rzeszutek Wilk
2013-06-12 13:47                     ` George Dunlap
2013-06-12 13:47                       ` George Dunlap
2013-06-12 14:27                       ` Konrad Rzeszutek Wilk
2013-06-12 17:28                     ` Bjorn Helgaas
2013-06-14 16:28                       ` Konrad Rzeszutek Wilk
2013-11-04 20:43                       ` Konrad Rzeszutek Wilk
2013-11-04 20:56                         ` Ben Guthro
     [not found]                           ` <20131104205917.GA18696@phenom.dumpdata.com>
2013-11-04 21:15                             ` Processed: " xen
2013-06-10 13:30     ` Processed: Re: xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not) xen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.