Commit 5cd76349 authored by Seth Howell's avatar Seth Howell Committed by Jim Harris
Browse files

nvme_ctrlr: enable the admin qpair before init. 



The driver has historically waited until we have to do a listen
before enabling the admin qpair. That is a very PCIe-centric mindset.
For fabric controllers, a lot of the early initialization operations such
as get_cc and set_cc are handled through the admin qpair so it should be
enabled before we begin the initialization process.

As a side effect of this cahnge, the internal API
nvme_ctrlr_enable_admin_qpair has been removed. It would have turned
into a one-liner.

Change-Id: Icd162657d01a85c227a3f20c295d0208e07ce44d
Signed-off-by: default avatarSeth Howell <seth.howell@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471743


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarAlexey Marchuk <alexeymar@mellanox.com>
parent fa9f668a

lib/nvme/nvme.c

+1 −0
Original line number Diff line number Diff line
@@ -429,6 +429,7 @@ nvme_ctrlr_probe(const struct spdk_nvme_transport_id *trid, 
		ctrlr->remove_cb = probe_ctx->remove_cb;

		ctrlr->cb_ctx = probe_ctx->cb_ctx;



		nvme_qpair_enable(ctrlr->adminq);

		TAILQ_INSERT_TAIL(&probe_ctx->init_ctrlrs, ctrlr, tailq);

		return 0;

	}

lib/nvme/nvme_ctrlr.c

+6 −12
Original line number Diff line number Diff line
@@ -769,8 +769,8 @@ nvme_ctrlr_state_string(enum nvme_ctrlr_state state) 
		return "enable controller by writing CC.EN = 1";

	case NVME_CTRLR_STATE_ENABLE_WAIT_FOR_READY_1:

		return "wait for CSTS.RDY = 1";

	case NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE:

		return "enable admin queue";

	case NVME_CTRLR_STATE_RESET_ADMIN_QUEUE:

		return "reset admin queue";

	case NVME_CTRLR_STATE_IDENTIFY:

		return "identify controller";

	case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY:
@@ -1006,6 +1006,7 @@ nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr) 
	/* Set the state back to INIT to cause a full hardware reset. */

	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_INIT, NVME_TIMEOUT_INFINITE);



	nvme_qpair_enable(ctrlr->adminq);

	while (ctrlr->state != NVME_CTRLR_STATE_READY) {

		if (nvme_ctrlr_process_init(ctrlr) != 0) {

			SPDK_ERRLOG("controller reinitialization failed\n");
@@ -2083,13 +2084,6 @@ nvme_ctrlr_proc_get_devhandle(struct spdk_nvme_ctrlr *ctrlr) 
	return devhandle;

}



static void

nvme_ctrlr_enable_admin_queue(struct spdk_nvme_ctrlr *ctrlr)

{

	nvme_transport_qpair_reset(ctrlr->adminq);

	nvme_qpair_enable(ctrlr->adminq);

}



/**

 * This function will be called repeatedly during initialization until the controller is ready.

 */
@@ -2232,14 +2226,14 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) 
			 * The controller has been enabled.

			 *  Perform the rest of initialization serially.

			 */

			nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE,

			nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_RESET_ADMIN_QUEUE,

					     ctrlr->opts.admin_timeout_ms);

			return 0;

		}

		break;



	case NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE:

		nvme_ctrlr_enable_admin_queue(ctrlr);

	case NVME_CTRLR_STATE_RESET_ADMIN_QUEUE:

		nvme_transport_qpair_reset(ctrlr->adminq);

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY,

				     ctrlr->opts.admin_timeout_ms);

		break;

lib/nvme/nvme_internal.h

+2 −2
Original line number Diff line number Diff line
@@ -433,9 +433,9 @@ enum nvme_ctrlr_state { 
	NVME_CTRLR_STATE_ENABLE_WAIT_FOR_READY_1,



	/**

	 * Enable the Admin queue of the controller.

	 * Reset the Admin queue of the controller.

	 */

	NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE,

	NVME_CTRLR_STATE_RESET_ADMIN_QUEUE,



	/**

	 * Identify Controller command will be sent to then controller.

test/unit/lib/nvme/nvme.c/nvme_ut.c

+9 −0
Original line number Diff line number Diff line
@@ -68,6 +68,12 @@ nvme_ctrlr_destruct(struct spdk_nvme_ctrlr *ctrlr) 
	ut_destruct_called = true;

}



void

nvme_qpair_enable(struct spdk_nvme_qpair *qpair)

{

	qpair->is_enabled = true;

}



void

spdk_nvme_ctrlr_get_default_ctrlr_opts(struct spdk_nvme_ctrlr_opts *opts, size_t opts_size)

{
@@ -722,12 +728,15 @@ test_nvme_ctrlr_probe(void) 
{

	int rc = 0;

	struct spdk_nvme_ctrlr ctrlr = {};

	struct spdk_nvme_qpair qpair = {};

	const struct spdk_nvme_transport_id trid = {};

	struct spdk_nvme_probe_ctx probe_ctx = {};

	void *devhandle = NULL;

	void *cb_ctx = NULL;

	struct spdk_nvme_ctrlr *dummy = NULL;



	ctrlr.adminq = &qpair;



	TAILQ_INIT(&probe_ctx.init_ctrlrs);

	nvme_driver_init();

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

+8 −8
Original line number Diff line number Diff line
@@ -525,7 +525,7 @@ test_nvme_ctrlr_init_en_1_rdy_0(void) 
	 */

	g_ut_nvme_regs.csts.bits.rdy = 1;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_RESET_ADMIN_QUEUE);



	/*

	 * Transition to READY.
@@ -579,7 +579,7 @@ test_nvme_ctrlr_init_en_1_rdy_1(void) 
	 */

	g_ut_nvme_regs.csts.bits.rdy = 1;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_RESET_ADMIN_QUEUE);



	/*

	 * Transition to READY.
@@ -754,7 +754,7 @@ test_nvme_ctrlr_init_en_0_rdy_0_ams_rr(void) 
	 */

	g_ut_nvme_regs.csts.bits.rdy = 1;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_RESET_ADMIN_QUEUE);



	/*

	 * Transition to READY.
@@ -931,7 +931,7 @@ test_nvme_ctrlr_init_en_0_rdy_0_ams_wrr(void) 
	 */

	g_ut_nvme_regs.csts.bits.rdy = 1;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_RESET_ADMIN_QUEUE);



	/*

	 * Transition to READY.
@@ -1107,7 +1107,7 @@ test_nvme_ctrlr_init_en_0_rdy_0_ams_vs(void) 
	 */

	g_ut_nvme_regs.csts.bits.rdy = 1;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_RESET_ADMIN_QUEUE);



	/*

	 * Transition to READY.
@@ -1153,7 +1153,7 @@ test_nvme_ctrlr_init_en_0_rdy_0(void) 
	 */

	g_ut_nvme_regs.csts.bits.rdy = 1;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_RESET_ADMIN_QUEUE);



	/*

	 * Transition to READY.
@@ -1205,7 +1205,7 @@ test_nvme_ctrlr_init_en_0_rdy_1(void) 
	 */

	g_ut_nvme_regs.csts.bits.rdy = 1;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_RESET_ADMIN_QUEUE);



	/*

	 * Transition to READY.
@@ -1829,7 +1829,7 @@ test_nvme_ctrlr_init_delay(void) 
	 */

	g_ut_nvme_regs.csts.bits.rdy = 1;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ENABLE_ADMIN_QUEUE);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_RESET_ADMIN_QUEUE);



	/*

	 * Transition to READY.