Page MenuHomeFreeBSD

nvme: Add reservation, format and features command building support
Needs ReviewPublic

Authored by ken on Wed, Sep 9, 3:33 PM.

Details

Reviewers
None
Group Reviewers
cam
Summary

Add command (struct nvme_command) building functions for the four
NVMe reservation opcodes (Report, Register, Acquire and Release),
Format NVM, and Get Features / Set Features, along with the
enumerations and data structures that go with them. These are
shared between the kernel and userland, and are the underpinnings
for new camcontrol(8) subcommands (nvres, nvformat, nvfeature) that
will be added separately.

The reservation type names are intentionally similar to the SCSI
names for the same reservation types.

sys/dev/nvme/nvme.h:

Add NVMe command building functions for the four NVMe
reservation opcodes: nvme_ns_resv_report_cmd(),
nvme_ns_resv_register_cmd(), nvme_ns_resv_acquire_cmd() and
nvme_ns_resv_release_cmd().

Add enumerated types for the various NVMe reservation
subcommand actions (acquire, register, release), reservation
types, and Change Persist Through Power Loss State values.

Add struct nvme_resv_acq_data and struct nvme_resv_reg_data,
the data payloads for reservation Acquire and Register.

Add a new NVMe Format NVM command building function,
nvme_admin_format_cmd(), and an enumeration for the Secure
Erase Setting parameter for format.

Add nvme_admin_get_feature_cmd() and
nvme_admin_set_feature_cmd() functions to fill a struct
nvme_command for Get Features / Set Features.

Add macros for bitfields in the ctratt and mic dwords of the
NVMe Identify Controller data structure.

Add nvme_le128tobe() and nvme_le128toh() helpers for 16 byte
quantities, and a prototype for nvme_hostid_sbuf().

Widen the nvme_ctrlr_cmd_get_feature() and
nvme_ctrlr_cmd_set_feature() prototypes as described below.

sys/dev/nvme/nvme_util.c:

Add nvme_hostid_sbuf(), which formats an 8 or 16 byte hostid
(from the Get Features hostid feature) into an sbuf as an
integer value in base 16.

sys/dev/nvme/nvme_ctrlr_cmd.c:

Refactor nvme_ctrlr_cmd_get_feature() and
nvme_ctrlr_cmd_set_feature() to use the new command building
functions from nvme.h and accept a namespace ID parameter.

Refactor nvme_ctrlr_cmd_set_feature() to also accept the sv
(save) parameter to persist a feature value across reset.

sys/dev/nvme/nvme_ctrlr.c:

Update the temperature threshold, autonomous power state
transition and host memory buffer callers for the new
nvme_ctrlr_cmd_get_feature() and nvme_ctrlr_cmd_set_feature()
arguments.

Co-authored-by: Reid Linnemann <reidl@spectralogic.com>
Co-authored-by: Dave Baukus <daveb@spectralogic.com>
Sponsored by: Spectra Logic

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 76696
Build 73579: arc lint + arc unit

Event Timeline

ken requested review of this revision.Wed, Sep 9, 3:33 PM

This is fine, but I'll note that this is just on the edge of reviewability and it would be best if you broke up the commits per command in the future.

sys/dev/nvme/nvme_util.c
274

Why assume it's in host byte order? In general, we've been trying to undo the magic twiddling of structures to host order except when needed. The original big endian support erred when it did this, and as standards expand old structures, too much high level interpretive knowledge lives at the lowest levels...

In addition, if it was always in device byte order, we wouldn't need an ifdef here.

276

exhid should be a bool