Date: Tue, 3 Sep 2019 15:26:11 +0000 (UTC) From: Warner Losh <imp@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r351747 - head/sys/dev/nvme Message-ID: <201909031526.x83FQBbE002770@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: imp Date: Tue Sep 3 15:26:11 2019 New Revision: 351747 URL: https://svnweb.freebsd.org/changeset/base/351747 Log: Implement nvme suspend / resume for pci attachment When we suspend, we need to properly shutdown the NVME controller. The controller may go into D3 state (or may have the power removed), and to properly flush the metadata to non-volatile RAM, we must complete a normal shutdown. This consists of deleting the I/O queues and setting the shutodown bit. We have to do some extra stuff to make sure we reset the software state of the queues as well. On resume, we have to reset the card twice, for reasons described in the attach funcion. Once we've done that, we can restart the card. If any of this fails, we'll fail the NVMe card, just like we do when a reset fails. Set is_resetting for the duration of the suspend / resume. This keeps the reset taskqueue from running a concurrent reset, and also is needed to prevent any hw completions from queueing more I/O to the card. Pass resetting flag to nvme_ctrlr_start. It doesn't need to get that from the global state of the ctrlr. Wait for any pending reset to finish. All queued I/O will get sent to the hardware as part of nvme_ctrlr_start(), though the upper layers shouldn't send any down. Disabling the qpairs is the other failsafe to ensure all I/O is queued. Rename nvme_ctrlr_destory_qpairs to nvme_ctrlr_delete_qpairs to avoid confusion with all the other destroy functions. It just removes the queues in hardware, while the other _destroy_ functions tear down driver data structures. Split parts of the hardware reset function up so that I can do part of the reset in suspsend. Split out the software disabling of the qpairs into nvme_ctrlr_disable_qpairs. Finally, fix a couple of spelling errors in comments related to this. Relnotes: Yes MFC After: 1 week Reviewed by: scottl@ (prior version) Differential Revision: https://reviews.freebsd.org/D21493 Modified: head/sys/dev/nvme/nvme.c head/sys/dev/nvme/nvme_ctrlr.c head/sys/dev/nvme/nvme_pci.c head/sys/dev/nvme/nvme_private.h Modified: head/sys/dev/nvme/nvme.c ============================================================================== --- head/sys/dev/nvme/nvme.c Tue Sep 3 14:55:19 2019 (r351746) +++ head/sys/dev/nvme/nvme.c Tue Sep 3 15:26:11 2019 (r351747) @@ -137,9 +137,10 @@ nvme_attach(device_t dev) } /* - * Reset controller twice to ensure we do a transition from cc.en==1 - * to cc.en==0. This is because we don't really know what status - * the controller was left in when boot handed off to OS. + * Reset controller twice to ensure we do a transition from cc.en==1 to + * cc.en==0. This is because we don't really know what status the + * controller was left in when boot handed off to OS. Linux doesn't do + * this, however. If we adopt that policy, see also nvme_ctrlr_resume(). */ status = nvme_ctrlr_hw_reset(ctrlr); if (status != 0) { Modified: head/sys/dev/nvme/nvme_ctrlr.c ============================================================================== --- head/sys/dev/nvme/nvme_ctrlr.c Tue Sep 3 14:55:19 2019 (r351746) +++ head/sys/dev/nvme/nvme_ctrlr.c Tue Sep 3 15:26:11 2019 (r351747) @@ -118,8 +118,8 @@ nvme_ctrlr_construct_io_qpairs(struct nvme_controller /* * Our best estimate for the maximum number of I/Os that we should - * noramlly have in flight at one time. This should be viewed as a hint, - * not a hard limit and will need to be revisitted when the upper layers + * normally have in flight at one time. This should be viewed as a hint, + * not a hard limit and will need to be revisited when the upper layers * of the storage system grows multi-queue support. */ ctrlr->max_hw_pend_io = num_trackers * ctrlr->num_io_queues * 3 / 4; @@ -344,10 +344,10 @@ nvme_ctrlr_enable(struct nvme_controller *ctrlr) return (nvme_ctrlr_wait_for_ready(ctrlr, 1)); } -int -nvme_ctrlr_hw_reset(struct nvme_controller *ctrlr) +static void +nvme_ctrlr_disable_qpairs(struct nvme_controller *ctrlr) { - int i, err; + int i; nvme_admin_qpair_disable(&ctrlr->adminq); /* @@ -359,7 +359,15 @@ nvme_ctrlr_hw_reset(struct nvme_controller *ctrlr) for (i = 0; i < ctrlr->num_io_queues; i++) nvme_io_qpair_disable(&ctrlr->ioq[i]); } +} +int +nvme_ctrlr_hw_reset(struct nvme_controller *ctrlr) +{ + int err; + + nvme_ctrlr_disable_qpairs(ctrlr); + DELAY(100*1000); err = nvme_ctrlr_disable(ctrlr); @@ -481,7 +489,7 @@ nvme_ctrlr_create_qpairs(struct nvme_controller *ctrlr } static int -nvme_ctrlr_destroy_qpairs(struct nvme_controller *ctrlr) +nvme_ctrlr_delete_qpairs(struct nvme_controller *ctrlr) { struct nvme_completion_poll_status status; struct nvme_qpair *qpair; @@ -820,7 +828,7 @@ nvme_ctrlr_configure_int_coalescing(struct nvme_contro } static void -nvme_ctrlr_start(void *ctrlr_arg) +nvme_ctrlr_start(void *ctrlr_arg, bool resetting) { struct nvme_controller *ctrlr = ctrlr_arg; uint32_t old_num_io_queues; @@ -833,7 +841,7 @@ nvme_ctrlr_start(void *ctrlr_arg) * the number of I/O queues supported, so cannot reset * the adminq again here. */ - if (ctrlr->is_resetting) + if (resetting) nvme_qpair_reset(&ctrlr->adminq); for (i = 0; i < ctrlr->num_io_queues; i++) @@ -854,7 +862,7 @@ nvme_ctrlr_start(void *ctrlr_arg) * explicit specify how many queues it will use. This value should * never change between resets, so panic if somehow that does happen. */ - if (ctrlr->is_resetting) { + if (resetting) { old_num_io_queues = ctrlr->num_io_queues; if (nvme_ctrlr_set_num_qpairs(ctrlr) != 0) { nvme_ctrlr_fail(ctrlr); @@ -894,7 +902,7 @@ nvme_ctrlr_start_config_hook(void *arg) if (nvme_ctrlr_set_num_qpairs(ctrlr) == 0 && nvme_ctrlr_construct_io_qpairs(ctrlr) == 0) - nvme_ctrlr_start(ctrlr); + nvme_ctrlr_start(ctrlr, false); else nvme_ctrlr_fail(ctrlr); @@ -923,7 +931,7 @@ nvme_ctrlr_reset_task(void *arg, int pending) */ pause("nvmereset", hz / 10); if (status == 0) - nvme_ctrlr_start(ctrlr); + nvme_ctrlr_start(ctrlr, true); else nvme_ctrlr_fail(ctrlr); @@ -946,7 +954,7 @@ nvme_ctrlr_poll(struct nvme_controller *ctrlr) } /* - * Poll the single-vector intertrupt case: num_io_queues will be 1 and + * Poll the single-vector interrupt case: num_io_queues will be 1 and * there's only a single vector. While we're polling, we mask further * interrupts in the controller. */ @@ -1012,7 +1020,7 @@ nvme_ctrlr_passthrough_cmd(struct nvme_controller *ctr if (is_user_buffer) { /* * Ensure the user buffer is wired for the duration of - * this passthrough command. + * this pass-through command. */ PHOLD(curproc); buf = uma_zalloc(pbuf_zone, M_WAITOK); @@ -1031,7 +1039,7 @@ nvme_ctrlr_passthrough_cmd(struct nvme_controller *ctr } else req = nvme_allocate_request_null(nvme_pt_done, pt); - /* Assume userspace already converted to little-endian */ + /* Assume user space already converted to little-endian */ req->cmd.opc = pt->cmd.opc; req->cmd.fuse = pt->cmd.fuse; req->cmd.rsvd2 = pt->cmd.rsvd2; @@ -1206,7 +1214,7 @@ nvme_ctrlr_destruct(struct nvme_controller *ctrlr, dev if (ctrlr->is_initialized) { if (!gone) - nvme_ctrlr_destroy_qpairs(ctrlr); + nvme_ctrlr_delete_qpairs(ctrlr); for (i = 0; i < ctrlr->num_io_queues; i++) nvme_io_qpair_destroy(&ctrlr->ioq[i]); free(ctrlr->ioq, M_NVME); @@ -1305,4 +1313,88 @@ nvme_ctrlr_get_data(struct nvme_controller *ctrlr) { return (&ctrlr->cdata); +} + +int +nvme_ctrlr_suspend(struct nvme_controller *ctrlr) +{ + int to = hz; + + /* + * Can't touch failed controllers, so it's already suspended. + */ + if (ctrlr->is_failed) + return (0); + + /* + * We don't want the reset taskqueue running, since it does similar + * things, so prevent it from running after we start. Wait for any reset + * that may have been started to complete. The reset process we follow + * will ensure that any new I/O will queue and be given to the hardware + * after we resume (though there should be none). + */ + while (atomic_cmpset_32(&ctrlr->is_resetting, 0, 1) == 0 && to-- > 0) + pause("nvmesusp", 1); + if (to <= 0) { + nvme_printf(ctrlr, + "Competing reset task didn't finish. Try again later.\n"); + return (EWOULDBLOCK); + } + + /* + * Per Section 7.6.2 of NVMe spec 1.4, to properly suspend, we need to + * delete the hardware I/O queues, and then shutdown. This properly + * flushes any metadata the drive may have stored so it can survive + * having its power removed and prevents the unsafe shutdown count from + * incriminating. Once we delete the qpairs, we have to disable them + * before shutting down. The delay is out of paranoia in + * nvme_ctrlr_hw_reset, and is repeated here (though we should have no + * pending I/O that the delay copes with). + */ + nvme_ctrlr_delete_qpairs(ctrlr); + nvme_ctrlr_disable_qpairs(ctrlr); + DELAY(100*1000); + nvme_ctrlr_shutdown(ctrlr); + + return (0); +} + +int +nvme_ctrlr_resume(struct nvme_controller *ctrlr) +{ + + /* + * Can't touch failed controllers, so nothing to do to resume. + */ + if (ctrlr->is_failed) + return (0); + + /* + * Have to reset the hardware twice, just like we do on attach. See + * nmve_attach() for why. + */ + if (nvme_ctrlr_hw_reset(ctrlr) != 0) + goto fail; + if (nvme_ctrlr_hw_reset(ctrlr) != 0) + goto fail; + + /* + * Now that we're reset the hardware, we can restart the controller. Any + * I/O that was pending is requeued. Any admin commands are aborted with + * an error. Once we've restarted, take the controller out of reset. + */ + nvme_ctrlr_start(ctrlr, true); + atomic_cmpset_32(&ctrlr->is_resetting, 1, 0); + + return (0); +fail: + /* + * Since we can't bring the controller out of reset, announce and fail + * the controller. However, we have to return success for the resume + * itself, due to questionable APIs. + */ + nvme_printf(ctrlr, "Failed to reset on resume, failing.\n"); + nvme_ctrlr_fail(ctrlr); + atomic_cmpset_32(&ctrlr->is_resetting, 1, 0); + return (0); } Modified: head/sys/dev/nvme/nvme_pci.c ============================================================================== --- head/sys/dev/nvme/nvme_pci.c Tue Sep 3 14:55:19 2019 (r351746) +++ head/sys/dev/nvme/nvme_pci.c Tue Sep 3 15:26:11 2019 (r351747) @@ -43,6 +43,8 @@ __FBSDID("$FreeBSD$"); static int nvme_pci_probe(device_t); static int nvme_pci_attach(device_t); static int nvme_pci_detach(device_t); +static int nvme_pci_suspend(device_t); +static int nvme_pci_resume(device_t); static void nvme_ctrlr_setup_interrupts(struct nvme_controller *ctrlr); @@ -51,6 +53,8 @@ static device_method_t nvme_pci_methods[] = { DEVMETHOD(device_probe, nvme_pci_probe), DEVMETHOD(device_attach, nvme_pci_attach), DEVMETHOD(device_detach, nvme_pci_detach), + DEVMETHOD(device_suspend, nvme_pci_suspend), + DEVMETHOD(device_resume, nvme_pci_resume), DEVMETHOD(device_shutdown, nvme_shutdown), { 0, 0 } }; @@ -331,4 +335,22 @@ nvme_ctrlr_setup_interrupts(struct nvme_controller *ct } ctrlr->msix_enabled = 1; +} + +static int +nvme_pci_suspend(device_t dev) +{ + struct nvme_controller *ctrlr; + + ctrlr = DEVICE2SOFTC(dev); + return (nvme_ctrlr_suspend(ctrlr)); +} + +static int +nvme_pci_resume(device_t dev) +{ + struct nvme_controller *ctrlr; + + ctrlr = DEVICE2SOFTC(dev); + return (nvme_ctrlr_resume(ctrlr)); } Modified: head/sys/dev/nvme/nvme_private.h ============================================================================== --- head/sys/dev/nvme/nvme_private.h Tue Sep 3 14:55:19 2019 (r351746) +++ head/sys/dev/nvme/nvme_private.h Tue Sep 3 15:26:11 2019 (r351747) @@ -556,4 +556,7 @@ void nvme_notify_ns(struct nvme_controller *ctrlr, int void nvme_ctrlr_intx_handler(void *arg); void nvme_ctrlr_poll(struct nvme_controller *ctrlr); +int nvme_ctrlr_suspend(struct nvme_controller *ctrlr); +int nvme_ctrlr_resume(struct nvme_controller *ctrlr); + #endif /* __NVME_PRIVATE_H__ */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201909031526.x83FQBbE002770>