consolidate T10 PI defintions

consolidate T10 PI defintions

[Christoph Hellwig]

Hi Martin,

this series ensures scsi_debug and qla2xxx use the common t10_pi_tuple
defintion, and then also moves the remaining T10 PI related defintions
to <linux/t10-pi.h>.

[PATCH 1/3] scsi_debug: use struct t10_pi_tuple instead of struct sd_dif_tuple

[Christoph Hellwig]

And remove the declaration of the latter in sd.h as scsi_debug was the
only user.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_debug.c | 15 ++++++++-------
 drivers/scsi/sd.h         |  9 ---------
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 6a219a0..044fc93 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -42,6 +42,7 @@
 #include <linux/atomic.h>
 #include <linux/hrtimer.h>
 #include <linux/uuid.h>
+#include <linux/t10-pi.h>
 
 #include <net/checksum.h>
 
@@ -627,7 +628,7 @@ static LIST_HEAD(sdebug_host_list);
 static DEFINE_SPINLOCK(sdebug_host_list_lock);
 
 static unsigned char *fake_storep;	/* ramdisk storage */
-static struct sd_dif_tuple *dif_storep;	/* protection info */
+static struct t10_pi_tuple *dif_storep;	/* protection info */
 static void *map_storep;		/* provisioning map */
 
 static unsigned long map_size;
@@ -682,7 +683,7 @@ static void *fake_store(unsigned long long lba)
 	return fake_storep + lba * sdebug_sector_size;
 }
 
-static struct sd_dif_tuple *dif_store(sector_t sector)
+static struct t10_pi_tuple *dif_store(sector_t sector)
 {
 	sector = sector_div(sector, sdebug_store_sectors);
 
@@ -2430,7 +2431,7 @@ static __be16 dif_compute_csum(const void *buf, int len)
 	return csum;
 }
 
