Reviewed-by: Yaniv Gardi <ygardi@xxxxxxxxxxxxxx> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation = > -----Original Message----- = > From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi- = > owner@xxxxxxxxxxxxxxx] On Behalf Of Dolev Raviv = > Sent: Monday, August 12, 2013 4:02 PM = > To: Sujit Reddy Thumma = > Cc: Vinayak Holikatti; Santosh Y; James E.J. Bottomley; linux- = > scsi@xxxxxxxxxxxxxxx; Sujit Reddy Thumma; linux-arm- = > msm@xxxxxxxxxxxxxxx = > Subject: Re: [PATCH V5 2/4] scsi: ufs: Fix hardware race conditions while = > aborting a command = > = > Tested-by: Dolev Raviv <draviv@xxxxxxxxxxxxxx> = > = > > There is a possible race condition in the hardware when the abort = > > command is issued to terminate the ongoing SCSI command as described = > > below: = > > = > > - A bit in the door-bell register is set in the controller for a = > > new SCSI command. = > > - In some rare situations, before controller get a chance to issue = > > the command to the device, the software issued an abort command. = > > - If the device recieves abort command first then it returns success = > > because the command itself is not present. = > > - Now if the controller commits the command to device it will be = > > processed. = > > - Software thinks that command is aborted and proceed while still = > > the device is processing it. = > > - The software, controller and device may go out of sync because of = > > this race condition. = > > = > > To avoid this, query task presence in the device before sending abort = > > task command so that after the abort operation, the command is = > > guaranteed to be non-existent in both controller and the device. = > > = > > Signed-off-by: Sujit Reddy Thumma <sthumma@xxxxxxxxxxxxxx> = > > --- = > > drivers/scsi/ufs/ufshcd.c | 70 = > > +++++++++++++++++++++++++++++++++++--------- = > > 1 files changed, 55 insertions(+), 15 deletions(-) = > > = > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c = > > index d7f3746..d4ee48d 100644 = > > --- a/drivers/scsi/ufs/ufshcd.c = > > +++ b/drivers/scsi/ufs/ufshcd.c = > > @@ -2485,6 +2485,12 @@ static int ufshcd_host_reset(struct scsi_cmnd = > *cmd) = > > * ufshcd_abort - abort a specific command = > > * @cmd: SCSI command pointer = > > * = > > + * Abort the pending command in device by sending UFS_ABORT_TASK = > task = > > management = > > + * command, and in host controller by clearing the door-bell register. = > > There can = > > + * be race between controller sending the command to the device while = > > abort is = > > + * issued. To avoid that, first issue UFS_QUERY_TASK to check if the = > > command is = > > + * really issued and then try to abort it. = > > + * = > > * Returns SUCCESS/FAILED = > > */ = > > static int ufshcd_abort(struct scsi_cmnd *cmd) @@ -2493,7 +2499,8 = > @@ = > > static int ufshcd_abort(struct scsi_cmnd *cmd) = > > struct ufs_hba *hba; = > > unsigned long flags; = > > unsigned int tag; = > > - int err; = > > + int err = 0; = > > + int poll_cnt; = > > u8 resp = 0xF; = > > struct ufshcd_lrb *lrbp; = > > = > > @@ -2501,33 +2508,59 @@ static int ufshcd_abort(struct scsi_cmnd = > *cmd) = > > hba = shost_priv(host); = > > tag = cmd->request->tag; = > > = > > - spin_lock_irqsave(host->host_lock, flags); = > > + /* If command is already aborted/completed, return SUCCESS */ = > > + if (!(test_bit(tag, &hba->outstanding_reqs))) = > > + goto out; = > > = > > - /* check if command is still pending */ = > > - if (!(test_bit(tag, &hba->outstanding_reqs))) { = > > - err = FAILED; = > > - spin_unlock_irqrestore(host->host_lock, flags); = > > + lrbp = &hba->lrb[tag]; = > > + for (poll_cnt = 100; poll_cnt; poll_cnt--) { = > > + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag, = > > + UFS_QUERY_TASK, &resp); = > > + if (!err && resp == = > UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) { = > > + /* cmd pending in the device */ = > > + break; = > > + } else if (!err && resp == = > UPIU_TASK_MANAGEMENT_FUNC_COMPL) { = > > + u32 reg; = > > + = > > + /* = > > + * cmd not pending in the device, check if it is = > > + * in transition. = > > + */ = > > + reg = ufshcd_readl(hba, = > REG_UTP_TRANSFER_REQ_DOOR_BELL); = > > + if (reg & (1 << tag)) { = > > + /* sleep for max. 2ms to stabilize */ = > > + usleep_range(1000, 2000); = > > + continue; = > > + } = > > + /* command completed already */ = > > + goto out; = > > + } else { = > > + if (!err) = > > + err = resp; /* service response error */ = > > + goto out; = > > + } = > > + } = > > + = > > + if (!poll_cnt) { = > > + err = -EBUSY; = > > goto out; = > > } = > > - spin_unlock_irqrestore(host->host_lock, flags); = > > = > > - lrbp = &hba->lrb[tag]; = > > err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag, = > > UFS_ABORT_TASK, &resp); = > > if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) { = > > - err = FAILED; = > > + if (!err) = > > + err = resp; /* service response error */ = > > goto out; = > > - } else { = > > - err = SUCCESS; = > > } = > > = > > + err = ufshcd_clear_cmd(hba, tag); = > > + if (err) = > > + goto out; = > > + = > > scsi_dma_unmap(cmd); = > > = > > spin_lock_irqsave(host->host_lock, flags); = > > - = > > - /* clear the respective UTRLCLR register bit */ = > > - ufshcd_utrl_clear(hba, tag); = > > - = > > __clear_bit(tag, &hba->outstanding_reqs); = > > hba->lrb[tag].cmd = NULL; = > > spin_unlock_irqrestore(host->host_lock, flags); @@ -2535,6 = > +2568,13 = > > @@ static int ufshcd_abort(struct scsi_cmnd *cmd) = > > clear_bit_unlock(tag, &hba->lrb_in_use); = > > wake_up(&hba->dev_cmd.tag_wq); = > > out: = > > + if (!err) { = > > + err = SUCCESS; = > > + } else { = > > + dev_err(hba->dev, "%s: failed with err %d\n", __func__, = > err); = > > + err = FAILED; = > > + } = > > + = > > return err; = > > } = > > = > > -- = > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a = > > member of Code Aurora Forum, hosted by The Linux Foundation. = > > = > > -- = > > To unsubscribe from this list: send the line "unsubscribe linux-scsi" = > > in the body of a message to majordomo@xxxxxxxxxxxxxxx More = > majordomo = > > info at http://vger.kernel.org/majordomo-info.html = > > = > = > = > -- = > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a = > member of Code Aurora Forum, hosted by The Linux Foundation = > = > -- = > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the = > body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info = > at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
