* [PATCH v3 0/2] PCI/IOV: sriov_numvfs bug fixes
[not found] <CGME20240214225938uscas1p28fc1f193d68b26b0e3b6e0e6da4574eb@uscas1p2.samsung.com>
@ 2024-02-14 22:59 ` Jim Harris
[not found] ` <CGME20240214225943uscas1p21ffa8b362bac11379168b83e42159160@uscas1p2.samsung.com>
[not found] ` <CGME20240214225948uscas1p2178d37968dc620a7e317ce3b0d57ab04@uscas1p2.samsung.com>
0 siblings, 2 replies; 3+ messages in thread
From: Jim Harris @ 2024-02-14 22:59 UTC (permalink / raw
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, Dan Williams, Kees Cook,
Gustavo A. R. Silva, linux-kernel@vger.kernel.org,
linux-cxl@vger.kernel.org, linux-hardening@vger.kernel.org,
Leon Romanovsky, Jason Gunthorpe, Alex Williamson,
pierre.cregut@orange.com
A deadlock condition was discovered by SPDK when removing an SRIOV-enabled and
VFIO-attached device, with a specific threading model. While discussing this
on the mailing list, a separate issue around updating sriov_numvfs and its
kobject_uevent() was also discovered. This series fixes both of those issues.
---
v1 => v2:
* No code changes
* Updated commit messages per feedback from Bjorn
* Added Leon's Reviewed-by tags
* Moved bulk of the v1 cover letter to the revert patch commit message where
it belonged
v2 => v3:
* Update iov->num_VFs before VFs are probed
---
Jim Harris (2):
PCI/IOV: Revert "PCI/IOV: Serialize sysfs sriov_numvfs reads vs writes"
PCI/IOV: set iov->num_VFs before VFs are probed
drivers/pci/iov.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
--
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v3 1/2] PCI/IOV: Revert "PCI/IOV: Serialize sysfs sriov_numvfs reads vs writes"
[not found] ` <CGME20240214225943uscas1p21ffa8b362bac11379168b83e42159160@uscas1p2.samsung.com>
@ 2024-02-14 22:59 ` Jim Harris
0 siblings, 0 replies; 3+ messages in thread
From: Jim Harris @ 2024-02-14 22:59 UTC (permalink / raw
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, Dan Williams, Kees Cook,
Gustavo A. R. Silva, linux-kernel@vger.kernel.org,
linux-cxl@vger.kernel.org, linux-hardening@vger.kernel.org,
Leon Romanovsky, Jason Gunthorpe, Alex Williamson,
pierre.cregut@orange.com
If an SR-IOV enabled device is held by vfio, and the device is removed,
vfio will hold device lock and notify userspace of the removal. If
userspace reads the sriov_numvfs sysfs entry, that thread will be blocked
since sriov_numvfs_show() also tries to acquire the device lock. If that
same thread is responsible for releasing the device to vfio, it results in
a deadlock.
The proper way to detect a change to the num_VFs value is to listen for a
sysfs event, not to add a device_lock() on the attribute _show() in the
kernel.
This reverts commit 35ff867b76576e32f34c698ccd11343f7d616204.
Revert had a small conflict, the sprintf() is now changed to sysfs_emit().
Link: https://lore.kernel.org/linux-pci/ZXJI5+f8bUelVXqu@ubuntu/
Suggested-by: Leon Romanovsky <leonro@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Jim Harris <jim.harris@samsung.com>
---
drivers/pci/iov.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index aaa33e8dc4c9..0ca20cd518d5 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -395,14 +395,8 @@ static ssize_t sriov_numvfs_show(struct device *dev,
char *buf)
{
struct pci_dev *pdev = to_pci_dev(dev);
- u16 num_vfs;
-
- /* Serialize vs sriov_numvfs_store() so readers see valid num_VFs */
- device_lock(&pdev->dev);
- num_vfs = pdev->sriov->num_VFs;
- device_unlock(&pdev->dev);
- return sysfs_emit(buf, "%u\n", num_vfs);
+ return sysfs_emit(buf, "%u\n", pdev->sriov->num_VFs);
}
/*
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v3 2/2] PCI/IOV: set iov->num_VFs before VFs are probed
[not found] ` <CGME20240214225948uscas1p2178d37968dc620a7e317ce3b0d57ab04@uscas1p2.samsung.com>
@ 2024-02-14 22:59 ` Jim Harris
0 siblings, 0 replies; 3+ messages in thread
From: Jim Harris @ 2024-02-14 22:59 UTC (permalink / raw
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, Dan Williams, Kees Cook,
Gustavo A. R. Silva, linux-kernel@vger.kernel.org,
linux-cxl@vger.kernel.org, linux-hardening@vger.kernel.org,
Leon Romanovsky, Jason Gunthorpe, Alex Williamson,
pierre.cregut@orange.com
Ensure userspace can see the updated number of VFs
via sysfs sriov_numvfs before those VFs are probed.
Suggested-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Jim Harris <jim.harris@samsung.com>
---
drivers/pci/iov.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 0ca20cd518d5..ebb7c6892ad6 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -673,12 +673,14 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
msleep(100);
pci_cfg_access_unlock(dev);
+ iov->num_VFs = nr_virtfn;
rc = sriov_add_vfs(dev, initial);
- if (rc)
+ if (rc) {
+ iov->num_VFs = 0;
goto err_pcibios;
+ }
kobject_uevent(&dev->dev.kobj, KOBJ_CHANGE);
- iov->num_VFs = nr_virtfn;
return 0;
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-14 22:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20240214225938uscas1p28fc1f193d68b26b0e3b6e0e6da4574eb@uscas1p2.samsung.com>
2024-02-14 22:59 ` [PATCH v3 0/2] PCI/IOV: sriov_numvfs bug fixes Jim Harris
[not found] ` <CGME20240214225943uscas1p21ffa8b362bac11379168b83e42159160@uscas1p2.samsung.com>
2024-02-14 22:59 ` [PATCH v3 1/2] PCI/IOV: Revert "PCI/IOV: Serialize sysfs sriov_numvfs reads vs writes" Jim Harris
[not found] ` <CGME20240214225948uscas1p2178d37968dc620a7e317ce3b0d57ab04@uscas1p2.samsung.com>
2024-02-14 22:59 ` [PATCH v3 2/2] PCI/IOV: set iov->num_VFs before VFs are probed Jim Harris
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 read-only IMAP folder(s) and NNTP newsgroup(s).