[PATCH v2 0/3] hw/ufs: Support Task Management Request (TMR) and MCQ status registers

[PATCH v2 0/3] hw/ufs: Support Task Management Request (TMR) and MCQ status registers

[Stanley Jhu]

This patch series implements JEDEC UFSHCI specification compliance for
Task Management Requests (TMR) and Multi-Circular Queue (MCQ) runtime
status and interrupt registers in the QEMU UFS emulator.

Why:
In QEMU UFS emulation, Linux guest error recovery paths (such as SCSI
abort during command timeout) issue Task Management Requests (TMR) via
the UTP Task Management Request List (UTMRL). Furthermore, when MCQ
is enabled, the Linux driver initializes and accesses per-queue runtime
registers (SQnRTC, SQnRTS, SQnIS, CQnIS, etc.). Currently, QEMU treats
UTMRLDBR and MCQ operational registers as unsupported, triggering
"invalid register offset" warnings and causing SCSI EH aborts to time out
and escalate unnecessarily to full controller resets. Additionally,
pending SCSI requests in the block layer were not cancelled when requests
were cleared or aborted in ufs_clear_req(), posing Use-After-Free
hazards upon asynchronous AIO callbacks.

What:
- Patch 1: Tracks SCSIRequest in UfsRequest and cancels pending requests
  via scsi_req_cancel() in ufs_clear_req(), ensuring clean Block AIO
  teardown.
- Patch 2: Implements MCQ runtime operational registers (queue lifecycle
  control, per-queue interrupt status/enable, global CQES sync) and HCE=0
  MMIO read protection.
- Patch 3: Implements Task Management Request handling for UTMRLDBR
  (supporting UFS_QUERY_TASK and UFS_ABORT_TASK for both legacy UTRL
  and MCQ queues).

Differences from v1:
- Split into two independent series: this series contains production
  specification compliance and memory safety fixes; experimental fault
  injection properties (x-hold-tag, x-hold-mode) have been moved to a
  separate follow-up patch based on this series.
- Separated scsi_req_cancel() into Patch 1 to isolate the block layer
  memory safety fix.
- Added HCE=0 operational register read guard (returning 0xffffffff) in
  Patch 2 per JEDEC UFSHCI specification.
- Cleaned up TMR completion handling and trace events in Patch 3.

Tested:
- Verified on ARM64 Linux guest (linux-next) running on QEMU.
- Verified ufshcd_mcq_make_queues_operational() succeeds without invalid
  register offset warnings.
- Verified SCSI command aborts and Task Management Requests via Linux
  SCSI error handling escalation.
- Verified 100% data integrity (cmp) on /dev/sda after abort and reset
  recovery.
- Passes ./scripts/checkpatch.pl with 0 errors and 0 warnings.

Stanley Jhu (3):
  hw/ufs: Track SCSIRequest and cancel pending requests in ufs_clear_req
  hw/ufs: Support MCQ runtime interrupt and queue status registers
  hw/ufs: Implement Task Management Request (TMR) handling

 hw/ufs/lu.c         |  12 +++
 hw/ufs/trace-events |   2 +
 hw/ufs/ufs.c        | 196 +++++++++++++++++++++++++++++++++++++++++++-
 hw/ufs/ufs.h        |   1 +
 include/block/ufs.h |   9 ++
 5 files changed, 217 insertions(+), 3 deletions(-)

-- 
2.55.0.1007.g17ff1f9808-goog

[PATCH v2 1/3] hw/ufs: Track SCSIRequest and cancel pending requests in ufs_clear_req

[Stanley Jhu]

In the UFS emulator, outstanding SCSI requests are dispatched asynchronously
to the block layer via scsi_req_enqueue(). When requests are cleared or
aborted (e.g. during HCE reset or TMR aborts), ufs_clear_req() releases the
scatter-gather list without cancelling the pending SCSIRequest. If an
asynchronous AIO callback completes afterwards, ufs_scsi_command_complete()
dereferences stale or freed request state, resulting in use-after-free
hazards.

Track the active SCSIRequest in UfsRequest and explicitly cancel any in-flight
requests in ufs_clear_req() before freeing request resources. To prevent
dangling references during cancellation cascades, decouple and clear req->sreq
in completion and cancellation handlers.

Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 hw/ufs/lu.c  | 12 ++++++++++++
 hw/ufs/ufs.c |  6 ++++++
 hw/ufs/ufs.h |  1 +
 3 files changed, 19 insertions(+)