-static int dif_verify(struct sd_dif_tuple *sdt, const void *data,
+static int dif_verify(struct t10_pi_tuple *sdt, const void *data,
 		      sector_t sector, u32 ei_lba)
 {
 	__be16 csum = dif_compute_csum(data, sdebug_sector_size);
@@ -2504,7 +2505,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
 			    unsigned int sectors, u32 ei_lba)
 {
 	unsigned int i;
-	struct sd_dif_tuple *sdt;
+	struct t10_pi_tuple *sdt;
 	sector_t sector;
 
 	for (i = 0; i < sectors; i++, ei_lba++) {
@@ -2696,7 +2697,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
 			     unsigned int sectors, u32 ei_lba)
 {
 	int ret;
-	struct sd_dif_tuple *sdt;
+	struct t10_pi_tuple *sdt;
 	void *daddr;
 	sector_t sector = start_sec;
 	int ppage_offset;
@@ -2722,7 +2723,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
 		}
 
 		for (ppage_offset = 0; ppage_offset < piter.length;
-		     ppage_offset += sizeof(struct sd_dif_tuple)) {
+		     ppage_offset += sizeof(struct t10_pi_tuple)) {
 			/* If we're at the end of the current
 			 * data page advance to the next one
 			 */
@@ -5026,7 +5027,7 @@ static int __init scsi_debug_init(void)
 	if (sdebug_dix) {
 		int dif_size;
 
-		dif_size = sdebug_store_sectors * sizeof(struct sd_dif_tuple);
+		dif_size = sdebug_store_sectors * sizeof(struct t10_pi_tuple);
 		dif_storep = vmalloc(dif_size);
 
 		pr_err("dif_storep %u bytes @ %p\n", dif_size, dif_storep);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 765a6f1..d00966d 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -239,15 +239,6 @@ static inline unsigned int sd_prot_flag_mask(unsigned int prot_op)
 	return flag_mask[prot_op];
 }
 
-/*
- * Data Integrity Field tuple.
- */
-struct sd_dif_tuple {
-       __be16 guard_tag;	/* Checksum */
-       __be16 app_tag;		/* Opaque storage */
-       __be32 ref_tag;		/* Target LBA or indirect LBA */
-};
-
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 
 extern void sd_dif_config_host(struct scsi_disk *);
-- 
2.1.4

Re: [PATCH 1/3] scsi_debug: use struct t10_pi_tuple instead of struct sd_dif_tuple

[Bart Van Assche]

On 09/11/2016 07:35 PM, Christoph Hellwig wrote:
> And remove the declaration of the latter in sd.h as scsi_debug was the
> only user.

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>

[PATCH 2/3] qla2xxx: use struct t10_pi_tuple

[Christoph Hellwig]

Instead of defining a local version of it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/qla2xxx/qla_def.h | 10 ----------
 drivers/scsi/qla2xxx/qla_isr.c |  2 +-
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index ae4a747..73b12e4 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -279,16 +279,6 @@ struct req_que;
 struct qla_tgt_sess;
 
 /*
- * (sd.h is not exported, hence local inclusion)
- * Data Integrity Field tuple.
- */
-struct sd_dif_tuple {
-	__be16 guard_tag;	/* Checksum */
-	__be16 app_tag;		/* Opaque storage */
-	__be32 ref_tag;		/* Target LBA or indirect LBA */
-};
-
-/*
  * SCSI Request Block
  */
 struct srb_cmd {
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 987f1c7..068c4e4 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1828,7 +1828,7 @@ qla2x00_handle_dif_error(srb_t *sp, struct sts_entry_24xx *sts24)
 		if (scsi_prot_sg_count(cmd)) {
 			uint32_t i, j = 0, k = 0, num_ent;
 			struct scatterlist *sg;
-			struct sd_dif_tuple *spt;
+			struct t10_pi_tuple *spt;
 
 			/* Patch the corresponding protection tags */
 			scsi_for_each_prot_sg(cmd, sg,
-- 
2.1.4

Re: [PATCH 2/3] qla2xxx: use struct t10_pi_tuple

[Bart Van Assche]

On 09/11/2016 07:35 PM, Christoph Hellwig wrote:
> Instead of defining a local version of it.

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>

[PATCH 3/3] sd: move DIF protection types to t10-pi.h

[Christoph Hellwig]

These should go together with the rest of the T10 protection
information defintions.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_debug.c | 39 +++++++++++++++++++--------------------
 drivers/scsi/sd.c         | 11 ++++++-----
 drivers/scsi/sd.h         | 21 ---------------------
 drivers/scsi/sd_dif.c     | 10 +++++-----
 include/linux/t10-pi.h    | 20 ++++++++++++++++++++
 5 files changed, 50 insertions(+), 51 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 044fc93..5f0695a 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1350,7 +1350,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		} else if (0x86 == cmd[2]) { /* extended inquiry */
 			arr[1] = cmd[2];	/*sanity */
 			arr[3] = 0x3c;	/* number of following entries */
-			if (sdebug_dif == SD_DIF_TYPE3_PROTECTION)
+			if (sdebug_dif == T10_DIF_TYPE3_PROTECTION)
 				arr[4] = 0x4;	/* SPT: GRD_CHK:1 */
 			else if (have_dif_prot)
 				arr[4] = 0x5;   /* SPT: GRD_CHK:1, REF_CHK:1 */
@@ -2443,13 +2443,13 @@ static int dif_verify(struct t10_pi_tuple *sdt, const void *data,
 			be16_to_cpu(csum));
 		return 0x01;
 	}
-	if (sdebug_dif == SD_DIF_TYPE1_PROTECTION &&
+	if (sdebug_dif == T10_DIF_TYPE1_PROTECTION &&
 	    be32_to_cpu(sdt->ref_tag) != (sector & 0xffffffff)) {
 		pr_err("REF check failed on sector %lu\n",
 			(unsigned long)sector);
 		return 0x03;
 	}
-	if (sdebug_dif == SD_DIF_TYPE2_PROTECTION &&
+	if (sdebug_dif == T10_DIF_TYPE2_PROTECTION &&
 	    be32_to_cpu(sdt->ref_tag) != ei_lba) {
 		pr_err("REF check failed on sector %lu\n",
 			(unsigned long)sector);
@@ -2581,13 +2581,13 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		break;
 	}
 	if (unlikely(have_dif_prot && check_prot)) {
-		if (sdebug_dif == SD_DIF_TYPE2_PROTECTION &&
+		if (sdebug_dif == T10_DIF_TYPE2_PROTECTION &&
 		    (cmd[1] & 0xe0)) {
 			mk_sense_invalid_opcode(scp);
 			return check_condition_result;
 		}
-		if ((sdebug_dif == SD_DIF_TYPE1_PROTECTION ||
-		     sdebug_dif == SD_DIF_TYPE3_PROTECTION) &&
+		if ((sdebug_dif == T10_DIF_TYPE1_PROTECTION ||
+		     sdebug_dif == T10_DIF_TYPE3_PROTECTION) &&
 		    (cmd[1] & 0xe0) == 0)
 			sdev_printk(KERN_ERR, scp->device, "Unprotected RD "
 				    "to DIF device\n");
@@ -2894,13 +2894,13 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		break;
 	}
 	if (unlikely(have_dif_prot && check_prot)) {
-		if (sdebug_dif == SD_DIF_TYPE2_PROTECTION &&
+		if (sdebug_dif == T10_DIF_TYPE2_PROTECTION &&
 		    (cmd[1] & 0xe0)) {
 			mk_sense_invalid_opcode(scp);
 			return check_condition_result;
 		}
-		if ((sdebug_dif == SD_DIF_TYPE1_PROTECTION ||
-		     sdebug_dif == SD_DIF_TYPE3_PROTECTION) &&
+		if ((sdebug_dif == T10_DIF_TYPE1_PROTECTION ||
+		     sdebug_dif == T10_DIF_TYPE3_PROTECTION) &&
 		    (cmd[1] & 0xe0) == 0)
 			sdev_printk(KERN_ERR, scp->device, "Unprotected WR "
 				    "to DIF device\n");
@@ -3136,13 +3136,13 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 	num = cmd[13];		/* 1 to a maximum of 255 logical blocks */
 	if (0 == num)
 		return 0;	/* degenerate case, not an error */
-	if (sdebug_dif == SD_DIF_TYPE2_PROTECTION &&
+	if (sdebug_dif == T10_DIF_TYPE2_PROTECTION &&
 	    (cmd[1] & 0xe0)) {
 		mk_sense_invalid_opcode(scp);
 		return check_condition_result;
 	}
-	if ((sdebug_dif == SD_DIF_TYPE1_PROTECTION ||
-	     sdebug_dif == SD_DIF_TYPE3_PROTECTION) &&
+	if ((sdebug_dif == T10_DIF_TYPE1_PROTECTION ||
+	     sdebug_dif == T10_DIF_TYPE3_PROTECTION) &&
 	    (cmd[1] & 0xe0) == 0)
 		sdev_printk(KERN_ERR, scp->device, "Unprotected WR "
 			    "to DIF device\n");
@@ -4940,12 +4940,11 @@ static int __init scsi_debug_init(void)
 	}
 
 	switch (sdebug_dif) {
-
-	case SD_DIF_TYPE0_PROTECTION:
+	case T10_DIF_TYPE0_PROTECTION:
 		break;
-	case SD_DIF_TYPE1_PROTECTION:
-	case SD_DIF_TYPE2_PROTECTION:
-	case SD_DIF_TYPE3_PROTECTION:
+	case T10_DIF_TYPE1_PROTECTION:
+	case T10_DIF_TYPE2_PROTECTION:
+	case T10_DIF_TYPE3_PROTECTION:
 		have_dif_prot = true;
 		break;
 
@@ -5481,19 +5480,19 @@ static int sdebug_driver_probe(struct device * dev)
 
 	switch (sdebug_dif) {
 
-	case SD_DIF_TYPE1_PROTECTION:
+	case T10_DIF_TYPE1_PROTECTION:
 		hprot = SHOST_DIF_TYPE1_PROTECTION;
 		if (sdebug_dix)
 			hprot |= SHOST_DIX_TYPE1_PROTECTION;
 		break;
 
-	case SD_DIF_TYPE2_PROTECTION:
+	case T10_DIF_TYPE2_PROTECTION:
 		hprot = SHOST_DIF_TYPE2_PROTECTION;
 		if (sdebug_dix)
 			hprot |= SHOST_DIX_TYPE2_PROTECTION;
 		break;
 
-	case SD_DIF_TYPE3_PROTECTION:
+	case T10_DIF_TYPE3_PROTECTION:
 		hprot = SHOST_DIF_TYPE3_PROTECTION;
 		if (sdebug_dix)
 			hprot |= SHOST_DIX_TYPE3_PROTECTION;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index d3e852a..bc58f4f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -52,6 +52,7 @@
 #include <linux/slab.h>
 #include <linux/pm_runtime.h>
 #include <linux/pr.h>
+#include <linux/t10-pi.h>
 #include <asm/uaccess.h>
 #include <asm/unaligned.h>
 
@@ -314,7 +315,7 @@ protection_type_store(struct device *dev, struct device_attribute *attr,
 	if (err)
 		return err;
 
-	if (val >= 0 && val <= SD_DIF_TYPE3_PROTECTION)
+	if (val >= 0 && val <= T10_DIF_TYPE3_PROTECTION)
 		sdkp->protection_type = val;
 
 	return count;
@@ -332,7 +333,7 @@ protection_mode_show(struct device *dev, struct device_attribute *attr,
 	dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
 	dix = scsi_host_dix_capable(sdp->host, sdkp->protection_type);
 
-	if (!dix && scsi_host_dix_capable(sdp->host, SD_DIF_TYPE0_PROTECTION)) {
+	if (!dix && scsi_host_dix_capable(sdp->host, T10_DIF_TYPE0_PROTECTION)) {
 		dif = 0;
 		dix = 1;
 	}
@@ -608,7 +609,7 @@ static unsigned char sd_setup_protect_cmnd(struct scsi_cmnd *scmd,
 			scmd->prot_flags |= SCSI_PROT_GUARD_CHECK;
 	}
 
-	if (dif != SD_DIF_TYPE3_PROTECTION) {	/* DIX/DIF Type 0, 1, 2 */
+	if (dif != T10_DIF_TYPE3_PROTECTION) {	/* DIX/DIF Type 0, 1, 2 */
 		scmd->prot_flags |= SCSI_PROT_REF_INCREMENT;
 
 		if (bio_integrity_flagged(bio, BIP_CTRL_NOCHECK) == false)
@@ -1031,7 +1032,7 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 	else
 		protect = 0;
 
-	if (protect && sdkp->protection_type == SD_DIF_TYPE2_PROTECTION) {
+	if (protect && sdkp->protection_type == T10_DIF_TYPE2_PROTECTION) {
 		SCpnt->cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
 
 		if (unlikely(SCpnt->cmnd == NULL)) {
@@ -1997,7 +1998,7 @@ static int sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer
 
 	type = ((buffer[12] >> 1) & 7) + 1; /* P_TYPE 0 = Type 1 */
 
-	if (type > SD_DIF_TYPE3_PROTECTION)
+	if (type > T10_DIF_TYPE3_PROTECTION)
 		ret = -ENODEV;
 	else if (scsi_host_dif_capable(sdp->host, type))
 		ret = 1;
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index d00966d..c8d9863 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -157,27 +157,6 @@ static inline unsigned int logical_to_bytes(struct scsi_device *sdev, sector_t b
 }
 
 /*
- * A DIF-capable target device can be formatted with different
- * protection schemes.  Currently 0 through 3 are defined:
- *
- * Type 0 is regular (unprotected) I/O
- *
- * Type 1 defines the contents of the guard and reference tags
- *
- * Type 2 defines the contents of the guard and reference tags and
- * uses 32-byte commands to seed the latter
- *
- * Type 3 defines the contents of the guard tag only
- */
-
-enum sd_dif_target_protection_types {
-	SD_DIF_TYPE0_PROTECTION = 0x0,
-	SD_DIF_TYPE1_PROTECTION = 0x1,
-	SD_DIF_TYPE2_PROTECTION = 0x2,
-	SD_DIF_TYPE3_PROTECTION = 0x3,
-};
-
-/*
  * Look up the DIX operation based on whether the command is read or
  * write and whether dix and dif are enabled.
  */
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 987bf39..d3f8fc9 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -60,14 +60,14 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
 
 	/* Enable DMA of protection information */
 	if (scsi_host_get_guard(sdkp->device->host) & SHOST_DIX_GUARD_IP) {
-		if (type == SD_DIF_TYPE3_PROTECTION)
+		if (type == T10_DIF_TYPE3_PROTECTION)
 			bi.profile = &t10_pi_type3_ip;
 		else
 			bi.profile = &t10_pi_type1_ip;
 
 		bi.flags |= BLK_INTEGRITY_IP_CHECKSUM;
 	} else
-		if (type == SD_DIF_TYPE3_PROTECTION)
+		if (type == T10_DIF_TYPE3_PROTECTION)
 			bi.profile = &t10_pi_type3_crc;
 		else
 			bi.profile = &t10_pi_type1_crc;
@@ -82,7 +82,7 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
 		if (!sdkp->ATO)
 			goto out;
 
-		if (type == SD_DIF_TYPE3_PROTECTION)
+		if (type == T10_DIF_TYPE3_PROTECTION)
 			bi.tag_size = sizeof(u16) + sizeof(u32);
 		else
 			bi.tag_size = sizeof(u16);
@@ -121,7 +121,7 @@ void sd_dif_prepare(struct scsi_cmnd *scmd)
 
 	sdkp = scsi_disk(scmd->request->rq_disk);
 
-	if (sdkp->protection_type == SD_DIF_TYPE3_PROTECTION)
+	if (sdkp->protection_type == T10_DIF_TYPE3_PROTECTION)
 		return;
 
 	phys = scsi_prot_ref_tag(scmd);
@@ -172,7 +172,7 @@ void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 
 	sdkp = scsi_disk(scmd->request->rq_disk);
 
-	if (sdkp->protection_type == SD_DIF_TYPE3_PROTECTION || good_bytes == 0)
+	if (sdkp->protection_type == T10_DIF_TYPE3_PROTECTION || good_bytes == 0)
 		return;
 
 	intervals = good_bytes / scsi_prot_interval(scmd);
diff --git a/include/linux/t10-pi.h b/include/linux/t10-pi.h
index dd8de82..f46bedc 100644
--- a/include/linux/t10-pi.h
+++ b/include/linux/t10-pi.h
@@ -5,6 +5,26 @@
 #include <linux/blkdev.h>
 
 /*
+ * A DIF-capable target device can be formatted with different
+ * protection schemes.  Currently 0 through 3 are defined:
+ *
+ * Type 0 is regular (unprotected) I/O
+ *
+ * Type 1 defines the contents of the guard and reference tags
+ *
+ * Type 2 defines the contents of the guard and reference tags and
+ * uses 32-byte commands to seed the latter
+ *
+ * Type 3 defines the contents of the guard tag only
+ */
+enum t10_dif_type {
+	T10_DIF_TYPE0_PROTECTION = 0x0,
+	T10_DIF_TYPE1_PROTECTION = 0x1,
+	T10_DIF_TYPE2_PROTECTION = 0x2,
+	T10_DIF_TYPE3_PROTECTION = 0x3,
+};
+
+/*
  * T10 Protection Information tuple.
  */
 struct t10_pi_tuple {
-- 
2.1.4

Re: [PATCH 3/3] sd: move DIF protection types to t10-pi.h

[Bart Van Assche]

On 09/11/2016 07:35 PM, Christoph Hellwig wrote:
> These should go together with the rest of the T10 protection
> information defintions.

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>

Re: [PATCH 3/3] sd: move DIF protection types to t10-pi.h

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> These should go together with the rest of the T10 protection
Christoph> information defintions.

Series looks good in general. However, now that you introduced t10-pi.h
it seems a bit odd to perpetuate the obsolete "DIF" terminology.

+enum t10_dif_type {
+	T10_DIF_TYPE0_PROTECTION = 0x0,
+	T10_DIF_TYPE1_PROTECTION = 0x1,
+	T10_DIF_TYPE2_PROTECTION = 0x2,
+	T10_DIF_TYPE3_PROTECTION = 0x3,
+};

How about?

enum t10_pi_type {
	T10_PI_TYPE0_PROTECTION = 0x0,
	T10_PI_TYPE1_PROTECTION = 0x1,
	T10_PI_TYPE2_PROTECTION = 0x2,
	T10_PI_TYPE3_PROTECTION = 0x3,
};

I don't mind fixing that up when I apply. Any objections?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 3/3] sd: move DIF protection types to t10-pi.h

[Christoph Hellwig]

On Wed, Sep 14, 2016 at 02:34:35PM -0400, Martin K. Petersen wrote:
> I don't mind fixing that up when I apply. Any objections?

Sure, go ahead!

Re: [PATCH 3/3] sd: move DIF protection types to t10-pi.h

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

>> I don't mind fixing that up when I apply. Any objections?

Christoph> Sure, go ahead!

Applied to 4.9/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering