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.

In D41167#937442, @mav wrote:

My position traditionally was that user-space should handle and report its own errors by itself, but kernel should handle some system-wide. I agree that there may be some merits in this. Aside of reporting to devctl this should also properly re-broadcast Unit Attentions, which do not belong to specific command and so periph. As I can see, devctl should already receive device name to differentiate pass-through requests. I wonder it is could report some process identification too?

I think this will be ok. It will give at least some avenue to report errors that go up via the pass(4) driver without additional console spamming.

Looks fine, thanks.

Looks good to me, thanks.

Looks ok to me.