git: e2b46f35f783 - main - ufshci: tear down the controller in a safe order

The branch main has been updated by jaeyoon:

URL: https://cgit.FreeBSD.org/src/commit/?id=e2b46f35f78321d2faa94a2cf3e71d7fc72f4ba4

commit e2b46f35f78321d2faa94a2cf3e71d7fc72f4ba4
Author:     Jaeyoon Choi <jaeyoon@FreeBSD.org>
AuthorDate: 2026-11-03 04:18:50 +0000
Commit:     Jaeyoon Choi <jaeyoon@FreeBSD.org>
CommitDate: 2026-11-03 04:29:43 +0000

    ufshci: tear down the controller in a safe order
    
    Detach freed the request queues while the interrupt, the watchdog or a
    running reset could still use them. It also left the controller
    running, so a pending interrupt could hit the next attach before its
    queues existed.
    
    Tear down the interrupt, stop the watchdogs, free the task queue,
    disable the controller, and only then destroy the queues. A stopped
    watchdog stays stopped, so a reset still running cannot arm it again.
    A reset requested after the task queue is freed is ignored.
    
    Reviewed by:            imp
    Sponsored by:           Samsung Electronics
    Differential Revision:  https://reviews.freebsd.org/D61115
---
 sys/dev/ufshci/ufshci_ctrlr.c     | 34 ++++++++++++++++++++++++++++------
 sys/dev/ufshci/ufshci_private.h   |  2 ++
 sys/dev/ufshci/ufshci_req_queue.c | 39 ++++++++++++++++++++++++++++++++-------
 3 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/sys/dev/ufshci/ufshci_ctrlr.c b/sys/dev/ufshci/ufshci_ctrlr.c
index cd321f8bdd29..7fb4cbf1bf21 100644
--- a/sys/dev/ufshci/ufshci_ctrlr.c
+++ b/sys/dev/ufshci/ufshci_ctrlr.c
@@ -481,10 +481,10 @@ ufshci_ctrlr_destruct(struct ufshci_controller *ctrlr, device_t dev)
 
 	/* TODO: Flush In-flight IOs */
 
-	/* Release resources */
-	ufshci_utmr_req_queue_destroy(ctrlr);
-	ufshci_utr_req_queue_destroy(ctrlr);
-
+	/*
+	 * Tear the interrupt down first, so the handler cannot touch a
+	 * queue while it is being freed.
+	 */
 	if (ctrlr->tag)
 		bus_teardown_intr(ctrlr->dev, ctrlr->res, ctrlr->tag);
 
@@ -493,14 +493,29 @@ ufshci_ctrlr_destruct(struct ufshci_controller *ctrlr, device_t dev)
 		    rman_get_rid(ctrlr->res), ctrlr->res);
 
 	/*
-	 * The interrupt and the timers are gone, so nothing enqueues new
-	 * tasks. Free the taskqueue before the SIM teardown below.
+	 * A watchdog escalates a timeout to a reset on the taskqueue, so
+	 * stop the watchdogs before the taskqueue goes away.
+	 */
+	ufshci_req_queue_stop_watchdog(&ctrlr->task_mgmt_req_queue);
+	ufshci_req_queue_stop_watchdog(&ctrlr->transfer_req_queue);
+
+	/*
+	 * Free the taskqueue next, which waits for a reset already
+	 * running. Only then destroy the queues, because that running
+	 * reset walks them.
 	 */
 	if (ctrlr->taskqueue != NULL) {
 		taskqueue_free(ctrlr->taskqueue);
 		ctrlr->taskqueue = NULL;
 	}
 
+	/* Quiet the hardware, so no DMA or interrupt outlives the queues. */
+	ufshci_ctrlr_disable(ctrlr);
+
+	/* Release resources */
+	ufshci_utmr_req_queue_destroy(ctrlr);
+	ufshci_utr_req_queue_destroy(ctrlr);
+
 	ufshci_sim_release_wlun_periph(ctrlr);
 
 	mtx_lock(&ctrlr->sc_mtx);
