* 32-byte CDB support in Libsas? @ 2013-05-06 18:41 Kamaljit Singh 2013-05-06 19:16 ` James Bottomley 0 siblings, 1 reply; 6+ messages in thread From: Kamaljit Singh @ 2013-05-06 18:41 UTC (permalink / raw) To: James Bottomley, Anand Kumar Santhanam Cc: linux-scsi, Harry Yang, Vishwanath Maram, sakthivel.sk, Rich Bono, Sangeetha Gnanasekaran Hello, Are there any plans to add 32-byte CDB support in Libsas? Thanks, Kamaljit Singh ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 32-byte CDB support in Libsas? 2013-05-06 18:41 32-byte CDB support in Libsas? Kamaljit Singh @ 2013-05-06 19:16 ` James Bottomley [not found] ` <CAD+HZHUynKmSgebmoLfPNgfbj8OVGaTUuFhJoJhNJxfvsNUFvg@mail.gmail.com> 0 siblings, 1 reply; 6+ messages in thread From: James Bottomley @ 2013-05-06 19:16 UTC (permalink / raw) To: Kamaljit Singh Cc: Anand Kumar Santhanam, linux-scsi, Harry Yang, Vishwanath Maram, sakthivel.sk, Rich Bono, Sangeetha Gnanasekaran On Mon, 2013-05-06 at 11:41 -0700, Kamaljit Singh wrote: > Are there any plans to add 32-byte CDB support in Libsas? I don't understand the question ... it should just work; is it broken? James ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CAD+HZHUynKmSgebmoLfPNgfbj8OVGaTUuFhJoJhNJxfvsNUFvg@mail.gmail.com>]
* RE: 32-byte CDB support in Libsas? [not found] ` <CAD+HZHUynKmSgebmoLfPNgfbj8OVGaTUuFhJoJhNJxfvsNUFvg@mail.gmail.com> @ 2013-05-07 19:46 ` Kamaljit Singh 2013-05-07 20:06 ` James Bottomley 0 siblings, 1 reply; 6+ messages in thread From: Kamaljit Singh @ 2013-05-07 19:46 UTC (permalink / raw) To: Jack Wang, James Bottomley Cc: Anand Kumar Santhanam, linux-scsi, Harry Yang, Vishwanath Maram, Sakthivel Saravanan Kamal Raju, Rich Bono, Sangeetha Gnanasekaran James, Jack, Thanks for your responses. > I don't understand the question ... it should just work; is it broken? sas_create_task() only copies 16 bytes to the sas_ssp_task struct’s cdb field, which itself is a 16 byte wide array. So it looks like sas_queuecommand() doesn’t support a 32-byte cdb. > It's easy to add that, define cdb 32 to ssp_task and modify sas_queuecommand, and lldd to pass it to fw Yes, that’s exactly what I’ve designed it as, i.e. in my lldd’s queuecommand function. At the moment I only need this function for 32-byte cdb support. But if there are plans to add that to libsas then I’d prefer to use sas_queuecommand rather than having to do manual merges from future libsas fixes/updates. > I wonder any user ask for cdb 32 support No. Just testing right now. Thanks, Kamaljit From: Jack Wang [mailto:xjtuwjp@gmail.com] Sent: Monday, May 06, 2013 12:38 PM To: James Bottomley Cc: Kamaljit Singh; Anand Kumar Santhanam; linux-scsi@vger.kernel.org; Harry Yang; Vishwanath Maram; Sakthivel Saravanan Kamal Raju; Rich Bono; Sangeetha Gnanasekaran Subject: Re: 32-byte CDB support in Libsas? It's easy to add that, define cdb 32 to ssp_task and modify sas_queuecommand, and lldd to pass it to fw. I wonder any user ask for cdb 32 support? Regards Jack 2013/5/6 James Bottomley <James.Bottomley@hansenpartnership.com> On Mon, 2013-05-06 at 11:41 -0700, Kamaljit Singh wrote: > Are there any plans to add 32-byte CDB support in Libsas? I don't understand the question ... it should just work; is it broken? James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 32-byte CDB support in Libsas? 2013-05-07 19:46 ` Kamaljit Singh @ 2013-05-07 20:06 ` James Bottomley 2013-05-07 20:26 ` Kamaljit Singh 0 siblings, 1 reply; 6+ messages in thread From: James Bottomley @ 2013-05-07 20:06 UTC (permalink / raw) To: Kamaljit Singh Cc: Jack Wang, Anand Kumar Santhanam, linux-scsi, Harry Yang, Vishwanath Maram, Sakthivel Saravanan Kamal Raju, Rich Bono, Sangeetha Gnanasekaran On Tue, 2013-05-07 at 12:46 -0700, Kamaljit Singh wrote: > James, Jack, > > Thanks for your responses. > > > I don't understand the question ... it should just work; is it > broken? > sas_create_task() only copies 16 bytes to the sas_ssp_task struct’s > cdb field, which itself is a 16 byte wide array. So it looks like > sas_queuecommand() doesn’t support a 32-byte cdb. Hm, it does it as a bare 16 instead of SCSI_MAX_CDB which is probably how it got missed in the long command conversion. > > It's easy to add that, define cdb 32 to ssp_task and modify > sas_queuecommand, and lldd to pass it to fw > Yes, that’s exactly what I’ve designed it as, i.e. in my lldd’s > queuecommand function. At the moment I only need this function for > 32-byte cdb support. But if there are plans to add that to libsas > then I’d prefer to use sas_queuecommand rather than having to do > manual merges from future libsas fixes/updates. I wouldn't do it that way. Since everything has to form a command IU anyway by copying, I'd just make sas_ssp->cdb a pointer to the actual command instead of an array of the command; if it points to the scsi_cmnd, then it has access to cmd_len as well. That way no copying and no problem when anyone wants longer commands. There should probably also be a macro to populate the command IU since the format of CDB > 16 is different. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: 32-byte CDB support in Libsas? 2013-05-07 20:06 ` James Bottomley @ 2013-05-07 20:26 ` Kamaljit Singh 2013-05-07 22:38 ` James Bottomley 0 siblings, 1 reply; 6+ messages in thread From: Kamaljit Singh @ 2013-05-07 20:26 UTC (permalink / raw) To: James Bottomley Cc: Jack Wang, Anand Kumar Santhanam, linux-scsi, Harry Yang, Vishwanath Maram, Sakthivel Saravanan Kamal Raju, Rich Bono, Sangeetha Gnanasekaran > I wouldn't do it that way. Since everything has to form a command IU > anyway by copying, I'd just make sas_ssp->cdb a pointer to the actual > command instead of an array of the command; if it points to the I'd prefer that too, specially since scsi_cmnd->cmnd is already a pointer. I considered that but any such changes to libsas.h would've had wide repercussions. Hence, I didn't go down that rathole at the moment. > no problem when anyone wants longer commands. There should probably >also be a macro to populate the command IU since the format of CDB > 16 > is different. Thanks for recongnizing that. Commands >32 bytes are supported by PMC's SAS Controllers. Kamaljit -----Original Message----- From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com] Sent: Tuesday, May 07, 2013 1:07 PM To: Kamaljit Singh Cc: Jack Wang; Anand Kumar Santhanam; linux-scsi@vger.kernel.org; Harry Yang; Vishwanath Maram; Sakthivel Saravanan Kamal Raju; Rich Bono; Sangeetha Gnanasekaran Subject: Re: 32-byte CDB support in Libsas? On Tue, 2013-05-07 at 12:46 -0700, Kamaljit Singh wrote: > James, Jack, > > Thanks for your responses. > > > I don't understand the question ... it should just work; is it > broken? > sas_create_task() only copies 16 bytes to the sas_ssp_task struct’s > cdb field, which itself is a 16 byte wide array. So it looks like > sas_queuecommand() doesn’t support a 32-byte cdb. Hm, it does it as a bare 16 instead of SCSI_MAX_CDB which is probably how it got missed in the long command conversion. > > It's easy to add that, define cdb 32 to ssp_task and modify > sas_queuecommand, and lldd to pass it to fw > Yes, that’s exactly what I’ve designed it as, i.e. in my lldd’s > queuecommand function. At the moment I only need this function for > 32-byte cdb support. But if there are plans to add that to libsas > then I’d prefer to use sas_queuecommand rather than having to do > manual merges from future libsas fixes/updates. I wouldn't do it that way. Since everything has to form a command IU anyway by copying, I'd just make sas_ssp->cdb a pointer to the actual command instead of an array of the command; if it points to the scsi_cmnd, then it has access to cmd_len as well. That way no copying and no problem when anyone wants longer commands. There should probably also be a macro to populate the command IU since the format of CDB > 16 is different. James ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 32-byte CDB support in Libsas? 2013-05-07 20:26 ` Kamaljit Singh @ 2013-05-07 22:38 ` James Bottomley 0 siblings, 0 replies; 6+ messages in thread From: James Bottomley @ 2013-05-07 22:38 UTC (permalink / raw) To: Kamaljit Singh Cc: Jack Wang, Anand Kumar Santhanam, linux-scsi, Harry Yang, Vishwanath Maram, Sakthivel Saravanan Kamal Raju, Rich Bono, Sangeetha Gnanasekaran On Tue, 2013-05-07 at 13:26 -0700, Kamaljit Singh wrote: > > I wouldn't do it that way. Since everything has to form a command IU > > anyway by copying, I'd just make sas_ssp->cdb a pointer to the actual > > command instead of an array of the command; if it points to the > I'd prefer that too, specially since scsi_cmnd->cmnd is already a pointer. I considered that but any such changes to libsas.h would've had wide repercussions. Hence, I didn't go down that rathole at the moment. Actually, the enabling is pretty trivial. I'll push this into the next merge window (so for 3.11) if there are no objections. This still doesn't mean we have any SAS driver that will send > 16 byte commands because they would need to alter Scsi_Host.max_cmd_len and cope with the different format, but at least it's now a driver issue. James --- drivers/scsi/aic94xx/aic94xx_task.c | 3 ++- drivers/scsi/isci/request.c | 4 ++-- drivers/scsi/libsas/sas_scsi_host.c | 2 +- drivers/scsi/mvsas/mv_sas.c | 3 ++- drivers/scsi/pm8001/pm8001_hwi.c | 3 ++- drivers/scsi/pm8001/pm80xx_hwi.c | 21 +++++++++++---------- include/scsi/libsas.h | 2 +- 7 files changed, 21 insertions(+), 17 deletions(-) diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c index 393e7ce..59b86e2 100644 --- a/drivers/scsi/aic94xx/aic94xx_task.c +++ b/drivers/scsi/aic94xx/aic94xx_task.c @@ -505,7 +505,8 @@ static int asd_build_ssp_ascb(struct asd_ascb *ascb, struct sas_task *task, scb->ssp_task.ssp_cmd.efb_prio_attr |= EFB_MASK; scb->ssp_task.ssp_cmd.efb_prio_attr |= (task->ssp_task.task_prio << 3); scb->ssp_task.ssp_cmd.efb_prio_attr |= (task->ssp_task.task_attr & 7); - memcpy(scb->ssp_task.ssp_cmd.cdb, task->ssp_task.cdb, 16); + memcpy(scb->ssp_task.ssp_cmd.cdb, task->ssp_task.cmd->cmnd, + task->ssp_task.cmd->cmd_len); scb->ssp_task.sister_scb = cpu_to_le16(0xFFFF); scb->ssp_task.conn_handle = cpu_to_le16( diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c index e3e3bcb..7b08215 100644 --- a/drivers/scsi/isci/request.c +++ b/drivers/scsi/isci/request.c @@ -184,8 +184,8 @@ static void sci_io_request_build_ssp_command_iu(struct isci_request *ireq) cmd_iu->task_attr = task->ssp_task.task_attr; cmd_iu->_r_c = 0; - sci_swab32_cpy(&cmd_iu->cdb, task->ssp_task.cdb, - sizeof(task->ssp_task.cdb) / sizeof(u32)); + sci_swab32_cpy(&cmd_iu->cdb, task->ssp_task.cmd->cmnd, + task->ssp_task.cmd->cmd_len / sizeof(u32)); } static void sci_task_request_build_ssp_task_iu(struct isci_request *ireq) diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c index 6e795a1..da3aee1 100644 --- a/drivers/scsi/libsas/sas_scsi_host.c +++ b/drivers/scsi/libsas/sas_scsi_host.c @@ -167,7 +167,7 @@ static struct sas_task *sas_create_task(struct scsi_cmnd *cmd, int_to_scsilun(cmd->device->lun, &lun); memcpy(task->ssp_task.LUN, &lun.scsi_lun, 8); task->ssp_task.task_attr = TASK_ATTR_SIMPLE; - memcpy(task->ssp_task.cdb, cmd->cmnd, 16); + task->ssp_task.cmd = cmd; task->scatter = scsi_sglist(cmd); task->num_scatter = scsi_sg_count(cmd); diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c index c9e2449..f14665a 100644 --- a/drivers/scsi/mvsas/mv_sas.c +++ b/drivers/scsi/mvsas/mv_sas.c @@ -686,7 +686,8 @@ static int mvs_task_prep_ssp(struct mvs_info *mvi, if (ssp_hdr->frame_type != SSP_TASK) { buf_cmd[9] = fburst | task->ssp_task.task_attr | (task->ssp_task.task_prio << 3); - memcpy(buf_cmd + 12, &task->ssp_task.cdb, 16); + memcpy(buf_cmd + 12, task->ssp_task.cmd->cmnd, + task->ssp_task.cmd->cmd_len); } else{ buf_cmd[10] = tmf->tmf; switch (tmf->tmf) { diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c index 69dd49c..a58546f 100644 --- a/drivers/scsi/pm8001/pm8001_hwi.c +++ b/drivers/scsi/pm8001/pm8001_hwi.c @@ -4291,7 +4291,8 @@ static int pm8001_chip_ssp_io_req(struct pm8001_hba_info *pm8001_ha, ssp_cmd.ssp_iu.efb_prio_attr |= 0x80; ssp_cmd.ssp_iu.efb_prio_attr |= (task->ssp_task.task_prio << 3); ssp_cmd.ssp_iu.efb_prio_attr |= (task->ssp_task.task_attr & 7); - memcpy(ssp_cmd.ssp_iu.cdb, task->ssp_task.cdb, 16); + memcpy(ssp_cmd.ssp_iu.cdb, task->ssp_task.cmd->cmnd, + task->ssp_task.cmd->cmd_len); circularQ = &pm8001_ha->inbnd_q_tbl[0]; /* fill in PRD (scatter/gather) table, if any */ diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 302514d..f6c65ee 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -3559,9 +3559,9 @@ err_out: static int check_enc_sas_cmd(struct sas_task *task) { - if ((task->ssp_task.cdb[0] == READ_10) - || (task->ssp_task.cdb[0] == WRITE_10) - || (task->ssp_task.cdb[0] == WRITE_VERIFY)) + u8 cmd = task->ssp_task.cmd->cmnd[0]; + + if (cmd == READ_10 || cmd == WRITE_10 || cmd == WRITE_VERIFY) return 1; else return 0; @@ -3624,7 +3624,8 @@ static int pm80xx_chip_ssp_io_req(struct pm8001_hba_info *pm8001_ha, ssp_cmd.ssp_iu.efb_prio_attr |= 0x80; ssp_cmd.ssp_iu.efb_prio_attr |= (task->ssp_task.task_prio << 3); ssp_cmd.ssp_iu.efb_prio_attr |= (task->ssp_task.task_attr & 7); - memcpy(ssp_cmd.ssp_iu.cdb, task->ssp_task.cdb, 16); + memcpy(ssp_cmd.ssp_iu.cdb, task->ssp_task.cmd->cmnd, + task->ssp_task.cmd->cmd_len); circularQ = &pm8001_ha->inbnd_q_tbl[0]; /* Check if encryption is set */ @@ -3632,7 +3633,7 @@ static int pm80xx_chip_ssp_io_req(struct pm8001_hba_info *pm8001_ha, !(pm8001_ha->encrypt_info.status) && check_enc_sas_cmd(task)) { PM8001_IO_DBG(pm8001_ha, pm8001_printk( "Encryption enabled.Sending Encrypt SAS command 0x%x\n", - task->ssp_task.cdb[0])); + task->ssp_task.cmd->cmnd[0])); opc = OPC_INB_SSP_INI_DIF_ENC_IO; /* enable encryption. 0 for SAS 1.1 and SAS 2.0 compatible TLR*/ ssp_cmd.dad_dir_m_tlr = cpu_to_le32 @@ -3666,14 +3667,14 @@ static int pm80xx_chip_ssp_io_req(struct pm8001_hba_info *pm8001_ha, /* XTS mode. All other fields are 0 */ ssp_cmd.key_cmode = 0x6 << 4; /* set tweak values. Should be the start lba */ - ssp_cmd.twk_val0 = cpu_to_le32((task->ssp_task.cdb[2] << 24) | - (task->ssp_task.cdb[3] << 16) | - (task->ssp_task.cdb[4] << 8) | - (task->ssp_task.cdb[5])); + ssp_cmd.twk_val0 = cpu_to_le32((task->ssp_task.cmd->cmnd[2] << 24) | + (task->ssp_task.cmd->cmnd[3] << 16) | + (task->ssp_task.cmd->cmnd[4] << 8) | + (task->ssp_task.cmd->cmnd[5])); } else { PM8001_IO_DBG(pm8001_ha, pm8001_printk( "Sending Normal SAS command 0x%x inb q %x\n", - task->ssp_task.cdb[0], inb)); + task->ssp_task.cmd->cmnd[0], inb)); /* fill in PRD (scatter/gather) table, if any */ if (task->num_scatter > 1) { pm8001_chip_make_sg(task->scatter, ccb->n_elem, diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h index e2c1e66..f843dd8 100644 --- a/include/scsi/libsas.h +++ b/include/scsi/libsas.h @@ -608,7 +608,7 @@ struct sas_ssp_task { u8 enable_first_burst:1; enum task_attribute task_attr; u8 task_prio; - u8 cdb[16]; + struct scsi_cmnd *cmd; }; struct sas_task { ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-07 22:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-06 18:41 32-byte CDB support in Libsas? Kamaljit Singh
2013-05-06 19:16 ` James Bottomley
[not found] ` <CAD+HZHUynKmSgebmoLfPNgfbj8OVGaTUuFhJoJhNJxfvsNUFvg@mail.gmail.com>
2013-05-07 19:46 ` Kamaljit Singh
2013-05-07 20:06 ` James Bottomley
2013-05-07 20:26 ` Kamaljit Singh
2013-05-07 22:38 ` James Bottomley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox