All of lore.kernel.org
 help / color / mirror / Atom feed
From: George Dunlap <george.dunlap@eu.citrix.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: <xen-devel@lists.xensource.com>, <linux-kernel@vger.kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>, <linux-pci@vger.kernel.org>,
	<stable@vger.kernel.org>
Subject: Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
Date: Wed, 12 Jun 2013 14:47:11 +0100	[thread overview]
Message-ID: <51B87BDF.1080101@eu.citrix.com> (raw)
In-Reply-To: <20130612134504.GG2918@phenom.dumpdata.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: George Dunlap <george.dunlap@eu.citrix.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: xen-devel@lists.xensource.com, linux-kernel@vger.kernel.org,
	Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] xen/pci: Deal with toolstack missing an 'XenbusStateClosing'.
Date: Wed, 12 Jun 2013 14:47:11 +0100	[thread overview]
Message-ID: <51B87BDF.1080101@eu.citrix.com> (raw)
In-Reply-To: <20130612134504.GG2918@phenom.dumpdata.com>

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

  reply	other threads:[~2013-06-12 13:47 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 13:03                 ` konrad wilk
2013-06-11 13:03                 ` [Xen-devel] " konrad wilk
2013-06-11  9:00               ` George Dunlap
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51B87BDF.1080101@eu.citrix.com \
    --to=george.dunlap@eu.citrix.com \
    --cc=bhelgaas@google.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.