diff mbox series

[v3,16/24] mpi3mr: hardware workaround for UNMAP commands to nvme drives

Message ID 20210419110156.1786882-17-kashyap.desai@broadcom.com (mailing list archive)
State Superseded
Headers show
Series Introducing mpi3mr driver | expand

Commit Message

Kashyap Desai April 19, 2021, 11:01 a.m. UTC 
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>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>

Cc: sathya.prakash@broadcom.com
---
 drivers/scsi/mpi3mr/mpi3mr_os.c | 99 +++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

Comments

Himanshu Madhani April 22, 2021, 4:01 p.m. UTC | #1 
> On Apr 19, 2021, at 6:01 AM, Kashyap Desai <kashyap.desai@broadcom.com> 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>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Tomas Henzl <thenzl@redhat.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 ac30699c7d69..01b82bd2e8df 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
> @@ -2786,6 +2786,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
> @@ -2878,6 +2972,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;
> -- 
> 2.18.1
> 

Looks Good.

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	 Oracle Linux Engineering
diff mbox series

Patch

diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index ac30699c7d69..01b82bd2e8df 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -2786,6 +2786,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
@@ -2878,6 +2972,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;