Page MenuHomeFreeBSD

Support doorbell strides != 0.
ClosedPublic

Authored by imp on Tue, Sep 3, 9:39 PM.

Details

Summary

The NVMe standard (1.4) states

8.6 Doorbell Stride for Software Emulation
The doorbell stride,...is useful in software emulation of an NVM
Express controller. ... For hardware implementations of the NVM
Express interface, the expected doorbell stride value is 0h.

However, hardware in the wild exists with a doorbell stride of
1h. This change supports that hardware, as well as software emulators
as envisioned in Section 8.6. Since this is the fast path, care has
been taken to make this computation efficient. The bit of math to
compute an offset for each is replaced by a memory load from cache.

Test Plan

Tested on new hardware that has DSTRD=1. works great. Also works on half a dozen older drives that have DSTRD=0.
Don't have an emulator that can set this easily,

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

imp created this revision.Tue, Sep 3, 9:39 PM
imp edited the test plan for this revision. (Show Details)Tue, Sep 3, 9:42 PM
imp added reviewers: scottl, jimharris.
imp added a reviewer: mav.Tue, Sep 3, 9:46 PM
imp updated this revision to Diff 61626.Tue, Sep 3, 10:20 PM

mixed up min / max

imp added inline comments.Tue, Sep 3, 10:21 PM
sys/dev/nvme/nvme_ctrlr.c
112 ↗(On Diff #61625)

This should be min. We want the smaller of these two.

imp updated this revision to Diff 61627.Tue, Sep 3, 10:22 PM

Fix some comment wrapping.

scottl accepted this revision.Wed, Sep 4, 2:18 PM
scottl added inline comments.
sys/dev/nvme/nvme_qpair.c
626 ↗(On Diff #61627)

I would have probably stuck with using nvme_mmio_write_4() because it's more compact and easier to read, but it's a super minor thing.

This revision is now accepted and ready to land.Wed, Sep 4, 2:18 PM
imp added inline comments.Wed, Sep 4, 4:01 PM
sys/dev/nvme/nvme_qpair.c
626 ↗(On Diff #61627)

I'd love to, but it hard-codes a offsetof() its second argument, so I couldn't use it.

This revision was automatically updated to reflect the committed changes.
imp marked an inline comment as done.