diff mbox series

[RFC,11/13] scsi: Allow device handler set their own CDB

Message ID 1589538614-24048-12-git-send-email-avri.altman@wdc.com (mailing list archive)
State Changes Requested
Headers show
Series scsi: ufs: Add HPB Support | expand

Commit Message

Avri Altman May 15, 2020, 10:30 a.m. UTC 
Allow scsi device handler handle their own CDB and ship it down the
stream of scsi passthrough command setup flow, without any further
interventions.

This is useful for setting DRV-OP that implements vendor commands via
the scsi device handlers framework.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/scsi/scsi_lib.c | 5 +++--
 drivers/scsi/sd.c       | 9 +++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

Comments

Bart Van Assche May 16, 2020, 3:19 a.m. UTC | #1 
On 2020-05-15 03:30, Avri Altman wrote:
> Allow scsi device handler handle their own CDB and ship it down the
> stream of scsi passthrough command setup flow, without any further
> interventions.
> 
> This is useful for setting DRV-OP that implements vendor commands via
> the scsi device handlers framework.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/scsi/scsi_lib.c | 5 +++--
>  drivers/scsi/sd.c       | 9 +++++++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 6b6dd40..4e98714 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1148,14 +1148,15 @@ static blk_status_t scsi_setup_fs_cmnd(struct scsi_device *sdev,
>  {
>  	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
>  
> +	cmd->cmnd = scsi_req(req)->cmd = scsi_req(req)->__cmd;
> +	memset(cmd->cmnd, 0, BLK_MAX_CDB);
> +
>  	if (unlikely(sdev->handler && sdev->handler->prep_fn)) {
>  		blk_status_t ret = sdev->handler->prep_fn(sdev, req);
>  		if (ret != BLK_STS_OK)
>  			return ret;
>  	}
>  
> -	cmd->cmnd = scsi_req(req)->cmd = scsi_req(req)->__cmd;
> -	memset(cmd->cmnd, 0, BLK_MAX_CDB);
>  	return scsi_cmd_to_driver(cmd)->init_command(cmd);
>  }
>  
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index a793cb0..246bef8 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1221,6 +1221,14 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
>  	} else if (sdp->use_16_for_rw || (nr_blocks > 0xffff)) {
>  		ret = sd_setup_rw16_cmnd(cmd, write, lba, nr_blocks,
>  					 protect | fua);
> +	} else if (unlikely(sdp->handler && blk_rq_is_private(rq))) {
> +		/*
> +		 * scsi device handler that implements vendor commands -
> +		 * the command was already constructed in the device handler's
> +		 * prep_fn, so no need to do anything here
> +		 */
> +		rq->cmd_flags = REQ_OP_READ;
> +		ret = BLK_STS_OK;
>  	} else if ((nr_blocks > 0xff) || (lba > 0x1fffff) ||
>  		   sdp->use_10_for_rw || protect) {
>  		ret = sd_setup_rw10_cmnd(cmd, write, lba, nr_blocks,
> @@ -1285,6 +1293,7 @@ static blk_status_t sd_init_command(struct scsi_cmnd *cmd)
>  		return sd_setup_write_same_cmnd(cmd);
>  	case REQ_OP_FLUSH:
>  		return sd_setup_flush_cmnd(cmd);
> +	case REQ_OP_DRV_IN:
>  	case REQ_OP_READ:
>  	case REQ_OP_WRITE:
>  		return sd_setup_read_write_cmnd(cmd);

The above looks weird to me. My understanding is that
scsi_setup_fs_cmnd() is intended for operations like REQ_OP_READ,
REQ_OP_WRITE and REQ_OP_DISCARD. I don't think that it is appropriate to
pass REQ_OP_DRV_IN requests to scsi_setup_fs_cmnd() and/or
sd_init_command().

Thanks,

Bart.
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6b6dd40..4e98714 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1148,14 +1148,15 @@  static blk_status_t scsi_setup_fs_cmnd(struct scsi_device *sdev,
 {
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
 
+	cmd->cmnd = scsi_req(req)->cmd = scsi_req(req)->__cmd;
+	memset(cmd->cmnd, 0, BLK_MAX_CDB);
+
 	if (unlikely(sdev->handler && sdev->handler->prep_fn)) {
 		blk_status_t ret = sdev->handler->prep_fn(sdev, req);
 		if (ret != BLK_STS_OK)
 			return ret;
 	}
 
-	cmd->cmnd = scsi_req(req)->cmd = scsi_req(req)->__cmd;
-	memset(cmd->cmnd, 0, BLK_MAX_CDB);
 	return scsi_cmd_to_driver(cmd)->init_command(cmd);
 }
 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a793cb0..246bef8 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1221,6 +1221,14 @@  static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
 	} else if (sdp->use_16_for_rw || (nr_blocks > 0xffff)) {
 		ret = sd_setup_rw16_cmnd(cmd, write, lba, nr_blocks,
 					 protect | fua);
+	} else if (unlikely(sdp->handler && blk_rq_is_private(rq))) {
+		/*
+		 * scsi device handler that implements vendor commands -
+		 * the command was already constructed in the device handler's
+		 * prep_fn, so no need to do anything here
+		 */
+		rq->cmd_flags = REQ_OP_READ;
+		ret = BLK_STS_OK;
 	} else if ((nr_blocks > 0xff) || (lba > 0x1fffff) ||
 		   sdp->use_10_for_rw || protect) {
 		ret = sd_setup_rw10_cmnd(cmd, write, lba, nr_blocks,
@@ -1285,6 +1293,7 @@  static blk_status_t sd_init_command(struct scsi_cmnd *cmd)
 		return sd_setup_write_same_cmnd(cmd);
 	case REQ_OP_FLUSH:
 		return sd_setup_flush_cmnd(cmd);
+	case REQ_OP_DRV_IN:
 	case REQ_OP_READ:
 	case REQ_OP_WRITE:
 		return sd_setup_read_write_cmnd(cmd);