diff --git a/hw/ufs/lu.c b/hw/ufs/lu.c
index bdb1650851..a62ebb51fd 100644
--- a/hw/ufs/lu.c
+++ b/hw/ufs/lu.c
@@ -173,6 +173,12 @@ static void ufs_scsi_command_complete(SCSIRequest *scsi_req, size_t resid)
     int16_t status = scsi_req->status;
     uint32_t transfered_len = scsi_req->cmd.xfer - resid;
 
+    if (!req) {
+        return;
+    }
+
+    req->sreq = NULL;
+
     /* WB / HID accounting should only happen for successful commands */
     if (status == GOOD) {
         ufs_wb_process_write_req(req, transfered_len);
@@ -190,6 +196,11 @@ static void ufs_scsi_command_complete(SCSIRequest *scsi_req, size_t resid)
 
 static void ufs_scsi_command_cancelled(SCSIRequest *scsi_req)
 {
+    UfsRequest *req = scsi_req->hba_private;
+
+    if (req) {
+        req->sreq = NULL;
+    }
     scsi_req->hba_private = NULL;
     scsi_req_unref(scsi_req);
 }
@@ -389,6 +400,7 @@ static UfsReqResult ufs_process_scsi_cmd(UfsLu *lu, UfsRequest *req)
     SCSIRequest *scsi_req =
         scsi_req_new(lu->scsi_dev, task_tag, lu->lun, req->req_upiu.sc.cdb,
                      UFS_CDB_SIZE, req);
+    req->sreq = scsi_req;
 
     uint32_t len = scsi_req_enqueue(scsi_req);
     if (len) {
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index 3c4d7424da..adae6639e1 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -2221,6 +2221,12 @@ void ufs_complete_req(UfsRequest *req, UfsReqResult req_result)
 
 static void ufs_clear_req(UfsRequest *req)
 {
+    if (req->sreq != NULL) {
+        SCSIRequest *sreq = req->sreq;
+        req->sreq = NULL;
+        scsi_req_cancel(sreq);
+    }
+
     if (req->sg != NULL) {
         qemu_sglist_destroy(req->sg);
         g_free(req->sg);
diff --git a/hw/ufs/ufs.h b/hw/ufs/ufs.h
index 6f2693b7ca..265a43faaa 100644
--- a/hw/ufs/ufs.h
+++ b/hw/ufs/ufs.h
@@ -60,6 +60,7 @@ typedef struct UfsRequest {
     UtpUpiuRsp rsp_upiu;
 
     /* for scsi command */
+    SCSIRequest *sreq;
     QEMUSGList *sg;
     uint32_t data_len;
 
-- 
2.55.0.1007.g17ff1f9808-goog

[PATCH v2 2/3] hw/ufs: Support MCQ runtime interrupt and queue status registers

[Stanley Jhu]

According to JEDEC UFSHCI 5.2.1 and 5.6, Multi-Circular Queue (MCQ)
architecture provides per-queue runtime control and interrupt registers.
When Linux initializes MCQ via ufshcd_mcq_make_queues_operational(),
it configures operational registers (SQnRTC, SQnCTI, SQnIS/IE, CQnIS/IE,
CQnIACR). Currently, QEMU treats these offsets as unhandled, generating
"invalid register offset" warnings and failing queue lifecycle transitions.

Implement MCQ runtime operational register handling:
- Support SQ run-time control (SQnRTC) to start, stop, and clean up submission
  queues, updating run-time status (SQnRTS) and synchronizing with the SQ
  processing bottom half.
- Support per-queue interrupt status and enable registers (SQnIS/IE, CQnIS/IE).
  Implement write-1-to-clear semantics and dynamically synchronize the global
  CQES (CQ Event Status) bit in IS to prevent interrupt storms.
- Store queue configuration for interrupt aggregation (CQnIACR) and completion
  timeout intervals (SQnCTI).
- Enforce controller reset state machine rules: guard MMIO reads when HCE=0,
  and reinitialize operational registers across HCE resets while preserving
  MCQ capability configurations.

Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 hw/ufs/trace-events |  1 +
 hw/ufs/ufs.c        | 73 +++++++++++++++++++++++++++++++++++++++++++--
 include/block/ufs.h |  9 ++++++
 3 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/hw/ufs/trace-events b/hw/ufs/trace-events
index 922293355b..d8173b12b6 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, uint32
 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_write_mcq_op_reg(uint8_t qid, uint32_t offset, uint32_t data) "qid %"PRIu8", offset 0x%"PRIx32", data 0x%"PRIx32""
 ufs_hce_reset(void) "HCE 1 -> 0 reset: cancelling BHs, resetting MCQ and request lists"
 
 # error condition
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index adae6639e1..4e22c31f89 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -446,13 +446,14 @@ static void ufs_mcq_process_sq(void *opaque)
 {
     UfsSq *sq = opaque;
     UfsHc *u = sq->u;
+    UfsMcqOpReg *opr = &u->mcq_op_reg[sq->sqid];
     UfsSqEntry sqe;
     UfsRequest *req;
     hwaddr addr;
     uint16_t head = ufs_mcq_sq_head(u, sq->sqid);
     int err;
 
-    if (u->resetting) {
+    if (u->resetting || FIELD_EX32(opr->sq.rts, SQRTS, STS)) {
         return;
     }
 
@@ -770,7 +771,17 @@ static void ufs_hce_reset(UfsHc *u)
     u->reg.utmrldbr = 0;
     u->reg.utrlcnr = 0;
     u->reg.utrlrsr = 0;
+    u->reg.utriacr = 0;
+    u->reg.utrlclr = 0;
+    if (u->params.mcq) {
+        u->reg.mcqconfig = FIELD_DP32(0, MCQCONFIG, MAC, 0x1f);
+    } else {
+        u->reg.mcqconfig = 0;
+    }
+    u->reg.ie = 0;
     u->reg.is = 0;
+    u->reg.utrlba = 0;
+    u->reg.utrlbau = 0;
 
     /* 4. Free MCQ Queues and reset MCQ dynamic registers */
     if (u->params.mcq) {
@@ -983,6 +994,9 @@ static void ufs_write_mcq_op_reg(UfsHc *u, hwaddr offset, uint32_t data,
 
     opr = &u->mcq_op_reg[qid];
 
+    trace_ufs_write_mcq_op_reg(qid, (uint32_t)(offset % sizeof(UfsMcqOpReg)),
+                               data);
+
     switch (offset % sizeof(UfsMcqOpReg)) {
     case offsetof(UfsMcqOpReg, sq.tp):
         if (opr->sq.tp != data) {
@@ -990,6 +1004,38 @@ static void ufs_write_mcq_op_reg(UfsHc *u, hwaddr offset, uint32_t data,
         }
         opr->sq.tp = data;
         break;
+    case offsetof(UfsMcqOpReg, sq.rtc):
+        opr->sq.rtc = data;
+        if (FIELD_EX32(data, SQRTC, ICU)) {
+            /* SQ_ICU: Initiate Cleanup (SQ_CUS = 1, RTC = 0) */
+            opr->sq.rts = FIELD_DP32(opr->sq.rts, SQRTS, CUS, 1);
+            opr->sq.rts = FIELD_DP32(opr->sq.rts, SQRTS, RTC, 0);
+        }
+        if (FIELD_EX32(data, SQRTC, STOP)) {
+            /* SQ_STOP: Stop queue */
+            opr->sq.rts = FIELD_DP32(opr->sq.rts, SQRTS, STS, 1);
+            if (u->sq[qid] && u->sq[qid]->bh) {
+                qemu_bh_cancel(u->sq[qid]->bh);
+            }
+        } else {
+            /* SQ_START: Start queue */
+            opr->sq.rts = FIELD_DP32(opr->sq.rts, SQRTS, STS, 0);
+            opr->sq.rts = FIELD_DP32(opr->sq.rts, SQRTS, CUS, 0);
+            if (u->sq[qid] && u->sq[qid]->bh) {
+                qemu_bh_schedule(u->sq[qid]->bh);
+            }
+        }
+        break;
+    case offsetof(UfsMcqOpReg, sq.cti):
+        opr->sq.cti = data;
+        break;
+    case offsetof(UfsMcqOpReg, sq_int.is):
+        opr->sq_int.is &= ~data;
+        ufs_irq_check(u);
+        break;
+    case offsetof(UfsMcqOpReg, sq_int.ie):
+        opr->sq_int.ie = data;
+        break;
     case offsetof(UfsMcqOpReg, cq.hp): {
         UfsCq *cq = u->cq[qid];
 
@@ -1006,8 +1052,27 @@ static void ufs_write_mcq_op_reg(UfsHc *u, hwaddr offset, uint32_t data,
         ufs_mcq_update_cq_head(u, qid, data);
         break;
     }
-    case offsetof(UfsMcqOpReg, cq_int.is):
+    case offsetof(UfsMcqOpReg, cq_int.is): {
+        bool pending = false;
+
         opr->cq_int.is &= ~data;
+        for (int i = 0; i < ARRAY_SIZE(u->mcq_op_reg); i++) {
+            if (u->mcq_op_reg[i].cq_int.is) {
+                pending = true;
+                break;
+            }
+        }
+        if (!pending) {
+            u->reg.is = FIELD_DP32(u->reg.is, IS, CQES, 0);
+        }
+        ufs_irq_check(u);
+        break;
+    }
+    case offsetof(UfsMcqOpReg, cq_int.ie):
+        opr->cq_int.ie = data;
+        break;
+    case offsetof(UfsMcqOpReg, cq_int.iacr):
+        opr->cq_int.iacr = data;
         break;
     default:
         trace_ufs_err_invalid_register_offset(offset);
@@ -1029,6 +1094,10 @@ static uint64_t ufs_mmio_read(void *opaque, hwaddr addr, unsigned size)
         offset = addr - ufs_mcq_reg_addr(u, 0);
         ptr = (uint32_t *)&u->mcq_reg;
     } else if (ufs_is_mcq_op_reg(u, addr, size)) {
+        if (!FIELD_EX32(u->reg.hce, HCE, HCE)) {
+            trace_ufs_err_invalid_register_offset(addr);
+            return 0xffffffff;
+        }
         offset = addr - ufs_mcq_op_reg_addr(u, 0);
         ptr = (uint32_t *)&u->mcq_op_reg;
     } else {
diff --git a/include/block/ufs.h b/include/block/ufs.h
index d19b3c65ef..00591aa755 100644
--- a/include/block/ufs.h
+++ b/include/block/ufs.h
@@ -224,6 +224,15 @@ typedef struct QEMU_PACKED UfsMcqSqReg {
     uint32_t rts;
 } UfsMcqSqReg;
 
+REG32(SQRTC, offsetof(UfsMcqSqReg, rtc))
+    FIELD(SQRTC, STOP, 0, 1)
+    FIELD(SQRTC, ICU, 1, 1)
+
+REG32(SQRTS, offsetof(UfsMcqSqReg, rts))
+    FIELD(SQRTS, STS, 0, 1)
+    FIELD(SQRTS, CUS, 1, 1)
+    FIELD(SQRTS, RTC, 4, 4)
+
 typedef struct QEMU_PACKED UfsMcqCqReg {
     uint32_t hp;
     uint32_t tp;
-- 
2.55.0.1007.g17ff1f9808-goog

[PATCH v2 3/3] hw/ufs: Implement Task Management Request (TMR) handling

[Stanley Jhu]

According to JEDEC UFSHCI 5.2.1 (Section 7.3 "UTP Task Management"), the
host controller processes Task Management Requests via the UTP Task
Management Request List (UTMRL). When software triggers UTMRLDBR, the
controller processes descriptors, executes the requested task management
function (such as UFS_ABORT_TASK or UFS_QUERY_TASK), returns the response
UPIU, and signals completion via the UTMRCS interrupt.

Currently, QEMU does not implement the UTMRL doorbell or operational
registers. Consequently, when Linux SCSI error recovery initiates task
aborts via ufshcd_abort() or ufshcd_mcq_abort(), the requests time out and
unnecessarily escalate to full host controller resets.

Implement Task Management Request processing:
- Support UTMRL registers (UTMRLDBR, UTMRLCLR, UTMRLRSR) and latch doorbell
  events per Section 5.3.3.
- Handle UFS_ABORT_TASK and UFS_QUERY_TASK functions across both legacy UTRL
  and MCQ queues, locating outstanding requests by task tag.
- Return compliant response UPIUs with appropriate status codes (setting both
  response header and output parameters for Linux driver compatibility), and
  update descriptor OCS while preserving vendor/reserved fields.
- Reset UTMRL operational registers on HCE reset.

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

diff --git a/hw/ufs/trace-events b/hw/ufs/trace-events
index d8173b12b6..5e5a54a3fb 100644
--- a/hw/ufs/trace-events
+++ b/hw/ufs/trace-events
@@ -15,6 +15,7 @@ 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_write_mcq_op_reg(uint8_t qid, uint32_t offset, uint32_t data) "qid %"PRIu8", offset 0x%"PRIx32", data 0x%"PRIx32""
+ufs_process_tmr(uint8_t func, uint32_t tag, uint8_t resp) "query_func 0x%"PRIx8", task_tag %"PRIu32", tm_resp 0x%"PRIx8""
 ufs_hce_reset(void) "HCE 1 -> 0 reset: cancelling BHs, resetting MCQ and request lists"
 
 # error condition
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index 4e22c31f89..ca5f6f2e96 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -771,8 +771,10 @@ static void ufs_hce_reset(UfsHc *u)
     u->reg.utmrldbr = 0;
     u->reg.utrlcnr = 0;
     u->reg.utrlrsr = 0;
+    u->reg.utmrlrsr = 0;
     u->reg.utriacr = 0;
     u->reg.utrlclr = 0;
+    u->reg.utmrlclr = 0;
     if (u->params.mcq) {
         u->reg.mcqconfig = FIELD_DP32(0, MCQCONFIG, MAC, 0x1f);
     } else {
@@ -782,6 +784,8 @@ static void ufs_hce_reset(UfsHc *u)
     u->reg.is = 0;
     u->reg.utrlba = 0;
     u->reg.utrlbau = 0;
+    u->reg.utmrlba = 0;
+    u->reg.utmrlbau = 0;
 
     /* 4. Free MCQ Queues and reset MCQ dynamic registers */
     if (u->params.mcq) {
@@ -818,6 +822,111 @@ static void ufs_hce_reset(UfsHc *u)
     ufs_irq_check(u);
 }
 
+static UfsRequest *ufs_find_req_by_tag(UfsHc *u, uint32_t task_tag)
+{
+    if (task_tag < u->params.nutrs) {
+        UfsRequest *req = &u->req_list[task_tag];
+        if (req->state == UFS_REQUEST_RUNNING ||
+            req->state == UFS_REQUEST_READY) {
+            return req;
+        }
+    }
+
+    if (u->params.mcq) {
+        for (int q = 0; q < ARRAY_SIZE(u->sq); q++) {
+            UfsSq *sq = u->sq[q];
+            if (!sq) {
+                continue;
+            }
+            for (int i = 0; i < sq->size; i++) {
+                UfsRequest *req = &sq->req[i];
+                if (req->state == UFS_REQUEST_RUNNING &&
+                    req->req_upiu.header.task_tag == task_tag) {
+                    return req;
+                }
+            }
+        }
+    }
+
+    return NULL;
+}
+
+static void ufs_process_tmr(UfsHc *u, uint32_t val)
+{
+    hwaddr base_addr = (((hwaddr)u->reg.utmrlbau) << 32) + u->reg.utmrlba;
+    uint32_t completed_mask = 0;
+
+    u->reg.utmrldbr |= val;
+
+    for (int i = 0; i < u->params.nutmrs; i++) {
+        if (val & (1 << i)) {
+            uint64_t desc_addr = base_addr + i * sizeof(UtpTaskReqDesc);
+            UtpTaskReqDesc desc;
+            uint8_t tm_func, tm_resp;
+            uint32_t task_tag;
+
+            if (ufs_addr_read(u, desc_addr, &desc, sizeof(desc))) {
+                continue;
+            }
+
+            tm_func = desc.upiu_req.req_header.query_func;
+            task_tag = be32_to_cpu(desc.upiu_req.input_param2);
+
+            if (tm_func == UFS_QUERY_TASK) {
+                UfsRequest *req = ufs_find_req_by_tag(u, task_tag);
+
+                if (req) {
+                    tm_resp = UFS_UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED;
+                } else {
+                    tm_resp = UFS_UPIU_TASK_MANAGEMENT_FUNC_COMPL;
+                }
+            } else if (tm_func == UFS_ABORT_TASK) {
+                UfsRequest *req = ufs_find_req_by_tag(u, task_tag);
+
+                if (req) {
+                    ufs_clear_req(req);
+                    req->state = UFS_REQUEST_IDLE;
+                    if (ufs_mcq_req(req)) {
+                        QTAILQ_INSERT_TAIL(&req->sq->req_list, req, entry);
+                        qemu_bh_schedule(req->sq->bh);
+                    } else {
+                        u->reg.utrldbr &= ~(1 << req->slot);
+                    }
+                }
+                tm_resp = UFS_UPIU_TASK_MANAGEMENT_FUNC_COMPL;
+            } else {
+                tm_resp = UFS_UPIU_TASK_MANAGEMENT_FUNC_NOT_SUPPORTED;
+            }
+
+            memset(&desc.upiu_rsp, 0, sizeof(desc.upiu_rsp));
+            desc.header.dword_2 = cpu_to_le32(
+                (le32_to_cpu(desc.header.dword_2) & ~UFS_MASK_OCS) |
+                UFS_OCS_SUCCESS);
+            desc.upiu_rsp.rsp_header.trans_type =
+                UFS_UPIU_TRANSACTION_TASK_RSP;
+            desc.upiu_rsp.rsp_header.flags = 0;
+            desc.upiu_rsp.rsp_header.lun = desc.upiu_req.req_header.lun;
+            desc.upiu_rsp.rsp_header.task_tag =
+                desc.upiu_req.req_header.task_tag;
+            desc.upiu_rsp.rsp_header.response = tm_resp;
+            desc.upiu_rsp.output_param1 = cpu_to_be32(tm_resp);
+
+            if (ufs_addr_write(u, desc_addr, &desc, sizeof(desc))) {
+                continue;
+            }
+
+            trace_ufs_process_tmr(tm_func, task_tag, tm_resp);
+            u->reg.utmrldbr &= ~(1 << i);
+            completed_mask |= (1 << i);
+        }
+    }
+
+    if (completed_mask) {
+        u->reg.is = FIELD_DP32(u->reg.is, IS, UTMRCS, 1);
+        ufs_irq_check(u);
+    }
+}
+
 static void ufs_write_reg(UfsHc *u, hwaddr offset, uint32_t data, unsigned size)
 {
     switch (offset) {
@@ -880,10 +989,16 @@ static void ufs_write_reg(UfsHc *u, hwaddr offset, uint32_t data, unsigned size)
     case A_MCQCONFIG:
         u->reg.mcqconfig = data;
         break;
-    case A_UTRLCLR:
     case A_UTMRLDBR:
+        ufs_process_tmr(u, data);
+        break;
     case A_UTMRLCLR:
+        u->reg.utmrldbr &= ~data;
+        break;
     case A_UTMRLRSR:
+        u->reg.utmrlrsr = data;
+        break;
+    case A_UTRLCLR:
         trace_ufs_err_unsupport_register_offset(offset);
         break;
     default:
-- 
2.55.0.1007.g17ff1f9808-goog

Re: [PATCH v2 2/3] hw/ufs: Support MCQ runtime interrupt and queue status registers

[Jeuk Kim]

On Wed, 09 Sep 2026 11:11:45 +0800, Stanley Jhu <stanleyjhu@google.com> wrote:
> [...]
>   processing bottom half.
> - Support per-queue interrupt status and enable registers (SQnIS/IE, CQnIS/IE).
>   Implement write-1-to-clear semantics and dynamically synchronize the global
>   CQES (CQ Event Status) bit in IS to prevent interrupt storms.
> - Store queue configuration for interrupt aggregation (CQnIACR) and completion
>   timeout intervals (SQnCTI).

SQnCTI is Submission Queue Cleanup Task Information, not a completion
timeout interval.

>
>
> diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
> index adae6639e16..4e22c31f898 100644
> --- a/hw/ufs/ufs.c
> +++ b/hw/ufs/ufs.c
> @@ -990,6 +1004,38 @@ static void ufs_write_mcq_op_reg(UfsHc *u, hwaddr offset, uint32_t data,
>          }
>          opr->sq.tp = data;
>          break;
> +    case offsetof(UfsMcqOpReg, sq.rtc):
> +        opr->sq.rtc = data;
> +        if (FIELD_EX32(data, SQRTC, ICU)) {
> +            /* SQ_ICU: Initiate Cleanup (SQ_CUS = 1, RTC = 0) */
> +            opr->sq.rts = FIELD_DP32(opr->sq.rts, SQRTS, CUS, 1);
> +            opr->sq.rts = FIELD_DP32(opr->sq.rts, SQRTS, RTC, 0);

SQRTC.ICU reports successful cleanup without using SQCTI or changing any
request state. Is the actual cleanup expected to be handled by the TMR path?

-- 
Jeuk Kim <jeuk20.kim@samsung.com>

Re: [PATCH v2 3/3] hw/ufs: Implement Task Management Request (TMR) handling

[Jeuk Kim]

On Wed, 09 Sep 2026 11:11:46 +0800, Stanley Jhu <stanleyjhu@google.com> wrote:
> diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
> index 4e22c31f89..ca5f6f2e96 100644
> --- a/hw/ufs/ufs.c
> +++ b/hw/ufs/ufs.c
> @@ -818,6 +822,111 @@ static void ufs_hce_reset(UfsHc *u)
> [ ... skip 63 lines ... ]
> +
> +                if (req) {
> +                    ufs_clear_req(req);
> +                    req->state = UFS_REQUEST_IDLE;
> +                    if (ufs_mcq_req(req)) {
> +                        QTAILQ_INSERT_TAIL(&req->sq->req_list, req, entry);

A successful UFS_ABORT_TASK does not complete MCQ host-side cleanup. 
The host driver will next issue SQRTCy.ICU, which must post an OCS = ABORTED CQE.
Please do not clear and recycle the request before that cleanup completes.

> @@ -880,10 +989,16 @@ static void ufs_write_reg(UfsHc *u, hwaddr offset, uint32_t data, unsigned size)
>      case A_MCQCONFIG:
>          u->reg.mcqconfig = data;
>          break;
> -    case A_UTRLCLR:
>      case A_UTMRLDBR:
> +        ufs_process_tmr(u, data);
> +        break;
>      case A_UTMRLCLR:
> +        u->reg.utmrldbr &= ~data;

UTMRLCLR is clear-on-zero, so this has the polarity backwards: zero clears the
corresponding UTMRLDBR bit and one leaves it unchanged.  This should be
`u->reg.utmrldbr &= data`.

-- 
Jeuk Kim <jeuk20.kim@samsung.com>

Re: [PATCH v2 1/3] hw/ufs: Track SCSIRequest and cancel pending requests in ufs_clear_req

[Jeuk Kim]

On Wed, 09 Sep 2026 11:11:44 +0800, Stanley Jhu <stanleyjhu@google.com> wrote:
> In the UFS emulator, outstanding SCSI requests are dispatched asynchronously
> to the block layer via scsi_req_enqueue(). When requests are cleared or
> aborted (e.g. during HCE reset or TMR aborts), ufs_clear_req() releases the
> scatter-gather list without cancelling the pending SCSIRequest. If an
> asynchronous AIO callback completes afterwards, ufs_scsi_command_complete()
> dereferences stale or freed request state, resulting in use-after-free
> hazards.

ufs_clear_req() currently releases request resources; it is not responsible
for cancelling in-flight SCSI requests. Its existing callers ensure that
the request is no longer in flight, so the current code does not have the
UAF described here. Please describe this as preparation for TMF support.

>
>
> diff --git a/hw/ufs/lu.c b/hw/ufs/lu.c
> index bdb1650851..a62ebb51fd 100644
> --- a/hw/ufs/lu.c
> +++ b/hw/ufs/lu.c
> @@ -173,6 +173,12 @@ static void ufs_scsi_command_complete(SCSIRequest *scsi_req, size_t resid)
>      int16_t status = scsi_req->status;
>      uint32_t transfered_len = scsi_req->cmd.xfer - resid;
>  
> +    if (!req) {
> +        return;
> +    }

Can we use assert(req != NULL) here? We only clear hba_private in the
completion and cancellation callbacks, so req should not be NULL here.
If that invariant is broken, it would be better to catch the bug with
an assertion than to silently return.

> @@ -190,6 +196,11 @@ static void ufs_scsi_command_complete(SCSIRequest *scsi_req, size_t resid)
>  
>  static void ufs_scsi_command_cancelled(SCSIRequest *scsi_req)
>  {
> +    UfsRequest *req = scsi_req->hba_private;
> +
> +    if (req) {

Same here: can we use assert(req != NULL)?

-- 
Jeuk Kim <jeuk20.kim@samsung.com>

Re: [PATCH v2 1/3] hw/ufs: Track SCSIRequest and cancel pending requests in ufs_clear_req

[Stanley Jhu]

On Wed, 23 Sep 2026 11:03:01 +0900, Jeuk Kim <jeuk20.kim@gmail.com> wrote:
> ufs_clear_req() currently releases request resources; it is not  
> responsible
> for cancelling in-flight SCSI requests. Its existing callers ensure that
> the request is no longer in flight, so the current code does not have the
> UAF described here. Please describe this as preparation for TMF support.

Agreed. Updated the commit message in v3 to frame this as preparation
for TMF support.

> > +    if (!req) {
> > +        return;
> > +    }

> Can we use assert(req != NULL) here? We only clear hba_private in the
> completion and cancellation callbacks, so req should not be NULL here.
> If that invariant is broken, it would be better to catch the bug with
> an assertion than to silently return.

> > @@ -190,6 +196,11 @@ static void ufs_scsi_command_complete(SCSIRequest  
> *scsi_req, size_t resid)
> >
> >  static void ufs_scsi_command_cancelled(SCSIRequest *scsi_req)
> >  {
> > +    UfsRequest *req = scsi_req->hba_private;
> > +
> > +    if (req) {

> Same here: can we use assert(req != NULL)?

Done in v3.

Re: [PATCH v2 2/3] hw/ufs: Support MCQ runtime interrupt and queue status registers

[Stanley Jhu]

On Wed, 23 Sep 2026 11:03:01 +0900, Jeuk Kim <jeuk20.kim@gmail.com> wrote:
> > - Store queue configuration for interrupt aggregation (CQnIACR) and  
> completion
> >   timeout intervals (SQnCTI).

> SQnCTI is Submission Queue Cleanup Task Information, not a completion
> timeout interval.

Fixed in v3.

> > +    case offsetof(UfsMcqOpReg, sq.rtc):
> > +        opr->sq.rtc = data;
> > +        if (FIELD_EX32(data, SQRTC, ICU)) {
> > +            /* SQ_ICU: Initiate Cleanup (SQ_CUS = 1, RTC = 0) */
> > +            opr->sq.rts = FIELD_DP32(opr->sq.rts, SQRTS, CUS, 1);
> > +            opr->sq.rts = FIELD_DP32(opr->sq.rts, SQRTS, RTC, 0);

> SQRTC.ICU reports successful cleanup without using SQCTI or changing any
> request state. Is the actual cleanup expected to be handled by the TMR  
> path?

Implemented actual cleanup in v3: `SQRTC.ICU` now matches the target
request via `SQCTI`, cancels any pending `SCSIRequest`, posts a CQE
with `OCS = UFS_OCS_ABORTED`, and recycles the request slot.

Re: [PATCH v2 3/3] hw/ufs: Implement Task Management Request (TMR) handling

[Stanley Jhu]

On Wed, 23 Sep 2026 11:03:01 +0900, Jeuk Kim <jeuk20.kim@gmail.com> wrote:
> > +                if (req) {
> > +                    ufs_clear_req(req);
> > +                    req->state = UFS_REQUEST_IDLE;
> > +                    if (ufs_mcq_req(req)) {
> > +                        QTAILQ_INSERT_TAIL(&req->sq->req_list, req,  
> entry);

> A successful UFS_ABORT_TASK does not complete MCQ host-side cleanup.
> The host driver will next issue SQRTCy.ICU, which must post an OCS =  
> ABORTED CQE.
> Please do not clear and recycle the request before that cleanup completes.

Agreed. In v3, `UFS_ABORT_TASK` in MCQ mode cancels the underlying
`SCSIRequest` and marks the request `UFS_REQUEST_ERROR`, deferring CQE
posting (`OCS = UFS_OCS_ABORTED`) and slot recycling to `SQRTC.ICU`.

> >      case A_UTMRLCLR:
> > +        u->reg.utmrldbr &= ~data;

> UTMRLCLR is clear-on-zero, so this has the polarity backwards: zero  
> clears the
> corresponding UTMRLDBR bit and one leaves it unchanged.  This should be
> `u->reg.utmrldbr &= data`.

Fixed in v3.