Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v3 0/2] PCI: Wait after FLR for devices that don't honor Immediate Readiness
@ 2026-10-11 19:01 Alexander Gruhlke
  2026-10-11 19:01 ` [PATCH v3 1/2] PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe Alexander Gruhlke
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Alexander Gruhlke @ 2026-10-11 19:01 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lukas Wunner, Rafael J . Wysocki, Alex Williamson, Hui Wang,
	Linux-Fan, linux-pci, linux-kernel, regressions,
	Alexander Gruhlke

Since v7.2, VFIO passthrough of a Samsung 990 PRO NVMe [144d:a80c] fails
on the second VM start, and the device is inaccessible until reboot.  The
drive advertises Immediate Readiness but returns ~0 for up to 7 ms after
FLR, so since 10baa9b4df40 ("PCI: Drop unnecessary retries when restoring
BARs") the config restore after FLR is lost.

Patch 1 ignores Immediate Readiness on that drive.  Firmware 8B2QJXD7
fixes it, but the Revision ID and Subsystem ID don't change, so the
quirk can't tell firmware versions apart.  A 990 EVO Plus [144d:a80d]
reports the same errors [1], but it isn't confirmed to advertise
Immediate Readiness, so it's not included.

Patch 2 is independent and keeps pci_dev_wait() from treating ~0 as
ready with RRS Software Visibility, which also affects the Intel
[8086:0a54] in the open d591f6804e7e regression.  Hui, Linux-Fan: testing
patch 2 on that drive would be appreciated.

Tested on v7.2.9 (Ryzen 9 7950X, X670E): with patch 1 and drive firmware
1B2QJXD7, three VM starts in a row work, and every FLR reports "ready
0ms after FLR".  With firmware 8B2QJXD7, plain v7.2.9 works.

Patch 2 is only tested for regressions, as the drive no longer shows the
problem: below a Root Port with RRS SV, three VM starts work and every
FLR reports "ready 0ms after FLR".  The VF path is untested with the
patch, but on another machine, an X550 VF below a Root Port with RRS SV
reads ~0 from the Vendor ID and valid data from the Command register
100 ms after FLR.  v1's variant, which read the Command register, was
tested against the old firmware, and the drive's Vendor ID read ~0
while it wasn't ready.  Applies to pci/next.

Changes since v2:
- Patch 1: add Lukas's Reviewed-by
- Patch 2: read the Command register on VFs whose Vendor ID reads ~0
  instead of treating them as ready (Lukas, sashiko)

v2: https://lore.kernel.org/r/20261011163249.18479-1-gruhlke@mailbox.org
v1: https://lore.kernel.org/r/20261010160529.27974-1-gruhlke@mailbox.org

[1] https://bbs.archlinux.org/viewtopic.php?id=314833

Alexander Gruhlke (2):
  PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe
  PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV

 drivers/pci/pci.c    | 14 ++++++++++----
 drivers/pci/quirks.c | 11 +++++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

-- 
2.56.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/2] PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe
  2026-10-11 19:01 [PATCH v3 0/2] PCI: Wait after FLR for devices that don't honor Immediate Readiness Alexander Gruhlke
@ 2026-10-11 19:01 ` Alexander Gruhlke
  2026-10-11 19:14   ` sashiko-bot
  2026-10-11 19:01 ` [PATCH v3 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV Alexander Gruhlke
  2026-10-12 22:23 ` [PATCH v3 0/2] PCI: Wait after FLR for devices that don't honor Immediate Readiness Bjorn Helgaas
  2 siblings, 1 reply; 8+ messages in thread
From: Alexander Gruhlke @ 2026-10-11 19:01 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lukas Wunner, Rafael J . Wysocki, Alex Williamson, Hui Wang,
	Linux-Fan, linux-pci, linux-kernel, regressions,
	Alexander Gruhlke

The Samsung 990 PRO [144d:a80c] advertises Immediate Readiness, so
pcie_flr() doesn't wait after FLR, but the drive returns ~0 for up to
7 ms.  Since commit 10baa9b4df40 ("PCI: Drop unnecessary retries when
restoring BARs"), the config restore after FLR is lost, which breaks
VFIO passthrough of the drive.  Firmware 8B2QJXD7 responds right after
FLR, but older firmware such as 1B2QJXD7 doesn't, and the firmware
version isn't visible here.

Ignore Immediate Readiness on this device, so pcie_flr() waits.

Fixes: 10baa9b4df40 ("PCI: Drop unnecessary retries when restoring BARs")
Link: https://bbs.archlinux.org/viewtopic.php?id=314833
Link: https://lore.kernel.org/r/20261010160529.27974-1-gruhlke@mailbox.org
Suggested-by: Lukas Wunner <lukas@wunner.de>
Assisted-by: Claude:claude-opus-5-5
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Alexander Gruhlke <gruhlke@mailbox.org>
---
 drivers/pci/quirks.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 0500d5fd4f..e8597d1209 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5623,6 +5623,17 @@ static void quirk_no_flr_snet(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SOLIDRUN, 0x1000, quirk_no_flr_snet);
 
+/* Samsung 990 PRO advertises Immediate Readiness, but isn't ready after FLR */
+static void quirk_no_imm_ready(struct pci_dev *dev)
+{
+	if (!dev->imm_ready)
+		return;
+
+	pci_info(dev, "Ignoring incorrectly advertised Immediate Readiness support\n");
+	dev->imm_ready = 0;
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SAMSUNG, 0xa80c, quirk_no_imm_ready);
+
 static void quirk_no_ext_tags(struct pci_dev *pdev)
 {
 	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV
  2026-10-11 19:01 [PATCH v3 0/2] PCI: Wait after FLR for devices that don't honor Immediate Readiness Alexander Gruhlke
  2026-10-11 19:01 ` [PATCH v3 1/2] PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe Alexander Gruhlke
@ 2026-10-11 19:01 ` Alexander Gruhlke
  2026-10-11 19:09   ` sashiko-bot
  2026-10-12 10:11   ` Lukas Wunner
  2026-10-12 22:23 ` [PATCH v3 0/2] PCI: Wait after FLR for devices that don't honor Immediate Readiness Bjorn Helgaas
  2 siblings, 2 replies; 8+ messages in thread
From: Alexander Gruhlke @ 2026-10-11 19:01 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lukas Wunner, Rafael J . Wysocki, Alex Williamson, Hui Wang,
	Linux-Fan, linux-pci, linux-kernel, regressions,
	Alexander Gruhlke

With RRS Software Visibility, pci_dev_wait() considers a device ready as
soon as reading the Vendor ID doesn't return the RRS value.  Devices
aren't required to respond with RRS while they aren't ready, so the read
may return ~0 and pci_dev_wait() stops waiting too early.  This was seen
with Intel [8086:0a54] and Samsung [144d:a80c] NVMe SSDs.

Also wait while the Vendor ID reads ~0.  A VF's Vendor ID always reads
~0, so check its Command register instead, as is done without RRS SV.

Fixes: d591f6804e7e ("PCI: Wait for device readiness with Configuration RRS")
Link: https://lore.kernel.org/r/20250611101442.387378-1-hui.wang@canonical.com
Suggested-by: Lukas Wunner <lukas@wunner.de>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Alexander Gruhlke <gruhlke@mailbox.org>
---
 drivers/pci/pci.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 350bae907e..d3d6e682fd 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1213,9 +1213,11 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
 	 * initialize.
 	 *
 	 * If the device is below a Root Port with Configuration RRS
-	 * Software Visibility enabled, reading the Vendor ID returns a
+	 * Software Visibility enabled, reading the Vendor ID may return a
 	 * special data value if the device responded with RRS.  Read the
-	 * Vendor ID until we get non-RRS status.
+	 * Vendor ID until we get non-RRS status and something other than
+	 * ~0.  The Vendor ID of VFs reads ~0, so check their Command
+	 * register instead.
 	 *
 	 * If there's no Root Port or Configuration RRS Software Visibility
 	 * is not enabled, the device may still respond with RRS, but
@@ -1235,8 +1237,12 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
 
 		if (root && root->config_rrs_sv) {
 			pci_read_config_dword(dev, PCI_VENDOR_ID, &id);
-			if (!pci_bus_rrs_vendor_id(id))
-				break;
+			if (!pci_bus_rrs_vendor_id(id)) {
+				if (PCI_POSSIBLE_ERROR(id) && dev->is_virtfn)
+					pci_read_config_dword(dev, PCI_COMMAND, &id);
+				if (!PCI_POSSIBLE_ERROR(id))
+					break;
+			}
 		} else {
 			pci_read_config_dword(dev, PCI_COMMAND, &id);
 			if (!PCI_POSSIBLE_ERROR(id))
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV
  2026-10-11 19:01 ` [PATCH v3 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV Alexander Gruhlke
@ 2026-10-11 19:09   ` sashiko-bot
  2026-10-12 10:11   ` Lukas Wunner
  1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-10-11 19:09 UTC (permalink / raw)
  To: Alexander Gruhlke; +Cc: linux-pci

> With RRS Software Visibility, pci_dev_wait() considers a device ready as
> soon as reading the Vendor ID doesn't return the RRS value.  Devices
> aren't required to respond with RRS while they aren't ready, so the read
> may return ~0 and pci_dev_wait() stops waiting too early.  This was seen
> with Intel [8086:0a54] and Samsung [144d:a80c] NVMe SSDs.
> 
> Also wait while the Vendor ID reads ~0.  A VF's Vendor ID always reads
> ~0, so check its Command register instead, as is done without RRS SV.
> 
> Fixes: d591f6804e7e ("PCI: Wait for device readiness with Configuration RRS")
> Link: https://lore.kernel.org/r/20250611101442.387378-1-hui.wang@canonical.com
> Suggested-by: Lukas Wunner <lukas@wunner.de>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Alexander Gruhlke <gruhlke@mailbox.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011190145.23621-1-gruhlke@mailbox.org?part=2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe
  2026-10-11 19:01 ` [PATCH v3 1/2] PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe Alexander Gruhlke
@ 2026-10-11 19:14   ` sashiko-bot
  2026-10-11 19:20     ` Alexander Gruhlke
  0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-10-11 19:14 UTC (permalink / raw)
  To: Alexander Gruhlke; +Cc: linux-pci

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: read-modify-write data race on dev->imm_ready bitfield
--

commit 0139f0dcf6cc99c5e4527404461aefca31dd96e8
Author: Alexander Gruhlke <gruhlke@mailbox.org>

PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe

The Samsung 990 PRO advertises Immediate Readiness, so pcie_flr() doesn't
wait after FLR, but the drive returns ~0 for up to 7 ms. Ignore Immediate
Readiness on this device, so pcie_flr() waits.

Link: https://bbs.archlinux.org/viewtopic.php?id=314833
Link: https://lore.kernel.org/r/20261010160529.27974-1-gruhlke@mailbox.org

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 38af25c390867..fc61e1ad9e574 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5747,6 +5747,17 @@ static void quirk_no_flr_snet(struct pci_dev *dev)
>  }
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SOLIDRUN, 0x1000, quirk_no_flr_snet);
>  
> +/* Samsung 990 PRO advertises Immediate Readiness, but isn't ready after FLR */
> +static void quirk_no_imm_ready(struct pci_dev *dev)
> +{
> +	if (!dev->imm_ready)
> +		return;
> +
> +	pci_info(dev, "Ignoring incorrectly advertised Immediate Readiness support\n");
> +	dev->imm_ready = 0;

[Severity: Medium]
Could this bitfield update be subject to a read-modify-write data race with
sysfs stores?

Because this quirk is executed via DECLARE_PCI_FIXUP_FINAL, it is invoked by
pci_bus_add_device(). This happens after pci_device_add() has already called
device_add(), exposing the device's sysfs attributes and firing a KOBJ_ADD
uevent.

This leaves a window where userspace (such as udev) might concurrently
write to sysfs attributes. Sysfs store handlers like d3cold_allowed_store()
write to bitfields that share the same underlying word as imm_ready without
locking.

Can this result in a C bitfield data race where either the imm_ready = 0
update or the sysfs attribute update is silently lost, potentially breaking
the VFIO passthrough (the bug this patch intended to fix) or corrupting PCI
power management state?

> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SAMSUNG, 0xa80c, quirk_no_imm_ready);
> +
>  static void quirk_no_ext_tags(struct pci_dev *pdev)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011190145.23621-1-gruhlke@mailbox.org?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe
  2026-10-11 19:14   ` sashiko-bot
@ 2026-10-11 19:20     ` Alexander Gruhlke
  0 siblings, 0 replies; 8+ messages in thread
