[PATCH v1 0/3] Fix integrity sysfs reporting inconsistencies across NVMe, SCSI, and DM

[PATCH v1 0/3] Fix integrity sysfs reporting inconsistencies across NVMe, SCSI, and DM

[Anuj Gupta]

This patch series addresses inconsistencies in block integrity sysfs
reporting across NVMe, SCSI, and device-mapper (DM) stacked devices.

Patch 1: Ensures DM devices correctly propagate device_is_integrity_capable
and properly report read_verify and write_generate sysfs attributes
Patch 2: Fixes NVMe integrity sysfs attributes for namespaces without
PI support.
Patch 3: Corrects SCSI integrity reporting when the HBA does not support
DIX.

Anuj Gupta (3):
  block: Fix incorrect integrity sysfs reporting for DM devices
  nvme: Fix incorrect block integrity sysfs values for non-PI namespaces
  scsi: Fix incorrect integrity sysfs values when HBA doesn't support
    DIX

 block/blk-settings.c     | 9 +++++++--
 drivers/nvme/host/core.c | 4 ++++
 drivers/scsi/sd_dif.c    | 4 +++-
 3 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH v1 1/3] block: Fix incorrect integrity sysfs reporting for DM devices

[Anuj Gupta]

The integrity stacking logic in device-mapper currently does not
explicitly mark the device with BLK_INTEGRITY_NOGENERATE and
BLK_INTEGRITY_NOVERIFY when the underlying device(s) do not support
integrity. This can lead to incorrect sysfs reporting of integrity
attributes.

Additionally, queue_limits_stack_integrity() incorrectly sets
BLK_INTEGRITY_DEVICE_CAPABLE for a DM device even when none of its
underlying devices support integrity. This happens because the flag is
blindly inherited from the first base device, even if it lacks integrity
support.

This patch ensures:
1. BLK_INTEGRITY_NOGENERATE and BLK_INTEGRITY_NOVERIFY are set correctly:
   - When the underlying device does not support integrity.
   - When integrity stacking fails due to incompatible profiles.
2. device_is_integrity_capable is correctly propagated to reflect the
actual capability of the stacked device.

Reported-by: M Nikhil <nikhilm@linux.ibm.com>
Link: https://lore.kernel.org/linux-block/f6130475-3ccd-45d2-abde-3ccceada0f0a@linux.ibm.com/
Fixes: c6e56cf6b2e7 ("block: move integrity information into queue_limits")
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
 block/blk-settings.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index c44dadc35e1e..c32517c8bc2e 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -861,7 +861,8 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
 
 	if (!ti->tuple_size) {
 		/* inherit the settings from the first underlying device */
-		if (!(ti->flags & BLK_INTEGRITY_STACKED)) {
+		if (!(ti->flags & BLK_INTEGRITY_STACKED) &&
+		    (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)) {
 			ti->flags = BLK_INTEGRITY_DEVICE_CAPABLE |
 				(bi->flags & BLK_INTEGRITY_REF_TAG);
 			ti->csum_type = bi->csum_type;
@@ -871,8 +872,11 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
 			ti->tag_size = bi->tag_size;
 			goto done;
 		}
-		if (!bi->tuple_size)
+		if (!bi->tuple_size) {
+			ti->flags |= BLK_INTEGRITY_NOGENERATE |
+				     BLK_INTEGRITY_NOVERIFY;
 			goto done;
+		}
 	}
 
 	if (ti->tuple_size != bi->tuple_size)
@@ -893,6 +897,7 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
 
 incompatible:
 	memset(ti, 0, sizeof(*ti));
+	ti->flags |= BLK_INTEGRITY_NOGENERATE | BLK_INTEGRITY_NOVERIFY;
 	return false;
 }
 EXPORT_SYMBOL_GPL(queue_limits_stack_integrity);
-- 
2.25.1

Re: [PATCH v1 1/3] block: Fix incorrect integrity sysfs reporting for DM devices

[Christoph Hellwig]

