Commit 32e22643 authored by Jim Harris's avatar Jim Harris
Browse files

nvme: add NVME_QUIRK_DELAY_BEFORE_INIT quirk 



Currently we *always* wait 2 seconds before starting
controller initialization during attach.  This
works around an issue where some older Intel NVMe SSDs
could not handle MMIO writes too soon after a PCIe
FLR (which would be triggered when VFIO was enabled).

After further discussion with Intel experts, we know
the SSD models that exhibit this issue.  So we can
quirk this so that only the older SSDs incur the extra
delay.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: Ieb408c24f6afd5bd5147d1c87239aa20f2d13511

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/466064


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarBroadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
parent d1d69a16

lib/nvme/nvme_ctrlr.c

+12 −12
Original line number Diff line number Diff line
@@ -2064,6 +2064,7 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) 
	switch (ctrlr->state) {

	case NVME_CTRLR_STATE_INIT_DELAY:

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_INIT, ready_timeout_in_ms);

		if (ctrlr->quirks & NVME_QUIRK_DELAY_BEFORE_INIT) {

			/*

			 * Controller may need some delay before it's enabled.

			 *
@@ -2071,11 +2072,10 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) 
			 * is not ready after VFIO reset. We delay the initialization rather than the

			 * enabling itself, because this is required only for the very first enabling

			 * - directly after a VFIO reset.

		 *

		 * TODO: Figure out what is actually going wrong.

			 */

			SPDK_DEBUGLOG(SPDK_LOG_NVME, "Adding 2 second delay before initializing the controller\n");

			ctrlr->sleep_timeout_tsc = spdk_get_ticks() + (2000 * spdk_get_ticks_hz() / 1000);

		}

		break;



	case NVME_CTRLR_STATE_INIT:

lib/nvme/nvme_internal.h

+6 −0
Original line number Diff line number Diff line
@@ -123,6 +123,12 @@ extern pid_t g_spdk_nvme_pid; 
 */

#define NVME_QUIRK_SHST_COMPLETE 0x200



/*

 * The controller requires an extra delay before starting the initialization process

 * during attach.

 */

#define NVME_QUIRK_DELAY_BEFORE_INIT 0x400



#define NVME_MAX_ASYNC_EVENTS	(8)



#define NVME_MAX_ADMIN_TIMEOUT_IN_SECS	(30)

lib/nvme/nvme_quirks.c

+4 −2
Original line number Diff line number Diff line
@@ -43,13 +43,15 @@ static const struct nvme_quirk nvme_quirks[] = { 
		NVME_INTEL_QUIRK_READ_LATENCY |

		NVME_INTEL_QUIRK_WRITE_LATENCY |

		NVME_INTEL_QUIRK_STRIPING |

		NVME_QUIRK_READ_ZERO_AFTER_DEALLOCATE

		NVME_QUIRK_READ_ZERO_AFTER_DEALLOCATE |

		NVME_QUIRK_DELAY_BEFORE_INIT

	},

	{	{SPDK_PCI_VID_INTEL, 0x0A53, SPDK_PCI_ANY_ID, SPDK_PCI_ANY_ID},

		NVME_INTEL_QUIRK_READ_LATENCY |

		NVME_INTEL_QUIRK_WRITE_LATENCY |

		NVME_INTEL_QUIRK_STRIPING |

		NVME_QUIRK_READ_ZERO_AFTER_DEALLOCATE

		NVME_QUIRK_READ_ZERO_AFTER_DEALLOCATE |

		NVME_QUIRK_DELAY_BEFORE_INIT

	},

	{	{SPDK_PCI_VID_INTEL, 0x0A54, SPDK_PCI_ANY_ID, SPDK_PCI_ANY_ID},

		NVME_INTEL_QUIRK_READ_LATENCY |

test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c

+5 −1
Original line number Diff line number Diff line
@@ -1793,8 +1793,12 @@ test_nvme_ctrlr_init_delay(void) 
	g_ut_nvme_regs.cc.bits.en = 0;

	g_ut_nvme_regs.csts.bits.rdy = 0;



	/* Delay initiation and default time is 2s */

	SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0);

	/* Test that the initialization delay works correctly.  We only

	 * do the initialization delay on SSDs that require it, so

	 * set that quirk here.

	 */

	ctrlr.quirks = NVME_QUIRK_DELAY_BEFORE_INIT;

	ctrlr.cdata.nn = 1;

	ctrlr.page_size = 0x1000;

	ctrlr.state = NVME_CTRLR_STATE_INIT_DELAY;