[PATCH] hw/ufs: Reset controller and MCQ state on HCE transition to 0

Stanley Jhu posted 1 patch 3 weeks, 4 days ago
Failed in applying to current master (apply log)
There is a newer version of this series
hw/ufs/trace-events |  1 +
hw/ufs/ufs.c        | 64 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+)
[PATCH] hw/ufs: Reset controller and MCQ state on HCE transition to 0
Posted by Stanley Jhu 3 weeks, 4 days ago
According to the JEDEC Universal Flash Storage Host Controller Interface
(UFSHCI) specification (Section 5.2.1 "Host Controller Enable"):
when Host Controller Enable (HCE) transitions from 1 to 0, a host
controller reset is initiated. The host controller shall abort all
active transfers, return internal state machines to idle, and de-assert
all interrupts.

Currently, QEMU's UFS emulator only clears HCS and HCE registers upon
HCE=0, leaving internal state active. Specifically:
- Outstanding SCSI requests in the block layer are not purged.
- Active bottom halves (doorbell_bh and complete_bh) remain scheduled.
- Allocated MCQ Submission and Completion Queues (sq and cq) are not freed.
- Dynamic MCQ queue registers and legacy UTRL request states remain stale.

Implement ufs_hce_reset() to:
1. Purge outstanding SCSI requests for all logical units via
   scsi_device_purge_requests().
2. Cancel active bottom halves (doorbell_bh and complete_bh).
3. Clear standard request list slots and release SGLs via
   ufs_clear_req().
4. Free allocated MCQ queues and clear dynamic queue registers while
   preserving static capability offsets.
5. De-assert interrupts via ufs_irq_check().
6. Add a trace_ufs_hce_reset() trace event.

Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 hw/ufs/trace-events |  1 +
 hw/ufs/ufs.c        | 64 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/hw/ufs/trace-events b/hw/ufs/trace-events
index 00e263c7ba..1eb85c6978 100644
--- a/hw/ufs/trace-events
+++ b/hw/ufs/trace-events
@@ -14,6 +14,7 @@ ufs_process_uiccmd(uint32_t uiccmd, uint32_t
ucmdarg1, uint32_t ucmdarg2, uint
 ufs_mcq_complete_req(uint8_t qid) "sqid %"PRIu8""
 ufs_mcq_create_sq(uint8_t sqid, uint8_t cqid, uint64_t addr, uint16_t
size) "mcq create sq sqid %"PRIu8", cqid %"PRIu8", addr 0x%"PRIx64",
size %"PRIu16""
 ufs_mcq_create_cq(uint8_t cqid, uint64_t addr, uint16_t size) "mcq
create cq cqid %"PRIu8", addr 0x%"PRIx64", size %"PRIu16""
+ufs_hce_reset(void) "HCE 1 -> 0 reset: cancelling BHs, resetting MCQ
and request lists"

 # error condition
 ufs_err_dma_read_utrd(uint32_t slot, uint64_t addr) "failed to read
utrd. UTRLDBR slot %"PRIu32", UTRD dma addr %"PRIu64""
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index a8e2df8087..b729b247f1 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -679,6 +679,69 @@ static bool ufs_mcq_delete_cq(UfsHc *u, uint8_t qid)
     return true;
 }

