Re: [PATCH v4] nvme: change nvme_ns_has_pi() to nvme_ns_supports_pract()

[Keith Busch <kbusch@kernel.org>]

On Wed, Oct 23, 2024 at 11:44:13PM +0900, Tokunori Ikegami wrote:
> About the mention "The spec says it only works if ms == PI tuple size,
> otherwise it doesn't do anything." do you mention for example the
> specification description below?
> --------------------------------------------------------------------------------
> (NVM-Express-NVM-Command-Set-Specification-Revision-1.1-2024.08.05-Ratified)
> 5.3.2.1 Protection Information and Write Commands
> Figure 166 provides some examples of the protection information processing
> that may occur as a side effect
> of a Write command.
> ...
> If the namespace is formatted with protection information and the PRACT bit
> is set to `1´, then:
> ...
> 2. If the namespace is formatted with Metadata Size greater than
> protection information size, then the logical block data and the
> metadata are transferred from the host buffer to the controller. As
> the metadata passes through the controller, the controller overwrites
> the protection information portion of the metadata without checking
> the protection information portion regardless of PRCHK settings.  The
> logical block data and metadata are written to the NVM (i.e., the
> metadata field remains the same size in the NVM and the host buffer).
> The location of the protection information within the metadata is
> configured when the namespace is formatted (refer to the DPS field in
> Figure 114).
> Figure 166: Write Command 16b Guard Protection Information Processing
> ...
> [figure b]
> b) MD>8 (e.g., 16), PI, PRACT=0: Metadata remains same size in NVM and host
> buffer
> ...
> [figure d]
> d) MD>8 (e.g., 16), PI, PRACT=1: Metadata remains same size in NVM and host
> buffer
> --------------------------------------------------------------------------------
> In the example case b) and d) the 16B MD data should be sent by the host
> application but the driver does not set or handle any 8MD and 8PI. Is this
> understanding correct?

The important point for those MS>8 formats is the host *must* provide a
buffer for the transfer, otherwise you'll get corruption. This means
those formats are not usable if your kernel didn't set
CONFIG_BLK_DEV_INTEGRTY, or you have a fabrics transport that doesn't
support these.

The format where MS == PI size can still work without
CONFIG_BLK_DEV_INTEGRTY or fabrics support because the the pr-action
disables the host<->device transfers.

The function, nvme_ns_has_pi(), is specifically looking for the formats
that don't require the host to provide a metadata buffer. The comment
above the function says exactly this:

  "NVMe ns supports metadata actions by the controller (generate/strip)"

The only time that happens with PRACT is if metadata size equals the PI
size. Section 2.1.5, Figure 9 says this more succiently.

> By the way actually does the kernel nvme driver support the ms >= PI LBAF
> case as correct by the current implementation? (Seems only supported for the
> ms == PI LBAF case only.)
>
>   Note: Only the function nvme_configure_metadata() checks the ms >= PI case
> to set PI type.

It should work fine with any MS>PI size as long as the disk can
successfully register with blk_integrity, which again, depends on your
kernel config and transport type. That block layer can handle protection
information with extra metadata in the same buffer.