Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 18 Sep 2025 14:42:04 GMT
From:      Jaeyoon Choi <jaeyoon@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 6b841d70960a - main - ufshci: revisit controller reset path and add I/O timeout handling
Message-ID:  <202509181442.58IEg4UQ077832@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by jaeyoon:

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

commit 6b841d70960a3a0ec4e43392683053878c403f9c
Author:     Jaeyoon Choi <jaeyoon@FreeBSD.org>
AuthorDate: 2025-09-18 14:37:08 +0000
Commit:     Jaeyoon Choi <jaeyoon@FreeBSD.org>
CommitDate: 2025-09-18 14:41:47 +0000

    ufshci: revisit controller reset path and add I/O timeout handling
    
    This patch revisits the controller reset path and introduces timeout
    handling for I/O commands.
    
    To support controller reset during driver operation, the controller’s
    construct, destruct, enable, and disable functions are clearly
    separated in ufshci_ctrlr.c. ufshci_ctrlr_hw_reset() function is
    added to leverage enable/disable.
    
    After initialization, ufshci_ctrlr_reset_task() is also introduced to
    ensure controller resets are performed via the task queue.
    
    Timeout handling is designed in five steps. This patch implements
    Step 1 and Step 5, while the remaining steps will be added later.
    The timeout mechanism follows the same shared timeout model used in
    the NVMe driver.
    
    Test: Intentionally delayed UPIU I/O in QEMU to trigger a timeout and
    verify timeout handling.
    
    Sponsored by:           Samsung Electronics
    Reviewed by:            imp (mentor)
    Differential Revision:  https://reviews.freebsd.org/D52440
---
 sys/dev/ufshci/ufshci_ctrlr.c     | 399 +++++++++++++++++++++++---------------
 sys/dev/ufshci/ufshci_ctrlr_cmd.c |   2 +-
 sys/dev/ufshci/ufshci_dev.c       |   1 -
 sys/dev/ufshci/ufshci_pci.c       |   3 +-
 sys/dev/ufshci/ufshci_private.h   |  41 +++-
 sys/dev/ufshci/ufshci_req_queue.c | 292 ++++++++++++++++++++++++++--
 sys/dev/ufshci/ufshci_req_sdb.c   |  77 +++++++-
 sys/dev/ufshci/ufshci_sim.c       |   1 -
 8 files changed, 633 insertions(+), 183 deletions(-)

diff --git a/sys/dev/ufshci/ufshci_ctrlr.c b/sys/dev/ufshci/ufshci_ctrlr.c
index 36be94b8b8b7..35663b480cfa 100644
--- a/sys/dev/ufshci/ufshci_ctrlr.c
+++ b/sys/dev/ufshci/ufshci_ctrlr.c
@@ -12,8 +12,108 @@
 #include "ufshci_private.h"
 #include "ufshci_reg.h"
 
+static void
+ufshci_ctrlr_fail(struct ufshci_controller *ctrlr)
+{
+	ctrlr->is_failed = true;
+
+	ufshci_req_queue_fail(ctrlr,
+	    ctrlr->task_mgmt_req_queue.qops.get_hw_queue(
+		&ctrlr->task_mgmt_req_queue));
+	ufshci_req_queue_fail(ctrlr,
+	    ctrlr->transfer_req_queue.qops.get_hw_queue(
+		&ctrlr->transfer_req_queue));
+}
+
+static void
+ufshci_ctrlr_start(struct ufshci_controller *ctrlr, bool resetting)
+{
+	TSENTER();
+
+	/*
+	 * If `resetting` is true, we are on the reset path.
+	 * Re-enable request queues here because ufshci_ctrlr_reset_task()
+	 * disables them during reset.
+	 */
+	if (resetting) {
+		if (ufshci_utmr_req_queue_enable(ctrlr) != 0) {
+			ufshci_ctrlr_fail(ctrlr);
+			return;
+		}
+		if (ufshci_utr_req_queue_enable(ctrlr) != 0) {
+			ufshci_ctrlr_fail(ctrlr);
+			return;
+		}
+	}
+
+	if (ufshci_ctrlr_send_nop(ctrlr) != 0) {
+		ufshci_ctrlr_fail(ctrlr);
+		return;
+	}
+
+	/* Initialize UFS target drvice */
+	if (ufshci_dev_init(ctrlr) != 0) {
+		ufshci_ctrlr_fail(ctrlr);
+		return;
+	}
+
+	/* Initialize Reference Clock */
+	if (ufshci_dev_init_reference_clock(ctrlr) != 0) {
+		ufshci_ctrlr_fail(ctrlr);
+		return;
+	}
+
+	/* Initialize unipro */
+	if (ufshci_dev_init_unipro(ctrlr) != 0) {
+		ufshci_ctrlr_fail(ctrlr);
+		return;
+	}
+
+	/*
+	 * Initialize UIC Power Mode
+	 * QEMU UFS devices do not support unipro and power mode.
+	 */
+	if (!(ctrlr->quirks & UFSHCI_QUIRK_IGNORE_UIC_POWER_MODE) &&
+	    ufshci_dev_init_uic_power_mode(ctrlr) != 0) {
+		ufshci_ctrlr_fail(ctrlr);
+		return;
+	}
+
+	/* Initialize UFS Power Mode */
+	if (ufshci_dev_init_ufs_power_mode(ctrlr) != 0) {
+		ufshci_ctrlr_fail(ctrlr);
+		return;
+	}
+
+	/* Read Controller Descriptor (Device, Geometry) */
+	if (ufshci_dev_get_descriptor(ctrlr) != 0) {
+		ufshci_ctrlr_fail(ctrlr);
+		return;
+	}
+
+	if (ufshci_dev_config_write_booster(ctrlr)) {
+		ufshci_ctrlr_fail(ctrlr);
+		return;
+	}
+
+	/* TODO: Configure Write Protect */
+
+	/* TODO: Configure Background Operations */
+
+	/*
+	 * If the reset is due to a timeout, it is already attached to the SIM
+	 * and does not need to be attached again.
+	 */
+	if (!resetting && ufshci_sim_attach(ctrlr) != 0) {
+		ufshci_ctrlr_fail(ctrlr);
+		return;
+	}
+
+	TSEXIT();
+}
+
 static int