+static void ufs_hce_reset(UfsHc *u)
+{
+    int i;
+
+    trace_ufs_hce_reset();
+
+    /* 1. Purge outstanding SCSI requests for all logical units */
+    for (i = 0; i < UFS_MAX_LUS; i++) {
+        if (u->lus[i] && u->lus[i]->scsi_dev) {
+            scsi_device_purge_requests(u->lus[i]->scsi_dev, SENSE_CODE(RESET));
+        }
+    }
+
+    /* 2. Cancel active Bottom Halves */
+    if (u->doorbell_bh) {
+        qemu_bh_cancel(u->doorbell_bh);
+    }
+    if (u->complete_bh) {
+        qemu_bh_cancel(u->complete_bh);
+    }
+
+    /* 3. Reset standard request list slots and doorbells */
+    for (i = 0; i < u->params.nutrs; i++) {
+        ufs_clear_req(&u->req_list[i]);
+        u->req_list[i].state = UFS_REQUEST_IDLE;
+    }
+    u->reg.utrldbr = 0;
+    u->reg.utmrldbr = 0;
+    u->reg.is = 0;
+
+    /* 4. Free MCQ Queues and reset MCQ dynamic registers */
+    if (u->params.mcq) {
+        for (i = 0; i < ARRAY_SIZE(u->sq); i++) {
+            if (u->sq[i]) {
+                ufs_mcq_free_sq(u->sq[i]);
+                u->sq[i] = NULL;
+            }
+        }
+        for (i = 0; i < ARRAY_SIZE(u->cq); i++) {
+            if (u->cq[i]) {
+                ufs_mcq_free_cq(u->cq[i]);
+                u->cq[i] = NULL;
+            }
+        }
+
+        /* Clear dynamic queue configuration without touching static offsets */
+        for (i = 0; i < ARRAY_SIZE(u->mcq_reg); i++) {
+            u->mcq_reg[i].sqattr = 0;
+            u->mcq_reg[i].sqlba = 0;
+            u->mcq_reg[i].squba = 0;
+            u->mcq_reg[i].sqcfg = 0;
+            u->mcq_reg[i].cqattr = 0;
+            u->mcq_reg[i].cqlba = 0;
+            u->mcq_reg[i].cquba = 0;
+            u->mcq_reg[i].cqcfg = 0;
+        }
+        memset(&u->mcq_op_reg, 0, sizeof(u->mcq_op_reg));
+    }
+
+    /* 5. De-assert IRQ */
+    ufs_irq_check(u);
+}
+
 static void ufs_write_reg(UfsHc *u, hwaddr offset, uint32_t data,
unsigned size)
 {
     switch (offset) {
@@ -696,6 +759,7 @@ static void ufs_write_reg(UfsHc *u, hwaddr offset,
uint32_t data, unsigned size)
             u->reg.hce = FIELD_DP32(u->reg.hce, HCE, HCE, 1);
         } else if (FIELD_EX32(u->reg.hce, HCE, HCE) &&
                    !FIELD_EX32(data, HCE, HCE)) {
+            ufs_hce_reset(u);
             u->reg.hcs = 0;
             u->reg.hce = FIELD_DP32(u->reg.hce, HCE, HCE, 0);
         }
--
2.43.0
Re: [PATCH] hw/ufs: Reset controller and MCQ state on HCE transition to 0
Posted by Jeuk Kim 3 weeks, 3 days ago
Hi Stanley,

Thanks for your contribution.


On 9/1/2026 10:03 PM, Stanley Jhu wrote:
> According to the JEDEC Universal Flash Storage Host Controller Interface
> (UFSHCI) specification (Section 5.2.1 "Host Controller Enable"):
> when Host Controller Enable (HCE) transitions from 1 to 0, a host
> controller reset is initiated. The host controller shall abort all
> active transfers, return internal state machines to idle, and de-assert
> all interrupts.
>
> Currently, QEMU's UFS emulator only clears HCS and HCE registers upon
> HCE=0, leaving internal state active. Specifically:
> - Outstanding SCSI requests in the block layer are not purged.
> - Active bottom halves (doorbell_bh and complete_bh) remain scheduled.
> - Allocated MCQ Submission and Completion Queues (sq and cq) are not freed.
> - Dynamic MCQ queue registers and legacy UTRL request states remain stale.
>
> Implement ufs_hce_reset() to:
> 1. Purge outstanding SCSI requests for all logical units via
>     scsi_device_purge_requests().
> 2. Cancel active bottom halves (doorbell_bh and complete_bh).
> 3. Clear standard request list slots and release SGLs via
>     ufs_clear_req().
> 4. Free allocated MCQ queues and clear dynamic queue registers while
>     preserving static capability offsets.
> 5. De-assert interrupts via ufs_irq_check().
> 6. Add a trace_ufs_hce_reset() trace event.
>
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
> ---
>   hw/ufs/trace-events |  1 +
>   hw/ufs/ufs.c        | 64 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 65 insertions(+)
>
> diff --git a/hw/ufs/trace-events b/hw/ufs/trace-events
> index 00e263c7ba..1eb85c6978 100644
> --- a/hw/ufs/trace-events
> +++ b/hw/ufs/trace-events
> @@ -14,6 +14,7 @@ ufs_process_uiccmd(uint32_t uiccmd, uint32_t
> ucmdarg1, uint32_t ucmdarg2, uint
>   ufs_mcq_complete_req(uint8_t qid) "sqid %"PRIu8""
>   ufs_mcq_create_sq(uint8_t sqid, uint8_t cqid, uint64_t addr, uint16_t
> size) "mcq create sq sqid %"PRIu8", cqid %"PRIu8", addr 0x%"PRIx64",
> size %"PRIu16""
>   ufs_mcq_create_cq(uint8_t cqid, uint64_t addr, uint16_t size) "mcq
> create cq cqid %"PRIu8", addr 0x%"PRIx64", size %"PRIu16""
> +ufs_hce_reset(void) "HCE 1 -> 0 reset: cancelling BHs, resetting MCQ
> and request lists"
>
>   # error condition
>   ufs_err_dma_read_utrd(uint32_t slot, uint64_t addr) "failed to read
> utrd. UTRLDBR slot %"PRIu32", UTRD dma addr %"PRIu64""
> diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
> index a8e2df8087..b729b247f1 100644
> --- a/hw/ufs/ufs.c
> +++ b/hw/ufs/ufs.c
> @@ -679,6 +679,69 @@ static bool ufs_mcq_delete_cq(UfsHc *u, uint8_t qid)
>       return true;
>   }
>
> +static void ufs_hce_reset(UfsHc *u)
> +{
> +    int i;
> +
> +    trace_ufs_hce_reset();
> +
> +    /* 1. Purge outstanding SCSI requests for all logical units */
> +    for (i = 0; i < UFS_MAX_LUS; i++) {
> +        if (u->lus[i] && u->lus[i]->scsi_dev) {
> +            scsi_device_purge_requests(u->lus[i]->scsi_dev, SENSE_CODE(RESET));

The UFS SCSI bus has no .cancel callback. The reference returned by 
scsi_req_new() is released only from ufs_scsi_command_complete(), so
a cancelled request would leak that reference.

> +        }
> +    }
> +
> +    /* 2. Cancel active Bottom Halves */
scsi_device_purge_requests() calls blk_drain(), which may run pending 
BHs. Please stop the request-producing BHs before
the purge and make sure they cannot be scheduled again while the reset 
is in progress. Note that an MCQ CQ BH may reschedule an SQ BH.
> +    if (u->doorbell_bh) {
> +        qemu_bh_cancel(u->doorbell_bh);
> +    }
> +    if (u->complete_bh) {
> +        qemu_bh_cancel(u->complete_bh);
> +    }
> +
> +    /* 3. Reset standard request list slots and doorbells */
> +    for (i = 0; i < u->params.nutrs; i++) {
> +        ufs_clear_req(&u->req_list[i]);
> +        u->req_list[i].state = UFS_REQUEST_IDLE;
> +    }
> +    u->reg.utrldbr = 0;
> +    u->reg.utmrldbr = 0;
> +    u->reg.is = 0;

Please clear UTRLCNR and UTRLRSR as well.


> +
> +    /* 4. Free MCQ Queues and reset MCQ dynamic registers */
> +    if (u->params.mcq) {
> +        for (i = 0; i < ARRAY_SIZE(u->sq); i++) {
> +            if (u->sq[i]) {
> +                ufs_mcq_free_sq(u->sq[i]);
> +                u->sq[i] = NULL;
> +            }
> +        }
> +        for (i = 0; i < ARRAY_SIZE(u->cq); i++) {
> +            if (u->cq[i]) {
> +                ufs_mcq_free_cq(u->cq[i]);
> +                u->cq[i] = NULL;
> +            }
> +        }
> +
> +        /* Clear dynamic queue configuration without touching static offsets */
> +        for (i = 0; i < ARRAY_SIZE(u->mcq_reg); i++) {
> +            u->mcq_reg[i].sqattr = 0;
> +            u->mcq_reg[i].sqlba = 0;
> +            u->mcq_reg[i].squba = 0;
> +            u->mcq_reg[i].sqcfg = 0;
> +            u->mcq_reg[i].cqattr = 0;
> +            u->mcq_reg[i].cqlba = 0;
> +            u->mcq_reg[i].cquba = 0;
> +            u->mcq_reg[i].cqcfg = 0;
> +        }
> +        memset(&u->mcq_op_reg, 0, sizeof(u->mcq_op_reg));
> +    }
> +
> +    /* 5. De-assert IRQ */
> +    ufs_irq_check(u);
> +}
> +
>   static void ufs_write_reg(UfsHc *u, hwaddr offset, uint32_t data,
> unsigned size)
>   {
>       switch (offset) {
> @@ -696,6 +759,7 @@ static void ufs_write_reg(UfsHc *u, hwaddr offset,
> uint32_t data, unsigned size)
>               u->reg.hce = FIELD_DP32(u->reg.hce, HCE, HCE, 1);
>           } else if (FIELD_EX32(u->reg.hce, HCE, HCE) &&
>                      !FIELD_EX32(data, HCE, HCE)) {
> +            ufs_hce_reset(u);
>               u->reg.hcs = 0;
>               u->reg.hce = FIELD_DP32(u->reg.hce, HCE, HCE, 0);
>           }
> --
> 2.43.0

There seem to be two possible approaches here:

1. Add the missing SCSI cancellation handling and stop the 
request-producing BHs before purging the requests.
2. Stop accepting new requests and defer the HCE reset until the 
in-flight requests complete normally.

The second approach looks simpler for now. HCE can remain set while the 
disable is pending and be cleared from
a BH once no request remains in flight.
Re: [PATCH] hw/ufs: Reset controller and MCQ state on HCE transition to 0
Posted by Stanley Jhu 3 weeks, 3 days ago
On Wed, Sep 2, 2026 at 3:51 PM Jeuk Kim <jeuk20.kim@gmail.com> wrote:
> > +    /* 1. Purge outstanding SCSI requests for all logical units */
> > +    for (i = 0; i < UFS_MAX_LUS; i++) {
> > +        if (u->lus[i] && u->lus[i]->scsi_dev) {
> > +            scsi_device_purge_requests(u->lus[i]->scsi_dev, SENSE_CODE(RESET));
>
> The UFS SCSI bus has no .cancel callback. The reference returned by
> scsi_req_new() is released only from ufs_scsi_command_complete(), so
> a cancelled request would leak that reference.

Good catch. I added ufs_scsi_command_cancelled() and registered .cancel
in ufs_scsi_info (in hw/ufs/lu.c). It sets scsi_req->hba_private = NULL
and drops the reference via scsi_req_unref(scsi_req), ensuring cancelled
requests do not leak.

> > +    /* 2. Cancel active Bottom Halves */
> scsi_device_purge_requests() calls blk_drain(), which may run pending
> BHs. Please stop the request-producing BHs before
> the purge and make sure they cannot be scheduled again while the reset
> is in progress. Note that an MCQ CQ BH may reschedule an SQ BH.

Agreed. I introduced a `u->resetting` flag and reordered the sequence:
1. All active BHs (doorbell_bh, complete_bh, and all MCQ sq[i]->bh and
   cq[i]->bh) are canceled BEFORE scsi_device_purge_requests().
2. In ufs_mcq_process_cq(), rescheduling sq->bh is explicitly guarded by
   `!u->resetting`.
3. Request processing in ufs_mcq_process_sq() and ufs_process_req()
early-returns
   if `u->resetting` is set.
This ensures blk_drain() cannot run or reschedule any request-producing BHs
during the purge.

> > +    u->reg.utrldbr = 0;
> > +    u->reg.utmrldbr = 0;
> > +    u->reg.is = 0;
>
> Please clear UTRLCNR and UTRLRSR as well.

Done. Added:
    u->reg.utrlcnr = 0;
    u->reg.utrlrsr = 0;

> There seem to be two possible approaches here:
>
> 1. Add the missing SCSI cancellation handling and stop the
> request-producing BHs before purging the requests.
> 2. Stop accepting new requests and defer the HCE reset until the
> in-flight requests complete normally.
>
> The second approach looks simpler for now. HCE can remain set while the
> disable is pending and be cleared from
> a BH once no request remains in flight.

I chose Approach 1 for two main reasons:
1. JEDEC UFSHCI Section 5.2.1 ("Host Controller Enable") explicitly specifies
   that when HCE transitions to 0, the controller "shall abort all active
   transfers". Deferring the reset to allow in-flight requests to complete
   normally risks writing completion UPIUs/CQEs back to guest memory after
   the guest OS has already aborted them or while the guest is actively
   re-initializing the controller.
2. Synchronous purge via scsi_device_purge_requests() guarantees that once
   the MMIO write to HCE=0 returns, the controller is immediately in a clean,
   idle state without lingering asynchronous completion windows.

With .cancel implemented and BHs guarded, Approach 1 is clean and compact.
I verified this end-to-end under ARM64 QEMU with UFS 4.0 MCQ fault injection
(dropping CQE to force guest timeout and mock abort failure): full host
reset completed cleanly, and subsequent 4KB direct I/O passed 100% binary
comparison with zero warnings or leaks.

I will send v2 shortly.

Thanks,
Stanley Jhu