Use the new nvme_ns_data_format_index() from D59627 to determine the current LBA format instead of open-coding the FLBAS extraction, so drives with more than 16 LBA formats get the right default.

Rebase on current head: re-anchor the nvme.h additions around the new nvme_ns_data_format_index() helper from D59627. No content changes to the additions themselves.

Rebase on current head. The moved nvme_print_namespace() now carries the FLBAS Format Index Upper fix from D59627 (2c41f7d8), which landed inside print_namespace() after this review was created.

Rework per the naming discussion: rename the subcommand from nvfeature to feature. SCSI has no feature command and the ATA SET FEATURES subcommands already have dedicated camcontrol subcommands (apm, aam, epc), so the name is unclaimed; the subcommand errors cleanly on non-NVMe devices. Also fix a being/bing typo in the man page.

Rework per the naming discussion: fold NVMe format support into the existing format subcommand (protocol dispatch like identify/persist) and retire nvformat. The NVMe path gains the same are-you-sure confirmation the SCSI path has (-q/-y honored); -r is rejected for NVMe and -w is a no-op since the NVMe format command does not return until the format completes. Tested on hardware: confirmation prompt and abort verified, no formats actually issued.

Address review feedback: error out instead of crashing when -a or -f is missing, and zero the hostid set buffer and only transfer 8 bytes for the non-extended host identifier.

Fix the crypto secure-erase setting to actually map to NVME_FORMAT_SES_CRYPTO (it wrongly mapped to a user data erase), and make the default timeout depend on the erase type: 30 seconds for crypto erase, 30 minutes otherwise. -T still overrides.

Constrain the number of reservation report entries printed to what fits in the data buffer, per review feedback.

Fix the entry count clamp in the reservation status swapbytes functions: sizeof(s) is the pointer size, which made the clamp one entry too permissive (the same out-of-bounds class flagged on D59534).

Rebase on the persist rework in D59534 (the nvres subcommand is gone; no changes to nvfeature itself).

Rebase on the persist rework in D59534 (the nvres subcommand is gone; no changes to nvformat itself).

Rework per review feedback: fold NVMe reservation support into the existing persist subcommand instead of adding an nvres subcommand. persist dispatches on device protocol like identify. SCSI syntax unchanged; NVMe uses the same -i/-o/-k/-K/-T vocabulary, with NVMe-only unregister/replace actions and a new -c option for Persist Through Power Loss State. Tested on reservation-capable hardware (PASCARI X200); details in the summary.

In D59534#1366238, @imp wrote:

I'd have also broken this up by subcommand because all together the review length is starting to get hard to review.
And it's unclear what the actual syntax here is too. It's a case where the old-school camcontrol commands are getting in the way of having commands that are more similar to linux's nvme cli. Translating between what we do and what they do is a lot of friction for no benefit.

Address review feedback: drop the Namespace Management sentence from the man page, and reword the -N usage text to: send identify to the NVMe namespace.

Address review feedback: drop the redundant sys/cdefs.h include (nvme.h pulls in sys/param.h), and annotate the arguments on all calls in the new functions so the comment style is consistent throughout. Also fix two annotation typos (dxfrer_len, and cdw10 on a cdw11 argument) in the hostid wrappers.

The rest of this series is already one commit per camcontrol
subcommand (D59533 through D59536); this revision is the shared nvme.h
definitions they build on. I can split it further (reservations / format /
features) if you'd like.

Adapt to the wire-byte-order hostid convention from D59531: drop the pre-print byte swapping (which also fixes a bug where the 8-byte hostid was truncated through a uint8_t on big-endian), decode the UUID display with uuid_dec_be() to match the uuid_enc_be() used on set, and use le64enc() to build the 8-byte set payload.

Keep the hostid in the little-endian byte order the device returns it in, per review feedback: no more byte swapping in the resv status swapbytes functions or ifdefs in nvme_hostid_sbuf(). exhid is now bool. Drop the nvme_le128tobe()/nvme_le128toh() split, no longer needed. Update nvmecontrol resv print for the wire-order hostid fields (also fixes zero padding and inconsistent LE/BE output).

Fix spelling of the changeable capability name

Looks good to me.

Fine with me...but are we not actually putting the text of the license in the file anymore?

The purpose of invalidating the pack in the da(4) driver is to deal with removable media. If the removable disk is pulled while the device is open, the pack is invalidated. Getting through a successful open again clears the flag.

Change fwload_only to fwload_force.

In D45660#1042762, @mav wrote:

In D45660#1042742, @ken wrote:

So here is what the debugging log message in isp_getpdb() shows. isp0 and isp1 are connected to LTO-6 tape drives via an 8Gb switch. isp2 is directly connected to an LTO-6 in loop mode:

isp0: Chan 0 handle 0x0 Port 0xfffc01 flags 0x0 curstate 77 laststate 77
isp0: Chan 0 handle 0x1 Port 0x011b26 flags 0x40a0 curstate 46 laststate 46
isp0: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp0: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp1: Chan 0 handle 0x0 Port 0xfffc01 flags 0x0 curstate 77 laststate 77
isp1: Chan 0 handle 0x1 Port 0x011a26 flags 0x40a0 curstate 46 laststate 46
isp1: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp1: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp2: Chan 0 handle 0x0 Port 0x000026 flags 0x40a0 curstate 46 laststate 46

So it seems the upper 4 bits can actually have different values, not some "non-NVMe" constant, as I thought. I am OK with this patch if it solves the issue, but it would definitely be good to understand what it is, in case there is something important.

In D45688#1042676, @mav wrote:

It seems a good tunable, except I am not getting the meaning of "only" there. Why not "always", "force" or something like that?

In D45660#1042675, @mav wrote:

None of QLogic documents I have know nothing about NVMe, and this state field is declared is byte there. I have no objections for this patch, but a bit curios what NVMe status do we see there for non-NVMe devices.

Looks like the copyright text is missing.

Looks good, thank you for doing this! I'm glad to finally see CTL NVMe support going in.

This should be fine, thanks!

Looks fine to me, thanks!

Use BUS_SPACE_MAXSIZE_32BIT+1 to represent the 4GB boundary instead of 0x100000000.

Good idea. Sbuf didn't exist back when the debug macros were written, this is an improvement.

Looks good, thank you!

This makes it sound like unmapped I/O and rotating media support will be removed in FreeBSD 15.

Looks ok to me.

Looks good to me.

Looks fine to me.