From owner-svn-src-head@freebsd.org Mon Dec 18 18:38:02 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 11ACCEA2DDD; Mon, 18 Dec 2017 18:38:02 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C564268EE5; Mon, 18 Dec 2017 18:38:01 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id vBIIc0h4043197; Mon, 18 Dec 2017 18:38:00 GMT (envelope-from imp@FreeBSD.org) Received: (from imp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id vBIIc0Rq043194; Mon, 18 Dec 2017 18:38:00 GMT (envelope-from imp@FreeBSD.org) Message-Id: <201712181838.vBIIc0Rq043194@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: imp set sender to imp@FreeBSD.org using -f From: Warner Losh Date: Mon, 18 Dec 2017 18:38:00 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r326937 - head/sys/dev/nvme X-SVN-Group: head X-SVN-Commit-Author: imp X-SVN-Commit-Paths: head/sys/dev/nvme X-SVN-Commit-Revision: 326937 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Dec 2017 18:38:02 -0000 Author: imp Date: Mon Dec 18 18:38:00 2017 New Revision: 326937 URL: https://svnweb.freebsd.org/changeset/base/326937 Log: When we're disabling the nvme device, some drives have a controller bug that requires 'hands off' for a period of time (2.3s) before we check the RDY bit. Sicne this is a very odd quirk for a very limited selection of drives, do this as a quirk. This prevented a successful reset of the card when the card wedged. Also, make sure that we comply with the advice from section 3.1.5 of the 1.3 spec says that transitioning CC.EN from 0 to 1 when CSTS.RDY is 1 or transitioning CC.EN from 1 to 0 when CSTS.RDY is 0 "has undefined results". Short circuit when EN == RDY == desired state. Finally, fail the reset if the disable fails. This will lead to a failed device, which is what we want. (note: nda device needs work for coping with a failed device). Sponsored by: Netflix Differential Revision: https://reviews.freebsd.org/D13389 Modified: head/sys/dev/nvme/nvme.c head/sys/dev/nvme/nvme_ctrlr.c head/sys/dev/nvme/nvme_private.h Modified: head/sys/dev/nvme/nvme.c ============================================================================== --- head/sys/dev/nvme/nvme.c Mon Dec 18 17:58:09 2017 (r326936) +++ head/sys/dev/nvme/nvme.c Mon Dec 18 18:38:00 2017 (r326937) @@ -90,6 +90,7 @@ static struct _pcsid int match_subdevice; uint16_t subdevice; const char *desc; + uint32_t quirks; } pci_ids[] = { { 0x01118086, 0, 0, "NVMe Controller" }, { IDT32_PCI_ID, 0, 0, "IDT NVMe Controller (32 channel)" }, @@ -100,6 +101,11 @@ static struct _pcsid { 0x09538086, 1, 0x3705, "DC P3500 SSD [2.5\" SFF]" }, { 0x09538086, 1, 0x3709, "DC P3600 SSD [Add-in Card]" }, { 0x09538086, 1, 0x370a, "DC P3600 SSD [2.5\" SFF]" }, + { 0x00031c58, 0, 0, "HGST SN100", QUIRK_DELAY_B4_CHK_RDY }, + { 0x00231c58, 0, 0, "WDC SN200", QUIRK_DELAY_B4_CHK_RDY }, + { 0x05401c5f, 0, 0, "Memblaze Pblaze4", QUIRK_DELAY_B4_CHK_RDY }, + { 0xa821144d, 0, 0, "Samsung PM1725", QUIRK_DELAY_B4_CHK_RDY }, + { 0xa822144d, 0, 0, "Samsung PM1725a", QUIRK_DELAY_B4_CHK_RDY }, { 0x00000000, 0, 0, NULL } }; @@ -240,6 +246,19 @@ nvme_attach(device_t dev) { struct nvme_controller *ctrlr = DEVICE2SOFTC(dev); int status; + struct _pcsid *ep; + uint32_t devid; + uint16_t subdevice; + + devid = pci_get_devid(dev); + subdevice = pci_get_subdevice(dev); + ep = pci_ids; + while (ep->devid) { + if (nvme_match(devid, subdevice, ep)) + break; + ++ep; + } + ctrlr->quirks = ep->quirks; status = nvme_ctrlr_construct(ctrlr, dev); Modified: head/sys/dev/nvme/nvme_ctrlr.c ============================================================================== --- head/sys/dev/nvme/nvme_ctrlr.c Mon Dec 18 17:58:09 2017 (r326936) +++ head/sys/dev/nvme/nvme_ctrlr.c Mon Dec 18 18:38:00 2017 (r326937) @@ -46,6 +46,8 @@ __FBSDID("$FreeBSD$"); #include "nvme_private.h" +#define B4_CHK_RDY_DELAY_MS 2300 /* work arond controller bug */ + static void nvme_ctrlr_construct_and_submit_aer(struct nvme_controller *ctrlr, struct nvme_async_event_request *aer); static void nvme_ctrlr_setup_interrupts(struct nvme_controller *ctrlr); @@ -241,49 +243,65 @@ static int nvme_ctrlr_wait_for_ready(struct nvme_controller *ctrlr, int desired_val) { int ms_waited; - union cc_register cc; union csts_register csts; - cc.raw = nvme_mmio_read_4(ctrlr, cc); csts.raw = nvme_mmio_read_4(ctrlr, csts); - if (cc.bits.en != desired_val) { - nvme_printf(ctrlr, "%s called with desired_val = %d " - "but cc.en = %d\n", __func__, desired_val, cc.bits.en); - return (ENXIO); - } - ms_waited = 0; - while (csts.bits.rdy != desired_val) { - DELAY(1000); if (ms_waited++ > ctrlr->ready_timeout_in_ms) { nvme_printf(ctrlr, "controller ready did not become %d " "within %d ms\n", desired_val, ctrlr->ready_timeout_in_ms); return (ENXIO); } + DELAY(1000); csts.raw = nvme_mmio_read_4(ctrlr, csts); } return (0); } -static void +static int nvme_ctrlr_disable(struct nvme_controller *ctrlr) { union cc_register cc; union csts_register csts; + int err; cc.raw = nvme_mmio_read_4(ctrlr, cc); csts.raw = nvme_mmio_read_4(ctrlr, csts); - if (cc.bits.en == 1 && csts.bits.rdy == 0) - nvme_ctrlr_wait_for_ready(ctrlr, 1); + /* + * Per 3.1.5 in NVME 1.3 spec, transitioning CC.EN from 0 to 1 + * when CSTS.RDY is 1 or transitioning CC.EN from 1 to 0 when + * CSTS.RDY is 0 "has undefined results" So make sure that CSTS.RDY + * isn't the desired value. Short circuit if we're already disabled. + */ + if (cc.bits.en == 1) { + if (csts.bits.rdy == 0) { + /* EN == 1, wait for RDY == 1 or fail */ + err = nvme_ctrlr_wait_for_ready(ctrlr, 1); + if (err != 0) + return (err); + } + } else { + /* EN == 0 already wait for RDY == 0 */ + if (csts.bits.rdy == 0) + return (0); + else + return (nvme_ctrlr_wait_for_ready(ctrlr, 0)); + } cc.bits.en = 0; nvme_mmio_write_4(ctrlr, cc, cc.raw); - DELAY(5000); - nvme_ctrlr_wait_for_ready(ctrlr, 0); + /* + * Some drives have issues with accessing the mmio after we + * disable, so delay for a bit after we write the bit to + * cope with these issues. + */ + if (ctrlr->quirks) + pause("nvmeR", B4_CHK_RDY_DELAY_MS * hz / 1000); + return (nvme_ctrlr_wait_for_ready(ctrlr, 0)); } static int @@ -292,15 +310,24 @@ nvme_ctrlr_enable(struct nvme_controller *ctrlr) union cc_register cc; union csts_register csts; union aqa_register aqa; + int err; cc.raw = nvme_mmio_read_4(ctrlr, cc); csts.raw = nvme_mmio_read_4(ctrlr, csts); + /* + * See note in nvme_ctrlr_disable. Short circuit if we're already enabled. + */ if (cc.bits.en == 1) { if (csts.bits.rdy == 1) return (0); else return (nvme_ctrlr_wait_for_ready(ctrlr, 1)); + } else { + /* EN == 0 already wait for RDY == 0 or fail */ + err = nvme_ctrlr_wait_for_ready(ctrlr, 0); + if (err != 0) + return (err); } nvme_mmio_write_8(ctrlr, asq, ctrlr->adminq.cmd_bus_addr); @@ -326,7 +353,6 @@ nvme_ctrlr_enable(struct nvme_controller *ctrlr) cc.bits.mps = (PAGE_SIZE >> 13); nvme_mmio_write_4(ctrlr, cc, cc.raw); - DELAY(5000); return (nvme_ctrlr_wait_for_ready(ctrlr, 1)); } @@ -334,7 +360,7 @@ nvme_ctrlr_enable(struct nvme_controller *ctrlr) int nvme_ctrlr_hw_reset(struct nvme_controller *ctrlr) { - int i; + int i, err; nvme_admin_qpair_disable(&ctrlr->adminq); /* @@ -349,7 +375,9 @@ nvme_ctrlr_hw_reset(struct nvme_controller *ctrlr) DELAY(100*1000); - nvme_ctrlr_disable(ctrlr); + err = nvme_ctrlr_disable(ctrlr); + if (err != 0) + return err; return (nvme_ctrlr_enable(ctrlr)); } Modified: head/sys/dev/nvme/nvme_private.h ============================================================================== --- head/sys/dev/nvme/nvme_private.h Mon Dec 18 17:58:09 2017 (r326936) +++ head/sys/dev/nvme/nvme_private.h Mon Dec 18 18:38:00 2017 (r326937) @@ -246,6 +246,8 @@ struct nvme_controller { struct mtx lock; uint32_t ready_timeout_in_ms; + uint32_t quirks; +#define QUIRK_DELAY_B4_CHK_RDY 1 /* Can't touch MMIO on disable */ bus_space_tag_t bus_tag; bus_space_handle_t bus_handle;