[PATCH] nvme: fix PI insert on write

[PATCH] nvme: fix PI insert on write

[Christoph Hellwig]

I recently ran into an issue where the PI generated using the block layer
integrity code differs from that from a kernel using the PRACT fallback
when the block layer integrity code is disabled, and I tracked this down
to us using PRACT incorrectly.

The NVM Command Set Specification (section 5.33 in 1.2, similar in older
versions) specifies the PRACT insert behavior as:

  Inserted protection information consists of the computed CRC for the
  protection information format (refer to section 5.3.1) in the Guard
  field, the LBAT field value in the Application Tag field, the LBST
  field value in the Storage Tag field, if defined, and the computed
  reference tag in the Logical Block Reference Tag.

Where the computed reference tag is defined as following for type 1 and
type 2 using the text below that is duplicated in the respective bullet
points:

  the value of the computed reference tag for the first logical block of
  the command is the value contained in the Initial Logical Block
  Reference Tag (ILBRT) or Expected Initial Logical Block Reference Tag
  (EILBRT) field in the command, and the computed reference tag is
  incremented for each subsequent logical block.

So we need to set ILBRT field, but we currently don't.  Interestingly
this works fine on my older type 1 formatted SSD, but Qemu trips up on
this.  We already set ILBRT for Write Same since commit aeb7bb061be5
("nvme: set the PRACT bit when using Write Zeroes with T10 PI").

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9707a0e8c431..3bd6ffd1f809 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1039,6 +1039,7 @@ static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
 			if (WARN_ON_ONCE(!nvme_ns_has_pi(ns->head)))
 				return BLK_STS_NOTSUPP;
 			control |= NVME_RW_PRINFO_PRACT;
+			nvme_set_ref_tag(ns, cmnd, req);
 		}
 
 		if (bio_integrity_flagged(req->bio, BIP_CHECK_GUARD))
-- 
2.47.2

Re: [PATCH] nvme: fix PI insert on write

[Keith Busch]

On Wed, Aug 20, 2025 at 10:11:47AM +0200, Christoph Hellwig wrote:
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 9707a0e8c431..3bd6ffd1f809 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1039,6 +1039,7 @@ static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
>  			if (WARN_ON_ONCE(!nvme_ns_has_pi(ns->head)))
>  				return BLK_STS_NOTSUPP;
>  			control |= NVME_RW_PRINFO_PRACT;
> +			nvme_set_ref_tag(ns, cmnd, req);
>  		}

Do we need to check that it's type 1 or 2 before setting this?

Re: [PATCH] nvme: fix PI insert on write

[Kanchan Joshi]

On 8/20/2025 8:09 PM, Keith Busch wrote:
>> @@ -1039,6 +1039,7 @@ static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
>>   			if (WARN_ON_ONCE(!nvme_ns_has_pi(ns->head)))
>>   				return BLK_STS_NOTSUPP;
>>   			control |= NVME_RW_PRINFO_PRACT;
>> +			nvme_set_ref_tag(ns, cmnd, req);
>>   		}
> Do we need to check that it's type 1 or 2 before setting this?

Seems so.
And if the check moves inside nvme_set_ref_tag(), it will keep 
current/future call-sites simple.

Re: [PATCH] nvme: fix PI insert on write

[Christoph Hellwig]

On Thu, Aug 21, 2025 at 10:32:13AM +0530, Kanchan Joshi wrote:
> On 8/20/2025 8:09 PM, Keith Busch wrote:
> >> @@ -1039,6 +1039,7 @@ static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
> >>   			if (WARN_ON_ONCE(!nvme_ns_has_pi(ns->head)))
> >>   				return BLK_STS_NOTSUPP;
> >>   			control |= NVME_RW_PRINFO_PRACT;
> >> +			nvme_set_ref_tag(ns, cmnd, req);
> >>   		}
> > Do we need to check that it's type 1 or 2 before setting this?
> 
> Seems so.
> And if the check moves inside nvme_set_ref_tag(), it will keep 
> current/future call-sites simple.

Yes, that was my thought as well when reading Keith' reply.