On Tue, Feb 25, 2025 at 10:16:51AM +0530, Anuj Gupta wrote:
> The integrity stacking logic in device-mapper currently does not
> explicitly mark the device with BLK_INTEGRITY_NOGENERATE and
> BLK_INTEGRITY_NOVERIFY when the underlying device(s) do not support
> integrity. This can lead to incorrect sysfs reporting of integrity
> attributes.
> 
> Additionally, queue_limits_stack_integrity() incorrectly sets
> BLK_INTEGRITY_DEVICE_CAPABLE for a DM device even when none of its
> underlying devices support integrity. This happens because the flag is
> blindly inherited from the first base device, even if it lacks integrity
> support.
> 
> This patch ensures:
> 1. BLK_INTEGRITY_NOGENERATE and BLK_INTEGRITY_NOVERIFY are set correctly:
>    - When the underlying device does not support integrity.
>    - When integrity stacking fails due to incompatible profiles.
> 2. device_is_integrity_capable is correctly propagated to reflect the
> actual capability of the stacked device.
> 
> Reported-by: M Nikhil <nikhilm@linux.ibm.com>
> Link: https://lore.kernel.org/linux-block/f6130475-3ccd-45d2-abde-3ccceada0f0a@linux.ibm.com/
> Fixes: c6e56cf6b2e7 ("block: move integrity information into queue_limits")
> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> ---
>  block/blk-settings.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index c44dadc35e1e..c32517c8bc2e 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -861,7 +861,8 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
>  
>  	if (!ti->tuple_size) {
>  		/* inherit the settings from the first underlying device */
> -		if (!(ti->flags & BLK_INTEGRITY_STACKED)) {
> +		if (!(ti->flags & BLK_INTEGRITY_STACKED) &&
> +		    (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)) {
>  			ti->flags = BLK_INTEGRITY_DEVICE_CAPABLE |
>  				(bi->flags & BLK_INTEGRITY_REF_TAG);
>  			ti->csum_type = bi->csum_type;

Hmm.  I wonder if this is the correct logic.  Basically we do not want to
allow mixing integrity capable and not integrity devices, do we?

So maybe the logic should be more something like:

	if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY))
		return true;

	if (ti->flags & BLK_INTEGRITY_STACKED) {
		/* check blk_integrity compatibility */
	} else {
		ti->flags = BLK_INTEGRITY_STACKED;
		/* inherit blk_integrity, including the empty one  */
	}

Re: [PATCH v1 1/3] block: Fix incorrect integrity sysfs reporting for DM devices

[Anuj gupta]

> > diff --git a/block/blk-settings.c b/block/blk-settings.c
> > index c44dadc35e1e..c32517c8bc2e 100644
> > --- a/block/blk-settings.c
> > +++ b/block/blk-settings.c
> > @@ -861,7 +861,8 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
> >
> >       if (!ti->tuple_size) {
> >               /* inherit the settings from the first underlying device */
> > -             if (!(ti->flags & BLK_INTEGRITY_STACKED)) {
> > +             if (!(ti->flags & BLK_INTEGRITY_STACKED) &&
> > +                 (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)) {
> >                       ti->flags = BLK_INTEGRITY_DEVICE_CAPABLE |
> >                               (bi->flags & BLK_INTEGRITY_REF_TAG);
> >                       ti->csum_type = bi->csum_type;
>
> Hmm.  I wonder if this is the correct logic.  Basically we do not want to
> allow mixing integrity capable and not integrity devices, do we?

It is about a situation where a non-integrity-capable device incorrectly
reports integrity capability due to improper flag propagation. The issue
is that BLK_INTEGRITY_DEVICE_CAPABLE is set incorrectly even when the
first underlying device does not support integrity. This part of the patch
tries to fix that.
For example, when I create a dm-linear device using an integrity-incapable
device, the resulting DM device wrongly reports integrity capability [1]

Rest of the handling in this patch would not be required once we correctly
initialize in blk_validate_integrity_limits as you suggested in the other
reply [2]

[1]
# cat /sys/block/nvme0n1/integrity/device_is_integrity_capable
0
# echo 0 409600 linear /dev/nvme0n1 0 > /tmp/table
# echo 409600 409600 linear /dev/nvme0n1 0 >> /tmp/table
# dmsetup create two /tmp/table
# cat /sys/block/dm-0/integrity/device_is_integrity_capable
1

[2]
https://lore.kernel.org/linux-block/20250225150753.GB6099@lst.de/

> So maybe the logic should be more something like:
>
>         if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY))
>                 return true;
>
>         if (ti->flags & BLK_INTEGRITY_STACKED) {
>                 /* check blk_integrity compatibility */
>         } else {
>                 ti->flags = BLK_INTEGRITY_STACKED;
>                 /* inherit blk_integrity, including the empty one  */
>         }
>

[PATCH v1 2/3] nvme: Fix incorrect block integrity sysfs values for non-PI namespaces

[Anuj Gupta]

Commit 9f4aa46f2a74
("block: invert the BLK_INTEGRITY{GENERATE,VERIFY} flags") caused
read_verify and write_generate to report 1 for NVMe namespaces
without PI support. This happens because BLK_INTEGRITY_NOGENERATE and
BLK_INTEGRITY_NOVERIFY were not explicitly set.

Fix this by initializing these flags by default and clearing them only
when the namespace supports PI.

Fixes: 9f4aa46f2a74 ("block: invert the BLK_INTEGRITY_{GENERATE,VERIFY} flags")
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
 drivers/nvme/host/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 818d4e49aab5..fe599a811d38 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1799,6 +1799,7 @@ static bool nvme_init_integrity(struct nvme_ns_head *head,
 
 	memset(bi, 0, sizeof(*bi));
 
+	bi->flags = BLK_INTEGRITY_NOGENERATE | BLK_INTEGRITY_NOVERIFY;
 	if (!head->ms)
 		return true;
 
@@ -1850,6 +1851,9 @@ static bool nvme_init_integrity(struct nvme_ns_head *head,
 		break;
 	}
 
+	if (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)
+		bi->flags &= ~(BLK_INTEGRITY_NOGENERATE |
+			       BLK_INTEGRITY_NOVERIFY);
 	bi->tuple_size = head->ms;
 	bi->pi_offset = info->pi_offset;
 	return true;
-- 
2.25.1

Re: [PATCH v1 2/3] nvme: Fix incorrect block integrity sysfs values for non-PI namespaces

[Christoph Hellwig]

On Tue, Feb 25, 2025 at 10:16:52AM +0530, Anuj Gupta wrote:
>  	memset(bi, 0, sizeof(*bi));
>  
> +	bi->flags = BLK_INTEGRITY_NOGENERATE | BLK_INTEGRITY_NOVERIFY;
>  	if (!head->ms)
>  		return true;
>  
> @@ -1850,6 +1851,9 @@ static bool nvme_init_integrity(struct nvme_ns_head *head,
>  		break;
>  	}
>  
> +	if (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)
> +		bi->flags &= ~(BLK_INTEGRITY_NOGENERATE |
> +			       BLK_INTEGRITY_NOVERIFY);

I don't think the driver is the proper place to do this, this should
be done in blk_validate_integrity_limits.  That should also take care
or the stacked devices and remove the need for the last hunk in the
previous patch.

[PATCH v1 3/3] scsi: Fix incorrect integrity sysfs values when HBA doesn't support DIX

[Anuj Gupta]

For SCSI disks where the host bus adapter (HBA) does not support
Data Integrity Extensions (DIX), the write_generate and read_verify sysfs
attributes incorrectly report 1 instead of 0.

This happens because the BLK_INTEGRITY_NOGENERATE and
BLK_INTEGRITY_NOVERIFY flags are not explicitly set when DIX is disabled.
Fix this by properly setting these flags when DIX is not enabled.

Reported-by: M Nikhil <nikhilm@linux.ibm.com>
Link: https://lore.kernel.org/linux-block/f6130475-3ccd-45d2-abde-3ccceada0f0a@linux.ibm.com/
Fixes: 9f4aa46f2a74 ("block: invert the BLK_INTEGRITY_{GENERATE,VERIFY} flags")
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
 drivers/scsi/sd_dif.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index ae6ce6f5d622..be2cd06f500b 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -40,8 +40,10 @@ void sd_dif_config_host(struct scsi_disk *sdkp, struct queue_limits *lim)
 		dif = 0; dix = 1;
 	}
 
-	if (!dix)
+	if (!dix) {
+		bi->flags |= BLK_INTEGRITY_NOGENERATE | BLK_INTEGRITY_NOVERIFY;
 		return;
+	}
 
 	/* Enable DMA of protection information */
 	if (scsi_host_get_guard(sdkp->device->host) & SHOST_DIX_GUARD_IP)
-- 
2.25.1