[PATCH v2 0/2] Fix residual handling in two SCSI LLDs

[PATCH v2 0/2] Fix residual handling in two SCSI LLDs

[Bart Van Assche]

Hi Martin,

This patch series fixes the documentation of scsi_set_resid() and also fixes
residual handling in two SCSI LLDs. Please consider this patch series for the
next merge window.

Thanks,

Bart.

Changes compared to v1:
- Left out a patch that has already been queued.
- Left out the device conformance checks.

Bart Van Assche (2):
  scsi: ufs: Fix residual handling
  RDMA/srp: Fix residual handling

 drivers/infiniband/ulp/srp/ib_srp.c |  4 ----
 drivers/ufs/core/ufshcd.c           | 12 ++++++++++--
 include/ufs/ufs.h                   |  6 ++++++
 3 files changed, 16 insertions(+), 6 deletions(-)

[PATCH v2 1/2] scsi: ufs: Fix residual handling

[Bart Van Assche]

Only call scsi_set_resid() in case of an underflow. Do not call
scsi_set_resid() in case of an overflow.

Cc: Avri Altman <avri.altman@wdc.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Fixes: cb38845d90fc ("scsi: ufs: core: Set the residual byte count")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 12 ++++++++++--
 include/ufs/ufs.h         |  6 ++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index c394dc50504a..27e1a4914837 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5222,9 +5222,17 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 	int result = 0;
 	int scsi_status;
 	enum utp_ocs ocs;
+	u8 upiu_flags;
+	u32 resid;
 
-	scsi_set_resid(lrbp->cmd,
-		be32_to_cpu(lrbp->ucd_rsp_ptr->sr.residual_transfer_count));
+	upiu_flags = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_0) >> 16;
+	resid = be32_to_cpu(lrbp->ucd_rsp_ptr->sr.residual_transfer_count);
+	/*
+	 * Test !overflow instead of underflow to support UFS devices that do
+	 * not set either flag.
+	 */
+	if (resid && !(upiu_flags & UPIU_RSP_FLAG_OVERFLOW))
+		scsi_set_resid(lrbp->cmd, resid);
 
 	/* overall command status of utrd */
 	ocs = ufshcd_get_tr_ocs(lrbp, cqe);
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 0dd546a20503..c789252b5fad 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -104,6 +104,12 @@ enum {
 	UPIU_CMD_FLAGS_READ	= 0x40,
 };
 
