public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0 of 3] DIF fixes
@ 2008-09-19 22:47 Martin K. Petersen
  2008-09-19 22:47 ` [PATCH 1 of 3] sd: Issue correct protection operation Martin K. Petersen
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Martin K. Petersen @ 2008-09-19 22:47 UTC (permalink / raw)
  To: James.Bottomley, linux-scsi


This series contains fixes for issuing DIF/DIX commands.

-- 
Martin K. Petersen	Oracle Linux Engineering



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1 of 3] sd: Issue correct protection operation
  2008-09-19 22:47 [PATCH 0 of 3] DIF fixes Martin K. Petersen
@ 2008-09-19 22:47 ` Martin K. Petersen
  2008-09-19 22:47 ` [PATCH 2 of 3] sd: Always print actual protection_type Martin K. Petersen
  2008-09-19 22:47 ` [PATCH 3 of 3] sd: Correctly handle all combinations of DIF and DIX Martin K. Petersen
  2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2008-09-19 22:47 UTC (permalink / raw)
  To: James.Bottomley, linux-scsi

Use the same logic to prepare RD/WRPROTECT and the protection
operation.  Fixes a corner case where we could issue an unprotected
CDB and yet tell the HBA to do DIF to the drive.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

---
1 file changed, 4 insertions(+), 3 deletions(-)
drivers/scsi/sd.c |    7 ++++---



diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -378,7 +378,7 @@ static int sd_prep_fn(struct request_que
 	sector_t threshold;
 	unsigned int this_count = rq->nr_sectors;
 	unsigned int timeout = sdp->timeout;
-	int ret;
+	int ret, host_dif;
 
 	if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
 		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
@@ -509,7 +509,8 @@ static int sd_prep_fn(struct request_que
 					rq->nr_sectors));
 
 	/* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
-	if (scsi_host_dif_capable(sdp->host, sdkp->protection_type))
+	host_dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
+	if (host_dif)
 		SCpnt->cmnd[1] = 1 << 5;
 	else
 		SCpnt->cmnd[1] = 0;
@@ -567,7 +568,7 @@ static int sd_prep_fn(struct request_que
 	SCpnt->sdb.length = this_count * sdp->sector_size;
 
 	/* If DIF or DIX is enabled, tell HBA how to handle request */
-	if (sdkp->protection_type || scsi_prot_sg_count(SCpnt))
+	if (host_dif || scsi_prot_sg_count(SCpnt))
 		sd_dif_op(SCpnt, sdkp->protection_type, scsi_prot_sg_count(SCpnt));
 
 	/*



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2 of 3] sd: Always print actual protection_type
  2008-09-19 22:47 [PATCH 0 of 3] DIF fixes Martin K. Petersen
  2008-09-19 22:47 ` [PATCH 1 of 3] sd: Issue correct protection operation Martin K. Petersen
@ 2008-09-19 22:47 ` Martin K. Petersen
  2008-09-19 22:47 ` [PATCH 3 of 3] sd: Correctly handle all combinations of DIF and DIX Martin K. Petersen
  2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2008-09-19 22:47 UTC (permalink / raw)
  To: James.Bottomley, linux-scsi

Now that we no longer use protection_type as trigger for preparing
protected CDBs we can remove the places that set it to zero.  This
allows userland to see which protection type the device is formatted
with regardless of whether the HBA supports DIF or not.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

---
2 files changed, 2 insertions(+), 6 deletions(-)
drivers/scsi/sd.c     |    7 ++-----
drivers/scsi/sd_dif.c |    1 -



diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1248,14 +1248,12 @@ void sd_read_protection_type(struct scsi
 	else
 		type = ((buffer[12] >> 1) & 7) + 1; /* P_TYPE 0 = Type 1 */
 
