From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org, bhelgaas@google.com,
linux@eikelenboom.it
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: [PATCH v4 7/7] xen/pciback: Restore configuration space when detaching from a guest.
Date: Fri, 21 Nov 2014 17:17:51 -0500 [thread overview]
Message-ID: <1416608271-18931-8-git-send-email-konrad.wilk@oracle.com> (raw)
In-Reply-To: <1416608271-18931-1-git-send-email-konrad.wilk@oracle.com>
The commit "xen/pciback: Don't deadlock when unbinding." was using
the version of pci_reset_function which would lock the device lock.
That is no good as we can dead-lock. As such we swapped to using
the lock-less version and requiring that the callers
of 'pcistub_put_pci_dev' take the device lock. And as such
this bug got exposed.
Using the lock-less version is OK, except that we tried to
use 'pci_restore_state' after the lock-less version of
__pci_reset_function_locked - which won't work as 'state_saved'
is set to false. Said 'state_saved' is a toggle boolean that
is to be used by the sequence of a) pci_save_state/pci_restore_state
or b) pci_load_and_free_saved_state/pci_restore_state. We don't
want to use a) as the guest might have messed up the PCI
configuration space and we want it to revert to the state
when the PCI device was binded to us. Therefore we pick
b) to restore the configuration space.
We restore from our 'golden' version of PCI configuration space, when an:
- Device is unbinded from pciback
- Device is detached from a guest.
Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/pci_stub.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
index 843a2ba..eb8b58e 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -105,7 +105,7 @@ static void pcistub_device_release(struct kref *kref)
*/
__pci_reset_function_locked(dev);
if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
- dev_dbg(&dev->dev, "Could not reload PCI state\n");
+ dev_info(&dev->dev, "Could not reload PCI state\n");
else
pci_restore_state(dev);
@@ -257,6 +257,8 @@ void pcistub_put_pci_dev(struct pci_dev *dev)
{
struct pcistub_device *psdev, *found_psdev = NULL;
unsigned long flags;
+ struct xen_pcibk_dev_data *dev_data;
+ int ret;
spin_lock_irqsave(&pcistub_devices_lock, flags);
@@ -279,9 +281,19 @@ void pcistub_put_pci_dev(struct pci_dev *dev)
* (so it's ready for the next domain)
*/
device_lock_assert(&dev->dev);
- __pci_reset_function_locked(dev);
- pci_restore_state(dev);
-
+ dev_data = pci_get_drvdata(dev);
+ ret = pci_load_saved_state(dev, dev_data->pci_saved_state);
+ if (ret < 0)
+ dev_warn(&dev->dev, "Could not reload PCI state\n");
+ else {
+ __pci_reset_function_locked(dev);
+ /*
+ * The usual sequence is pci_save_state & pci_restore_state
+ * but the guest might have messed the configuration space up.
+ * Use the initial version (when device was bound to us).
+ */
+ pci_restore_state(dev);
+ }
/* This disables the device. */
xen_pcibk_reset_device(dev);
--
1.9.3
next prev parent reply other threads:[~2014-11-21 22:18 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-21 22:17 [PATCH v4] Fixes for PCI backend for 3.19 Konrad Rzeszutek Wilk
2014-11-21 22:17 ` [PATCH v4 1/7] xen/pciback: Don't deadlock when unbinding Konrad Rzeszutek Wilk
2014-11-21 22:17 ` [PATCH v4 2/7] driver core: Provide an wrapper around the mutex to do lockdep warnings Konrad Rzeszutek Wilk
2014-11-21 22:17 ` [PATCH v4 3/7] xen/pciback: Include the domain id if removing the device whilst still in use Konrad Rzeszutek Wilk
2014-11-21 22:17 ` [PATCH v4 4/7] xen/pciback: Print out the domain owning the device Konrad Rzeszutek Wilk
2014-11-21 22:17 ` [PATCH v4 5/7] xen/pciback: Remove tons of dereferences Konrad Rzeszutek Wilk
2014-11-21 22:17 ` [PATCH v4 6/7] PCI: Expose pci_load_saved_state for public consumption Konrad Rzeszutek Wilk
2014-11-21 22:17 ` Konrad Rzeszutek Wilk [this message]
2014-12-01 14:14 ` [Xen-devel] [PATCH v4 7/7] xen/pciback: Restore configuration space when detaching from a guest David Vrabel
2014-12-01 21:59 ` Konrad Rzeszutek Wilk
2014-12-02 23:11 ` Boris Ostrovsky
2014-12-03 15:52 ` Konrad Rzeszutek Wilk
2014-12-02 10:10 ` [Xen-devel] [PATCH v4] Fixes for PCI backend for 3.19 Jan Beulich
2014-12-02 15:05 ` Konrad Rzeszutek Wilk
2014-12-02 15:09 ` Jan Beulich
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=1416608271-18931-8-git-send-email-konrad.wilk@oracle.com \
--to=konrad.wilk@oracle.com \
--cc=bhelgaas@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux@eikelenboom.it \
--cc=xen-devel@lists.xenproject.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).