Commit 60ce1414 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

nvme: Use Host Behavior Support Feature to enable LBA Format Extension 



If the Extended LBA Formats Supported (ELBAS) is 1, we should set
the LBA Format Extension Enable (LBAFEE) to 1 to get the Extended LBA
formats.

We already have a state NVME_CTRLR_STATE_SET_SUPPORTED_FEATURES and
a function nvme_ctrlr_set_supported_features(). However, they are
implemented synchronously.

Synchronous implementation has potential deadlock bug when used in
cluster SPDK target applications.

Hence, we implement the host behavior support feature asynchronously.

In future, other features may be re-implemented asynchronously too.

feature_supported[SPDK_NVME_FEAT_HOST_BEHAVIOR_SUPPORT] will not be
queried explicitly but set it to true if LBAFEE is set to 1
successfully. In this case, we can say that the Host Behavior Support
Feature is actually enabled.

Add unit test case for simple verification.

Signed-off-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Change-Id: I88e8367277c861e98bd3568713f19d307802be39
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/23620


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Community-CI: Mellanox Build Bot
parent 457d0f6d

lib/nvme/nvme_ctrlr.c

+78 −1
Original line number Diff line number Diff line
@@ -1005,6 +1005,74 @@ nvme_ctrlr_set_supported_features(struct spdk_nvme_ctrlr *ctrlr) 
	nvme_ctrlr_set_arbitration_feature(ctrlr);

}



static void

nvme_ctrlr_set_host_feature_done(void *arg, const struct spdk_nvme_cpl *cpl)

{

	struct spdk_nvme_ctrlr *ctrlr = (struct spdk_nvme_ctrlr *)arg;



	spdk_free(ctrlr->tmp_ptr);

	ctrlr->tmp_ptr = NULL;



	if (spdk_nvme_cpl_is_error(cpl)) {

		NVME_CTRLR_ERRLOG(ctrlr, "Set host behavior support feature failed: SC %x SCT %x\n",

				  cpl->status.sc, cpl->status.sct);

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_ERROR, NVME_TIMEOUT_INFINITE);

		return;

	}



	ctrlr->feature_supported[SPDK_NVME_FEAT_HOST_BEHAVIOR_SUPPORT] = true;



	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_DB_BUF_CFG,

			     ctrlr->opts.admin_timeout_ms);

}



/* We do not want to do add synchronous operation anymore.

 * We set the Host Behavior Support feature asynchronousin in different states.

 */

static int

nvme_ctrlr_set_host_feature(struct spdk_nvme_ctrlr *ctrlr)

{

	struct spdk_nvme_host_behavior *host;

	int rc;



	if (!ctrlr->cdata.ctratt.bits.elbas) {

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_DB_BUF_CFG,

				     ctrlr->opts.admin_timeout_ms);

		return 0;

	}



	ctrlr->tmp_ptr = spdk_dma_zmalloc(sizeof(struct spdk_nvme_host_behavior), 4096, NULL);

	if (!ctrlr->tmp_ptr) {

		NVME_CTRLR_ERRLOG(ctrlr, "Failed to allocate host behavior support data\n");

		rc = -ENOMEM;

		goto error;

	}



	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_WAIT_FOR_SET_HOST_FEATURE,

			     ctrlr->opts.admin_timeout_ms);



	host = ctrlr->tmp_ptr;



	host->lbafee = 1;



	rc = spdk_nvme_ctrlr_cmd_set_feature(ctrlr, SPDK_NVME_FEAT_HOST_BEHAVIOR_SUPPORT,

					     0, 0, host, sizeof(struct spdk_nvme_host_behavior),

					     nvme_ctrlr_set_host_feature_done, ctrlr);

	if (rc != 0) {

		NVME_CTRLR_ERRLOG(ctrlr, "Set host behavior support feature failed: %d\n", rc);

		goto error;

	}



	return 0;



error:

	spdk_free(ctrlr->tmp_ptr);

	ctrlr->tmp_ptr = NULL;



	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_ERROR, NVME_TIMEOUT_INFINITE);

	return rc;

}



bool

spdk_nvme_ctrlr_is_failed(struct spdk_nvme_ctrlr *ctrlr)

{
@@ -1422,6 +1490,10 @@ nvme_ctrlr_state_string(enum nvme_ctrlr_state state) 
		return "wait for supported INTEL log pages";

	case NVME_CTRLR_STATE_SET_SUPPORTED_FEATURES:

		return "set supported features";

	case NVME_CTRLR_STATE_SET_HOST_FEATURE:

		return "set host behavior support feature";

	case NVME_CTRLR_STATE_WAIT_FOR_SET_HOST_FEATURE:

		return "wait for set host behavior support feature";

	case NVME_CTRLR_STATE_SET_DB_BUF_CFG:

		return "set doorbell buffer config";

	case NVME_CTRLR_STATE_WAIT_FOR_DB_BUF_CFG:
@@ -4013,10 +4085,14 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) 


	case NVME_CTRLR_STATE_SET_SUPPORTED_FEATURES:

		nvme_ctrlr_set_supported_features(ctrlr);

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_DB_BUF_CFG,

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_HOST_FEATURE,

				     ctrlr->opts.admin_timeout_ms);

		break;



	case NVME_CTRLR_STATE_SET_HOST_FEATURE:

		rc = nvme_ctrlr_set_host_feature(ctrlr);

		break;



	case NVME_CTRLR_STATE_SET_DB_BUF_CFG:

		rc = nvme_ctrlr_set_doorbell_buffer_config(ctrlr);

		break;
@@ -4062,6 +4138,7 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) 
	case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_ID_DESCS:

	case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_NS_IOCS_SPECIFIC:

	case NVME_CTRLR_STATE_WAIT_FOR_SUPPORTED_INTEL_LOG_PAGES:

	case NVME_CTRLR_STATE_WAIT_FOR_SET_HOST_FEATURE:

	case NVME_CTRLR_STATE_WAIT_FOR_DB_BUF_CFG:

	case NVME_CTRLR_STATE_WAIT_FOR_HOST_ID:

		/*

lib/nvme/nvme_internal.h

+10 −0
Original line number Diff line number Diff line
@@ -824,6 +824,16 @@ enum nvme_ctrlr_state { 
	 */

	NVME_CTRLR_STATE_SET_SUPPORTED_FEATURES,



	/**

	 * Set the Host Behavior Support feature of the controller.

	 */

	NVME_CTRLR_STATE_SET_HOST_FEATURE,



	/**

	 * Waiting for the Host Behavior Support feature of the controller.

	 */

	NVME_CTRLR_STATE_WAIT_FOR_SET_HOST_FEATURE,



	/**

	 * Set Doorbell Buffer Config of the controller.

	 */

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

+28 −0
Original line number Diff line number Diff line
@@ -332,6 +332,7 @@ spdk_nvme_ctrlr_cmd_set_feature(struct spdk_nvme_ctrlr *ctrlr, uint8_t feature, 
				spdk_nvme_cmd_cb cb_fn, void *cb_arg)

{

	g_ut_cdw11 = cdw11;

	fake_cpl_sc(cb_fn, cb_arg);

	return 0;

}
@@ -1912,6 +1913,32 @@ test_nvme_ctrlr_set_supported_features(void) 
	CU_ASSERT(res == true);

}



static void

test_nvme_ctrlr_set_host_feature(void)

{

	DECLARE_AND_CONSTRUCT_CTRLR();



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



	ctrlr.cdata.ctratt.bits.elbas = 0;

	ctrlr.state = NVME_CTRLR_STATE_SET_HOST_FEATURE;



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

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_DB_BUF_CFG);



	ctrlr.cdata.ctratt.bits.elbas = 1;

	ctrlr.state = NVME_CTRLR_STATE_SET_HOST_FEATURE;



	while (ctrlr.state != NVME_CTRLR_STATE_SET_DB_BUF_CFG) {

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

	}



	CU_ASSERT(ctrlr.tmp_ptr == NULL);

	CU_ASSERT(ctrlr.feature_supported[SPDK_NVME_FEAT_HOST_BEHAVIOR_SUPPORT] == true);



	nvme_ctrlr_destruct(&ctrlr);

}



static void

test_ctrlr_get_default_ctrlr_opts(void)

{
@@ -3399,6 +3426,7 @@ main(int argc, char **argv) 
	CU_ADD_TEST(suite, test_nvme_ctrlr_fail);

	CU_ADD_TEST(suite, test_nvme_ctrlr_construct_intel_support_log_page_list);

	CU_ADD_TEST(suite, test_nvme_ctrlr_set_supported_features);

	CU_ADD_TEST(suite, test_nvme_ctrlr_set_host_feature);

	CU_ADD_TEST(suite, test_spdk_nvme_ctrlr_doorbell_buffer_config);

#if 0 /* TODO: move to PCIe-specific unit test */

	CU_ADD_TEST(suite, test_nvme_ctrlr_alloc_cmb);