* DIF/DIX updates for 2.6.32
@ 2009-09-11 19:20 Martin K. Petersen
2009-09-11 19:20 ` [PATCH 1/5] scsi: Deprecate SCSI_PROT_*_CONVERT operations Martin K. Petersen
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Martin K. Petersen @ 2009-09-11 19:20 UTC (permalink / raw)
To: James.Bottomley, linux-scsi
Updates since last post:
- The CDB allocation has been moved to sd.c and the Type 2 support collapsed
into a single patch.
The first three patches constitute fixes. The last two contain new features.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5] scsi: Deprecate SCSI_PROT_*_CONVERT operations
2009-09-11 19:20 DIF/DIX updates for 2.6.32 Martin K. Petersen
@ 2009-09-11 19:20 ` Martin K. Petersen
2009-09-18 23:16 ` James Smart
2009-09-11 19:20 ` [PATCH 2/5] sd: Detach DIF from block integrity infrastructure Martin K. Petersen
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Martin K. Petersen @ 2009-09-11 19:20 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Martin K. Petersen
The checksum format is orthogonal to whether the protection information
is being passed on beyond the HBA or not. It is perfectly valid to use
a non-T10 CRC with WRITE_STRIP and READ_INSERT.
Consequently it no longer makes sense to explicitly refer to the
conversion in the protection operation. Update sd_dif and lpfc
accordingly.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
drivers/scsi/lpfc/lpfc_scsi.c | 15 +++------------
drivers/scsi/sd_dif.c | 20 ++++----------------
include/scsi/scsi_cmnd.h | 4 ----
3 files changed, 7 insertions(+), 32 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index da59c4f..ad75e19 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -56,8 +56,6 @@ static char *dif_op_str[] = {
"SCSI_PROT_WRITE_INSERT",
"SCSI_PROT_READ_PASS",
"SCSI_PROT_WRITE_PASS",
- "SCSI_PROT_READ_CONVERT",
- "SCSI_PROT_WRITE_CONVERT"
};
static void
lpfc_release_scsi_buf_s4(struct lpfc_hba *phba, struct lpfc_scsi_buf *psb);
@@ -1131,13 +1129,11 @@ lpfc_sc_to_sli_prof(struct scsi_cmnd *sc)
ret_prof = LPFC_PROF_A1;
break;
- case SCSI_PROT_READ_CONVERT:
- case SCSI_PROT_WRITE_CONVERT:
+ case SCSI_PROT_READ_PASS:
+ case SCSI_PROT_WRITE_PASS:
ret_prof = LPFC_PROF_AST1;
break;
- case SCSI_PROT_READ_PASS:
- case SCSI_PROT_WRITE_PASS:
case SCSI_PROT_NORMAL:
default:
printk(KERN_ERR "Bad op/guard:%d/%d combination\n",
@@ -1157,8 +1153,6 @@ lpfc_sc_to_sli_prof(struct scsi_cmnd *sc)
ret_prof = LPFC_PROF_C1;
break;
- case SCSI_PROT_READ_CONVERT:
- case SCSI_PROT_WRITE_CONVERT:
case SCSI_PROT_READ_INSERT:
case SCSI_PROT_WRITE_STRIP:
case SCSI_PROT_NORMAL:
@@ -1209,8 +1203,7 @@ lpfc_get_cmd_dif_parms(struct scsi_cmnd *sc, uint16_t *apptagmask,
static int cnt;
if (protcnt && (op == SCSI_PROT_WRITE_STRIP ||
- op == SCSI_PROT_WRITE_PASS ||
- op == SCSI_PROT_WRITE_CONVERT)) {
+ op == SCSI_PROT_WRITE_PASS)) {
cnt++;
spt = page_address(sg_page(scsi_prot_sglist(sc))) +
@@ -1501,8 +1494,6 @@ lpfc_prot_group_type(struct lpfc_hba *phba, struct scsi_cmnd *sc)
case SCSI_PROT_WRITE_STRIP:
case SCSI_PROT_READ_PASS:
case SCSI_PROT_WRITE_PASS:
- case SCSI_PROT_WRITE_CONVERT:
- case SCSI_PROT_READ_CONVERT:
ret = LPFC_PG_TYPE_DIF_BUF;
break;
default:
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 82f14a9..84224dd 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -364,15 +364,9 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
*/
void sd_dif_op(struct scsi_cmnd *scmd, unsigned int dif, unsigned int dix, unsigned int type)
{
- int csum_convert, prot_op;
+ int prot_op;
- prot_op = 0;
-
- /* Convert checksum? */
- if (scsi_host_get_guard(scmd->device->host) != SHOST_DIX_GUARD_CRC)
- csum_convert = 1;
- else
- csum_convert = 0;
+ prot_op = SCSI_PROT_NORMAL;
BUG_ON(dif && (scmd->cmnd[0] == READ_6 || scmd->cmnd[0] == WRITE_6));
@@ -382,10 +376,7 @@ void sd_dif_op(struct scsi_cmnd *scmd, unsigned int dif, unsigned int dix, unsig
case READ_12:
case READ_16:
if (dif && dix)
- if (csum_convert)
- prot_op = SCSI_PROT_READ_CONVERT;
- else
- prot_op = SCSI_PROT_READ_PASS;
+ prot_op = SCSI_PROT_READ_PASS;
else if (dif && !dix)
prot_op = SCSI_PROT_READ_STRIP;
else if (!dif && dix)
@@ -398,10 +389,7 @@ void sd_dif_op(struct scsi_cmnd *scmd, unsigned int dif, unsigned int dix, unsig
case WRITE_12:
case WRITE_16:
if (dif && dix)
- if (csum_convert)
- prot_op = SCSI_PROT_WRITE_CONVERT;
- else
- prot_op = SCSI_PROT_WRITE_PASS;
+ prot_op = SCSI_PROT_WRITE_PASS;
else if (dif && !dix)
prot_op = SCSI_PROT_WRITE_INSERT;
else if (!dif && dix)
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 3878d1d..a5e885a 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -229,10 +229,6 @@ enum scsi_prot_operations {
/* OS-HBA: Protected, HBA-Target: Protected */
SCSI_PROT_READ_PASS,
SCSI_PROT_WRITE_PASS,
-
- /* OS-HBA: Protected, HBA-Target: Protected, checksum conversion */
- SCSI_PROT_READ_CONVERT,
- SCSI_PROT_WRITE_CONVERT,
};
static inline void scsi_set_prot_op(struct scsi_cmnd *scmd, unsigned char op)
--
1.6.0.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/5] sd: Detach DIF from block integrity infrastructure
2009-09-11 19:20 DIF/DIX updates for 2.6.32 Martin K. Petersen
2009-09-11 19:20 ` [PATCH 1/5] scsi: Deprecate SCSI_PROT_*_CONVERT operations Martin K. Petersen
@ 2009-09-11 19:20 ` Martin K. Petersen
2009-09-11 19:20 ` [PATCH 3/5] scsi: Fix protection scsi_data_buffer leak Martin K. Petersen
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Martin K. Petersen @ 2009-09-11 19:20 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Martin K. Petersen
So far we have only issued DIF commands if CONFIG_BLK_DEV_INTEGRITY is
enabled. However, communication between initiator and target should be
independent of protection information DMA. There are DIF-only host
adapters coming out that will be able to take advantage of this.
Move the relevant DIF bits to sd.c.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
drivers/scsi/sd.c | 61 +++++++++++++++++++++++++++++++--------------
drivers/scsi/sd.h | 4 ---
drivers/scsi/sd_dif.c | 53 ----------------------------------------
include/scsi/scsi_host.h | 15 ++++++++---
4 files changed, 53 insertions(+), 80 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b7b9fec..36e2333 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -370,6 +370,31 @@ static void scsi_disk_put(struct scsi_disk *sdkp)
mutex_unlock(&sd_ref_mutex);
}
+static void sd_prot_op(struct scsi_cmnd *scmd, unsigned int dif)
+{
+ unsigned int prot_op = SCSI_PROT_NORMAL;
+ unsigned int dix = scsi_prot_sg_count(scmd);
+
+ if (scmd->sc_data_direction == DMA_FROM_DEVICE) {
+ if (dif && dix)
+ prot_op = SCSI_PROT_READ_PASS;
+ else if (dif && !dix)
+ prot_op = SCSI_PROT_READ_STRIP;
+ else if (!dif && dix)
+ prot_op = SCSI_PROT_READ_INSERT;
+ } else {
+ if (dif && dix)
+ prot_op = SCSI_PROT_WRITE_PASS;
+ else if (dif && !dix)
+ prot_op = SCSI_PROT_WRITE_INSERT;
+ else if (!dif && dix)
+ prot_op = SCSI_PROT_WRITE_STRIP;
+ }
+
+ scsi_set_prot_op(scmd, prot_op);
+ scsi_set_prot_type(scmd, dif);
+}
+
/**
* sd_init_command - build a scsi (read or write) command from
* information in the request structure.
@@ -578,8 +603,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
/* If DIF or DIX is enabled, tell HBA how to handle request */
if (host_dif || scsi_prot_sg_count(SCpnt))
- sd_dif_op(SCpnt, host_dif, scsi_prot_sg_count(SCpnt),
- sdkp->protection_type);
+ sd_prot_op(SCpnt, host_dif);
/*
* We shouldn't disconnect in the middle of a sector, so with a dumb
@@ -1238,34 +1262,33 @@ void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
u8 type;
if (scsi_device_protection(sdp) == 0 || (buffer[12] & 1) == 0)
- type = 0;
- else
- type = ((buffer[12] >> 1) & 7) + 1; /* P_TYPE 0 = Type 1 */
+ return;
+
+ type = ((buffer[12] >> 1) & 7) + 1; /* P_TYPE 0 = Type 1 */
+
+ if (type == sdkp->protection_type || !sdkp->first_scan)
+ return;
sdkp->protection_type = type;
switch (type) {
- case SD_DIF_TYPE0_PROTECTION:
case SD_DIF_TYPE1_PROTECTION:
case SD_DIF_TYPE3_PROTECTION:
break;
- case SD_DIF_TYPE2_PROTECTION:
- sd_printk(KERN_ERR, sdkp, "formatted with DIF Type 2 " \
- "protection which is currently unsupported. " \
- "Disabling disk!\n");
- goto disable;
-
default:
- sd_printk(KERN_ERR, sdkp, "formatted with unknown " \
- "protection type %d. Disabling disk!\n", type);
- goto disable;
+ sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
+ "protection type %u. Disabling disk!\n", type);
+ sdkp->capacity = 0;
+ return;
}
- return;
-
-disable:
- sdkp->capacity = 0;
+ if (scsi_host_dif_capable(sdp->host, type))
+ sd_printk(KERN_NOTICE, sdkp,
+ "Enabling DIF Type %u protection\n", type);
+ else
+ sd_printk(KERN_NOTICE, sdkp,
+ "Disabling DIF Type %u protection\n", type);
}
static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp,
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 8474b5b..ce1f5f8 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -101,16 +101,12 @@ struct sd_dif_tuple {
#ifdef CONFIG_BLK_DEV_INTEGRITY
-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);
#else /* CONFIG_BLK_DEV_INTEGRITY */
-static inline void sd_dif_op(struct scsi_cmnd *cmd, unsigned int a, unsigned int b, unsigned int c)
-{
-}
static inline void sd_dif_config_host(struct scsi_disk *disk)
{
}
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 84224dd..88da977 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -320,15 +320,6 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
dif = 0; dix = 1;
}
- if (type) {
- if (dif)
- sd_printk(KERN_NOTICE, sdkp,
- "Enabling DIF Type %d protection\n", type);
- else
- sd_printk(KERN_NOTICE, sdkp,
- "Disabling DIF Type %d protection\n", type);
- }
-
if (!dix)
return;
@@ -360,50 +351,6 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
}
/*
- * DIF DMA operation magic decoder ring.
- */
-void sd_dif_op(struct scsi_cmnd *scmd, unsigned int dif, unsigned int dix, unsigned int type)
-{
- int prot_op;
-
- prot_op = SCSI_PROT_NORMAL;
-
- BUG_ON(dif && (scmd->cmnd[0] == READ_6 || scmd->cmnd[0] == WRITE_6));
-
- switch (scmd->cmnd[0]) {
- case READ_6:
- case READ_10:
- case READ_12:
- case READ_16:
- if (dif && dix)
- prot_op = SCSI_PROT_READ_PASS;
- else if (dif && !dix)
- prot_op = SCSI_PROT_READ_STRIP;
- else if (!dif && dix)
- prot_op = SCSI_PROT_READ_INSERT;
-
- break;
-
- case WRITE_6:
- case WRITE_10:
- case WRITE_12:
- case WRITE_16:
- if (dif && dix)
- prot_op = SCSI_PROT_WRITE_PASS;
- else if (dif && !dix)
- prot_op = SCSI_PROT_WRITE_INSERT;
- else if (!dif && dix)
- prot_op = SCSI_PROT_WRITE_STRIP;
-
- break;
- }
-
- scsi_set_prot_op(scmd, prot_op);
- if (dif)
- scsi_set_prot_type(scmd, type);
-}
-
-/*
* The virtual start sector is the one that was originally submitted
* by the block layer. Due to partitioning, MD/DM cloning, etc. the
* actual physical start sector is likely to be different. Remap
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index b62a097..6e728b1 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -798,9 +798,15 @@ static inline unsigned int scsi_host_get_prot(struct Scsi_Host *shost)
static inline unsigned int scsi_host_dif_capable(struct Scsi_Host *shost, unsigned int target_type)
{
switch (target_type) {
- case 1: return shost->prot_capabilities & SHOST_DIF_TYPE1_PROTECTION;
- case 2: return shost->prot_capabilities & SHOST_DIF_TYPE2_PROTECTION;
- case 3: return shost->prot_capabilities & SHOST_DIF_TYPE3_PROTECTION;
+ case 1:
+ if (shost->prot_capabilities & SHOST_DIF_TYPE1_PROTECTION)
+ return target_type;
+ case 2:
+ if (shost->prot_capabilities & SHOST_DIF_TYPE2_PROTECTION)
+ return target_type;
+ case 3:
+ if (shost->prot_capabilities & SHOST_DIF_TYPE3_PROTECTION)
+ return target_type;
}
return 0;
@@ -808,13 +814,14 @@ static inline unsigned int scsi_host_dif_capable(struct Scsi_Host *shost, unsign
static inline unsigned int scsi_host_dix_capable(struct Scsi_Host *shost, unsigned int target_type)
{
+#if defined(CONFIG_BLK_DEV_INTEGRITY)
switch (target_type) {
case 0: return shost->prot_capabilities & SHOST_DIX_TYPE0_PROTECTION;
case 1: return shost->prot_capabilities & SHOST_DIX_TYPE1_PROTECTION;
case 2: return shost->prot_capabilities & SHOST_DIX_TYPE2_PROTECTION;
case 3: return shost->prot_capabilities & SHOST_DIX_TYPE3_PROTECTION;
}
-
+#endif
return 0;
}
--
1.6.0.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/5] scsi: Fix protection scsi_data_buffer leak
2009-09-11 19:20 DIF/DIX updates for 2.6.32 Martin K. Petersen
2009-09-11 19:20 ` [PATCH 1/5] scsi: Deprecate SCSI_PROT_*_CONVERT operations Martin K. Petersen
2009-09-11 19:20 ` [PATCH 2/5] sd: Detach DIF from block integrity infrastructure Martin K. Petersen
@ 2009-09-11 19:20 ` Martin K. Petersen
2009-09-13 9:36 ` Boaz Harrosh
2009-09-11 19:20 ` [PATCH 4/5] sd: Support disks formatted with DIF Type 2 Martin K. Petersen
2009-09-11 19:20 ` [PATCH 5/5] scsi_debug: Implement support for " Martin K. Petersen
4 siblings, 1 reply; 17+ messages in thread
From: Martin K. Petersen @ 2009-09-11 19:20 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Martin K. Petersen
We would leak a scsi_data_buffer if the free_list command was of the
protected variety.
Reported-by: Boaz Harrosh <bharrosh@panasas.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
drivers/scsi/scsi.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 2de5f3a..69397bb 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -241,10 +241,7 @@ scsi_host_alloc_command(struct Scsi_Host *shost, gfp_t gfp_mask)
*/
struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask)
{
- struct scsi_cmnd *cmd;
- unsigned char *buf;
-
- cmd = scsi_host_alloc_command(shost, gfp_mask);
+ struct scsi_cmnd *cmd = scsi_host_alloc_command(shost, gfp_mask);
if (unlikely(!cmd)) {
unsigned long flags;
@@ -258,9 +255,15 @@ struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask)
spin_unlock_irqrestore(&shost->free_list_lock, flags);
if (cmd) {
+ void *buf, *prot;
+
buf = cmd->sense_buffer;
+ prot = cmd->prot_sdb;
+
memset(cmd, 0, sizeof(*cmd));
+
cmd->sense_buffer = buf;
+ cmd->prot_sdb = prot;
}
}
--
1.6.0.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5] sd: Support disks formatted with DIF Type 2
2009-09-11 19:20 DIF/DIX updates for 2.6.32 Martin K. Petersen
` (2 preceding siblings ...)
2009-09-11 19:20 ` [PATCH 3/5] scsi: Fix protection scsi_data_buffer leak Martin K. Petersen
@ 2009-09-11 19:20 ` Martin K. Petersen
2009-09-13 9:37 ` Boaz Harrosh
2009-09-11 19:20 ` [PATCH 5/5] scsi_debug: Implement support for " Martin K. Petersen
4 siblings, 1 reply; 17+ messages in thread
From: Martin K. Petersen @ 2009-09-11 19:20 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Martin K. Petersen
Disks formatted with DIF Type 2 reject READ/WRITE 6/10/12/16 commands
when protection is enabled. Only the 32-byte variants are supported.
Implement support for issusing 32-byte READ/WRITE and enable Type 2
drives in the protection type detection logic.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
drivers/scsi/sd.c | 80 ++++++++++++++++++++++++++++++++++++++++++++-------
drivers/scsi/sd.h | 5 +++
include/scsi/scsi.h | 3 ++
3 files changed, 77 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 36e2333..d138e6b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -116,6 +116,9 @@ static DEFINE_IDA(sd_index_ida);
* object after last put) */
static DEFINE_MUTEX(sd_ref_mutex);
+struct kmem_cache *sd_cdb_cache;
+mempool_t *sd_cdb_pool;
+
static const char *sd_cache_types[] = {
"write through", "none", "write back",
"write back, no read (daft)"
@@ -413,6 +416,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
sector_t threshold;
unsigned int this_count = blk_rq_sectors(rq);
int ret, host_dif;
+ unsigned char protect;
if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
ret = scsi_setup_blk_pc_cmnd(sdp, rq);
@@ -545,13 +549,48 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
/* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
host_dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
if (host_dif)
- SCpnt->cmnd[1] = 1 << 5;
+ protect = 1 << 5;
else
- SCpnt->cmnd[1] = 0;
+ protect = 0;
+
+ if (host_dif == SD_DIF_TYPE2_PROTECTION) {
+ SCpnt->cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
+
+ if (unlikely(SCpnt->cmnd == NULL)) {
+ ret = BLKPREP_DEFER;
+ goto out;
+ }
- if (block > 0xffffffff) {
+ memset(SCpnt->cmnd, 0, SD_EXT_CDB_SIZE);
+ SCpnt->cmnd[0] = VARIABLE_LENGTH_CMD;
+ SCpnt->cmnd[7] = 0x18;
+ SCpnt->cmnd[9] = (rq_data_dir(rq) == READ) ? READ_32 : WRITE_32;
+ SCpnt->cmnd[10] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
+
+ /* LBA */
+ SCpnt->cmnd[12] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
+ SCpnt->cmnd[13] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
+ SCpnt->cmnd[14] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
+ SCpnt->cmnd[15] = sizeof(block) > 4 ? (unsigned char) (block >> 32) & 0xff : 0;
+ SCpnt->cmnd[16] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[17] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[18] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[19] = (unsigned char) block & 0xff;
+
+ /* Expected Indirect LBA */
+ SCpnt->cmnd[20] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[21] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[22] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[23] = (unsigned char) block & 0xff;
+
+ /* Transfer length */
+ SCpnt->cmnd[28] = (unsigned char) (this_count >> 24) & 0xff;
+ SCpnt->cmnd[29] = (unsigned char) (this_count >> 16) & 0xff;
+ SCpnt->cmnd[30] = (unsigned char) (this_count >> 8) & 0xff;
+ SCpnt->cmnd[31] = (unsigned char) this_count & 0xff;
+ } else if (block > 0xffffffff) {
SCpnt->cmnd[0] += READ_16 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
SCpnt->cmnd[3] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
SCpnt->cmnd[4] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
@@ -572,7 +611,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
this_count = 0xffff;
SCpnt->cmnd[0] += READ_10 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = (unsigned char) (block >> 24) & 0xff;
SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff;
SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;
@@ -1047,6 +1086,7 @@ static int sd_done(struct scsi_cmnd *SCpnt)
int result = SCpnt->result;
unsigned int good_bytes = result ? 0 : scsi_bufflen(SCpnt);
struct scsi_sense_hdr sshdr;
+ struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk);
int sense_valid = 0;
int sense_deferred = 0;
@@ -1108,6 +1148,10 @@ static int sd_done(struct scsi_cmnd *SCpnt)
if (rq_data_dir(SCpnt->request) == READ && scsi_prot_sg_count(SCpnt))
sd_dif_complete(SCpnt, good_bytes);
+ if (scsi_host_dif_capable(sdkp->device->host, sdkp->protection_type)
+ == SD_DIF_TYPE2_PROTECTION)
+ mempool_free(SCpnt->cmnd, sd_cdb_pool);
+
return good_bytes;
}
@@ -1271,12 +1315,7 @@ void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
sdkp->protection_type = type;
- switch (type) {
- case SD_DIF_TYPE1_PROTECTION:
- case SD_DIF_TYPE3_PROTECTION:
- break;
-
- default:
+ if (type > SD_DIF_TYPE3_PROTECTION) {
sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
"protection type %u. Disabling disk!\n", type);
sdkp->capacity = 0;
@@ -2321,8 +2360,24 @@ static int __init init_sd(void)
if (err)
goto err_out_class;
+ sd_cdb_cache = kmem_cache_create("sd_ext_cdb", SD_EXT_CDB_SIZE,
+ 0, 0, NULL);
+ if (!sd_cdb_cache) {
+ printk(KERN_ERR "sd: can't init extended cdb cache\n");
+ goto err_out_class;
+ }
+
+ sd_cdb_pool = mempool_create_slab_pool(SD_MEMPOOL_SIZE, sd_cdb_cache);
+ if (!sd_cdb_pool) {
+ printk(KERN_ERR "sd: can't init extended cdb pool\n");
+ goto err_out_cache;
+ }
+
return 0;
+err_out_cache:
+ kmem_cache_destroy(sd_cdb_cache);
+
err_out_class:
class_unregister(&sd_disk_class);
err_out:
@@ -2342,6 +2397,9 @@ static void __exit exit_sd(void)
SCSI_LOG_HLQUEUE(3, printk("exit_sd: exiting sd driver\n"));
+ mempool_destroy(sd_cdb_pool);
+ kmem_cache_destroy(sd_cdb_cache);
+
scsi_unregister_driver(&sd_template.gendrv);
class_unregister(&sd_disk_class);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index ce1f5f8..e374804 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -37,6 +37,11 @@
*/
#define SD_LAST_BUGGY_SECTORS 8
+enum {
+ SD_EXT_CDB_SIZE = 32, /* Extended CDB size */
+ SD_MEMPOOL_SIZE = 2, /* CDB pool size */
+};
+
struct scsi_disk {
struct scsi_driver *driver; /* always &sd_template */
struct scsi_device *device;
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 084478e..34c46ab 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -129,6 +129,9 @@ struct scsi_cmnd;
#define MI_REPORT_TARGET_PGS 0x0a
/* values for maintenance out */
#define MO_SET_TARGET_PGS 0x0a
+/* values for variable length command */
+#define READ_32 0x09
+#define WRITE_32 0x0b
/* Values for T10/04-262r7 */
#define ATA_16 0x85 /* 16-byte pass-thru */
--
1.6.0.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/5] scsi_debug: Implement support for DIF Type 2
2009-09-11 19:20 DIF/DIX updates for 2.6.32 Martin K. Petersen
` (3 preceding siblings ...)
2009-09-11 19:20 ` [PATCH 4/5] sd: Support disks formatted with DIF Type 2 Martin K. Petersen
@ 2009-09-11 19:20 ` Martin K. Petersen
2009-09-11 23:06 ` Douglas Gilbert
4 siblings, 1 reply; 17+ messages in thread
From: Martin K. Petersen @ 2009-09-11 19:20 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Martin K. Petersen
Add support for 32-byte READ/WRITE as well as DIF Type 2 protection.
Reject protected 10/12/16 byte READ/WRITE commands when Type 2 is
enabled.
Verify Type 2 reference tag according to Expected Initial LBA in 32-byte
CDB.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
drivers/scsi/scsi_debug.c | 135 +++++++++++++++++++++++++++++++++++++--------
1 files changed, 112 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index fb9af20..f551ec3 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -50,6 +50,7 @@
#include <scsi/scsi_host.h>
#include <scsi/scsicam.h>
#include <scsi/scsi_eh.h>
+#include <scsi/scsi_dbg.h>
#include "sd.h"
#include "scsi_logging.h"
@@ -64,6 +65,7 @@ static const char * scsi_debug_version_date = "20070104";
#define PARAMETER_LIST_LENGTH_ERR 0x1a
#define INVALID_OPCODE 0x20
#define ADDR_OUT_OF_RANGE 0x21
+#define INVALID_COMMAND_OPCODE 0x20
#define INVALID_FIELD_IN_CDB 0x24
#define INVALID_FIELD_IN_PARAM_LIST 0x26
#define POWERON_RESET 0x29
@@ -180,7 +182,7 @@ static int sdebug_sectors_per; /* sectors per cylinder */
#define SDEBUG_SENSE_LEN 32
#define SCSI_DEBUG_CANQUEUE 255
-#define SCSI_DEBUG_MAX_CMD_LEN 16
+#define SCSI_DEBUG_MAX_CMD_LEN 32
struct sdebug_dev_info {
struct list_head dev_list;
@@ -296,9 +298,25 @@ static void mk_sense_buffer(struct sdebug_dev_info *devip, int key,
}
static void get_data_transfer_info(unsigned char *cmd,
- unsigned long long *lba, unsigned int *num)
+ unsigned long long *lba, unsigned int *num,
+ u32 *ei_lba)
{
+ *ei_lba = 0;
+
switch (*cmd) {
+ case VARIABLE_LENGTH_CMD:
+ *lba = (u64)cmd[19] | (u64)cmd[18] << 8 |
+ (u64)cmd[17] << 16 | (u64)cmd[16] << 24 |
+ (u64)cmd[15] << 32 | (u64)cmd[14] << 40 |
+ (u64)cmd[13] << 48 | (u64)cmd[12] << 56;
+
+ *ei_lba = (u32)cmd[23] | (u32)cmd[22] << 8 |
+ (u32)cmd[21] << 16 | (u32)cmd[20] << 24;
+
+ *num = (u32)cmd[31] | (u32)cmd[30] << 8 | (u32)cmd[29] << 16 |
+ (u32)cmd[28] << 24;
+ break;
+
case WRITE_16:
case READ_16:
*lba = (u64)cmd[9] | (u64)cmd[8] << 8 |
@@ -1589,7 +1607,7 @@ static int do_device_access(struct scsi_cmnd *scmd,
}
static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
- unsigned int sectors)
+ unsigned int sectors, u32 ei_lba)
{
unsigned int i, resid;
struct scatterlist *psgl;
@@ -1636,13 +1654,23 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
return 0x01;
}
- if (scsi_debug_dif != SD_DIF_TYPE3_PROTECTION &&
+ if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION &&
be32_to_cpu(sdt[i].ref_tag) != (sector & 0xffffffff)) {
printk(KERN_ERR "%s: REF check failed on sector %lu\n",
__func__, (unsigned long)sector);
dif_errors++;
return 0x03;
}
+
+ if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION &&
+ be32_to_cpu(sdt[i].ref_tag) != ei_lba) {
+ printk(KERN_ERR "%s: REF check failed on sector %lu\n",
+ __func__, (unsigned long)sector);
+ dif_errors++;
+ return 0x03;
+ }
+
+ ei_lba++;
}
resid = sectors * 8; /* Bytes of protection data to copy into sgl */
@@ -1670,7 +1698,8 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
}
static int resp_read(struct scsi_cmnd *SCpnt, unsigned long long lba,
- unsigned int num, struct sdebug_dev_info *devip)
+ unsigned int num, struct sdebug_dev_info *devip,
+ u32 ei_lba)
{
unsigned long iflags;
int ret;
@@ -1699,7 +1728,7 @@ static int resp_read(struct scsi_cmnd *SCpnt, unsigned long long lba,
/* DIX + T10 DIF */
if (scsi_debug_dix && scsi_prot_sg_count(SCpnt)) {
- int prot_ret = prot_verify_read(SCpnt, lba, num);
+ int prot_ret = prot_verify_read(SCpnt, lba, num, ei_lba);
if (prot_ret) {
mk_sense_buffer(devip, ABORTED_COMMAND, 0x10, prot_ret);
@@ -1735,7 +1764,7 @@ void dump_sector(unsigned char *buf, int len)
}
static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
- unsigned int sectors)
+ unsigned int sectors, u32 ei_lba)
{
int i, j, ret;
struct sd_dif_tuple *sdt;
@@ -1749,11 +1778,6 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
sector = do_div(tmp_sec, sdebug_store_sectors);
- if (((SCpnt->cmnd[1] >> 5) & 7) != 1) {
- printk(KERN_WARNING "scsi_debug: WRPROTECT != 1\n");
- return 0;
- }
-
BUG_ON(scsi_sg_count(SCpnt) == 0);
BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
@@ -1808,7 +1832,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
goto out;
}
- if (scsi_debug_dif != SD_DIF_TYPE3_PROTECTION &&
+ if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION &&
be32_to_cpu(sdt->ref_tag)
!= (start_sec & 0xffffffff)) {
printk(KERN_ERR
@@ -1819,6 +1843,16 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
goto out;
}
+ if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION &&
+ be32_to_cpu(sdt->ref_tag) != ei_lba) {
+ printk(KERN_ERR
+ "%s: REF check failed on sector %lu\n",
+ __func__, (unsigned long)sector);
+ ret = 0x03;
+ dump_sector(daddr, scsi_debug_sector_size);
+ goto out;
+ }
+
/* Would be great to copy this in bigger
* chunks. However, for the sake of
* correctness we need to verify each sector
@@ -1832,6 +1866,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
sector = 0; /* Force wrap */
start_sec++;
+ ei_lba++;
daddr += scsi_debug_sector_size;
ppage_offset += sizeof(struct sd_dif_tuple);
}
@@ -1853,7 +1888,8 @@ out:
}
static int resp_write(struct scsi_cmnd *SCpnt, unsigned long long lba,
- unsigned int num, struct sdebug_dev_info *devip)
+ unsigned int num, struct sdebug_dev_info *devip,
+ u32 ei_lba)
{
unsigned long iflags;
int ret;
@@ -1864,7 +1900,7 @@ static int resp_write(struct scsi_cmnd *SCpnt, unsigned long long lba,
/* DIX + T10 DIF */
if (scsi_debug_dix && scsi_prot_sg_count(SCpnt)) {
- int prot_ret = prot_verify_write(SCpnt, lba, num);
+ int prot_ret = prot_verify_write(SCpnt, lba, num, ei_lba);
if (prot_ret) {
mk_sense_buffer(devip, ILLEGAL_REQUEST, 0x10, prot_ret);
@@ -2872,11 +2908,12 @@ static int __init scsi_debug_init(void)
case SD_DIF_TYPE0_PROTECTION:
case SD_DIF_TYPE1_PROTECTION:
+ case SD_DIF_TYPE2_PROTECTION:
case SD_DIF_TYPE3_PROTECTION:
break;
default:
- printk(KERN_ERR "scsi_debug_init: dif must be 0, 1 or 3\n");
+ printk(KERN_ERR "scsi_debug_init: dif must be 0, 1, 2 or 3\n");
return -EINVAL;
}
@@ -3121,6 +3158,7 @@ int scsi_debug_queuecommand(struct scsi_cmnd *SCpnt, done_funct_t done)
int len, k;
unsigned int num;
unsigned long long lba;
+ u32 ei_lba;
int errsts = 0;
int target = SCpnt->device->id;
struct sdebug_dev_info *devip = NULL;
@@ -3254,14 +3292,30 @@ int scsi_debug_queuecommand(struct scsi_cmnd *SCpnt, done_funct_t done)
case READ_16:
case READ_12:
case READ_10:
+ /* READ{10,12,16} and DIF Type 2 are natural enemies */
+ if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION &&
+ cmd[1] & 0xe0) {
+ mk_sense_buffer(devip, ILLEGAL_REQUEST,
+ INVALID_COMMAND_OPCODE, 0);
+ errsts = check_condition_result;
+ break;
+ }
+
+ if ((scsi_debug_dif == SD_DIF_TYPE1_PROTECTION ||
+ scsi_debug_dif == SD_DIF_TYPE3_PROTECTION) &&
+ (cmd[1] & 0xe0) == 0)
+ printk(KERN_ERR "Unprotected RD/WR to DIF device\n");
+
+ /* fall through */
case READ_6:
+read:
errsts = check_readiness(SCpnt, 0, devip);
if (errsts)
break;
if (scsi_debug_fake_rw)
break;
- get_data_transfer_info(cmd, &lba, &num);
- errsts = resp_read(SCpnt, lba, num, devip);
+ get_data_transfer_info(cmd, &lba, &num, &ei_lba);
+ errsts = resp_read(SCpnt, lba, num, devip, ei_lba);
if (inj_recovered && (0 == errsts)) {
mk_sense_buffer(devip, RECOVERED_ERROR,
THRESHOLD_EXCEEDED, 0);
@@ -3288,14 +3342,30 @@ int scsi_debug_queuecommand(struct scsi_cmnd *SCpnt, done_funct_t done)
case WRITE_16:
case WRITE_12:
case WRITE_10:
+ /* WRITE{10,12,16} and DIF Type 2 are natural enemies */
+ if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION &&
+ cmd[1] & 0xe0) {
+ mk_sense_buffer(devip, ILLEGAL_REQUEST,
+ INVALID_COMMAND_OPCODE, 0);
+ errsts = check_condition_result;
+ break;
+ }
+
+ if ((scsi_debug_dif == SD_DIF_TYPE1_PROTECTION ||
+ scsi_debug_dif == SD_DIF_TYPE3_PROTECTION) &&
+ (cmd[1] & 0xe0) == 0)
+ printk(KERN_ERR "Unprotected RD/WR to DIF device\n");
+
+ /* fall through */
case WRITE_6:
+write:
errsts = check_readiness(SCpnt, 0, devip);
if (errsts)
break;
if (scsi_debug_fake_rw)
break;
- get_data_transfer_info(cmd, &lba, &num);
- errsts = resp_write(SCpnt, lba, num, devip);
+ get_data_transfer_info(cmd, &lba, &num, &ei_lba);
+ errsts = resp_write(SCpnt, lba, num, devip, ei_lba);
if (inj_recovered && (0 == errsts)) {
mk_sense_buffer(devip, RECOVERED_ERROR,
THRESHOLD_EXCEEDED, 0);
@@ -3341,15 +3411,34 @@ int scsi_debug_queuecommand(struct scsi_cmnd *SCpnt, done_funct_t done)
break;
if (scsi_debug_fake_rw)
break;
- get_data_transfer_info(cmd, &lba, &num);
- errsts = resp_read(SCpnt, lba, num, devip);
+ get_data_transfer_info(cmd, &lba, &num, &ei_lba);
+ errsts = resp_read(SCpnt, lba, num, devip, ei_lba);
if (errsts)
break;
- errsts = resp_write(SCpnt, lba, num, devip);
+ errsts = resp_write(SCpnt, lba, num, devip, ei_lba);
if (errsts)
break;
errsts = resp_xdwriteread(SCpnt, lba, num, devip);
break;
+ case VARIABLE_LENGTH_CMD:
+ if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION) {
+
+ if ((cmd[10] & 0xe0) == 0)
+ printk(KERN_ERR
+ "Unprotected RD/WR to DIF device\n");
+
+ if (cmd[9] == READ_32)
+ goto read;
+
+ if (cmd[9] == WRITE_32)
+ goto write;
+ }
+
+ mk_sense_buffer(devip, ILLEGAL_REQUEST,
+ INVALID_FIELD_IN_CDB, 0);
+ errsts = check_condition_result;
+ break;
+
default:
if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
printk(KERN_INFO "scsi_debug: Opcode: 0x%x not "
--
1.6.0.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] scsi_debug: Implement support for DIF Type 2
2009-09-11 19:20 ` [PATCH 5/5] scsi_debug: Implement support for " Martin K. Petersen
@ 2009-09-11 23:06 ` Douglas Gilbert
0 siblings, 0 replies; 17+ messages in thread
From: Douglas Gilbert @ 2009-09-11 23:06 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: James.Bottomley, linux-scsi
Martin K. Petersen wrote:
> Add support for 32-byte READ/WRITE as well as DIF Type 2 protection.
>
> Reject protected 10/12/16 byte READ/WRITE commands when Type 2 is
> enabled.
>
> Verify Type 2 reference tag according to Expected Initial LBA in 32-byte
> CDB.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] scsi: Fix protection scsi_data_buffer leak
2009-09-11 19:20 ` [PATCH 3/5] scsi: Fix protection scsi_data_buffer leak Martin K. Petersen
@ 2009-09-13 9:36 ` Boaz Harrosh
0 siblings, 0 replies; 17+ messages in thread
From: Boaz Harrosh @ 2009-09-13 9:36 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: James.Bottomley, linux-scsi
On 09/11/2009 10:20 PM, Martin K. Petersen wrote:
> We would leak a scsi_data_buffer if the free_list command was of the
> protected variety.
>
> Reported-by: Boaz Harrosh <bharrosh@panasas.com>
Reviewed-by: Boaz Harrosh <bharrosh@panasas.com>
I like the locality of the temp variables, thanks.
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
> drivers/scsi/scsi.c | 11 +++++++----
> 1 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 2de5f3a..69397bb 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -241,10 +241,7 @@ scsi_host_alloc_command(struct Scsi_Host *shost, gfp_t gfp_mask)
> */
> struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask)
> {
> - struct scsi_cmnd *cmd;
> - unsigned char *buf;
> -
> - cmd = scsi_host_alloc_command(shost, gfp_mask);
> + struct scsi_cmnd *cmd = scsi_host_alloc_command(shost, gfp_mask);
>
> if (unlikely(!cmd)) {
> unsigned long flags;
> @@ -258,9 +255,15 @@ struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask)
> spin_unlock_irqrestore(&shost->free_list_lock, flags);
>
> if (cmd) {
> + void *buf, *prot;
> +
> buf = cmd->sense_buffer;
> + prot = cmd->prot_sdb;
> +
> memset(cmd, 0, sizeof(*cmd));
> +
> cmd->sense_buffer = buf;
> + cmd->prot_sdb = prot;
> }
> }
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] sd: Support disks formatted with DIF Type 2
2009-09-11 19:20 ` [PATCH 4/5] sd: Support disks formatted with DIF Type 2 Martin K. Petersen
@ 2009-09-13 9:37 ` Boaz Harrosh
0 siblings, 0 replies; 17+ messages in thread
From: Boaz Harrosh @ 2009-09-13 9:37 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: James.Bottomley, linux-scsi
On 09/11/2009 10:20 PM, Martin K. Petersen wrote:
> Disks formatted with DIF Type 2 reject READ/WRITE 6/10/12/16 commands
> when protection is enabled. Only the 32-byte variants are supported.
>
> Implement support for issusing 32-byte READ/WRITE and enable Type 2
> drives in the protection type detection logic.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Boaz Harrosh <bharrosh@panasas.com>
> ---
> drivers/scsi/sd.c | 80 ++++++++++++++++++++++++++++++++++++++++++++-------
> drivers/scsi/sd.h | 5 +++
> include/scsi/scsi.h | 3 ++
> 3 files changed, 77 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 36e2333..d138e6b 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -116,6 +116,9 @@ static DEFINE_IDA(sd_index_ida);
> * object after last put) */
> static DEFINE_MUTEX(sd_ref_mutex);
>
> +struct kmem_cache *sd_cdb_cache;
> +mempool_t *sd_cdb_pool;
> +
> static const char *sd_cache_types[] = {
> "write through", "none", "write back",
> "write back, no read (daft)"
> @@ -413,6 +416,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
> sector_t threshold;
> unsigned int this_count = blk_rq_sectors(rq);
> int ret, host_dif;
> + unsigned char protect;
>
> if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
> ret = scsi_setup_blk_pc_cmnd(sdp, rq);
> @@ -545,13 +549,48 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
> /* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
> host_dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
> if (host_dif)
> - SCpnt->cmnd[1] = 1 << 5;
> + protect = 1 << 5;
> else
> - SCpnt->cmnd[1] = 0;
> + protect = 0;
> +
> + if (host_dif == SD_DIF_TYPE2_PROTECTION) {
> + SCpnt->cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
> +
> + if (unlikely(SCpnt->cmnd == NULL)) {
> + ret = BLKPREP_DEFER;
> + goto out;
> + }
>
> - if (block > 0xffffffff) {
> + memset(SCpnt->cmnd, 0, SD_EXT_CDB_SIZE);
> + SCpnt->cmnd[0] = VARIABLE_LENGTH_CMD;
> + SCpnt->cmnd[7] = 0x18;
> + SCpnt->cmnd[9] = (rq_data_dir(rq) == READ) ? READ_32 : WRITE_32;
> + SCpnt->cmnd[10] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
> +
> + /* LBA */
> + SCpnt->cmnd[12] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
> + SCpnt->cmnd[13] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
> + SCpnt->cmnd[14] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
> + SCpnt->cmnd[15] = sizeof(block) > 4 ? (unsigned char) (block >> 32) & 0xff : 0;
> + SCpnt->cmnd[16] = (unsigned char) (block >> 24) & 0xff;
> + SCpnt->cmnd[17] = (unsigned char) (block >> 16) & 0xff;
> + SCpnt->cmnd[18] = (unsigned char) (block >> 8) & 0xff;
> + SCpnt->cmnd[19] = (unsigned char) block & 0xff;
> +
> + /* Expected Indirect LBA */
> + SCpnt->cmnd[20] = (unsigned char) (block >> 24) & 0xff;
> + SCpnt->cmnd[21] = (unsigned char) (block >> 16) & 0xff;
> + SCpnt->cmnd[22] = (unsigned char) (block >> 8) & 0xff;
> + SCpnt->cmnd[23] = (unsigned char) block & 0xff;
> +
> + /* Transfer length */
> + SCpnt->cmnd[28] = (unsigned char) (this_count >> 24) & 0xff;
> + SCpnt->cmnd[29] = (unsigned char) (this_count >> 16) & 0xff;
> + SCpnt->cmnd[30] = (unsigned char) (this_count >> 8) & 0xff;
> + SCpnt->cmnd[31] = (unsigned char) this_count & 0xff;
> + } else if (block > 0xffffffff) {
> SCpnt->cmnd[0] += READ_16 - READ_6;
> - SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
> + SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
> SCpnt->cmnd[2] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
> SCpnt->cmnd[3] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
> SCpnt->cmnd[4] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
> @@ -572,7 +611,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
> this_count = 0xffff;
>
> SCpnt->cmnd[0] += READ_10 - READ_6;
> - SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
> + SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
> SCpnt->cmnd[2] = (unsigned char) (block >> 24) & 0xff;
> SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff;
> SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;
> @@ -1047,6 +1086,7 @@ static int sd_done(struct scsi_cmnd *SCpnt)
> int result = SCpnt->result;
> unsigned int good_bytes = result ? 0 : scsi_bufflen(SCpnt);
> struct scsi_sense_hdr sshdr;
> + struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk);
> int sense_valid = 0;
> int sense_deferred = 0;
>
> @@ -1108,6 +1148,10 @@ static int sd_done(struct scsi_cmnd *SCpnt)
> if (rq_data_dir(SCpnt->request) == READ && scsi_prot_sg_count(SCpnt))
> sd_dif_complete(SCpnt, good_bytes);
>
> + if (scsi_host_dif_capable(sdkp->device->host, sdkp->protection_type)
> + == SD_DIF_TYPE2_PROTECTION)
> + mempool_free(SCpnt->cmnd, sd_cdb_pool);
> +
> return good_bytes;
> }
>
> @@ -1271,12 +1315,7 @@ void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
>
> sdkp->protection_type = type;
>
> - switch (type) {
> - case SD_DIF_TYPE1_PROTECTION:
> - case SD_DIF_TYPE3_PROTECTION:
> - break;
> -
> - default:
> + if (type > SD_DIF_TYPE3_PROTECTION) {
> sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
> "protection type %u. Disabling disk!\n", type);
> sdkp->capacity = 0;
> @@ -2321,8 +2360,24 @@ static int __init init_sd(void)
> if (err)
> goto err_out_class;
>
> + sd_cdb_cache = kmem_cache_create("sd_ext_cdb", SD_EXT_CDB_SIZE,
> + 0, 0, NULL);
> + if (!sd_cdb_cache) {
> + printk(KERN_ERR "sd: can't init extended cdb cache\n");
> + goto err_out_class;
> + }
> +
> + sd_cdb_pool = mempool_create_slab_pool(SD_MEMPOOL_SIZE, sd_cdb_cache);
> + if (!sd_cdb_pool) {
> + printk(KERN_ERR "sd: can't init extended cdb pool\n");
> + goto err_out_cache;
> + }
> +
> return 0;
>
> +err_out_cache:
> + kmem_cache_destroy(sd_cdb_cache);
> +
> err_out_class:
> class_unregister(&sd_disk_class);
> err_out:
> @@ -2342,6 +2397,9 @@ static void __exit exit_sd(void)
>
> SCSI_LOG_HLQUEUE(3, printk("exit_sd: exiting sd driver\n"));
>
> + mempool_destroy(sd_cdb_pool);
> + kmem_cache_destroy(sd_cdb_cache);
> +
> scsi_unregister_driver(&sd_template.gendrv);
> class_unregister(&sd_disk_class);
>
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index ce1f5f8..e374804 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -37,6 +37,11 @@
> */
> #define SD_LAST_BUGGY_SECTORS 8
>
> +enum {
> + SD_EXT_CDB_SIZE = 32, /* Extended CDB size */
> + SD_MEMPOOL_SIZE = 2, /* CDB pool size */
> +};
> +
> struct scsi_disk {
> struct scsi_driver *driver; /* always &sd_template */
> struct scsi_device *device;
> diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
> index 084478e..34c46ab 100644
> --- a/include/scsi/scsi.h
> +++ b/include/scsi/scsi.h
> @@ -129,6 +129,9 @@ struct scsi_cmnd;
> #define MI_REPORT_TARGET_PGS 0x0a
> /* values for maintenance out */
> #define MO_SET_TARGET_PGS 0x0a
> +/* values for variable length command */
> +#define READ_32 0x09
> +#define WRITE_32 0x0b
>
> /* Values for T10/04-262r7 */
> #define ATA_16 0x85 /* 16-byte pass-thru */
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] scsi: Deprecate SCSI_PROT_*_CONVERT operations
2009-09-11 19:20 ` [PATCH 1/5] scsi: Deprecate SCSI_PROT_*_CONVERT operations Martin K. Petersen
@ 2009-09-18 23:16 ` James Smart
0 siblings, 0 replies; 17+ messages in thread
From: James Smart @ 2009-09-18 23:16 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: James.Bottomley, linux-scsi
lpfc mods look fine.
Acked-by: James Smart <james.smart@emulex.com>
Acked-by: Ihab Hamadi<Ihab.Hamadi@Emulex.Com>
-- james s
Martin K. Petersen wrote:
> The checksum format is orthogonal to whether the protection information
> is being passed on beyond the HBA or not. It is perfectly valid to use
> a non-T10 CRC with WRITE_STRIP and READ_INSERT.
>
> Consequently it no longer makes sense to explicitly refer to the
> conversion in the protection operation. Update sd_dif and lpfc
> accordingly.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
> drivers/scsi/lpfc/lpfc_scsi.c | 15 +++------------
> drivers/scsi/sd_dif.c | 20 ++++----------------
> include/scsi/scsi_cmnd.h | 4 ----
> 3 files changed, 7 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
> index da59c4f..ad75e19 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.c
> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
> @@ -56,8 +56,6 @@ static char *dif_op_str[] = {
> "SCSI_PROT_WRITE_INSERT",
> "SCSI_PROT_READ_PASS",
> "SCSI_PROT_WRITE_PASS",
> - "SCSI_PROT_READ_CONVERT",
> - "SCSI_PROT_WRITE_CONVERT"
> };
> static void
> lpfc_release_scsi_buf_s4(struct lpfc_hba *phba, struct lpfc_scsi_buf *psb);
> @@ -1131,13 +1129,11 @@ lpfc_sc_to_sli_prof(struct scsi_cmnd *sc)
> ret_prof = LPFC_PROF_A1;
> break;
>
> - case SCSI_PROT_READ_CONVERT:
> - case SCSI_PROT_WRITE_CONVERT:
> + case SCSI_PROT_READ_PASS:
> + case SCSI_PROT_WRITE_PASS:
> ret_prof = LPFC_PROF_AST1;
> break;
>
> - case SCSI_PROT_READ_PASS:
> - case SCSI_PROT_WRITE_PASS:
> case SCSI_PROT_NORMAL:
> default:
> printk(KERN_ERR "Bad op/guard:%d/%d combination\n",
> @@ -1157,8 +1153,6 @@ lpfc_sc_to_sli_prof(struct scsi_cmnd *sc)
> ret_prof = LPFC_PROF_C1;
> break;
>
> - case SCSI_PROT_READ_CONVERT:
> - case SCSI_PROT_WRITE_CONVERT:
> case SCSI_PROT_READ_INSERT:
> case SCSI_PROT_WRITE_STRIP:
> case SCSI_PROT_NORMAL:
> @@ -1209,8 +1203,7 @@ lpfc_get_cmd_dif_parms(struct scsi_cmnd *sc, uint16_t *apptagmask,
> static int cnt;
>
> if (protcnt && (op == SCSI_PROT_WRITE_STRIP ||
> - op == SCSI_PROT_WRITE_PASS ||
> - op == SCSI_PROT_WRITE_CONVERT)) {
> + op == SCSI_PROT_WRITE_PASS)) {
>
> cnt++;
> spt = page_address(sg_page(scsi_prot_sglist(sc))) +
> @@ -1501,8 +1494,6 @@ lpfc_prot_group_type(struct lpfc_hba *phba, struct scsi_cmnd *sc)
> case SCSI_PROT_WRITE_STRIP:
> case SCSI_PROT_READ_PASS:
> case SCSI_PROT_WRITE_PASS:
> - case SCSI_PROT_WRITE_CONVERT:
> - case SCSI_PROT_READ_CONVERT:
> ret = LPFC_PG_TYPE_DIF_BUF;
> break;
> default:
> diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
> index 82f14a9..84224dd 100644
> --- a/drivers/scsi/sd_dif.c
> +++ b/drivers/scsi/sd_dif.c
> @@ -364,15 +364,9 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
> */
> void sd_dif_op(struct scsi_cmnd *scmd, unsigned int dif, unsigned int dix, unsigned int type)
> {
> - int csum_convert, prot_op;
> + int prot_op;
>
> - prot_op = 0;
> -
> - /* Convert checksum? */
> - if (scsi_host_get_guard(scmd->device->host) != SHOST_DIX_GUARD_CRC)
> - csum_convert = 1;
> - else
> - csum_convert = 0;
> + prot_op = SCSI_PROT_NORMAL;
>
> BUG_ON(dif && (scmd->cmnd[0] == READ_6 || scmd->cmnd[0] == WRITE_6));
>
> @@ -382,10 +376,7 @@ void sd_dif_op(struct scsi_cmnd *scmd, unsigned int dif, unsigned int dix, unsig
> case READ_12:
> case READ_16:
> if (dif && dix)
> - if (csum_convert)
> - prot_op = SCSI_PROT_READ_CONVERT;
> - else
> - prot_op = SCSI_PROT_READ_PASS;
> + prot_op = SCSI_PROT_READ_PASS;
> else if (dif && !dix)
> prot_op = SCSI_PROT_READ_STRIP;
> else if (!dif && dix)
> @@ -398,10 +389,7 @@ void sd_dif_op(struct scsi_cmnd *scmd, unsigned int dif, unsigned int dix, unsig
> case WRITE_12:
> case WRITE_16:
> if (dif && dix)
> - if (csum_convert)
> - prot_op = SCSI_PROT_WRITE_CONVERT;
> - else
> - prot_op = SCSI_PROT_WRITE_PASS;
> + prot_op = SCSI_PROT_WRITE_PASS;
> else if (dif && !dix)
> prot_op = SCSI_PROT_WRITE_INSERT;
> else if (!dif && dix)
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 3878d1d..a5e885a 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -229,10 +229,6 @@ enum scsi_prot_operations {
> /* OS-HBA: Protected, HBA-Target: Protected */
> SCSI_PROT_READ_PASS,
> SCSI_PROT_WRITE_PASS,
> -
> - /* OS-HBA: Protected, HBA-Target: Protected, checksum conversion */
> - SCSI_PROT_READ_CONVERT,
> - SCSI_PROT_WRITE_CONVERT,
> };
>
> static inline void scsi_set_prot_op(struct scsi_cmnd *scmd, unsigned char op)
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] sd: Support disks formatted with DIF Type 2
2009-09-18 21:57 ` James Bottomley
@ 2009-09-20 20:49 ` Martin K. Petersen
0 siblings, 0 replies; 17+ messages in thread
From: Martin K. Petersen @ 2009-09-20 20:49 UTC (permalink / raw)
To: James Bottomley; +Cc: Martin K. Petersen, linux-scsi
>>>>> "James" == James Bottomley <James.Bottomley@suse.de> writes:
James> If you look at Christoph's discard patch set, you'll see they
James> come back through this path in scsi_finish_io(), which would
James> cause a bogus free for discard on DIF/DIX systems.
That's fine. The existing check is already on the conservative side.
Ideally we'd have a flag corresponding to the allocation instead of
asking all these questions at sd_done() time. I was just trying to
prevent scsi_cmnd from growing.
sd: Support disks formatted with DIF Type 2
Disks formatted with DIF Type 2 reject READ/WRITE 6/10/12/16 commands
when protection is enabled. Only the 32-byte variants are supported.
Implement support for issusing 32-byte READ/WRITE and enable Type 2
drives in the protection type detection logic.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 7f9face..bc41294 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -116,6 +116,9 @@ static DEFINE_IDA(sd_index_ida);
* object after last put) */
static DEFINE_MUTEX(sd_ref_mutex);
+struct kmem_cache *sd_cdb_cache;
+mempool_t *sd_cdb_pool;
+
static const char *sd_cache_types[] = {
"write through", "none", "write back",
"write back, no read (daft)"
@@ -413,6 +416,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
sector_t threshold;
unsigned int this_count = blk_rq_sectors(rq);
int ret, host_dif;
+ unsigned char protect;
if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
ret = scsi_setup_blk_pc_cmnd(sdp, rq);
@@ -545,13 +549,49 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
/* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
host_dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
if (host_dif)
- SCpnt->cmnd[1] = 1 << 5;
+ protect = 1 << 5;
else
- SCpnt->cmnd[1] = 0;
+ protect = 0;
+
+ if (host_dif == SD_DIF_TYPE2_PROTECTION) {
+ SCpnt->cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
+
+ if (unlikely(SCpnt->cmnd == NULL)) {
+ ret = BLKPREP_DEFER;
+ goto out;
+ }
- if (block > 0xffffffff) {
+ SCpnt->cmd_len = SD_EXT_CDB_SIZE;
+ memset(SCpnt->cmnd, 0, SCpnt->cmd_len);
+ SCpnt->cmnd[0] = VARIABLE_LENGTH_CMD;
+ SCpnt->cmnd[7] = 0x18;
+ SCpnt->cmnd[9] = (rq_data_dir(rq) == READ) ? READ_32 : WRITE_32;
+ SCpnt->cmnd[10] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
+
+ /* LBA */
+ SCpnt->cmnd[12] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
+ SCpnt->cmnd[13] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
+ SCpnt->cmnd[14] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
+ SCpnt->cmnd[15] = sizeof(block) > 4 ? (unsigned char) (block >> 32) & 0xff : 0;
+ SCpnt->cmnd[16] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[17] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[18] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[19] = (unsigned char) block & 0xff;
+
+ /* Expected Indirect LBA */
+ SCpnt->cmnd[20] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[21] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[22] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[23] = (unsigned char) block & 0xff;
+
+ /* Transfer length */
+ SCpnt->cmnd[28] = (unsigned char) (this_count >> 24) & 0xff;
+ SCpnt->cmnd[29] = (unsigned char) (this_count >> 16) & 0xff;
+ SCpnt->cmnd[30] = (unsigned char) (this_count >> 8) & 0xff;
+ SCpnt->cmnd[31] = (unsigned char) this_count & 0xff;
+ } else if (block > 0xffffffff) {
SCpnt->cmnd[0] += READ_16 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
SCpnt->cmnd[3] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
SCpnt->cmnd[4] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
@@ -572,7 +612,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
this_count = 0xffff;
SCpnt->cmnd[0] += READ_10 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = (unsigned char) (block >> 24) & 0xff;
SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff;
SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;
@@ -1047,6 +1087,7 @@ static int sd_done(struct scsi_cmnd *SCpnt)
int result = SCpnt->result;
unsigned int good_bytes = result ? 0 : scsi_bufflen(SCpnt);
struct scsi_sense_hdr sshdr;
+ struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk);
int sense_valid = 0;
int sense_deferred = 0;
@@ -1108,6 +1149,10 @@ static int sd_done(struct scsi_cmnd *SCpnt)
if (rq_data_dir(SCpnt->request) == READ && scsi_prot_sg_count(SCpnt))
sd_dif_complete(SCpnt, good_bytes);
+ if (scsi_host_dif_capable(sdkp->device->host, sdkp->protection_type)
+ == SD_DIF_TYPE2_PROTECTION && SCpnt->cmnd != SCpnt->request->cmd)
+ mempool_free(SCpnt->cmnd, sd_cdb_pool);
+
return good_bytes;
}
@@ -1271,12 +1316,7 @@ void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
sdkp->protection_type = type;
- switch (type) {
- case SD_DIF_TYPE1_PROTECTION:
- case SD_DIF_TYPE3_PROTECTION:
- break;
-
- default:
+ if (type > SD_DIF_TYPE3_PROTECTION) {
sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
"protection type %u. Disabling disk!\n", type);
sdkp->capacity = 0;
@@ -2323,8 +2363,24 @@ static int __init init_sd(void)
if (err)
goto err_out_class;
+ sd_cdb_cache = kmem_cache_create("sd_ext_cdb", SD_EXT_CDB_SIZE,
+ 0, 0, NULL);
+ if (!sd_cdb_cache) {
+ printk(KERN_ERR "sd: can't init extended cdb cache\n");
+ goto err_out_class;
+ }
+
+ sd_cdb_pool = mempool_create_slab_pool(SD_MEMPOOL_SIZE, sd_cdb_cache);
+ if (!sd_cdb_pool) {
+ printk(KERN_ERR "sd: can't init extended cdb pool\n");
+ goto err_out_cache;
+ }
+
return 0;
+err_out_cache:
+ kmem_cache_destroy(sd_cdb_cache);
+
err_out_class:
class_unregister(&sd_disk_class);
err_out:
@@ -2344,6 +2400,9 @@ static void __exit exit_sd(void)
SCSI_LOG_HLQUEUE(3, printk("exit_sd: exiting sd driver\n"));
+ mempool_destroy(sd_cdb_pool);
+ kmem_cache_destroy(sd_cdb_cache);
+
scsi_unregister_driver(&sd_template.gendrv);
class_unregister(&sd_disk_class);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index ce1f5f8..e374804 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -37,6 +37,11 @@
*/
#define SD_LAST_BUGGY_SECTORS 8
+enum {
+ SD_EXT_CDB_SIZE = 32, /* Extended CDB size */
+ SD_MEMPOOL_SIZE = 2, /* CDB pool size */
+};
+
struct scsi_disk {
struct scsi_driver *driver; /* always &sd_template */
struct scsi_device *device;
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 084478e..34c46ab 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -129,6 +129,9 @@ struct scsi_cmnd;
#define MI_REPORT_TARGET_PGS 0x0a
/* values for maintenance out */
#define MO_SET_TARGET_PGS 0x0a
+/* values for variable length command */
+#define READ_32 0x09
+#define WRITE_32 0x0b
/* Values for T10/04-262r7 */
#define ATA_16 0x85 /* 16-byte pass-thru */
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] sd: Support disks formatted with DIF Type 2
2009-09-18 21:33 ` [PATCH 4/5] sd: Support disks formatted with DIF Type 2 Martin K. Petersen
@ 2009-09-18 21:57 ` James Bottomley
2009-09-20 20:49 ` Martin K. Petersen
0 siblings, 1 reply; 17+ messages in thread
From: James Bottomley @ 2009-09-18 21:57 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: linux-scsi
On Fri, 2009-09-18 at 17:33 -0400, Martin K. Petersen wrote:
> Disks formatted with DIF Type 2 reject READ/WRITE 6/10/12/16 commands
> when protection is enabled. Only the 32-byte variants are supported.
>
> Implement support for issusing 32-byte READ/WRITE and enable Type 2
> drives in the protection type detection logic.
All looks fine (in all patches [well, except the signed-off-by from doug
should be acked-by]), except this:
> @@ -1108,6 +1149,10 @@ static int sd_done(struct scsi_cmnd *SCpnt)
> if (rq_data_dir(SCpnt->request) == READ && scsi_prot_sg_count(SCpnt))
> sd_dif_complete(SCpnt, good_bytes);
>
> + if (scsi_host_dif_capable(sdkp->device->host, sdkp->protection_type)
> + == SD_DIF_TYPE2_PROTECTION)
> + mempool_free(SCpnt->cmnd, sd_cdb_pool);
> +
> return good_bytes;
> }
If you look at Christoph's discard patch set, you'll see they come back
through this path in scsi_finish_io(), which would cause a bogus free
for discard on DIF/DIX systems.
I'd really rather this check were tightened to something like
if (scsi_host_dif_capable(sdkp->device->host, sdkp->protection_type)
== SD_DIF_TYPE2_PROTECTION && SCpnt->cmnd != SCpnt->request->cmd)
So we're totally positive we fiddled with the command pointer before we
free it.
James
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/5] sd: Support disks formatted with DIF Type 2
2009-09-18 21:32 Final DIF/DIX patches for 2.6.32 Martin K. Petersen
@ 2009-09-18 21:33 ` Martin K. Petersen
2009-09-18 21:57 ` James Bottomley
0 siblings, 1 reply; 17+ messages in thread
From: Martin K. Petersen @ 2009-09-18 21:33 UTC (permalink / raw)
To: linux-scsi, James.Bottomley; +Cc: Martin K. Petersen
Disks formatted with DIF Type 2 reject READ/WRITE 6/10/12/16 commands
when protection is enabled. Only the 32-byte variants are supported.
Implement support for issusing 32-byte READ/WRITE and enable Type 2
drives in the protection type detection logic.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
drivers/scsi/sd.c | 81 ++++++++++++++++++++++++++++++++++++++++++++-------
drivers/scsi/sd.h | 5 +++
include/scsi/scsi.h | 3 ++
3 files changed, 78 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 36e2333..fa44936 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -116,6 +116,9 @@ static DEFINE_IDA(sd_index_ida);
* object after last put) */
static DEFINE_MUTEX(sd_ref_mutex);
+struct kmem_cache *sd_cdb_cache;
+mempool_t *sd_cdb_pool;
+
static const char *sd_cache_types[] = {
"write through", "none", "write back",
"write back, no read (daft)"
@@ -413,6 +416,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
sector_t threshold;
unsigned int this_count = blk_rq_sectors(rq);
int ret, host_dif;
+ unsigned char protect;
if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
ret = scsi_setup_blk_pc_cmnd(sdp, rq);
@@ -545,13 +549,49 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
/* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
host_dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
if (host_dif)
- SCpnt->cmnd[1] = 1 << 5;
+ protect = 1 << 5;
else
- SCpnt->cmnd[1] = 0;
+ protect = 0;
+
+ if (host_dif == SD_DIF_TYPE2_PROTECTION) {
+ SCpnt->cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
+
+ if (unlikely(SCpnt->cmnd == NULL)) {
+ ret = BLKPREP_DEFER;
+ goto out;
+ }
- if (block > 0xffffffff) {
+ SCpnt->cmd_len = SD_EXT_CDB_SIZE;
+ memset(SCpnt->cmnd, 0, SCpnt->cmd_len);
+ SCpnt->cmnd[0] = VARIABLE_LENGTH_CMD;
+ SCpnt->cmnd[7] = 0x18;
+ SCpnt->cmnd[9] = (rq_data_dir(rq) == READ) ? READ_32 : WRITE_32;
+ SCpnt->cmnd[10] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
+
+ /* LBA */
+ SCpnt->cmnd[12] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
+ SCpnt->cmnd[13] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
+ SCpnt->cmnd[14] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
+ SCpnt->cmnd[15] = sizeof(block) > 4 ? (unsigned char) (block >> 32) & 0xff : 0;
+ SCpnt->cmnd[16] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[17] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[18] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[19] = (unsigned char) block & 0xff;
+
+ /* Expected Indirect LBA */
+ SCpnt->cmnd[20] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[21] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[22] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[23] = (unsigned char) block & 0xff;
+
+ /* Transfer length */
+ SCpnt->cmnd[28] = (unsigned char) (this_count >> 24) & 0xff;
+ SCpnt->cmnd[29] = (unsigned char) (this_count >> 16) & 0xff;
+ SCpnt->cmnd[30] = (unsigned char) (this_count >> 8) & 0xff;
+ SCpnt->cmnd[31] = (unsigned char) this_count & 0xff;
+ } else if (block > 0xffffffff) {
SCpnt->cmnd[0] += READ_16 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
SCpnt->cmnd[3] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
SCpnt->cmnd[4] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
@@ -572,7 +612,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
this_count = 0xffff;
SCpnt->cmnd[0] += READ_10 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = (unsigned char) (block >> 24) & 0xff;
SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff;
SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;
@@ -1047,6 +1087,7 @@ static int sd_done(struct scsi_cmnd *SCpnt)
int result = SCpnt->result;
unsigned int good_bytes = result ? 0 : scsi_bufflen(SCpnt);
struct scsi_sense_hdr sshdr;
+ struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk);
int sense_valid = 0;
int sense_deferred = 0;
@@ -1108,6 +1149,10 @@ static int sd_done(struct scsi_cmnd *SCpnt)
if (rq_data_dir(SCpnt->request) == READ && scsi_prot_sg_count(SCpnt))
sd_dif_complete(SCpnt, good_bytes);
+ if (scsi_host_dif_capable(sdkp->device->host, sdkp->protection_type)
+ == SD_DIF_TYPE2_PROTECTION)
+ mempool_free(SCpnt->cmnd, sd_cdb_pool);
+
return good_bytes;
}
@@ -1271,12 +1316,7 @@ void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
sdkp->protection_type = type;
- switch (type) {
- case SD_DIF_TYPE1_PROTECTION:
- case SD_DIF_TYPE3_PROTECTION:
- break;
-
- default:
+ if (type > SD_DIF_TYPE3_PROTECTION) {
sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
"protection type %u. Disabling disk!\n", type);
sdkp->capacity = 0;
@@ -2321,8 +2361,24 @@ static int __init init_sd(void)
if (err)
goto err_out_class;
+ sd_cdb_cache = kmem_cache_create("sd_ext_cdb", SD_EXT_CDB_SIZE,
+ 0, 0, NULL);
+ if (!sd_cdb_cache) {
+ printk(KERN_ERR "sd: can't init extended cdb cache\n");
+ goto err_out_class;
+ }
+
+ sd_cdb_pool = mempool_create_slab_pool(SD_MEMPOOL_SIZE, sd_cdb_cache);
+ if (!sd_cdb_pool) {
+ printk(KERN_ERR "sd: can't init extended cdb pool\n");
+ goto err_out_cache;
+ }
+
return 0;
+err_out_cache:
+ kmem_cache_destroy(sd_cdb_cache);
+
err_out_class:
class_unregister(&sd_disk_class);
err_out:
@@ -2342,6 +2398,9 @@ static void __exit exit_sd(void)
SCSI_LOG_HLQUEUE(3, printk("exit_sd: exiting sd driver\n"));
+ mempool_destroy(sd_cdb_pool);
+ kmem_cache_destroy(sd_cdb_cache);
+
scsi_unregister_driver(&sd_template.gendrv);
class_unregister(&sd_disk_class);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index ce1f5f8..e374804 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -37,6 +37,11 @@
*/
#define SD_LAST_BUGGY_SECTORS 8
+enum {
+ SD_EXT_CDB_SIZE = 32, /* Extended CDB size */
+ SD_MEMPOOL_SIZE = 2, /* CDB pool size */
+};
+
struct scsi_disk {
struct scsi_driver *driver; /* always &sd_template */
struct scsi_device *device;
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 084478e..34c46ab 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -129,6 +129,9 @@ struct scsi_cmnd;
#define MI_REPORT_TARGET_PGS 0x0a
/* values for maintenance out */
#define MO_SET_TARGET_PGS 0x0a
+/* values for variable length command */
+#define READ_32 0x09
+#define WRITE_32 0x0b
/* Values for T10/04-262r7 */
#define ATA_16 0x85 /* 16-byte pass-thru */
--
1.6.0.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5] sd: Support disks formatted with DIF Type 2
2009-09-04 8:36 DIF/DIX updates for 2.6.32 Martin K. Petersen
@ 2009-09-04 8:36 ` Martin K. Petersen
0 siblings, 0 replies; 17+ messages in thread
From: Martin K. Petersen @ 2009-09-04 8:36 UTC (permalink / raw)
To: linux-scsi; +Cc: Martin K. Petersen
Disks formatted with DIF Type 2 reject READ/WRITE 6/10/12/16 commands
when protection is enabled. Only the 32-byte variants are supported.
Implement support for issusing 32-byte READ/WRITE and enable support for
Type 2 drives in the protection type detection logic.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
drivers/scsi/sd.c | 54 +++++++++++++++++++++++++++++++++++++-------------
include/scsi/scsi.h | 3 ++
2 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 36e2333..bf7ead6 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -413,6 +413,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
sector_t threshold;
unsigned int this_count = blk_rq_sectors(rq);
int ret, host_dif;
+ unsigned char protect;
if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
ret = scsi_setup_blk_pc_cmnd(sdp, rq);
@@ -545,13 +546,40 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
/* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
host_dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
if (host_dif)
- SCpnt->cmnd[1] = 1 << 5;
+ protect = 1 << 5;
else
- SCpnt->cmnd[1] = 0;
-
- if (block > 0xffffffff) {
+ protect = 0;
+
+ if (host_dif == SD_DIF_TYPE2_PROTECTION) {
+ SCpnt->cmnd[0] = VARIABLE_LENGTH_CMD;
+ SCpnt->cmnd[7] = 0x18;
+ SCpnt->cmnd[9] = (rq_data_dir(rq) == READ) ? READ_32 : WRITE_32;
+ SCpnt->cmnd[10] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
+
+ /* LBA */
+ SCpnt->cmnd[12] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
+ SCpnt->cmnd[13] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
+ SCpnt->cmnd[14] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
+ SCpnt->cmnd[15] = sizeof(block) > 4 ? (unsigned char) (block >> 32) & 0xff : 0;
+ SCpnt->cmnd[16] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[17] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[18] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[19] = (unsigned char) block & 0xff;
+
+ /* Expected Indirect LBA */
+ SCpnt->cmnd[20] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[21] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[22] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[23] = (unsigned char) block & 0xff;
+
+ /* Transfer length */
+ SCpnt->cmnd[28] = (unsigned char) (this_count >> 24) & 0xff;
+ SCpnt->cmnd[29] = (unsigned char) (this_count >> 16) & 0xff;
+ SCpnt->cmnd[30] = (unsigned char) (this_count >> 8) & 0xff;
+ SCpnt->cmnd[31] = (unsigned char) this_count & 0xff;
+ } else if (block > 0xffffffff) {
SCpnt->cmnd[0] += READ_16 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
SCpnt->cmnd[3] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
SCpnt->cmnd[4] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
@@ -572,7 +600,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
this_count = 0xffff;
SCpnt->cmnd[0] += READ_10 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = (unsigned char) (block >> 24) & 0xff;
SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff;
SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;
@@ -1271,22 +1299,20 @@ void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
sdkp->protection_type = type;
- switch (type) {
- case SD_DIF_TYPE1_PROTECTION:
- case SD_DIF_TYPE3_PROTECTION:
- break;
-
- default:
+ if (type > SD_DIF_TYPE3_PROTECTION) {
sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
"protection type %u. Disabling disk!\n", type);
sdkp->capacity = 0;
return;
}
- if (scsi_host_dif_capable(sdp->host, type))
+ if (scsi_host_dif_capable(sdp->host, type)) {
sd_printk(KERN_NOTICE, sdkp,
"Enabling DIF Type %u protection\n", type);
- else
+
+ if (type == SD_DIF_TYPE2_PROTECTION)
+ sdp->use_32_for_rw = 1;
+ } else
sd_printk(KERN_NOTICE, sdkp,
"Disabling DIF Type %u protection\n", type);
}
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index a7ae10a..847de86 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -129,6 +129,9 @@ struct scsi_cmnd;
#define MI_REPORT_TARGET_PGS 0x0a
/* values for maintenance out */
#define MO_SET_TARGET_PGS 0x0a
+/* values for variable length command */
+#define READ_32 0x09
+#define WRITE_32 0x0b
/* Values for T10/04-262r7 */
#define ATA_16 0x85 /* 16-byte pass-thru */
--
1.6.0.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] sd: Support disks formatted with DIF Type 2
2009-08-26 12:26 ` Boaz Harrosh
@ 2009-08-27 6:41 ` Martin K. Petersen
0 siblings, 0 replies; 17+ messages in thread
From: Martin K. Petersen @ 2009-08-27 6:41 UTC (permalink / raw)
To: Boaz Harrosh; +Cc: linux-scsi
>> + BUG_ON(rq->cmd_len != SCSI_EXT_CDB_SIZE);
Boaz> This is not nice you check on the rq->cmd_len but proceed to use
Boaz> the SCpnt->cmnd. Better ask for SCpnt->cmd_len, just for
Boaz> consistency
Well, I didn't actually mean to leave that BUG_ON in the hot path. In
any case we won't get this far if the cmd_len is not 32.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] sd: Support disks formatted with DIF Type 2
2009-08-26 6:18 ` [PATCH 4/5] sd: Support disks formatted with DIF Type 2 Martin K. Petersen
@ 2009-08-26 12:26 ` Boaz Harrosh
2009-08-27 6:41 ` Martin K. Petersen
0 siblings, 1 reply; 17+ messages in thread
From: Boaz Harrosh @ 2009-08-26 12:26 UTC (permalink / raw)
To: Martin K. Petersen
Cc: James.Bottomley, dgilbert, Ihab.Hamadi, James.Smart, linux-scsi
On 08/26/2009 09:18 AM, Martin K. Petersen wrote:
> Disks formatted with DIF Type 2 reject READ/WRITE 6/10/12/16 commands
> when protection is enabled. Only the 32-byte variants are supported.
>
> Implement support for issusing 32-byte READ/WRITE and enable support for
> Type 2 drives in the protection type detection logic.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
> drivers/scsi/sd.c | 55 ++++++++++++++++++++++++++++++++++++++-------------
> include/scsi/scsi.h | 3 ++
> 2 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 36e2333..a26371f 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -413,6 +413,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
> sector_t threshold;
> unsigned int this_count = blk_rq_sectors(rq);
> int ret, host_dif;
> + unsigned char protect;
>
> if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
> ret = scsi_setup_blk_pc_cmnd(sdp, rq);
> @@ -545,13 +546,41 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
> /* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
> host_dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
> if (host_dif)
> - SCpnt->cmnd[1] = 1 << 5;
> + protect = 1 << 5;
> else
> - SCpnt->cmnd[1] = 0;
> -
> - if (block > 0xffffffff) {
> + protect = 0;
> +
> + if (host_dif == SD_DIF_TYPE2_PROTECTION) {
> + BUG_ON(rq->cmd_len != SCSI_EXT_CDB_SIZE);
This is not nice you check on the rq->cmd_len but proceed to use
the SCpnt->cmnd. Better ask for SCpnt->cmd_len, just for consistency
> + SCpnt->cmnd[0] = VARIABLE_LENGTH_CMD;
> + SCpnt->cmnd[7] = 0x18;
> + SCpnt->cmnd[9] = (rq_data_dir(rq) == READ) ? READ_32 : WRITE_32;
> + SCpnt->cmnd[10] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
> +
> + /* LBA */
> + SCpnt->cmnd[12] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
> + SCpnt->cmnd[13] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
> + SCpnt->cmnd[14] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
> + SCpnt->cmnd[15] = sizeof(block) > 4 ? (unsigned char) (block >> 32) & 0xff : 0;
> + SCpnt->cmnd[16] = (unsigned char) (block >> 24) & 0xff;
> + SCpnt->cmnd[17] = (unsigned char) (block >> 16) & 0xff;
> + SCpnt->cmnd[18] = (unsigned char) (block >> 8) & 0xff;
> + SCpnt->cmnd[19] = (unsigned char) block & 0xff;
> +
> + /* Expected Indirect LBA */
> + SCpnt->cmnd[20] = (unsigned char) (block >> 24) & 0xff;
> + SCpnt->cmnd[21] = (unsigned char) (block >> 16) & 0xff;
> + SCpnt->cmnd[22] = (unsigned char) (block >> 8) & 0xff;
> + SCpnt->cmnd[23] = (unsigned char) block & 0xff;
> +
> + /* Transfer length */
> + SCpnt->cmnd[28] = (unsigned char) (this_count >> 24) & 0xff;
> + SCpnt->cmnd[29] = (unsigned char) (this_count >> 16) & 0xff;
> + SCpnt->cmnd[30] = (unsigned char) (this_count >> 8) & 0xff;
> + SCpnt->cmnd[31] = (unsigned char) this_count & 0xff;
> + } else if (block > 0xffffffff) {
> SCpnt->cmnd[0] += READ_16 - READ_6;
> - SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
> + SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
> SCpnt->cmnd[2] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
> SCpnt->cmnd[3] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
> SCpnt->cmnd[4] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
> @@ -572,7 +601,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
> this_count = 0xffff;
>
> SCpnt->cmnd[0] += READ_10 - READ_6;
> - SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
> + SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
> SCpnt->cmnd[2] = (unsigned char) (block >> 24) & 0xff;
> SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff;
> SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;
> @@ -1271,22 +1300,20 @@ void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
>
> sdkp->protection_type = type;
>
> - switch (type) {
> - case SD_DIF_TYPE1_PROTECTION:
> - case SD_DIF_TYPE3_PROTECTION:
> - break;
> -
> - default:
> + if (type > SD_DIF_TYPE3_PROTECTION) {
> sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
> "protection type %u. Disabling disk!\n", type);
> sdkp->capacity = 0;
> return;
> }
>
> - if (scsi_host_dif_capable(sdp->host, type))
> + if (scsi_host_dif_capable(sdp->host, type)) {
> sd_printk(KERN_NOTICE, sdkp,
> "Enabling DIF Type %u protection\n", type);
> - else
> +
> + if (type == SD_DIF_TYPE2_PROTECTION)
> + sdp->use_32_for_rw = 1;
> + } else
> sd_printk(KERN_NOTICE, sdkp,
> "Disabling DIF Type %u protection\n", type);
> }
> diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
> index a7ae10a..847de86 100644
> --- a/include/scsi/scsi.h
> +++ b/include/scsi/scsi.h
> @@ -129,6 +129,9 @@ struct scsi_cmnd;
> #define MI_REPORT_TARGET_PGS 0x0a
> /* values for maintenance out */
> #define MO_SET_TARGET_PGS 0x0a
> +/* values for variable length command */
> +#define READ_32 0x09
> +#define WRITE_32 0x0b
>
> /* Values for T10/04-262r7 */
> #define ATA_16 0x85 /* 16-byte pass-thru */
Boaz
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/5] sd: Support disks formatted with DIF Type 2
2009-08-26 6:17 DIF/DIX updates for 2.6.32 Martin K. Petersen
@ 2009-08-26 6:18 ` Martin K. Petersen
2009-08-26 12:26 ` Boaz Harrosh
0 siblings, 1 reply; 17+ messages in thread
From: Martin K. Petersen @ 2009-08-26 6:18 UTC (permalink / raw)
To: James.Bottomley, dgilbert, Ihab.Hamadi, James.Smart, linux-scsi
Cc: Martin K. Petersen
Disks formatted with DIF Type 2 reject READ/WRITE 6/10/12/16 commands
when protection is enabled. Only the 32-byte variants are supported.
Implement support for issusing 32-byte READ/WRITE and enable support for
Type 2 drives in the protection type detection logic.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
drivers/scsi/sd.c | 55 ++++++++++++++++++++++++++++++++++++++-------------
include/scsi/scsi.h | 3 ++
2 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 36e2333..a26371f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -413,6 +413,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
sector_t threshold;
unsigned int this_count = blk_rq_sectors(rq);
int ret, host_dif;
+ unsigned char protect;
if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
ret = scsi_setup_blk_pc_cmnd(sdp, rq);
@@ -545,13 +546,41 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
/* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
host_dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type);
if (host_dif)
- SCpnt->cmnd[1] = 1 << 5;
+ protect = 1 << 5;
else
- SCpnt->cmnd[1] = 0;
-
- if (block > 0xffffffff) {
+ protect = 0;
+
+ if (host_dif == SD_DIF_TYPE2_PROTECTION) {
+ BUG_ON(rq->cmd_len != SCSI_EXT_CDB_SIZE);
+ SCpnt->cmnd[0] = VARIABLE_LENGTH_CMD;
+ SCpnt->cmnd[7] = 0x18;
+ SCpnt->cmnd[9] = (rq_data_dir(rq) == READ) ? READ_32 : WRITE_32;
+ SCpnt->cmnd[10] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
+
+ /* LBA */
+ SCpnt->cmnd[12] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
+ SCpnt->cmnd[13] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
+ SCpnt->cmnd[14] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
+ SCpnt->cmnd[15] = sizeof(block) > 4 ? (unsigned char) (block >> 32) & 0xff : 0;
+ SCpnt->cmnd[16] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[17] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[18] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[19] = (unsigned char) block & 0xff;
+
+ /* Expected Indirect LBA */
+ SCpnt->cmnd[20] = (unsigned char) (block >> 24) & 0xff;
+ SCpnt->cmnd[21] = (unsigned char) (block >> 16) & 0xff;
+ SCpnt->cmnd[22] = (unsigned char) (block >> 8) & 0xff;
+ SCpnt->cmnd[23] = (unsigned char) block & 0xff;
+
+ /* Transfer length */
+ SCpnt->cmnd[28] = (unsigned char) (this_count >> 24) & 0xff;
+ SCpnt->cmnd[29] = (unsigned char) (this_count >> 16) & 0xff;
+ SCpnt->cmnd[30] = (unsigned char) (this_count >> 8) & 0xff;
+ SCpnt->cmnd[31] = (unsigned char) this_count & 0xff;
+ } else if (block > 0xffffffff) {
SCpnt->cmnd[0] += READ_16 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0;
SCpnt->cmnd[3] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0;
SCpnt->cmnd[4] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;
@@ -572,7 +601,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
this_count = 0xffff;
SCpnt->cmnd[0] += READ_10 - READ_6;
- SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
+ SCpnt->cmnd[1] = protect | (blk_fua_rq(rq) ? 0x8 : 0);
SCpnt->cmnd[2] = (unsigned char) (block >> 24) & 0xff;
SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff;
SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;
@@ -1271,22 +1300,20 @@ void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
sdkp->protection_type = type;
- switch (type) {
- case SD_DIF_TYPE1_PROTECTION:
- case SD_DIF_TYPE3_PROTECTION:
- break;
-
- default:
+ if (type > SD_DIF_TYPE3_PROTECTION) {
sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
"protection type %u. Disabling disk!\n", type);
sdkp->capacity = 0;
return;
}
- if (scsi_host_dif_capable(sdp->host, type))
+ if (scsi_host_dif_capable(sdp->host, type)) {
sd_printk(KERN_NOTICE, sdkp,
"Enabling DIF Type %u protection\n", type);
- else
+
+ if (type == SD_DIF_TYPE2_PROTECTION)
+ sdp->use_32_for_rw = 1;
+ } else
sd_printk(KERN_NOTICE, sdkp,
"Disabling DIF Type %u protection\n", type);
}
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index a7ae10a..847de86 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -129,6 +129,9 @@ struct scsi_cmnd;
#define MI_REPORT_TARGET_PGS 0x0a
/* values for maintenance out */
#define MO_SET_TARGET_PGS 0x0a
+/* values for variable length command */
+#define READ_32 0x09
+#define WRITE_32 0x0b
/* Values for T10/04-262r7 */
#define ATA_16 0x85 /* 16-byte pass-thru */
--
1.6.0.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2009-09-20 20:49 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-11 19:20 DIF/DIX updates for 2.6.32 Martin K. Petersen
2009-09-11 19:20 ` [PATCH 1/5] scsi: Deprecate SCSI_PROT_*_CONVERT operations Martin K. Petersen
2009-09-18 23:16 ` James Smart
2009-09-11 19:20 ` [PATCH 2/5] sd: Detach DIF from block integrity infrastructure Martin K. Petersen
2009-09-11 19:20 ` [PATCH 3/5] scsi: Fix protection scsi_data_buffer leak Martin K. Petersen
2009-09-13 9:36 ` Boaz Harrosh
2009-09-11 19:20 ` [PATCH 4/5] sd: Support disks formatted with DIF Type 2 Martin K. Petersen
2009-09-13 9:37 ` Boaz Harrosh
2009-09-11 19:20 ` [PATCH 5/5] scsi_debug: Implement support for " Martin K. Petersen
2009-09-11 23:06 ` Douglas Gilbert
-- strict thread matches above, loose matches on Subject: below --
2009-09-18 21:32 Final DIF/DIX patches for 2.6.32 Martin K. Petersen
2009-09-18 21:33 ` [PATCH 4/5] sd: Support disks formatted with DIF Type 2 Martin K. Petersen
2009-09-18 21:57 ` James Bottomley
2009-09-20 20:49 ` Martin K. Petersen
2009-09-04 8:36 DIF/DIX updates for 2.6.32 Martin K. Petersen
2009-09-04 8:36 ` [PATCH 4/5] sd: Support disks formatted with DIF Type 2 Martin K. Petersen
2009-08-26 6:17 DIF/DIX updates for 2.6.32 Martin K. Petersen
2009-08-26 6:18 ` [PATCH 4/5] sd: Support disks formatted with DIF Type 2 Martin K. Petersen
2009-08-26 12:26 ` Boaz Harrosh
2009-08-27 6:41 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.