* [PATCH v1 1/5] ufs: mcq: Add supporting functions for mcq abort
[not found] <cover.1680083571.git.quic_nguyenb@quicinc.com>
@ 2023-03-29 10:01 ` Bao D. Nguyen
2023-04-11 13:14 ` Powen Kao (高伯文)
2023-03-29 10:01 ` [PATCH v1 2/5] ufs: mcq: Add support for clean up mcq resources Bao D. Nguyen
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Bao D. Nguyen @ 2023-03-29 10:01 UTC (permalink / raw)
To: quic_asutoshd, quic_cang, bvanassche, mani, stanley.chu,
adrian.hunter, beanhuo, avri.altman, martin.petersen
Cc: linux-scsi, Bao D. Nguyen, Alim Akhtar, James E.J. Bottomley,
Arthur Simchaev, Eric Biggers, open list
Add supporting functions to handle ufs abort in mcq mode.
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
drivers/ufs/core/ufs-mcq.c | 174 +++++++++++++++++++++++++++++++++++++++++
drivers/ufs/core/ufshcd-priv.h | 10 +++
drivers/ufs/core/ufshcd.c | 2 -
include/ufs/ufshci.h | 17 ++++
4 files changed, 201 insertions(+), 2 deletions(-)
diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 31df052..6f6b22c 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -12,6 +12,10 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include "ufshcd-priv.h"
+#include <linux/delay.h>
+#include <scsi/scsi_cmnd.h>
+#include <linux/bitfield.h>
+#include <linux/iopoll.h>
#define MAX_QUEUE_SUP GENMASK(7, 0)
#define UFS_MCQ_MIN_RW_QUEUES 2
@@ -27,6 +31,9 @@
#define MCQ_ENTRY_SIZE_IN_DWORD 8
#define CQE_UCD_BA GENMASK_ULL(63, 7)
+/* Max mcq register polling time in micro second unit */
+#define MCQ_POLL_US 500000
+
static int rw_queue_count_set(const char *val, const struct kernel_param *kp)
{
return param_set_uint_minmax(val, kp, UFS_MCQ_MIN_RW_QUEUES,
@@ -429,3 +436,170 @@ int ufshcd_mcq_init(struct ufs_hba *hba)
host->host_tagset = 1;
return 0;
}
+
+static int ufshcd_mcq_sq_stop(struct ufs_hba *hba, struct ufs_hw_queue *hwq)
+{
+ void __iomem *reg;
+ u32 i = hwq->id, val;
+ int err;
+
+ writel(SQ_STOP, mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTC);
+ reg = mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTS;
+ err = read_poll_timeout(readl, val, val & SQ_STS, 20,
+ MCQ_POLL_US, false, reg);
+ if (err)
+ dev_err(hba->dev, "%s: failed. hwq-id=%d, err=%d\n",
+ __func__, i, err);
+ return err;
+}
+
+static int ufshcd_mcq_sq_start(struct ufs_hba *hba, struct ufs_hw_queue *hwq)
+{
+ void __iomem *reg;
+ u32 i = hwq->id, val;
+ int err;
+
+ writel(SQ_START, mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTC);
+ reg = mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTS;
+ err = read_poll_timeout(readl, val, !(val & SQ_STS), 20,
+ MCQ_POLL_US, false, reg);
+ if (err)
+ dev_err(hba->dev, "%s: failed. hwq-id=%d, err=%d\n",
+ __func__, i, err);
+ return err;
+}
+
+/**
+ * ufshcd_mcq_sq_cleanup - Clean up Submission Queue resources
+ * associated with the pending command.
+ * @hba - per adapter instance.
+ * @task_tag - The command's task tag.
+ * @result - Result of the Clean up operation.
+ *
+ * Returns 0 and result on completion. Returns error code if
+ * the operation fails.
+ */
+int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result)
+{
+ struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
+ struct scsi_cmnd *cmd = lrbp->cmd;
+ struct ufs_hw_queue *hwq;
+ void __iomem *reg, *opr_sqd_base;
+ u32 nexus, i, val;
+ int err;
+
+ if (task_tag != hba->nutrs - UFSHCD_NUM_RESERVED) {
+ if (!cmd)
+ return FAILED;
+ hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
+ } else {
+ hwq = hba->dev_cmd_queue;
+ }
+
+ i = hwq->id;
+
+ spin_lock(&hwq->sq_lock);
+
+ /* stop the SQ fetching before working on it */
+ err = ufshcd_mcq_sq_stop(hba, hwq);
+ if (err)
+ goto unlock;
+
+ /* SQCTI = EXT_IID, IID, LUN, Task Tag */
+ nexus = lrbp->lun << 8 | task_tag;
+ opr_sqd_base = mcq_opr_base(hba, OPR_SQD, i);
+ writel(nexus, opr_sqd_base + REG_SQCTI);
+
+ /* SQRTCy.ICU = 1 */
+ writel(SQ_ICU, opr_sqd_base + REG_SQRTC);
+
+ /* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
+ reg = opr_sqd_base + REG_SQRTS;
+ err = read_poll_timeout(readl, val, val & SQ_CUS, 20,
+ MCQ_POLL_US, false, reg);
+ if (err)
+ dev_err(hba->dev, "%s: failed. hwq=%d, lun=0x%x, tag=%d\n",
+ __func__, i, lrbp->lun, task_tag);
+
+ *result = FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg));
+
+ if (ufshcd_mcq_sq_start(hba, hwq))
+ err = FAILED;
+
+unlock:
+ spin_unlock(&hwq->sq_lock);
+ return err;
+}
+
+static u64 ufshcd_mcq_get_cmd_desc_addr(struct ufs_hba *hba,
+ int task_tag)
+{
+ struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
+ __le32 hi = lrbp->utr_descriptor_ptr->command_desc_base_addr_hi;
+ __le32 lo = lrbp->utr_descriptor_ptr->command_desc_base_addr_lo;
+
+ return le64_to_cpu((__le64)hi << 32 | lo);
+}
+
+/**
+ * ufshcd_mcq_nullify_cmd - Nullify utrd. Host controller does not fetch
+ * transfer with Command Type = 0xF. post the Completion Queue with OCS=ABORTED.
+ * @hba - per adapter instance.
+ * @hwq - Hardware Queue of the nullified utrd.
+ */
+static void ufshcd_mcq_nullify_cmd(struct ufs_hba *hba, struct ufs_hw_queue *hwq)
+{
+ struct utp_transfer_req_desc *utrd;
+ u32 dword_0;
+
+ utrd = (struct utp_transfer_req_desc *)(hwq->sqe_base_addr +
+ hwq->id * sizeof(struct utp_transfer_req_desc));
+ dword_0 = le32_to_cpu(utrd->header.dword_0);
+ dword_0 &= ~UPIU_COMMAND_TYPE_MASK;
+ dword_0 |= FIELD_PREP(UPIU_COMMAND_TYPE_MASK, 0xF);
+ utrd->header.dword_0 = cpu_to_le32(dword_0);
+}
+
+/**
+ * ufshcd_mcq_sqe_search - Search for the cmd in the Submission Queue (SQ)
+ * @hba - per adapter instance.
+ * @hwq - Hardware Queue to be searched.
+ * @task_tag - The command's task tag.
+ *
+ * Returns true if the SQE containing the command is present in the SQ
+ * (not fetched by the controller); returns false if the SQE is not in the SQ.
+ */
+static bool ufshcd_mcq_sqe_search(struct ufs_hba *hba,
+ struct ufs_hw_queue *hwq, int task_tag)
+{
+ struct utp_transfer_req_desc *utrd;
+ u32 mask = hwq->max_entries - 1;
+ bool ret = false;
+ u64 addr, match;
+ u32 sq_head_slot;
+
+ spin_lock(&hwq->sq_lock);
+
+ ufshcd_mcq_sq_stop(hba, hwq);
+ sq_head_slot = ufshcd_mcq_get_sq_head_slot(hwq);
+ if (sq_head_slot == hwq->sq_tail_slot)
+ goto out;
+
+ addr = ufshcd_mcq_get_cmd_desc_addr(hba, task_tag);
+ while (sq_head_slot != hwq->sq_tail_slot) {
+ utrd = (struct utp_transfer_req_desc *)(hwq->sqe_base_addr +
+ sq_head_slot * sizeof(struct utp_transfer_req_desc));
+ match = le64_to_cpu((__le64)utrd->command_desc_base_addr_hi << 32 |
+ utrd->command_desc_base_addr_lo) & CQE_UCD_BA;
+ if (addr == match) {
+ ret = true;
+ goto out;
+ }
+ sq_head_slot = (sq_head_slot + 1) & mask;
+ }
+
+out:
+ ufshcd_mcq_sq_start(hba, hwq);
+ spin_unlock(&hwq->sq_lock);
+ return ret;
+}
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 529f850..1a40cf7 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -78,6 +78,8 @@ struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba,
unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
struct ufs_hw_queue *hwq);
+int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result);
+
#define UFSHCD_MCQ_IO_QUEUE_OFFSET 1
#define SD_ASCII_STD true
#define SD_RAW false
@@ -403,4 +405,12 @@ static inline struct cq_entry *ufshcd_mcq_cur_cqe(struct ufs_hw_queue *q)
return cqe + q->cq_head_slot;
}
+
+static inline u32 ufshcd_mcq_get_sq_head_slot(struct ufs_hw_queue *q)
+{
+ u32 val = readl(q->mcq_sq_head);
+
+ return val / sizeof(struct utp_transfer_req_desc);
+}
+
#endif /* _UFSHCD_PRIV_H_ */
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 35a3bd9..808387c 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -56,7 +56,6 @@
#define NOP_OUT_RETRIES 10
/* Timeout after 50 msecs if NOP OUT hangs without response */
#define NOP_OUT_TIMEOUT 50 /* msecs */
-
/* Query request retries */
#define QUERY_REQ_RETRIES 3
/* Query request timeout */
@@ -173,7 +172,6 @@ EXPORT_SYMBOL_GPL(ufshcd_dump_regs);
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
- UFSHCD_NUM_RESERVED = 1,
UFSHCD_CMD_PER_LUN = 32 - UFSHCD_NUM_RESERVED,
UFSHCD_CAN_QUEUE = 32 - UFSHCD_NUM_RESERVED,
};
diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
index 11424bb..ceb8664 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -99,6 +99,9 @@ enum {
enum {
REG_SQHP = 0x0,
REG_SQTP = 0x4,
+ REG_SQRTC = 0x8,
+ REG_SQCTI = 0xC,
+ REG_SQRTS = 0x10,
};
enum {
@@ -111,12 +114,26 @@ enum {
REG_CQIE = 0x4,
};
+enum {
+ SQ_START = 0x0,
+ SQ_STOP = 0x1,
+ SQ_ICU = 0x2,
+};
+
+enum {
+ SQ_STS = 0x1,
+ SQ_CUS = 0x2,
+};
+
+#define SQ_ICU_ERR_CODE_MASK GENMASK(7, 4)
+#define UPIU_COMMAND_TYPE_MASK GENMASK(31, 28)
#define UFS_MASK(mask, offset) ((mask) << (offset))
/* UFS Version 08h */
#define MINOR_VERSION_NUM_MASK UFS_MASK(0xFFFF, 0)
#define MAJOR_VERSION_NUM_MASK UFS_MASK(0xFFFF, 16)
+#define UFSHCD_NUM_RESERVED 1
/*
* Controller UFSHCI version
* - 2.x and newer use the following scheme:
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 1/5] ufs: mcq: Add supporting functions for mcq abort
2023-03-29 10:01 ` [PATCH v1 1/5] ufs: mcq: Add supporting functions for mcq abort Bao D. Nguyen
@ 2023-04-11 13:14 ` Powen Kao (高伯文)
2023-04-13 6:08 ` Bao D. Nguyen
0 siblings, 1 reply; 9+ messages in thread
From: Powen Kao (高伯文) @ 2023-04-11 13:14 UTC (permalink / raw)
To: quic_nguyenb, beanhuo, avri.altman, bvanassche, quic_asutoshd,
martin.petersen, mani, quic_cang, adrian.hunter
Cc: linux-scsi, alim.akhtar, jejb,
Stanley Chu (朱原陞),
Arthur.Simchaev, ebiggers, linux-kernel
Hi Bao D.,
We have done some test based on your RFC v3 patches and an issue is
reported.
kworker/u16:4: BUG: scheduling while atomic:
kworker/u16:4/5736/0x00000002
kworker/u16:4: [name:core&]Preemption disabled at:
kworker/u16:4: [<ffffffef97e33024>] ufshcd_mcq_sq_cleanup+0x9c/0x27c
kworker/u16:4: CPU: 2 PID: 5736 Comm: kworker/u16:4 Tainted: G
S W OE
kworker/u16:4: Workqueue: ufs_eh_wq_0 ufshcd_err_handler
kworker/u16:4: Call trace:
kworker/u16:4: dump_backtrace+0x108/0x15c
kworker/u16:4: show_stack+0x20/0x30
kworker/u16:4: dump_stack_lvl+0x6c/0x8c
kworker/u16:4: dump_stack+0x20/0x44
kworker/u16:4: __schedule_bug+0xd4/0x100
kworker/u16:4: __schedule+0x660/0xa5c
kworker/u16:4: schedule+0x80/0xec
kworker/u16:4: schedule_hrtimeout_range_clock+0xa0/0x140
kworker/u16:4: schedule_hrtimeout_range+0x1c/0x30
kworker/u16:4: usleep_range_state+0x88/0xd8
kworker/u16:4: ufshcd_mcq_sq_cleanup+0x170/0x27c
kworker/u16:4: ufshcd_clear_cmds+0x78/0x184
kworker/u16:4: ufshcd_wait_for_dev_cmd+0x234/0x348
kworker/u16:4: ufshcd_exec_dev_cmd+0x220/0x298
kworker/u16:4: ufshcd_verify_dev_init+0x68/0x124
kworker/u16:4: ufshcd_probe_hba+0x390/0x9bc
kworker/u16:4: ufshcd_host_reset_and_restore+0x74/0x158
kworker/u16:4: ufshcd_reset_and_restore+0x70/0x31c
kworker/u16:4: ufshcd_err_handler+0xad4/0xe58
kworker/u16:4: process_one_work+0x214/0x5b8
kworker/u16:4: worker_thread+0x2d4/0x448
kworker/u16:4: kthread+0x110/0x1e0
kworker/u16:4: ret_from_fork+0x10/0x20
kworker/u16:4: ------------[ cut here ]------------
On Wed, 2023-03-29 at 03:01 -0700, Bao D. Nguyen wrote:
> +/**
> + * ufshcd_mcq_sq_cleanup - Clean up Submission Queue resources
> + * associated with the pending command.
> + * @hba - per adapter instance.
> + * @task_tag - The command's task tag.
> + * @result - Result of the Clean up operation.
> + *
> + * Returns 0 and result on completion. Returns error code if
> + * the operation fails.
> + */
> +int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int
> *result)
> +{
> + struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
> + struct scsi_cmnd *cmd = lrbp->cmd;
> + struct ufs_hw_queue *hwq;
> + void __iomem *reg, *opr_sqd_base;
> + u32 nexus, i, val;
> + int err;
> +
> + if (task_tag != hba->nutrs - UFSHCD_NUM_RESERVED) {
> + if (!cmd)
> + return FAILED;
> + hwq = ufshcd_mcq_req_to_hwq(hba,
> scsi_cmd_to_rq(cmd));
> + } else {
> + hwq = hba->dev_cmd_queue;
> + }
> +
> + i = hwq->id;
> +
> + spin_lock(&hwq->sq_lock);
As spin_lock() disable preemption
> +
> + /* stop the SQ fetching before working on it */
> + err = ufshcd_mcq_sq_stop(hba, hwq);
> + if (err)
> + goto unlock;
> +
> + /* SQCTI = EXT_IID, IID, LUN, Task Tag */
> + nexus = lrbp->lun << 8 | task_tag;
> + opr_sqd_base = mcq_opr_base(hba, OPR_SQD, i);
> + writel(nexus, opr_sqd_base + REG_SQCTI);
> +
> + /* SQRTCy.ICU = 1 */
> + writel(SQ_ICU, opr_sqd_base + REG_SQRTC);
> +
> + /* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
> + reg = opr_sqd_base + REG_SQRTS;
> + err = read_poll_timeout(readl, val, val & SQ_CUS, 20,
> + MCQ_POLL_US, false, reg);
read_poll_timeout() was ufshcd_mcq_poll_register() in last patch,
right? ufshcd_mcq_poll_register() calls usleep_range() causing KE as
reported above. Same issue seems to still exist as read_poll_timeout()
sleeps.
Skipping ufshcd_mcq_sq_cleanup() by returning FAILED directly to
trigger reset in ufshcd error handler successfully recover host.
BTW, is there maybe a change list between RFC v3 and this v1 patch? :)
Thanks
Po-Wen
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 1/5] ufs: mcq: Add supporting functions for mcq abort
2023-04-11 13:14 ` Powen Kao (高伯文)
@ 2023-04-13 6:08 ` Bao D. Nguyen
0 siblings, 0 replies; 9+ messages in thread
From: Bao D. Nguyen @ 2023-04-13 6:08 UTC (permalink / raw)
To: Powen Kao (高伯文),
beanhuo, avri.altman, bvanassche, quic_asutoshd, martin.petersen,
mani, quic_cang, adrian.hunter
Cc: linux-scsi, alim.akhtar, jejb,
Stanley Chu (朱原陞),
Arthur.Simchaev, ebiggers, linux-kernel
On 4/11/2023 6:14 AM, Powen Kao (高伯文) wrote:
> Hi Bao D.,
>
> We have done some test based on your RFC v3 patches and an issue is
> reported.
Hi Powen,
Thank you very much for catching this issue. It seems that I cannot use
read_poll_timeout() because it sleeps while holding the spin_lock().
I will make the change to poll the registers in a tight loop with
udelay(20) polling interval in the next revision.
Thanks,
Bao
>
> kworker/u16:4: BUG: scheduling while atomic:
> kworker/u16:4/5736/0x00000002
> kworker/u16:4: [name:core&]Preemption disabled at:
> kworker/u16:4: [<ffffffef97e33024>] ufshcd_mcq_sq_cleanup+0x9c/0x27c
> kworker/u16:4: CPU: 2 PID: 5736 Comm: kworker/u16:4 Tainted: G
> S W OE
> kworker/u16:4: Workqueue: ufs_eh_wq_0 ufshcd_err_handler
> kworker/u16:4: Call trace:
> kworker/u16:4: dump_backtrace+0x108/0x15c
> kworker/u16:4: show_stack+0x20/0x30
> kworker/u16:4: dump_stack_lvl+0x6c/0x8c
> kworker/u16:4: dump_stack+0x20/0x44
> kworker/u16:4: __schedule_bug+0xd4/0x100
> kworker/u16:4: __schedule+0x660/0xa5c
> kworker/u16:4: schedule+0x80/0xec
> kworker/u16:4: schedule_hrtimeout_range_clock+0xa0/0x140
> kworker/u16:4: schedule_hrtimeout_range+0x1c/0x30
> kworker/u16:4: usleep_range_state+0x88/0xd8
> kworker/u16:4: ufshcd_mcq_sq_cleanup+0x170/0x27c
> kworker/u16:4: ufshcd_clear_cmds+0x78/0x184
> kworker/u16:4: ufshcd_wait_for_dev_cmd+0x234/0x348
> kworker/u16:4: ufshcd_exec_dev_cmd+0x220/0x298
> kworker/u16:4: ufshcd_verify_dev_init+0x68/0x124
> kworker/u16:4: ufshcd_probe_hba+0x390/0x9bc
> kworker/u16:4: ufshcd_host_reset_and_restore+0x74/0x158
> kworker/u16:4: ufshcd_reset_and_restore+0x70/0x31c
> kworker/u16:4: ufshcd_err_handler+0xad4/0xe58
> kworker/u16:4: process_one_work+0x214/0x5b8
> kworker/u16:4: worker_thread+0x2d4/0x448
> kworker/u16:4: kthread+0x110/0x1e0
> kworker/u16:4: ret_from_fork+0x10/0x20
> kworker/u16:4: ------------[ cut here ]------------
>
>
> On Wed, 2023-03-29 at 03:01 -0700, Bao D. Nguyen wrote:
>
>> +/**
>> + * ufshcd_mcq_sq_cleanup - Clean up Submission Queue resources
>> + * associated with the pending command.
>> + * @hba - per adapter instance.
>> + * @task_tag - The command's task tag.
>> + * @result - Result of the Clean up operation.
>> + *
>> + * Returns 0 and result on completion. Returns error code if
>> + * the operation fails.
>> + */
>> +int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int
>> *result)
>> +{
>> + struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
>> + struct scsi_cmnd *cmd = lrbp->cmd;
>> + struct ufs_hw_queue *hwq;
>> + void __iomem *reg, *opr_sqd_base;
>> + u32 nexus, i, val;
>> + int err;
>> +
>> + if (task_tag != hba->nutrs - UFSHCD_NUM_RESERVED) {
>> + if (!cmd)
>> + return FAILED;
>> + hwq = ufshcd_mcq_req_to_hwq(hba,
>> scsi_cmd_to_rq(cmd));
>> + } else {
>> + hwq = hba->dev_cmd_queue;
>> + }
>> +
>> + i = hwq->id;
>> +
>> + spin_lock(&hwq->sq_lock);
> As spin_lock() disable preemption
>
>> +
>> + /* stop the SQ fetching before working on it */
>> + err = ufshcd_mcq_sq_stop(hba, hwq);
>> + if (err)
>> + goto unlock;
>> +
>> + /* SQCTI = EXT_IID, IID, LUN, Task Tag */
>> + nexus = lrbp->lun << 8 | task_tag;
>> + opr_sqd_base = mcq_opr_base(hba, OPR_SQD, i);
>> + writel(nexus, opr_sqd_base + REG_SQCTI);
>> +
>> + /* SQRTCy.ICU = 1 */
>> + writel(SQ_ICU, opr_sqd_base + REG_SQRTC);
>> +
>> + /* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
>> + reg = opr_sqd_base + REG_SQRTS;
>> + err = read_poll_timeout(readl, val, val & SQ_CUS, 20,
>> + MCQ_POLL_US, false, reg);
> read_poll_timeout() was ufshcd_mcq_poll_register() in last patch,
> right? ufshcd_mcq_poll_register() calls usleep_range() causing KE as
> reported above. Same issue seems to still exist as read_poll_timeout()
> sleeps.
>
> Skipping ufshcd_mcq_sq_cleanup() by returning FAILED directly to
> trigger reset in ufshcd error handler successfully recover host.
>
> BTW, is there maybe a change list between RFC v3 and this v1 patch? :)
> Thanks
>
> Po-Wen
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 2/5] ufs: mcq: Add support for clean up mcq resources
[not found] <cover.1680083571.git.quic_nguyenb@quicinc.com>
2023-03-29 10:01 ` [PATCH v1 1/5] ufs: mcq: Add supporting functions for mcq abort Bao D. Nguyen
@ 2023-03-29 10:01 ` Bao D. Nguyen
2023-03-29 10:01 ` [PATCH v1 3/5] ufs: mcq: Added ufshcd_mcq_abort() Bao D. Nguyen
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Bao D. Nguyen @ 2023-03-29 10:01 UTC (permalink / raw)
To: quic_asutoshd, quic_cang, bvanassche, mani, stanley.chu,
adrian.hunter, beanhuo, avri.altman, martin.petersen
Cc: linux-scsi, Bao D. Nguyen, Alim Akhtar, James E.J. Bottomley, open list
Update ufshcd_clear_cmds() to clean up the mcq resources similar
to the function ufshcd_utrl_clear() does for sdb mode.
Update ufshcd_try_to_abort_task() to support mcq mode so that
this function can be invoked in either mcq or sdb mode.
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
drivers/ufs/core/ufshcd.c | 45 +++++++++++++++++++++++++++++++++++++++------
1 file changed, 39 insertions(+), 6 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 808387c..ffccb91 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3007,10 +3007,28 @@ static int ufshcd_compose_dev_cmd(struct ufs_hba *hba,
* @mask and wait until the controller confirms that these requests have been
* cleared.
*/
-static int ufshcd_clear_cmds(struct ufs_hba *hba, u32 mask)
+static int ufshcd_clear_cmds(struct ufs_hba *hba, unsigned long mask)
{
unsigned long flags;
+ int err, tag, result = FAILED;
+ if (is_mcq_enabled(hba)) {
+ /*
+ * MCQ mode. Clean up the MCQ resources similar to
+ * what the ufshcd_utrl_clear() does for SDB mode.
+ */
+ for_each_set_bit(tag, &mask, hba->nutrs) {
+ err = ufshcd_mcq_sq_cleanup(hba, tag, &result);
+ if (err || result) {
+ dev_err(hba->dev, "%s: failed tag=%d. err=%d, result=%d\n",
+ __func__, tag, err, result);
+ return FAILED;
+ }
+ }
+ return 0;
+ }
+
+ /* Single Doorbell Mode */
/* clear outstanding transaction before retry */
spin_lock_irqsave(hba->host->host_lock, flags);
ufshcd_utrl_clear(hba, mask);
@@ -3110,7 +3128,7 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
err = -ETIMEDOUT;
dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n",
__func__, lrbp->task_tag);
- if (ufshcd_clear_cmds(hba, 1U << lrbp->task_tag) == 0) {
+ if (ufshcd_clear_cmds(hba, 1UL << lrbp->task_tag) == 0) {
/* successfully cleared the command, retry if needed */
err = -EAGAIN;
/*
@@ -7379,6 +7397,20 @@ static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
*/
dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
__func__, tag);
+ if (is_mcq_enabled(hba)) {
+ /* MCQ mode */
+ if (lrbp->cmd) {
+ /* sleep for max. 200us to stabilize */
+ usleep_range(100, 200);
+ continue;
+ }
+ /* command completed already */
+ dev_err(hba->dev, "%s: cmd at tag=%d is cleared.\n",
+ __func__, tag);
+ goto out;
+ }
+
+ /* Single Doorbell Mode */
reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
if (reg & (1 << tag)) {
/* sleep for max. 200us to stabilize */
@@ -7415,7 +7447,7 @@ static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
goto out;
}
- err = ufshcd_clear_cmds(hba, 1U << tag);
+ err = ufshcd_clear_cmds(hba, 1UL << tag);
if (err)
dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
__func__, tag, err);
@@ -7445,8 +7477,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
ufshcd_hold(hba, false);
reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
- /* If command is already aborted/completed, return FAILED. */
- if (!(test_bit(tag, &hba->outstanding_reqs))) {
+ if (!is_mcq_enabled(hba) && !(test_bit(tag, &hba->outstanding_reqs))) {
+ /* If command is already aborted/completed, return FAILED. */
dev_err(hba->dev,
"%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n",
__func__, tag, hba->outstanding_reqs, reg);
@@ -7475,7 +7507,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
}
hba->req_abort_count++;
- if (!(reg & (1 << tag))) {
+ if (!is_mcq_enabled(hba) && !(reg & (1 << tag))) {
+ /* only execute this code in single doorbell mode */
dev_err(hba->dev,
"%s: cmd was completed, but without a notifying intr, tag = %d",
__func__, tag);
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v1 3/5] ufs: mcq: Added ufshcd_mcq_abort()
[not found] <cover.1680083571.git.quic_nguyenb@quicinc.com>
2023-03-29 10:01 ` [PATCH v1 1/5] ufs: mcq: Add supporting functions for mcq abort Bao D. Nguyen
2023-03-29 10:01 ` [PATCH v1 2/5] ufs: mcq: Add support for clean up mcq resources Bao D. Nguyen
@ 2023-03-29 10:01 ` Bao D. Nguyen
2023-03-29 10:01 ` [PATCH v1 4/5] ufs: mcq: Use ufshcd_mcq_poll_cqe_lock() in mcq mode Bao D. Nguyen
2023-03-29 10:01 ` [PATCH v1 5/5] ufs: core: Add error handling for MCQ mode Bao D. Nguyen
4 siblings, 0 replies; 9+ messages in thread
From: Bao D. Nguyen @ 2023-03-29 10:01 UTC (permalink / raw)
To: quic_asutoshd, quic_cang, bvanassche, mani, stanley.chu,
adrian.hunter, beanhuo, avri.altman, martin.petersen
Cc: linux-scsi, Bao D. Nguyen, Alim Akhtar, James E.J. Bottomley,
Arthur Simchaev, open list
Add ufshcd_mcq_abort() to support ufs abort in mcq mode.
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
drivers/ufs/core/ufs-mcq.c | 62 ++++++++++++++++++++++++++++++++++++++++++
drivers/ufs/core/ufshcd-priv.h | 5 +++-
drivers/ufs/core/ufshcd.c | 11 ++++++--
3 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 6f6b22c..78bd5d6 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -603,3 +603,65 @@ static bool ufshcd_mcq_sqe_search(struct ufs_hba *hba,
spin_unlock(&hwq->sq_lock);
return ret;
}
+
+/**
+ * ufshcd_mcq_abort - Abort the command in MCQ.
+ * @cmd - The command to be aborted.
+ *
+ * Returns SUCCESS or FAILED error codes
+ */
+int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
+{
+ struct Scsi_Host *host = cmd->device->host;
+ struct ufs_hba *hba = shost_priv(host);
+ int tag = scsi_cmd_to_rq(cmd)->tag;
+ struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+ struct ufs_hw_queue *hwq;
+ int err = FAILED;
+
+ if (!lrbp->cmd) {
+ dev_err(hba->dev,
+ "%s: skip abort. cmd at tag %d already completed.\n",
+ __func__, tag);
+ goto out;
+ }
+
+ /* Skip task abort in case previous aborts failed and report failure */
+ if (lrbp->req_abort_skip) {
+ dev_err(hba->dev, "%s: skip abort. tag %d failed earlier\n",
+ __func__, tag);
+ goto out;
+ }
+
+ hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
+
+ if (ufshcd_mcq_sqe_search(hba, hwq, tag)) {
+ /*
+ * Failure. The command should not be "stuck" in SQ for
+ * a long time which resulted in command being aborted.
+ */
+ dev_err(hba->dev, "%s: cmd found in sq. hwq=%d, tag=%d\n",
+ __func__, hwq->id, tag);
+ /* Set the Command Type to 0xF per the spec */
+ ufshcd_mcq_nullify_cmd(hba, hwq);
+ goto out;
+ }
+
+ /*
+ * The command is not in the Submission Queue, and it is not
+ * in the Completion Queue either. Query the device to see if
+ * the command is being processed in the device.
+ */
+ if (ufshcd_try_to_abort_task(hba, tag)) {
+ dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
+ lrbp->req_abort_skip = true;
+ goto out;
+ }
+
+ err = SUCCESS;
+ if (lrbp->cmd)
+ ufshcd_release_scsi_cmd(hba, lrbp);
+
+out:
+ return err;
+}
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 1a40cf7..ef66151 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -79,7 +79,10 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
struct ufs_hw_queue *hwq);
int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result);
-
+int ufshcd_mcq_abort(struct scsi_cmnd *cmd);
+int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
+void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
+ struct ufshcd_lrb *lrbp);
#define UFSHCD_MCQ_IO_QUEUE_OFFSET 1
#define SD_ASCII_STD true
#define SD_RAW false
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ffccb91..84efb52 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -299,7 +299,6 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
struct ufs_vreg *vreg);
-static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
bool enable);
static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
@@ -5438,7 +5437,7 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
}
/* Release the resources allocated for processing a SCSI command. */
-static void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
+void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp)
{
struct scsi_cmnd *cmd = lrbp->cmd;
@@ -7374,7 +7373,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
*
* Returns zero on success, non-zero on failure
*/
-static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
+int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
{
struct ufshcd_lrb *lrbp = &hba->lrb[tag];
int err = 0;
@@ -7534,6 +7533,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
goto release;
}
+ if (is_mcq_enabled(hba)) {
+ /* MCQ mode. Branch off to handle abort for mcq mode */
+ err = ufshcd_mcq_abort(cmd);
+ goto release;
+ }
+
/* Skip task abort in case previous aborts failed and report failure */
if (lrbp->req_abort_skip) {
dev_err(hba->dev, "%s: skipping abort\n", __func__);
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v1 4/5] ufs: mcq: Use ufshcd_mcq_poll_cqe_lock() in mcq mode
[not found] <cover.1680083571.git.quic_nguyenb@quicinc.com>
` (2 preceding siblings ...)
2023-03-29 10:01 ` [PATCH v1 3/5] ufs: mcq: Added ufshcd_mcq_abort() Bao D. Nguyen
@ 2023-03-29 10:01 ` Bao D. Nguyen
2023-03-29 10:01 ` [PATCH v1 5/5] ufs: core: Add error handling for MCQ mode Bao D. Nguyen
4 siblings, 0 replies; 9+ messages in thread
From: Bao D. Nguyen @ 2023-03-29 10:01 UTC (permalink / raw)
To: quic_asutoshd, quic_cang, bvanassche, mani, stanley.chu,
adrian.hunter, beanhuo, avri.altman, martin.petersen
Cc: linux-scsi, Bao D. Nguyen, Alim Akhtar, James E.J. Bottomley,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Arthur Simchaev,
open list,
open list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
In preparation for adding mcq error handler support, update the mcq
code to use the ufshcd_mcq_poll_cqe_lock() in interrupt context
instead of using ufshcd_mcq_poll_cqe_nolock(). This is to keep
synchronization between mcq interrupt and error handler contexts
because both need to access the mcq hardware in separate contexts.
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
drivers/ufs/core/ufs-mcq.c | 6 +++---
drivers/ufs/core/ufshcd-priv.h | 2 --
drivers/ufs/core/ufshcd.c | 2 +-
drivers/ufs/host/ufs-qcom.c | 2 +-
include/ufs/ufshcd.h | 2 +-
5 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 78bd5d6..1ae2ec4 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -284,8 +284,8 @@ static void ufshcd_mcq_process_cqe(struct ufs_hba *hba,
ufshcd_compl_one_cqe(hba, tag, cqe);
}
-unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
- struct ufs_hw_queue *hwq)
+static unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
+ struct ufs_hw_queue *hwq)
{
unsigned long completed_reqs = 0;
@@ -301,7 +301,6 @@ unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
return completed_reqs;
}
-EXPORT_SYMBOL_GPL(ufshcd_mcq_poll_cqe_nolock);
unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
struct ufs_hw_queue *hwq)
@@ -314,6 +313,7 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
return completed_reqs;
}
+EXPORT_SYMBOL_GPL(ufshcd_mcq_poll_cqe_lock);
void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba)
{
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index ef66151..70b7ffc 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -71,8 +71,6 @@ void ufshcd_mcq_config_mac(struct ufs_hba *hba, u32 max_active_cmds);
void ufshcd_mcq_select_mcq_mode(struct ufs_hba *hba);
u32 ufshcd_mcq_read_cqis(struct ufs_hba *hba, int i);
void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
-unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
- struct ufs_hw_queue *hwq);
struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba,
struct request *req);
unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 84efb52..fef1907 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6792,7 +6792,7 @@ static irqreturn_t ufshcd_handle_mcq_cq_events(struct ufs_hba *hba)
ufshcd_mcq_write_cqis(hba, events, i);
if (events & UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS)
- ufshcd_mcq_poll_cqe_nolock(hba, hwq);
+ ufshcd_mcq_poll_cqe_lock(hba, hwq);
}
return IRQ_HANDLED;
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 6568fdf..34d3814 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1558,7 +1558,7 @@ static irqreturn_t ufs_qcom_mcq_esi_handler(int irq, void *__hba)
struct ufs_hw_queue *hwq = &hba->uhq[id];
ufshcd_mcq_write_cqis(hba, 0x1, id);
- ufshcd_mcq_poll_cqe_nolock(hba, hwq);
+ ufshcd_mcq_poll_cqe_lock(hba, hwq);
return IRQ_HANDLED;
}
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 25aab8e..3fca558 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1246,7 +1246,7 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val);
void ufshcd_hba_stop(struct ufs_hba *hba);
void ufshcd_schedule_eh_work(struct ufs_hba *hba);
void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
-unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba,
+unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
struct ufs_hw_queue *hwq);
void ufshcd_mcq_enable_esi(struct ufs_hba *hba);
void ufshcd_mcq_config_esi(struct ufs_hba *hba, struct msi_msg *msg);
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v1 5/5] ufs: core: Add error handling for MCQ mode
[not found] <cover.1680083571.git.quic_nguyenb@quicinc.com>
` (3 preceding siblings ...)
2023-03-29 10:01 ` [PATCH v1 4/5] ufs: mcq: Use ufshcd_mcq_poll_cqe_lock() in mcq mode Bao D. Nguyen
@ 2023-03-29 10:01 ` Bao D. Nguyen
2023-04-27 7:17 ` Stanley Chu
4 siblings, 1 reply; 9+ messages in thread
From: Bao D. Nguyen @ 2023-03-29 10:01 UTC (permalink / raw)
To: quic_asutoshd, quic_cang, bvanassche, mani, stanley.chu,
adrian.hunter, beanhuo, avri.altman, martin.petersen
Cc: linux-scsi, Bao D. Nguyen, Alim Akhtar, James E.J. Bottomley, open list
Add support for error handling for MCQ mode.
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
drivers/ufs/core/ufshcd.c | 80 ++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 69 insertions(+), 11 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index fef1907..e947f7f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3127,6 +3127,12 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
err = -ETIMEDOUT;
dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n",
__func__, lrbp->task_tag);
+
+ /* MCQ mode */
+ if (is_mcq_enabled(hba))
+ return ufshcd_clear_cmds(hba, 1UL << lrbp->task_tag);
+
+ /* SDB mode */
if (ufshcd_clear_cmds(hba, 1UL << lrbp->task_tag) == 0) {
/* successfully cleared the command, retry if needed */
err = -EAGAIN;
@@ -5562,6 +5568,10 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
*/
static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
{
+ struct ufshcd_lrb *lrbp;
+ u32 hwq_num, utag;
+ int tag;
+
/* Resetting interrupt aggregation counters first and reading the
* DOOR_BELL afterward allows us to handle all the completed requests.
* In order to prevent other interrupts starvation the DB is read once
@@ -5580,7 +5590,22 @@ static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
* Ignore the ufshcd_poll() return value and return IRQ_HANDLED since we
* do not want polling to trigger spurious interrupt complaints.
*/
- ufshcd_poll(hba->host, UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);
+ if (!is_mcq_enabled(hba)) {
+ ufshcd_poll(hba->host, UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);
+ goto out;
+ }
+
+ /* MCQ mode */
+ for (tag = 0; tag < hba->nutrs; tag++) {
+ lrbp = &hba->lrb[tag];
+ if (lrbp->cmd) {
+ utag = blk_mq_unique_tag(scsi_cmd_to_rq(lrbp->cmd));
+ hwq_num = blk_mq_unique_tag_to_hwq(utag);
+ ufshcd_poll(hba->host, hwq_num);
+ }
+ }
+
+out:
return IRQ_HANDLED;
}
@@ -6359,18 +6384,36 @@ static bool ufshcd_abort_all(struct ufs_hba *hba)
bool needs_reset = false;
int tag, ret;
- /* Clear pending transfer requests */
- for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
- ret = ufshcd_try_to_abort_task(hba, tag);
- dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
- hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
- ret ? "failed" : "succeeded");
- if (ret) {
- needs_reset = true;
- goto out;
+ if (is_mcq_enabled(hba)) {
+ struct ufshcd_lrb *lrbp;
+ int tag;
+
+ for (tag = 0; tag < hba->nutrs; tag++) {
+ lrbp = &hba->lrb[tag];
+ if (lrbp->cmd) {
+ ret = ufshcd_try_to_abort_task(hba, tag);
+ dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
+ hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
+ ret ? "failed" : "succeeded");
+ }
+ if (ret) {
+ needs_reset = true;
+ goto out;
+ }
+ }
+ } else {
+ /* Clear pending transfer requests */
+ for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
+ ret = ufshcd_try_to_abort_task(hba, tag);
+ dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
+ hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
+ ret ? "failed" : "succeeded");
+ if (ret) {
+ needs_reset = true;
+ goto out;
+ }
}
}
-
/* Clear pending task management requests */
for_each_set_bit(tag, &hba->outstanding_tasks, hba->nutmrs) {
if (ufshcd_clear_tm_cmd(hba, tag)) {
@@ -7302,6 +7345,8 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
unsigned long flags, pending_reqs = 0, not_cleared = 0;
struct Scsi_Host *host;
struct ufs_hba *hba;
+ struct ufs_hw_queue *hwq;
+ struct ufshcd_lrb *lrbp;
u32 pos;
int err;
u8 resp = 0xF, lun;
@@ -7317,6 +7362,19 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
goto out;
}
+ if (is_mcq_enabled(hba)) {
+ for (pos = 0; pos < hba->nutrs; pos++) {
+ lrbp = &hba->lrb[pos];
+ if (lrbp->cmd && lrbp->lun == lun) {
+ ufshcd_clear_cmds(hba, 1UL << pos);
+ hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(lrbp->cmd));
+ ufshcd_mcq_poll_cqe_lock(hba, hwq);
+ }
+ }
+ err = 0;
+ goto out;
+ }
+
/* clear the commands that were pending for corresponding LUN */
spin_lock_irqsave(&hba->outstanding_lock, flags);
for_each_set_bit(pos, &hba->outstanding_reqs, hba->nutrs)
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 5/5] ufs: core: Add error handling for MCQ mode
2023-03-29 10:01 ` [PATCH v1 5/5] ufs: core: Add error handling for MCQ mode Bao D. Nguyen
@ 2023-04-27 7:17 ` Stanley Chu
2023-05-04 4:18 ` Bao D. Nguyen
0 siblings, 1 reply; 9+ messages in thread
From: Stanley Chu @ 2023-04-27 7:17 UTC (permalink / raw)
To: Bao D. Nguyen
Cc: quic_asutoshd, quic_cang, bvanassche, mani, stanley.chu,
adrian.hunter, beanhuo, avri.altman, martin.petersen, linux-scsi,
Alim Akhtar, James E.J. Bottomley, open list, Po-Wen Kao,
peter.wang, Alice Chao, naomi.chu
Hi Bao,
Bao D. Nguyen <quic_nguyenb@quicinc.com> 於 2023年3月29日 週三 下午6:14寫道：
>
> Add support for error handling for MCQ mode.
>
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> ---
> drivers/ufs/core/ufshcd.c | 80 ++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 69 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index fef1907..e947f7f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -3127,6 +3127,12 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
> err = -ETIMEDOUT;
> dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n",
> __func__, lrbp->task_tag);
> +
> + /* MCQ mode */
> + if (is_mcq_enabled(hba))
> + return ufshcd_clear_cmds(hba, 1UL << lrbp->task_tag);
When a time-out occurs during the command-clearing process, it appears
that the MCQ path does not properly clear 'hba->dev_cmd.complete'.
This could result in a null pointer reference if the device command
interrupt arrives at a later time.
Could you please help check this?
Thanks,
Stanley Chu
> +
> + /* SDB mode */
> if (ufshcd_clear_cmds(hba, 1UL << lrbp->task_tag) == 0) {
> /* successfully cleared the command, retry if needed */
> err = -EAGAIN;
> @@ -5562,6 +5568,10 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
> */
> static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
> {
> + struct ufshcd_lrb *lrbp;
> + u32 hwq_num, utag;
> + int tag;
> +
> /* Resetting interrupt aggregation counters first and reading the
> * DOOR_BELL afterward allows us to handle all the completed requests.
> * In order to prevent other interrupts starvation the DB is read once
> @@ -5580,7 +5590,22 @@ static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
> * Ignore the ufshcd_poll() return value and return IRQ_HANDLED since we
> * do not want polling to trigger spurious interrupt complaints.
> */
> - ufshcd_poll(hba->host, UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);
> + if (!is_mcq_enabled(hba)) {
> + ufshcd_poll(hba->host, UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);
> + goto out;
> + }
> +
> + /* MCQ mode */
> + for (tag = 0; tag < hba->nutrs; tag++) {
> + lrbp = &hba->lrb[tag];
> + if (lrbp->cmd) {
> + utag = blk_mq_unique_tag(scsi_cmd_to_rq(lrbp->cmd));
> + hwq_num = blk_mq_unique_tag_to_hwq(utag);
> + ufshcd_poll(hba->host, hwq_num);
> + }
> + }
> +
> +out:
>
> return IRQ_HANDLED;
> }
> @@ -6359,18 +6384,36 @@ static bool ufshcd_abort_all(struct ufs_hba *hba)
> bool needs_reset = false;
> int tag, ret;
>
> - /* Clear pending transfer requests */
> - for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
> - ret = ufshcd_try_to_abort_task(hba, tag);
> - dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
> - hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
> - ret ? "failed" : "succeeded");
> - if (ret) {
> - needs_reset = true;
> - goto out;
> + if (is_mcq_enabled(hba)) {
> + struct ufshcd_lrb *lrbp;
> + int tag;
> +
> + for (tag = 0; tag < hba->nutrs; tag++) {
> + lrbp = &hba->lrb[tag];
> + if (lrbp->cmd) {
> + ret = ufshcd_try_to_abort_task(hba, tag);
> + dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
> + hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
> + ret ? "failed" : "succeeded");
> + }
> + if (ret) {
> + needs_reset = true;
> + goto out;
> + }
> + }
> + } else {
> + /* Clear pending transfer requests */
> + for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
> + ret = ufshcd_try_to_abort_task(hba, tag);
> + dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
> + hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
> + ret ? "failed" : "succeeded");
> + if (ret) {
> + needs_reset = true;
> + goto out;
> + }
> }
> }
> -
> /* Clear pending task management requests */
> for_each_set_bit(tag, &hba->outstanding_tasks, hba->nutmrs) {
> if (ufshcd_clear_tm_cmd(hba, tag)) {
> @@ -7302,6 +7345,8 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
> unsigned long flags, pending_reqs = 0, not_cleared = 0;
> struct Scsi_Host *host;
> struct ufs_hba *hba;
> + struct ufs_hw_queue *hwq;
> + struct ufshcd_lrb *lrbp;
> u32 pos;
> int err;
> u8 resp = 0xF, lun;
> @@ -7317,6 +7362,19 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
> goto out;
> }
>
> + if (is_mcq_enabled(hba)) {
> + for (pos = 0; pos < hba->nutrs; pos++) {
> + lrbp = &hba->lrb[pos];
> + if (lrbp->cmd && lrbp->lun == lun) {
> + ufshcd_clear_cmds(hba, 1UL << pos);
> + hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(lrbp->cmd));
> + ufshcd_mcq_poll_cqe_lock(hba, hwq);
> + }
> + }
> + err = 0;
> + goto out;
> + }
> +
> /* clear the commands that were pending for corresponding LUN */
> spin_lock_irqsave(&hba->outstanding_lock, flags);
> for_each_set_bit(pos, &hba->outstanding_reqs, hba->nutrs)
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 5/5] ufs: core: Add error handling for MCQ mode
2023-04-27 7:17 ` Stanley Chu
@ 2023-05-04 4:18 ` Bao D. Nguyen
0 siblings, 0 replies; 9+ messages in thread
From: Bao D. Nguyen @ 2023-05-04 4:18 UTC (permalink / raw)
To: Stanley Chu
Cc: quic_asutoshd, quic_cang, bvanassche, mani, stanley.chu,
adrian.hunter, beanhuo, avri.altman, martin.petersen, linux-scsi,
Alim Akhtar, James E.J. Bottomley, open list, Po-Wen Kao,
peter.wang, Alice Chao, naomi.chu
On 4/27/2023 12:17 AM, Stanley Chu wrote:
> Hi Bao,
>
> Bao D. Nguyen <quic_nguyenb@quicinc.com> 於 2023年3月29日 週三 下午6:14寫道：
>>
>> Add support for error handling for MCQ mode.
>>
>> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
>> ---
>> drivers/ufs/core/ufshcd.c | 80 ++++++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 69 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index fef1907..e947f7f 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -3127,6 +3127,12 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
>> err = -ETIMEDOUT;
>> dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n",
>> __func__, lrbp->task_tag);
>> +
>> + /* MCQ mode */
>> + if (is_mcq_enabled(hba))
>> + return ufshcd_clear_cmds(hba, 1UL << lrbp->task_tag);
>
> When a time-out occurs during the command-clearing process, it appears
> that the MCQ path does not properly clear 'hba->dev_cmd.complete'.
> This could result in a null pointer reference if the device command
> interrupt arrives at a later time.
>
> Could you please help check this?
Thanks Stanley. I will take a look.
>
> Thanks,
> Stanley Chu
>
>> +
>> + /* SDB mode */
>> if (ufshcd_clear_cmds(hba, 1UL << lrbp->task_tag) == 0) {
>> /* successfully cleared the command, retry if needed */
>> err = -EAGAIN;
>> @@ -5562,6 +5568,10 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
>> */
>> static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
>> {
>> + struct ufshcd_lrb *lrbp;
>> + u32 hwq_num, utag;
>> + int tag;
>> +
>> /* Resetting interrupt aggregation counters first and reading the
>> * DOOR_BELL afterward allows us to handle all the completed requests.
>> * In order to prevent other interrupts starvation the DB is read once
>> @@ -5580,7 +5590,22 @@ static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
>> * Ignore the ufshcd_poll() return value and return IRQ_HANDLED since we
>> * do not want polling to trigger spurious interrupt complaints.
>> */
>> - ufshcd_poll(hba->host, UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);
>> + if (!is_mcq_enabled(hba)) {
>> + ufshcd_poll(hba->host, UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);
>> + goto out;
>> + }
>> +
>> + /* MCQ mode */
>> + for (tag = 0; tag < hba->nutrs; tag++) {
>> + lrbp = &hba->lrb[tag];
>> + if (lrbp->cmd) {
>> + utag = blk_mq_unique_tag(scsi_cmd_to_rq(lrbp->cmd));
>> + hwq_num = blk_mq_unique_tag_to_hwq(utag);
>> + ufshcd_poll(hba->host, hwq_num);
>> + }
>> + }
>> +
>> +out:
>>
>> return IRQ_HANDLED;
>> }
>> @@ -6359,18 +6384,36 @@ static bool ufshcd_abort_all(struct ufs_hba *hba)
>> bool needs_reset = false;
>> int tag, ret;
>>
>> - /* Clear pending transfer requests */
>> - for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
>> - ret = ufshcd_try_to_abort_task(hba, tag);
>> - dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
>> - hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
>> - ret ? "failed" : "succeeded");
>> - if (ret) {
>> - needs_reset = true;
>> - goto out;
>> + if (is_mcq_enabled(hba)) {
>> + struct ufshcd_lrb *lrbp;
>> + int tag;
>> +
>> + for (tag = 0; tag < hba->nutrs; tag++) {
>> + lrbp = &hba->lrb[tag];
>> + if (lrbp->cmd) {
>> + ret = ufshcd_try_to_abort_task(hba, tag);
>> + dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
>> + hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
>> + ret ? "failed" : "succeeded");
>> + }
>> + if (ret) {
>> + needs_reset = true;
>> + goto out;
>> + }
>> + }
>> + } else {
>> + /* Clear pending transfer requests */
>> + for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
>> + ret = ufshcd_try_to_abort_task(hba, tag);
>> + dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
>> + hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
>> + ret ? "failed" : "succeeded");
>> + if (ret) {
>> + needs_reset = true;
>> + goto out;
>> + }
>> }
>> }
>> -
>> /* Clear pending task management requests */
>> for_each_set_bit(tag, &hba->outstanding_tasks, hba->nutmrs) {
>> if (ufshcd_clear_tm_cmd(hba, tag)) {
>> @@ -7302,6 +7345,8 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
>> unsigned long flags, pending_reqs = 0, not_cleared = 0;
>> struct Scsi_Host *host;
>> struct ufs_hba *hba;
>> + struct ufs_hw_queue *hwq;
>> + struct ufshcd_lrb *lrbp;
>> u32 pos;
>> int err;
>> u8 resp = 0xF, lun;
>> @@ -7317,6 +7362,19 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
>> goto out;
>> }
>>
>> + if (is_mcq_enabled(hba)) {
>> + for (pos = 0; pos < hba->nutrs; pos++) {
>> + lrbp = &hba->lrb[pos];
>> + if (lrbp->cmd && lrbp->lun == lun) {
>> + ufshcd_clear_cmds(hba, 1UL << pos);
>> + hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(lrbp->cmd));
>> + ufshcd_mcq_poll_cqe_lock(hba, hwq);
>> + }
>> + }
>> + err = 0;
>> + goto out;
>> + }
>> +
>> /* clear the commands that were pending for corresponding LUN */
>> spin_lock_irqsave(&hba->outstanding_lock, flags);
>> for_each_set_bit(pos, &hba->outstanding_reqs, hba->nutrs)
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 9+ messages in thread