-ufshci_ctrlr_enable_host_ctrlr(struct ufshci_controller *ctrlr)
+ufshci_ctrlr_disable_host_ctrlr(struct ufshci_controller *ctrlr)
 {
 	int timeout = ticks + MSEC_2_TICKS(ctrlr->device_init_timeout_in_ms);
 	sbintime_t delta_t = SBT_1US;
@@ -27,6 +127,35 @@ ufshci_ctrlr_enable_host_ctrlr(struct ufshci_controller *ctrlr)
 		ufshci_mmio_write_4(ctrlr, hce, hce);
 	}
 
+	/* Wait for the HCE flag to change */
+	while (1) {
+		hce = ufshci_mmio_read_4(ctrlr, hce);
+		if (!UFSHCIV(UFSHCI_HCE_REG_HCE, hce))
+			break;
+		if (timeout - ticks < 0) {
+			ufshci_printf(ctrlr,
+			    "host controller failed to disable "
+			    "within %d ms\n",
+			    ctrlr->device_init_timeout_in_ms);
+			return (ENXIO);
+		}
+
+		pause_sbt("ufshci_disable_hce", delta_t, 0, C_PREL(1));
+		delta_t = min(SBT_1MS, delta_t * 3 / 2);
+	}
+
+	return (0);
+}
+
+static int
+ufshci_ctrlr_enable_host_ctrlr(struct ufshci_controller *ctrlr)
+{
+	int timeout = ticks + MSEC_2_TICKS(ctrlr->device_init_timeout_in_ms);
+	sbintime_t delta_t = SBT_1US;
+	uint32_t hce;
+
+	hce = ufshci_mmio_read_4(ctrlr, hce);
+
 	/* Enable UFS host controller */
 	hce |= UFSHCIM(UFSHCI_HCE_REG_HCE);
 	ufshci_mmio_write_4(ctrlr, hce, hce);
@@ -36,7 +165,7 @@ ufshci_ctrlr_enable_host_ctrlr(struct ufshci_controller *ctrlr)
 	 * unstable, so we need to read the HCE value after some time after
 	 * initialization is complete.
 	 */
-	pause_sbt("ufshci_hce", ustosbt(100), 0, C_PREL(1));
+	pause_sbt("ufshci_enable_hce", ustosbt(100), 0, C_PREL(1));
 
 	/* Wait for the HCE flag to change */
 	while (1) {
@@ -51,17 +180,103 @@ ufshci_ctrlr_enable_host_ctrlr(struct ufshci_controller *ctrlr)
 			return (ENXIO);
 		}
 
-		pause_sbt("ufshci_hce", delta_t, 0, C_PREL(1));
+		pause_sbt("ufshci_enable_hce", delta_t, 0, C_PREL(1));
 		delta_t = min(SBT_1MS, delta_t * 3 / 2);
 	}
 
 	return (0);
 }
 
