| Message ID | 20230804071944.27214-3-njavali@marvell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | qla2xxx: allow 32 bytes CDB | expand |
On Fri, 2023-08-04 at 12:49 +0530, Nilesh Javali wrote: > From: Quinn Tran <qutran@marvell.com> > > System crash when 32 bytes CDB was sent to a non T10-PI disk, > > [ 177.143279] ? qla2xxx_dif_start_scsi_mq+0xcd8/0xce0 [qla2xxx] > [ 177.149165] ? internal_add_timer+0x42/0x70 > [ 177.153372] qla2xxx_mqueuecommand+0x207/0x2b0 [qla2xxx] > [ 177.158730] scsi_queue_rq+0x2b7/0xc00 > [ 177.162501] blk_mq_dispatch_rq_list+0x3ea/0x7e0 > > Current code attempt to use CRC IOCB to send the command but failed. > Instead, type 6 IOCB should be used to send the IO. > > Clone existing type 6 IOCB code with addition of MQ support > to allow 32 bytes CDB to go through. > > Signed-off-by: Quinn Tran <qutran@marvell.com> > Cc: Laurence Oberman <loberman@redhat.com> > Cc: stable@vger.kernel.org > Signed-off-by: Nilesh Javali <njavali@marvell.com> > --- > drivers/scsi/qla2xxx/qla_iocb.c | 270 > ++++++++++++++++++++++++++++++++ > drivers/scsi/qla2xxx/qla_nx.h | 4 +- > 2 files changed, 273 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c > b/drivers/scsi/qla2xxx/qla_iocb.c > index 0caa64a7df26..e99ebf7e1c7a 100644 > --- a/drivers/scsi/qla2xxx/qla_iocb.c > +++ b/drivers/scsi/qla2xxx/qla_iocb.c > @@ -11,6 +11,7 @@ > > #include <scsi/scsi_tcq.h> > > +static int qla_start_scsi_type6(srb_t *sp); > /** > * qla2x00_get_cmd_direction() - Determine control_flag data > direction. > * @sp: SCSI command > @@ -1721,6 +1722,8 @@ qla24xx_dif_start_scsi(srb_t *sp) > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > if (cmd->cmd_len <= 16) > return qla24xx_start_scsi(sp); > + else > + return qla_start_scsi_type6(sp); > } > > /* Setup device pointers. */ > @@ -2100,6 +2103,8 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp) > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > if (cmd->cmd_len <= 16) > return qla2xxx_start_scsi_mq(sp); > + else > + return qla_start_scsi_type6(sp); > } > > spin_lock_irqsave(&qpair->qp_lock, flags); > @@ -4205,3 +4210,268 @@ qla2x00_start_bidir(srb_t *sp, struct > scsi_qla_host *vha, uint32_t tot_dsds) > > return rval; > } > + > +/** > + * qla_start_scsi_type6() - Send a SCSI command to the ISP > + * @sp: command to send to the ISP > + * > + * Returns non-zero if a failure occurred, else zero. > + */ > +static int > +qla_start_scsi_type6(srb_t *sp) > +{ > + int nseg; > + unsigned long flags; > + uint32_t *clr_ptr; > + uint32_t handle; > + struct cmd_type_6 *cmd_pkt; > + uint16_t cnt; > + uint16_t req_cnt; > + uint16_t tot_dsds; > + struct req_que *req = NULL; > + struct rsp_que *rsp; > + struct scsi_cmnd *cmd = GET_CMD_SP(sp); > + struct scsi_qla_host *vha = sp->fcport->vha; > + struct qla_hw_data *ha = vha->hw; > + struct qla_qpair *qpair = sp->qpair; > + uint16_t more_dsd_lists = 0; > + struct dsd_dma *dsd_ptr; > + uint16_t i; > + __be32 *fcp_dl; > + uint8_t additional_cdb_len; > + struct ct6_dsd *ctx; > + > + > + /* Acquire qpair specific lock */ > + spin_lock_irqsave(&qpair->qp_lock, flags); > + > + /* Setup qpair pointers */ > + req = qpair->req; > + rsp = qpair->rsp; > + > + /* So we know we haven't pci_map'ed anything yet */ > + tot_dsds = 0; > + > + /* Send marker if required */ > + if (vha->marker_needed != 0) { > + if (__qla2x00_marker(vha, qpair, 0, 0, MK_SYNC_ALL) > != QLA_SUCCESS) { > + spin_unlock_irqrestore(&qpair->qp_lock, > flags); > + return QLA_FUNCTION_FAILED; > + } > + vha->marker_needed = 0; > + } > + > + handle = qla2xxx_get_next_handle(req); > + if (handle == 0) > + goto queuing_error; > + > + /* Map the sg table so we have an accurate count of sg > entries needed */ > + if (scsi_sg_count(cmd)) { > + nseg = dma_map_sg(&ha->pdev->dev, scsi_sglist(cmd), > + scsi_sg_count(cmd), cmd- > >sc_data_direction); > + if (unlikely(!nseg)) > + goto queuing_error; > + } else { > + nseg = 0; > + } > + > + tot_dsds = nseg; > + > + /* eventhough driver only need 1 T6 IOCB, FW still convert > DSD to Continueation IOCB */ > + req_cnt = qla24xx_calc_iocbs(vha, tot_dsds); > + > + sp->iores.res_type = RESOURCE_IOCB | RESOURCE_EXCH; > + sp->iores.exch_cnt = 1; > + sp->iores.iocb_cnt = req_cnt; > + > + if (qla_get_fw_resources(sp->qpair, &sp->iores)) > + goto queuing_error; > + > + more_dsd_lists = qla24xx_calc_dsd_lists(tot_dsds); > + if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { > + ql_dbg(ql_dbg_io, vha, 0x3028, > + "Num of DSD list %d is than %d for cmd=%p.\n", > + more_dsd_lists + qpair->dsd_inuse, > NUM_DSD_CHAIN, cmd); > + goto queuing_error; > + } > + > + if (more_dsd_lists <= qpair->dsd_avail) > + goto sufficient_dsds; > + else > + more_dsd_lists -= qpair->dsd_avail; > + > + for (i = 0; i < more_dsd_lists; i++) { > + dsd_ptr = kzalloc(sizeof(*dsd_ptr), GFP_ATOMIC); > + if (!dsd_ptr) { > + ql_log(ql_log_fatal, vha, 0x3029, > + "Failed to allocate memory for dsd_dma > for cmd=%p.\n", cmd); > + goto queuing_error; > + } > + INIT_LIST_HEAD(&dsd_ptr->list); > + > + dsd_ptr->dsd_addr = dma_pool_alloc(ha->dl_dma_pool, > + GFP_ATOMIC, &dsd_ptr->dsd_list_dma); > + if (!dsd_ptr->dsd_addr) { > + kfree(dsd_ptr); > + ql_log(ql_log_fatal, vha, 0x302a, > + "Failed to allocate memory for dsd_addr > for cmd=%p.\n", cmd); > + goto queuing_error; > + } > + list_add_tail(&dsd_ptr->list, &qpair->dsd_list); > + qpair->dsd_avail++; > + } > + > +sufficient_dsds: > + req_cnt = 1; > + > + if (req->cnt < (req_cnt + 2)) { > + if (IS_SHADOW_REG_CAPABLE(ha)) { > + cnt = *req->out_ptr; > + } else { > + cnt = (uint16_t)rd_reg_dword_relaxed(req- > >req_q_out); > + if (qla2x00_check_reg16_for_disconnect(vha, > cnt)) > + goto queuing_error; > + } > + > + if (req->ring_index < cnt) > + req->cnt = cnt - req->ring_index; > + else > + req->cnt = req->length - (req->ring_index - > cnt); > + if (req->cnt < (req_cnt + 2)) > + goto queuing_error; > + } > + > + ctx = &sp->u.scmd.ct6_ctx; > + > + memset(ctx, 0, sizeof(struct ct6_dsd)); > + ctx->fcp_cmnd = dma_pool_zalloc(ha->fcp_cmnd_dma_pool, > + GFP_ATOMIC, &ctx->fcp_cmnd_dma); > + if (!ctx->fcp_cmnd) { > + ql_log(ql_log_fatal, vha, 0x3031, > + "Failed to allocate fcp_cmnd for cmd=%p.\n", > cmd); > + goto queuing_error; > + } > + > + /* Initialize the DSD list and dma handle */ > + INIT_LIST_HEAD(&ctx->dsd_list); > + ctx->dsd_use_cnt = 0; > + > + if (cmd->cmd_len > 16) { > + additional_cdb_len = cmd->cmd_len - 16; > + if (cmd->cmd_len % 4 || > + cmd->cmd_len > QLA_CDB_BUF_SIZE) { > + /* SCSI command bigger than 16 bytes must be > + * multiple of 4 or too big. > + */ > + ql_log(ql_log_warn, vha, 0x3033, > + "scsi cmd len %d not multiple of 4 for > cmd=%p.\n", > + cmd->cmd_len, cmd); > + goto queuing_error_fcp_cmnd; > + } > + ctx->fcp_cmnd_len = 12 + cmd->cmd_len + 4; > + } else { > + additional_cdb_len = 0; > + ctx->fcp_cmnd_len = 12 + 16 + 4; > + } > + > + /* Build command packet. */ > + req->current_outstanding_cmd = handle; > + req->outstanding_cmds[handle] = sp; > + sp->handle = handle; > + cmd->host_scribble = (unsigned char *)(unsigned long)handle; > + req->cnt -= req_cnt; > + > + cmd_pkt = (struct cmd_type_6 *)req->ring_ptr; > + cmd_pkt->handle = make_handle(req->id, handle); > + > + /* Zero out remaining portion of packet. */ > + /* tagged queuing modifier -- default is TSK_SIMPLE (0). > */ > + clr_ptr = (uint32_t *)cmd_pkt + 2; > + memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); > + cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); > + > + /* Set NPORT-ID and LUN number*/ > + cmd_pkt->nport_handle = cpu_to_le16(sp->fcport->loop_id); > + cmd_pkt->port_id[0] = sp->fcport->d_id.b.al_pa; > + cmd_pkt->port_id[1] = sp->fcport->d_id.b.area; > + cmd_pkt->port_id[2] = sp->fcport->d_id.b.domain; > + cmd_pkt->vp_index = sp->vha->vp_idx; > + > + /* Build IOCB segments */ > + qla24xx_build_scsi_type_6_iocbs(sp, cmd_pkt, tot_dsds); > + > + int_to_scsilun(cmd->device->lun, &cmd_pkt->lun); > + host_to_fcp_swap((uint8_t *)&cmd_pkt->lun, sizeof(cmd_pkt- > >lun)); > + > + /* build FCP_CMND IU */ > + int_to_scsilun(cmd->device->lun, &ctx->fcp_cmnd->lun); > + ctx->fcp_cmnd->additional_cdb_len = additional_cdb_len; > + > + if (cmd->sc_data_direction == DMA_TO_DEVICE) > + ctx->fcp_cmnd->additional_cdb_len |= 1; > + else if (cmd->sc_data_direction == DMA_FROM_DEVICE) > + ctx->fcp_cmnd->additional_cdb_len |= 2; > + > + /* Populate the FCP_PRIO. */ > + if (ha->flags.fcp_prio_enabled) > + ctx->fcp_cmnd->task_attribute |= > + sp->fcport->fcp_prio << 3; > + > + memcpy(ctx->fcp_cmnd->cdb, cmd->cmnd, cmd->cmd_len); > + > + fcp_dl = (__be32 *)(ctx->fcp_cmnd->cdb + 16 + > + additional_cdb_len); > + *fcp_dl = htonl((uint32_t)scsi_bufflen(cmd)); > + > + cmd_pkt->fcp_cmnd_dseg_len = cpu_to_le16(ctx->fcp_cmnd_len); > + put_unaligned_le64(ctx->fcp_cmnd_dma, > + &cmd_pkt->fcp_cmnd_dseg_address); > + > + sp->flags |= SRB_FCP_CMND_DMA_VALID; > + cmd_pkt->byte_count = > cpu_to_le32((uint32_t)scsi_bufflen(cmd)); > + /* Set total data segment count. */ > + cmd_pkt->entry_count = (uint8_t)req_cnt; > + > + wmb(); > + /* Adjust ring index. */ > + req->ring_index++; > + if (req->ring_index == req->length) { > + req->ring_index = 0; > + req->ring_ptr = req->ring; > + } else { > + req->ring_ptr++; > + } > + > + sp->qpair->cmd_cnt++; > + sp->flags |= SRB_DMA_VALID; > + > + /* Set chip new ring index. */ > + wrt_reg_dword(req->req_q_in, req->ring_index); > + > + /* Manage unprocessed RIO/ZIO commands in response queue. */ > + if (vha->flags.process_response_queue && > + rsp->ring_ptr->signature != RESPONSE_PROCESSED) > + qla24xx_process_response_queue(vha, rsp); > + > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > + > + return QLA_SUCCESS; > + > +queuing_error_fcp_cmnd: > + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx->fcp_cmnd, ctx- > >fcp_cmnd_dma); > + > +queuing_error: > + if (tot_dsds) > + scsi_dma_unmap(cmd); > + > + qla_put_fw_resources(sp->qpair, &sp->iores); > + > + if (sp->u.scmd.crc_ctx) { > + mempool_free(sp->u.scmd.crc_ctx, ha->ctx_mempool); > + sp->u.scmd.crc_ctx = NULL; > + } > + > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > + > + return QLA_FUNCTION_FAILED; > +} > diff --git a/drivers/scsi/qla2xxx/qla_nx.h > b/drivers/scsi/qla2xxx/qla_nx.h > index 6dc80c8ddf79..5d1bdc15b75c 100644 > --- a/drivers/scsi/qla2xxx/qla_nx.h > +++ b/drivers/scsi/qla2xxx/qla_nx.h > @@ -857,7 +857,9 @@ struct fcp_cmnd { > uint8_t task_attribute; > uint8_t task_management; > uint8_t additional_cdb_len; > - uint8_t cdb[260]; /* 256 for CDB len and 4 for FCP_DL */ > +#define QLA_CDB_BUF_SIZE 256 > +#define QLA_FCP_DL_SIZE 4 > + uint8_t cdb[QLA_CDB_BUF_SIZE + QLA_FCP_DL_SIZE]; /* 256 for > CDB len and 4 for FCP_DL */ > }; > > struct dsd_dma { QT and Nilesh, Thank you. I need more time to look at this fully but I will get it tested in the meantime. I will reply with a review or questions later. Thanks for sending this Laurence
On Fri, 2023-08-04 at 12:58 -0400, Laurence Oberman wrote: > On Fri, 2023-08-04 at 12:49 +0530, Nilesh Javali wrote: > > From: Quinn Tran <qutran@marvell.com> > > > > System crash when 32 bytes CDB was sent to a non T10-PI disk, > > > > [ 177.143279] ? qla2xxx_dif_start_scsi_mq+0xcd8/0xce0 [qla2xxx] > > [ 177.149165] ? internal_add_timer+0x42/0x70 > > [ 177.153372] qla2xxx_mqueuecommand+0x207/0x2b0 [qla2xxx] > > [ 177.158730] scsi_queue_rq+0x2b7/0xc00 > > [ 177.162501] blk_mq_dispatch_rq_list+0x3ea/0x7e0 > > > > Current code attempt to use CRC IOCB to send the command but > > failed. > > Instead, type 6 IOCB should be used to send the IO. > > > > Clone existing type 6 IOCB code with addition of MQ support > > to allow 32 bytes CDB to go through. > > > > Signed-off-by: Quinn Tran <qutran@marvell.com> > > Cc: Laurence Oberman <loberman@redhat.com> > > Cc: stable@vger.kernel.org > > Signed-off-by: Nilesh Javali <njavali@marvell.com> > > --- > > drivers/scsi/qla2xxx/qla_iocb.c | 270 > > ++++++++++++++++++++++++++++++++ > > drivers/scsi/qla2xxx/qla_nx.h | 4 +- > > 2 files changed, 273 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c > > b/drivers/scsi/qla2xxx/qla_iocb.c > > index 0caa64a7df26..e99ebf7e1c7a 100644 > > --- a/drivers/scsi/qla2xxx/qla_iocb.c > > +++ b/drivers/scsi/qla2xxx/qla_iocb.c > > @@ -11,6 +11,7 @@ > > > > #include <scsi/scsi_tcq.h> > > > > +static int qla_start_scsi_type6(srb_t *sp); > > /** > > * qla2x00_get_cmd_direction() - Determine control_flag data > > direction. > > * @sp: SCSI command > > @@ -1721,6 +1722,8 @@ qla24xx_dif_start_scsi(srb_t *sp) > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > if (cmd->cmd_len <= 16) > > return qla24xx_start_scsi(sp); > > + else > > + return qla_start_scsi_type6(sp); > > } > > > > /* Setup device pointers. */ > > @@ -2100,6 +2103,8 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp) > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > if (cmd->cmd_len <= 16) > > return qla2xxx_start_scsi_mq(sp); > > + else > > + return qla_start_scsi_type6(sp); > > } > > > > spin_lock_irqsave(&qpair->qp_lock, flags); > > @@ -4205,3 +4210,268 @@ qla2x00_start_bidir(srb_t *sp, struct > > scsi_qla_host *vha, uint32_t tot_dsds) > > > > return rval; > > } > > + > > +/** > > + * qla_start_scsi_type6() - Send a SCSI command to the ISP > > + * @sp: command to send to the ISP > > + * > > + * Returns non-zero if a failure occurred, else zero. > > + */ > > +static int > > +qla_start_scsi_type6(srb_t *sp) > > +{ > > + int nseg; > > + unsigned long flags; > > + uint32_t *clr_ptr; > > + uint32_t handle; > > + struct cmd_type_6 *cmd_pkt; > > + uint16_t cnt; > > + uint16_t req_cnt; > > + uint16_t tot_dsds; > > + struct req_que *req = NULL; > > + struct rsp_que *rsp; > > + struct scsi_cmnd *cmd = GET_CMD_SP(sp); > > + struct scsi_qla_host *vha = sp->fcport->vha; > > + struct qla_hw_data *ha = vha->hw; > > + struct qla_qpair *qpair = sp->qpair; > > + uint16_t more_dsd_lists = 0; > > + struct dsd_dma *dsd_ptr; > > + uint16_t i; > > + __be32 *fcp_dl; > > + uint8_t additional_cdb_len; > > + struct ct6_dsd *ctx; > > + > > + > > + /* Acquire qpair specific lock */ > > + spin_lock_irqsave(&qpair->qp_lock, flags); > > + > > + /* Setup qpair pointers */ > > + req = qpair->req; > > + rsp = qpair->rsp; > > + > > + /* So we know we haven't pci_map'ed anything yet */ > > + tot_dsds = 0; > > + > > + /* Send marker if required */ > > + if (vha->marker_needed != 0) { > > + if (__qla2x00_marker(vha, qpair, 0, 0, MK_SYNC_ALL) > > != QLA_SUCCESS) { > > + spin_unlock_irqrestore(&qpair->qp_lock, > > flags); > > + return QLA_FUNCTION_FAILED; > > + } > > + vha->marker_needed = 0; > > + } > > + > > + handle = qla2xxx_get_next_handle(req); > > + if (handle == 0) > > + goto queuing_error; > > + > > + /* Map the sg table so we have an accurate count of sg > > entries needed */ > > + if (scsi_sg_count(cmd)) { > > + nseg = dma_map_sg(&ha->pdev->dev, scsi_sglist(cmd), > > + scsi_sg_count(cmd), cmd- > > > sc_data_direction); > > + if (unlikely(!nseg)) > > + goto queuing_error; > > + } else { > > + nseg = 0; > > + } > > + > > + tot_dsds = nseg; > > + > > + /* eventhough driver only need 1 T6 IOCB, FW still convert > > DSD to Continueation IOCB */ > > + req_cnt = qla24xx_calc_iocbs(vha, tot_dsds); > > + > > + sp->iores.res_type = RESOURCE_IOCB | RESOURCE_EXCH; > > + sp->iores.exch_cnt = 1; > > + sp->iores.iocb_cnt = req_cnt; > > + > > + if (qla_get_fw_resources(sp->qpair, &sp->iores)) > > + goto queuing_error; > > + > > + more_dsd_lists = qla24xx_calc_dsd_lists(tot_dsds); > > + if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { > > + ql_dbg(ql_dbg_io, vha, 0x3028, > > + "Num of DSD list %d is than %d for > > cmd=%p.\n", > > + more_dsd_lists + qpair->dsd_inuse, > > NUM_DSD_CHAIN, cmd); > > + goto queuing_error; > > + } > > + > > + if (more_dsd_lists <= qpair->dsd_avail) > > + goto sufficient_dsds; > > + else > > + more_dsd_lists -= qpair->dsd_avail; > > + > > + for (i = 0; i < more_dsd_lists; i++) { > > + dsd_ptr = kzalloc(sizeof(*dsd_ptr), GFP_ATOMIC); > > + if (!dsd_ptr) { > > + ql_log(ql_log_fatal, vha, 0x3029, > > + "Failed to allocate memory for dsd_dma > > for cmd=%p.\n", cmd); > > + goto queuing_error; > > + } > > + INIT_LIST_HEAD(&dsd_ptr->list); > > + > > + dsd_ptr->dsd_addr = dma_pool_alloc(ha->dl_dma_pool, > > + GFP_ATOMIC, &dsd_ptr->dsd_list_dma); > > + if (!dsd_ptr->dsd_addr) { > > + kfree(dsd_ptr); > > + ql_log(ql_log_fatal, vha, 0x302a, > > + "Failed to allocate memory for dsd_addr > > for cmd=%p.\n", cmd); > > + goto queuing_error; > > + } > > + list_add_tail(&dsd_ptr->list, &qpair->dsd_list); > > + qpair->dsd_avail++; > > + } > > + > > +sufficient_dsds: > > + req_cnt = 1; > > + > > + if (req->cnt < (req_cnt + 2)) { > > + if (IS_SHADOW_REG_CAPABLE(ha)) { > > + cnt = *req->out_ptr; > > + } else { > > + cnt = (uint16_t)rd_reg_dword_relaxed(req- > > > req_q_out); > > + if (qla2x00_check_reg16_for_disconnect(vha, > > cnt)) > > + goto queuing_error; > > + } > > + > > + if (req->ring_index < cnt) > > + req->cnt = cnt - req->ring_index; > > + else > > + req->cnt = req->length - (req->ring_index - > > cnt); > > + if (req->cnt < (req_cnt + 2)) > > + goto queuing_error; > > + } > > + > > + ctx = &sp->u.scmd.ct6_ctx; > > + > > + memset(ctx, 0, sizeof(struct ct6_dsd)); > > + ctx->fcp_cmnd = dma_pool_zalloc(ha->fcp_cmnd_dma_pool, > > + GFP_ATOMIC, &ctx->fcp_cmnd_dma); > > + if (!ctx->fcp_cmnd) { > > + ql_log(ql_log_fatal, vha, 0x3031, > > + "Failed to allocate fcp_cmnd for cmd=%p.\n", > > cmd); > > + goto queuing_error; > > + } > > + > > + /* Initialize the DSD list and dma handle */ > > + INIT_LIST_HEAD(&ctx->dsd_list); > > + ctx->dsd_use_cnt = 0; > > + > > + if (cmd->cmd_len > 16) { > > + additional_cdb_len = cmd->cmd_len - 16; > > + if (cmd->cmd_len % 4 || > > + cmd->cmd_len > QLA_CDB_BUF_SIZE) { > > + /* SCSI command bigger than 16 bytes must > > be > > + * multiple of 4 or too big. > > + */ > > + ql_log(ql_log_warn, vha, 0x3033, > > + "scsi cmd len %d not multiple of 4 for > > cmd=%p.\n", > > + cmd->cmd_len, cmd); > > + goto queuing_error_fcp_cmnd; > > + } > > + ctx->fcp_cmnd_len = 12 + cmd->cmd_len + 4; > > + } else { > > + additional_cdb_len = 0; > > + ctx->fcp_cmnd_len = 12 + 16 + 4; > > + } > > + > > + /* Build command packet. */ > > + req->current_outstanding_cmd = handle; > > + req->outstanding_cmds[handle] = sp; > > + sp->handle = handle; > > + cmd->host_scribble = (unsigned char *)(unsigned > > long)handle; > > + req->cnt -= req_cnt; > > + > > + cmd_pkt = (struct cmd_type_6 *)req->ring_ptr; > > + cmd_pkt->handle = make_handle(req->id, handle); > > + > > + /* Zero out remaining portion of packet. */ > > + /* tagged queuing modifier -- default is TSK_SIMPLE (0). > > */ > > + clr_ptr = (uint32_t *)cmd_pkt + 2; > > + memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); > > + cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); > > + > > + /* Set NPORT-ID and LUN number*/ > > + cmd_pkt->nport_handle = cpu_to_le16(sp->fcport->loop_id); > > + cmd_pkt->port_id[0] = sp->fcport->d_id.b.al_pa; > > + cmd_pkt->port_id[1] = sp->fcport->d_id.b.area; > > + cmd_pkt->port_id[2] = sp->fcport->d_id.b.domain; > > + cmd_pkt->vp_index = sp->vha->vp_idx; > > + > > + /* Build IOCB segments */ > > + qla24xx_build_scsi_type_6_iocbs(sp, cmd_pkt, tot_dsds); > > + > > + int_to_scsilun(cmd->device->lun, &cmd_pkt->lun); > > + host_to_fcp_swap((uint8_t *)&cmd_pkt->lun, sizeof(cmd_pkt- > > > lun)); > > + > > + /* build FCP_CMND IU */ > > + int_to_scsilun(cmd->device->lun, &ctx->fcp_cmnd->lun); > > + ctx->fcp_cmnd->additional_cdb_len = additional_cdb_len; > > + > > + if (cmd->sc_data_direction == DMA_TO_DEVICE) > > + ctx->fcp_cmnd->additional_cdb_len |= 1; > > + else if (cmd->sc_data_direction == DMA_FROM_DEVICE) > > + ctx->fcp_cmnd->additional_cdb_len |= 2; > > + > > + /* Populate the FCP_PRIO. */ > > + if (ha->flags.fcp_prio_enabled) > > + ctx->fcp_cmnd->task_attribute |= > > + sp->fcport->fcp_prio << 3; > > + > > + memcpy(ctx->fcp_cmnd->cdb, cmd->cmnd, cmd->cmd_len); > > + > > + fcp_dl = (__be32 *)(ctx->fcp_cmnd->cdb + 16 + > > + additional_cdb_len); > > + *fcp_dl = htonl((uint32_t)scsi_bufflen(cmd)); > > + > > + cmd_pkt->fcp_cmnd_dseg_len = cpu_to_le16(ctx- > > >fcp_cmnd_len); > > + put_unaligned_le64(ctx->fcp_cmnd_dma, > > + &cmd_pkt->fcp_cmnd_dseg_address); > > + > > + sp->flags |= SRB_FCP_CMND_DMA_VALID; > > + cmd_pkt->byte_count = > > cpu_to_le32((uint32_t)scsi_bufflen(cmd)); > > + /* Set total data segment count. */ > > + cmd_pkt->entry_count = (uint8_t)req_cnt; > > + > > + wmb(); > > + /* Adjust ring index. */ > > + req->ring_index++; > > + if (req->ring_index == req->length) { > > + req->ring_index = 0; > > + req->ring_ptr = req->ring; > > + } else { > > + req->ring_ptr++; > > + } > > + > > + sp->qpair->cmd_cnt++; > > + sp->flags |= SRB_DMA_VALID; > > + > > + /* Set chip new ring index. */ > > + wrt_reg_dword(req->req_q_in, req->ring_index); > > + > > + /* Manage unprocessed RIO/ZIO commands in response queue. > > */ > > + if (vha->flags.process_response_queue && > > + rsp->ring_ptr->signature != RESPONSE_PROCESSED) > > + qla24xx_process_response_queue(vha, rsp); > > + > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > + > > + return QLA_SUCCESS; > > + > > +queuing_error_fcp_cmnd: > > + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx->fcp_cmnd, ctx- > > > fcp_cmnd_dma); > > + > > +queuing_error: > > + if (tot_dsds) > > + scsi_dma_unmap(cmd); > > + > > + qla_put_fw_resources(sp->qpair, &sp->iores); > > + > > + if (sp->u.scmd.crc_ctx) { > > + mempool_free(sp->u.scmd.crc_ctx, ha->ctx_mempool); > > + sp->u.scmd.crc_ctx = NULL; > > + } > > + > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > + > > + return QLA_FUNCTION_FAILED; > > +} > > diff --git a/drivers/scsi/qla2xxx/qla_nx.h > > b/drivers/scsi/qla2xxx/qla_nx.h > > index 6dc80c8ddf79..5d1bdc15b75c 100644 > > --- a/drivers/scsi/qla2xxx/qla_nx.h > > +++ b/drivers/scsi/qla2xxx/qla_nx.h > > @@ -857,7 +857,9 @@ struct fcp_cmnd { > > uint8_t task_attribute; > > uint8_t task_management; > > uint8_t additional_cdb_len; > > - uint8_t cdb[260]; /* 256 for CDB len and 4 for FCP_DL */ > > +#define QLA_CDB_BUF_SIZE 256 > > +#define QLA_FCP_DL_SIZE 4 > > + uint8_t cdb[QLA_CDB_BUF_SIZE + QLA_FCP_DL_SIZE]; /* 256 for > > CDB len and 4 for FCP_DL */ > > }; > > > > struct dsd_dma { > > QT and Nilesh, Thank you. > I need more time to look at this fully but I will get it tested in > the > meantime. > I will reply with a review or questions later. > > Thanks for sending this > Laurence @QT and Nilesh Which kernel was this against. Did you guys test this only agains your driver or also upstream I cannot build it clean because There is no structure member dsd_inuse Can you send one for upstream please Patch applied fine to 6.5-rc4 [loberman@segstorage3 linux]$ patch -p1 < qt.patch patching file drivers/scsi/qla2xxx/qla_iocb.c Hunk #2 succeeded at 1718 (offset -4 lines). Hunk #3 succeeded at 2099 (offset -4 lines). Hunk #4 succeeded at 4179 (offset -31 lines). patching file drivers/scsi/qla2xxx/qla_nx.h Build fails drivers/scsi/qla2xxx/qla_iocb.c: In function ‘qla_start_scsi_type6’: drivers/scsi/qla2xxx/qla_iocb.c:4260:29: error: ‘struct qla_qpair’ has no member named ‘dsd_inuse’ if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { ^~ drivers/scsi/qla2xxx/qla_iocb.c:4263:32: error: ‘struct qla_qpair’ has no member named ‘dsd_inuse’ more_dsd_lists + qpair->dsd_inuse, NUM_DSD_CHAIN, cmd); ^~ drivers/scsi/qla2xxx/qla_iocb.c:4267:29: error: ‘struct qla_qpair’ has no member named ‘dsd_avail’ if (more_dsd_lists <= qpair->dsd_avail) ^~ drivers/scsi/qla2xxx/qla_iocb.c:4270:26: error: ‘struct qla_qpair’ has no member named ‘dsd_avail’ more_dsd_lists -= qpair->dsd_avail; ^~ drivers/scsi/qla2xxx/qla_iocb.c:4289:41: error: ‘struct qla_qpair’ has no member named ‘dsd_list’; did you mean ‘hints_list’? list_add_tail(&dsd_ptr->list, &qpair->dsd_list); ^~~~~~~~ hints_list drivers/scsi/qla2xxx/qla_iocb.c:4290:8: error: ‘struct qla_qpair’ has no member named ‘dsd_avail’ qpair->dsd_avail++;
On Fri, 2023-08-04 at 13:51 -0400, Laurence Oberman wrote: > On Fri, 2023-08-04 at 12:58 -0400, Laurence Oberman wrote: > > On Fri, 2023-08-04 at 12:49 +0530, Nilesh Javali wrote: > > > From: Quinn Tran <qutran@marvell.com> > > > > > > System crash when 32 bytes CDB was sent to a non T10-PI disk, > > > > > > [ 177.143279] ? qla2xxx_dif_start_scsi_mq+0xcd8/0xce0 [qla2xxx] > > > [ 177.149165] ? internal_add_timer+0x42/0x70 > > > [ 177.153372] qla2xxx_mqueuecommand+0x207/0x2b0 [qla2xxx] > > > [ 177.158730] scsi_queue_rq+0x2b7/0xc00 > > > [ 177.162501] blk_mq_dispatch_rq_list+0x3ea/0x7e0 > > > > > > Current code attempt to use CRC IOCB to send the command but > > > failed. > > > Instead, type 6 IOCB should be used to send the IO. > > > > > > Clone existing type 6 IOCB code with addition of MQ support > > > to allow 32 bytes CDB to go through. > > > > > > Signed-off-by: Quinn Tran <qutran@marvell.com> > > > Cc: Laurence Oberman <loberman@redhat.com> > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Nilesh Javali <njavali@marvell.com> > > > --- > > > drivers/scsi/qla2xxx/qla_iocb.c | 270 > > > ++++++++++++++++++++++++++++++++ > > > drivers/scsi/qla2xxx/qla_nx.h | 4 +- > > > 2 files changed, 273 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c > > > b/drivers/scsi/qla2xxx/qla_iocb.c > > > index 0caa64a7df26..e99ebf7e1c7a 100644 > > > --- a/drivers/scsi/qla2xxx/qla_iocb.c > > > +++ b/drivers/scsi/qla2xxx/qla_iocb.c > > > @@ -11,6 +11,7 @@ > > > > > > #include <scsi/scsi_tcq.h> > > > > > > +static int qla_start_scsi_type6(srb_t *sp); > > > /** > > > * qla2x00_get_cmd_direction() - Determine control_flag data > > > direction. > > > * @sp: SCSI command > > > @@ -1721,6 +1722,8 @@ qla24xx_dif_start_scsi(srb_t *sp) > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > if (cmd->cmd_len <= 16) > > > return qla24xx_start_scsi(sp); > > > + else > > > + return qla_start_scsi_type6(sp); > > > } > > > > > > /* Setup device pointers. */ > > > @@ -2100,6 +2103,8 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp) > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > if (cmd->cmd_len <= 16) > > > return qla2xxx_start_scsi_mq(sp); > > > + else > > > + return qla_start_scsi_type6(sp); > > > } > > > > > > spin_lock_irqsave(&qpair->qp_lock, flags); > > > @@ -4205,3 +4210,268 @@ qla2x00_start_bidir(srb_t *sp, struct > > > scsi_qla_host *vha, uint32_t tot_dsds) > > > > > > return rval; > > > } > > > + > > > +/** > > > + * qla_start_scsi_type6() - Send a SCSI command to the ISP > > > + * @sp: command to send to the ISP > > > + * > > > + * Returns non-zero if a failure occurred, else zero. > > > + */ > > > +static int > > > +qla_start_scsi_type6(srb_t *sp) > > > +{ > > > + int nseg; > > > + unsigned long flags; > > > + uint32_t *clr_ptr; > > > + uint32_t handle; > > > + struct cmd_type_6 *cmd_pkt; > > > + uint16_t cnt; > > > + uint16_t req_cnt; > > > + uint16_t tot_dsds; > > > + struct req_que *req = NULL; > > > + struct rsp_que *rsp; > > > + struct scsi_cmnd *cmd = GET_CMD_SP(sp); > > > + struct scsi_qla_host *vha = sp->fcport->vha; > > > + struct qla_hw_data *ha = vha->hw; > > > + struct qla_qpair *qpair = sp->qpair; > > > + uint16_t more_dsd_lists = 0; > > > + struct dsd_dma *dsd_ptr; > > > + uint16_t i; > > > + __be32 *fcp_dl; > > > + uint8_t additional_cdb_len; > > > + struct ct6_dsd *ctx; > > > + > > > + > > > + /* Acquire qpair specific lock */ > > > + spin_lock_irqsave(&qpair->qp_lock, flags); > > > + > > > + /* Setup qpair pointers */ > > > + req = qpair->req; > > > + rsp = qpair->rsp; > > > + > > > + /* So we know we haven't pci_map'ed anything yet */ > > > + tot_dsds = 0; > > > + > > > + /* Send marker if required */ > > > + if (vha->marker_needed != 0) { > > > + if (__qla2x00_marker(vha, qpair, 0, 0, > > > MK_SYNC_ALL) > > > != QLA_SUCCESS) { > > > + spin_unlock_irqrestore(&qpair->qp_lock, > > > flags); > > > + return QLA_FUNCTION_FAILED; > > > + } > > > + vha->marker_needed = 0; > > > + } > > > + > > > + handle = qla2xxx_get_next_handle(req); > > > + if (handle == 0) > > > + goto queuing_error; > > > + > > > + /* Map the sg table so we have an accurate count of sg > > > entries needed */ > > > + if (scsi_sg_count(cmd)) { > > > + nseg = dma_map_sg(&ha->pdev->dev, > > > scsi_sglist(cmd), > > > + scsi_sg_count(cmd), cmd- > > > > sc_data_direction); > > > + if (unlikely(!nseg)) > > > + goto queuing_error; > > > + } else { > > > + nseg = 0; > > > + } > > > + > > > + tot_dsds = nseg; > > > + > > > + /* eventhough driver only need 1 T6 IOCB, FW still > > > convert > > > DSD to Continueation IOCB */ > > > + req_cnt = qla24xx_calc_iocbs(vha, tot_dsds); > > > + > > > + sp->iores.res_type = RESOURCE_IOCB | RESOURCE_EXCH; > > > + sp->iores.exch_cnt = 1; > > > + sp->iores.iocb_cnt = req_cnt; > > > + > > > + if (qla_get_fw_resources(sp->qpair, &sp->iores)) > > > + goto queuing_error; > > > + > > > + more_dsd_lists = qla24xx_calc_dsd_lists(tot_dsds); > > > + if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) > > > { > > > + ql_dbg(ql_dbg_io, vha, 0x3028, > > > + "Num of DSD list %d is than %d for > > > cmd=%p.\n", > > > + more_dsd_lists + qpair->dsd_inuse, > > > NUM_DSD_CHAIN, cmd); > > > + goto queuing_error; > > > + } > > > + > > > + if (more_dsd_lists <= qpair->dsd_avail) > > > + goto sufficient_dsds; > > > + else > > > + more_dsd_lists -= qpair->dsd_avail; > > > + > > > + for (i = 0; i < more_dsd_lists; i++) { > > > + dsd_ptr = kzalloc(sizeof(*dsd_ptr), GFP_ATOMIC); > > > + if (!dsd_ptr) { > > > + ql_log(ql_log_fatal, vha, 0x3029, > > > + "Failed to allocate memory for > > > dsd_dma > > > for cmd=%p.\n", cmd); > > > + goto queuing_error; > > > + } > > > + INIT_LIST_HEAD(&dsd_ptr->list); > > > + > > > + dsd_ptr->dsd_addr = dma_pool_alloc(ha- > > > >dl_dma_pool, > > > + GFP_ATOMIC, &dsd_ptr->dsd_list_dma); > > > + if (!dsd_ptr->dsd_addr) { > > > + kfree(dsd_ptr); > > > + ql_log(ql_log_fatal, vha, 0x302a, > > > + "Failed to allocate memory for > > > dsd_addr > > > for cmd=%p.\n", cmd); > > > + goto queuing_error; > > > + } > > > + list_add_tail(&dsd_ptr->list, &qpair->dsd_list); > > > + qpair->dsd_avail++; > > > + } > > > + > > > +sufficient_dsds: > > > + req_cnt = 1; > > > + > > > + if (req->cnt < (req_cnt + 2)) { > > > + if (IS_SHADOW_REG_CAPABLE(ha)) { > > > + cnt = *req->out_ptr; > > > + } else { > > > + cnt = (uint16_t)rd_reg_dword_relaxed(req- > > > > req_q_out); > > > + if > > > (qla2x00_check_reg16_for_disconnect(vha, > > > cnt)) > > > + goto queuing_error; > > > + } > > > + > > > + if (req->ring_index < cnt) > > > + req->cnt = cnt - req->ring_index; > > > + else > > > + req->cnt = req->length - (req->ring_index > > > - > > > cnt); > > > + if (req->cnt < (req_cnt + 2)) > > > + goto queuing_error; > > > + } > > > + > > > + ctx = &sp->u.scmd.ct6_ctx; > > > + > > > + memset(ctx, 0, sizeof(struct ct6_dsd)); > > > + ctx->fcp_cmnd = dma_pool_zalloc(ha->fcp_cmnd_dma_pool, > > > + GFP_ATOMIC, &ctx->fcp_cmnd_dma); > > > + if (!ctx->fcp_cmnd) { > > > + ql_log(ql_log_fatal, vha, 0x3031, > > > + "Failed to allocate fcp_cmnd for cmd=%p.\n", > > > cmd); > > > + goto queuing_error; > > > + } > > > + > > > + /* Initialize the DSD list and dma handle */ > > > + INIT_LIST_HEAD(&ctx->dsd_list); > > > + ctx->dsd_use_cnt = 0; > > > + > > > + if (cmd->cmd_len > 16) { > > > + additional_cdb_len = cmd->cmd_len - 16; > > > + if (cmd->cmd_len % 4 || > > > + cmd->cmd_len > QLA_CDB_BUF_SIZE) { > > > + /* SCSI command bigger than 16 bytes must > > > be > > > + * multiple of 4 or too big. > > > + */ > > > + ql_log(ql_log_warn, vha, 0x3033, > > > + "scsi cmd len %d not multiple of 4 > > > for > > > cmd=%p.\n", > > > + cmd->cmd_len, cmd); > > > + goto queuing_error_fcp_cmnd; > > > + } > > > + ctx->fcp_cmnd_len = 12 + cmd->cmd_len + 4; > > > + } else { > > > + additional_cdb_len = 0; > > > + ctx->fcp_cmnd_len = 12 + 16 + 4; > > > + } > > > + > > > + /* Build command packet. */ > > > + req->current_outstanding_cmd = handle; > > > + req->outstanding_cmds[handle] = sp; > > > + sp->handle = handle; > > > + cmd->host_scribble = (unsigned char *)(unsigned > > > long)handle; > > > + req->cnt -= req_cnt; > > > + > > > + cmd_pkt = (struct cmd_type_6 *)req->ring_ptr; > > > + cmd_pkt->handle = make_handle(req->id, handle); > > > + > > > + /* Zero out remaining portion of packet. */ > > > + /* tagged queuing modifier -- default is TSK_SIMPLE > > > (0). > > > */ > > > + clr_ptr = (uint32_t *)cmd_pkt + 2; > > > + memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); > > > + cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); > > > + > > > + /* Set NPORT-ID and LUN number*/ > > > + cmd_pkt->nport_handle = cpu_to_le16(sp->fcport->loop_id); > > > + cmd_pkt->port_id[0] = sp->fcport->d_id.b.al_pa; > > > + cmd_pkt->port_id[1] = sp->fcport->d_id.b.area; > > > + cmd_pkt->port_id[2] = sp->fcport->d_id.b.domain; > > > + cmd_pkt->vp_index = sp->vha->vp_idx; > > > + > > > + /* Build IOCB segments */ > > > + qla24xx_build_scsi_type_6_iocbs(sp, cmd_pkt, tot_dsds); > > > + > > > + int_to_scsilun(cmd->device->lun, &cmd_pkt->lun); > > > + host_to_fcp_swap((uint8_t *)&cmd_pkt->lun, > > > sizeof(cmd_pkt- > > > > lun)); > > > + > > > + /* build FCP_CMND IU */ > > > + int_to_scsilun(cmd->device->lun, &ctx->fcp_cmnd->lun); > > > + ctx->fcp_cmnd->additional_cdb_len = additional_cdb_len; > > > + > > > + if (cmd->sc_data_direction == DMA_TO_DEVICE) > > > + ctx->fcp_cmnd->additional_cdb_len |= 1; > > > + else if (cmd->sc_data_direction == DMA_FROM_DEVICE) > > > + ctx->fcp_cmnd->additional_cdb_len |= 2; > > > + > > > + /* Populate the FCP_PRIO. */ > > > + if (ha->flags.fcp_prio_enabled) > > > + ctx->fcp_cmnd->task_attribute |= > > > + sp->fcport->fcp_prio << 3; > > > + > > > + memcpy(ctx->fcp_cmnd->cdb, cmd->cmnd, cmd->cmd_len); > > > + > > > + fcp_dl = (__be32 *)(ctx->fcp_cmnd->cdb + 16 + > > > + additional_cdb_len); > > > + *fcp_dl = htonl((uint32_t)scsi_bufflen(cmd)); > > > + > > > + cmd_pkt->fcp_cmnd_dseg_len = cpu_to_le16(ctx- > > > > fcp_cmnd_len); > > > + put_unaligned_le64(ctx->fcp_cmnd_dma, > > > + &cmd_pkt->fcp_cmnd_dseg_address); > > > + > > > + sp->flags |= SRB_FCP_CMND_DMA_VALID; > > > + cmd_pkt->byte_count = > > > cpu_to_le32((uint32_t)scsi_bufflen(cmd)); > > > + /* Set total data segment count. */ > > > + cmd_pkt->entry_count = (uint8_t)req_cnt; > > > + > > > + wmb(); > > > + /* Adjust ring index. */ > > > + req->ring_index++; > > > + if (req->ring_index == req->length) { > > > + req->ring_index = 0; > > > + req->ring_ptr = req->ring; > > > + } else { > > > + req->ring_ptr++; > > > + } > > > + > > > + sp->qpair->cmd_cnt++; > > > + sp->flags |= SRB_DMA_VALID; > > > + > > > + /* Set chip new ring index. */ > > > + wrt_reg_dword(req->req_q_in, req->ring_index); > > > + > > > + /* Manage unprocessed RIO/ZIO commands in response queue. > > > */ > > > + if (vha->flags.process_response_queue && > > > + rsp->ring_ptr->signature != RESPONSE_PROCESSED) > > > + qla24xx_process_response_queue(vha, rsp); > > > + > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > + > > > + return QLA_SUCCESS; > > > + > > > +queuing_error_fcp_cmnd: > > > + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx->fcp_cmnd, ctx- > > > > fcp_cmnd_dma); > > > + > > > +queuing_error: > > > + if (tot_dsds) > > > + scsi_dma_unmap(cmd); > > > + > > > + qla_put_fw_resources(sp->qpair, &sp->iores); > > > + > > > + if (sp->u.scmd.crc_ctx) { > > > + mempool_free(sp->u.scmd.crc_ctx, ha- > > > >ctx_mempool); > > > + sp->u.scmd.crc_ctx = NULL; > > > + } > > > + > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > + > > > + return QLA_FUNCTION_FAILED; > > > +} > > > diff --git a/drivers/scsi/qla2xxx/qla_nx.h > > > b/drivers/scsi/qla2xxx/qla_nx.h > > > index 6dc80c8ddf79..5d1bdc15b75c 100644 > > > --- a/drivers/scsi/qla2xxx/qla_nx.h > > > +++ b/drivers/scsi/qla2xxx/qla_nx.h > > > @@ -857,7 +857,9 @@ struct fcp_cmnd { > > > uint8_t task_attribute; > > > uint8_t task_management; > > > uint8_t additional_cdb_len; > > > - uint8_t cdb[260]; /* 256 for CDB len and 4 for FCP_DL */ > > > +#define QLA_CDB_BUF_SIZE 256 > > > +#define QLA_FCP_DL_SIZE 4 > > > + uint8_t cdb[QLA_CDB_BUF_SIZE + QLA_FCP_DL_SIZE]; /* 256 > > > for > > > CDB len and 4 for FCP_DL */ > > > }; > > > > > > struct dsd_dma { > > > > QT and Nilesh, Thank you. > > I need more time to look at this fully but I will get it tested in > > the > > meantime. > > I will reply with a review or questions later. > > > > Thanks for sending this > > Laurence > > @QT and Nilesh > Which kernel was this against. > Did you guys test this only agains your driver or also upstream > I cannot build it clean because There is no structure member > dsd_inuse > > Can you send one for upstream please > > Patch applied fine to 6.5-rc4 > > [loberman@segstorage3 linux]$ patch -p1 < qt.patch > patching file drivers/scsi/qla2xxx/qla_iocb.c > Hunk #2 succeeded at 1718 (offset -4 lines). > Hunk #3 succeeded at 2099 (offset -4 lines). > Hunk #4 succeeded at 4179 (offset -31 lines). > patching file drivers/scsi/qla2xxx/qla_nx.h > > > Build fails > > drivers/scsi/qla2xxx/qla_iocb.c: In function ‘qla_start_scsi_type6’: > drivers/scsi/qla2xxx/qla_iocb.c:4260:29: error: ‘struct qla_qpair’ > has > no member named ‘dsd_inuse’ > if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { > ^~ > drivers/scsi/qla2xxx/qla_iocb.c:4263:32: error: ‘struct qla_qpair’ > has > no member named ‘dsd_inuse’ > more_dsd_lists + qpair->dsd_inuse, NUM_DSD_CHAIN, cmd); > ^~ > drivers/scsi/qla2xxx/qla_iocb.c:4267:29: error: ‘struct qla_qpair’ > has > no member named ‘dsd_avail’ > if (more_dsd_lists <= qpair->dsd_avail) > ^~ > drivers/scsi/qla2xxx/qla_iocb.c:4270:26: error: ‘struct qla_qpair’ > has > no member named ‘dsd_avail’ > more_dsd_lists -= qpair->dsd_avail; > ^~ > drivers/scsi/qla2xxx/qla_iocb.c:4289:41: error: ‘struct qla_qpair’ > has > no member named ‘dsd_list’; did you mean ‘hints_list’? > list_add_tail(&dsd_ptr->list, &qpair->dsd_list); > ^~~~~~~~ > hints_list > drivers/scsi/qla2xxx/qla_iocb.c:4290:8: error: ‘struct qla_qpair’ has > no member named ‘dsd_avail’ > qpair->dsd_avail++; I see where this was done, there are two more patches So I will add those and test [PATCH 1/2] qla2xxx: Move resource to allow code reuse >
On Fri, 2023-08-04 at 14:35 -0400, Laurence Oberman wrote: > On Fri, 2023-08-04 at 13:51 -0400, Laurence Oberman wrote: > > On Fri, 2023-08-04 at 12:58 -0400, Laurence Oberman wrote: > > > On Fri, 2023-08-04 at 12:49 +0530, Nilesh Javali wrote: > > > > From: Quinn Tran <qutran@marvell.com> > > > > > > > > System crash when 32 bytes CDB was sent to a non T10-PI disk, > > > > > > > > [ 177.143279] ? qla2xxx_dif_start_scsi_mq+0xcd8/0xce0 > > > > [qla2xxx] > > > > [ 177.149165] ? internal_add_timer+0x42/0x70 > > > > [ 177.153372] qla2xxx_mqueuecommand+0x207/0x2b0 [qla2xxx] > > > > [ 177.158730] scsi_queue_rq+0x2b7/0xc00 > > > > [ 177.162501] blk_mq_dispatch_rq_list+0x3ea/0x7e0 > > > > > > > > Current code attempt to use CRC IOCB to send the command but > > > > failed. > > > > Instead, type 6 IOCB should be used to send the IO. > > > > > > > > Clone existing type 6 IOCB code with addition of MQ support > > > > to allow 32 bytes CDB to go through. > > > > > > > > Signed-off-by: Quinn Tran <qutran@marvell.com> > > > > Cc: Laurence Oberman <loberman@redhat.com> > > > > Cc: stable@vger.kernel.org > > > > Signed-off-by: Nilesh Javali <njavali@marvell.com> > > > > --- > > > > drivers/scsi/qla2xxx/qla_iocb.c | 270 > > > > ++++++++++++++++++++++++++++++++ > > > > drivers/scsi/qla2xxx/qla_nx.h | 4 +- > > > > 2 files changed, 273 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c > > > > b/drivers/scsi/qla2xxx/qla_iocb.c > > > > index 0caa64a7df26..e99ebf7e1c7a 100644 > > > > --- a/drivers/scsi/qla2xxx/qla_iocb.c > > > > +++ b/drivers/scsi/qla2xxx/qla_iocb.c > > > > @@ -11,6 +11,7 @@ > > > > > > > > #include <scsi/scsi_tcq.h> > > > > > > > > +static int qla_start_scsi_type6(srb_t *sp); > > > > /** > > > > * qla2x00_get_cmd_direction() - Determine control_flag data > > > > direction. > > > > * @sp: SCSI command > > > > @@ -1721,6 +1722,8 @@ qla24xx_dif_start_scsi(srb_t *sp) > > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > > if (cmd->cmd_len <= 16) > > > > return qla24xx_start_scsi(sp); > > > > + else > > > > + return qla_start_scsi_type6(sp); > > > > } > > > > > > > > /* Setup device pointers. */ > > > > @@ -2100,6 +2103,8 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp) > > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > > if (cmd->cmd_len <= 16) > > > > return qla2xxx_start_scsi_mq(sp); > > > > + else > > > > + return qla_start_scsi_type6(sp); > > > > } > > > > > > > > spin_lock_irqsave(&qpair->qp_lock, flags); > > > > @@ -4205,3 +4210,268 @@ qla2x00_start_bidir(srb_t *sp, struct > > > > scsi_qla_host *vha, uint32_t tot_dsds) > > > > > > > > return rval; > > > > } > > > > + > > > > +/** > > > > + * qla_start_scsi_type6() - Send a SCSI command to the ISP > > > > + * @sp: command to send to the ISP > > > > + * > > > > + * Returns non-zero if a failure occurred, else zero. > > > > + */ > > > > +static int > > > > +qla_start_scsi_type6(srb_t *sp) > > > > +{ > > > > + int nseg; > > > > + unsigned long flags; > > > > + uint32_t *clr_ptr; > > > > + uint32_t handle; > > > > + struct cmd_type_6 *cmd_pkt; > > > > + uint16_t cnt; > > > > + uint16_t req_cnt; > > > > + uint16_t tot_dsds; > > > > + struct req_que *req = NULL; > > > > + struct rsp_que *rsp; > > > > + struct scsi_cmnd *cmd = GET_CMD_SP(sp); > > > > + struct scsi_qla_host *vha = sp->fcport->vha; > > > > + struct qla_hw_data *ha = vha->hw; > > > > + struct qla_qpair *qpair = sp->qpair; > > > > + uint16_t more_dsd_lists = 0; > > > > + struct dsd_dma *dsd_ptr; > > > > + uint16_t i; > > > > + __be32 *fcp_dl; > > > > + uint8_t additional_cdb_len; > > > > + struct ct6_dsd *ctx; > > > > + > > > > + > > > > + /* Acquire qpair specific lock */ > > > > + spin_lock_irqsave(&qpair->qp_lock, flags); > > > > + > > > > + /* Setup qpair pointers */ > > > > + req = qpair->req; > > > > + rsp = qpair->rsp; > > > > + > > > > + /* So we know we haven't pci_map'ed anything yet */ > > > > + tot_dsds = 0; > > > > + > > > > + /* Send marker if required */ > > > > + if (vha->marker_needed != 0) { > > > > + if (__qla2x00_marker(vha, qpair, 0, 0, > > > > MK_SYNC_ALL) > > > > != QLA_SUCCESS) { > > > > + spin_unlock_irqrestore(&qpair->qp_lock, > > > > flags); > > > > + return QLA_FUNCTION_FAILED; > > > > + } > > > > + vha->marker_needed = 0; > > > > + } > > > > + > > > > + handle = qla2xxx_get_next_handle(req); > > > > + if (handle == 0) > > > > + goto queuing_error; > > > > + > > > > + /* Map the sg table so we have an accurate count of sg > > > > entries needed */ > > > > + if (scsi_sg_count(cmd)) { > > > > + nseg = dma_map_sg(&ha->pdev->dev, > > > > scsi_sglist(cmd), > > > > + scsi_sg_count(cmd), cmd- > > > > > sc_data_direction); > > > > + if (unlikely(!nseg)) > > > > + goto queuing_error; > > > > + } else { > > > > + nseg = 0; > > > > + } > > > > + > > > > + tot_dsds = nseg; > > > > + > > > > + /* eventhough driver only need 1 T6 IOCB, FW still > > > > convert > > > > DSD to Continueation IOCB */ > > > > + req_cnt = qla24xx_calc_iocbs(vha, tot_dsds); > > > > + > > > > + sp->iores.res_type = RESOURCE_IOCB | RESOURCE_EXCH; > > > > + sp->iores.exch_cnt = 1; > > > > + sp->iores.iocb_cnt = req_cnt; > > > > + > > > > + if (qla_get_fw_resources(sp->qpair, &sp->iores)) > > > > + goto queuing_error; > > > > + > > > > + more_dsd_lists = qla24xx_calc_dsd_lists(tot_dsds); > > > > + if ((more_dsd_lists + qpair->dsd_inuse) >= > > > > NUM_DSD_CHAIN) > > > > { > > > > + ql_dbg(ql_dbg_io, vha, 0x3028, > > > > + "Num of DSD list %d is than %d for > > > > cmd=%p.\n", > > > > + more_dsd_lists + qpair->dsd_inuse, > > > > NUM_DSD_CHAIN, cmd); > > > > + goto queuing_error; > > > > + } > > > > + > > > > + if (more_dsd_lists <= qpair->dsd_avail) > > > > + goto sufficient_dsds; > > > > + else > > > > + more_dsd_lists -= qpair->dsd_avail; > > > > + > > > > + for (i = 0; i < more_dsd_lists; i++) { > > > > + dsd_ptr = kzalloc(sizeof(*dsd_ptr), > > > > GFP_ATOMIC); > > > > + if (!dsd_ptr) { > > > > + ql_log(ql_log_fatal, vha, 0x3029, > > > > + "Failed to allocate memory for > > > > dsd_dma > > > > for cmd=%p.\n", cmd); > > > > + goto queuing_error; > > > > + } > > > > + INIT_LIST_HEAD(&dsd_ptr->list); > > > > + > > > > + dsd_ptr->dsd_addr = dma_pool_alloc(ha- > > > > > dl_dma_pool, > > > > + GFP_ATOMIC, &dsd_ptr->dsd_list_dma); > > > > + if (!dsd_ptr->dsd_addr) { > > > > + kfree(dsd_ptr); > > > > + ql_log(ql_log_fatal, vha, 0x302a, > > > > + "Failed to allocate memory for > > > > dsd_addr > > > > for cmd=%p.\n", cmd); > > > > + goto queuing_error; > > > > + } > > > > + list_add_tail(&dsd_ptr->list, &qpair- > > > > >dsd_list); > > > > + qpair->dsd_avail++; > > > > + } > > > > + > > > > +sufficient_dsds: > > > > + req_cnt = 1; > > > > + > > > > + if (req->cnt < (req_cnt + 2)) { > > > > + if (IS_SHADOW_REG_CAPABLE(ha)) { > > > > + cnt = *req->out_ptr; > > > > + } else { > > > > + cnt = > > > > (uint16_t)rd_reg_dword_relaxed(req- > > > > > req_q_out); > > > > + if > > > > (qla2x00_check_reg16_for_disconnect(vha, > > > > cnt)) > > > > + goto queuing_error; > > > > + } > > > > + > > > > + if (req->ring_index < cnt) > > > > + req->cnt = cnt - req->ring_index; > > > > + else > > > > + req->cnt = req->length - (req- > > > > >ring_index > > > > - > > > > cnt); > > > > + if (req->cnt < (req_cnt + 2)) > > > > + goto queuing_error; > > > > + } > > > > + > > > > + ctx = &sp->u.scmd.ct6_ctx; > > > > + > > > > + memset(ctx, 0, sizeof(struct ct6_dsd)); > > > > + ctx->fcp_cmnd = dma_pool_zalloc(ha->fcp_cmnd_dma_pool, > > > > + GFP_ATOMIC, &ctx->fcp_cmnd_dma); > > > > + if (!ctx->fcp_cmnd) { > > > > + ql_log(ql_log_fatal, vha, 0x3031, > > > > + "Failed to allocate fcp_cmnd for > > > > cmd=%p.\n", > > > > cmd); > > > > + goto queuing_error; > > > > + } > > > > + > > > > + /* Initialize the DSD list and dma handle */ > > > > + INIT_LIST_HEAD(&ctx->dsd_list); > > > > + ctx->dsd_use_cnt = 0; > > > > + > > > > + if (cmd->cmd_len > 16) { > > > > + additional_cdb_len = cmd->cmd_len - 16; > > > > + if (cmd->cmd_len % 4 || > > > > + cmd->cmd_len > QLA_CDB_BUF_SIZE) { > > > > + /* SCSI command bigger than 16 bytes > > > > must > > > > be > > > > + * multiple of 4 or too big. > > > > + */ > > > > + ql_log(ql_log_warn, vha, 0x3033, > > > > + "scsi cmd len %d not multiple of 4 > > > > for > > > > cmd=%p.\n", > > > > + cmd->cmd_len, cmd); > > > > + goto queuing_error_fcp_cmnd; > > > > + } > > > > + ctx->fcp_cmnd_len = 12 + cmd->cmd_len + 4; > > > > + } else { > > > > + additional_cdb_len = 0; > > > > + ctx->fcp_cmnd_len = 12 + 16 + 4; > > > > + } > > > > + > > > > + /* Build command packet. */ > > > > + req->current_outstanding_cmd = handle; > > > > + req->outstanding_cmds[handle] = sp; > > > > + sp->handle = handle; > > > > + cmd->host_scribble = (unsigned char *)(unsigned > > > > long)handle; > > > > + req->cnt -= req_cnt; > > > > + > > > > + cmd_pkt = (struct cmd_type_6 *)req->ring_ptr; > > > > + cmd_pkt->handle = make_handle(req->id, handle); > > > > + > > > > + /* Zero out remaining portion of packet. */ > > > > + /* tagged queuing modifier -- default is TSK_SIMPLE > > > > (0). > > > > */ > > > > + clr_ptr = (uint32_t *)cmd_pkt + 2; > > > > + memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); > > > > + cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); > > > > + > > > > + /* Set NPORT-ID and LUN number*/ > > > > + cmd_pkt->nport_handle = cpu_to_le16(sp->fcport- > > > > >loop_id); > > > > + cmd_pkt->port_id[0] = sp->fcport->d_id.b.al_pa; > > > > + cmd_pkt->port_id[1] = sp->fcport->d_id.b.area; > > > > + cmd_pkt->port_id[2] = sp->fcport->d_id.b.domain; > > > > + cmd_pkt->vp_index = sp->vha->vp_idx; > > > > + > > > > + /* Build IOCB segments */ > > > > + qla24xx_build_scsi_type_6_iocbs(sp, cmd_pkt, tot_dsds); > > > > + > > > > + int_to_scsilun(cmd->device->lun, &cmd_pkt->lun); > > > > + host_to_fcp_swap((uint8_t *)&cmd_pkt->lun, > > > > sizeof(cmd_pkt- > > > > > lun)); > > > > + > > > > + /* build FCP_CMND IU */ > > > > + int_to_scsilun(cmd->device->lun, &ctx->fcp_cmnd->lun); > > > > + ctx->fcp_cmnd->additional_cdb_len = additional_cdb_len; > > > > + > > > > + if (cmd->sc_data_direction == DMA_TO_DEVICE) > > > > + ctx->fcp_cmnd->additional_cdb_len |= 1; > > > > + else if (cmd->sc_data_direction == DMA_FROM_DEVICE) > > > > + ctx->fcp_cmnd->additional_cdb_len |= 2; > > > > + > > > > + /* Populate the FCP_PRIO. */ > > > > + if (ha->flags.fcp_prio_enabled) > > > > + ctx->fcp_cmnd->task_attribute |= > > > > + sp->fcport->fcp_prio << 3; > > > > + > > > > + memcpy(ctx->fcp_cmnd->cdb, cmd->cmnd, cmd->cmd_len); > > > > + > > > > + fcp_dl = (__be32 *)(ctx->fcp_cmnd->cdb + 16 + > > > > + additional_cdb_len); > > > > + *fcp_dl = htonl((uint32_t)scsi_bufflen(cmd)); > > > > + > > > > + cmd_pkt->fcp_cmnd_dseg_len = cpu_to_le16(ctx- > > > > > fcp_cmnd_len); > > > > + put_unaligned_le64(ctx->fcp_cmnd_dma, > > > > + &cmd_pkt->fcp_cmnd_dseg_address); > > > > + > > > > + sp->flags |= SRB_FCP_CMND_DMA_VALID; > > > > + cmd_pkt->byte_count = > > > > cpu_to_le32((uint32_t)scsi_bufflen(cmd)); > > > > + /* Set total data segment count. */ > > > > + cmd_pkt->entry_count = (uint8_t)req_cnt; > > > > + > > > > + wmb(); > > > > + /* Adjust ring index. */ > > > > + req->ring_index++; > > > > + if (req->ring_index == req->length) { > > > > + req->ring_index = 0; > > > > + req->ring_ptr = req->ring; > > > > + } else { > > > > + req->ring_ptr++; > > > > + } > > > > + > > > > + sp->qpair->cmd_cnt++; > > > > + sp->flags |= SRB_DMA_VALID; > > > > + > > > > + /* Set chip new ring index. */ > > > > + wrt_reg_dword(req->req_q_in, req->ring_index); > > > > + > > > > + /* Manage unprocessed RIO/ZIO commands in response > > > > queue. > > > > */ > > > > + if (vha->flags.process_response_queue && > > > > + rsp->ring_ptr->signature != RESPONSE_PROCESSED) > > > > + qla24xx_process_response_queue(vha, rsp); > > > > + > > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > > + > > > > + return QLA_SUCCESS; > > > > + > > > > +queuing_error_fcp_cmnd: > > > > + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx->fcp_cmnd, > > > > ctx- > > > > > fcp_cmnd_dma); > > > > + > > > > +queuing_error: > > > > + if (tot_dsds) > > > > + scsi_dma_unmap(cmd); > > > > + > > > > + qla_put_fw_resources(sp->qpair, &sp->iores); > > > > + > > > > + if (sp->u.scmd.crc_ctx) { > > > > + mempool_free(sp->u.scmd.crc_ctx, ha- > > > > > ctx_mempool); > > > > + sp->u.scmd.crc_ctx = NULL; > > > > + } > > > > + > > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > > + > > > > + return QLA_FUNCTION_FAILED; > > > > +} > > > > diff --git a/drivers/scsi/qla2xxx/qla_nx.h > > > > b/drivers/scsi/qla2xxx/qla_nx.h > > > > index 6dc80c8ddf79..5d1bdc15b75c 100644 > > > > --- a/drivers/scsi/qla2xxx/qla_nx.h > > > > +++ b/drivers/scsi/qla2xxx/qla_nx.h > > > > @@ -857,7 +857,9 @@ struct fcp_cmnd { > > > > uint8_t task_attribute; > > > > uint8_t task_management; > > > > uint8_t additional_cdb_len; > > > > - uint8_t cdb[260]; /* 256 for CDB len and 4 for FCP_DL > > > > */ > > > > +#define QLA_CDB_BUF_SIZE 256 > > > > +#define QLA_FCP_DL_SIZE 4 > > > > + uint8_t cdb[QLA_CDB_BUF_SIZE + QLA_FCP_DL_SIZE]; /* 256 > > > > for > > > > CDB len and 4 for FCP_DL */ > > > > }; > > > > > > > > struct dsd_dma { > > > > > > QT and Nilesh, Thank you. > > > I need more time to look at this fully but I will get it tested > > > in > > > the > > > meantime. > > > I will reply with a review or questions later. > > > > > > Thanks for sending this > > > Laurence > > > > @QT and Nilesh > > Which kernel was this against. > > Did you guys test this only agains your driver or also upstream > > I cannot build it clean because There is no structure member > > dsd_inuse > > > > Can you send one for upstream please > > > > Patch applied fine to 6.5-rc4 > > > > [loberman@segstorage3 linux]$ patch -p1 < qt.patch > > patching file drivers/scsi/qla2xxx/qla_iocb.c > > Hunk #2 succeeded at 1718 (offset -4 lines). > > Hunk #3 succeeded at 2099 (offset -4 lines). > > Hunk #4 succeeded at 4179 (offset -31 lines). > > patching file drivers/scsi/qla2xxx/qla_nx.h > > > > > > Build fails > > > > drivers/scsi/qla2xxx/qla_iocb.c: In function > > ‘qla_start_scsi_type6’: > > drivers/scsi/qla2xxx/qla_iocb.c:4260:29: error: ‘struct qla_qpair’ > > has > > no member named ‘dsd_inuse’ > > if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { > > ^~ > > drivers/scsi/qla2xxx/qla_iocb.c:4263:32: error: ‘struct qla_qpair’ > > has > > no member named ‘dsd_inuse’ > > more_dsd_lists + qpair->dsd_inuse, NUM_DSD_CHAIN, cmd); > > ^~ > > drivers/scsi/qla2xxx/qla_iocb.c:4267:29: error: ‘struct qla_qpair’ > > has > > no member named ‘dsd_avail’ > > if (more_dsd_lists <= qpair->dsd_avail) > > ^~ > > drivers/scsi/qla2xxx/qla_iocb.c:4270:26: error: ‘struct qla_qpair’ > > has > > no member named ‘dsd_avail’ > > more_dsd_lists -= qpair->dsd_avail; > > ^~ > > drivers/scsi/qla2xxx/qla_iocb.c:4289:41: error: ‘struct qla_qpair’ > > has > > no member named ‘dsd_list’; did you mean ‘hints_list’? > > list_add_tail(&dsd_ptr->list, &qpair->dsd_list); > > ^~~~~~~~ > > hints_list > > drivers/scsi/qla2xxx/qla_iocb.c:4290:8: error: ‘struct qla_qpair’ > > has > > no member named ‘dsd_avail’ > > qpair->dsd_avail++; > > I see where this was done, there are two more patches > So I will add those and test > [PATCH 1/2] qla2xxx: Move resource to allow code reuse > > > OK, I was able to now build and and test this. To be honest there are a lot of changes. I just tested the original issue for which I had sent my first patch. Unfortunately the test fails in a different way to the orginal BUG but this may be firmware behavior on the MSA2050 that takes the device paths away. This is because the test passes on an LIO target Against 6.5-rc4 Test notes segstorage3 6.5.0-rc4+ Note that on a MSA2050 it will hang and timeout no more panics as expected. However as noted we lose devices. Starts out fine mpathbp (3600c0ff00043feab07a7ff6307000000) dm-8 HPE,MSA 2050 SAN size=9.3G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw |-+- policy='service-time 0' prio=50 status=active | |- 1:0:1:4 sdh 8:112 active ready running | |- 1:0:4:4 sdaf 65:240 active ready running | |- 3:0:2:4 sdfk 130:96 active ready running | `- 3:0:3:4 sdft 130:240 active ready running `-+- policy='service-time 0' prio=10 status=enabled |- 1:0:2:4 sdp 8:240 active ready running |- 1:0:3:4 sdx 65:112 active ready running |- 3:0:1:4 sdfb 129:208 active ready running `- 3:0:4:4 sdge 131:160 active ready running Do the test [root@segstorage3 ~]# sg_write_same -T --lba=1 /dev/mapper/mpathbp Write same: transport: Host_status=0x03 [DID_TIME_OUT] Driver_status=0x00 [DRIVER_OK] It then caused problems with two of my multipath device paths but perhaps this is array firmware behavior. [ 820.836495] sd 3:0:2:4: [sdfk] tag#1704 FAILED Result: hostbyte=DID_TRANSPORT_DISRUPTED driverbyte=DRIVER_OK cmd_age=23s [ 820.836497] sd 3:0:2:4: [sdfk] tag#1704 CDB: Test Unit Ready 00 00 00 00 00 00 [ 825.926075] sd 3:0:2:0: rejecting I/O to offline device [ 825.926136] sd 3:0:2:1: rejecting I/O to offline device [ 825.926165] sd 3:0:2:3: rejecting I/O to offline device And two paths get lost mpathbp (3600c0ff00043feab07a7ff6307000000) dm-8 HPE,MSA 2050 SAN size=9.3G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw |-+- policy='service-time 0' prio=50 status=active | |- 1:0:4:4 sdaf 65:240 active ready running | `- 3:0:3:4 sdft 130:240 active ready running `-+- policy='service-time 0' prio=10 status=enabled |- 1:0:2:4 sdp 8:240 active ready running |- 1:0:3:4 sdx 65:112 active ready running |- 3:0:1:4 sdfb 129:208 active ready running `- 3:0:4:4 sdge 131:160 active ready running I re-scanned but sdh and sdfk are gone from the device tree and do not come back with a rescan. [ 846.043511] sd 1:0:1:4: [sdh] Synchronizing SCSI cache [ 846.043536] sd 1:0:1:4: [sdh] Synchronize Cache(10) failed: Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK [ 820.836495] sd 3:0:2:4: [sdfk] tag#1704 FAILED Result: hostbyte=DID_TRANSPORT_DISRUPTED driverbyte=DRIVER_OK cmd_age=23s [ 820.836497] sd 3:0:2:4: [sdfk] tag#1704 CDB: Test Unit Ready 00 00 00 00 00 00 [ 851.166451] sd 3:0:2:4: [sdfk] Synchronizing SCSI cache [ 851.166477] sd 3:0:2:4: [sdfk] Synchronize Cache(10) failed: Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK Now using an LIO target via tcm_qla2xxx mpathy (3600140549ed41778d414038bf1f1bb42) dm-74 LIO-ORG,test-70 size=1.0G features='0' hwhandler='1 alua' wp=rw `-+- policy='service-time 0' prio=50 status=active |- 1:0:5:105 sdbv 68:144 active ready running |- 1:0:6:105 sdei 128:160 active ready running |- 3:0:5:105 sdhz 134:144 active ready running `- 3:0:6:105 sdkm 66:416 active ready running [root@segstorage3 block]# sg_write_same -T --lba=1 /dev/mapper/mpathy Write same: transport: Host_status=0x07 [DID_ERROR] Driver_status=0x08 [DRIVER_SENSE] No issues with the LIO device paths [root@segstorage3 block]# multipath -ll mpathy mpathy (3600140549ed41778d414038bf1f1bb42) dm-74 LIO-ORG,test-70 size=1.0G features='0' hwhandler='1 alua' wp=rw `-+- policy='service-time 0' prio=50 status=active |- 1:0:5:105 sdbv 68:144 active ready running |- 1:0:6:105 sdei 128:160 active ready running |- 3:0:5:105 sdhz 134:144 active ready running `- 3:0:6:105 sdkm 66:416 active ready running So I would like to give a positive test result but will need you folks at Marvell to maybe first work with HPE on the MSA2050 behavior. Thanks Laurence
On Fri, 2023-08-04 at 17:35 -0400, Laurence Oberman wrote: > On Fri, 2023-08-04 at 14:35 -0400, Laurence Oberman wrote: > > On Fri, 2023-08-04 at 13:51 -0400, Laurence Oberman wrote: > > > On Fri, 2023-08-04 at 12:58 -0400, Laurence Oberman wrote: > > > > On Fri, 2023-08-04 at 12:49 +0530, Nilesh Javali wrote: > > > > > From: Quinn Tran <qutran@marvell.com> > > > > > > > > > > System crash when 32 bytes CDB was sent to a non T10-PI disk, > > > > > > > > > > [ 177.143279] ? qla2xxx_dif_start_scsi_mq+0xcd8/0xce0 > > > > > [qla2xxx] > > > > > [ 177.149165] ? internal_add_timer+0x42/0x70 > > > > > [ 177.153372] qla2xxx_mqueuecommand+0x207/0x2b0 [qla2xxx] > > > > > [ 177.158730] scsi_queue_rq+0x2b7/0xc00 > > > > > [ 177.162501] blk_mq_dispatch_rq_list+0x3ea/0x7e0 > > > > > > > > > > Current code attempt to use CRC IOCB to send the command but > > > > > failed. > > > > > Instead, type 6 IOCB should be used to send the IO. > > > > > > > > > > Clone existing type 6 IOCB code with addition of MQ support > > > > > to allow 32 bytes CDB to go through. > > > > > > > > > > Signed-off-by: Quinn Tran <qutran@marvell.com> > > > > > Cc: Laurence Oberman <loberman@redhat.com> > > > > > Cc: stable@vger.kernel.org > > > > > Signed-off-by: Nilesh Javali <njavali@marvell.com> > > > > > --- > > > > > drivers/scsi/qla2xxx/qla_iocb.c | 270 > > > > > ++++++++++++++++++++++++++++++++ > > > > > drivers/scsi/qla2xxx/qla_nx.h | 4 +- > > > > > 2 files changed, 273 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c > > > > > b/drivers/scsi/qla2xxx/qla_iocb.c > > > > > index 0caa64a7df26..e99ebf7e1c7a 100644 > > > > > --- a/drivers/scsi/qla2xxx/qla_iocb.c > > > > > +++ b/drivers/scsi/qla2xxx/qla_iocb.c > > > > > @@ -11,6 +11,7 @@ > > > > > > > > > > #include <scsi/scsi_tcq.h> > > > > > > > > > > +static int qla_start_scsi_type6(srb_t *sp); > > > > > /** > > > > > * qla2x00_get_cmd_direction() - Determine control_flag data > > > > > direction. > > > > > * @sp: SCSI command > > > > > @@ -1721,6 +1722,8 @@ qla24xx_dif_start_scsi(srb_t *sp) > > > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > > > if (cmd->cmd_len <= 16) > > > > > return qla24xx_start_scsi(sp); > > > > > + else > > > > > + return qla_start_scsi_type6(sp); > > > > > } > > > > > > > > > > /* Setup device pointers. */ > > > > > @@ -2100,6 +2103,8 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp) > > > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > > > if (cmd->cmd_len <= 16) > > > > > return qla2xxx_start_scsi_mq(sp); > > > > > + else > > > > > + return qla_start_scsi_type6(sp); > > > > > } > > > > > > > > > > spin_lock_irqsave(&qpair->qp_lock, flags); > > > > > @@ -4205,3 +4210,268 @@ qla2x00_start_bidir(srb_t *sp, struct > > > > > scsi_qla_host *vha, uint32_t tot_dsds) > > > > > > > > > > return rval; > > > > > } > > > > > + > > > > > +/** > > > > > + * qla_start_scsi_type6() - Send a SCSI command to the ISP > > > > > + * @sp: command to send to the ISP > > > > > + * > > > > > + * Returns non-zero if a failure occurred, else zero. > > > > > + */ > > > > > +static int > > > > > +qla_start_scsi_type6(srb_t *sp) > > > > > +{ > > > > > + int nseg; > > > > > + unsigned long flags; > > > > > + uint32_t *clr_ptr; > > > > > + uint32_t handle; > > > > > + struct cmd_type_6 *cmd_pkt; > > > > > + uint16_t cnt; > > > > > + uint16_t req_cnt; > > > > > + uint16_t tot_dsds; > > > > > + struct req_que *req = NULL; > > > > > + struct rsp_que *rsp; > > > > > + struct scsi_cmnd *cmd = GET_CMD_SP(sp); > > > > > + struct scsi_qla_host *vha = sp->fcport->vha; > > > > > + struct qla_hw_data *ha = vha->hw; > > > > > + struct qla_qpair *qpair = sp->qpair; > > > > > + uint16_t more_dsd_lists = 0; > > > > > + struct dsd_dma *dsd_ptr; > > > > > + uint16_t i; > > > > > + __be32 *fcp_dl; > > > > > + uint8_t additional_cdb_len; > > > > > + struct ct6_dsd *ctx; > > > > > + > > > > > + > > > > > + /* Acquire qpair specific lock */ > > > > > + spin_lock_irqsave(&qpair->qp_lock, flags); > > > > > + > > > > > + /* Setup qpair pointers */ > > > > > + req = qpair->req; > > > > > + rsp = qpair->rsp; > > > > > + > > > > > + /* So we know we haven't pci_map'ed anything yet */ > > > > > + tot_dsds = 0; > > > > > + > > > > > + /* Send marker if required */ > > > > > + if (vha->marker_needed != 0) { > > > > > + if (__qla2x00_marker(vha, qpair, 0, 0, > > > > > MK_SYNC_ALL) > > > > > != QLA_SUCCESS) { > > > > > + spin_unlock_irqrestore(&qpair- > > > > > >qp_lock, > > > > > flags); > > > > > + return QLA_FUNCTION_FAILED; > > > > > + } > > > > > + vha->marker_needed = 0; > > > > > + } > > > > > + > > > > > + handle = qla2xxx_get_next_handle(req); > > > > > + if (handle == 0) > > > > > + goto queuing_error; > > > > > + > > > > > + /* Map the sg table so we have an accurate count of > > > > > sg > > > > > entries needed */ > > > > > + if (scsi_sg_count(cmd)) { > > > > > + nseg = dma_map_sg(&ha->pdev->dev, > > > > > scsi_sglist(cmd), > > > > > + scsi_sg_count(cmd), cmd- > > > > > > sc_data_direction); > > > > > + if (unlikely(!nseg)) > > > > > + goto queuing_error; > > > > > + } else { > > > > > + nseg = 0; > > > > > + } > > > > > + > > > > > + tot_dsds = nseg; > > > > > + > > > > > + /* eventhough driver only need 1 T6 IOCB, FW still > > > > > convert > > > > > DSD to Continueation IOCB */ > > > > > + req_cnt = qla24xx_calc_iocbs(vha, tot_dsds); > > > > > + > > > > > + sp->iores.res_type = RESOURCE_IOCB | RESOURCE_EXCH; > > > > > + sp->iores.exch_cnt = 1; > > > > > + sp->iores.iocb_cnt = req_cnt; > > > > > + > > > > > + if (qla_get_fw_resources(sp->qpair, &sp->iores)) > > > > > + goto queuing_error; > > > > > + > > > > > + more_dsd_lists = qla24xx_calc_dsd_lists(tot_dsds); > > > > > + if ((more_dsd_lists + qpair->dsd_inuse) >= > > > > > NUM_DSD_CHAIN) > > > > > { > > > > > + ql_dbg(ql_dbg_io, vha, 0x3028, > > > > > + "Num of DSD list %d is than %d for > > > > > cmd=%p.\n", > > > > > + more_dsd_lists + qpair->dsd_inuse, > > > > > NUM_DSD_CHAIN, cmd); > > > > > + goto queuing_error; > > > > > + } > > > > > + > > > > > + if (more_dsd_lists <= qpair->dsd_avail) > > > > > + goto sufficient_dsds; > > > > > + else > > > > > + more_dsd_lists -= qpair->dsd_avail; > > > > > + > > > > > + for (i = 0; i < more_dsd_lists; i++) { > > > > > + dsd_ptr = kzalloc(sizeof(*dsd_ptr), > > > > > GFP_ATOMIC); > > > > > + if (!dsd_ptr) { > > > > > + ql_log(ql_log_fatal, vha, 0x3029, > > > > > + "Failed to allocate memory for > > > > > dsd_dma > > > > > for cmd=%p.\n", cmd); > > > > > + goto queuing_error; > > > > > + } > > > > > + INIT_LIST_HEAD(&dsd_ptr->list); > > > > > + > > > > > + dsd_ptr->dsd_addr = dma_pool_alloc(ha- > > > > > > dl_dma_pool, > > > > > + GFP_ATOMIC, &dsd_ptr->dsd_list_dma); > > > > > + if (!dsd_ptr->dsd_addr) { > > > > > + kfree(dsd_ptr); > > > > > + ql_log(ql_log_fatal, vha, 0x302a, > > > > > + "Failed to allocate memory for > > > > > dsd_addr > > > > > for cmd=%p.\n", cmd); > > > > > + goto queuing_error; > > > > > + } > > > > > + list_add_tail(&dsd_ptr->list, &qpair- > > > > > > dsd_list); > > > > > + qpair->dsd_avail++; > > > > > + } > > > > > + > > > > > +sufficient_dsds: > > > > > + req_cnt = 1; > > > > > + > > > > > + if (req->cnt < (req_cnt + 2)) { > > > > > + if (IS_SHADOW_REG_CAPABLE(ha)) { > > > > > + cnt = *req->out_ptr; > > > > > + } else { > > > > > + cnt = > > > > > (uint16_t)rd_reg_dword_relaxed(req- > > > > > > req_q_out); > > > > > + if > > > > > (qla2x00_check_reg16_for_disconnect(vha, > > > > > cnt)) > > > > > + goto queuing_error; > > > > > + } > > > > > + > > > > > + if (req->ring_index < cnt) > > > > > + req->cnt = cnt - req->ring_index; > > > > > + else > > > > > + req->cnt = req->length - (req- > > > > > > ring_index > > > > > - > > > > > cnt); > > > > > + if (req->cnt < (req_cnt + 2)) > > > > > + goto queuing_error; > > > > > + } > > > > > + > > > > > + ctx = &sp->u.scmd.ct6_ctx; > > > > > + > > > > > + memset(ctx, 0, sizeof(struct ct6_dsd)); > > > > > + ctx->fcp_cmnd = dma_pool_zalloc(ha- > > > > > >fcp_cmnd_dma_pool, > > > > > + GFP_ATOMIC, &ctx->fcp_cmnd_dma); > > > > > + if (!ctx->fcp_cmnd) { > > > > > + ql_log(ql_log_fatal, vha, 0x3031, > > > > > + "Failed to allocate fcp_cmnd for > > > > > cmd=%p.\n", > > > > > cmd); > > > > > + goto queuing_error; > > > > > + } > > > > > + > > > > > + /* Initialize the DSD list and dma handle */ > > > > > + INIT_LIST_HEAD(&ctx->dsd_list); > > > > > + ctx->dsd_use_cnt = 0; > > > > > + > > > > > + if (cmd->cmd_len > 16) { > > > > > + additional_cdb_len = cmd->cmd_len - 16; > > > > > + if (cmd->cmd_len % 4 || > > > > > + cmd->cmd_len > QLA_CDB_BUF_SIZE) { > > > > > + /* SCSI command bigger than 16 bytes > > > > > must > > > > > be > > > > > + * multiple of 4 or too big. > > > > > + */ > > > > > + ql_log(ql_log_warn, vha, 0x3033, > > > > > + "scsi cmd len %d not multiple of > > > > > 4 > > > > > for > > > > > cmd=%p.\n", > > > > > + cmd->cmd_len, cmd); > > > > > + goto queuing_error_fcp_cmnd; > > > > > + } > > > > > + ctx->fcp_cmnd_len = 12 + cmd->cmd_len + 4; > > > > > + } else { > > > > > + additional_cdb_len = 0; > > > > > + ctx->fcp_cmnd_len = 12 + 16 + 4; > > > > > + } > > > > > + > > > > > + /* Build command packet. */ > > > > > + req->current_outstanding_cmd = handle; > > > > > + req->outstanding_cmds[handle] = sp; > > > > > + sp->handle = handle; > > > > > + cmd->host_scribble = (unsigned char *)(unsigned > > > > > long)handle; > > > > > + req->cnt -= req_cnt; > > > > > + > > > > > + cmd_pkt = (struct cmd_type_6 *)req->ring_ptr; > > > > > + cmd_pkt->handle = make_handle(req->id, handle); > > > > > + > > > > > + /* Zero out remaining portion of packet. */ > > > > > + /* tagged queuing modifier -- default is > > > > > TSK_SIMPLE > > > > > (0). > > > > > */ > > > > > + clr_ptr = (uint32_t *)cmd_pkt + 2; > > > > > + memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); > > > > > + cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); > > > > > + > > > > > + /* Set NPORT-ID and LUN number*/ > > > > > + cmd_pkt->nport_handle = cpu_to_le16(sp->fcport- > > > > > > loop_id); > > > > > + cmd_pkt->port_id[0] = sp->fcport->d_id.b.al_pa; > > > > > + cmd_pkt->port_id[1] = sp->fcport->d_id.b.area; > > > > > + cmd_pkt->port_id[2] = sp->fcport->d_id.b.domain; > > > > > + cmd_pkt->vp_index = sp->vha->vp_idx; > > > > > + > > > > > + /* Build IOCB segments */ > > > > > + qla24xx_build_scsi_type_6_iocbs(sp, cmd_pkt, > > > > > tot_dsds); > > > > > + > > > > > + int_to_scsilun(cmd->device->lun, &cmd_pkt->lun); > > > > > + host_to_fcp_swap((uint8_t *)&cmd_pkt->lun, > > > > > sizeof(cmd_pkt- > > > > > > lun)); > > > > > + > > > > > + /* build FCP_CMND IU */ > > > > > + int_to_scsilun(cmd->device->lun, &ctx->fcp_cmnd- > > > > > >lun); > > > > > + ctx->fcp_cmnd->additional_cdb_len = > > > > > additional_cdb_len; > > > > > + > > > > > + if (cmd->sc_data_direction == DMA_TO_DEVICE) > > > > > + ctx->fcp_cmnd->additional_cdb_len |= 1; > > > > > + else if (cmd->sc_data_direction == DMA_FROM_DEVICE) > > > > > + ctx->fcp_cmnd->additional_cdb_len |= 2; > > > > > + > > > > > + /* Populate the FCP_PRIO. */ > > > > > + if (ha->flags.fcp_prio_enabled) > > > > > + ctx->fcp_cmnd->task_attribute |= > > > > > + sp->fcport->fcp_prio << 3; > > > > > + > > > > > + memcpy(ctx->fcp_cmnd->cdb, cmd->cmnd, cmd->cmd_len); > > > > > + > > > > > + fcp_dl = (__be32 *)(ctx->fcp_cmnd->cdb + 16 + > > > > > + additional_cdb_len); > > > > > + *fcp_dl = htonl((uint32_t)scsi_bufflen(cmd)); > > > > > + > > > > > + cmd_pkt->fcp_cmnd_dseg_len = cpu_to_le16(ctx- > > > > > > fcp_cmnd_len); > > > > > + put_unaligned_le64(ctx->fcp_cmnd_dma, > > > > > + &cmd_pkt->fcp_cmnd_dseg_address); > > > > > + > > > > > + sp->flags |= SRB_FCP_CMND_DMA_VALID; > > > > > + cmd_pkt->byte_count = > > > > > cpu_to_le32((uint32_t)scsi_bufflen(cmd)); > > > > > + /* Set total data segment count. */ > > > > > + cmd_pkt->entry_count = (uint8_t)req_cnt; > > > > > + > > > > > + wmb(); > > > > > + /* Adjust ring index. */ > > > > > + req->ring_index++; > > > > > + if (req->ring_index == req->length) { > > > > > + req->ring_index = 0; > > > > > + req->ring_ptr = req->ring; > > > > > + } else { > > > > > + req->ring_ptr++; > > > > > + } > > > > > + > > > > > + sp->qpair->cmd_cnt++; > > > > > + sp->flags |= SRB_DMA_VALID; > > > > > + > > > > > + /* Set chip new ring index. */ > > > > > + wrt_reg_dword(req->req_q_in, req->ring_index); > > > > > + > > > > > + /* Manage unprocessed RIO/ZIO commands in response > > > > > queue. > > > > > */ > > > > > + if (vha->flags.process_response_queue && > > > > > + rsp->ring_ptr->signature != RESPONSE_PROCESSED) > > > > > + qla24xx_process_response_queue(vha, rsp); > > > > > + > > > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > > > + > > > > > + return QLA_SUCCESS; > > > > > + > > > > > +queuing_error_fcp_cmnd: > > > > > + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx->fcp_cmnd, > > > > > ctx- > > > > > > fcp_cmnd_dma); > > > > > + > > > > > +queuing_error: > > > > > + if (tot_dsds) > > > > > + scsi_dma_unmap(cmd); > > > > > + > > > > > + qla_put_fw_resources(sp->qpair, &sp->iores); > > > > > + > > > > > + if (sp->u.scmd.crc_ctx) { > > > > > + mempool_free(sp->u.scmd.crc_ctx, ha- > > > > > > ctx_mempool); > > > > > + sp->u.scmd.crc_ctx = NULL; > > > > > + } > > > > > + > > > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > > > + > > > > > + return QLA_FUNCTION_FAILED; > > > > > +} > > > > > diff --git a/drivers/scsi/qla2xxx/qla_nx.h > > > > > b/drivers/scsi/qla2xxx/qla_nx.h > > > > > index 6dc80c8ddf79..5d1bdc15b75c 100644 > > > > > --- a/drivers/scsi/qla2xxx/qla_nx.h > > > > > +++ b/drivers/scsi/qla2xxx/qla_nx.h > > > > > @@ -857,7 +857,9 @@ struct fcp_cmnd { > > > > > uint8_t task_attribute; > > > > > uint8_t task_management; > > > > > uint8_t additional_cdb_len; > > > > > - uint8_t cdb[260]; /* 256 for CDB len and 4 for FCP_DL > > > > > */ > > > > > +#define QLA_CDB_BUF_SIZE 256 > > > > > +#define QLA_FCP_DL_SIZE 4 > > > > > + uint8_t cdb[QLA_CDB_BUF_SIZE + QLA_FCP_DL_SIZE]; /* > > > > > 256 > > > > > for > > > > > CDB len and 4 for FCP_DL */ > > > > > }; > > > > > > > > > > struct dsd_dma { > > > > > > > > QT and Nilesh, Thank you. > > > > I need more time to look at this fully but I will get it tested > > > > in > > > > the > > > > meantime. > > > > I will reply with a review or questions later. > > > > > > > > Thanks for sending this > > > > Laurence > > > > > > @QT and Nilesh > > > Which kernel was this against. > > > Did you guys test this only agains your driver or also upstream > > > I cannot build it clean because There is no structure member > > > dsd_inuse > > > > > > Can you send one for upstream please > > > > > > Patch applied fine to 6.5-rc4 > > > > > > [loberman@segstorage3 linux]$ patch -p1 < qt.patch > > > patching file drivers/scsi/qla2xxx/qla_iocb.c > > > Hunk #2 succeeded at 1718 (offset -4 lines). > > > Hunk #3 succeeded at 2099 (offset -4 lines). > > > Hunk #4 succeeded at 4179 (offset -31 lines). > > > patching file drivers/scsi/qla2xxx/qla_nx.h > > > > > > > > > Build fails > > > > > > drivers/scsi/qla2xxx/qla_iocb.c: In function > > > ‘qla_start_scsi_type6’: > > > drivers/scsi/qla2xxx/qla_iocb.c:4260:29: error: ‘struct > > > qla_qpair’ > > > has > > > no member named ‘dsd_inuse’ > > > if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { > > > ^~ > > > drivers/scsi/qla2xxx/qla_iocb.c:4263:32: error: ‘struct > > > qla_qpair’ > > > has > > > no member named ‘dsd_inuse’ > > > more_dsd_lists + qpair->dsd_inuse, NUM_DSD_CHAIN, cmd); > > > ^~ > > > drivers/scsi/qla2xxx/qla_iocb.c:4267:29: error: ‘struct > > > qla_qpair’ > > > has > > > no member named ‘dsd_avail’ > > > if (more_dsd_lists <= qpair->dsd_avail) > > > ^~ > > > drivers/scsi/qla2xxx/qla_iocb.c:4270:26: error: ‘struct > > > qla_qpair’ > > > has > > > no member named ‘dsd_avail’ > > > more_dsd_lists -= qpair->dsd_avail; > > > ^~ > > > drivers/scsi/qla2xxx/qla_iocb.c:4289:41: error: ‘struct > > > qla_qpair’ > > > has > > > no member named ‘dsd_list’; did you mean ‘hints_list’? > > > list_add_tail(&dsd_ptr->list, &qpair->dsd_list); > > > ^~~~~~~~ > > > hints_list > > > drivers/scsi/qla2xxx/qla_iocb.c:4290:8: error: ‘struct qla_qpair’ > > > has > > > no member named ‘dsd_avail’ > > > qpair->dsd_avail++; > > > > I see where this was done, there are two more patches > > So I will add those and test > > [PATCH 1/2] qla2xxx: Move resource to allow code reuse > > > > > > > OK, I was able to now build and and test this. > To be honest there are a lot of changes. > I just tested the original issue for which I had sent my first patch. > > Unfortunately the test fails in a different way to the orginal BUG > but > this may be firmware behavior on the MSA2050 that takes the device > paths away. > This is because the test passes on an LIO target > > Against 6.5-rc4 > > Test notes > segstorage3 6.5.0-rc4+ > Note that on a MSA2050 it will hang and timeout no more panics as > expected. However as noted we lose devices. > > Starts out fine > > mpathbp (3600c0ff00043feab07a7ff6307000000) dm-8 HPE,MSA 2050 SAN > size=9.3G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw > > -+- policy='service-time 0' prio=50 status=active > > > - 1:0:1:4 sdh 8:112 active ready running > > > - 1:0:4:4 sdaf 65:240 active ready running > > > - 3:0:2:4 sdfk 130:96 active ready running > > `- 3:0:3:4 sdft 130:240 active ready running > `-+- policy='service-time 0' prio=10 status=enabled > |- 1:0:2:4 sdp 8:240 active ready running > |- 1:0:3:4 sdx 65:112 active ready running > |- 3:0:1:4 sdfb 129:208 active ready running > `- 3:0:4:4 sdge 131:160 active ready running > > Do the test > > [root@segstorage3 ~]# sg_write_same -T --lba=1 /dev/mapper/mpathbp > Write same: transport: Host_status=0x03 [DID_TIME_OUT] > Driver_status=0x00 [DRIVER_OK] > > It then caused problems with two of my multipath device paths but > perhaps this is array firmware behavior. > > [ 820.836495] sd 3:0:2:4: [sdfk] tag#1704 FAILED Result: > hostbyte=DID_TRANSPORT_DISRUPTED driverbyte=DRIVER_OK cmd_age=23s > [ 820.836497] sd 3:0:2:4: [sdfk] tag#1704 CDB: Test Unit Ready 00 00 > 00 00 00 00 > > [ 825.926075] sd 3:0:2:0: rejecting I/O to offline device > [ 825.926136] sd 3:0:2:1: rejecting I/O to offline device > [ 825.926165] sd 3:0:2:3: rejecting I/O to offline device > > And two paths get lost > > mpathbp (3600c0ff00043feab07a7ff6307000000) dm-8 HPE,MSA 2050 SAN > size=9.3G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw > > -+- policy='service-time 0' prio=50 status=active > > > - 1:0:4:4 sdaf 65:240 active ready running > > `- 3:0:3:4 sdft 130:240 active ready running > `-+- policy='service-time 0' prio=10 status=enabled > |- 1:0:2:4 sdp 8:240 active ready running > |- 1:0:3:4 sdx 65:112 active ready running > |- 3:0:1:4 sdfb 129:208 active ready running > `- 3:0:4:4 sdge 131:160 active ready running > > I re-scanned but sdh and sdfk are gone from the device tree and do > not > come back with a rescan. > > [ 846.043511] sd 1:0:1:4: [sdh] Synchronizing SCSI cache > [ 846.043536] sd 1:0:1:4: [sdh] Synchronize Cache(10) failed: > Result: > hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK > > [ 820.836495] sd 3:0:2:4: [sdfk] tag#1704 FAILED Result: > hostbyte=DID_TRANSPORT_DISRUPTED driverbyte=DRIVER_OK cmd_age=23s > [ 820.836497] sd 3:0:2:4: [sdfk] tag#1704 CDB: Test Unit Ready 00 00 > 00 00 00 00 > [ 851.166451] sd 3:0:2:4: [sdfk] Synchronizing SCSI cache > [ 851.166477] sd 3:0:2:4: [sdfk] Synchronize Cache(10) failed: > Result: > hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK > > Now using an LIO target via tcm_qla2xxx > > mpathy (3600140549ed41778d414038bf1f1bb42) dm-74 LIO-ORG,test-70 > size=1.0G features='0' hwhandler='1 alua' wp=rw > `-+- policy='service-time 0' prio=50 status=active > |- 1:0:5:105 sdbv 68:144 active ready running > |- 1:0:6:105 sdei 128:160 active ready running > |- 3:0:5:105 sdhz 134:144 active ready running > `- 3:0:6:105 sdkm 66:416 active ready running > > [root@segstorage3 block]# sg_write_same -T --lba=1 /dev/mapper/mpathy > Write same: transport: Host_status=0x07 [DID_ERROR] > Driver_status=0x08 [DRIVER_SENSE] > > No issues with the LIO device paths > > [root@segstorage3 block]# multipath -ll mpathy > mpathy (3600140549ed41778d414038bf1f1bb42) dm-74 LIO-ORG,test-70 > size=1.0G features='0' hwhandler='1 alua' wp=rw > `-+- policy='service-time 0' prio=50 status=active > |- 1:0:5:105 sdbv 68:144 active ready running > |- 1:0:6:105 sdei 128:160 active ready running > |- 3:0:5:105 sdhz 134:144 active ready running > `- 3:0:6:105 sdkm 66:416 active ready running > > So I would like to give a positive test result but will need you > folks > at Marvell to maybe first work with HPE on the MSA2050 behavior. > > Thanks > Laurence Hello Nilesh and Marvell I ran a few more tests, this 32 bytes CDB patch set totally breaks the MSA2050 such that a full storage side reboot of the controller pair is needed to get the paths back. For that reason I am going to NACK this until you chat with HPE. Those MSA2050's are installed everywhere. Not pushing my original fix, but my patch would land up with the MSA2050 timing out but then all would function as it should after the timeout. Something about your change is not making the MSA2050 happy at all. Thanks Laurence
On Sat, 2023-08-05 at 12:54 -0400, Laurence Oberman wrote: > On Fri, 2023-08-04 at 17:35 -0400, Laurence Oberman wrote: > > On Fri, 2023-08-04 at 14:35 -0400, Laurence Oberman wrote: > > > On Fri, 2023-08-04 at 13:51 -0400, Laurence Oberman wrote: > > > > On Fri, 2023-08-04 at 12:58 -0400, Laurence Oberman wrote: > > > > > On Fri, 2023-08-04 at 12:49 +0530, Nilesh Javali wrote: > > > > > > From: Quinn Tran <qutran@marvell.com> > > > > > > > > > > > > System crash when 32 bytes CDB was sent to a non T10-PI > > > > > > disk, > > > > > > > > > > > > [ 177.143279] ? qla2xxx_dif_start_scsi_mq+0xcd8/0xce0 > > > > > > [qla2xxx] > > > > > > [ 177.149165] ? internal_add_timer+0x42/0x70 > > > > > > [ 177.153372] qla2xxx_mqueuecommand+0x207/0x2b0 [qla2xxx] > > > > > > [ 177.158730] scsi_queue_rq+0x2b7/0xc00 > > > > > > [ 177.162501] blk_mq_dispatch_rq_list+0x3ea/0x7e0 > > > > > > > > > > > > Current code attempt to use CRC IOCB to send the command > > > > > > but > > > > > > failed. > > > > > > Instead, type 6 IOCB should be used to send the IO. > > > > > > > > > > > > Clone existing type 6 IOCB code with addition of MQ support > > > > > > to allow 32 bytes CDB to go through. > > > > > > > > > > > > Signed-off-by: Quinn Tran <qutran@marvell.com> > > > > > > Cc: Laurence Oberman <loberman@redhat.com> > > > > > > Cc: stable@vger.kernel.org > > > > > > Signed-off-by: Nilesh Javali <njavali@marvell.com> > > > > > > --- > > > > > > drivers/scsi/qla2xxx/qla_iocb.c | 270 > > > > > > ++++++++++++++++++++++++++++++++ > > > > > > drivers/scsi/qla2xxx/qla_nx.h | 4 +- > > > > > > 2 files changed, 273 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c > > > > > > b/drivers/scsi/qla2xxx/qla_iocb.c > > > > > > index 0caa64a7df26..e99ebf7e1c7a 100644 > > > > > > --- a/drivers/scsi/qla2xxx/qla_iocb.c > > > > > > +++ b/drivers/scsi/qla2xxx/qla_iocb.c > > > > > > @@ -11,6 +11,7 @@ > > > > > > > > > > > > #include <scsi/scsi_tcq.h> > > > > > > > > > > > > +static int qla_start_scsi_type6(srb_t *sp); > > > > > > /** > > > > > > * qla2x00_get_cmd_direction() - Determine control_flag > > > > > > data > > > > > > direction. > > > > > > * @sp: SCSI command > > > > > > @@ -1721,6 +1722,8 @@ qla24xx_dif_start_scsi(srb_t *sp) > > > > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > > > > if (cmd->cmd_len <= 16) > > > > > > return qla24xx_start_scsi(sp); > > > > > > + else > > > > > > + return qla_start_scsi_type6(sp); > > > > > > } > > > > > > > > > > > > /* Setup device pointers. */ > > > > > > @@ -2100,6 +2103,8 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp) > > > > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > > > > if (cmd->cmd_len <= 16) > > > > > > return qla2xxx_start_scsi_mq(sp); > > > > > > + else > > > > > > + return qla_start_scsi_type6(sp); > > > > > > } > > > > > > > > > > > > spin_lock_irqsave(&qpair->qp_lock, flags); > > > > > > @@ -4205,3 +4210,268 @@ qla2x00_start_bidir(srb_t *sp, > > > > > > struct > > > > > > scsi_qla_host *vha, uint32_t tot_dsds) > > > > > > > > > > > > return rval; > > > > > > } > > > > > > + > > > > > > +/** > > > > > > + * qla_start_scsi_type6() - Send a SCSI command to the ISP > > > > > > + * @sp: command to send to the ISP > > > > > > + * > > > > > > + * Returns non-zero if a failure occurred, else zero. > > > > > > + */ > > > > > > +static int > > > > > > +qla_start_scsi_type6(srb_t *sp) > > > > > > +{ > > > > > > + int nseg; > > > > > > + unsigned long flags; > > > > > > + uint32_t *clr_ptr; > > > > > > + uint32_t handle; > > > > > > + struct cmd_type_6 *cmd_pkt; > > > > > > + uint16_t cnt; > > > > > > + uint16_t req_cnt; > > > > > > + uint16_t tot_dsds; > > > > > > + struct req_que *req = NULL; > > > > > > + struct rsp_que *rsp; > > > > > > + struct scsi_cmnd *cmd = GET_CMD_SP(sp); > > > > > > + struct scsi_qla_host *vha = sp->fcport->vha; > > > > > > + struct qla_hw_data *ha = vha->hw; > > > > > > + struct qla_qpair *qpair = sp->qpair; > > > > > > + uint16_t more_dsd_lists = 0; > > > > > > + struct dsd_dma *dsd_ptr; > > > > > > + uint16_t i; > > > > > > + __be32 *fcp_dl; > > > > > > + uint8_t additional_cdb_len; > > > > > > + struct ct6_dsd *ctx; > > > > > > + > > > > > > + > > > > > > + /* Acquire qpair specific lock */ > > > > > > + spin_lock_irqsave(&qpair->qp_lock, flags); > > > > > > + > > > > > > + /* Setup qpair pointers */ > > > > > > + req = qpair->req; > > > > > > + rsp = qpair->rsp; > > > > > > + > > > > > > + /* So we know we haven't pci_map'ed anything yet */ > > > > > > + tot_dsds = 0; > > > > > > + > > > > > > + /* Send marker if required */ > > > > > > + if (vha->marker_needed != 0) { > > > > > > + if (__qla2x00_marker(vha, qpair, 0, 0, > > > > > > MK_SYNC_ALL) > > > > > > != QLA_SUCCESS) { > > > > > > + spin_unlock_irqrestore(&qpair- > > > > > > > qp_lock, > > > > > > flags); > > > > > > + return QLA_FUNCTION_FAILED; > > > > > > + } > > > > > > + vha->marker_needed = 0; > > > > > > + } > > > > > > + > > > > > > + handle = qla2xxx_get_next_handle(req); > > > > > > + if (handle == 0) > > > > > > + goto queuing_error; > > > > > > + > > > > > > + /* Map the sg table so we have an accurate count of > > > > > > sg > > > > > > entries needed */ > > > > > > + if (scsi_sg_count(cmd)) { > > > > > > + nseg = dma_map_sg(&ha->pdev->dev, > > > > > > scsi_sglist(cmd), > > > > > > + scsi_sg_count(cmd), cmd- > > > > > > > sc_data_direction); > > > > > > + if (unlikely(!nseg)) > > > > > > + goto queuing_error; > > > > > > + } else { > > > > > > + nseg = 0; > > > > > > + } > > > > > > + > > > > > > + tot_dsds = nseg; > > > > > > + > > > > > > + /* eventhough driver only need 1 T6 IOCB, FW still > > > > > > convert > > > > > > DSD to Continueation IOCB */ > > > > > > + req_cnt = qla24xx_calc_iocbs(vha, tot_dsds); > > > > > > + > > > > > > + sp->iores.res_type = RESOURCE_IOCB | RESOURCE_EXCH; > > > > > > + sp->iores.exch_cnt = 1; > > > > > > + sp->iores.iocb_cnt = req_cnt; > > > > > > + > > > > > > + if (qla_get_fw_resources(sp->qpair, &sp->iores)) > > > > > > + goto queuing_error; > > > > > > + > > > > > > + more_dsd_lists = qla24xx_calc_dsd_lists(tot_dsds); > > > > > > + if ((more_dsd_lists + qpair->dsd_inuse) >= > > > > > > NUM_DSD_CHAIN) > > > > > > { > > > > > > + ql_dbg(ql_dbg_io, vha, 0x3028, > > > > > > + "Num of DSD list %d is than %d for > > > > > > cmd=%p.\n", > > > > > > + more_dsd_lists + qpair->dsd_inuse, > > > > > > NUM_DSD_CHAIN, cmd); > > > > > > + goto queuing_error; > > > > > > + } > > > > > > + > > > > > > + if (more_dsd_lists <= qpair->dsd_avail) > > > > > > + goto sufficient_dsds; > > > > > > + else > > > > > > + more_dsd_lists -= qpair->dsd_avail; > > > > > > + > > > > > > + for (i = 0; i < more_dsd_lists; i++) { > > > > > > + dsd_ptr = kzalloc(sizeof(*dsd_ptr), > > > > > > GFP_ATOMIC); > > > > > > + if (!dsd_ptr) { > > > > > > + ql_log(ql_log_fatal, vha, 0x3029, > > > > > > + "Failed to allocate memory for > > > > > > dsd_dma > > > > > > for cmd=%p.\n", cmd); > > > > > > + goto queuing_error; > > > > > > + } > > > > > > + INIT_LIST_HEAD(&dsd_ptr->list); > > > > > > + > > > > > > + dsd_ptr->dsd_addr = dma_pool_alloc(ha- > > > > > > > dl_dma_pool, > > > > > > + GFP_ATOMIC, &dsd_ptr- > > > > > > >dsd_list_dma); > > > > > > + if (!dsd_ptr->dsd_addr) { > > > > > > + kfree(dsd_ptr); > > > > > > + ql_log(ql_log_fatal, vha, 0x302a, > > > > > > + "Failed to allocate memory for > > > > > > dsd_addr > > > > > > for cmd=%p.\n", cmd); > > > > > > + goto queuing_error; > > > > > > + } > > > > > > + list_add_tail(&dsd_ptr->list, &qpair- > > > > > > > dsd_list); > > > > > > + qpair->dsd_avail++; > > > > > > + } > > > > > > + > > > > > > +sufficient_dsds: > > > > > > + req_cnt = 1; > > > > > > + > > > > > > + if (req->cnt < (req_cnt + 2)) { > > > > > > + if (IS_SHADOW_REG_CAPABLE(ha)) { > > > > > > + cnt = *req->out_ptr; > > > > > > + } else { > > > > > > + cnt = > > > > > > (uint16_t)rd_reg_dword_relaxed(req- > > > > > > > req_q_out); > > > > > > + if > > > > > > (qla2x00_check_reg16_for_disconnect(vha, > > > > > > cnt)) > > > > > > + goto queuing_error; > > > > > > + } > > > > > > + > > > > > > + if (req->ring_index < cnt) > > > > > > + req->cnt = cnt - req->ring_index; > > > > > > + else > > > > > > + req->cnt = req->length - (req- > > > > > > > ring_index > > > > > > - > > > > > > cnt); > > > > > > + if (req->cnt < (req_cnt + 2)) > > > > > > + goto queuing_error; > > > > > > + } > > > > > > + > > > > > > + ctx = &sp->u.scmd.ct6_ctx; > > > > > > + > > > > > > + memset(ctx, 0, sizeof(struct ct6_dsd)); > > > > > > + ctx->fcp_cmnd = dma_pool_zalloc(ha- > > > > > > > fcp_cmnd_dma_pool, > > > > > > + GFP_ATOMIC, &ctx->fcp_cmnd_dma); > > > > > > + if (!ctx->fcp_cmnd) { > > > > > > + ql_log(ql_log_fatal, vha, 0x3031, > > > > > > + "Failed to allocate fcp_cmnd for > > > > > > cmd=%p.\n", > > > > > > cmd); > > > > > > + goto queuing_error; > > > > > > + } > > > > > > + > > > > > > + /* Initialize the DSD list and dma handle */ > > > > > > + INIT_LIST_HEAD(&ctx->dsd_list); > > > > > > + ctx->dsd_use_cnt = 0; > > > > > > + > > > > > > + if (cmd->cmd_len > 16) { > > > > > > + additional_cdb_len = cmd->cmd_len - 16; > > > > > > + if (cmd->cmd_len % 4 || > > > > > > + cmd->cmd_len > QLA_CDB_BUF_SIZE) { > > > > > > + /* SCSI command bigger than 16 > > > > > > bytes > > > > > > must > > > > > > be > > > > > > + * multiple of 4 or too big. > > > > > > + */ > > > > > > + ql_log(ql_log_warn, vha, 0x3033, > > > > > > + "scsi cmd len %d not multiple > > > > > > of > > > > > > 4 > > > > > > for > > > > > > cmd=%p.\n", > > > > > > + cmd->cmd_len, cmd); > > > > > > + goto queuing_error_fcp_cmnd; > > > > > > + } > > > > > > + ctx->fcp_cmnd_len = 12 + cmd->cmd_len + 4; > > > > > > + } else { > > > > > > + additional_cdb_len = 0; > > > > > > + ctx->fcp_cmnd_len = 12 + 16 + 4; > > > > > > + } > > > > > > + > > > > > > + /* Build command packet. */ > > > > > > + req->current_outstanding_cmd = handle; > > > > > > + req->outstanding_cmds[handle] = sp; > > > > > > + sp->handle = handle; > > > > > > + cmd->host_scribble = (unsigned char *)(unsigned > > > > > > long)handle; > > > > > > + req->cnt -= req_cnt; > > > > > > + > > > > > > + cmd_pkt = (struct cmd_type_6 *)req->ring_ptr; > > > > > > + cmd_pkt->handle = make_handle(req->id, handle); > > > > > > + > > > > > > + /* Zero out remaining portion of packet. */ > > > > > > + /* tagged queuing modifier -- default is > > > > > > TSK_SIMPLE > > > > > > (0). > > > > > > */ > > > > > > + clr_ptr = (uint32_t *)cmd_pkt + 2; > > > > > > + memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); > > > > > > + cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); > > > > > > + > > > > > > + /* Set NPORT-ID and LUN number*/ > > > > > > + cmd_pkt->nport_handle = cpu_to_le16(sp->fcport- > > > > > > > loop_id); > > > > > > + cmd_pkt->port_id[0] = sp->fcport->d_id.b.al_pa; > > > > > > + cmd_pkt->port_id[1] = sp->fcport->d_id.b.area; > > > > > > + cmd_pkt->port_id[2] = sp->fcport->d_id.b.domain; > > > > > > + cmd_pkt->vp_index = sp->vha->vp_idx; > > > > > > + > > > > > > + /* Build IOCB segments */ > > > > > > + qla24xx_build_scsi_type_6_iocbs(sp, cmd_pkt, > > > > > > tot_dsds); > > > > > > + > > > > > > + int_to_scsilun(cmd->device->lun, &cmd_pkt->lun); > > > > > > + host_to_fcp_swap((uint8_t *)&cmd_pkt->lun, > > > > > > sizeof(cmd_pkt- > > > > > > > lun)); > > > > > > + > > > > > > + /* build FCP_CMND IU */ > > > > > > + int_to_scsilun(cmd->device->lun, &ctx->fcp_cmnd- > > > > > > > lun); > > > > > > + ctx->fcp_cmnd->additional_cdb_len = > > > > > > additional_cdb_len; > > > > > > + > > > > > > + if (cmd->sc_data_direction == DMA_TO_DEVICE) > > > > > > + ctx->fcp_cmnd->additional_cdb_len |= 1; > > > > > > + else if (cmd->sc_data_direction == DMA_FROM_DEVICE) > > > > > > + ctx->fcp_cmnd->additional_cdb_len |= 2; > > > > > > + > > > > > > + /* Populate the FCP_PRIO. */ > > > > > > + if (ha->flags.fcp_prio_enabled) > > > > > > + ctx->fcp_cmnd->task_attribute |= > > > > > > + sp->fcport->fcp_prio << 3; > > > > > > + > > > > > > + memcpy(ctx->fcp_cmnd->cdb, cmd->cmnd, cmd- > > > > > > >cmd_len); > > > > > > + > > > > > > + fcp_dl = (__be32 *)(ctx->fcp_cmnd->cdb + 16 + > > > > > > + additional_cdb_len); > > > > > > + *fcp_dl = htonl((uint32_t)scsi_bufflen(cmd)); > > > > > > + > > > > > > + cmd_pkt->fcp_cmnd_dseg_len = cpu_to_le16(ctx- > > > > > > > fcp_cmnd_len); > > > > > > + put_unaligned_le64(ctx->fcp_cmnd_dma, > > > > > > + &cmd_pkt- > > > > > > >fcp_cmnd_dseg_address); > > > > > > + > > > > > > + sp->flags |= SRB_FCP_CMND_DMA_VALID; > > > > > > + cmd_pkt->byte_count = > > > > > > cpu_to_le32((uint32_t)scsi_bufflen(cmd)); > > > > > > + /* Set total data segment count. */ > > > > > > + cmd_pkt->entry_count = (uint8_t)req_cnt; > > > > > > + > > > > > > + wmb(); > > > > > > + /* Adjust ring index. */ > > > > > > + req->ring_index++; > > > > > > + if (req->ring_index == req->length) { > > > > > > + req->ring_index = 0; > > > > > > + req->ring_ptr = req->ring; > > > > > > + } else { > > > > > > + req->ring_ptr++; > > > > > > + } > > > > > > + > > > > > > + sp->qpair->cmd_cnt++; > > > > > > + sp->flags |= SRB_DMA_VALID; > > > > > > + > > > > > > + /* Set chip new ring index. */ > > > > > > + wrt_reg_dword(req->req_q_in, req->ring_index); > > > > > > + > > > > > > + /* Manage unprocessed RIO/ZIO commands in response > > > > > > queue. > > > > > > */ > > > > > > + if (vha->flags.process_response_queue && > > > > > > + rsp->ring_ptr->signature != RESPONSE_PROCESSED) > > > > > > + qla24xx_process_response_queue(vha, rsp); > > > > > > + > > > > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > > > > + > > > > > > + return QLA_SUCCESS; > > > > > > + > > > > > > +queuing_error_fcp_cmnd: > > > > > > + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx->fcp_cmnd, > > > > > > ctx- > > > > > > > fcp_cmnd_dma); > > > > > > + > > > > > > +queuing_error: > > > > > > + if (tot_dsds) > > > > > > + scsi_dma_unmap(cmd); > > > > > > + > > > > > > + qla_put_fw_resources(sp->qpair, &sp->iores); > > > > > > + > > > > > > + if (sp->u.scmd.crc_ctx) { > > > > > > + mempool_free(sp->u.scmd.crc_ctx, ha- > > > > > > > ctx_mempool); > > > > > > + sp->u.scmd.crc_ctx = NULL; > > > > > > + } > > > > > > + > > > > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > > > > + > > > > > > + return QLA_FUNCTION_FAILED; > > > > > > +} > > > > > > diff --git a/drivers/scsi/qla2xxx/qla_nx.h > > > > > > b/drivers/scsi/qla2xxx/qla_nx.h > > > > > > index 6dc80c8ddf79..5d1bdc15b75c 100644 > > > > > > --- a/drivers/scsi/qla2xxx/qla_nx.h > > > > > > +++ b/drivers/scsi/qla2xxx/qla_nx.h > > > > > > @@ -857,7 +857,9 @@ struct fcp_cmnd { > > > > > > uint8_t task_attribute; > > > > > > uint8_t task_management; > > > > > > uint8_t additional_cdb_len; > > > > > > - uint8_t cdb[260]; /* 256 for CDB len and 4 for > > > > > > FCP_DL > > > > > > */ > > > > > > +#define QLA_CDB_BUF_SIZE 256 > > > > > > +#define QLA_FCP_DL_SIZE 4 > > > > > > + uint8_t cdb[QLA_CDB_BUF_SIZE + QLA_FCP_DL_SIZE]; /* > > > > > > 256 > > > > > > for > > > > > > CDB len and 4 for FCP_DL */ > > > > > > }; > > > > > > > > > > > > struct dsd_dma { > > > > > > > > > > QT and Nilesh, Thank you. > > > > > I need more time to look at this fully but I will get it > > > > > tested > > > > > in > > > > > the > > > > > meantime. > > > > > I will reply with a review or questions later. > > > > > > > > > > Thanks for sending this > > > > > Laurence > > > > > > > > @QT and Nilesh > > > > Which kernel was this against. > > > > Did you guys test this only agains your driver or also upstream > > > > I cannot build it clean because There is no structure member > > > > dsd_inuse > > > > > > > > Can you send one for upstream please > > > > > > > > Patch applied fine to 6.5-rc4 > > > > > > > > [loberman@segstorage3 linux]$ patch -p1 < qt.patch > > > > patching file drivers/scsi/qla2xxx/qla_iocb.c > > > > Hunk #2 succeeded at 1718 (offset -4 lines). > > > > Hunk #3 succeeded at 2099 (offset -4 lines). > > > > Hunk #4 succeeded at 4179 (offset -31 lines). > > > > patching file drivers/scsi/qla2xxx/qla_nx.h > > > > > > > > > > > > Build fails > > > > > > > > drivers/scsi/qla2xxx/qla_iocb.c: In function > > > > ‘qla_start_scsi_type6’: > > > > drivers/scsi/qla2xxx/qla_iocb.c:4260:29: error: ‘struct > > > > qla_qpair’ > > > > has > > > > no member named ‘dsd_inuse’ > > > > if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { > > > > ^~ > > > > drivers/scsi/qla2xxx/qla_iocb.c:4263:32: error: ‘struct > > > > qla_qpair’ > > > > has > > > > no member named ‘dsd_inuse’ > > > > more_dsd_lists + qpair->dsd_inuse, NUM_DSD_CHAIN, > > > > cmd); > > > > ^~ > > > > drivers/scsi/qla2xxx/qla_iocb.c:4267:29: error: ‘struct > > > > qla_qpair’ > > > > has > > > > no member named ‘dsd_avail’ > > > > if (more_dsd_lists <= qpair->dsd_avail) > > > > ^~ > > > > drivers/scsi/qla2xxx/qla_iocb.c:4270:26: error: ‘struct > > > > qla_qpair’ > > > > has > > > > no member named ‘dsd_avail’ > > > > more_dsd_lists -= qpair->dsd_avail; > > > > ^~ > > > > drivers/scsi/qla2xxx/qla_iocb.c:4289:41: error: ‘struct > > > > qla_qpair’ > > > > has > > > > no member named ‘dsd_list’; did you mean ‘hints_list’? > > > > list_add_tail(&dsd_ptr->list, &qpair->dsd_list); > > > > ^~~~~~~~ > > > > hints_list > > > > drivers/scsi/qla2xxx/qla_iocb.c:4290:8: error: ‘struct > > > > qla_qpair’ > > > > has > > > > no member named ‘dsd_avail’ > > > > qpair->dsd_avail++; > > > > > > I see where this was done, there are two more patches > > > So I will add those and test > > > [PATCH 1/2] qla2xxx: Move resource to allow code reuse > > > > > > > > > > > OK, I was able to now build and and test this. > > To be honest there are a lot of changes. > > I just tested the original issue for which I had sent my first > > patch. > > > > Unfortunately the test fails in a different way to the orginal BUG > > but > > this may be firmware behavior on the MSA2050 that takes the device > > paths away. > > This is because the test passes on an LIO target > > > > Against 6.5-rc4 > > > > Test notes > > segstorage3 6.5.0-rc4+ > > Note that on a MSA2050 it will hang and timeout no more panics as > > expected. However as noted we lose devices. > > > > Starts out fine > > > > mpathbp (3600c0ff00043feab07a7ff6307000000) dm-8 HPE,MSA 2050 SAN > > size=9.3G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw > > > -+- policy='service-time 0' prio=50 status=active > > > > - 1:0:1:4 sdh 8:112 active ready running > > > > - 1:0:4:4 sdaf 65:240 active ready running > > > > - 3:0:2:4 sdfk 130:96 active ready running > > > `- 3:0:3:4 sdft 130:240 active ready running > > `-+- policy='service-time 0' prio=10 status=enabled > > |- 1:0:2:4 sdp 8:240 active ready running > > |- 1:0:3:4 sdx 65:112 active ready running > > |- 3:0:1:4 sdfb 129:208 active ready running > > `- 3:0:4:4 sdge 131:160 active ready running > > > > Do the test > > > > [root@segstorage3 ~]# sg_write_same -T --lba=1 /dev/mapper/mpathbp > > Write same: transport: Host_status=0x03 [DID_TIME_OUT] > > Driver_status=0x00 [DRIVER_OK] > > > > It then caused problems with two of my multipath device paths but > > perhaps this is array firmware behavior. > > > > [ 820.836495] sd 3:0:2:4: [sdfk] tag#1704 FAILED Result: > > hostbyte=DID_TRANSPORT_DISRUPTED driverbyte=DRIVER_OK cmd_age=23s > > [ 820.836497] sd 3:0:2:4: [sdfk] tag#1704 CDB: Test Unit Ready 00 > > 00 > > 00 00 00 00 > > > > [ 825.926075] sd 3:0:2:0: rejecting I/O to offline device > > [ 825.926136] sd 3:0:2:1: rejecting I/O to offline device > > [ 825.926165] sd 3:0:2:3: rejecting I/O to offline device > > > > And two paths get lost > > > > mpathbp (3600c0ff00043feab07a7ff6307000000) dm-8 HPE,MSA 2050 SAN > > size=9.3G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw > > > -+- policy='service-time 0' prio=50 status=active > > > > - 1:0:4:4 sdaf 65:240 active ready running > > > `- 3:0:3:4 sdft 130:240 active ready running > > `-+- policy='service-time 0' prio=10 status=enabled > > |- 1:0:2:4 sdp 8:240 active ready running > > |- 1:0:3:4 sdx 65:112 active ready running > > |- 3:0:1:4 sdfb 129:208 active ready running > > `- 3:0:4:4 sdge 131:160 active ready running > > > > I re-scanned but sdh and sdfk are gone from the device tree and do > > not > > come back with a rescan. > > > > [ 846.043511] sd 1:0:1:4: [sdh] Synchronizing SCSI cache > > [ 846.043536] sd 1:0:1:4: [sdh] Synchronize Cache(10) failed: > > Result: > > hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK > > > > [ 820.836495] sd 3:0:2:4: [sdfk] tag#1704 FAILED Result: > > hostbyte=DID_TRANSPORT_DISRUPTED driverbyte=DRIVER_OK cmd_age=23s > > [ 820.836497] sd 3:0:2:4: [sdfk] tag#1704 CDB: Test Unit Ready 00 > > 00 > > 00 00 00 00 > > [ 851.166451] sd 3:0:2:4: [sdfk] Synchronizing SCSI cache > > [ 851.166477] sd 3:0:2:4: [sdfk] Synchronize Cache(10) failed: > > Result: > > hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK > > > > Now using an LIO target via tcm_qla2xxx > > > > mpathy (3600140549ed41778d414038bf1f1bb42) dm-74 LIO-ORG,test-70 > > size=1.0G features='0' hwhandler='1 alua' wp=rw > > `-+- policy='service-time 0' prio=50 status=active > > |- 1:0:5:105 sdbv 68:144 active ready running > > |- 1:0:6:105 sdei 128:160 active ready running > > |- 3:0:5:105 sdhz 134:144 active ready running > > `- 3:0:6:105 sdkm 66:416 active ready running > > > > [root@segstorage3 block]# sg_write_same -T --lba=1 > > /dev/mapper/mpathy > > Write same: transport: Host_status=0x07 [DID_ERROR] > > Driver_status=0x08 [DRIVER_SENSE] > > > > No issues with the LIO device paths > > > > [root@segstorage3 block]# multipath -ll mpathy > > mpathy (3600140549ed41778d414038bf1f1bb42) dm-74 LIO-ORG,test-70 > > size=1.0G features='0' hwhandler='1 alua' wp=rw > > `-+- policy='service-time 0' prio=50 status=active > > |- 1:0:5:105 sdbv 68:144 active ready running > > |- 1:0:6:105 sdei 128:160 active ready running > > |- 3:0:5:105 sdhz 134:144 active ready running > > `- 3:0:6:105 sdkm 66:416 active ready running > > > > So I would like to give a positive test result but will need you > > folks > > at Marvell to maybe first work with HPE on the MSA2050 behavior. > > > > Thanks > > Laurence > Hello Nilesh and Marvell > > I ran a few more tests, this 32 bytes CDB patch set totally breaks > the > MSA2050 such that a full storage side reboot of the controller pair > is > needed to get the paths back. > > For that reason I am going to NACK this until you chat with HPE. > Those MSA2050's are installed everywhere. > > Not pushing my original fix, but my patch would land up with the > MSA2050 timing out but then all would function as it should after the > timeout. Something about your change is not making the MSA2050 happy > at > all. > > Thanks > Laurence > > > Turns out the patch I had sent earlier also makes the MSA2050 unhappy so I will reach out to HPE about your submission and the behavior and get back to you. Again, the LIO seems to work fine with your patch set. This patch below also breaks the MSA, I guess I did not not see it during my first set of tests diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c index 730d8609276c..32809e556ec6 100644 --- a/drivers/scsi/qla2xxx/qla_iocb.c +++ b/drivers/scsi/qla2xxx/qla_iocb.c @@ -1386,7 +1386,8 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, struct cmd_type_crc_2 *cmd_pkt, if ((scsi_get_prot_op(cmd) == SCSI_PROT_READ_INSERT) || (scsi_get_prot_op(cmd) == SCSI_PROT_WRITE_STRIP) || (scsi_get_prot_op(cmd) == SCSI_PROT_READ_STRIP) || - (scsi_get_prot_op(cmd) == SCSI_PROT_WRITE_INSERT)) + (scsi_get_prot_op(cmd) == SCSI_PROT_WRITE_INSERT) || + (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL)) bundling = 0; /* Allocate CRC context from global pool */ @@ -1448,6 +1449,9 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, struct cmd_type_crc_2 *cmd_pkt, dif_bytes = (data_bytes / blk_size) * 8; switch (scsi_get_prot_op(GET_CMD_SP(sp))) { + case SCSI_PROT_NORMAL: + total_bytes = data_bytes; + break; case SCSI_PROT_READ_INSERT: case SCSI_PROT_WRITE_STRIP: total_bytes = data_bytes;
On Sat, 2023-08-05 at 14:16 -0400, Laurence Oberman wrote: > On Sat, 2023-08-05 at 12:54 -0400, Laurence Oberman wrote: > > On Fri, 2023-08-04 at 17:35 -0400, Laurence Oberman wrote: > > > On Fri, 2023-08-04 at 14:35 -0400, Laurence Oberman wrote: > > > > On Fri, 2023-08-04 at 13:51 -0400, Laurence Oberman wrote: > > > > > On Fri, 2023-08-04 at 12:58 -0400, Laurence Oberman wrote: > > > > > > On Fri, 2023-08-04 at 12:49 +0530, Nilesh Javali wrote: > > > > > > > From: Quinn Tran <qutran@marvell.com> > > > > > > > > > > > > > > System crash when 32 bytes CDB was sent to a non T10-PI > > > > > > > disk, > > > > > > > > > > > > > > [ 177.143279] ? qla2xxx_dif_start_scsi_mq+0xcd8/0xce0 > > > > > > > [qla2xxx] > > > > > > > [ 177.149165] ? internal_add_timer+0x42/0x70 > > > > > > > [ 177.153372] qla2xxx_mqueuecommand+0x207/0x2b0 > > > > > > > [qla2xxx] > > > > > > > [ 177.158730] scsi_queue_rq+0x2b7/0xc00 > > > > > > > [ 177.162501] blk_mq_dispatch_rq_list+0x3ea/0x7e0 > > > > > > > > > > > > > > Current code attempt to use CRC IOCB to send the command > > > > > > > but > > > > > > > failed. > > > > > > > Instead, type 6 IOCB should be used to send the IO. > > > > > > > > > > > > > > Clone existing type 6 IOCB code with addition of MQ > > > > > > > support > > > > > > > to allow 32 bytes CDB to go through. > > > > > > > > > > > > > > Signed-off-by: Quinn Tran <qutran@marvell.com> > > > > > > > Cc: Laurence Oberman <loberman@redhat.com> > > > > > > > Cc: stable@vger.kernel.org > > > > > > > Signed-off-by: Nilesh Javali <njavali@marvell.com> > > > > > > > --- > > > > > > > drivers/scsi/qla2xxx/qla_iocb.c | 270 > > > > > > > ++++++++++++++++++++++++++++++++ > > > > > > > drivers/scsi/qla2xxx/qla_nx.h | 4 +- > > > > > > > 2 files changed, 273 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c > > > > > > > b/drivers/scsi/qla2xxx/qla_iocb.c > > > > > > > index 0caa64a7df26..e99ebf7e1c7a 100644 > > > > > > > --- a/drivers/scsi/qla2xxx/qla_iocb.c > > > > > > > +++ b/drivers/scsi/qla2xxx/qla_iocb.c > > > > > > > @@ -11,6 +11,7 @@ > > > > > > > > > > > > > > #include <scsi/scsi_tcq.h> > > > > > > > > > > > > > > +static int qla_start_scsi_type6(srb_t *sp); > > > > > > > /** > > > > > > > * qla2x00_get_cmd_direction() - Determine control_flag > > > > > > > data > > > > > > > direction. > > > > > > > * @sp: SCSI command > > > > > > > @@ -1721,6 +1722,8 @@ qla24xx_dif_start_scsi(srb_t *sp) > > > > > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > > > > > if (cmd->cmd_len <= 16) > > > > > > > return qla24xx_start_scsi(sp); > > > > > > > + else > > > > > > > + return qla_start_scsi_type6(sp); > > > > > > > } > > > > > > > > > > > > > > /* Setup device pointers. */ > > > > > > > @@ -2100,6 +2103,8 @@ qla2xxx_dif_start_scsi_mq(srb_t > > > > > > > *sp) > > > > > > > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > > > > > > > if (cmd->cmd_len <= 16) > > > > > > > return qla2xxx_start_scsi_mq(sp); > > > > > > > + else > > > > > > > + return qla_start_scsi_type6(sp); > > > > > > > } > > > > > > > > > > > > > > spin_lock_irqsave(&qpair->qp_lock, flags); > > > > > > > @@ -4205,3 +4210,268 @@ qla2x00_start_bidir(srb_t *sp, > > > > > > > struct > > > > > > > scsi_qla_host *vha, uint32_t tot_dsds) > > > > > > > > > > > > > > return rval; > > > > > > > } > > > > > > > + > > > > > > > +/** > > > > > > > + * qla_start_scsi_type6() - Send a SCSI command to the > > > > > > > ISP > > > > > > > + * @sp: command to send to the ISP > > > > > > > + * > > > > > > > + * Returns non-zero if a failure occurred, else zero. > > > > > > > + */ > > > > > > > +static int > > > > > > > +qla_start_scsi_type6(srb_t *sp) > > > > > > > +{ > > > > > > > + int nseg; > > > > > > > + unsigned long flags; > > > > > > > + uint32_t *clr_ptr; > > > > > > > + uint32_t handle; > > > > > > > + struct cmd_type_6 *cmd_pkt; > > > > > > > + uint16_t cnt; > > > > > > > + uint16_t req_cnt; > > > > > > > + uint16_t tot_dsds; > > > > > > > + struct req_que *req = NULL; > > > > > > > + struct rsp_que *rsp; > > > > > > > + struct scsi_cmnd *cmd = GET_CMD_SP(sp); > > > > > > > + struct scsi_qla_host *vha = sp->fcport->vha; > > > > > > > + struct qla_hw_data *ha = vha->hw; > > > > > > > + struct qla_qpair *qpair = sp->qpair; > > > > > > > + uint16_t more_dsd_lists = 0; > > > > > > > + struct dsd_dma *dsd_ptr; > > > > > > > + uint16_t i; > > > > > > > + __be32 *fcp_dl; > > > > > > > + uint8_t additional_cdb_len; > > > > > > > + struct ct6_dsd *ctx; > > > > > > > + > > > > > > > + > > > > > > > + /* Acquire qpair specific lock */ > > > > > > > + spin_lock_irqsave(&qpair->qp_lock, flags); > > > > > > > + > > > > > > > + /* Setup qpair pointers */ > > > > > > > + req = qpair->req; > > > > > > > + rsp = qpair->rsp; > > > > > > > + > > > > > > > + /* So we know we haven't pci_map'ed anything yet > > > > > > > */ > > > > > > > + tot_dsds = 0; > > > > > > > + > > > > > > > + /* Send marker if required */ > > > > > > > + if (vha->marker_needed != 0) { > > > > > > > + if (__qla2x00_marker(vha, qpair, 0, 0, > > > > > > > MK_SYNC_ALL) > > > > > > > != QLA_SUCCESS) { > > > > > > > + spin_unlock_irqrestore(&qpair- > > > > > > > > qp_lock, > > > > > > > flags); > > > > > > > + return QLA_FUNCTION_FAILED; > > > > > > > + } > > > > > > > + vha->marker_needed = 0; > > > > > > > + } > > > > > > > + > > > > > > > + handle = qla2xxx_get_next_handle(req); > > > > > > > + if (handle == 0) > > > > > > > + goto queuing_error; > > > > > > > + > > > > > > > + /* Map the sg table so we have an accurate count > > > > > > > of > > > > > > > sg > > > > > > > entries needed */ > > > > > > > + if (scsi_sg_count(cmd)) { > > > > > > > + nseg = dma_map_sg(&ha->pdev->dev, > > > > > > > scsi_sglist(cmd), > > > > > > > + scsi_sg_count(cmd), > > > > > > > cmd- > > > > > > > > sc_data_direction); > > > > > > > + if (unlikely(!nseg)) > > > > > > > + goto queuing_error; > > > > > > > + } else { > > > > > > > + nseg = 0; > > > > > > > + } > > > > > > > + > > > > > > > + tot_dsds = nseg; > > > > > > > + > > > > > > > + /* eventhough driver only need 1 T6 IOCB, FW > > > > > > > still > > > > > > > convert > > > > > > > DSD to Continueation IOCB */ > > > > > > > + req_cnt = qla24xx_calc_iocbs(vha, tot_dsds); > > > > > > > + > > > > > > > + sp->iores.res_type = RESOURCE_IOCB | > > > > > > > RESOURCE_EXCH; > > > > > > > + sp->iores.exch_cnt = 1; > > > > > > > + sp->iores.iocb_cnt = req_cnt; > > > > > > > + > > > > > > > + if (qla_get_fw_resources(sp->qpair, &sp->iores)) > > > > > > > + goto queuing_error; > > > > > > > + > > > > > > > + more_dsd_lists = > > > > > > > qla24xx_calc_dsd_lists(tot_dsds); > > > > > > > + if ((more_dsd_lists + qpair->dsd_inuse) >= > > > > > > > NUM_DSD_CHAIN) > > > > > > > { > > > > > > > + ql_dbg(ql_dbg_io, vha, 0x3028, > > > > > > > + "Num of DSD list %d is than %d for > > > > > > > cmd=%p.\n", > > > > > > > + more_dsd_lists + qpair->dsd_inuse, > > > > > > > NUM_DSD_CHAIN, cmd); > > > > > > > + goto queuing_error; > > > > > > > + } > > > > > > > + > > > > > > > + if (more_dsd_lists <= qpair->dsd_avail) > > > > > > > + goto sufficient_dsds; > > > > > > > + else > > > > > > > + more_dsd_lists -= qpair->dsd_avail; > > > > > > > + > > > > > > > + for (i = 0; i < more_dsd_lists; i++) { > > > > > > > + dsd_ptr = kzalloc(sizeof(*dsd_ptr), > > > > > > > GFP_ATOMIC); > > > > > > > + if (!dsd_ptr) { > > > > > > > + ql_log(ql_log_fatal, vha, 0x3029, > > > > > > > + "Failed to allocate memory > > > > > > > for > > > > > > > dsd_dma > > > > > > > for cmd=%p.\n", cmd); > > > > > > > + goto queuing_error; > > > > > > > + } > > > > > > > + INIT_LIST_HEAD(&dsd_ptr->list); > > > > > > > + > > > > > > > + dsd_ptr->dsd_addr = dma_pool_alloc(ha- > > > > > > > > dl_dma_pool, > > > > > > > + GFP_ATOMIC, &dsd_ptr- > > > > > > > > dsd_list_dma); > > > > > > > + if (!dsd_ptr->dsd_addr) { > > > > > > > + kfree(dsd_ptr); > > > > > > > + ql_log(ql_log_fatal, vha, 0x302a, > > > > > > > + "Failed to allocate memory > > > > > > > for > > > > > > > dsd_addr > > > > > > > for cmd=%p.\n", cmd); > > > > > > > + goto queuing_error; > > > > > > > + } > > > > > > > + list_add_tail(&dsd_ptr->list, &qpair- > > > > > > > > dsd_list); > > > > > > > + qpair->dsd_avail++; > > > > > > > + } > > > > > > > + > > > > > > > +sufficient_dsds: > > > > > > > + req_cnt = 1; > > > > > > > + > > > > > > > + if (req->cnt < (req_cnt + 2)) { > > > > > > > + if (IS_SHADOW_REG_CAPABLE(ha)) { > > > > > > > + cnt = *req->out_ptr; > > > > > > > + } else { > > > > > > > + cnt = > > > > > > > (uint16_t)rd_reg_dword_relaxed(req- > > > > > > > > req_q_out); > > > > > > > + if > > > > > > > (qla2x00_check_reg16_for_disconnect(vha, > > > > > > > cnt)) > > > > > > > + goto queuing_error; > > > > > > > + } > > > > > > > + > > > > > > > + if (req->ring_index < cnt) > > > > > > > + req->cnt = cnt - req->ring_index; > > > > > > > + else > > > > > > > + req->cnt = req->length - (req- > > > > > > > > ring_index > > > > > > > - > > > > > > > cnt); > > > > > > > + if (req->cnt < (req_cnt + 2)) > > > > > > > + goto queuing_error; > > > > > > > + } > > > > > > > + > > > > > > > + ctx = &sp->u.scmd.ct6_ctx; > > > > > > > + > > > > > > > + memset(ctx, 0, sizeof(struct ct6_dsd)); > > > > > > > + ctx->fcp_cmnd = dma_pool_zalloc(ha- > > > > > > > > fcp_cmnd_dma_pool, > > > > > > > + GFP_ATOMIC, &ctx->fcp_cmnd_dma); > > > > > > > + if (!ctx->fcp_cmnd) { > > > > > > > + ql_log(ql_log_fatal, vha, 0x3031, > > > > > > > + "Failed to allocate fcp_cmnd for > > > > > > > cmd=%p.\n", > > > > > > > cmd); > > > > > > > + goto queuing_error; > > > > > > > + } > > > > > > > + > > > > > > > + /* Initialize the DSD list and dma handle */ > > > > > > > + INIT_LIST_HEAD(&ctx->dsd_list); > > > > > > > + ctx->dsd_use_cnt = 0; > > > > > > > + > > > > > > > + if (cmd->cmd_len > 16) { > > > > > > > + additional_cdb_len = cmd->cmd_len - 16; > > > > > > > + if (cmd->cmd_len % 4 || > > > > > > > + cmd->cmd_len > QLA_CDB_BUF_SIZE) { > > > > > > > + /* SCSI command bigger than 16 > > > > > > > bytes > > > > > > > must > > > > > > > be > > > > > > > + * multiple of 4 or too big. > > > > > > > + */ > > > > > > > + ql_log(ql_log_warn, vha, 0x3033, > > > > > > > + "scsi cmd len %d not multiple > > > > > > > of > > > > > > > 4 > > > > > > > for > > > > > > > cmd=%p.\n", > > > > > > > + cmd->cmd_len, cmd); > > > > > > > + goto queuing_error_fcp_cmnd; > > > > > > > + } > > > > > > > + ctx->fcp_cmnd_len = 12 + cmd->cmd_len + > > > > > > > 4; > > > > > > > + } else { > > > > > > > + additional_cdb_len = 0; > > > > > > > + ctx->fcp_cmnd_len = 12 + 16 + 4; > > > > > > > + } > > > > > > > + > > > > > > > + /* Build command packet. */ > > > > > > > + req->current_outstanding_cmd = handle; > > > > > > > + req->outstanding_cmds[handle] = sp; > > > > > > > + sp->handle = handle; > > > > > > > + cmd->host_scribble = (unsigned char *)(unsigned > > > > > > > long)handle; > > > > > > > + req->cnt -= req_cnt; > > > > > > > + > > > > > > > + cmd_pkt = (struct cmd_type_6 *)req->ring_ptr; > > > > > > > + cmd_pkt->handle = make_handle(req->id, handle); > > > > > > > + > > > > > > > + /* Zero out remaining portion of packet. */ > > > > > > > + /* tagged queuing modifier -- default is > > > > > > > TSK_SIMPLE > > > > > > > (0). > > > > > > > */ > > > > > > > + clr_ptr = (uint32_t *)cmd_pkt + 2; > > > > > > > + memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); > > > > > > > + cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); > > > > > > > + > > > > > > > + /* Set NPORT-ID and LUN number*/ > > > > > > > + cmd_pkt->nport_handle = cpu_to_le16(sp->fcport- > > > > > > > > loop_id); > > > > > > > + cmd_pkt->port_id[0] = sp->fcport->d_id.b.al_pa; > > > > > > > + cmd_pkt->port_id[1] = sp->fcport->d_id.b.area; > > > > > > > + cmd_pkt->port_id[2] = sp->fcport->d_id.b.domain; > > > > > > > + cmd_pkt->vp_index = sp->vha->vp_idx; > > > > > > > + > > > > > > > + /* Build IOCB segments */ > > > > > > > + qla24xx_build_scsi_type_6_iocbs(sp, cmd_pkt, > > > > > > > tot_dsds); > > > > > > > + > > > > > > > + int_to_scsilun(cmd->device->lun, &cmd_pkt->lun); > > > > > > > + host_to_fcp_swap((uint8_t *)&cmd_pkt->lun, > > > > > > > sizeof(cmd_pkt- > > > > > > > > lun)); > > > > > > > + > > > > > > > + /* build FCP_CMND IU */ > > > > > > > + int_to_scsilun(cmd->device->lun, &ctx->fcp_cmnd- > > > > > > > > lun); > > > > > > > + ctx->fcp_cmnd->additional_cdb_len = > > > > > > > additional_cdb_len; > > > > > > > + > > > > > > > + if (cmd->sc_data_direction == DMA_TO_DEVICE) > > > > > > > + ctx->fcp_cmnd->additional_cdb_len |= 1; > > > > > > > + else if (cmd->sc_data_direction == > > > > > > > DMA_FROM_DEVICE) > > > > > > > + ctx->fcp_cmnd->additional_cdb_len |= 2; > > > > > > > + > > > > > > > + /* Populate the FCP_PRIO. */ > > > > > > > + if (ha->flags.fcp_prio_enabled) > > > > > > > + ctx->fcp_cmnd->task_attribute |= > > > > > > > + sp->fcport->fcp_prio << 3; > > > > > > > + > > > > > > > + memcpy(ctx->fcp_cmnd->cdb, cmd->cmnd, cmd- > > > > > > > > cmd_len); > > > > > > > + > > > > > > > + fcp_dl = (__be32 *)(ctx->fcp_cmnd->cdb + 16 + > > > > > > > + additional_cdb_len); > > > > > > > + *fcp_dl = htonl((uint32_t)scsi_bufflen(cmd)); > > > > > > > + > > > > > > > + cmd_pkt->fcp_cmnd_dseg_len = cpu_to_le16(ctx- > > > > > > > > fcp_cmnd_len); > > > > > > > + put_unaligned_le64(ctx->fcp_cmnd_dma, > > > > > > > + &cmd_pkt- > > > > > > > > fcp_cmnd_dseg_address); > > > > > > > + > > > > > > > + sp->flags |= SRB_FCP_CMND_DMA_VALID; > > > > > > > + cmd_pkt->byte_count = > > > > > > > cpu_to_le32((uint32_t)scsi_bufflen(cmd)); > > > > > > > + /* Set total data segment count. */ > > > > > > > + cmd_pkt->entry_count = (uint8_t)req_cnt; > > > > > > > + > > > > > > > + wmb(); > > > > > > > + /* Adjust ring index. */ > > > > > > > + req->ring_index++; > > > > > > > + if (req->ring_index == req->length) { > > > > > > > + req->ring_index = 0; > > > > > > > + req->ring_ptr = req->ring; > > > > > > > + } else { > > > > > > > + req->ring_ptr++; > > > > > > > + } > > > > > > > + > > > > > > > + sp->qpair->cmd_cnt++; > > > > > > > + sp->flags |= SRB_DMA_VALID; > > > > > > > + > > > > > > > + /* Set chip new ring index. */ > > > > > > > + wrt_reg_dword(req->req_q_in, req->ring_index); > > > > > > > + > > > > > > > + /* Manage unprocessed RIO/ZIO commands in > > > > > > > response > > > > > > > queue. > > > > > > > */ > > > > > > > + if (vha->flags.process_response_queue && > > > > > > > + rsp->ring_ptr->signature != > > > > > > > RESPONSE_PROCESSED) > > > > > > > + qla24xx_process_response_queue(vha, rsp); > > > > > > > + > > > > > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > > > > > + > > > > > > > + return QLA_SUCCESS; > > > > > > > + > > > > > > > +queuing_error_fcp_cmnd: > > > > > > > + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx- > > > > > > > >fcp_cmnd, > > > > > > > ctx- > > > > > > > > fcp_cmnd_dma); > > > > > > > + > > > > > > > +queuing_error: > > > > > > > + if (tot_dsds) > > > > > > > + scsi_dma_unmap(cmd); > > > > > > > + > > > > > > > + qla_put_fw_resources(sp->qpair, &sp->iores); > > > > > > > + > > > > > > > + if (sp->u.scmd.crc_ctx) { > > > > > > > + mempool_free(sp->u.scmd.crc_ctx, ha- > > > > > > > > ctx_mempool); > > > > > > > + sp->u.scmd.crc_ctx = NULL; > > > > > > > + } > > > > > > > + > > > > > > > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > > > > > > > + > > > > > > > + return QLA_FUNCTION_FAILED; > > > > > > > +} > > > > > > > diff --git a/drivers/scsi/qla2xxx/qla_nx.h > > > > > > > b/drivers/scsi/qla2xxx/qla_nx.h > > > > > > > index 6dc80c8ddf79..5d1bdc15b75c 100644 > > > > > > > --- a/drivers/scsi/qla2xxx/qla_nx.h > > > > > > > +++ b/drivers/scsi/qla2xxx/qla_nx.h > > > > > > > @@ -857,7 +857,9 @@ struct fcp_cmnd { > > > > > > > uint8_t task_attribute; > > > > > > > uint8_t task_management; > > > > > > > uint8_t additional_cdb_len; > > > > > > > - uint8_t cdb[260]; /* 256 for CDB len and 4 for > > > > > > > FCP_DL > > > > > > > */ > > > > > > > +#define QLA_CDB_BUF_SIZE 256 > > > > > > > +#define QLA_FCP_DL_SIZE 4 > > > > > > > + uint8_t cdb[QLA_CDB_BUF_SIZE + QLA_FCP_DL_SIZE]; > > > > > > > /* > > > > > > > 256 > > > > > > > for > > > > > > > CDB len and 4 for FCP_DL */ > > > > > > > }; > > > > > > > > > > > > > > struct dsd_dma { > > > > > > > > > > > > QT and Nilesh, Thank you. > > > > > > I need more time to look at this fully but I will get it > > > > > > tested > > > > > > in > > > > > > the > > > > > > meantime. > > > > > > I will reply with a review or questions later. > > > > > > > > > > > > Thanks for sending this > > > > > > Laurence > > > > > > > > > > @QT and Nilesh > > > > > Which kernel was this against. > > > > > Did you guys test this only agains your driver or also > > > > > upstream > > > > > I cannot build it clean because There is no structure member > > > > > dsd_inuse > > > > > > > > > > Can you send one for upstream please > > > > > > > > > > Patch applied fine to 6.5-rc4 > > > > > > > > > > [loberman@segstorage3 linux]$ patch -p1 < qt.patch > > > > > patching file drivers/scsi/qla2xxx/qla_iocb.c > > > > > Hunk #2 succeeded at 1718 (offset -4 lines). > > > > > Hunk #3 succeeded at 2099 (offset -4 lines). > > > > > Hunk #4 succeeded at 4179 (offset -31 lines). > > > > > patching file drivers/scsi/qla2xxx/qla_nx.h > > > > > > > > > > > > > > > Build fails > > > > > > > > > > drivers/scsi/qla2xxx/qla_iocb.c: In function > > > > > ‘qla_start_scsi_type6’: > > > > > drivers/scsi/qla2xxx/qla_iocb.c:4260:29: error: ‘struct > > > > > qla_qpair’ > > > > > has > > > > > no member named ‘dsd_inuse’ > > > > > if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { > > > > > ^~ > > > > > drivers/scsi/qla2xxx/qla_iocb.c:4263:32: error: ‘struct > > > > > qla_qpair’ > > > > > has > > > > > no member named ‘dsd_inuse’ > > > > > more_dsd_lists + qpair->dsd_inuse, NUM_DSD_CHAIN, > > > > > cmd); > > > > > ^~ > > > > > drivers/scsi/qla2xxx/qla_iocb.c:4267:29: error: ‘struct > > > > > qla_qpair’ > > > > > has > > > > > no member named ‘dsd_avail’ > > > > > if (more_dsd_lists <= qpair->dsd_avail) > > > > > ^~ > > > > > drivers/scsi/qla2xxx/qla_iocb.c:4270:26: error: ‘struct > > > > > qla_qpair’ > > > > > has > > > > > no member named ‘dsd_avail’ > > > > > more_dsd_lists -= qpair->dsd_avail; > > > > > ^~ > > > > > drivers/scsi/qla2xxx/qla_iocb.c:4289:41: error: ‘struct > > > > > qla_qpair’ > > > > > has > > > > > no member named ‘dsd_list’; did you mean ‘hints_list’? > > > > > list_add_tail(&dsd_ptr->list, &qpair->dsd_list); > > > > > ^~~~~~~~ > > > > > hints_list > > > > > drivers/scsi/qla2xxx/qla_iocb.c:4290:8: error: ‘struct > > > > > qla_qpair’ > > > > > has > > > > > no member named ‘dsd_avail’ > > > > > qpair->dsd_avail++; > > > > > > > > I see where this was done, there are two more patches > > > > So I will add those and test > > > > [PATCH 1/2] qla2xxx: Move resource to allow code reuse > > > > > > > > > > > > > > > OK, I was able to now build and and test this. > > > To be honest there are a lot of changes. > > > I just tested the original issue for which I had sent my first > > > patch. > > > > > > Unfortunately the test fails in a different way to the orginal > > > BUG > > > but > > > this may be firmware behavior on the MSA2050 that takes the > > > device > > > paths away. > > > This is because the test passes on an LIO target > > > > > > Against 6.5-rc4 > > > > > > Test notes > > > segstorage3 6.5.0-rc4+ > > > Note that on a MSA2050 it will hang and timeout no more panics as > > > expected. However as noted we lose devices. > > > > > > Starts out fine > > > > > > mpathbp (3600c0ff00043feab07a7ff6307000000) dm-8 HPE,MSA 2050 SAN > > > size=9.3G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw > > > > -+- policy='service-time 0' prio=50 status=active > > > > > - 1:0:1:4 sdh 8:112 active ready running > > > > > - 1:0:4:4 sdaf 65:240 active ready running > > > > > - 3:0:2:4 sdfk 130:96 active ready running > > > > `- 3:0:3:4 sdft 130:240 active ready running > > > `-+- policy='service-time 0' prio=10 status=enabled > > > |- 1:0:2:4 sdp 8:240 active ready running > > > |- 1:0:3:4 sdx 65:112 active ready running > > > |- 3:0:1:4 sdfb 129:208 active ready running > > > `- 3:0:4:4 sdge 131:160 active ready running > > > > > > Do the test > > > > > > [root@segstorage3 ~]# sg_write_same -T --lba=1 > > > /dev/mapper/mpathbp > > > Write same: transport: Host_status=0x03 [DID_TIME_OUT] > > > Driver_status=0x00 [DRIVER_OK] > > > > > > It then caused problems with two of my multipath device paths but > > > perhaps this is array firmware behavior. > > > > > > [ 820.836495] sd 3:0:2:4: [sdfk] tag#1704 FAILED Result: > > > hostbyte=DID_TRANSPORT_DISRUPTED driverbyte=DRIVER_OK cmd_age=23s > > > [ 820.836497] sd 3:0:2:4: [sdfk] tag#1704 CDB: Test Unit Ready > > > 00 > > > 00 > > > 00 00 00 00 > > > > > > [ 825.926075] sd 3:0:2:0: rejecting I/O to offline device > > > [ 825.926136] sd 3:0:2:1: rejecting I/O to offline device > > > [ 825.926165] sd 3:0:2:3: rejecting I/O to offline device > > > > > > And two paths get lost > > > > > > mpathbp (3600c0ff00043feab07a7ff6307000000) dm-8 HPE,MSA 2050 SAN > > > size=9.3G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw > > > > -+- policy='service-time 0' prio=50 status=active > > > > > - 1:0:4:4 sdaf 65:240 active ready running > > > > `- 3:0:3:4 sdft 130:240 active ready running > > > `-+- policy='service-time 0' prio=10 status=enabled > > > |- 1:0:2:4 sdp 8:240 active ready running > > > |- 1:0:3:4 sdx 65:112 active ready running > > > |- 3:0:1:4 sdfb 129:208 active ready running > > > `- 3:0:4:4 sdge 131:160 active ready running > > > > > > I re-scanned but sdh and sdfk are gone from the device tree and > > > do > > > not > > > come back with a rescan. > > > > > > [ 846.043511] sd 1:0:1:4: [sdh] Synchronizing SCSI cache > > > [ 846.043536] sd 1:0:1:4: [sdh] Synchronize Cache(10) failed: > > > Result: > > > hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK > > > > > > [ 820.836495] sd 3:0:2:4: [sdfk] tag#1704 FAILED Result: > > > hostbyte=DID_TRANSPORT_DISRUPTED driverbyte=DRIVER_OK cmd_age=23s > > > [ 820.836497] sd 3:0:2:4: [sdfk] tag#1704 CDB: Test Unit Ready > > > 00 > > > 00 > > > 00 00 00 00 > > > [ 851.166451] sd 3:0:2:4: [sdfk] Synchronizing SCSI cache > > > [ 851.166477] sd 3:0:2:4: [sdfk] Synchronize Cache(10) failed: > > > Result: > > > hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK > > > > > > Now using an LIO target via tcm_qla2xxx > > > > > > mpathy (3600140549ed41778d414038bf1f1bb42) dm-74 LIO-ORG,test-70 > > > size=1.0G features='0' hwhandler='1 alua' wp=rw > > > `-+- policy='service-time 0' prio=50 status=active > > > |- 1:0:5:105 sdbv 68:144 active ready running > > > |- 1:0:6:105 sdei 128:160 active ready running > > > |- 3:0:5:105 sdhz 134:144 active ready running > > > `- 3:0:6:105 sdkm 66:416 active ready running > > > > > > [root@segstorage3 block]# sg_write_same -T --lba=1 > > > /dev/mapper/mpathy > > > Write same: transport: Host_status=0x07 [DID_ERROR] > > > Driver_status=0x08 [DRIVER_SENSE] > > > > > > No issues with the LIO device paths > > > > > > [root@segstorage3 block]# multipath -ll mpathy > > > mpathy (3600140549ed41778d414038bf1f1bb42) dm-74 LIO-ORG,test-70 > > > size=1.0G features='0' hwhandler='1 alua' wp=rw > > > `-+- policy='service-time 0' prio=50 status=active > > > |- 1:0:5:105 sdbv 68:144 active ready running > > > |- 1:0:6:105 sdei 128:160 active ready running > > > |- 3:0:5:105 sdhz 134:144 active ready running > > > `- 3:0:6:105 sdkm 66:416 active ready running > > > > > > So I would like to give a positive test result but will need you > > > folks > > > at Marvell to maybe first work with HPE on the MSA2050 behavior. > > > > > > Thanks > > > Laurence > > Hello Nilesh and Marvell > > > > I ran a few more tests, this 32 bytes CDB patch set totally breaks > > the > > MSA2050 such that a full storage side reboot of the controller pair > > is > > needed to get the paths back. > > > > For that reason I am going to NACK this until you chat with HPE. > > Those MSA2050's are installed everywhere. > > > > Not pushing my original fix, but my patch would land up with the > > MSA2050 timing out but then all would function as it should after > > the > > timeout. Something about your change is not making the MSA2050 > > happy > > at > > all. > > > > Thanks > > Laurence > > > > > > > > Turns out the patch I had sent earlier also makes the MSA2050 unhappy > so I will reach out to HPE about your submission and the behavior and > get back to you. > > Again, the LIO seems to work fine with your patch set. > > This patch below also breaks the MSA, I guess I did not not see it > during my first set of tests > > > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c > b/drivers/scsi/qla2xxx/qla_iocb.c > index 730d8609276c..32809e556ec6 100644 > --- a/drivers/scsi/qla2xxx/qla_iocb.c > +++ b/drivers/scsi/qla2xxx/qla_iocb.c > @@ -1386,7 +1386,8 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, > struct > cmd_type_crc_2 *cmd_pkt, > if ((scsi_get_prot_op(cmd) == SCSI_PROT_READ_INSERT) || > (scsi_get_prot_op(cmd) == SCSI_PROT_WRITE_STRIP) || > (scsi_get_prot_op(cmd) == SCSI_PROT_READ_STRIP) || > - (scsi_get_prot_op(cmd) == SCSI_PROT_WRITE_INSERT)) > + (scsi_get_prot_op(cmd) == SCSI_PROT_WRITE_INSERT) || > + (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL)) > bundling = 0; > > /* Allocate CRC context from global pool */ > @@ -1448,6 +1449,9 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, > struct > cmd_type_crc_2 *cmd_pkt, > dif_bytes = (data_bytes / blk_size) * 8; > > switch (scsi_get_prot_op(GET_CMD_SP(sp))) { > + case SCSI_PROT_NORMAL: > + total_bytes = data_bytes; > + break; > case SCSI_PROT_READ_INSERT: > case SCSI_PROT_WRITE_STRIP: > total_bytes = data_bytes; > Hello I think we can ack this patch. The MSA20250 behavior seems to be a firmware issue specific to that array because the LIO target passes all the tests. I did reach out to HPE and let them know but given that without this patch we panic a server, the patch is needed. Hopefully HPE follows up with me. So: Tested-by: Laurence Oberman <loberman@redhat.com> noting that the MSA array sees an issue not seen on other arrays tested so far. Thanks
On 8/4/23 00:19, Nilesh Javali wrote: > From: Quinn Tran <qutran@marvell.com> > > System crash when 32 bytes CDB was sent to a non T10-PI disk, > > [ 177.143279] ? qla2xxx_dif_start_scsi_mq+0xcd8/0xce0 [qla2xxx] > [ 177.149165] ? internal_add_timer+0x42/0x70 > [ 177.153372] qla2xxx_mqueuecommand+0x207/0x2b0 [qla2xxx] > [ 177.158730] scsi_queue_rq+0x2b7/0xc00 > [ 177.162501] blk_mq_dispatch_rq_list+0x3ea/0x7e0 > > Current code attempt to use CRC IOCB to send the command but failed. > Instead, type 6 IOCB should be used to send the IO. > > Clone existing type 6 IOCB code with addition of MQ support > to allow 32 bytes CDB to go through. > > Signed-off-by: Quinn Tran <qutran@marvell.com> > Cc: Laurence Oberman <loberman@redhat.com> > Cc: stable@vger.kernel.org > Signed-off-by: Nilesh Javali <njavali@marvell.com> > --- > drivers/scsi/qla2xxx/qla_iocb.c | 270 ++++++++++++++++++++++++++++++++ > drivers/scsi/qla2xxx/qla_nx.h | 4 +- > 2 files changed, 273 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c > index 0caa64a7df26..e99ebf7e1c7a 100644 > --- a/drivers/scsi/qla2xxx/qla_iocb.c > +++ b/drivers/scsi/qla2xxx/qla_iocb.c > @@ -11,6 +11,7 @@ > > #include <scsi/scsi_tcq.h> > > +static int qla_start_scsi_type6(srb_t *sp); > /** > * qla2x00_get_cmd_direction() - Determine control_flag data direction. > * @sp: SCSI command > @@ -1721,6 +1722,8 @@ qla24xx_dif_start_scsi(srb_t *sp) > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > if (cmd->cmd_len <= 16) > return qla24xx_start_scsi(sp); > + else > + return qla_start_scsi_type6(sp); > } > > /* Setup device pointers. */ > @@ -2100,6 +2103,8 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp) > if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { > if (cmd->cmd_len <= 16) > return qla2xxx_start_scsi_mq(sp); > + else > + return qla_start_scsi_type6(sp); > } > > spin_lock_irqsave(&qpair->qp_lock, flags); > @@ -4205,3 +4210,268 @@ qla2x00_start_bidir(srb_t *sp, struct scsi_qla_host *vha, uint32_t tot_dsds) > > return rval; > } > + > +/** > + * qla_start_scsi_type6() - Send a SCSI command to the ISP > + * @sp: command to send to the ISP > + * > + * Returns non-zero if a failure occurred, else zero. > + */ > +static int > +qla_start_scsi_type6(srb_t *sp) > +{ > + int nseg; > + unsigned long flags; > + uint32_t *clr_ptr; > + uint32_t handle; > + struct cmd_type_6 *cmd_pkt; > + uint16_t cnt; > + uint16_t req_cnt; > + uint16_t tot_dsds; > + struct req_que *req = NULL; > + struct rsp_que *rsp; > + struct scsi_cmnd *cmd = GET_CMD_SP(sp); > + struct scsi_qla_host *vha = sp->fcport->vha; > + struct qla_hw_data *ha = vha->hw; > + struct qla_qpair *qpair = sp->qpair; > + uint16_t more_dsd_lists = 0; > + struct dsd_dma *dsd_ptr; > + uint16_t i; > + __be32 *fcp_dl; > + uint8_t additional_cdb_len; > + struct ct6_dsd *ctx; > + remove extra newline > + > + /* Acquire qpair specific lock */ > + spin_lock_irqsave(&qpair->qp_lock, flags); > + > + /* Setup qpair pointers */ > + req = qpair->req; > + rsp = qpair->rsp; > + > + /* So we know we haven't pci_map'ed anything yet */ > + tot_dsds = 0; > + > + /* Send marker if required */ > + if (vha->marker_needed != 0) { > + if (__qla2x00_marker(vha, qpair, 0, 0, MK_SYNC_ALL) != QLA_SUCCESS) { > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > + return QLA_FUNCTION_FAILED; > + } > + vha->marker_needed = 0; > + } > + > + handle = qla2xxx_get_next_handle(req); > + if (handle == 0) > + goto queuing_error; > + > + /* Map the sg table so we have an accurate count of sg entries needed */ > + if (scsi_sg_count(cmd)) { > + nseg = dma_map_sg(&ha->pdev->dev, scsi_sglist(cmd), > + scsi_sg_count(cmd), cmd->sc_data_direction); > + if (unlikely(!nseg)) > + goto queuing_error; > + } else { > + nseg = 0; > + } > + > + tot_dsds = nseg; > + > + /* eventhough driver only need 1 T6 IOCB, FW still convert DSD to Continueation IOCB */ > + req_cnt = qla24xx_calc_iocbs(vha, tot_dsds); > + > + sp->iores.res_type = RESOURCE_IOCB | RESOURCE_EXCH; > + sp->iores.exch_cnt = 1; > + sp->iores.iocb_cnt = req_cnt; > + > + if (qla_get_fw_resources(sp->qpair, &sp->iores)) > + goto queuing_error; > + > + more_dsd_lists = qla24xx_calc_dsd_lists(tot_dsds); > + if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { > + ql_dbg(ql_dbg_io, vha, 0x3028, > + "Num of DSD list %d is than %d for cmd=%p.\n", > + more_dsd_lists + qpair->dsd_inuse, NUM_DSD_CHAIN, cmd); > + goto queuing_error; > + } > + > + if (more_dsd_lists <= qpair->dsd_avail) > + goto sufficient_dsds; > + else > + more_dsd_lists -= qpair->dsd_avail; > + > + for (i = 0; i < more_dsd_lists; i++) { > + dsd_ptr = kzalloc(sizeof(*dsd_ptr), GFP_ATOMIC); > + if (!dsd_ptr) { > + ql_log(ql_log_fatal, vha, 0x3029, > + "Failed to allocate memory for dsd_dma for cmd=%p.\n", cmd); > + goto queuing_error; > + } > + INIT_LIST_HEAD(&dsd_ptr->list); > + > + dsd_ptr->dsd_addr = dma_pool_alloc(ha->dl_dma_pool, > + GFP_ATOMIC, &dsd_ptr->dsd_list_dma); > + if (!dsd_ptr->dsd_addr) { > + kfree(dsd_ptr); > + ql_log(ql_log_fatal, vha, 0x302a, > + "Failed to allocate memory for dsd_addr for cmd=%p.\n", cmd); > + goto queuing_error; > + } > + list_add_tail(&dsd_ptr->list, &qpair->dsd_list); > + qpair->dsd_avail++; > + } > + > +sufficient_dsds: > + req_cnt = 1; > + > + if (req->cnt < (req_cnt + 2)) { > + if (IS_SHADOW_REG_CAPABLE(ha)) { > + cnt = *req->out_ptr; > + } else { > + cnt = (uint16_t)rd_reg_dword_relaxed(req->req_q_out); > + if (qla2x00_check_reg16_for_disconnect(vha, cnt)) > + goto queuing_error; > + } > + > + if (req->ring_index < cnt) > + req->cnt = cnt - req->ring_index; > + else > + req->cnt = req->length - (req->ring_index - cnt); > + if (req->cnt < (req_cnt + 2)) > + goto queuing_error; > + } > + > + ctx = &sp->u.scmd.ct6_ctx; > + > + memset(ctx, 0, sizeof(struct ct6_dsd)); > + ctx->fcp_cmnd = dma_pool_zalloc(ha->fcp_cmnd_dma_pool, > + GFP_ATOMIC, &ctx->fcp_cmnd_dma); > + if (!ctx->fcp_cmnd) { > + ql_log(ql_log_fatal, vha, 0x3031, > + "Failed to allocate fcp_cmnd for cmd=%p.\n", cmd); > + goto queuing_error; > + } > + > + /* Initialize the DSD list and dma handle */ > + INIT_LIST_HEAD(&ctx->dsd_list); > + ctx->dsd_use_cnt = 0; > + > + if (cmd->cmd_len > 16) { > + additional_cdb_len = cmd->cmd_len - 16; > + if (cmd->cmd_len % 4 || > + cmd->cmd_len > QLA_CDB_BUF_SIZE) { > + /* SCSI command bigger than 16 bytes must be > + * multiple of 4 or too big. > + */ > fix comment formatting + ql_log(ql_log_warn, vha, 0x3033, > + "scsi cmd len %d not multiple of 4 for cmd=%p.\n", > + cmd->cmd_len, cmd); > + goto queuing_error_fcp_cmnd; > + } > + ctx->fcp_cmnd_len = 12 + cmd->cmd_len + 4; > + } else { > + additional_cdb_len = 0; > + ctx->fcp_cmnd_len = 12 + 16 + 4; > + } > + > + /* Build command packet. */ > + req->current_outstanding_cmd = handle; > + req->outstanding_cmds[handle] = sp; > + sp->handle = handle; > + cmd->host_scribble = (unsigned char *)(unsigned long)handle; > + req->cnt -= req_cnt; > + > + cmd_pkt = (struct cmd_type_6 *)req->ring_ptr; > + cmd_pkt->handle = make_handle(req->id, handle); > + > + /* Zero out remaining portion of packet. */ ^^ remove self-explanatory comment. > + /* tagged queuing modifier -- default is TSK_SIMPLE (0). */ fix comment format > + clr_ptr = (uint32_t *)cmd_pkt + 2; > + memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); > + cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); > + > + /* Set NPORT-ID and LUN number*/ > + cmd_pkt->nport_handle = cpu_to_le16(sp->fcport->loop_id); > + cmd_pkt->port_id[0] = sp->fcport->d_id.b.al_pa; > + cmd_pkt->port_id[1] = sp->fcport->d_id.b.area; > + cmd_pkt->port_id[2] = sp->fcport->d_id.b.domain; > + cmd_pkt->vp_index = sp->vha->vp_idx; > + > + /* Build IOCB segments */ > + qla24xx_build_scsi_type_6_iocbs(sp, cmd_pkt, tot_dsds); > + > + int_to_scsilun(cmd->device->lun, &cmd_pkt->lun); > + host_to_fcp_swap((uint8_t *)&cmd_pkt->lun, sizeof(cmd_pkt->lun)); > + > + /* build FCP_CMND IU */ > + int_to_scsilun(cmd->device->lun, &ctx->fcp_cmnd->lun); > + ctx->fcp_cmnd->additional_cdb_len = additional_cdb_len; > + > + if (cmd->sc_data_direction == DMA_TO_DEVICE) > + ctx->fcp_cmnd->additional_cdb_len |= 1; > + else if (cmd->sc_data_direction == DMA_FROM_DEVICE) > + ctx->fcp_cmnd->additional_cdb_len |= 2; > + > + /* Populate the FCP_PRIO. */ > + if (ha->flags.fcp_prio_enabled) > + ctx->fcp_cmnd->task_attribute |= > + sp->fcport->fcp_prio << 3; > + > + memcpy(ctx->fcp_cmnd->cdb, cmd->cmnd, cmd->cmd_len); > + > + fcp_dl = (__be32 *)(ctx->fcp_cmnd->cdb + 16 + > + additional_cdb_len); > + *fcp_dl = htonl((uint32_t)scsi_bufflen(cmd)); > + > + cmd_pkt->fcp_cmnd_dseg_len = cpu_to_le16(ctx->fcp_cmnd_len); > + put_unaligned_le64(ctx->fcp_cmnd_dma, > + &cmd_pkt->fcp_cmnd_dseg_address); > + > + sp->flags |= SRB_FCP_CMND_DMA_VALID; > + cmd_pkt->byte_count = cpu_to_le32((uint32_t)scsi_bufflen(cmd)); > + /* Set total data segment count. */ > + cmd_pkt->entry_count = (uint8_t)req_cnt; > + > + wmb(); > + /* Adjust ring index. */ > + req->ring_index++; > + if (req->ring_index == req->length) { > + req->ring_index = 0; > + req->ring_ptr = req->ring; > + } else { > + req->ring_ptr++; > + } > + > + sp->qpair->cmd_cnt++; > + sp->flags |= SRB_DMA_VALID; > + > + /* Set chip new ring index. */ > + wrt_reg_dword(req->req_q_in, req->ring_index); > + > + /* Manage unprocessed RIO/ZIO commands in response queue. */ > + if (vha->flags.process_response_queue && > + rsp->ring_ptr->signature != RESPONSE_PROCESSED) > + qla24xx_process_response_queue(vha, rsp); > + > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > + > + return QLA_SUCCESS; > + > +queuing_error_fcp_cmnd: > + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx->fcp_cmnd, ctx->fcp_cmnd_dma); > + > +queuing_error: > + if (tot_dsds) > + scsi_dma_unmap(cmd); > + > + qla_put_fw_resources(sp->qpair, &sp->iores); > + > + if (sp->u.scmd.crc_ctx) { > + mempool_free(sp->u.scmd.crc_ctx, ha->ctx_mempool); > + sp->u.scmd.crc_ctx = NULL; > + } > + > + spin_unlock_irqrestore(&qpair->qp_lock, flags); > + > + return QLA_FUNCTION_FAILED; > +} > diff --git a/drivers/scsi/qla2xxx/qla_nx.h b/drivers/scsi/qla2xxx/qla_nx.h > index 6dc80c8ddf79..5d1bdc15b75c 100644 > --- a/drivers/scsi/qla2xxx/qla_nx.h > +++ b/drivers/scsi/qla2xxx/qla_nx.h > @@ -857,7 +857,9 @@ struct fcp_cmnd { > uint8_t task_attribute; > uint8_t task_management; > uint8_t additional_cdb_len; > - uint8_t cdb[260]; /* 256 for CDB len and 4 for FCP_DL */ > +#define QLA_CDB_BUF_SIZE 256 > +#define QLA_FCP_DL_SIZE 4 > + uint8_t cdb[QLA_CDB_BUF_SIZE + QLA_FCP_DL_SIZE]; /* 256 for CDB len and 4 for FCP_DL */ > }; > > struct dsd_dma { Once you fix comments in this patch... you can add Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c index 0caa64a7df26..e99ebf7e1c7a 100644 --- a/drivers/scsi/qla2xxx/qla_iocb.c +++ b/drivers/scsi/qla2xxx/qla_iocb.c @@ -11,6 +11,7 @@ #include <scsi/scsi_tcq.h> +static int qla_start_scsi_type6(srb_t *sp); /** * qla2x00_get_cmd_direction() - Determine control_flag data direction. * @sp: SCSI command @@ -1721,6 +1722,8 @@ qla24xx_dif_start_scsi(srb_t *sp) if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { if (cmd->cmd_len <= 16) return qla24xx_start_scsi(sp); + else + return qla_start_scsi_type6(sp); } /* Setup device pointers. */ @@ -2100,6 +2103,8 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp) if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) { if (cmd->cmd_len <= 16) return qla2xxx_start_scsi_mq(sp); + else + return qla_start_scsi_type6(sp); } spin_lock_irqsave(&qpair->qp_lock, flags); @@ -4205,3 +4210,268 @@ qla2x00_start_bidir(srb_t *sp, struct scsi_qla_host *vha, uint32_t tot_dsds) return rval; } + +/** + * qla_start_scsi_type6() - Send a SCSI command to the ISP + * @sp: command to send to the ISP + * + * Returns non-zero if a failure occurred, else zero. + */ +static int +qla_start_scsi_type6(srb_t *sp) +{ + int nseg; + unsigned long flags; + uint32_t *clr_ptr; + uint32_t handle; + struct cmd_type_6 *cmd_pkt; + uint16_t cnt; + uint16_t req_cnt; + uint16_t tot_dsds; + struct req_que *req = NULL; + struct rsp_que *rsp; + struct scsi_cmnd *cmd = GET_CMD_SP(sp); + struct scsi_qla_host *vha = sp->fcport->vha; + struct qla_hw_data *ha = vha->hw; + struct qla_qpair *qpair = sp->qpair; + uint16_t more_dsd_lists = 0; + struct dsd_dma *dsd_ptr; + uint16_t i; + __be32 *fcp_dl; + uint8_t additional_cdb_len; + struct ct6_dsd *ctx; + + + /* Acquire qpair specific lock */ + spin_lock_irqsave(&qpair->qp_lock, flags); + + /* Setup qpair pointers */ + req = qpair->req; + rsp = qpair->rsp; + + /* So we know we haven't pci_map'ed anything yet */ + tot_dsds = 0; + + /* Send marker if required */ + if (vha->marker_needed != 0) { + if (__qla2x00_marker(vha, qpair, 0, 0, MK_SYNC_ALL) != QLA_SUCCESS) { + spin_unlock_irqrestore(&qpair->qp_lock, flags); + return QLA_FUNCTION_FAILED; + } + vha->marker_needed = 0; + } + + handle = qla2xxx_get_next_handle(req); + if (handle == 0) + goto queuing_error; + + /* Map the sg table so we have an accurate count of sg entries needed */ + if (scsi_sg_count(cmd)) { + nseg = dma_map_sg(&ha->pdev->dev, scsi_sglist(cmd), + scsi_sg_count(cmd), cmd->sc_data_direction); + if (unlikely(!nseg)) + goto queuing_error; + } else { + nseg = 0; + } + + tot_dsds = nseg; + + /* eventhough driver only need 1 T6 IOCB, FW still convert DSD to Continueation IOCB */ + req_cnt = qla24xx_calc_iocbs(vha, tot_dsds); + + sp->iores.res_type = RESOURCE_IOCB | RESOURCE_EXCH; + sp->iores.exch_cnt = 1; + sp->iores.iocb_cnt = req_cnt; + + if (qla_get_fw_resources(sp->qpair, &sp->iores)) + goto queuing_error; + + more_dsd_lists = qla24xx_calc_dsd_lists(tot_dsds); + if ((more_dsd_lists + qpair->dsd_inuse) >= NUM_DSD_CHAIN) { + ql_dbg(ql_dbg_io, vha, 0x3028, + "Num of DSD list %d is than %d for cmd=%p.\n", + more_dsd_lists + qpair->dsd_inuse, NUM_DSD_CHAIN, cmd); + goto queuing_error; + } + + if (more_dsd_lists <= qpair->dsd_avail) + goto sufficient_dsds; + else + more_dsd_lists -= qpair->dsd_avail; + + for (i = 0; i < more_dsd_lists; i++) { + dsd_ptr = kzalloc(sizeof(*dsd_ptr), GFP_ATOMIC); + if (!dsd_ptr) { + ql_log(ql_log_fatal, vha, 0x3029, + "Failed to allocate memory for dsd_dma for cmd=%p.\n", cmd); + goto queuing_error; + } + INIT_LIST_HEAD(&dsd_ptr->list); + + dsd_ptr->dsd_addr = dma_pool_alloc(ha->dl_dma_pool, + GFP_ATOMIC, &dsd_ptr->dsd_list_dma); + if (!dsd_ptr->dsd_addr) { + kfree(dsd_ptr); + ql_log(ql_log_fatal, vha, 0x302a, + "Failed to allocate memory for dsd_addr for cmd=%p.\n", cmd); + goto queuing_error; + } + list_add_tail(&dsd_ptr->list, &qpair->dsd_list); + qpair->dsd_avail++; + } + +sufficient_dsds: + req_cnt = 1; + + if (req->cnt < (req_cnt + 2)) { + if (IS_SHADOW_REG_CAPABLE(ha)) { + cnt = *req->out_ptr; + } else { + cnt = (uint16_t)rd_reg_dword_relaxed(req->req_q_out); + if (qla2x00_check_reg16_for_disconnect(vha, cnt)) + goto queuing_error; + } + + if (req->ring_index < cnt) + req->cnt = cnt - req->ring_index; + else + req->cnt = req->length - (req->ring_index - cnt); + if (req->cnt < (req_cnt + 2)) + goto queuing_error; + } + + ctx = &sp->u.scmd.ct6_ctx; + + memset(ctx, 0, sizeof(struct ct6_dsd)); + ctx->fcp_cmnd = dma_pool_zalloc(ha->fcp_cmnd_dma_pool, + GFP_ATOMIC, &ctx->fcp_cmnd_dma); + if (!ctx->fcp_cmnd) { + ql_log(ql_log_fatal, vha, 0x3031, + "Failed to allocate fcp_cmnd for cmd=%p.\n", cmd); + goto queuing_error; + } + + /* Initialize the DSD list and dma handle */ + INIT_LIST_HEAD(&ctx->dsd_list); + ctx->dsd_use_cnt = 0; + + if (cmd->cmd_len > 16) { + additional_cdb_len = cmd->cmd_len - 16; + if (cmd->cmd_len % 4 || + cmd->cmd_len > QLA_CDB_BUF_SIZE) { + /* SCSI command bigger than 16 bytes must be + * multiple of 4 or too big. + */ + ql_log(ql_log_warn, vha, 0x3033, + "scsi cmd len %d not multiple of 4 for cmd=%p.\n", + cmd->cmd_len, cmd); + goto queuing_error_fcp_cmnd; + } + ctx->fcp_cmnd_len = 12 + cmd->cmd_len + 4; + } else { + additional_cdb_len = 0; + ctx->fcp_cmnd_len = 12 + 16 + 4; + } + + /* Build command packet. */ + req->current_outstanding_cmd = handle; + req->outstanding_cmds[handle] = sp; + sp->handle = handle; + cmd->host_scribble = (unsigned char *)(unsigned long)handle; + req->cnt -= req_cnt; + + cmd_pkt = (struct cmd_type_6 *)req->ring_ptr; + cmd_pkt->handle = make_handle(req->id, handle); + + /* Zero out remaining portion of packet. */ + /* tagged queuing modifier -- default is TSK_SIMPLE (0). */ + clr_ptr = (uint32_t *)cmd_pkt + 2; + memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); + cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); + + /* Set NPORT-ID and LUN number*/ + cmd_pkt->nport_handle = cpu_to_le16(sp->fcport->loop_id); + cmd_pkt->port_id[0] = sp->fcport->d_id.b.al_pa; + cmd_pkt->port_id[1] = sp->fcport->d_id.b.area; + cmd_pkt->port_id[2] = sp->fcport->d_id.b.domain; + cmd_pkt->vp_index = sp->vha->vp_idx; + + /* Build IOCB segments */ + qla24xx_build_scsi_type_6_iocbs(sp, cmd_pkt, tot_dsds); + + int_to_scsilun(cmd->device->lun, &cmd_pkt->lun); + host_to_fcp_swap((uint8_t *)&cmd_pkt->lun, sizeof(cmd_pkt->lun)); + + /* build FCP_CMND IU */ + int_to_scsilun(cmd->device->lun, &ctx->fcp_cmnd->lun); + ctx->fcp_cmnd->additional_cdb_len = additional_cdb_len; + + if (cmd->sc_data_direction == DMA_TO_DEVICE) + ctx->fcp_cmnd->additional_cdb_len |= 1; + else if (cmd->sc_data_direction == DMA_FROM_DEVICE) + ctx->fcp_cmnd->additional_cdb_len |= 2; + + /* Populate the FCP_PRIO. */ + if (ha->flags.fcp_prio_enabled) + ctx->fcp_cmnd->task_attribute |= + sp->fcport->fcp_prio << 3; + + memcpy(ctx->fcp_cmnd->cdb, cmd->cmnd, cmd->cmd_len); + + fcp_dl = (__be32 *)(ctx->fcp_cmnd->cdb + 16 + + additional_cdb_len); + *fcp_dl = htonl((uint32_t)scsi_bufflen(cmd)); + + cmd_pkt->fcp_cmnd_dseg_len = cpu_to_le16(ctx->fcp_cmnd_len); + put_unaligned_le64(ctx->fcp_cmnd_dma, + &cmd_pkt->fcp_cmnd_dseg_address); + + sp->flags |= SRB_FCP_CMND_DMA_VALID; + cmd_pkt->byte_count = cpu_to_le32((uint32_t)scsi_bufflen(cmd)); + /* Set total data segment count. */ + cmd_pkt->entry_count = (uint8_t)req_cnt; + + wmb(); + /* Adjust ring index. */ + req->ring_index++; + if (req->ring_index == req->length) { + req->ring_index = 0; + req->ring_ptr = req->ring; + } else { + req->ring_ptr++; + } + + sp->qpair->cmd_cnt++; + sp->flags |= SRB_DMA_VALID; + + /* Set chip new ring index. */ + wrt_reg_dword(req->req_q_in, req->ring_index); + + /* Manage unprocessed RIO/ZIO commands in response queue. */ + if (vha->flags.process_response_queue && + rsp->ring_ptr->signature != RESPONSE_PROCESSED) + qla24xx_process_response_queue(vha, rsp); + + spin_unlock_irqrestore(&qpair->qp_lock, flags); + + return QLA_SUCCESS; + +queuing_error_fcp_cmnd: + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx->fcp_cmnd, ctx->fcp_cmnd_dma); + +queuing_error: + if (tot_dsds) + scsi_dma_unmap(cmd); + + qla_put_fw_resources(sp->qpair, &sp->iores); + + if (sp->u.scmd.crc_ctx) { + mempool_free(sp->u.scmd.crc_ctx, ha->ctx_mempool); + sp->u.scmd.crc_ctx = NULL; + } + + spin_unlock_irqrestore(&qpair->qp_lock, flags); + + return QLA_FUNCTION_FAILED; +} diff --git a/drivers/scsi/qla2xxx/qla_nx.h b/drivers/scsi/qla2xxx/qla_nx.h index 6dc80c8ddf79..5d1bdc15b75c 100644 --- a/drivers/scsi/qla2xxx/qla_nx.h +++ b/drivers/scsi/qla2xxx/qla_nx.h @@ -857,7 +857,9 @@ struct fcp_cmnd { uint8_t task_attribute; uint8_t task_management; uint8_t additional_cdb_len; - uint8_t cdb[260]; /* 256 for CDB len and 4 for FCP_DL */ +#define QLA_CDB_BUF_SIZE 256 +#define QLA_FCP_DL_SIZE 4 + uint8_t cdb[QLA_CDB_BUF_SIZE + QLA_FCP_DL_SIZE]; /* 256 for CDB len and 4 for FCP_DL */ }; struct dsd_dma {