Re: [PATCH] nvme-pci: skip FLR after a failed controller reset

[Haowen Bai <calvin.bai@ugreen.com>]

Keith,

Thanks for the review.

The goal on our side is that a dead NVMe function must not hard-lock
the host. Losing the device (and bcache) is acceptable; an NMI
lockup of unrelated PCI users is not. I don't want to take FLR away
from a path that still recovers some devices — I want teardown not to
pin pci_config_lock across a hung config cycle.

> Shouldn't PCIe CTO have kicked in to fail the transaction? Do you
> know which transaction is failing? Is the stall specific to FLR or
> could any config access stall in your setup?

I don't know which config cycle is stuck. There is no vmcore / lock
owner. The NMI captures the waiter (an unrelated eMMC runtime-resume
spinning in pci_conf1_read -> acpi_pci_set_power_state), not the
holder.

What the pstore timestamps do show:

  [t+0]     nvme_wait_ready timeout, CSTS=0x1   (MMIO, first disable)
  [t+128s]  nvme_wait_ready timeout, CSTS=0x1   (second disable after FLR)
  [t+139s]  hard lockup on pci_config_lock

So the 128s gap is CAP.TO on the second nvme_disable_ctrl(), which is
MMIO and does not take pci_config_lock. The lockup is ~11s after that
returns, i.e. on the post-FLR teardown path (pci_free_irq_vectors /
pci_disable_device or a config access still in flight), not inside
nvme_wait_ready().

I cannot prove the stall is unique to the FLR write vs any later
config access to that function. Both events went through
disable-timeout -> FLR -> disable-timeout -> teardown. Linux 6.12 has
no FLR fallback here; the same class of NVMe drop usually only took
the cache offline. That is correlation, not a single-cycle trace.

There is no AER / UR / completion-timeout message in the log. I do
not know whether CTO was disabled, longer than the NMI watchdog
(~10s), or not applicable because the root port never completed. I
won't claim CTO is broken. Even if CTO should have aborted the
cycle, it did not save the machine here.

> Are you able to fix the device instead? Maybe add your device to the
> "quirk_no_flr" list if you can't fix it.

The two ZHITAI Ti600 functions (1e49:0081) each reproduced the same
sequence independently, so I agree this is a nasty device bug. We
are taking them out of the bcache path on the affected machines.

quirk_no_flr would stop nvme from requesting FLR, but it would not
stop pci_disable_device() from touching config on the way out, which
is where the lockup lines up. Pinning host protection to one VID:DID
also misses the next broken device. A bad endpoint should be allowed
to die; it should not be able to stall pci_config_lock and take the
rest of the platform with it. I can add a quirk as a device note if
you want it on record; I don't think it is the host fix.

> I've seen FLR recover devices both on first probe and IO timeout, so
> skipping for RESETTING will miss recovering when it was possible

Agreed — that makes v1 too broad. I'll drop the RESETTING special
case rather than take FLR away from a path that still recovers some
devices.

If a v2 is useful, I think it needs to stop issuing config cycles to
a function that already failed CC.EN and FLR (so teardown cannot
hold pci_config_lock across a hung inl), without skipping FLR on the
reset path. I have not written that patch yet.

Thanks,
Haowen