From a0f2dee0cd651efb5fac6a1d35b0a14460ebcdd4 Mon Sep 17 00:00:00 2001 From: Stefano Stabellini Date: Fri, 21 Nov 2014 11:04:39 +0000 Subject: xen: add a dma_addr_t dev_addr argument to xen_dma_map_page dev_addr is the machine address of the page. The new parameter can be used by the ARM and ARM64 implementations of xen_dma_map_page to find out if the page is a local page (pfn == mfn) or a foreign page (pfn != mfn). dev_addr could be retrieved again from the physical address, using pfn_to_mfn, but it requires accessing an rbtree. Since we already have the dev_addr in our hands at the call site there is no need to get the mfn twice. Signed-off-by: Stefano Stabellini Reviewed-by: Catalin Marinas Acked-by: Konrad Rzeszutek Wilk --- drivers/xen/swiotlb-xen.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index ebd8f218a788..ad2c5eb8a9c7 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -403,7 +403,7 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page, /* we are not interested in the dma_addr returned by * xen_dma_map_page, only in the potential cache flushes executed * by the function. */ - xen_dma_map_page(dev, page, offset, size, dir, attrs); + xen_dma_map_page(dev, page, dev_addr, offset, size, dir, attrs); return dev_addr; } @@ -417,7 +417,7 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page, return DMA_ERROR_CODE; xen_dma_map_page(dev, pfn_to_page(map >> PAGE_SHIFT), - map & ~PAGE_MASK, size, dir, attrs); + dev_addr, map & ~PAGE_MASK, size, dir, attrs); dev_addr = xen_phys_to_bus(map); /* @@ -574,6 +574,7 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, return 0; } xen_dma_map_page(hwdev, pfn_to_page(map >> PAGE_SHIFT), + dev_addr, map & ~PAGE_MASK, sg->length, dir, @@ -584,6 +585,7 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, * xen_dma_map_page, only in the potential cache flushes executed * by the function. */ xen_dma_map_page(hwdev, pfn_to_page(paddr >> PAGE_SHIFT), + dev_addr, paddr & ~PAGE_MASK, sg->length, dir, -- cgit v1.2.3 From a4dba130891271084344c12537731542ec77cb85 Mon Sep 17 00:00:00 2001 From: Stefano Stabellini Date: Fri, 21 Nov 2014 11:07:39 +0000 Subject: xen/arm/arm64: introduce xen_arch_need_swiotlb Introduce an arch specific function to find out whether a particular dma mapping operation needs to bounce on the swiotlb buffer. On ARM and ARM64, if the page involved is a foreign page and the device is not coherent, we need to bounce because at unmap time we cannot execute any required cache maintenance operations (we don't know how to find the pfn from the mfn). No change of behaviour for x86. Signed-off-by: Stefano Stabellini Reviewed-by: David Vrabel Reviewed-by: Catalin Marinas Acked-by: Ian Campbell Acked-by: Konrad Rzeszutek Wilk --- drivers/xen/swiotlb-xen.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/xen') diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index ad2c5eb8a9c7..3725ee4ff43c 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -399,7 +399,9 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page, * buffering it. */ if (dma_capable(dev, dev_addr, size) && - !range_straddles_page_boundary(phys, size) && !swiotlb_force) { + !range_straddles_page_boundary(phys, size) && + !xen_arch_need_swiotlb(dev, PFN_DOWN(phys), PFN_DOWN(dev_addr)) && + !swiotlb_force) { /* we are not interested in the dma_addr returned by * xen_dma_map_page, only in the potential cache flushes executed * by the function. */ @@ -557,6 +559,7 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, dma_addr_t dev_addr = xen_phys_to_bus(paddr); if (swiotlb_force || + xen_arch_need_swiotlb(hwdev, PFN_DOWN(paddr), PFN_DOWN(dev_addr)) || !dma_capable(hwdev, dev_addr, sg->length) || range_straddles_page_boundary(paddr, sg->length)) { phys_addr_t map = swiotlb_tbl_map_single(hwdev, -- cgit v1.2.3 From d6883e6f32e07ef2cc974753ba00927de099e6d7 Mon Sep 17 00:00:00 2001 From: Stefano Stabellini Date: Fri, 21 Nov 2014 11:09:39 +0000 Subject: swiotlb-xen: pass dev_addr to xen_dma_unmap_page and xen_dma_sync_single_for_cpu xen_dma_unmap_page and xen_dma_sync_single_for_cpu take a dma_addr_t handle as argument, not a physical address. Signed-off-by: Stefano Stabellini Reviewed-by: Catalin Marinas Acked-by: Konrad Rzeszutek Wilk CC: stable@vger.kernel.org --- drivers/xen/swiotlb-xen.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 3725ee4ff43c..498b6544fd62 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -449,7 +449,7 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr, BUG_ON(dir == DMA_NONE); - xen_dma_unmap_page(hwdev, paddr, size, dir, attrs); + xen_dma_unmap_page(hwdev, dev_addr, size, dir, attrs); /* NOTE: We use dev_addr here, not paddr! */ if (is_xen_swiotlb_buffer(dev_addr)) { @@ -497,14 +497,14 @@ xen_swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr, BUG_ON(dir == DMA_NONE); if (target == SYNC_FOR_CPU) - xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir); + xen_dma_sync_single_for_cpu(hwdev, dev_addr, size, dir); /* NOTE: We use dev_addr here, not paddr! */ if (is_xen_swiotlb_buffer(dev_addr)) swiotlb_tbl_sync_single(hwdev, paddr, size, dir, target); if (target == SYNC_FOR_DEVICE) - xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir); + xen_dma_sync_single_for_cpu(hwdev, dev_addr, size, dir); if (dir != DMA_FROM_DEVICE) return; -- cgit v1.2.3 From c884227eaae9936f8ecbde6e1387bccdab5f4e90 Mon Sep 17 00:00:00 2001 From: Stefano Stabellini Date: Fri, 21 Nov 2014 11:10:39 +0000 Subject: swiotlb-xen: remove BUG_ON in xen_bus_to_phys On x86 truncation cannot occur because config XEN depends on X86_64 || (X86_32 && X86_PAE). On ARM truncation can occur without CONFIG_ARM_LPAE, when the dma operation involves foreign grants. However in that case the physical address returned by xen_bus_to_phys is actually invalid (there is no mfn to pfn tracking for foreign grants on ARM) and it is not used. Signed-off-by: Stefano Stabellini Reviewed-by: Catalin Marinas Acked-by: Konrad Rzeszutek Wilk CC: stable@vger.kernel.org --- drivers/xen/swiotlb-xen.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 498b6544fd62..153cf1482125 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -96,8 +96,6 @@ static inline phys_addr_t xen_bus_to_phys(dma_addr_t baddr) dma_addr_t dma = (dma_addr_t)pfn << PAGE_SHIFT; phys_addr_t paddr = dma; - BUG_ON(paddr != dma); /* truncation has occurred, should never happen */ - paddr |= baddr & ~PAGE_MASK; return paddr; -- cgit v1.2.3 From 9490c6c67e2f41760de8ece4e4f56f75f84ceb9e Mon Sep 17 00:00:00 2001 From: Stefano Stabellini Date: Fri, 21 Nov 2014 16:55:12 +0000 Subject: swiotlb-xen: call xen_dma_sync_single_for_device when appropriate In xen_swiotlb_sync_single we always call xen_dma_sync_single_for_cpu, even when we should call xen_dma_sync_single_for_device. Fix that. Signed-off-by: Stefano Stabellini Acked-by: Konrad Rzeszutek Wilk CC: stable@vger.kernel.org --- drivers/xen/swiotlb-xen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/xen') diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 153cf1482125..810ad419e34c 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -502,7 +502,7 @@ xen_swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr, swiotlb_tbl_sync_single(hwdev, paddr, size, dir, target); if (target == SYNC_FOR_DEVICE) - xen_dma_sync_single_for_cpu(hwdev, dev_addr, size, dir); + xen_dma_sync_single_for_device(hwdev, dev_addr, size, dir); if (dir != DMA_FROM_DEVICE) return; -- cgit v1.2.3 From 2c3fc8d26dd09b9d7069687eead849ee81c78e46 Mon Sep 17 00:00:00 2001 From: Stefano Stabellini Date: Fri, 21 Nov 2014 16:56:12 +0000 Subject: swiotlb-xen: pass dev_addr to swiotlb_tbl_unmap_single Need to pass the pointer within the swiotlb internal buffer to the swiotlb library, that in the case of xen_unmap_single is dev_addr, not paddr. Signed-off-by: Stefano Stabellini Acked-by: Konrad Rzeszutek Wilk CC: stable@vger.kernel.org --- drivers/xen/swiotlb-xen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/xen') diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 810ad419e34c..5ea1e3c10907 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -451,7 +451,7 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr, /* NOTE: We use dev_addr here, not paddr! */ if (is_xen_swiotlb_buffer(dev_addr)) { - swiotlb_tbl_unmap_single(hwdev, paddr, size, dir); + swiotlb_tbl_unmap_single(hwdev, dev_addr, size, dir); return; } -- cgit v1.2.3 From e8801a7418dda995a70f30874aef77e6d064828e Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Wed, 3 Dec 2014 16:40:26 -0500 Subject: xen/pciback: Don't deadlock when unbinding. As commit 0a9fd0152929db372ff61b0d6c280fdd34ae8bdb 'xen/pciback: Document the entry points for 'pcistub_put_pci_dev'' explained there are four entry points in this function. Two of them are when the user fiddles in the SysFS to unbind a device which might be in use by a guest or not. Both 'unbind' states will cause a deadlock as the the PCI lock has already been taken, which then pci_device_reset tries to take. We can simplify this by requiring that all callers of pcistub_put_pci_dev MUST hold the device lock. And then we can just call the lockless version of pci_device_reset. To make it even simpler we will modify xen_pcibk_release_pci_dev to quality whether it should take a lock or not - as it ends up calling xen_pcibk_release_pci_dev and needs to hold the lock. Reviewed-by: Boris Ostrovsky Signed-off-by: Konrad Rzeszutek Wilk Signed-off-by: David Vrabel --- drivers/xen/xen-pciback/passthrough.c | 14 +++++++++++--- drivers/xen/xen-pciback/pci_stub.c | 12 ++++++------ drivers/xen/xen-pciback/pciback.h | 7 ++++--- drivers/xen/xen-pciback/vpci.c | 14 +++++++++++--- drivers/xen/xen-pciback/xenbus.c | 2 +- 5 files changed, 33 insertions(+), 16 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/xen-pciback/passthrough.c b/drivers/xen/xen-pciback/passthrough.c index 828dddc360df..f16a30e2a110 100644 --- a/drivers/xen/xen-pciback/passthrough.c +++ b/drivers/xen/xen-pciback/passthrough.c @@ -69,7 +69,7 @@ static int __xen_pcibk_add_pci_dev(struct xen_pcibk_device *pdev, } static void __xen_pcibk_release_pci_dev(struct xen_pcibk_device *pdev, - struct pci_dev *dev) + struct pci_dev *dev, bool lock) { struct passthrough_dev_data *dev_data = pdev->pci_dev_data; struct pci_dev_entry *dev_entry, *t; @@ -87,8 +87,13 @@ static void __xen_pcibk_release_pci_dev(struct xen_pcibk_device *pdev, mutex_unlock(&dev_data->lock); - if (found_dev) + if (found_dev) { + if (lock) + device_lock(&found_dev->dev); pcistub_put_pci_dev(found_dev); + if (lock) + device_unlock(&found_dev->dev); + } } static int __xen_pcibk_init_devices(struct xen_pcibk_device *pdev) @@ -156,8 +161,11 @@ static void __xen_pcibk_release_devices(struct xen_pcibk_device *pdev) struct pci_dev_entry *dev_entry, *t; list_for_each_entry_safe(dev_entry, t, &dev_data->dev_list, list) { + struct pci_dev *dev = dev_entry->dev; list_del(&dev_entry->list); - pcistub_put_pci_dev(dev_entry->dev); + device_lock(&dev->dev); + pcistub_put_pci_dev(dev); + device_unlock(&dev->dev); kfree(dev_entry); } diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index 017069a455d4..9cbe1a31c1e5 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -250,6 +250,8 @@ struct pci_dev *pcistub_get_pci_dev(struct xen_pcibk_device *pdev, * - 'echo BDF > unbind' with a guest still using it. See pcistub_remove * * As such we have to be careful. + * + * To make this easier, the caller has to hold the device lock. */ void pcistub_put_pci_dev(struct pci_dev *dev) { @@ -276,11 +278,8 @@ void pcistub_put_pci_dev(struct pci_dev *dev) /* Cleanup our device * (so it's ready for the next domain) */ - - /* This is OK - we are running from workqueue context - * and want to inhibit the user from fiddling with 'reset' - */ - pci_reset_function(dev); + lockdep_assert_held(&dev->dev.mutex); + __pci_reset_function_locked(dev); pci_restore_state(dev); /* This disables the device. */ @@ -567,7 +566,8 @@ static void pcistub_remove(struct pci_dev *dev) /* N.B. This ends up calling pcistub_put_pci_dev which ends up * doing the FLR. */ xen_pcibk_release_pci_dev(found_psdev->pdev, - found_psdev->dev); + found_psdev->dev, + false /* caller holds the lock. */); } spin_lock_irqsave(&pcistub_devices_lock, flags); diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h index f72af87640e0..58e38d586f52 100644 --- a/drivers/xen/xen-pciback/pciback.h +++ b/drivers/xen/xen-pciback/pciback.h @@ -99,7 +99,8 @@ struct xen_pcibk_backend { unsigned int *domain, unsigned int *bus, unsigned int *devfn); int (*publish)(struct xen_pcibk_device *pdev, publish_pci_root_cb cb); - void (*release)(struct xen_pcibk_device *pdev, struct pci_dev *dev); + void (*release)(struct xen_pcibk_device *pdev, struct pci_dev *dev, + bool lock); int (*add)(struct xen_pcibk_device *pdev, struct pci_dev *dev, int devid, publish_pci_dev_cb publish_cb); struct pci_dev *(*get)(struct xen_pcibk_device *pdev, @@ -122,10 +123,10 @@ static inline int xen_pcibk_add_pci_dev(struct xen_pcibk_device *pdev, } static inline void xen_pcibk_release_pci_dev(struct xen_pcibk_device *pdev, - struct pci_dev *dev) + struct pci_dev *dev, bool lock) { if (xen_pcibk_backend && xen_pcibk_backend->release) - return xen_pcibk_backend->release(pdev, dev); + return xen_pcibk_backend->release(pdev, dev, lock); } static inline struct pci_dev * diff --git a/drivers/xen/xen-pciback/vpci.c b/drivers/xen/xen-pciback/vpci.c index 51afff96c515..c99f8bb1c56c 100644 --- a/drivers/xen/xen-pciback/vpci.c +++ b/drivers/xen/xen-pciback/vpci.c @@ -145,7 +145,7 @@ out: } static void __xen_pcibk_release_pci_dev(struct xen_pcibk_device *pdev, - struct pci_dev *dev) + struct pci_dev *dev, bool lock) { int slot; struct vpci_dev_data *vpci_dev = pdev->pci_dev_data; @@ -169,8 +169,13 @@ static void __xen_pcibk_release_pci_dev(struct xen_pcibk_device *pdev, out: mutex_unlock(&vpci_dev->lock); - if (found_dev) + if (found_dev) { + if (lock) + device_lock(&found_dev->dev); pcistub_put_pci_dev(found_dev); + if (lock) + device_unlock(&found_dev->dev); + } } static int __xen_pcibk_init_devices(struct xen_pcibk_device *pdev) @@ -208,8 +213,11 @@ static void __xen_pcibk_release_devices(struct xen_pcibk_device *pdev) struct pci_dev_entry *e, *tmp; list_for_each_entry_safe(e, tmp, &vpci_dev->dev_list[slot], list) { + struct pci_dev *dev = e->dev; list_del(&e->list); - pcistub_put_pci_dev(e->dev); + device_lock(&dev->dev); + pcistub_put_pci_dev(dev); + device_unlock(&dev->dev); kfree(e); } } diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c index ad8d30c088fe..eeee499d4b3c 100644 --- a/drivers/xen/xen-pciback/xenbus.c +++ b/drivers/xen/xen-pciback/xenbus.c @@ -291,7 +291,7 @@ static int xen_pcibk_remove_device(struct xen_pcibk_device *pdev, /* N.B. This ends up calling pcistub_put_pci_dev which ends up * doing the FLR. */ - xen_pcibk_release_pci_dev(pdev, dev); + xen_pcibk_release_pci_dev(pdev, dev, true /* use the lock. */); out: return err; -- cgit v1.2.3 From ac8010221d3fa8697151dfe9a1bb2c504adc68c1 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Wed, 3 Dec 2014 16:40:27 -0500 Subject: driver core: Provide an wrapper around the mutex to do lockdep warnings Instead of open-coding it in drivers that want to double check that their functions are indeed holding the device lock. Signed-off-by: Konrad Rzeszutek Wilk Suggested-by: David Vrabel Acked-by: Greg Kroah-Hartman Signed-off-by: David Vrabel --- drivers/xen/xen-pciback/pci_stub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/xen') diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index 9cbe1a31c1e5..8b77089eddac 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -278,7 +278,7 @@ void pcistub_put_pci_dev(struct pci_dev *dev) /* Cleanup our device * (so it's ready for the next domain) */ - lockdep_assert_held(&dev->dev.mutex); + device_lock_assert(&dev->dev); __pci_reset_function_locked(dev); pci_restore_state(dev); -- cgit v1.2.3 From b5d512147aecc11ba8ecc62f4083f5a9f86d332f Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Wed, 3 Dec 2014 16:40:28 -0500 Subject: xen/pciback: Include the domain id if removing the device whilst still in use Cleanup the function a bit - also include the id of the domain that is using the device. Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: David Vrabel Signed-off-by: David Vrabel --- drivers/xen/xen-pciback/pci_stub.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index 8b77089eddac..e5ff09d8a242 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -553,12 +553,14 @@ static void pcistub_remove(struct pci_dev *dev) spin_unlock_irqrestore(&pcistub_devices_lock, flags); if (found_psdev) { - dev_dbg(&dev->dev, "found device to remove - in use? %p\n", - found_psdev->pdev); + dev_dbg(&dev->dev, "found device to remove %s\n", + found_psdev->pdev ? "- in-use" : ""); if (found_psdev->pdev) { - pr_warn("****** removing device %s while still in-use! ******\n", - pci_name(found_psdev->dev)); + int domid = xen_find_device_domain_owner(dev); + + pr_warn("****** removing device %s while still in-use by domain %d! ******\n", + pci_name(found_psdev->dev), domid); pr_warn("****** driver domain may still access this device's i/o resources!\n"); pr_warn("****** shutdown driver domain before binding device\n"); pr_warn("****** to other drivers or domains\n"); -- cgit v1.2.3 From 15390613b6554b554205d6d76cbbbb4a018a9ce4 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Wed, 3 Dec 2014 16:40:29 -0500 Subject: xen/pciback: Print out the domain owning the device. We had been printing it only if the device was built with debug enabled. But this information is useful in the field to troubleshoot. Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: David Vrabel Signed-off-by: David Vrabel --- drivers/xen/xen-pciback/xenbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/xen') diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c index eeee499d4b3c..fe17c80ff4b7 100644 --- a/drivers/xen/xen-pciback/xenbus.c +++ b/drivers/xen/xen-pciback/xenbus.c @@ -247,7 +247,7 @@ static int xen_pcibk_export_device(struct xen_pcibk_device *pdev, if (err) goto out; - dev_dbg(&dev->dev, "registering for %d\n", pdev->xdev->otherend_id); + dev_info(&dev->dev, "registering for %d\n", pdev->xdev->otherend_id); if (xen_register_device_domain_owner(dev, pdev->xdev->otherend_id) != 0) { dev_err(&dev->dev, "Stealing ownership from dom%d.\n", -- cgit v1.2.3 From c1a04339e55d808d328d28e3526df31e80b55383 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Wed, 3 Dec 2014 16:40:30 -0500 Subject: xen/pciback: Remove tons of dereferences A little cleanup. No functional difference. Reviewed-by: Boris Ostrovsky Signed-off-by: Konrad Rzeszutek Wilk Signed-off-by: David Vrabel --- drivers/xen/xen-pciback/pci_stub.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index e5ff09d8a242..843a2baffc2b 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -631,10 +631,12 @@ static pci_ers_result_t common_process(struct pcistub_device *psdev, { pci_ers_result_t res = result; struct xen_pcie_aer_op *aer_op; + struct xen_pcibk_device *pdev = psdev->pdev; + struct xen_pci_sharedinfo *sh_info = pdev->sh_info; int ret; /*with PV AER drivers*/ - aer_op = &(psdev->pdev->sh_info->aer_op); + aer_op = &(sh_info->aer_op); aer_op->cmd = aer_cmd ; /*useful for error_detected callback*/ aer_op->err = state; @@ -655,36 +657,36 @@ static pci_ers_result_t common_process(struct pcistub_device *psdev, * this flag to judge whether we need to check pci-front give aer * service ack signal */ - set_bit(_PCIB_op_pending, (unsigned long *)&psdev->pdev->flags); + set_bit(_PCIB_op_pending, (unsigned long *)&pdev->flags); /*It is possible that a pcifront conf_read_write ops request invokes * the callback which cause the spurious execution of wake_up. * Yet it is harmless and better than a spinlock here */ set_bit(_XEN_PCIB_active, - (unsigned long *)&psdev->pdev->sh_info->flags); + (unsigned long *)&sh_info->flags); wmb(); - notify_remote_via_irq(psdev->pdev->evtchn_irq); + notify_remote_via_irq(pdev->evtchn_irq); ret = wait_event_timeout(xen_pcibk_aer_wait_queue, !(test_bit(_XEN_PCIB_active, (unsigned long *) - &psdev->pdev->sh_info->flags)), 300*HZ); + &sh_info->flags)), 300*HZ); if (!ret) { if (test_bit(_XEN_PCIB_active, - (unsigned long *)&psdev->pdev->sh_info->flags)) { + (unsigned long *)&sh_info->flags)) { dev_err(&psdev->dev->dev, "pcifront aer process not responding!\n"); clear_bit(_XEN_PCIB_active, - (unsigned long *)&psdev->pdev->sh_info->flags); + (unsigned long *)&sh_info->flags); aer_op->err = PCI_ERS_RESULT_NONE; return res; } } - clear_bit(_PCIB_op_pending, (unsigned long *)&psdev->pdev->flags); + clear_bit(_PCIB_op_pending, (unsigned long *)&pdev->flags); if (test_bit(_XEN_PCIF_active, - (unsigned long *)&psdev->pdev->sh_info->flags)) { + (unsigned long *)&sh_info->flags)) { dev_dbg(&psdev->dev->dev, "schedule pci_conf service in " DRV_NAME "\n"); xen_pcibk_test_and_schedule_op(psdev->pdev); -- cgit v1.2.3 From b1df4a56bf4a61113e8928f932d346bed6eef553 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Wed, 3 Dec 2014 16:40:32 -0500 Subject: xen/pciback: Restore configuration space when detaching from a guest. 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 Signed-off-by: Konrad Rzeszutek Wilk Signed-off-by: David Vrabel --- drivers/xen/xen-pciback/pci_stub.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index 843a2baffc2b..8580e53355ec 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); @@ -280,8 +282,18 @@ void pcistub_put_pci_dev(struct pci_dev *dev) */ 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) { + /* + * 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); + } else + dev_info(&dev->dev, "Could not reload PCI state\n"); /* This disables the device. */ xen_pcibk_reset_device(dev); -- cgit v1.2.3 From 6945c59c772be5edd578b12831dee3f7c8576103 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Wed, 3 Dec 2014 16:40:33 -0500 Subject: xen-pciback: drop SR-IOV VFs when PF driver unloads When a PF driver unloads, it may find it necessary to leave the VFs around simply because of pciback having marked them as assigned to a guest. Utilize a suitable notification to let go of the VFs, thus allowing the PF to go back into the state it was before its driver loaded (which in particular allows the driver to be loaded again with it being able to create the VFs anew, but which also allows to then pass through the PF instead of the VFs). Don't do this however for any VFs currently in active use by a guest. Signed-off-by: Jan Beulich [v2: Removed the switch statement, moved it about] [v3: Redid it a bit differently] Signed-off-by: Konrad Rzeszutek Wilk Signed-off-by: David Vrabel --- drivers/xen/xen-pciback/pci_stub.c | 54 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) (limited to 'drivers/xen') diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index 8580e53355ec..cc3cbb4435f8 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -1518,6 +1518,53 @@ parse_error: fs_initcall(pcistub_init); #endif +#ifdef CONFIG_PCI_IOV +static struct pcistub_device *find_vfs(const struct pci_dev *pdev) +{ + struct pcistub_device *psdev = NULL; + unsigned long flags; + bool found = false; + + spin_lock_irqsave(&pcistub_devices_lock, flags); + list_for_each_entry(psdev, &pcistub_devices, dev_list) { + if (!psdev->pdev && psdev->dev != pdev + && pci_physfn(psdev->dev) == pdev) { + found = true; + break; + } + } + spin_unlock_irqrestore(&pcistub_devices_lock, flags); + if (found) + return psdev; + return NULL; +} + +static int pci_stub_notifier(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct device *dev = data; + const struct pci_dev *pdev = to_pci_dev(dev); + + if (action != BUS_NOTIFY_UNBIND_DRIVER) + return NOTIFY_DONE; + + if (!pdev->is_physfn) + return NOTIFY_DONE; + + for (;;) { + struct pcistub_device *psdev = find_vfs(pdev); + if (!psdev) + break; + device_release_driver(&psdev->dev->dev); + } + return NOTIFY_DONE; +} + +static struct notifier_block pci_stub_nb = { + .notifier_call = pci_stub_notifier, +}; +#endif + static int __init xen_pcibk_init(void) { int err; @@ -1539,12 +1586,19 @@ static int __init xen_pcibk_init(void) err = xen_pcibk_xenbus_register(); if (err) pcistub_exit(); +#ifdef CONFIG_PCI_IOV + else + bus_register_notifier(&pci_bus_type, &pci_stub_nb); +#endif return err; } static void __exit xen_pcibk_cleanup(void) { +#ifdef CONFIG_PCI_IOV + bus_unregister_notifier(&pci_bus_type, &pci_stub_nb); +#endif xen_pcibk_xenbus_unregister(); pcistub_exit(); } -- cgit v1.2.3 From 4ef8e3f3504808621e594f01852476a1d4e7ef93 Mon Sep 17 00:00:00 2001 From: David Vrabel Date: Wed, 10 Dec 2014 14:48:43 +0000 Subject: Revert "swiotlb-xen: pass dev_addr to swiotlb_tbl_unmap_single" This reverts commit 2c3fc8d26dd09b9d7069687eead849ee81c78e46. This commit broke on x86 PV because entries in the generic SWIOTLB are indexed using (pseudo-)physical address not DMA address and these are not the same in a x86 PV guest. Signed-off-by: David Vrabel Reviewed-by: Stefano Stabellini --- drivers/xen/swiotlb-xen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/xen') diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 5ea1e3c10907..810ad419e34c 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -451,7 +451,7 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr, /* NOTE: We use dev_addr here, not paddr! */ if (is_xen_swiotlb_buffer(dev_addr)) { - swiotlb_tbl_unmap_single(hwdev, dev_addr, size, dir); + swiotlb_tbl_unmap_single(hwdev, paddr, size, dir); return; } -- cgit v1.2.3