Page MenuHomeFreeBSD
Differential D59535

camcontrol: Support NVMe namespace formatting via the format subcommand
Needs ReviewPublic
Actions

Authored by ken on Wed, Sep 9, 3:34 PM.
Tags
None
Referenced Files
F174398530: D59535.id188082.diff
Fri, Oct 2, 10:17 PM
F174391083: D59535.id187925.diff
Fri, Oct 2, 9:09 PM
F174383578: D59535.diff
Fri, Oct 2, 8:10 PM
F174310882: D59535.id188142.diff
Fri, Oct 2, 6:51 AM
F174293342: D59535.id.diff
Fri, Oct 2, 2:58 AM
Unknown Object (File)
Thu, Oct 1, 4:19 AM
Unknown Object (File)
Wed, Sep 30, 1:04 AM
Unknown Object (File)
Wed, Sep 30, 12:59 AM
View All 48 Files
Subscribers
imp
ziaee

Details

Reviewers
None
Group Reviewers
cam
Summary

Support formatting NVMe namespaces through the existing format
subcommand rather than a separate nvformat subcommand. The format
subcommand now dispatches on the device protocol, in the same way the
identify and persist subcommands do, and issues the NVMe FORMAT NVM
command for NVMe devices.

The SCSI syntax and behavior are unchanged. The NVMe-specific
options are -e (secure erase setting), -G (format all namespaces),
-f (LBA format), -l (protection information location), -m (metadata
setting) and -p (protection information type). The -q and -y options
work for both protocols; NVMe formats gain the same confirmation
question that SCSI formats have always had. The -r and -w options
are SCSI only; -r returns an error for NVMe devices and -w has no
effect, since the NVMe format command does not complete until the
format is finished.

The default timeout for NVMe is 30 seconds for a cryptographic erase
and 30 minutes otherwise; -T overrides.

sbin/camcontrol/nvformat.c:

New file, the NVMe format backend for the format subcommand.

sbin/camcontrol/camcontrol.c:

Dispatch the format subcommand to scsiformat() or
nvmeformat() based on the device protocol.

sbin/camcontrol/camcontrol.h:

Add the nvmeformat() prototype.

sbin/camcontrol/Makefile:

Add nvformat.c to the build.

sbin/camcontrol/camcontrol.8:

Document NVMe support in the format subcommand section, and
note the differences from SCSI where they occur.

Sponsored by: Spectra Logic

Depends on D59534

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 77426
Build 74309: arc lint + arc unit

Event Timeline

ken created this revision.Wed, Sep 9, 3:34 PM
Herald added subscribers: ziaee, imp. · View Herald TranscriptWed, Sep 9, 3:35 PM
ken requested review of this revision.Wed, Sep 9, 3:35 PM
Harbormaster completed remote builds in B76700: Diff 186278.Wed, Sep 9, 3:35 PM
ken added a child revision: D59536: camcontrol: Add a feature subcommand for NVMe Get/Set Features.Wed, Sep 9, 3:35 PM
ken edited the summary of this revision. (Show Details)Wed, Sep 9, 3:41 PM
imp added inline comments.Wed, Sep 9, 7:54 PM
sbin/camcontrol/camcontrol.c
238

What's NV?
This formats a namespace

Also, why not just format with a protocol selector like we do for identify, sanitize, etc.

sbin/camcontrol/nvformat.c
97

I'd be tempted to rename scsi_get_nv to something that doesn't have scsi in the name, since it's not a scsi thing at all anymore, with backwards compat symbols for a while.

253

I'm pretty sure the extra comments here aren't really helping

ken updated this revision to Diff 187358.Mon, Sep 21, 9:32 PM
ken edited the summary of this revision. (Show Details)

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

Harbormaster completed remote builds in B77145: Diff 187358.Mon, Sep 21, 9:32 PM
imp added inline comments.Fri, Sep 25, 5:36 PM
sbin/camcontrol/nvformat.c
63

Sure this should be NVME_FORMAT_SES_CRYPTO?

253

So I think the timeout is too large. For a crypto format it should be shorter, like 30seconds not 30 minutes.
Otherwise its likely the least sucky thing we can do without a lot of extra code in the kernel.

I ran into this issue in nvmecontrol in https://reviews.freebsd.org/D59828

ken updated this revision to Diff 187925.Mon, Sep 28, 7:34 PM

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.

Harbormaster completed remote builds in B77426: Diff 187925.Mon, Sep 28, 7:34 PM
ken updated this revision to Diff 188082.Tue, Sep 29, 6:49 PM
ken retitled this revision from camcontrol: Add an nvformat subcommand to format NVMe devices to camcontrol: Support NVMe namespace formatting via the format subcommand.
ken edited the summary of this revision. (Show Details)

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.

Harbormaster completed remote builds in B77462: Diff 188082.Tue, Sep 29, 6:49 PM
ken updated this revision to Diff 188142.Tue, Sep 29, 9:11 PM

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.

Harbormaster completed remote builds in B77507: Diff 188142.Tue, Sep 29, 9:11 PM

Revision Contents
Changeset List

PathSize
sbin/
camcontrol/
2 lines
3 lines
75 lines
20 lines
278 lines

Diff 187925

View Options

sbin/camcontrol/Makefile

Loading...
View Options

sbin/camcontrol/camcontrol.h

Loading...
View Options

sbin/camcontrol/camcontrol.8

Loading...
View Options

sbin/camcontrol/camcontrol.c

Loading...
View Options

sbin/camcontrol/nvformat.c

Loading...