Bug 200458 - [nvme] nvme_ctrlr_disable does not wait for the READY bit to become zero
Summary: [nvme] nvme_ctrlr_disable does not wait for the READY bit to become zero
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: 10.1-RELEASE
Hardware: Any Any
: --- Affects Many People
Assignee: Jim Harris
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-26 09:14 UTC by oren.berman
Modified: 2016-08-08 07:30 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description oren.berman 2015-05-26 09:14:18 UTC
The function nvme_ctrlr_disable in the nvme driver code which sets the CC enable bit to 0 does not wait for the READY bit in the CSTS register to become zero.
When triggering a reset It  means that the nvme_ctrlr_eanble is called immediately after the disable operation without making sure that the previous operation was successful. 

My proposed fix is to add a new function called nvme_ctrlr_wait_for_not_ready:
static int nvme_ctrlr_wait_for_not_ready(struct nvme_controller *ctrlr)
{
                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) {
                                nvme_printf(ctrlr, "%s called with cc.en = 1\n", __func__);
                                return (ENXIO);
                }

                ms_waited = 0;

                while (csts.bits.rdy) {
                                DELAY(1000);
                                if (ms_waited++ > ctrlr->ready_timeout_in_ms) {
                                                nvme_printf(ctrlr, "controller did not become ready "
                                                    "within %d ms\n", ctrlr->ready_timeout_in_ms);
                                                return (ENXIO);
                                }
                                csts.raw = nvme_mmio_read_4(ctrlr, csts);
                }

                return (0);
}

And then modify nvme_ctrlr_disable to call this function:

static void
nvme_ctrlr_disable(struct nvme_controller *ctrlr)
{
                union cc_register cc,cc1;
                union csts_register csts;

                cc.raw = nvme_mmio_read_4(ctrlr, cc);
                csts.raw = nvme_mmio_read_4(ctrlr, csts);
                cc1.raw=0;
                if (cc.bits.en == 1 && csts.bits.rdy == 0)
                                nvme_ctrlr_wait_for_ready(ctrlr);


                cc.bits.en = 0;
                nvme_mmio_write_4(ctrlr, cc, cc.raw);
                DELAY(5000);
                nvme_ctrlr_wait_for_not_ready(ctrlr); -->check if READY bit was set to zero.

}

The same check is being done in the nvme linux driver.

BR
Oren
Comment 1 commit-hook freebsd_committer freebsd_triage 2015-07-23 15:51:26 UTC
A commit references this bug:

Author: jimharris
Date: Thu Jul 23 15:50:40 UTC 2015
New revision: 285816
URL: https://svnweb.freebsd.org/changeset/base/285816

Log:
  nvme: ensure csts.rdy bit is cleared before returning from nvme_ctrlr_disable

  PR:		200458
  MFC after:	3 days
  Sponsored by:	Intel

Changes:
  head/sys/dev/nvme/nvme_ctrlr.c
Comment 2 Mark Linimon freebsd_committer freebsd_triage 2016-08-08 07:30:54 UTC
Already committed in 2015.