| Message ID | 20201222101156.98308-17-kashyap.desai@broadcom.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Introducing mpi3mr driver | expand |
On 12/22/20 11:11 AM, Kashyap Desai wrote: > The controller hardware can not handle certain unmap commands for NVMe > drives, this patch adds support in the driver to check those commands and > handle as appropriate. > > Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com> > Cc: sathya.prakash@broadcom.com > --- > drivers/scsi/mpi3mr/mpi3mr_os.c | 99 +++++++++++++++++++++++++++++++++ > 1 file changed, 99 insertions(+) > > diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c > index 07a7b1efbc4f..742cf45d4878 100644 > --- a/drivers/scsi/mpi3mr/mpi3mr_os.c > +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c > @@ -2865,6 +2865,100 @@ static int mpi3mr_target_alloc(struct scsi_target *starget) > return retval; > } > > +/** > + * mpi3mr_check_return_unmap - Whether an unmap is allowed > + * @mrioc: Adapter instance reference > + * @scmd: SCSI Command reference > + * > + * The controller hardware cannot handle certain unmap commands > + * for NVMe drives, this routine checks those and return true > + * and completes the SCSI command with proper status and sense > + * data. > + * > + * Return: TRUE for not allowed unmap, FALSE otherwise. > + */ > +static bool mpi3mr_check_return_unmap(struct mpi3mr_ioc *mrioc, > + struct scsi_cmnd *scmd) > +{ > + unsigned char *buf; > + u16 param_len, desc_len; > + > + param_len = get_unaligned_be16(scmd->cmnd + 7); > + > + if (!param_len) { > + ioc_warn(mrioc, > + "%s: CDB received with zero parameter length\n", > + __func__); > + scsi_print_command(scmd); > + scmd->result = DID_OK << 16; > + scmd->scsi_done(scmd); > + return true; > + } > + > + if (param_len < 24) { > + ioc_warn(mrioc, > + "%s: CDB received with invalid param_len: %d\n", > + __func__, param_len); > + scsi_print_command(scmd); > + scmd->result = (DRIVER_SENSE << 24) | > + SAM_STAT_CHECK_CONDITION; > + scsi_build_sense_buffer(0, scmd->sense_buffer, ILLEGAL_REQUEST, > + 0x1A, 0); > + scmd->scsi_done(scmd); > + return true; > + } > + if (param_len != scsi_bufflen(scmd)) { > + ioc_warn(mrioc, > + "%s: CDB received with param_len: %d bufflen: %d\n", > + __func__, param_len, scsi_bufflen(scmd)); > + scsi_print_command(scmd); > + scmd->result = (DRIVER_SENSE << 24) | > + SAM_STAT_CHECK_CONDITION; > + scsi_build_sense_buffer(0, scmd->sense_buffer, ILLEGAL_REQUEST, > + 0x1A, 0); > + scmd->scsi_done(scmd); > + return true; > + } > + buf = kzalloc(scsi_bufflen(scmd), GFP_ATOMIC); > + if (!buf) { > + scsi_print_command(scmd); > + scmd->result = (DRIVER_SENSE << 24) | > + SAM_STAT_CHECK_CONDITION; > + scsi_build_sense_buffer(0, scmd->sense_buffer, ILLEGAL_REQUEST, > + 0x55, 0x03); > + scmd->scsi_done(scmd); > + return true; > + } > + scsi_sg_copy_to_buffer(scmd, buf, scsi_bufflen(scmd)); > + desc_len = get_unaligned_be16(&buf[2]); > + > + if (desc_len < 16) { > + ioc_warn(mrioc, > + "%s: Invalid descriptor length in param list: %d\n", > + __func__, desc_len); > + scsi_print_command(scmd); > + scmd->result = (DRIVER_SENSE << 24) | > + SAM_STAT_CHECK_CONDITION; > + scsi_build_sense_buffer(0, scmd->sense_buffer, ILLEGAL_REQUEST, > + 0x26, 0); > + scmd->scsi_done(scmd); > + kfree(buf); > + return true; > + } > + > + if (param_len > (desc_len + 8)) { > + scsi_print_command(scmd); > + ioc_warn(mrioc, > + "%s: Truncating param_len(%d) to desc_len+8(%d)\n", > + __func__, param_len, (desc_len + 8)); > + param_len = desc_len + 8; > + put_unaligned_be16(param_len, scmd->cmnd+7); > + scsi_print_command(scmd); > + } > + > + kfree(buf); > + return false; > +} > > /** > * mpi3mr_allow_scmd_to_fw - Command is allowed during shutdown > @@ -2957,6 +3051,11 @@ static int mpi3mr_qcmd(struct Scsi_Host *shost, > goto out; > } > > + if ((scmd->cmnd[0] == UNMAP) && > + (stgt_priv_data->dev_type == MPI3_DEVICE_DEVFORM_PCIE) && > + mpi3mr_check_return_unmap(mrioc, scmd)) > + goto out; > + > host_tag = mpi3mr_host_tag_for_scmd(mrioc, scmd); > if (host_tag == MPI3MR_HOSTTAG_INVALID) { > scmd->result = DID_ERROR << 16; > One _could_ have modified the firmware instead ... oh well. Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c index 07a7b1efbc4f..742cf45d4878 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_os.c +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c @@ -2865,6 +2865,100 @@ static int mpi3mr_target_alloc(struct scsi_target *starget) return retval; } +/** + * mpi3mr_check_return_unmap - Whether an unmap is allowed + * @mrioc: Adapter instance reference + * @scmd: SCSI Command reference + * + * The controller hardware cannot handle certain unmap commands + * for NVMe drives, this routine checks those and return true + * and completes the SCSI command with proper status and sense + * data. + * + * Return: TRUE for not allowed unmap, FALSE otherwise. + */ +static bool mpi3mr_check_return_unmap(struct mpi3mr_ioc *mrioc, + struct scsi_cmnd *scmd) +{ + unsigned char *buf; + u16 param_len, desc_len; + + param_len = get_unaligned_be16(scmd->cmnd + 7); + + if (!param_len) { + ioc_warn(mrioc, + "%s: CDB received with zero parameter length\n", + __func__); + scsi_print_command(scmd); + scmd->result = DID_OK << 16; + scmd->scsi_done(scmd); + return true; + } + + if (param_len < 24) { + ioc_warn(mrioc, + "%s: CDB received with invalid param_len: %d\n", + __func__, param_len); + scsi_print_command(scmd); + scmd->result = (DRIVER_SENSE << 24) | + SAM_STAT_CHECK_CONDITION; + scsi_build_sense_buffer(0, scmd->sense_buffer, ILLEGAL_REQUEST, + 0x1A, 0); + scmd->scsi_done(scmd); + return true; + } + if (param_len != scsi_bufflen(scmd)) { + ioc_warn(mrioc, + "%s: CDB received with param_len: %d bufflen: %d\n", + __func__, param_len, scsi_bufflen(scmd)); + scsi_print_command(scmd); + scmd->result = (DRIVER_SENSE << 24) | + SAM_STAT_CHECK_CONDITION; + scsi_build_sense_buffer(0, scmd->sense_buffer, ILLEGAL_REQUEST, + 0x1A, 0); + scmd->scsi_done(scmd); + return true; + } + buf = kzalloc(scsi_bufflen(scmd), GFP_ATOMIC); + if (!buf) { + scsi_print_command(scmd); + scmd->result = (DRIVER_SENSE << 24) | + SAM_STAT_CHECK_CONDITION; + scsi_build_sense_buffer(0, scmd->sense_buffer, ILLEGAL_REQUEST, + 0x55, 0x03); + scmd->scsi_done(scmd); + return true; + } + scsi_sg_copy_to_buffer(scmd, buf, scsi_bufflen(scmd)); + desc_len = get_unaligned_be16(&buf[2]); + + if (desc_len < 16) { + ioc_warn(mrioc, + "%s: Invalid descriptor length in param list: %d\n", + __func__, desc_len); + scsi_print_command(scmd); + scmd->result = (DRIVER_SENSE << 24) | + SAM_STAT_CHECK_CONDITION; + scsi_build_sense_buffer(0, scmd->sense_buffer, ILLEGAL_REQUEST, + 0x26, 0); + scmd->scsi_done(scmd); + kfree(buf); + return true; + } + + if (param_len > (desc_len + 8)) { + scsi_print_command(scmd); + ioc_warn(mrioc, + "%s: Truncating param_len(%d) to desc_len+8(%d)\n", + __func__, param_len, (desc_len + 8)); + param_len = desc_len + 8; + put_unaligned_be16(param_len, scmd->cmnd+7); + scsi_print_command(scmd); + } + + kfree(buf); + return false; +} /** * mpi3mr_allow_scmd_to_fw - Command is allowed during shutdown @@ -2957,6 +3051,11 @@ static int mpi3mr_qcmd(struct Scsi_Host *shost, goto out; } + if ((scmd->cmnd[0] == UNMAP) && + (stgt_priv_data->dev_type == MPI3_DEVICE_DEVFORM_PCIE) && + mpi3mr_check_return_unmap(mrioc, scmd)) + goto out; + host_tag = mpi3mr_host_tag_for_scmd(mrioc, scmd); if (host_tag == MPI3MR_HOSTTAG_INVALID) { scmd->result = DID_ERROR << 16;
The controller hardware can not handle certain unmap commands for NVMe drives, this patch adds support in the driver to check those commands and handle as appropriate. Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com> Cc: sathya.prakash@broadcom.com --- drivers/scsi/mpi3mr/mpi3mr_os.c | 99 +++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+)