Page MenuHomeFreeBSD

Add CAM/NVMe support for CAM_DATA_SG
ClosedPublic

Authored by chuck_tuffli.net on Jun 25 2017, 10:04 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 24 2023, 7:28 AM
Unknown Object (File)
Dec 20 2023, 3:17 AM
Unknown Object (File)
Dec 13 2023, 6:19 AM
Unknown Object (File)
Nov 10 2023, 1:18 PM
Unknown Object (File)
Nov 8 2023, 1:11 PM
Unknown Object (File)
Nov 6 2023, 4:54 AM
Unknown Object (File)
Oct 7 2023, 12:03 PM
Unknown Object (File)
Oct 5 2023, 3:33 AM

Details

Summary

This adds support in pass(4) for data to be described with a scatter-gather list (sglist) to augment the existing (single) virtual address.

Test Plan

Modified camdd(8) to add support for NVMe to test SG support

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/cam/cam_ccb.h
820 ↗(On Diff #30074)

Before we remove the residual, we would need to confirm that it is not possible to have a residual in NVMe. IMO, it seems wrong to remove it.

sys/cam/nvme/nvme_da.c
1009 ↗(On Diff #30074)

See above about removing the residual.

This looks good to me.

sys/cam/cam_ccb.h
820 ↗(On Diff #30074)

I suggested we remove it because we never use it.

sys/cam/cam_ccb.h
820 ↗(On Diff #30074)

Just because we don't use it doesn't mean we shouldn't. You can represent certain scenarios with the residual. For instance, if a drive can only read part of the requested data, it could return a short read, and we could represent that with the residual. Or if we did a read that went past the end of the drive, the drive could report a short read. (Although reading past the end of the drive shouldn't happen in the block case, we could do it with a passthrough command.)

sys/cam/cam_ccb.h
820 ↗(On Diff #30074)

The primary question here is whether it is possible to have a residual in NVMe, i.e. an overrun or an underrun. If it is not possible, then we don't need the residual. If it is possible, we should have a way to communicate that in the CCB.

sys/cam/cam_ccb.h
820 ↗(On Diff #30074)

The NVME spec has no concept of residual length. A command either fully completes, or it fully fails. If fails then the host should consider that 0 bytes were successful, and therefore the residual should be set to the bio_bcount length of the command. See NVME 1.3 6.9.1, 10.1, and 10.2. My reading is that there is no value in having a resid field in the CCB, the status of success or failure is all that is needed to communicate this information.

sys/cam/cam_ccb.h
820 ↗(On Diff #30074)

No, NVMe doesn't have the concept of a residual. If you feel strongly, I can keep it, but as you can see from the changes, it isn't being used.

sys/cam/cam_ccb.h
820 ↗(On Diff #30074)

Please add a uint16_t field at the end to pad out the removed uint32_t field. It'll help to explicitly provide some space for future needs.

sys/cam/cam_ccb.h
820 ↗(On Diff #30074)

Ok, since it isn't in the spec, I agree, take the residual out.

chuck_tuffli.net marked an inline comment as done.

Added suggested padding to end of struct ccb_nvmeio

This revision is now accepted and ready to land.Aug 29 2017, 6:40 AM
This revision was automatically updated to reflect the committed changes.