+/* UPIU response flags */
+enum {
+	UPIU_RSP_FLAG_UNDERFLOW	= 0x20,
+	UPIU_RSP_FLAG_OVERFLOW	= 0x40,
+};
+
 /* UPIU Task Attributes */
 enum {
 	UPIU_TASK_ATTR_SIMPLE	= 0x00,

RE: [PATCH v2 1/2] scsi: ufs: Fix residual handling

[Avri Altman]

> 
> Only call scsi_set_resid() in case of an underflow. Do not call
> scsi_set_resid() in case of an overflow.
> 
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Fixes: cb38845d90fc ("scsi: ufs: core: Set the residual byte count")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

Re: [PATCH v2 1/2] scsi: ufs: Fix residual handling

[Adrian Hunter]

On 24/07/23 23:08, Bart Van Assche wrote:
> Only call scsi_set_resid() in case of an underflow. Do not call
> scsi_set_resid() in case of an overflow.
> 
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Fixes: cb38845d90fc ("scsi: ufs: core: Set the residual byte count")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/ufs/core/ufshcd.c | 12 ++++++++++--
>  include/ufs/ufs.h         |  6 ++++++
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index c394dc50504a..27e1a4914837 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -5222,9 +5222,17 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
>  	int result = 0;
>  	int scsi_status;
>  	enum utp_ocs ocs;
> +	u8 upiu_flags;
> +	u32 resid;
>  
> -	scsi_set_resid(lrbp->cmd,
> -		be32_to_cpu(lrbp->ucd_rsp_ptr->sr.residual_transfer_count));
> +	upiu_flags = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_0) >> 16;
> +	resid = be32_to_cpu(lrbp->ucd_rsp_ptr->sr.residual_transfer_count);
> +	/*
> +	 * Test !overflow instead of underflow to support UFS devices that do
> +	 * not set either flag.
> +	 */
> +	if (resid && !(upiu_flags & UPIU_RSP_FLAG_OVERFLOW))
> +		scsi_set_resid(lrbp->cmd, resid);
>  
>  	/* overall command status of utrd */
>  	ocs = ufshcd_get_tr_ocs(lrbp, cqe);
> diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
> index 0dd546a20503..c789252b5fad 100644
> --- a/include/ufs/ufs.h
> +++ b/include/ufs/ufs.h
> @@ -104,6 +104,12 @@ enum {
>  	UPIU_CMD_FLAGS_READ	= 0x40,
>  };
>  
> +/* UPIU response flags */
> +enum {
> +	UPIU_RSP_FLAG_UNDERFLOW	= 0x20,
> +	UPIU_RSP_FLAG_OVERFLOW	= 0x40,
> +};
> +
>  /* UPIU Task Attributes */
>  enum {
>  	UPIU_TASK_ATTR_SIMPLE	= 0x00,

[PATCH v2 2/2] RDMA/srp: Fix residual handling

[Bart Van Assche]

Although the code for residual handling in the SRP initiator follows the
SCSI documentation, that documentation has never been correct. Because
scsi_finish_command() starts from the data buffer length and subtracts
the residual, scsi_set_resid() must not be called if a residual overflow
occurs. Hence remove the scsi_set_resid() calls from the SRP initiator
if a residual overflow occurrs.

Cc: Leon Romanovsky <leon@kernel.org>
Cc: Jason Gunthorpe <jgg@nvidia.com>
Fixes: 9237f04e12cc ("scsi: core: Fix scsi_get/set_resid() interface")
Fixes: e714531a349f ("IB/srp: Fix residual handling")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 0e513a7e5ac8..1574218764e0 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1979,12 +1979,8 @@ static void srp_process_rsp(struct srp_rdma_ch *ch, struct srp_rsp *rsp)
 
 		if (unlikely(rsp->flags & SRP_RSP_FLAG_DIUNDER))
 			scsi_set_resid(scmnd, be32_to_cpu(rsp->data_in_res_cnt));
-		else if (unlikely(rsp->flags & SRP_RSP_FLAG_DIOVER))
-			scsi_set_resid(scmnd, -be32_to_cpu(rsp->data_in_res_cnt));
 		else if (unlikely(rsp->flags & SRP_RSP_FLAG_DOUNDER))
 			scsi_set_resid(scmnd, be32_to_cpu(rsp->data_out_res_cnt));
-		else if (unlikely(rsp->flags & SRP_RSP_FLAG_DOOVER))
-			scsi_set_resid(scmnd, -be32_to_cpu(rsp->data_out_res_cnt));
 
 		srp_free_req(ch, req, scmnd,
 			     be32_to_cpu(rsp->req_lim_delta));

Re: [PATCH v2 2/2] RDMA/srp: Fix residual handling

[Leon Romanovsky]

On Mon, Jul 24, 2023 at 01:08:30PM -0700, Bart Van Assche wrote:
> Although the code for residual handling in the SRP initiator follows the
> SCSI documentation, that documentation has never been correct. Because
> scsi_finish_command() starts from the data buffer length and subtracts
> the residual, scsi_set_resid() must not be called if a residual overflow
> occurs. Hence remove the scsi_set_resid() calls from the SRP initiator
> if a residual overflow occurrs.
> 
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> Fixes: 9237f04e12cc ("scsi: core: Fix scsi_get/set_resid() interface")
> Fixes: e714531a349f ("IB/srp: Fix residual handling")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/infiniband/ulp/srp/ib_srp.c | 4 ----
>  1 file changed, 4 deletions(-)
> 

Thanks,
Acked-by: Leon Romanovsky <leon@kernel.org>

Re: [PATCH v2 0/2] Fix residual handling in two SCSI LLDs

[Martin K. Petersen]

Bart,

> This patch series fixes the documentation of scsi_set_resid() and also
> fixes residual handling in two SCSI LLDs. Please consider this patch
> series for the next merge window.

Applied to 6.6/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 0/2] Fix residual handling in two SCSI LLDs

[Martin K. Petersen]

On Mon, 24 Jul 2023 13:08:28 -0700, Bart Van Assche wrote:

> This patch series fixes the documentation of scsi_set_resid() and also fixes
> residual handling in two SCSI LLDs. Please consider this patch series for the
> next merge window.
> 
> Thanks,
> 
> Bart.
> 
> [...]

Applied to 6.6/scsi-queue, thanks!

[1/2] scsi: ufs: Fix residual handling
      https://git.kernel.org/mkp/scsi/c/2903265e27bf
[2/2] RDMA/srp: Fix residual handling
      https://git.kernel.org/mkp/scsi/c/89e637c19b24

-- 
Martin K. Petersen	Oracle Linux Engineering