Bug 221616 - incorrect nvme driver init sequence
Summary: incorrect nvme driver init sequence
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 10.3-STABLE
Hardware: amd64 Any
: --- Affects Some People
Assignee: Warner Losh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-18 21:00 UTC by Kinjal Patel
Modified: 2018-05-28 22:37 UTC (History)
3 users (show)

See Also:


Attachments
patch to correct the NVMe driver init sequence (652 bytes, patch)
2017-08-18 21:00 UTC, Kinjal Patel
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kinjal Patel 2017-08-18 21:00:21 UTC
Created attachment 185557 [details]
patch to correct the NVMe driver init sequence

At present, the NVMe driver init sequence is,
1)      Enable controller (CC.EN=1)
2)      Wait for controller ready (CSTS.RDY=1)
3)      Set PCI bus master enable (BME=1)

As per NVMe spec, when NVMe controller becomes ready it has to process command.

"Enable (EN): When set to '1', then the controller shall process commands based on Submission Queue Tail doorbell writes"

And per PCI Express spec when BME is not set, the PCI Express Function is not allowed to issue any Memory or I/O requests.
"Bus Master Enable - Controls the ability of a PCI Express Endpoint to issue Memory95 and I/O Read/Write Requests, and
the ability of a Root or Switch Port to forward Memory and I/O Read/Write Requests in the Upstream direction"

Enabling controller before setting BME=1 is violation of spec, as controller 
has to accept commands but BME is prerequisite for that.

The FreeBSD NVMe driver sequence should be changed to set BME=1 before attempting to enable controller.

The Linux device driver init sequence also does the same. That is,
1)     Set PCI bus master enable (BME=1)
2)     Enable Controller (CC.EN=1)
3)     Wait for controller ready (CSTS.RDY=1)

Please find attached patch to correct the NVMe driver init sequence.
The patch is based on "FreeBSD 10.3-STABLE amd64" source.

Thanks,
Kinjal Patel
Comment 1 Warner Losh freebsd_committer freebsd_triage 2017-08-19 02:26:26 UTC
I'll push this in when I get back from Eclipse watching unless someone beats me to it.
Comment 2 Warner Losh freebsd_committer freebsd_triage 2017-08-25 04:53:08 UTC
commit 08cdb8187e658581fbcd3f1053ea64b1ecc70ada
Author: imp <imp@ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f>
Date:   Fri Aug 25 03:15:18 2017 +0000

    Enable bus mastering on the device before resetting the device. The
    card has to do PCIe transactions to complete the reset process, but
    can't do them, per the PCIe spec, unless bus mastering is enabled.
    
    Submitted by: Kinjal Patel
    PR: 22166

Note the wrong PR here.
Comment 3 Eitan Adler freebsd_committer freebsd_triage 2018-05-28 19:47:20 UTC
batch change:

For bugs that match the following
-  Status Is In progress 
AND
- Untouched since 2018-01-01.
AND
- Affects Base System OR Documentation

DO:

Reset to open status.


Note:
I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
Comment 4 Warner Losh freebsd_committer freebsd_triage 2018-05-28 22:37:21 UTC
fixed.