[PATCH v2 1/2] scsi: ufs: core: Recover link in ufshcd_set_dev_pwr_mode() on timeout

[Stanley Jhu <stanleyjhu@google.com>]

ufshcd_eh_timed_out() calls ufshcd_link_recovery() when a command
times out during a power management operation. Link recovery sends a
NOP OUT, which needs hba->dev_cmd.lock and the only reserved tag.

1. Runtime resume sends a query with the reserved tag while it holds
   hba->dev_cmd.lock.
2. The device does not answer and the query times out.
3. ufshcd_eh_timed_out() calls ufshcd_link_recovery().
4. In MCQ mode ufshcd_verify_dev_init() waits for hba->dev_cmd.lock
   while __ufshcd_wl_resume() waits for the query. Resume hangs.
5. In legacy mode the query completes but the timeout handler still
   holds the reserved tag. The NOP OUT fails with -ENOMEM and the
   host enters UFSHCD_STATE_ERROR.

Stop the host controller in ufshcd_eh_timed_out() and recover the
link in ufshcd_set_dev_pwr_mode(), which issues START STOP UNIT
without hba->dev_cmd.lock or the reserved tag. For modes other than
active, ufshcd_set_dev_pwr_mode() then fails and the caller aborts
the power transition.

Recovering the link there needs four more changes. First, START STOP
UNIT is completed with DID_TRANSPORT_FAILFAST and is not retried.
ufshcd_set_dev_pwr_mode() also recovers the link if START STOP UNIT
succeeds but the controller is disabled. Hosts with
UFSHCI_QUIRK_BROKEN_HCE do not use the HCE bit, so
ufshcd_set_dev_pwr_mode() checks only the timeout result on them.
Second, in MCQ mode ufshcd_eh_timed_out() reaps the completion queue
before it completes the command, so a late completion cannot complete
the tag twice. Third, a timed-out reserved request forces
ufshcd_err_handler(), because no power management caller recovers
after it. Fourth, in MCQ mode ufshcd_err_handler() does not poll the
completion queues while the controller is disabled.

Fixes: 01d5e237b339 ("scsi: ufs: core: Handle PM commands timeout before SCSI EH")
Cc: stable@vger.kernel.org # 7.2.x
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 43 ++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ddc953b45ce9..0f5365bd90f0 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7108,8 +7108,12 @@ static void ufshcd_err_handler(struct work_struct *work)
 	ufshcd_set_eh_in_progress(hba);
 	spin_unlock_irqrestore(&hba->host->host_lock, flags);
 
-	/* Complete requests that have door-bell cleared by h/w */
-	ufshcd_complete_requests(hba, false);
+	/*
+	 * Complete requests that have door-bell cleared by h/w.
+	 * CQTPy must not be read while the controller is disabled.
+	 */
+	if (!hba->mcq_enabled || ufshcd_is_hba_active(hba))
+		ufshcd_complete_requests(hba, false);
 	spin_lock_irqsave(&hba->host->host_lock, flags);
 again:
 	needs_restore = false;
@@ -9660,17 +9664,20 @@ static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 	}
 
 	/*
-	 * Handle the timeout directly to prevent a deadlock between
-	 * ufshcd_set_dev_pwr_mode() and ufshcd_err_handler().
+	 * Stop the host controller so that @scmd can be completed here.
+	 * ufshcd_set_dev_pwr_mode() recovers the link for START STOP
+	 * UNIT. ufshcd_err_handler() recovers it for other commands.
 	 */
-	ufshcd_link_recovery(hba);
-	dev_info(hba->dev, "%s() finished; outstanding_tasks = %#lx.\n",
-		 __func__, hba->outstanding_tasks);
+	ufshcd_hba_stop(hba);
+	if (!ufshcd_is_scsi_cmd(scmd))
+		ufshcd_force_error_recovery(hba);
+	if (hba->mcq_enabled) {
+		struct ufs_hw_queue *hwq =
+			ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(scmd));
+
+		ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
+	}
 
-	/*
-	 * ufshcd_link_recovery() may already have completed @scmd, e.g. via
-	 * the existing MCQ force-completion path.
-	 */
 	if (!test_bit(SCMD_STATE_COMPLETE, &scmd->state)) {
 		if (!hba->mcq_enabled) {
 			struct request *rq = scsi_cmd_to_rq(scmd);
@@ -9680,7 +9687,7 @@ static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 		}
 
 		if (ufshcd_is_scsi_cmd(scmd)) {
-			set_host_byte(scmd, DID_REQUEUE);
+			set_host_byte(scmd, DID_TRANSPORT_FAILFAST);
 			ufshcd_release_scsi_cmd(hba, scmd);
 		} else {
 			set_host_byte(scmd, DID_TIME_OUT);
@@ -10137,6 +10144,9 @@ static int ufshcd_execute_start_stop(struct scsi_device *sdev,
 {
 	const unsigned char cdb[6] = { START_STOP, 0, 0, 0, pwr_mode << 4, 0 };
 	struct scsi_failure failure_defs[] = {
+		{
+			.result = DID_TRANSPORT_FAILFAST << 16,
+		},
 		{
 			.allowed = 2,
 			.result = SCMD_FAILURE_RESULT_ANY,
@@ -10172,6 +10182,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
 	struct scsi_sense_hdr sshdr;
 	struct scsi_device *sdp;
 	unsigned long flags;
+	bool fenced;
 	int ret;
 
 	spin_lock_irqsave(&hba->host->host_lock, flags);
@@ -10199,6 +10210,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
 	 * already suspended childs.
 	 */
 	ret = ufshcd_execute_start_stop(sdp, pwr_mode, &sshdr);
+	fenced = ret > 0 && host_byte(ret) == DID_TRANSPORT_FAILFAST;
 	if (ret) {
 		sdev_printk(KERN_WARNING, sdp,
 			    "START_STOP failed for power mode: %d, result %x\n",
@@ -10212,6 +10224,13 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
 		hba->curr_dev_pwr_mode = pwr_mode;
 	}
 
+	if (fenced || (!(hba->quirks & UFSHCI_QUIRK_BROKEN_HCE) &&
+		       !ufshcd_is_hba_active(hba))) {
+		ret = ufshcd_link_recovery(hba);
+		if (!ret && hba->curr_dev_pwr_mode != pwr_mode)
+			ret = -EIO;
+	}
+
 	scsi_device_put(sdp);
 	WRITE_ONCE(hba->host->eh_noresume, 0);
 	return ret;
-- 
2.56.0.611.g1030f0b940-goog