From 8a9c1d40118d15b642ba4e8bd84205a1f7879f67 Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Fri, 2 Dec 2016 09:31:06 -0700 Subject: [PATCH] nvme: Make striping a quirk Use the standard quirk mechanism to specify which devices need software assisted striping. Change-Id: Id8156876a90b4caf9d687637e14c7ad4a66ceda6 Signed-off-by: Ben Walker --- lib/nvme/nvme_ctrlr.c | 13 +-- lib/nvme/nvme_internal.h | 8 +- lib/nvme/nvme_ns.c | 11 +- lib/nvme/nvme_pcie.c | 2 + lib/nvme/nvme_quirks.c | 17 +-- test/lib/nvme/unit/Makefile | 3 +- .../nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c | 31 ++---- test/lib/nvme/unit/nvme_quirks_c/.gitignore | 1 + test/lib/nvme/unit/nvme_quirks_c/Makefile | 38 +++++++ .../nvme/unit/nvme_quirks_c/nvme_quirks_ut.c | 100 ++++++++++++++++++ 10 files changed, 171 insertions(+), 53 deletions(-) create mode 100644 test/lib/nvme/unit/nvme_quirks_c/.gitignore create mode 100644 test/lib/nvme/unit/nvme_quirks_c/Makefile create mode 100644 test/lib/nvme/unit/nvme_quirks_c/nvme_quirks_ut.c diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index f19bc0c0c..cfe0594cb 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -225,17 +225,11 @@ static void nvme_ctrlr_construct_intel_support_log_page_list(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_intel_log_page_directory *log_page_directory) { - struct spdk_pci_id pci_id; - if (log_page_directory == NULL) { return; } - if (nvme_transport_ctrlr_get_pci_id(ctrlr, &pci_id)) { - return; - } - - if (pci_id.vendor_id != SPDK_PCI_VID_INTEL) { + if (ctrlr->cdata.vid != SPDK_PCI_VID_INTEL) { return; } @@ -1286,7 +1280,6 @@ nvme_robust_mutex_init_recursive_shared(pthread_mutex_t *mtx) int nvme_ctrlr_construct(struct spdk_nvme_ctrlr *ctrlr) { - struct spdk_pci_id pci_id; int rc; nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_INIT, NVME_TIMEOUT_INFINITE); @@ -1305,10 +1298,6 @@ nvme_ctrlr_construct(struct spdk_nvme_ctrlr *ctrlr) return rc; } - if (nvme_transport_ctrlr_get_pci_id(ctrlr, &pci_id) == 0) { - ctrlr->quirks = nvme_get_quirks(&pci_id); - } - TAILQ_INIT(&ctrlr->active_procs); return rc; diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index a964a9278..47d55b914 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -78,6 +78,12 @@ */ #define NVME_QUIRK_DELAY_BEFORE_CHK_RDY 0x4 +/* + * The controller performs best when I/O is split on particular + * LBA boundaries. + */ +#define NVME_INTEL_QUIRK_STRIPING 0x8 + #define NVME_MAX_ASYNC_EVENTS (8) #define NVME_MIN_TIMEOUT_PERIOD (5) @@ -419,8 +425,6 @@ extern struct nvme_driver *g_spdk_nvme_driver; #define nvme_min(a,b) (((a)<(b))?(a):(b)) -#define INTEL_DC_P3X00_DEVID 0x0953 - #define nvme_delay usleep static inline uint32_t diff --git a/lib/nvme/nvme_ns.c b/lib/nvme/nvme_ns.c index b809f6031..82b71a99f 100644 --- a/lib/nvme/nvme_ns.c +++ b/lib/nvme/nvme_ns.c @@ -183,20 +183,15 @@ spdk_nvme_ns_get_data(struct spdk_nvme_ns *ns) int nvme_ns_construct(struct spdk_nvme_ns *ns, uint16_t id, struct spdk_nvme_ctrlr *ctrlr) { - struct spdk_pci_id pci_id; - assert(id > 0); ns->ctrlr = ctrlr; ns->id = id; ns->stripe_size = 0; - if (nvme_transport_ctrlr_get_pci_id(ctrlr, &pci_id) == 0) { - if (pci_id.vendor_id == SPDK_PCI_VID_INTEL && - pci_id.device_id == INTEL_DC_P3X00_DEVID && - ctrlr->cdata.vs[3] != 0) { - ns->stripe_size = (1 << ctrlr->cdata.vs[3]) * ctrlr->min_page_size; - } + if (ctrlr->quirks & NVME_INTEL_QUIRK_STRIPING && + ctrlr->cdata.vs[3] != 0) { + ns->stripe_size = (1 << ctrlr->cdata.vs[3]) * ctrlr->min_page_size; } return nvme_ns_identify_update(ns); diff --git a/lib/nvme/nvme_pcie.c b/lib/nvme/nvme_pcie.c index 433717cda..b1d5a8b01 100644 --- a/lib/nvme/nvme_pcie.c +++ b/lib/nvme/nvme_pcie.c @@ -632,6 +632,8 @@ struct spdk_nvme_ctrlr *nvme_pcie_ctrlr_construct(enum spdk_nvme_transport_type return NULL; } + pctrlr->ctrlr.quirks = nvme_get_quirks(&probe_info->pci_id); + rc = nvme_pcie_ctrlr_construct_admin_qpair(&pctrlr->ctrlr); if (rc != 0) { nvme_ctrlr_destruct(&pctrlr->ctrlr); diff --git a/lib/nvme/nvme_quirks.c b/lib/nvme/nvme_quirks.c index 9f8954976..5ad6d7232 100644 --- a/lib/nvme/nvme_quirks.c +++ b/lib/nvme/nvme_quirks.c @@ -39,14 +39,15 @@ struct nvme_quirk { }; static const struct nvme_quirk nvme_quirks[] = { - {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3702}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY }, - {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3703}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY }, - {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3704}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY }, - {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3705}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY }, - {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3709}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY }, - {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x370a}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY }, - {{SPDK_PCI_VID_MEMBLAZE, 0x0540, SPDK_PCI_ANY_ID, SPDK_PCI_ANY_ID}, NVME_QUIRK_DELAY_BEFORE_CHK_RDY }, - {{0x0000, 0x0000, 0x0000, 0x0000}, 0 } + {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3702}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY | NVME_INTEL_QUIRK_STRIPING }, + {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3703}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY | NVME_INTEL_QUIRK_STRIPING }, + {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3704}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY | NVME_INTEL_QUIRK_STRIPING }, + {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3705}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY | NVME_INTEL_QUIRK_STRIPING }, + {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x3709}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY | NVME_INTEL_QUIRK_STRIPING }, + {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_VID_INTEL, 0x370a}, NVME_INTEL_QUIRK_READ_LATENCY | NVME_INTEL_QUIRK_WRITE_LATENCY | NVME_INTEL_QUIRK_STRIPING }, + {{SPDK_PCI_VID_INTEL, 0x0953, SPDK_PCI_ANY_ID, SPDK_PCI_ANY_ID}, NVME_INTEL_QUIRK_STRIPING }, + {{SPDK_PCI_VID_MEMBLAZE, 0x0540, SPDK_PCI_ANY_ID, SPDK_PCI_ANY_ID}, NVME_QUIRK_DELAY_BEFORE_CHK_RDY }, + {{0x0000, 0x0000, 0x0000, 0x0000}, 0 } }; /* Compare each field. SPDK_PCI_ANY_ID in s1 matches everything */ diff --git a/test/lib/nvme/unit/Makefile b/test/lib/nvme/unit/Makefile index b8ee86164..c3ca28957 100644 --- a/test/lib/nvme/unit/Makefile +++ b/test/lib/nvme/unit/Makefile @@ -34,7 +34,8 @@ SPDK_ROOT_DIR := $(abspath $(CURDIR)/../../../..) include $(SPDK_ROOT_DIR)/mk/spdk.common.mk -DIRS-y = nvme_c nvme_ns_cmd_c nvme_qpair_c nvme_ctrlr_c nvme_ctrlr_cmd_c +DIRS-y = nvme_c nvme_ns_cmd_c nvme_qpair_c nvme_ctrlr_c \ + nvme_ctrlr_cmd_c nvme_quirks_c .PHONY: all clean $(DIRS-y) diff --git a/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c b/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c index 47b360bfb..4a3f0d1e0 100644 --- a/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c +++ b/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c @@ -51,11 +51,6 @@ struct nvme_driver _g_nvme_driver = { .request_mempool = NULL, }; -static uint16_t g_pci_vendor_id; -static uint16_t g_pci_device_id; -static uint16_t g_pci_subvendor_id; -static uint16_t g_pci_subdevice_id; - uint64_t g_ut_tsc = 0; struct spdk_nvme_registers g_ut_nvme_regs = {}; @@ -89,11 +84,6 @@ nvme_transport_ctrlr_get_pci_id(struct spdk_nvme_ctrlr *ctrlr, struct spdk_pci_i return -EINVAL; } - pci_id->vendor_id = g_pci_vendor_id; - pci_id->device_id = g_pci_device_id; - pci_id->subvendor_id = g_pci_subvendor_id; - pci_id->subdevice_id = g_pci_subdevice_id; - return 0; } @@ -1181,21 +1171,19 @@ test_nvme_ctrlr_construct_intel_support_log_page_list(void) bool res; struct spdk_nvme_ctrlr ctrlr = {}; struct spdk_nvme_intel_log_page_directory payload = {}; - struct spdk_pci_id pci_id; + struct spdk_pci_id pci_id = {}; - /* set a invalid vendor id */ - g_pci_vendor_id = 0xFFFF; - nvme_transport_ctrlr_get_pci_id(&ctrlr, &pci_id); + /* Get quirks for a device with all 0 vendor/device id */ ctrlr.quirks = nvme_get_quirks(&pci_id); + CU_ASSERT(ctrlr.quirks == 0); nvme_ctrlr_construct_intel_support_log_page_list(&ctrlr, &payload); res = spdk_nvme_ctrlr_is_log_page_supported(&ctrlr, SPDK_NVME_INTEL_LOG_TEMPERATURE); CU_ASSERT(res == false); - /* set valid vendor id and log page directory*/ - g_pci_vendor_id = SPDK_PCI_VID_INTEL; + /* Set the vendor to Intel, but provide no device id */ + ctrlr.cdata.vid = pci_id.vendor_id = SPDK_PCI_VID_INTEL; payload.temperature_statistics_log_len = 1; - nvme_transport_ctrlr_get_pci_id(&ctrlr, &pci_id); ctrlr.quirks = nvme_get_quirks(&pci_id); memset(ctrlr.log_page_supported, 0, sizeof(ctrlr.log_page_supported)); @@ -1212,11 +1200,10 @@ test_nvme_ctrlr_construct_intel_support_log_page_list(void) /* set valid vendor id, device id and sub device id*/ ctrlr.cdata.vid = SPDK_PCI_VID_INTEL; payload.temperature_statistics_log_len = 0; - g_pci_vendor_id = SPDK_PCI_VID_INTEL; - g_pci_device_id = 0x0953; - g_pci_subvendor_id = SPDK_PCI_VID_INTEL; - g_pci_subdevice_id = 0x3702; - nvme_transport_ctrlr_get_pci_id(&ctrlr, &pci_id); + pci_id.vendor_id = SPDK_PCI_VID_INTEL; + pci_id.device_id = 0x0953; + pci_id.subvendor_id = SPDK_PCI_VID_INTEL; + pci_id.subdevice_id = 0x3702; ctrlr.quirks = nvme_get_quirks(&pci_id); memset(ctrlr.log_page_supported, 0, sizeof(ctrlr.log_page_supported)); diff --git a/test/lib/nvme/unit/nvme_quirks_c/.gitignore b/test/lib/nvme/unit/nvme_quirks_c/.gitignore new file mode 100644 index 000000000..eca86651b --- /dev/null +++ b/test/lib/nvme/unit/nvme_quirks_c/.gitignore @@ -0,0 +1 @@ +nvme_quirks_ut diff --git a/test/lib/nvme/unit/nvme_quirks_c/Makefile b/test/lib/nvme/unit/nvme_quirks_c/Makefile new file mode 100644 index 000000000..396f8a853 --- /dev/null +++ b/test/lib/nvme/unit/nvme_quirks_c/Makefile @@ -0,0 +1,38 @@ +# +# BSD LICENSE +# +# Copyright (c) Intel Corporation. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in +# the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Intel Corporation nor the names of its +# contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# + +SPDK_ROOT_DIR := $(abspath $(CURDIR)/../../../../..) + +TEST_FILE = nvme_quirks_ut.c + +include $(SPDK_ROOT_DIR)/mk/nvme.unittest.mk diff --git a/test/lib/nvme/unit/nvme_quirks_c/nvme_quirks_ut.c b/test/lib/nvme/unit/nvme_quirks_c/nvme_quirks_ut.c new file mode 100644 index 000000000..ede75f472 --- /dev/null +++ b/test/lib/nvme/unit/nvme_quirks_c/nvme_quirks_ut.c @@ -0,0 +1,100 @@ +/*- + * BSD LICENSE + * + * Copyright (c) Intel Corporation. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "spdk_cunit.h" + +#include "nvme/nvme_quirks.c" + +static void +test_nvme_quirks_striping(void) +{ + struct spdk_pci_id pci_id = {}; + uint64_t quirks = 0; + + /* Non-Intel device should not have striping enabled */ + quirks = nvme_get_quirks(&pci_id); + CU_ASSERT((quirks & NVME_INTEL_QUIRK_STRIPING) == 0); + + /* Set the vendor id to Intel, but no device id. No striping. */ + pci_id.vendor_id = SPDK_PCI_VID_INTEL; + quirks = nvme_get_quirks(&pci_id); + CU_ASSERT((quirks & NVME_INTEL_QUIRK_STRIPING) == 0); + + /* Device ID 0x0953 should have striping enabled */ + pci_id.device_id = 0x0953; + quirks = nvme_get_quirks(&pci_id); + CU_ASSERT((quirks & NVME_INTEL_QUIRK_STRIPING) != 0); + + /* Even if specific subvendor/subdevice ids are set, + * striping should be enabled. + */ + pci_id.subvendor_id = SPDK_PCI_VID_INTEL; + pci_id.subdevice_id = 0x3704; + quirks = nvme_get_quirks(&pci_id); + CU_ASSERT((quirks & NVME_INTEL_QUIRK_STRIPING) != 0); + + pci_id.subvendor_id = 1234; + pci_id.subdevice_id = 42; + quirks = nvme_get_quirks(&pci_id); + CU_ASSERT((quirks & NVME_INTEL_QUIRK_STRIPING) != 0); +} + +int main(int argc, char **argv) +{ + CU_pSuite suite = NULL; + unsigned int num_failures; + + if (CU_initialize_registry() != CUE_SUCCESS) { + return CU_get_error(); + } + + suite = CU_add_suite("nvme_quirks", NULL, NULL); + if (suite == NULL) { + CU_cleanup_registry(); + return CU_get_error(); + } + + if ( + CU_add_test(suite, "test nvme_quirks striping", + test_nvme_quirks_striping) == NULL + ) { + CU_cleanup_registry(); + return CU_get_error(); + } + + CU_basic_set_mode(CU_BRM_VERBOSE); + CU_basic_run_tests(); + num_failures = CU_get_number_of_failures(); + CU_cleanup_registry(); + return num_failures; +}