+static int
+ufshci_ctrlr_disable(struct ufshci_controller *ctrlr)
+{
+	int error;
+
+	/* Disable all interrupts */
+	ufshci_mmio_write_4(ctrlr, ie, 0);
+
+	error = ufshci_ctrlr_disable_host_ctrlr(ctrlr);
+	return (error);
+}
+
+static int
+ufshci_ctrlr_enable(struct ufshci_controller *ctrlr)
+{
+	uint32_t ie, hcs;
+	int error;
+
+	error = ufshci_ctrlr_enable_host_ctrlr(ctrlr);
+	if (error)
+		return (error);
+
+	/* Send DME_LINKSTARTUP command to start the link startup procedure */
+	error = ufshci_uic_send_dme_link_startup(ctrlr);
+	if (error)
+		return (error);
+
+	/*
+	 * The device_present(UFSHCI_HCS_REG_DP) bit becomes true if the host
+	 * controller has successfully received a Link Startup UIC command
+	 * response and the UFS device has found a physical link to the
+	 * controller.
+	 */
+	hcs = ufshci_mmio_read_4(ctrlr, hcs);
+	if (!UFSHCIV(UFSHCI_HCS_REG_DP, hcs)) {
+		ufshci_printf(ctrlr, "UFS device not found\n");
+		return (ENXIO);
+	}
+
+	/* Enable additional interrupts by programming the IE register. */
+	ie = ufshci_mmio_read_4(ctrlr, ie);
+	ie |= UFSHCIM(UFSHCI_IE_REG_UTRCE);  /* UTR Completion */
+	ie |= UFSHCIM(UFSHCI_IE_REG_UEE);    /* UIC Error */
+	ie |= UFSHCIM(UFSHCI_IE_REG_UTMRCE); /* UTMR Completion */
+	ie |= UFSHCIM(UFSHCI_IE_REG_DFEE);   /* Device Fatal Error */
+	ie |= UFSHCIM(UFSHCI_IE_REG_UTPEE);  /* UTP Error */
+	ie |= UFSHCIM(UFSHCI_IE_REG_HCFEE);  /* Host Ctrlr Fatal Error */
+	ie |= UFSHCIM(UFSHCI_IE_REG_SBFEE);  /* System Bus Fatal Error */
+	ie |= UFSHCIM(UFSHCI_IE_REG_CEFEE);  /* Crypto Engine Fatal Error */
+	ufshci_mmio_write_4(ctrlr, ie, ie);
+
+	/* TODO: Initialize interrupt Aggregation Control Register (UTRIACR) */
+
+	return (0);
+}
+
+static int
+ufshci_ctrlr_hw_reset(struct ufshci_controller *ctrlr)
+{
+	int error;
+
+	error = ufshci_ctrlr_disable(ctrlr);
+	if (error)
+		return (error);
+
+	error = ufshci_ctrlr_enable(ctrlr);
+	return (error);
+}
+
+static void
+ufshci_ctrlr_reset_task(void *arg, int pending)
+{
+	struct ufshci_controller *ctrlr = arg;
+	int error;
+
+	/* Release resources */
+	ufshci_utmr_req_queue_disable(ctrlr);
+	ufshci_utr_req_queue_disable(ctrlr);
+
+	error = ufshci_ctrlr_hw_reset(ctrlr);
+	if (error)
+		return (ufshci_ctrlr_fail(ctrlr));
+
+	ufshci_ctrlr_start(ctrlr, true);
+}
+
 int
 ufshci_ctrlr_construct(struct ufshci_controller *ctrlr, device_t dev)
 {
-	uint32_t ver, cap, hcs, ie, ahit;
+	uint32_t ver, cap, ahit;
 	uint32_t timeout_period, retry_count;
 	int error;
 
@@ -114,16 +329,15 @@ ufshci_ctrlr_construct(struct ufshci_controller *ctrlr, device_t dev)
 	TUNABLE_INT_FETCH("hw.ufshci.retry_count", &retry_count);
 	ctrlr->retry_count = retry_count;
 
-	/* Disable all interrupts */
-	ufshci_mmio_write_4(ctrlr, ie, 0);
-
-	/* Enable Host Controller */
-	error = ufshci_ctrlr_enable_host_ctrlr(ctrlr);
-	if (error)
-		return (error);
+	ctrlr->enable_aborts = 1;
+	if (ctrlr->quirks & UFSHCI_QUIRK_NOT_SUPPORT_ABORT_TASK)
+		ctrlr->enable_aborts = 0;
+	else
+		TUNABLE_INT_FETCH("hw.ufshci.enable_aborts",
+		    &ctrlr->enable_aborts);
 
-	/* Send DME_LINKSTARTUP command to start the link startup procedure */
-	error = ufshci_uic_send_dme_link_startup(ctrlr);
+	/* Reset the UFSHCI controller */
+	error = ufshci_ctrlr_hw_reset(ctrlr);
 	if (error)
 		return (error);
 
@@ -134,18 +348,6 @@ ufshci_ctrlr_construct(struct ufshci_controller *ctrlr, device_t dev)
 	ahit = 0;
 	ufshci_mmio_write_4(ctrlr, ahit, ahit);
 
-	/*
-	 * The device_present(UFSHCI_HCS_REG_DP) bit becomes true if the host
-	 * controller has successfully received a Link Startup UIC command
-	 * response and the UFS device has found a physical link to the
-	 * controller.
-	 */
-	hcs = ufshci_mmio_read_4(ctrlr, hcs);
-	if (!UFSHCIV(UFSHCI_HCS_REG_DP, hcs)) {
-		ufshci_printf(ctrlr, "UFS device not found\n");
-		return (ENXIO);
-	}
-
 	/* Allocate and initialize UTP Task Management Request List. */
 	error = ufshci_utmr_req_queue_construct(ctrlr);
 	if (error)
@@ -156,27 +358,21 @@ ufshci_ctrlr_construct(struct ufshci_controller *ctrlr, device_t dev)
 	if (error)
 		return (error);
 
-	/* Enable additional interrupts by programming the IE register. */
-	ie = ufshci_mmio_read_4(ctrlr, ie);
-	ie |= UFSHCIM(UFSHCI_IE_REG_UTRCE);  /* UTR Completion */
-	ie |= UFSHCIM(UFSHCI_IE_REG_UEE);    /* UIC Error */
-	ie |= UFSHCIM(UFSHCI_IE_REG_UTMRCE); /* UTMR Completion */
-	ie |= UFSHCIM(UFSHCI_IE_REG_DFEE);   /* Device Fatal Error */
-	ie |= UFSHCIM(UFSHCI_IE_REG_UTPEE);  /* UTP Error */
-	ie |= UFSHCIM(UFSHCI_IE_REG_HCFEE);  /* Host Ctrlr Fatal Error */
-	ie |= UFSHCIM(UFSHCI_IE_REG_SBFEE);  /* System Bus Fatal Error */
-	ie |= UFSHCIM(UFSHCI_IE_REG_CEFEE);  /* Crypto Engine Fatal Error */
-	ufshci_mmio_write_4(ctrlr, ie, ie);
-
-	/* TODO: Initialize interrupt Aggregation Control Register (UTRIACR) */
-
 	/* TODO: Separate IO and Admin slot */
+
 	/*
 	 * max_hw_pend_io is the number of slots in the transfer_req_queue.
 	 * Reduce num_entries by one to reserve an admin slot.
 	 */
 	ctrlr->max_hw_pend_io = ctrlr->transfer_req_queue.num_entries - 1;
 
+	/* Create a thread for the taskqueue. */
+	ctrlr->taskqueue = taskqueue_create("ufshci_taskq", M_WAITOK,
+	    taskqueue_thread_enqueue, &ctrlr->taskqueue);
+	taskqueue_start_threads(&ctrlr->taskqueue, 1, PI_DISK, "ufshci taskq");
+
+	TASK_INIT(&ctrlr->reset_task, 0, ufshci_ctrlr_reset_task, ctrlr);
+
 	return (0);
 }
 
