[PATCH] nvme-pci: try function level reset on init failure

[PATCH] nvme-pci: try function level reset on init failure

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

I've encountered various nvme devices that for whatever reason are stuck
in a reset state. Historically these have required a power cycle to make
them usable again. Vendors don't report any problem with the device when
we ship these for analysis.

In many cases, a PCIe FLR is sufficient to restart operation without a
power cycle. Try it if controller reset fails the first time.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/pci.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 4cf87fb5d8573..85749f19e3a23 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2067,8 +2067,18 @@ static int nvme_pci_configure_admin_queue(struct nvme_dev *dev)
 	 * might be pointing at!
 	 */
 	result = nvme_disable_ctrl(&dev->ctrl, false);
-	if (result < 0)
-		return result;
+	if (result < 0) {
+		struct pci_dev *pdev = to_pci_dev(dev->dev);
+
+		result = pcie_flr(pdev);
+		if (result < 0)
+			return result;
+		pci_restore_state(pdev);
+
+		result = nvme_disable_ctrl(&dev->ctrl, false);
+		if (result < 0)
+			return result;
+	}
 
 	result = nvme_alloc_queue(dev, 0, NVME_AQ_DEPTH);
 	if (result)
-- 
2.47.1

Re: [PATCH] nvme-pci: try function level reset on init failure

[Chaitanya Kulkarni]

On 7/14/25 10:13, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> I've encountered various nvme devices that for whatever reason are stuck
> in a reset state. Historically these have required a power cycle to make
> them usable again. Vendors don't report any problem with the device when
> we ship these for analysis.
>
> In many cases, a PCIe FLR is sufficient to restart operation without a
> power cycle. Try it if controller reset fails the first time.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>

I hope there are no setups that are relying on this failure and okay
with implicit PCIe FLR and any side-effects it might have,
but I think that is okay, looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

> ---
>   drivers/nvme/host/pci.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 4cf87fb5d8573..85749f19e3a23 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2067,8 +2067,18 @@ static int nvme_pci_configure_admin_queue(struct nvme_dev *dev)
>   	 * might be pointing at!
>   	 */
>   	result = nvme_disable_ctrl(&dev->ctrl, false);
> -	if (result < 0)
> -		return result;
> +	if (result < 0) {
> +		struct pci_dev *pdev = to_pci_dev(dev->dev);
> +
> +		result = pcie_flr(pdev);
> +		if (result < 0)
> +			return result;
> +		pci_restore_state(pdev);
> +
> +		result = nvme_disable_ctrl(&dev->ctrl, false);
> +		if (result < 0)
> +			return result;
> +	}
>   
>   	result = nvme_alloc_queue(dev, 0, NVME_AQ_DEPTH);
>   	if (result)

Re: [PATCH] nvme-pci: try function level reset on init failure

[Christoph Hellwig]

On Mon, Jul 14, 2025 at 10:13:28AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> I've encountered various nvme devices that for whatever reason are stuck
> in a reset state. Historically these have required a power cycle to make
> them usable again. Vendors don't report any problem with the device when
> we ship these for analysis.

Who is the "we" here?

> In many cases, a PCIe FLR is sufficient to restart operation without a
> power cycle. Try it if controller reset fails the first time.

Why is that only done in the probe path and not the runtime reset path?

> +	if (result < 0) {
> +		struct pci_dev *pdev = to_pci_dev(dev->dev);
> +
> +		result = pcie_flr(pdev);
> +		if (result < 0)
> +			return result;
> +		pci_restore_state(pdev);
> +
> +		result = nvme_disable_ctrl(&dev->ctrl, false);
> +		if (result < 0)
> +			return result;
> +	}

Either way this warrants a big comment explaining what we are doing
here.

Re: [PATCH] nvme-pci: try function level reset on init failure

[Keith Busch]

On Tue, Jul 15, 2025 at 09:45:58AM +0200, Christoph Hellwig wrote:
> On Mon, Jul 14, 2025 at 10:13:28AM -0700, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> > 
> > I've encountered various nvme devices that for whatever reason are stuck
> > in a reset state. Historically these have required a power cycle to make
> > them usable again. Vendors don't report any problem with the device when
> > we ship these for analysis.
> 
> Who is the "we" here?

Meta.
 
> > In many cases, a PCIe FLR is sufficient to restart operation without a
> > power cycle. Try it if controller reset fails the first time.
> 
> Why is that only done in the probe path and not the runtime reset path?

nvme_pci_configure_admin_queue() is called for both probe and
reset_work.

Is it because I wrote "fails the first time"? I mean the first reset for
each initialization attempt, whether it happens during probe or a later
reset. The code path will try an FLR on every single nvme reset if
CSTS.RDY doesn't clear as expected.

> > +	if (result < 0) {
> > +		struct pci_dev *pdev = to_pci_dev(dev->dev);
> > +
> > +		result = pcie_flr(pdev);
> > +		if (result < 0)
> > +			return result;
> > +		pci_restore_state(pdev);
> > +
> > +		result = nvme_disable_ctrl(&dev->ctrl, false);
> > +		if (result < 0)
> > +			return result;
> > +	}
> 
> Either way this warrants a big comment explaining what we are doing
> here.

Sure, no problem. I think also a dev_warn() if the 2nd disable_ctrl call
was successful to indicate an FLR was needed to get an expected
response.