+	sdkp->protection_type = type;
+
 	switch (type) {
 	case SD_DIF_TYPE0_PROTECTION:
-		sdkp->protection_type = 0;
-		break;
-
 	case SD_DIF_TYPE1_PROTECTION:
 	case SD_DIF_TYPE3_PROTECTION:
-		sdkp->protection_type = type;
 		break;
 
 	case SD_DIF_TYPE2_PROTECTION:
@@ -1273,7 +1271,6 @@ void sd_read_protection_type(struct scsi
 	return;
 
 disable:
-	sdkp->protection_type = 0;
 	sdkp->capacity = 0;
 }
 
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -321,7 +321,6 @@ void sd_dif_config_host(struct scsi_disk
 		if (scsi_host_dif_capable(sdp->host, type) == 0) {
 			sd_printk(KERN_INFO, sdkp, "Type %d protection " \
 				  "unsupported by HBA. Disabling DIF.\n", type);
-			sdkp->protection_type = 0;
 			return;
 		}
 



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 3 of 3] sd: Correctly handle all combinations of DIF and DIX
  2008-09-19 22:47 [PATCH 0 of 3] DIF fixes Martin K. Petersen
  2008-09-19 22:47 ` [PATCH 1 of 3] sd: Issue correct protection operation Martin K. Petersen
  2008-09-19 22:47 ` [PATCH 2 of 3] sd: Always print actual protection_type Martin K. Petersen
@ 2008-09-19 22:47 ` Martin K. Petersen
  2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2008-09-19 22:47 UTC (permalink / raw)
  To: James.Bottomley, linux-scsi

The old detection code couldn't handle all possible combinations of
DIX and DIF.  This version does, giving priority to DIX if the
controller is capable.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

---
3 files changed, 23 insertions(+), 19 deletions(-)
drivers/scsi/sd.c     |    3 ++-
drivers/scsi/sd.h     |    2 +-
drivers/scsi/sd_dif.c |   37 ++++++++++++++++++++-----------------



diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -569,7 +569,8 @@ static int sd_prep_fn(struct request_que
 
 	/* If DIF or DIX is enabled, tell HBA how to handle request */
 	if (host_dif || scsi_prot_sg_count(SCpnt))
-		sd_dif_op(SCpnt, sdkp->protection_type, scsi_prot_sg_count(SCpnt));
+		sd_dif_op(SCpnt, host_dif, scsi_prot_sg_count(SCpnt),
+			  sdkp->protection_type);
 
 	/*
 	 * We shouldn't disconnect in the middle of a sector, so with a dumb
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -99,7 +99,7 @@ struct sd_dif_tuple {
 
 #if defined(CONFIG_BLK_DEV_INTEGRITY)
 
-extern void sd_dif_op(struct scsi_cmnd *, unsigned int, unsigned int);
+extern void sd_dif_op(struct scsi_cmnd *, unsigned int, unsigned int, unsigned int);
 extern void sd_dif_config_host(struct scsi_disk *);
 extern int sd_dif_prepare(struct request *rq, sector_t, unsigned int);
 extern void sd_dif_complete(struct scsi_cmnd *, unsigned int);
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -311,24 +311,26 @@ void sd_dif_config_host(struct scsi_disk
 	struct scsi_device *sdp = sdkp->device;
 	struct gendisk *disk = sdkp->disk;
 	u8 type = sdkp->protection_type;
+	int dif, dix;
 
-	/* If this HBA doesn't support DIX, resort to normal I/O or DIF */
-	if (scsi_host_dix_capable(sdp->host, type) == 0) {
+	dif = scsi_host_dif_capable(sdp->host, type);
+	dix = scsi_host_dix_capable(sdp->host, type);
 
-		if (type == SD_DIF_TYPE0_PROTECTION)
-			return;
+	if (!dix && scsi_host_dix_capable(sdp->host, 0)) {
+		dif = 0; dix = 1;
+	}
 
-		if (scsi_host_dif_capable(sdp->host, type) == 0) {
-			sd_printk(KERN_INFO, sdkp, "Type %d protection " \
-				  "unsupported by HBA. Disabling DIF.\n", type);
-			return;
-		}
+	if (type) {
+		if (dif)
+			sd_printk(KERN_INFO, sdkp,
+				  "Enabling DIF Type %d protection\n", type);
+		else
+			sd_printk(KERN_INFO, sdkp,
+				  "Disabling DIF Type %d protection\n", type);
+	}
 
-		sd_printk(KERN_INFO, sdkp, "Enabling DIF Type %d protection\n",
-			  type);
-
+	if (!dix)
 		return;
-	}
 
 	/* Enable DMA of protection information */
 	if (scsi_host_get_guard(sdkp->device->host) & SHOST_DIX_GUARD_IP)
@@ -343,10 +345,10 @@ void sd_dif_config_host(struct scsi_disk
 			blk_integrity_register(disk, &dif_type1_integrity_crc);
 
 	sd_printk(KERN_INFO, sdkp,
-		  "Enabling %s integrity protection\n", disk->integrity->name);
+		  "Enabling DIX %s protection\n", disk->integrity->name);
 
 	/* Signal to block layer that we support sector tagging */
-	if (type && sdkp->ATO) {
+	if (dif && type && sdkp->ATO) {
 		if (type == SD_DIF_TYPE3_PROTECTION)
 			disk->integrity->tag_size = sizeof(u16) + sizeof(u32);
 		else
@@ -360,7 +362,7 @@ void sd_dif_config_host(struct scsi_disk
 /*
  * DIF DMA operation magic decoder ring.
  */
-void sd_dif_op(struct scsi_cmnd *scmd, unsigned int dif, unsigned int dix)
+void sd_dif_op(struct scsi_cmnd *scmd, unsigned int dif, unsigned int dix, unsigned int type)
 {
 	int csum_convert, prot_op;
 
@@ -405,7 +407,8 @@ void sd_dif_op(struct scsi_cmnd *scmd, u
 	}
 
 	scsi_set_prot_op(scmd, prot_op);
-	scsi_set_prot_type(scmd, dif);
+	if (dif)
+		scsi_set_prot_type(scmd, type);
 }
 
 /*



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-09-19 22:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-19 22:47 [PATCH 0 of 3] DIF fixes Martin K. Petersen
2008-09-19 22:47 ` [PATCH 1 of 3] sd: Issue correct protection operation Martin K. Petersen
2008-09-19 22:47 ` [PATCH 2 of 3] sd: Always print actual protection_type Martin K. Petersen
2008-09-19 22:47 ` [PATCH 3 of 3] sd: Correctly handle all combinations of DIF and DIX Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox