Re: [PATCH v2 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV

Re: [PATCH v2 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV

[Alexander Gruhlke]

On Sun, Oct 11, 2026 at 04:41:42PM +0000, sashiko-bot@kernel.org wrote:
> Does this cause pci_dev_wait() to exit prematurely for Virtual Functions
> (VFs) that return ~0 under Configuration RRS Software Visibility?

Yes, but that's the current behavior for all devices; the patch only
changes it for non-VFs.

VFs could be covered by reading the Command register when the Vendor ID
reads ~0 and waiting while that is ~0, too.  Devices that are ready would
still need only one read.

Lukas, Bjorn: should I do that in v3, or keep the current form?

Thanks,
Alexander

Re: [PATCH v2 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV

[Lukas Wunner]

On Sun, Oct 11, 2026 at 06:49:55PM +0200, Alexander Gruhlke wrote:
> On Sun, Oct 11, 2026 at 04:41:42PM +0000, sashiko-bot@kernel.org wrote:
> > Does this cause pci_dev_wait() to exit prematurely for Virtual Functions
> > (VFs) that return ~0 under Configuration RRS Software Visibility?
> 
> Yes, but that's the current behavior for all devices; the patch only
> changes it for non-VFs.
> 
> VFs could be covered by reading the Command register when the Vendor ID
> reads ~0 and waiting while that is ~0, too.  Devices that are ready would
> still need only one read.
> 
> Lukas, Bjorn: should I do that in v3, or keep the current form?

For a patch which is going to be backported to stable (due to the
Fixes tag), changing behavior is not a good idea, so I guess you
were right to read the Command Register on VFs if the return value
is 0xffff.  I apologize for my earlier misleading comments.

You could probably constrain the Command Register read operation
to VFs as those are the only ones that would need it, right?

I've started to wonder if VFs can actually be inaccessible and
thus cause a 0xffff response.  So I looked up where the spec
language in PCIe r7.1 sec 7.5.1.1.1 originates from:

It was introduced in PCIe r5.0 sec 9.3.4.1.2 and then r6.0 moved
spec language to sec 7.5.1.1.1.  As rationale, PCIe r5.0 only says:

   "This field in all VFs returns FFFFh when read. VI software should
    return the VF Device ID (see Section 9.3.3.11) value from the
    associated PF as the Device ID for the VF.
    
    IMPLEMENTATION NOTE: Legacy PCI Probing Software
    Returning FFFFh for Device ID and Vendor ID values allows some
    legacy software to ignore VFs. See Section 7.5.1.1.1."

So it's a little unclear to me whether inaccessibility of VFs can
actually occur, but again, for a patch with a Fixes tag, it's better
to play it safe.

Thanks,

Lukas

[PATCH v2 2/2] PCI: Don't treat ~0 as ready in pci_dev_wait() with RRS SV

[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 for a Vendor ID other than ~0, except on VFs, whose Vendor ID
normally reads ~0.

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 | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 350bae907e..fef8a799c1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1213,9 +1213,10 @@ 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, unless it's a VF,
+	 * something other than ~0.
 	 *
 	 * If there's no Root Port or Configuration RRS Software Visibility
 	 * is not enabled, the device may still respond with RRS, but
@@ -1235,7 +1236,8 @@ 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))
+			if (!pci_bus_rrs_vendor_id(id) &&
+			    (!PCI_POSSIBLE_ERROR(id) || dev->is_virtfn))
 				break;
 		} else {
 			pci_read_config_dword(dev, PCI_COMMAND, &id);
-- 
2.56.0