git: 6e6f63c3807d - main - ufshci: hold back I/O during a reset
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 03 Nov 2026 04:33:48 UTC
The branch main has been updated by jaeyoon:
URL: https://cgit.FreeBSD.org/src/commit/?id=6e6f63c3807dd5557bb8f4621b262ad81241c1b2
commit 6e6f63c3807dd5557bb8f4621b262ad81241c1b2
Author: Jaeyoon Choi <jaeyoon@FreeBSD.org>
AuthorDate: 2026-11-03 04:23:35 +0000
Commit: Jaeyoon Choi <jaeyoon@FreeBSD.org>
CommitDate: 2026-11-03 04:29:44 +0000
ufshci: hold back I/O during a reset
The disable path only claims what is in flight at that moment. An I/O
submitted after a reset was requested rang a doorbell that the reset
then wiped, so it was lost. An I/O sent as soon as the queue was back
reached the device before the bring-up had initialized it, and the
device answered NOT READY.
Keep such an I/O on a list instead, without a slot, from the moment a
reset is requested until the bring-up has finished, and send the list
in order then. A request that a reset returned to CAM comes back while
the list is still held, so it waits there too. A completion sends the
next one when the slots have run out. If the controller fails first,
the failure path completes the list with an error. Admin and task
management requests still go out, because the reset task waits on its
own commands and the watchdog sends the aborts. This is how nvme(4)
holds requests while a queue is in recovery.
Reviewed by: imp
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D61118
---
sys/dev/ufshci/ufshci_ctrlr.c | 12 +++++
sys/dev/ufshci/ufshci_private.h | 10 +++++
sys/dev/ufshci/ufshci_req_queue.c | 95 ++++++++++++++++++++++++++++++++++++++-
sys/dev/ufshci/ufshci_req_sdb.c | 1 +
4 files changed, 117 insertions(+), 1 deletion(-)
diff --git a/sys/dev/ufshci/ufshci_ctrlr.c b/sys/dev/ufshci/ufshci_ctrlr.c
index 7fb4cbf1bf21..8b2471fd0558 100644
--- a/sys/dev/ufshci/ufshci_ctrlr.c
+++ b/sys/dev/ufshci/ufshci_ctrlr.c
@@ -167,6 +167,15 @@ ufshci_ctrlr_start(struct ufshci_controller *ctrlr, bool resetting)
/* TODO: Configure Background Operations */
+ /*
+ * The device is ready for I/O now, so let the held I/O go out. The
+ * steps below need I/O to flow: a bus scan and the lookup of the
+ * well-known LUN that it creates.
+ */
+ ctrlr->io_ready = true;
+ ufshci_req_queue_resume(&ctrlr->task_mgmt_req_queue);
+ ufshci_req_queue_resume(&ctrlr->transfer_req_queue);
+
/*
* A reset normally arrives after the SIM is attached. But if the
* first start attempt failed early, the reset path runs without a
@@ -359,6 +368,7 @@ ufshci_ctrlr_construct(struct ufshci_controller *ctrlr, device_t dev)
int error;
ctrlr->device_init_timeout_in_ms = UFSHCI_DEVICE_INIT_TIMEOUT_MS;
+ ctrlr->io_ready = false;
ctrlr->uic_cmd_timeout_in_ms = UFSHCI_UIC_CMD_TIMEOUT_MS;
ctrlr->dev = dev;
ctrlr->sc_unit = device_get_unit(dev);
@@ -548,6 +558,8 @@ ufshci_ctrlr_reset(struct ufshci_controller *ctrlr)
if (ctrlr->taskqueue == NULL)
return;
+ /* I/O waits from here until the reset has brought the device up. */
+ ctrlr->io_ready = false;
taskqueue_enqueue(ctrlr->taskqueue, &ctrlr->reset_task);
}
diff --git a/sys/dev/ufshci/ufshci_private.h b/sys/dev/ufshci/ufshci_private.h
index 1a2179a30299..5b3360c48198 100644
--- a/sys/dev/ufshci/ufshci_private.h
+++ b/sys/dev/ufshci/ufshci_private.h
@@ -180,6 +180,8 @@ struct ufshci_hw_queue {
TAILQ_HEAD(, ufshci_tracker) free_tr;
TAILQ_HEAD(, ufshci_tracker) outstanding_tr;
+ /* I/O held back while the queue is in recovery (qlock). */
+ STAILQ_HEAD(, ufshci_request) queued_req;
/*
* A Request List using the single doorbell method uses a dedicated
@@ -412,6 +414,12 @@ struct ufshci_controller {
uint32_t max_rx_lanes;
uint32_t is_failed;
+ /*
+ * The bring-up finished, so I/O may go to the device. Cleared when
+ * a reset is requested. Read under qlock, written without it, so a
+ * stale read costs at most one doorbell that the disable path claims.
+ */
+ bool io_ready;
/* Debug aids: requests left to drop, per I/O and admin. */
uint32_t debug_drop_ios;
@@ -514,6 +522,8 @@ void ufshci_utr_req_queue_disable(struct ufshci_controller *ctrlr);
int ufshci_utr_req_queue_enable(struct ufshci_controller *ctrlr);
void ufshci_req_queue_stop_watchdog(struct ufshci_req_queue *req_queue);
void ufshci_req_queue_complete_aborted_hwq(struct ufshci_hw_queue *hwq);
+void ufshci_req_queue_resume(struct ufshci_req_queue *req_queue);
+void ufshci_req_queue_submit_queued(struct ufshci_hw_queue *hwq);
void ufshci_req_queue_fail(struct ufshci_controller *ctrlr,
struct ufshci_req_queue *req_queue);
int ufshci_req_queue_submit_request(struct ufshci_req_queue *req_queue,
diff --git a/sys/dev/ufshci/ufshci_req_queue.c b/sys/dev/ufshci/ufshci_req_queue.c
index 1c7ae5f54f48..868ff0b2c1ca 100644
--- a/sys/dev/ufshci/ufshci_req_queue.c
+++ b/sys/dev/ufshci/ufshci_req_queue.c
@@ -138,6 +138,17 @@ ufshci_utr_req_queue_enable(struct ufshci_controller *ctrlr)
&ctrlr->transfer_req_queue));
}
+/* Let the held I/O go out, once the controller is ready for I/O. */
+void
+ufshci_req_queue_resume(struct ufshci_req_queue *req_queue)
+{
+ struct ufshci_hw_queue *hwq = req_queue->qops.get_hw_queue(req_queue);
+
+ mtx_lock(&hwq->qlock);
+ ufshci_req_queue_submit_queued(hwq);
+ mtx_unlock(&hwq->qlock);
+}
+
/*
* Stop the watchdog of the hardware queue. The trackers stay in
* place, so a reset that is still running can finish with them.
@@ -220,6 +231,28 @@ ufshci_req_queue_manual_complete_tracker(struct ufshci_tracker *tr, uint8_t ocs,
ufshci_req_queue_complete_tracker(tr);
}
+/*
+ * Complete a request that never got a tracker, so there is no
+ * descriptor to write the response into. The caller must not hold
+ * the queue lock, because the callback may submit a request.
+ */
+static void
+ufshci_req_queue_manual_complete_request(struct ufshci_request *req,
+ uint8_t ocs, uint8_t rc)
+{
+ struct ufshci_completion cpl;
+
+ memset(&cpl, 0, sizeof(cpl));
+ cpl.size = req->response_size;
+ cpl.response_upiu.header.response = rc;
+ cpl.ocs = ocs;
+
+ if (req->cb_fn)
+ req->cb_fn(req->cb_arg, &cpl, true);
+
+ ufshci_free_request(req);
+}
+
/*
* Complete every tracker the reset claimed. The claim keeps the
* completion scan away, so no tracker completes twice.
@@ -260,6 +293,7 @@ ufshci_req_queue_fail(struct ufshci_controller *ctrlr,
{
struct ufshci_hw_queue *hwq = req_queue->qops.get_hw_queue(req_queue);
struct ufshci_tracker *tr;
+ struct ufshci_request *req;
int i;
if (!mtx_initialized(&hwq->qlock))
@@ -295,6 +329,16 @@ ufshci_req_queue_fail(struct ufshci_controller *ctrlr,
mtx_lock(&hwq->qlock);
}
+ /* The held I/O never reached the hardware, so no slot to clear. */
+ while ((req = STAILQ_FIRST(&hwq->queued_req)) != NULL) {
+ STAILQ_REMOVE_HEAD(&hwq->queued_req, stailq);
+ mtx_unlock(&hwq->qlock);
+ ufshci_req_queue_manual_complete_request(req,
+ UFSHCI_DESC_ABORTED,
+ UFSHCI_RESPONSE_CODE_GENERAL_FAILURE);
+ mtx_lock(&hwq->qlock);
+ }
+
mtx_unlock(&hwq->qlock);
}
@@ -380,6 +424,10 @@ ufshci_req_queue_complete_tracker(struct ufshci_tracker *tr)
TAILQ_REMOVE(&hwq->outstanding_tr, tr, tailq);
TAILQ_INSERT_HEAD(&hwq->free_tr, tr, tailq);
+
+ /* A slot came free, so the next held I/O can go out. */
+ if (req_queue->ctrlr->io_ready && !req_queue->ctrlr->is_failed)
+ ufshci_req_queue_submit_queued(hwq);
}
mtx_unlock(&tr->hwq->qlock);
@@ -860,14 +908,27 @@ static int
_ufshci_req_queue_submit_request(struct ufshci_req_queue *req_queue,
struct ufshci_request *req)
{
+ struct ufshci_hw_queue *hwq = req_queue->qops.get_hw_queue(req_queue);
struct ufshci_tracker *tr = NULL;
int error;
- mtx_assert(&req_queue->qops.get_hw_queue(req_queue)->qlock, MA_OWNED);
+ mtx_assert(&hwq->qlock, MA_OWNED);
if (req_queue->ctrlr->is_failed)
return (ENXIO);
+ /*
+ * The controller is not ready for I/O: a reset was asked for, or
+ * the bring-up has not finished. Hold the I/O until it is. Admin
+ * and task management requests still go out, because the reset
+ * task waits on its own commands and the watchdog sends the aborts.
+ */
+ if (!req->is_admin && !req_queue->is_task_mgmt &&
+ !req_queue->ctrlr->io_ready) {
+ STAILQ_INSERT_TAIL(&hwq->queued_req, req, stailq);
+ return (0);
+ }
+
error = req_queue->qops.reserve_slot(req_queue, &tr,
req->is_admin);
if (error != 0) {
@@ -896,6 +957,38 @@ _ufshci_req_queue_submit_request(struct ufshci_req_queue *req_queue,
return (0);
}
+/*
+ * Send the held I/O in order, as far as the free slots go. The rest
+ * waits for the next completion.
+ */
+void
+ufshci_req_queue_submit_queued(struct ufshci_hw_queue *hwq)
+{
+ struct ufshci_req_queue *req_queue = hwq->req_queue;
+ struct ufshci_request *req;
+ struct ufshci_tracker *tr;
+
+ mtx_assert(&hwq->qlock, MA_OWNED);
+
+ while ((req = STAILQ_FIRST(&hwq->queued_req)) != NULL) {
+ /*
+ * A reset can start while this runs. The submit would then put
+ * each request back at the tail, and this loop would never end.
+ */
+ if (!req_queue->ctrlr->io_ready || req_queue->ctrlr->is_failed)
+ break;
+ /* Probe for an I/O slot first, so a full queue stays quiet. */
+ if (req_queue->qops.reserve_slot(req_queue, &tr, false) != 0)
+ break;
+ STAILQ_REMOVE_HEAD(&hwq->queued_req, stailq);
+ if (_ufshci_req_queue_submit_request(req_queue, req) != 0) {
+ /* The failure path completes what is left here. */
+ STAILQ_INSERT_HEAD(&hwq->queued_req, req, stailq);
+ break;
+ }
+ }
+}
+
int
ufshci_req_queue_submit_request(struct ufshci_req_queue *req_queue,
struct ufshci_request *req)
diff --git a/sys/dev/ufshci/ufshci_req_sdb.c b/sys/dev/ufshci/ufshci_req_sdb.c
index 34b12c0c0136..7eb7028f3c82 100644
--- a/sys/dev/ufshci/ufshci_req_sdb.c
+++ b/sys/dev/ufshci/ufshci_req_sdb.c
@@ -257,6 +257,7 @@ ufshci_req_sdb_construct(struct ufshci_controller *ctrlr,
TAILQ_INIT(&hwq->free_tr);
TAILQ_INIT(&hwq->outstanding_tr);
+ STAILQ_INIT(&hwq->queued_req);
for (i = 0; i < req_queue->num_trackers; i++) {
tr = malloc_domainset(sizeof(struct ufshci_tracker), M_UFSHCI,