@@ -526,6 +541,13 @@ nores:
 void
 ufshci_ctrlr_reset(struct ufshci_controller *ctrlr)
 {
+	/*
+	 * Detach frees the taskqueue before it tears the rest down, so a
+	 * request that arrives after that has nowhere to go.
+	 */
+	if (ctrlr->taskqueue == NULL)
+		return;
+
 	taskqueue_enqueue(ctrlr->taskqueue, &ctrlr->reset_task);
 }
 
diff --git a/sys/dev/ufshci/ufshci_private.h b/sys/dev/ufshci/ufshci_private.h
index b268e337976e..d38e7c66ff46 100644
--- a/sys/dev/ufshci/ufshci_private.h
+++ b/sys/dev/ufshci/ufshci_private.h
@@ -161,6 +161,7 @@ struct ufshci_hw_queue {
 
 	struct callout timer;		     /* recovery lock */
 	bool timer_armed;		     /* recovery lock */
+	bool watchdog_stopped;		     /* qlock */
 	enum ufshci_recovery recovery_state; /* recovery lock */
 
 	union {
@@ -511,6 +512,7 @@ void ufshci_utmr_req_queue_disable(struct ufshci_controller *ctrlr);
 int ufshci_utmr_req_queue_enable(struct ufshci_controller *ctrlr);
 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_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 ee1ce97e6942..c5a874018d0b 100644
--- a/sys/dev/ufshci/ufshci_req_queue.c
+++ b/sys/dev/ufshci/ufshci_req_queue.c
@@ -138,6 +138,32 @@ ufshci_utr_req_queue_enable(struct ufshci_controller *ctrlr)
 	    &ctrlr->transfer_req_queue));
 }
 
+/*
+ * Stop the watchdog of the hardware queue. The trackers stay in
+ * place, so a reset that is still running can finish with them.
+ */
+void
+ufshci_req_queue_stop_watchdog(struct ufshci_req_queue *req_queue)
+{
+	struct ufshci_hw_queue *hwq;
+
+	/* Attach may fail before the queue is set up. */
+	if (req_queue->hwq == NULL)
+		return;
+
+	hwq = req_queue->qops.get_hw_queue(req_queue);
+
+	/* Submits arm the timer under qlock. None can after this. */
+	mtx_lock(&hwq->qlock);
+	hwq->watchdog_stopped = true;
+	mtx_unlock(&hwq->qlock);
+
+	mtx_lock(&hwq->recovery_lock);
+	hwq->timer_armed = false;
+	mtx_unlock(&hwq->recovery_lock);
+	callout_drain(&hwq->timer);
+}
+
 static bool
 ufshci_req_queue_response_is_error(struct ufshci_req_queue *req_queue,
     uint8_t ocs, union ufshci_reponse_upiu *response)
@@ -550,15 +576,14 @@ ufshci_req_queue_timeout(void *arg)
 	}
 
 	/*
-	 * Shutdown condition: We set hwq->timer_armed to false in
-	 * ufshci_req_sdb_destroy before calling callout_drain. When we call
-	 * that, this routine might get called one last time. Exit w/o setting a
-	 * timeout. None of the watchdog stuff needs to be done since we're
-	 * destroying the hwq.
+	 * Shutdown condition: timer_armed is cleared before callout_drain
+	 * when the watchdog stops and when the hwq is destroyed. This
+	 * routine might get called one last time then. Exit w/o setting a
+	 * timeout.
 	 */
 	if (!hwq->timer_armed) {
 		ufshci_printf(ctrlr,
-		    "Timeout fired during ufshci_utr_req_queue_destroy\n");
+		    "Timeout fired while the watchdog stopped\n");
 		return;
 	}
 
@@ -722,7 +747,7 @@ ufshci_req_queue_submit_tracker(struct ufshci_req_queue *req_queue,
 	else
 		timeout = ctrlr->timeout_period;
 	tr->deadline = getsbinuptime() + timeout * SBT_1S;
-	if (!hwq->timer_armed) {
+	if (!hwq->timer_armed && !hwq->watchdog_stopped) {
 		hwq->timer_armed = true;
 		/*
 		 * It wakes up once every 0.5 seconds to check if the deadline