Add CAM passthrough for NVMe
ClosedPublic
Actions

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

Details

Reviewers

imp
scottl
ken

Commits

rS320984: This adds CAM pass(4) support for NVMe IO's. Applications indicate

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

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

chuck_tuffli.net created this revision.Apr 3 2017, 12:38 PM

Herald added subscribers: Contributor Reviews (src), imp. · View Herald TranscriptApr 3 2017, 12:38 PM

imp added inline comments.Apr 3 2017, 2:59 PM

sys/cam/scsi/scsi_pass.c
2185 ↗	(On Diff #26922)	why XXX?
sys/dev/nvme/nvme_sim.c
113 ↗	(On Diff #26922)	why 0x10?

chuck_tuffli.net edited the summary of this revision. (Show Details)Apr 4 2017, 3:55 PM

jimharris added a subscriber: jimharris.Apr 4 2017, 3:59 PM

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'.

chuck_tuffli.net added inline comments.Apr 4 2017, 4:01 PM

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.

imp added inline comments.Apr 4 2017, 4:07 PM

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 added reviewers: imp, scottl.Apr 4 2017, 5:24 PM

chuck_tuffli.net marked 2 inline comments as done.Apr 7 2017, 8:55 PM

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.

Looks good to me.

This revision is now accepted and ready to land.Apr 7 2017, 9:24 PM

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