@@ -208,50 +404,21 @@ ufshci_ctrlr_destruct(struct ufshci_controller *ctrlr, device_t dev)
 	bus_release_resource(dev, SYS_RES_MEMORY, ctrlr->resource_id,
 	    ctrlr->resource);
 nores:
+	KASSERT(!mtx_owned(&ctrlr->uic_cmd_lock),
+	    ("destroying uic_cmd_lock while still owned"));
 	mtx_destroy(&ctrlr->uic_cmd_lock);
+
+	KASSERT(!mtx_owned(&ctrlr->sc_mtx),
+	    ("destroying sc_mtx while still owned"));
 	mtx_destroy(&ctrlr->sc_mtx);
 
 	return;
 }
 
-int
+void
 ufshci_ctrlr_reset(struct ufshci_controller *ctrlr)
 {
-	uint32_t ie;
-	int error;
-
-	/* Backup and disable all interrupts */
-	ie = ufshci_mmio_read_4(ctrlr, ie);
-	ufshci_mmio_write_4(ctrlr, ie, 0);
-
-	/* Release resources */
-	ufshci_utmr_req_queue_destroy(ctrlr);
-	ufshci_utr_req_queue_destroy(ctrlr);
-
-	/* Reset Host Controller */
-	error = ufshci_ctrlr_enable_host_ctrlr(ctrlr);
-	if (error)
-		return (error);
-
-	/* Send DME_LINKSTARTUP command to start the link startup procedure */
-	error = ufshci_uic_send_dme_link_startup(ctrlr);
-	if (error)
-		return (error);
-
-	/* Enable interrupts */
-	ufshci_mmio_write_4(ctrlr, ie, ie);
-
-	/* Allocate and initialize UTP Task Management Request List. */
-	error = ufshci_utmr_req_queue_construct(ctrlr);
-	if (error)
-		return (error);
-
-	/* Allocate and initialize UTP Transfer Request List or SQ/CQ. */
-	error = ufshci_utr_req_queue_construct(ctrlr);
-	if (error)
-		return (error);
-
-	return (0);
+	taskqueue_enqueue(ctrlr->taskqueue, &ctrlr->reset_task);
 }
 
 int
@@ -295,84 +462,6 @@ ufshci_ctrlr_send_nop(struct ufshci_controller *ctrlr)
 	return (0);
 }
 