From: Alexander Gruhlke @ 2026-10-11 19:20 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lukas Wunner, Rafael J . Wysocki, Alex Williamson, Hui Wang,
	Linux-Fan, sashiko-reviews, linux-pci, linux-kernel, regressions

On Sun, Oct 11, 2026 at 07:14:44PM +0000, sashiko-bot@kernel.org wrote:
> Can this result in a C bitfield data race where either the imm_ready = 0
> update or the sysfs attribute update is silently lost, potentially breaking
> the VFIO passthrough (the bug this patch intended to fix) or corrupting PCI
> power management state?

imm_ready is set in pci_init_capabilities(), after the header fixups, so
a final fixup is the earliest place a quirk can clear it.  At boot, final
fixups run before userspace starts, and other final quirks such as
quirk_no_msi() and quirk_broken_intx_masking() set pci_dev bitfields the
same way.  So I'd leave it as is.

Thanks,
Alexander

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV
  2026-10-11 19:01 ` [PATCH v3 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV Alexander Gruhlke
  2026-10-11 19:09   ` sashiko-bot
@ 2026-10-12 10:11   ` Lukas Wunner
  1 sibling, 0 replies; 8+ messages in thread
From: Lukas Wunner @ 2026-10-12 10:11 UTC (permalink / raw)
  To: Alexander Gruhlke
  Cc: Bjorn Helgaas, Rafael J . Wysocki, Alex Williamson, Hui Wang,
	Linux-Fan, linux-pci, linux-kernel, regressions

On Sun, Oct 11, 2026 at 09:01:45PM +0200, Alexander Gruhlke wrote:
> @@ -1235,8 +1237,12 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
>  
>  		if (root && root->config_rrs_sv) {
>  			pci_read_config_dword(dev, PCI_VENDOR_ID, &id);
> -			if (!pci_bus_rrs_vendor_id(id))
> -				break;
> +			if (!pci_bus_rrs_vendor_id(id)) {
> +				if (PCI_POSSIBLE_ERROR(id) && dev->is_virtfn)
> +					pci_read_config_dword(dev, PCI_COMMAND, &id);
> +				if (!PCI_POSSIBLE_ERROR(id))
> +					break;
> +			}
>  		} else {
>  			pci_read_config_dword(dev, PCI_COMMAND, &id);
>  			if (!PCI_POSSIBLE_ERROR(id))

Lean and clean, thank you.  The additional pci_read_config_dword()
exceeds 80 chars, but I guess Bjorn can fix that up when applying
if he dislikes it.

Reviewed-by: Lukas Wunner <lukas@wunner.de>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/2] PCI: Wait after FLR for devices that don't honor Immediate Readiness
  2026-10-11 19:01 [PATCH v3 0/2] PCI: Wait after FLR for devices that don't honor Immediate Readiness Alexander Gruhlke
  2026-10-11 19:01 ` [PATCH v3 1/2] PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe Alexander Gruhlke
  2026-10-11 19:01 ` [PATCH v3 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV Alexander Gruhlke
@ 2026-10-12 22:23 ` Bjorn Helgaas
  2 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2026-10-12 22:23 UTC (permalink / raw)
  To: Alexander Gruhlke
  Cc: Bjorn Helgaas, Lukas Wunner, Rafael J . Wysocki, Alex Williamson,
	Hui Wang, Linux-Fan, linux-pci, linux-kernel, regressions

