Page MenuHomeFreeBSD

Add CAM passthrough for NVMe
ClosedPublic

Authored by chuck_tuffli.net on Apr 3 2017, 12:38 PM.

Details

Summary

This patch adds CAM pass(4) support for NVMe IO's. Applications indicate the IO type (Admin or NVM) using bit 4 of the xflags variable in struct ccb_nvmeio.ccb_h XPT op-codes XPT_NVME_ADMIN or XPT_NVME_IO.

Test Plan

Tested pass-through of both NVM and Admin commands via cam_send_ccb and NVM Read/Write commands via camdd(8)

Diff Detail

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

Event Timeline

sys/cam/scsi/scsi_pass.c
2185 ↗(On Diff #26922)

why XXX?

sys/dev/nvme/nvme_sim.c
113 ↗(On Diff #26922)

why 0x10?

jimharris added inline comments.
sys/cam/cam_periph.c
822 ↗(On Diff #26922)

Maybe create different XPT_NVME_* enum values for IO and ADMIN?

824 ↗(On Diff #26922)

Style - need a space after 'return'.

sys/cam/scsi/scsi_pass.c
2185 ↗(On Diff #26922)

Being the first person to need xflags copied to the new ccb raised the question in my mind as to whether this was the correct approach

sys/dev/nvme/nvme_sim.c
113 ↗(On Diff #26922)

Bit 4 (i.e. 0x10) appeared to be the first unused bit in xflags and is used to differentiate between Admin commands (bit 4 set) and NVM commands (bit 4 clear). As I wasn't sure about the approach, I didn't create a #define for it at this time.

sys/cam/scsi/scsi_pass.c
2185 ↗(On Diff #26922)

We should get other people's notions as well (Scott Long maybe). How would this be used in the kernel to allow access to the admin queue from CAM devices. Most of the stuff we have today won't need an in-kernel consumer, but it's instructive to think of it that way.

sys/dev/nvme/nvme_sim.c
113 ↗(On Diff #26922)

Yea, that was the point of my rather cryptic, mysterious comment :)

chuck_tuffli.net edited the summary of this revision. (Show Details)

This is v2 of the patch based on feedback / discussions on freebsd-scsi to use different XPT op-codes for NVMe Admin and NVM commands instead of the original xflags approach.

This revision is now accepted and ready to land.Apr 7 2017, 9:24 PM
ken requested changes to this revision.May 3 2017, 9:30 PM
ken added a subscriber: ken.

This needs a case added to passmemsetup() and the CAMIOQUEUE case in passdoioctl() in the pass(4) driver so that asynchronous I/O will work for NVMe devices.

If you want a test case, you could add NVMe support to camdd(8), but that may be more work than you're signing up for. :)

This revision now requires changes to proceed.May 3 2017, 9:30 PM
chuck_tuffli.net edited edge metadata.
chuck_tuffli.net edited the test plan for this revision. (Show Details)

v3 of the patch adds the missing changes to passmemsetup() and passdoioctl() as noted in the previous review.

Looks good, thank you!

This revision is now accepted and ready to land.Jun 20 2017, 2:23 PM

This looks good to me as well...

This revision was automatically updated to reflect the committed changes.