[PATCH RFC] PCI: Start the reset of WD PC SN730 NVMe controllers early

[PATCH RFC] PCI: Start the reset of WD PC SN730 NVMe controllers early

[Paul Menzel]

[Please consider this a bug report.]

On a Dell XPS 13 9310 (BIOS 3.17.0) with a WD PC SN730 1 TB (15b7:5006,
firmware 11170012, the newest available for this OEM drive), about one
boot in ten mounts the root file system 100 to 700 ms late. With the
nvme module probing at 0.29 s from a small initrd, the time from probe
to Identify grows from 9 ms to 74 to 229 ms, and the partition table
read and the mount get slower too:

    nvme nvme0: pci function 0000:01:00.0                 [0.293894]
    nvme nvme0: passthrough uses implicit buffer lengths   [0.522525]  (+228.6 ms, usually +9 ms)
     nvme0n1: p1 p2 p3 p4                                   +27.3 ms  (usually 3 ms)
    EXT4-fs (nvme0n1p3): mounted filesystem                 +82.2 ms  (usually 8 ms)

Tracing nvme_disable_ctrl() and nvme_enable_ctrl() with function_graph
over 120 boots shows the time is spent waiting for the controller reset
nvme_pci_configure_admin_queue() does first, as the firmware hands the
controller over enabled:

    boot  disable (CSTS.RDY 1->0)  enable (0->1)  4 KiB reads after
    fast          2 ms                  2 ms           ~0.1 ms
    slow     81 to 584 ms          16 to 60 ms        5 to 7 ms

The controller state at hand-off, read from the initrd before nvme
binds, is the same in fast and slow boots, CC=0x00460001 CSTS=0x00000001,
so there is no shutdown in progress. The slow resets also happen without
GRUB’s grubenv write before the hand-off (2 of 31 boots) and with the
probe delayed to 0.76 s (1 of 30 boots), so it is no busy period after
the firmware phase that a later probe would avoid.

Clearing CC.EN before the driver binds, emulated from the initrd with
devmem, works: the driver finds CSTS.RDY clear and continues, the reset
completed within the first poll in 29 of 30 boots (35 polls once), and
none of the 30 boots showed a slow reset, enable or read, against 8 of
90 without it. The difference is not yet significant (p = 0.1).

So start the reset in a final fixup during enumeration, at 0.19 s on
this machine, about 100 ms before nvme probes, which overlaps a slow
reset with the remaining kernel initialization and the module loading,
and costs nothing for fast ones.

With this change, 1 of 59 untraced boots stalls, against 7 of 41
untraced boots without it. In the other 58 boots Identify comes 7 ms
after probe and the root file system is mounted at 0.31 s. The stalled
boot:

    pci 0000:01:00.0: NVMe controller reset started early  [0.192544]
    nvme nvme0: pci function 0000:01:00.0                 [0.292395]
    nvme nvme0: passthrough uses implicit buffer lengths   [0.441335]  (+148.9 ms)
     nvme0n1: p1 p2 p3 p4                                   +67.0 ms
    EXT4-fs (nvme0n1p3): mounted filesystem                +104.7 ms

That boot was not traced, so whether the reset outlasted the 100 ms head
start or the enable was slow is unknown.

Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/pci/quirks.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Note, assisted by claude-opus-5-5 (found, traced and drafted)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 7f1f47ea2ee41..e6535bcb0553e 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4187,6 +4187,44 @@ static int nvme_disable_and_flr(struct pci_dev *dev, bool probe)
 	return 0;
 }
 
+/*
+ * The firmware leaves the NVMe controller enabled, so nvme-pci starts by
+ * resetting it (CC.EN 1 -> 0) to set up its own admin queue.  The WD PC
+ * SN730 (firmware 11170012) usually completes that within 2 ms, but in
+ * about one boot in ten it takes 80 to 600 ms, and the enable and the
+ * first reads after it are slow as well, delaying the root file system.
+ * Start the reset during enumeration, so it runs in parallel with the
+ * rest of the boot, and nvme_disable_ctrl() finds the controller disabled.
+ * Like nvme_disable_ctrl(), skip the shutdown notification, as it could
+ * complete commands into memory the firmware owned.
+ */
+static void quirk_nvme_disable_early(struct pci_dev *dev)
+{
+	void __iomem *bar;
+	u16 cmd;
+	u32 cc;
+
+	if (dev->class != PCI_CLASS_STORAGE_EXPRESS || !pci_resource_start(dev, 0))
+		return;
+
+	pci_read_config_word(dev, PCI_COMMAND, &cmd);
+	if (!(cmd & PCI_COMMAND_MEMORY))
+		return;
+
+	bar = pci_iomap(dev, 0, NVME_REG_CSTS + sizeof(u32));
+	if (!bar)
+		return;
+
+	cc = readl(bar + NVME_REG_CC);
+	if (cc != ~0U && (cc & NVME_CC_ENABLE)) {
+		writel(cc & ~(NVME_CC_SHN_MASK | NVME_CC_ENABLE), bar + NVME_REG_CC);
+		pci_info(dev, "NVMe controller reset started early\n");
+	}
+
+	pci_iounmap(dev, bar);
+}
+DECLARE_PCI_FIXUP_FINAL(0x15b7, 0x5006, quirk_nvme_disable_early);
+
 /*
  * Some NVMe controllers such as Intel DC P3700 and Solidigm P44 Pro will
  * timeout waiting for ready status to change after NVMe enable if the driver
-- 
2.54.0