On Sun, Oct 11, 2026 at 09:01:43PM +0200, Alexander Gruhlke wrote:
> Since v7.2, VFIO passthrough of a Samsung 990 PRO NVMe [144d:a80c] fails
> on the second VM start, and the device is inaccessible until reboot.  The
> drive advertises Immediate Readiness but returns ~0 for up to 7 ms after
> FLR, so since 10baa9b4df40 ("PCI: Drop unnecessary retries when restoring
> BARs") the config restore after FLR is lost.
> 
> Patch 1 ignores Immediate Readiness on that drive.  Firmware 8B2QJXD7
> fixes it, but the Revision ID and Subsystem ID don't change, so the
> quirk can't tell firmware versions apart.  A 990 EVO Plus [144d:a80d]
> reports the same errors [1], but it isn't confirmed to advertise
> Immediate Readiness, so it's not included.
> 
> Patch 2 is independent and keeps pci_dev_wait() from treating ~0 as
> ready with RRS Software Visibility, which also affects the Intel
> [8086:0a54] in the open d591f6804e7e regression.  Hui, Linux-Fan: testing
> patch 2 on that drive would be appreciated.
> 
> Tested on v7.2.9 (Ryzen 9 7950X, X670E): with patch 1 and drive firmware
> 1B2QJXD7, three VM starts in a row work, and every FLR reports "ready
> 0ms after FLR".  With firmware 8B2QJXD7, plain v7.2.9 works.
> 
> Patch 2 is only tested for regressions, as the drive no longer shows the
> problem: below a Root Port with RRS SV, three VM starts work and every
> FLR reports "ready 0ms after FLR".  The VF path is untested with the
> patch, but on another machine, an X550 VF below a Root Port with RRS SV
> reads ~0 from the Vendor ID and valid data from the Command register
> 100 ms after FLR.  v1's variant, which read the Command register, was
> tested against the old firmware, and the drive's Vendor ID read ~0
> while it wasn't ready.  Applies to pci/next.
> 
> Changes since v2:
> - Patch 1: add Lukas's Reviewed-by
> - Patch 2: read the Command register on VFs whose Vendor ID reads ~0
>   instead of treating them as ready (Lukas, sashiko)
> 
> v2: https://lore.kernel.org/r/20261011163249.18479-1-gruhlke@mailbox.org
> v1: https://lore.kernel.org/r/20261010160529.27974-1-gruhlke@mailbox.org
> 
> [1] https://bbs.archlinux.org/viewtopic.php?id=314833
> 
> Alexander Gruhlke (2):
>   PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe
>   PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV
> 
>  drivers/pci/pci.c    | 14 ++++++++++----
>  drivers/pci/quirks.c | 11 +++++++++++
>  2 files changed, 21 insertions(+), 4 deletions(-)

Applied to pci/reset for v7.4, thank you!

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-12 22:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-11 19:01 [PATCH v3 0/2] PCI: Wait after FLR for devices that don't honor Immediate Readiness Alexander Gruhlke
2026-10-11 19:01 ` [PATCH v3 1/2] PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe Alexander Gruhlke
2026-10-11 19:14   ` sashiko-bot
2026-10-11 19:20     ` Alexander Gruhlke
2026-10-11 19:01 ` [PATCH v3 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV Alexander Gruhlke
2026-10-11 19:09   ` sashiko-bot
2026-10-12 10:11   ` Lukas Wunner
2026-10-12 22:23 ` [PATCH v3 0/2] PCI: Wait after FLR for devices that don't honor Immediate Readiness Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox