From 04cd292237bff7fe0841c6e70a2ea07c873a115e Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Sun, 21 Mar 2021 21:52:40 -0400 Subject: [PATCH] nvme/quirk: add MDTS excludes interleaved metadata quirk The specification for Maximum Data Transfer Size (MDTS) says this field should include the length of metadata, if metadata is interleaved with the logical block data. However, some drives can support MDTS without counting the interleaved metadata, so for this case SPDK will only use data length without interleaved metadata length. Change-Id: I29920a25885699e2689be043b87122367be0e416 Signed-off-by: Changpeng Liu Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6813 Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk --- lib/nvme/nvme_internal.h | 5 ++++ lib/nvme/nvme_ns.c | 3 ++ lib/nvme/nvme_quirks.c | 3 ++ test/unit/lib/nvme/nvme_ns.c/nvme_ns_ut.c | 24 ++++++++++------ .../lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c | 28 +++++++++++++++++++ 5 files changed, 54 insertions(+), 9 deletions(-) diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 74cf12875..e39dc8caf 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -156,6 +156,11 @@ extern pid_t g_spdk_nvme_pid; */ #define NVME_QUIRK_NO_SGL_FOR_DSM 0x4000 +/** + * Maximum Data Transfer Size(MDTS) excludes interleaved metadata. + */ +#define NVME_QUIRK_MDTS_EXCLUDE_MD 0x8000 + #define NVME_MAX_ASYNC_EVENTS (8) #define NVME_MAX_ADMIN_TIMEOUT_IN_SECS (30) diff --git a/lib/nvme/nvme_ns.c b/lib/nvme/nvme_ns.c index 7903c8423..21bab4b74 100644 --- a/lib/nvme/nvme_ns.c +++ b/lib/nvme/nvme_ns.c @@ -65,6 +65,9 @@ nvme_ns_set_identify_data(struct spdk_nvme_ns *ns) ns->sectors_per_max_io = spdk_nvme_ns_get_max_io_xfer_size(ns) / ns->extended_lba_size; ns->sectors_per_max_io_no_md = spdk_nvme_ns_get_max_io_xfer_size(ns) / ns->sector_size; + if (ns->ctrlr->quirks & NVME_QUIRK_MDTS_EXCLUDE_MD) { + ns->sectors_per_max_io = ns->sectors_per_max_io_no_md; + } if (nsdata->noiob) { ns->sectors_per_stripe = nsdata->noiob; diff --git a/lib/nvme/nvme_quirks.c b/lib/nvme/nvme_quirks.c index c499ae0b0..9f3b98c21 100644 --- a/lib/nvme/nvme_quirks.c +++ b/lib/nvme/nvme_quirks.c @@ -104,6 +104,9 @@ static const struct nvme_quirk nvme_quirks[] = { { {SPDK_PCI_CLASS_NVME, SPDK_PCI_VID_INTEL, 0x2700, SPDK_PCI_ANY_ID, SPDK_PCI_ANY_ID}, NVME_QUIRK_OACS_SECURITY }, + { {SPDK_PCI_CLASS_NVME, SPDK_PCI_VID_INTEL, 0x4140, SPDK_PCI_ANY_ID, SPDK_PCI_ANY_ID}, + NVME_QUIRK_MDTS_EXCLUDE_MD + }, { {0x000000, 0x0000, 0x0000, 0x0000, 0x0000}, 0} }; diff --git a/test/unit/lib/nvme/nvme_ns.c/nvme_ns_ut.c b/test/unit/lib/nvme/nvme_ns.c/nvme_ns_ut.c index e39157709..357cbd293 100644 --- a/test/unit/lib/nvme/nvme_ns.c/nvme_ns_ut.c +++ b/test/unit/lib/nvme/nvme_ns.c/nvme_ns_ut.c @@ -257,27 +257,27 @@ test_nvme_ns_set_identify_data(void) ns.ctrlr->cdata.vwc.present = 1; ns.ctrlr->cdata.oncs.write_zeroes = 1; ns.ctrlr->cdata.oncs.write_unc = 1; - ns.ctrlr->min_page_size = 1024; - ns.ctrlr->max_xfer_size = 65536; + ns.ctrlr->min_page_size = 4096; + ns.ctrlr->max_xfer_size = 131072; ns.nsdata.flbas.extended = 1; ns.nsdata.nsrescap.raw = 1; ns.nsdata.dps.pit = SPDK_NVME_FMT_NVM_PROTECTION_TYPE1; ns.nsdata.flbas.format = 0; ns.nsdata.lbaf[0].lbads = 9; - ns.nsdata.lbaf[0].ms = 512; + ns.nsdata.lbaf[0].ms = 8; - /* case1: nsdata->noiob > 0 */ + /* case 1: nsdata->noiob > 0 */ ns.nsdata.noiob = 1; nvme_ns_set_identify_data(&ns); CU_ASSERT(spdk_nvme_ns_get_optimal_io_boundary(&ns) == 1) CU_ASSERT(spdk_nvme_ns_get_sector_size(&ns) == 512); - CU_ASSERT(spdk_nvme_ns_get_extended_sector_size(&ns) == 1024); - CU_ASSERT(spdk_nvme_ns_get_md_size(&ns) == 512); - CU_ASSERT(spdk_nvme_ns_get_max_io_xfer_size(&ns) == 65536); - CU_ASSERT(ns.sectors_per_max_io == 64); - CU_ASSERT(ns.sectors_per_max_io_no_md == 128); + CU_ASSERT(spdk_nvme_ns_get_extended_sector_size(&ns) == 520); + CU_ASSERT(spdk_nvme_ns_get_md_size(&ns) == 8); + CU_ASSERT(spdk_nvme_ns_get_max_io_xfer_size(&ns) == 131072); + CU_ASSERT(ns.sectors_per_max_io == 252); + CU_ASSERT(ns.sectors_per_max_io_no_md == 256); CU_ASSERT(spdk_nvme_ns_get_pi_type(&ns) == SPDK_NVME_FMT_NVM_PROTECTION_TYPE1); CU_ASSERT(spdk_nvme_ns_get_flags(&ns) & SPDK_NVME_NS_EXTENDED_LBA_SUPPORTED); @@ -288,6 +288,12 @@ test_nvme_ns_set_identify_data(void) CU_ASSERT(spdk_nvme_ns_get_flags(&ns) & SPDK_NVME_NS_WRITE_UNCORRECTABLE_SUPPORTED); CU_ASSERT(spdk_nvme_ns_get_flags(&ns) & SPDK_NVME_NS_RESERVATION_SUPPORTED); CU_ASSERT(spdk_nvme_ns_get_flags(&ns) & SPDK_NVME_NS_DPS_PI_SUPPORTED); + + /* case 2: quirks for NVME_QUIRK_MDTS_EXCLUDE_MD */ + ns.ctrlr->quirks = NVME_QUIRK_MDTS_EXCLUDE_MD; + nvme_ns_set_identify_data(&ns); + CU_ASSERT(ns.sectors_per_max_io == 256); + CU_ASSERT(ns.sectors_per_max_io_no_md == 256); } static void diff --git a/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c b/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c index 0e165aa97..513ddc4a6 100644 --- a/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c +++ b/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c @@ -241,6 +241,9 @@ prepare_for_test(struct spdk_nvme_ns *ns, struct spdk_nvme_ctrlr *ctrlr, ns->md_size = md_size; ns->sectors_per_max_io = spdk_nvme_ns_get_max_io_xfer_size(ns) / ns->extended_lba_size; ns->sectors_per_max_io_no_md = spdk_nvme_ns_get_max_io_xfer_size(ns) / ns->sector_size; + if (ctrlr->quirks & NVME_QUIRK_MDTS_EXCLUDE_MD) { + ns->sectors_per_max_io = ns->sectors_per_max_io_no_md; + } ns->sectors_per_stripe = stripe_size / ns->extended_lba_size; memset(qpair, 0, sizeof(*qpair)); @@ -1319,6 +1322,31 @@ test_nvme_ns_cmd_write_with_md(void) nvme_free_request(g_request); cleanup_after_test(&qpair); + /* + * 512 byte data + 128 byte metadata + * Extended LBA + * Max data transfer size 128 KB + * No stripe size + * Enable NVME_QUIRK_MDTS_EXCLUDE_MD quirk + * + * 256 blocks * 512 bytes per block = single 128 KB I/O (no splitting required) + */ + ctrlr.quirks = NVME_QUIRK_MDTS_EXCLUDE_MD; + prepare_for_test(&ns, &ctrlr, &qpair, 512, 128, 128 * 1024, 0, true); + + rc = spdk_nvme_ns_cmd_write_with_md(&ns, &qpair, buffer, NULL, 0x1000, 256, NULL, NULL, 0, 0, + 0); + + SPDK_CU_ASSERT_FATAL(rc == 0); + SPDK_CU_ASSERT_FATAL(g_request != NULL); + SPDK_CU_ASSERT_FATAL(g_request->num_children == 0); + CU_ASSERT(g_request->md_size == 256 * 128); + CU_ASSERT(g_request->payload_size == 256 * (512 + 128)); + + nvme_free_request(g_request); + cleanup_after_test(&qpair); + ctrlr.quirks = 0; + /* * 512 byte data + 8 byte metadata * Extended LBA