-static void
-ufshci_ctrlr_fail(struct ufshci_controller *ctrlr, bool admin_also)
-{
-	printf("ufshci(4): ufshci_ctrlr_fail\n");
-
-	ctrlr->is_failed = true;
-
-	/* TODO: task_mgmt_req_queue should be handled as fail */
-
-	ufshci_req_queue_fail(ctrlr,
-	    &ctrlr->transfer_req_queue.hwq[UFSHCI_SDB_Q]);
-}
-
-static void
-ufshci_ctrlr_start(struct ufshci_controller *ctrlr)
-{
-	TSENTER();
-
-	if (ufshci_ctrlr_send_nop(ctrlr) != 0) {
-		ufshci_ctrlr_fail(ctrlr, false);
-		return;
-	}
-
-	/* Initialize UFS target drvice */
-	if (ufshci_dev_init(ctrlr) != 0) {
-		ufshci_ctrlr_fail(ctrlr, false);
-		return;
-	}
-
-	/* Initialize Reference Clock */
-	if (ufshci_dev_init_reference_clock(ctrlr) != 0) {
-		ufshci_ctrlr_fail(ctrlr, false);
-		return;
-	}
-
-	/* Initialize unipro */
-	if (ufshci_dev_init_unipro(ctrlr) != 0) {
-		ufshci_ctrlr_fail(ctrlr, false);
-		return;
-	}
-
-	/*
-	 * Initialize UIC Power Mode
-	 * QEMU UFS devices do not support unipro and power mode.
-	 */
-	if (!(ctrlr->quirks & UFSHCI_QUIRK_IGNORE_UIC_POWER_MODE) &&
-	    ufshci_dev_init_uic_power_mode(ctrlr) != 0) {
-		ufshci_ctrlr_fail(ctrlr, false);
-		return;
-	}
-
-	/* Initialize UFS Power Mode */
-	if (ufshci_dev_init_ufs_power_mode(ctrlr) != 0) {
-		ufshci_ctrlr_fail(ctrlr, false);
-		return;
-	}
-
-	/* Read Controller Descriptor (Device, Geometry) */
-	if (ufshci_dev_get_descriptor(ctrlr) != 0) {
-		ufshci_ctrlr_fail(ctrlr, false);
-		return;
-	}
-
-	if (ufshci_dev_config_write_booster(ctrlr)) {
-		ufshci_ctrlr_fail(ctrlr, false);
-		return;
-	}
-
-	/* TODO: Configure Background Operations */
-
-	if (ufshci_sim_attach(ctrlr) != 0) {
-		ufshci_ctrlr_fail(ctrlr, false);
-		return;
-	}
-
-	TSEXIT();
-}
-
 void
 ufshci_ctrlr_start_config_hook(void *arg)
 {
@@ -382,9 +471,9 @@ ufshci_ctrlr_start_config_hook(void *arg)
 
 	if (ufshci_utmr_req_queue_enable(ctrlr) == 0 &&
 	    ufshci_utr_req_queue_enable(ctrlr) == 0)
-		ufshci_ctrlr_start(ctrlr);
+		ufshci_ctrlr_start(ctrlr, false);
 	else
-		ufshci_ctrlr_fail(ctrlr, false);
+		ufshci_ctrlr_fail(ctrlr);
 
 	ufshci_sysctl_initialize_ctrlr(ctrlr);
 	config_intrhook_disestablish(&ctrlr->config_hook);
diff --git a/sys/dev/ufshci/ufshci_ctrlr_cmd.c b/sys/dev/ufshci/ufshci_ctrlr_cmd.c
index 71d163d998af..253f31a93c2e 100644
--- a/sys/dev/ufshci/ufshci_ctrlr_cmd.c
+++ b/sys/dev/ufshci/ufshci_ctrlr_cmd.c
@@ -15,7 +15,7 @@ ufshci_ctrlr_cmd_send_task_mgmt_request(struct ufshci_controller *ctrlr,
 	struct ufshci_request *req;
 	struct ufshci_task_mgmt_request_upiu *upiu;
 
-	req = ufshci_allocate_request_vaddr(NULL, 0, M_WAITOK, cb_fn, cb_arg);
+	req = ufshci_allocate_request_vaddr(NULL, 0, M_NOWAIT, cb_fn, cb_arg);
 
 	req->request_size = sizeof(struct ufshci_task_mgmt_request_upiu);
 	req->response_size = sizeof(struct ufshci_task_mgmt_response_upiu);
diff --git a/sys/dev/ufshci/ufshci_dev.c b/sys/dev/ufshci/ufshci_dev.c
index dd196b1d638b..975468e5156f 100644
--- a/sys/dev/ufshci/ufshci_dev.c
+++ b/sys/dev/ufshci/ufshci_dev.c
@@ -774,4 +774,3 @@ out:
 	ufshci_dev_disable_write_booster(ctrlr);
 	return (error);
 }
-
diff --git a/sys/dev/ufshci/ufshci_pci.c b/sys/dev/ufshci/ufshci_pci.c
index d64b7526f713..992026fd4f4d 100644
--- a/sys/dev/ufshci/ufshci_pci.c
+++ b/sys/dev/ufshci/ufshci_pci.c
@@ -49,7 +49,8 @@ static struct _pcsid {
 	uint32_t ref_clk;
 	uint32_t quirks;
 } pci_ids[] = { { 0x131b36, "QEMU UFS Host Controller", UFSHCI_REF_CLK_19_2MHz,
-		    UFSHCI_QUIRK_IGNORE_UIC_POWER_MODE },
+		    UFSHCI_QUIRK_IGNORE_UIC_POWER_MODE |
+			UFSHCI_QUIRK_NOT_SUPPORT_ABORT_TASK },
 	{ 0x98fa8086, "Intel Lakefield UFS Host Controller",
 	    UFSHCI_REF_CLK_19_2MHz,
 	    UFSHCI_QUIRK_LONG_PEER_PA_TACTIVATE |
diff --git a/sys/dev/ufshci/ufshci_private.h b/sys/dev/ufshci/ufshci_private.h
index 2e033f84c373..ec388c06e248 100644
--- a/sys/dev/ufshci/ufshci_private.h
+++ b/sys/dev/ufshci/ufshci_private.h
@@ -68,7 +68,6 @@ struct ufshci_request {
 	bool is_admin;
 	int32_t retries;
 	bool payload_valid;
-	bool timeout;
 	bool spare[2]; /* Future use */
 	STAILQ_ENTRY(ufshci_request) stailq;
 };
@@ -82,6 +81,7 @@ enum ufshci_slot_state {
 };
 
 struct ufshci_tracker {
+	TAILQ_ENTRY(ufshci_tracker) tailq;
 	struct ufshci_request *req;
 	struct ufshci_req_queue *req_queue;
 	struct ufshci_hw_queue *hwq;
@@ -121,6 +121,8 @@ struct ufshci_qops {
 	    struct ufshci_req_queue *req_queue);
 	int (*enable)(struct ufshci_controller *ctrlr,
 	    struct ufshci_req_queue *req_queue);
+	void (*disable)(struct ufshci_controller *ctrlr,
+	    struct ufshci_req_queue *req_queue);
 	int (*reserve_slot)(struct ufshci_req_queue *req_queue,
 	    struct ufshci_tracker **tr);
 	int (*reserve_admin_slot)(struct ufshci_req_queue *req_queue,
@@ -137,16 +139,27 @@ struct ufshci_qops {
 
 #define UFSHCI_SDB_Q 0 /* Queue number for a single doorbell queue */
 
+enum ufshci_recovery {
+	RECOVERY_NONE = 0, /* Normal operations */
+	RECOVERY_WAITING,  /* waiting for the reset to complete */
+};
+
 /*
  * Generic queue container used by both SDB (fixed 32-slot bitmap) and MCQ
  * (ring buffer) modes. Fields are shared; some such as sq_head, sq_tail and
  * cq_head are not used in SDB but used in MCQ.
  */
 struct ufshci_hw_queue {
+	struct ufshci_controller *ctrlr;
+	struct ufshci_req_queue *req_queue;
 	uint32_t id;
 	int domain;
 	int cpu;
 
+	struct callout timer;		     /* recovery lock */
+	bool timer_armed;		     /* recovery lock */
+	enum ufshci_recovery recovery_state; /* recovery lock */
+
 	union {
 		struct ufshci_utp_xfer_req_desc *utrd;
 		struct ufshci_utp_task_mgmt_req_desc *utmrd;
@@ -161,6 +174,9 @@ struct ufshci_hw_queue {
 	uint32_t num_entries;
 	uint32_t num_trackers;
 
+	TAILQ_HEAD(, ufshci_tracker) free_tr;
+	TAILQ_HEAD(, ufshci_tracker) outstanding_tr;
+
 	/*
 	 * A Request List using the single doorbell method uses a dedicated
 	 * ufshci_tracker, one per slot.
@@ -177,7 +193,13 @@ struct ufshci_hw_queue {
 	int64_t num_retries;
 	int64_t num_failures;
 
+	/*
+	 * Each lock may be acquired independently.
+	 * When both are required, acquire them in this order to avoid
+	 * deadlocks. (recovery_lock -> qlock)
+	 */
 	struct mtx_padalign qlock;
+	struct mtx_padalign recovery_lock;
 };
 
 struct ufshci_req_queue {
@@ -242,6 +264,9 @@ struct ufshci_controller {
 	4 /* Need to wait 1250us after power mode change */
 #define UFSHCI_QUIRK_CHANGE_LANE_AND_GEAR_SEPARATELY \
 	8 /* Need to change the number of lanes before changing HS-GEAR. */
+#define UFSHCI_QUIRK_NOT_SUPPORT_ABORT_TASK \
+	16 /* QEMU does not support Task Management Request */
+
 	uint32_t ref_clk;
 
 	struct cam_sim *ufshci_sim;
@@ -264,6 +289,9 @@ struct ufshci_controller {
 	/* Fields for tracking progress during controller initialization. */
 	struct intr_config_hook config_hook;
 
+	struct task reset_task;
+	struct taskqueue *taskqueue;
+
 	/* For shared legacy interrupt. */
 	int rid;
 	struct resource *res;
@@ -272,6 +300,8 @@ struct ufshci_controller {
 	uint32_t major_version;
 	uint32_t minor_version;
 
+	uint32_t enable_aborts;
+
 	uint32_t num_io_queues;
 	uint32_t max_hw_pend_io;
 
@@ -345,7 +375,7 @@ void ufshci_sim_detach(struct ufshci_controller *ctrlr);
 /* Controller */
 int ufshci_ctrlr_construct(struct ufshci_controller *ctrlr, device_t dev);
 void ufshci_ctrlr_destruct(struct ufshci_controller *ctrlr, device_t dev);
-int ufshci_ctrlr_reset(struct ufshci_controller *ctrlr);
+void ufshci_ctrlr_reset(struct ufshci_controller *ctrlr);
 /* ctrlr defined as void * to allow use with config_intrhook. */
 void ufshci_ctrlr_start_config_hook(void *arg);
 void ufshci_ctrlr_poll(struct ufshci_controller *ctrlr);
@@ -388,7 +418,9 @@ int ufshci_utmr_req_queue_construct(struct ufshci_controller *ctrlr);
 int ufshci_utr_req_queue_construct(struct ufshci_controller *ctrlr);
 void ufshci_utmr_req_queue_destroy(struct ufshci_controller *ctrlr);
 void ufshci_utr_req_queue_destroy(struct ufshci_controller *ctrlr);
+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_fail(struct ufshci_controller *ctrlr,
     struct ufshci_hw_queue *hwq);
@@ -404,6 +436,8 @@ void ufshci_req_sdb_destroy(struct ufshci_controller *ctrlr,
     struct ufshci_req_queue *req_queue);
 struct ufshci_hw_queue *ufshci_req_sdb_get_hw_queue(
     struct ufshci_req_queue *req_queue);
+void ufshci_req_sdb_disable(struct ufshci_controller *ctrlr,
+    struct ufshci_req_queue *req_queue);
 int ufshci_req_sdb_enable(struct ufshci_controller *ctrlr,
     struct ufshci_req_queue *req_queue);
 int ufshci_req_sdb_reserve_slot(struct ufshci_req_queue *req_queue,
@@ -489,13 +523,12 @@ _ufshci_allocate_request(const int how, ufshci_cb_fn_t cb_fn, void *cb_arg)
 	struct ufshci_request *req;
 
 	KASSERT(how == M_WAITOK || how == M_NOWAIT,
-	    ("nvme_allocate_request: invalid how %d", how));
+	    ("ufshci_allocate_request: invalid how %d", how));
 
 	req = malloc(sizeof(*req), M_UFSHCI, how | M_ZERO);
 	if (req != NULL) {
 		req->cb_fn = cb_fn;
 		req->cb_arg = cb_arg;
-		req->timeout = true;
 	}
 	return (req);
 }
diff --git a/sys/dev/ufshci/ufshci_req_queue.c b/sys/dev/ufshci/ufshci_req_queue.c
index bb6efa6d2ccc..7aa164d00bec 100644
--- a/sys/dev/ufshci/ufshci_req_queue.c
+++ b/sys/dev/ufshci/ufshci_req_queue.c
@@ -24,6 +24,7 @@ static const struct ufshci_qops sdb_utmr_qops = {
 	.destroy = ufshci_req_sdb_destroy,
 	.get_hw_queue = ufshci_req_sdb_get_hw_queue,
 	.enable = ufshci_req_sdb_enable,
+	.disable = ufshci_req_sdb_disable,
 	.reserve_slot = ufshci_req_sdb_reserve_slot,
 	.reserve_admin_slot = ufshci_req_sdb_reserve_slot,
 	.ring_doorbell = ufshci_req_sdb_utmr_ring_doorbell,
@@ -38,6 +39,7 @@ static const struct ufshci_qops sdb_utr_qops = {
 	.destroy = ufshci_req_sdb_destroy,
 	.get_hw_queue = ufshci_req_sdb_get_hw_queue,
 	.enable = ufshci_req_sdb_enable,
+	.disable = ufshci_req_sdb_disable,
 	.reserve_slot = ufshci_req_sdb_reserve_slot,
 	.reserve_admin_slot = ufshci_req_sdb_reserve_slot,
 	.ring_doorbell = ufshci_req_sdb_utr_ring_doorbell,
@@ -74,6 +76,13 @@ ufshci_utmr_req_queue_destroy(struct ufshci_controller *ctrlr)
 	    &ctrlr->task_mgmt_req_queue);
 }
 
+void
+ufshci_utmr_req_queue_disable(struct ufshci_controller *ctrlr)
+{
+	ctrlr->task_mgmt_req_queue.qops.disable(ctrlr,
+	    &ctrlr->task_mgmt_req_queue);
+}
+
 int
 ufshci_utmr_req_queue_enable(struct ufshci_controller *ctrlr)
 {
@@ -109,6 +118,13 @@ ufshci_utr_req_queue_destroy(struct ufshci_controller *ctrlr)
 	    &ctrlr->transfer_req_queue);
 }
 
+void
+ufshci_utr_req_queue_disable(struct ufshci_controller *ctrlr)
+{
+	ctrlr->transfer_req_queue.qops.disable(ctrlr,
+	    &ctrlr->transfer_req_queue);
+}
+
 int
 ufshci_utr_req_queue_enable(struct ufshci_controller *ctrlr)
 {
@@ -226,31 +242,30 @@ void
 ufshci_req_queue_complete_tracker(struct ufshci_tracker *tr)
 {
 	struct ufshci_req_queue *req_queue = tr->req_queue;
+	struct ufshci_hw_queue *hwq = tr->hwq;
 	struct ufshci_request *req = tr->req;
 	struct ufshci_completion cpl;
 	uint8_t ocs;
 	bool retry, error, retriable;
 
-	mtx_assert(&tr->hwq->qlock, MA_NOTOWNED);
+	mtx_assert(&hwq->qlock, MA_NOTOWNED);
 
 	/* Copy the response from the Request Descriptor or UTP Command
 	 * Descriptor. */
+	cpl.size = tr->response_size;
 	if (req_queue->is_task_mgmt) {
-		cpl.size = tr->response_size;
 		memcpy(&cpl.response_upiu,
-		    (void *)tr->hwq->utmrd[tr->slot_num].response_upiu,
-		    cpl.size);
+		    (void *)hwq->utmrd[tr->slot_num].response_upiu, cpl.size);
 
-		ocs = tr->hwq->utmrd[tr->slot_num].overall_command_status;
+		ocs = hwq->utmrd[tr->slot_num].overall_command_status;
 	} else {
 		bus_dmamap_sync(req_queue->dma_tag_ucd, req_queue->ucdmem_map,
 		    BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
 
-		cpl.size = tr->response_size;
 		memcpy(&cpl.response_upiu, (void *)tr->ucd->response_upiu,
 		    cpl.size);
 
-		ocs = tr->hwq->utrd[tr->slot_num].overall_command_status;
+		ocs = hwq->utrd[tr->slot_num].overall_command_status;
 	}
 
 	error = ufshci_req_queue_response_is_error(req_queue, ocs,
@@ -262,9 +277,9 @@ ufshci_req_queue_complete_tracker(struct ufshci_tracker *tr)
 	retry = error && retriable &&
 	    req->retries < req_queue->ctrlr->retry_count;
 	if (retry)
-		tr->hwq->num_retries++;
+		hwq->num_retries++;
 	if (error && req->retries >= req_queue->ctrlr->retry_count && retriable)
-		tr->hwq->num_failures++;
+		hwq->num_failures++;
 
 	KASSERT(tr->req, ("there is no request assigned to the tracker\n"));
 	KASSERT(cpl.response_upiu.header.task_tag ==
@@ -282,7 +297,7 @@ ufshci_req_queue_complete_tracker(struct ufshci_tracker *tr)
 			req->cb_fn(req->cb_arg, &cpl, error);
 	}
 
-	mtx_lock(&tr->hwq->qlock);
+	mtx_lock(&hwq->qlock);
 
 	/* Clear the UTRL Completion Notification register */
 	req_queue->qops.clear_cpl_ntf(req_queue->ctrlr, tr);
@@ -301,6 +316,9 @@ ufshci_req_queue_complete_tracker(struct ufshci_tracker *tr)
 		ufshci_free_request(req);
 		tr->req = NULL;
 		tr->slot_state = UFSHCI_SLOT_STATE_FREE;
+
+		TAILQ_REMOVE(&hwq->outstanding_tr, tr, tailq);
+		TAILQ_INSERT_HEAD(&hwq->free_tr, tr, tailq);
 	}
 
 	mtx_unlock(&tr->hwq->qlock);
@@ -309,7 +327,16 @@ ufshci_req_queue_complete_tracker(struct ufshci_tracker *tr)
 bool
 ufshci_req_queue_process_completions(struct ufshci_req_queue *req_queue)
 {
-	return (req_queue->qops.process_cpl(req_queue));
+	struct ufshci_hw_queue *hwq;
+	bool done;
+
+	hwq = req_queue->qops.get_hw_queue(req_queue);
+
+	mtx_lock(&hwq->recovery_lock);
+	done = req_queue->qops.process_cpl(req_queue);
+	mtx_unlock(&hwq->recovery_lock);
+
+	return (done);
 }
 
 static void
@@ -427,6 +454,225 @@ ufshci_req_queue_fill_utr_descriptor(struct ufshci_utp_xfer_req_desc *desc,
 	desc->prdt_length = prdt_entry_cnt;
 }
 
+static void
+ufshci_req_queue_timeout_recovery(struct ufshci_controller *ctrlr,
+    struct ufshci_hw_queue *hwq)
+{
+	/* TODO: Step 2. Logical unit reset */
+	/* TODO: Step 3. Target device reset */
+	/* TODO: Step 4. Bus reset */
+
+	/*
+	 * Step 5. All previous commands were timeout.
+	 * Recovery failed, reset the host controller.
+	 */
+	ufshci_printf(ctrlr,
+	    "Recovery step 5: Resetting controller due to a timeout.\n");
+	hwq->recovery_state = RECOVERY_WAITING;
+
+	ufshci_ctrlr_reset(ctrlr);
+}
+
+static void
+ufshci_abort_complete(void *arg, const struct ufshci_completion *status,
+    bool error)
+{
+	struct ufshci_tracker *tr = arg;
+
+	/*
+	 * We still need to check the active tracker array, to cover race where
+	 * I/O timed out at same time controller was completing the I/O. An
+	 * abort request always is on the Task Management Request queue, but
+	 * affects either an Task Management Request or an I/O (UTRL) queue, so
+	 * take the appropriate queue lock for the original command's queue,
+	 * since we'll need it to avoid races with the completion code and to
+	 * complete the command manually.
+	 */
+	mtx_lock(&tr->hwq->qlock);
+	if (tr->slot_state != UFSHCI_SLOT_STATE_FREE) {
+		mtx_unlock(&tr->hwq->qlock);
+		/*
+		 * An I/O has timed out, and the controller was unable to abort
+		 * it for some reason.  And we've not processed a completion for
+		 * it yet. Construct a fake completion status, and then complete
+		 * the I/O's tracker manually.
+		 */
+		ufshci_printf(tr->hwq->ctrlr,
+		    "abort task request failed, aborting task manually\n");
+		ufshci_req_queue_manual_complete_tracker(tr,
+		    UFSHCI_DESC_ABORTED, UFSHCI_RESPONSE_CODE_GENERAL_FAILURE);
+
+		if ((status->response_upiu.task_mgmt_response_upiu
+			    .output_param1 ==
+			UFSHCI_TASK_MGMT_SERVICE_RESPONSE_FUNCTION_COMPLETE) ||
+		    (status->response_upiu.task_mgmt_response_upiu
+			    .output_param1 ==
+			UFSHCI_TASK_MGMT_SERVICE_RESPONSE_FUNCTION_SUCCEEDED)) {
+			ufshci_printf(tr->hwq->ctrlr,
+			    "Warning: the abort task request completed \
+			    successfully, but the original task is still incomplete.");
+			return;
+		}
+
+		/* Abort Task failed. Perform recovery steps 2-5 */
+		ufshci_req_queue_timeout_recovery(tr->hwq->ctrlr, tr->hwq);
+	} else {
+		mtx_unlock(&tr->hwq->qlock);
+	}
+}
+
+static void
+ufshci_req_queue_timeout(void *arg)
+{
+	struct ufshci_hw_queue *hwq = arg;
+	struct ufshci_controller *ctrlr = hwq->ctrlr;
+	struct ufshci_tracker *tr;
+	sbintime_t now;
+	bool idle = true;
+	bool fast;
+
+	mtx_assert(&hwq->recovery_lock, MA_OWNED);
+
+	/*
+	 * If the controller is failed, then stop polling. This ensures that any
+	 * failure processing that races with the hwq timeout will fail safely.
+	 */
+	if (ctrlr->is_failed) {
+		ufshci_printf(ctrlr,
+		    "Failed controller, stopping watchdog timeout.\n");
+		hwq->timer_armed = false;
+		return;
+	}
+
+	/*
+	 * 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.
+	 */
+	if (!hwq->timer_armed) {
+		ufshci_printf(ctrlr,
+		    "Timeout fired during ufshci_utr_req_queue_destroy\n");
+		return;
+	}
+
+	switch (hwq->recovery_state) {
+	case RECOVERY_NONE:
+		/*
+		 * See if there's any recovery needed. First, do a fast check to
+		 * see if anything could have timed out. If not, then skip
+		 * everything else.
+		 */
+		fast = false;
+		mtx_lock(&hwq->qlock);
+		now = getsbinuptime();
*** 337 LINES SKIPPED ***



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202509181442.58IEg4UQ077832>