From 01e3c0895b0c9ee957b09f946429226c241bb5d2 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Wed, 6 Aug 2014 16:21:32 -0400 Subject: [PATCH 06/10] xen/pciback: Restore configuration space when detaching from a guest. The commit 9eea3f7695226f9af9992cebf8e98ac0ad78b277 "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. To still retain the PCI configuration space, we save it once more and store it on our private copy to be restored when: - Device is unbinded from pciback - Device is detached from a guest. Reported-by: Sander Eikelenboom Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/xen-pciback/pci_stub.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index 843a2ba..1126c5a 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,7 @@ 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; spin_lock_irqsave(&pcistub_devices_lock, flags); @@ -279,9 +280,25 @@ 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); + if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state)) + dev_info(&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 binded to us). + */ + pci_restore_state(dev); + /* + * The next steps are to reload the configuration for the + * next time we bind & unbind to a guest - or unload from + * pciback. + */ + pci_save_state(dev); + dev_data->pci_saved_state = pci_store_saved_state(dev); + } /* This disables the device. */ xen_pcibk_reset_device(dev); -- 1.8.4.2