[PATCH v2 6/8] ufs: Refactor common core out of the PCI implementation

Mikail Sadic <[email protected]> Thu, 30 Jul 2026 15:09:41 -0500
Newsgroups org.nongnu.qemu-arm,org.nongnu.qemu-devel
Message-ID <[email protected]>
In preparation for a sysbus UFS frontend (used by the Aspeed AST2700),
separate the transport-independent UFS host-controller logic from the
PCI-specific glue, following the pattern used by SDHCI and XHCI.

UfsHc's PCIDevice parent is replaced by an anonymous union of a
PCIDevice and a SysBusDevice, and the controller gains an explicit
AddressSpace *dma_as used by ufs_addr_read()/ufs_addr_write() and the
SG-list helpers instead of the PCI address space. Interrupt delivery
goes through qemu_set_irq() in ufs_irq_check(). The realize path is
split into a transport-agnostic ufs_realize_core() plus ufs_init_mmio(),
and UfsBus gains an 'hc' back-pointer so ufs_lu_realize() no longer
reaches through the PCI device. TYPE_UFS remains a PCI device, so
'-device ufs' is unchanged and the PCI ufs-tests qtests still pass.

Two small behaviour additions ride along on the core so that frontends
with differing geometry and bus-scan behaviour can be modelled:

  - ufs-lu gains configurable 'logical-block-size' property. It
    defaults to UFS_BLOCK_SIZE, so the PCI device is unaffected.

  - an unmapped LUN now answers INQUIRY as "not connected" rather than
    failing the request, so that a host bus-scan does not observe
    controller errors for absent LUNs.

Signed-off-by: Mikail Sadic <[email protected]>
---
 hw/ufs/ufs.h | 18 ++++++++++++++++-
 hw/ufs/lu.c  | 50 +++++++++++++++++++++++++++++++++++++++++----
 hw/ufs/ufs.c | 57 ++++++++++++++++++++++++++++++++++++----------------
 3 files changed, 103 insertions(+), 22 deletions(-)

diff --git a/hw/ufs/ufs.h b/hw/ufs/ufs.h
index feb47f460d..3d4be8deb5 100644
--- a/hw/ufs/ufs.h
+++ b/hw/ufs/ufs.h
@@ -12,7 +12,9 @@
 #define HW_UFS_UFS_H
 
 #include "hw/pci/pci_device.h"
+#include "hw/core/sysbus.h"
 #include "hw/scsi/scsi.h"
+#include "scsi/constants.h"
 #include "block/ufs.h"
 
 #define UFS_MAX_LUS 32
@@ -27,6 +29,7 @@ typedef struct UfsBusClass {
 
 typedef struct UfsBus {
     BusState parent_bus;
+    struct UfsHc *hc; /* host controller owning this bus (frontend-agnostic) */
 } UfsBus;
 
 #define TYPE_UFS_BUS "ufs-bus"
@@ -77,6 +80,7 @@ typedef UfsReqResult (*UfsScsiOp)(struct UfsLu *, UfsRequest *);
 typedef struct UfsLu {
     DeviceState qdev;
     uint8_t lun;
+    uint32_t logical_block_size;
     UnitDescriptor unit_desc;
     SCSIBus bus;
     SCSIDevice *scsi_dev;
@@ -141,7 +145,15 @@ typedef struct UfsWb {
 } UfsWb;
 
 typedef struct UfsHc {
-    PCIDevice parent_obj;
+    /*
+     * The controller can be instantiated either as a PCI function or as a
+     * sysbus device.  Following the SDHCI model, both frontends share this
+     * state structure via a union of the possible parent objects.
+     */
+    union {
+        PCIDevice pci_dev;
+        SysBusDevice sbdev;
+    };
     UfsBus bus;
     MemoryRegion iomem;
     UfsReg reg;
@@ -162,6 +174,7 @@ typedef struct UfsHc {
     Flags flags;
 
     qemu_irq irq;
+    AddressSpace *dma_as;
     QEMUBH *doorbell_bh;
     QEMUBH *complete_bh;
 
@@ -302,4 +315,7 @@ void ufs_build_query_response(UfsRequest *req);
 void ufs_complete_req(UfsRequest *req, UfsReqResult req_result);
 void ufs_wb_update_avail_buffer(UfsHc *u);
 void ufs_init_wlu(UfsLu *wlu, uint8_t wlun);
+UfsReqResult ufs_emulate_absent_lun(UfsRequest *req);
+void ufs_init_mmio(UfsHc *u);
+bool ufs_realize_core(UfsHc *u, Error **errp);
 #endif /* HW_UFS_UFS_H */
diff --git a/hw/ufs/lu.c b/hw/ufs/lu.c
index 13f4a90145..a0b5574668 100644
--- a/hw/ufs/lu.c
+++ b/hw/ufs/lu.c
@@ -308,6 +308,44 @@ static int ufs_emulate_wlun_inquiry(UfsRequest *req, uint8_t *outbuf,
     return SCSI_INQUIRY_LEN;
 }
 
+UfsReqResult ufs_emulate_absent_lun(UfsRequest *req)
+{
+    QEMU_UNINITIALIZED uint8_t outbuf[SCSI_INQUIRY_LEN];
+    uint8_t sense_buf[UFS_SENSE_SIZE];
+    uint8_t scsi_status;
+    int len = 0;
+
+    if (req->req_upiu.sc.cdb[0] == INQUIRY &&
+        !(req->req_upiu.sc.cdb[1] & 0x1)) {
+        /*
+         * Standard INQUIRY to a logical unit that is not mapped: report it
+         * as "not connected" (peripheral qualifier 0x3, device type 0x1f)
+         * with GOOD status, like real hardware. Host-side bus scans then
+         * skip the unit quietly instead of flagging a controller error.
+         */
+        memset(outbuf, 0, sizeof(outbuf));
+        outbuf[0] = TYPE_NO_LUN;
+        outbuf[3] = 0x2;
+        outbuf[4] = SCSI_INQUIRY_LEN - 5;
+        len = SCSI_INQUIRY_LEN;
+        scsi_status = GOOD;
+    } else {
+        scsi_build_sense(sense_buf, SENSE_CODE(LUN_NOT_SUPPORTED));
+        scsi_status = CHECK_CONDITION;
+    }
+
+    len = MIN(len, (int)req->data_len);
+    if (scsi_status == GOOD && len > 0 &&
+        dma_buf_read(outbuf, len, NULL, req->sg, MEMTXATTRS_UNSPECIFIED) !=
+            MEMTX_OK) {
+        return UFS_REQUEST_FAIL;
+    }
+
+    ufs_build_scsi_response_upiu(req, sense_buf, sizeof(sense_buf), len,
+                                 scsi_status);
+    return UFS_REQUEST_SUCCESS;
+}
+
 static UfsReqResult ufs_emulate_scsi_cmd(UfsLu *lu, UfsRequest *req)
 {
     uint8_t lun = lu->lun;
@@ -394,6 +432,8 @@ static UfsReqResult ufs_process_scsi_cmd(UfsLu *lu, UfsRequest *req)
 static const Property ufs_lu_props[] = {
     DEFINE_PROP_DRIVE("drive", UfsLu, conf.blk),
     DEFINE_PROP_UINT8("lun", UfsLu, lun, 0),
+    DEFINE_PROP_UINT32("logical-block-size", UfsLu, logical_block_size,
+                       UFS_BLOCK_SIZE),
 };
 
 static bool ufs_add_lu(UfsHc *u, UfsLu *lu, Error **errp)
@@ -435,7 +475,7 @@ static void ufs_init_lu(UfsLu *lu)
     lu->unit_desc.length = sizeof(UnitDescriptor);
     lu->unit_desc.descriptor_idn = UFS_QUERY_DESC_IDN_UNIT;
     lu->unit_desc.lu_enable = 0x01;
-    lu->unit_desc.logical_block_size = UFS_BLOCK_SIZE_SHIFT;
+    lu->unit_desc.logical_block_size = ctz32(lu->logical_block_size);
     lu->unit_desc.unit_index = lu->lun;
     lu->unit_desc.logical_block_count =
         cpu_to_be64(brdv_len / (1 << lu->unit_desc.logical_block_size));
@@ -475,8 +515,10 @@ static void ufs_init_scsi_device(UfsLu *lu, BlockBackend *blk, Error **errp)
     scsi_dev = qdev_new("scsi-hd");
     object_property_add_child(OBJECT(&lu->bus), "ufs-scsi", OBJECT(scsi_dev));
 
-    qdev_prop_set_uint32(scsi_dev, "physical_block_size", UFS_BLOCK_SIZE);
-    qdev_prop_set_uint32(scsi_dev, "logical_block_size", UFS_BLOCK_SIZE);
+    qdev_prop_set_uint32(scsi_dev, "physical_block_size",
+                         lu->logical_block_size);
+    qdev_prop_set_uint32(scsi_dev, "logical_block_size",
+                         lu->logical_block_size);
     qdev_prop_set_uint32(scsi_dev, "scsi-id", 0);
     qdev_prop_set_uint32(scsi_dev, "lun", lu->lun);
     if (!qdev_prop_set_drive_err(scsi_dev, "drive", blk, errp)) {
@@ -497,7 +539,7 @@ static void ufs_lu_realize(DeviceState *dev, Error **errp)
 {
     UfsLu *lu = DO_UPCAST(UfsLu, qdev, dev);
     BusState *s = qdev_get_parent_bus(dev);
-    UfsHc *u = UFS(s->parent);
+    UfsHc *u = UFS_BUS(s)->hc;
     BlockBackend *blk = lu->conf.blk;
 
     if (!ufs_lu_check_constraints(lu, errp)) {
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index 464fd465b3..1489bd3321 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -102,7 +102,8 @@ static MemTxResult ufs_addr_read(UfsHc *u, hwaddr addr, void *buf, int size)
         return MEMTX_DECODE_ERROR;
     }
 
-    return pci_dma_read(PCI_DEVICE(u), addr, buf, size);
+    return dma_memory_read(u->dma_as, addr, buf, size,
+                           MEMTXATTRS_UNSPECIFIED);
 }
 
 static MemTxResult ufs_addr_write(UfsHc *u, hwaddr addr, const void *buf,
@@ -117,7 +118,8 @@ static MemTxResult ufs_addr_write(UfsHc *u, hwaddr addr, const void *buf,
         return MEMTX_DECODE_ERROR;
     }
 
-    return pci_dma_write(PCI_DEVICE(u), addr, buf, size);
+    return dma_memory_write(u->dma_as, addr, buf, size,
+                            MEMTXATTRS_UNSPECIFIED);
 }
 
 static inline hwaddr ufs_get_utrd_addr(UfsHc *u, uint32_t slot)
@@ -222,7 +224,7 @@ static MemTxResult ufs_dma_read_prdt(UfsRequest *req)
     }
 
     req->sg = g_malloc0(sizeof(QEMUSGList));
-    pci_dma_sglist_init(req->sg, PCI_DEVICE(u), prdt_len);
+    qemu_sglist_init(req->sg, DEVICE(u), prdt_len, u->dma_as);
     req->data_len = 0;
 
     for (uint16_t i = 0; i < prdt_len; ++i) {
@@ -317,14 +319,12 @@ static MemTxResult ufs_dma_write_upiu(UfsRequest *req)
 
 static void ufs_irq_check(UfsHc *u)
 {
-    PCIDevice *pci = PCI_DEVICE(u);
-
     if ((u->reg.is & UFS_INTR_MASK) & u->reg.ie) {
         trace_ufs_irq_raise();
-        pci_irq_assert(pci);
+        qemu_set_irq(u->irq, 1);
     } else {
         trace_ufs_irq_lower();
-        pci_irq_deassert(pci);
+        qemu_set_irq(u->irq, 0);
     }
 }
 
@@ -1092,7 +1092,7 @@ static UfsReqResult ufs_exec_scsi_cmd(UfsRequest *req)
 
     if (!is_wlun(lun) && (lun >= UFS_MAX_LUS || u->lus[lun] == NULL)) {
         trace_ufs_err_scsi_cmd_invalid_lun(lun);
-        return UFS_REQUEST_FAIL;
+        return ufs_emulate_absent_lun(req);
     }
 
     switch (lun) {
@@ -2488,6 +2488,12 @@ static bool ufs_check_constraints(UfsHc *u, Error **errp)
     return true;
 }
 
+void ufs_init_mmio(UfsHc *u)
+{
+    memory_region_init_io(&u->iomem, OBJECT(u), &ufs_mmio_ops, u, "ufs",
+                          u->reg_size);
+}
+
 static void ufs_init_pci(UfsHc *u, PCIDevice *pci_dev)
 {
     uint8_t *pci_conf = pci_dev->config;
@@ -2495,8 +2501,7 @@ static void ufs_init_pci(UfsHc *u, PCIDevice *pci_dev)
     pci_conf[PCI_INTERRUPT_PIN] = 1;
     pci_config_set_prog_interface(pci_conf, 0x1);
 
-    memory_region_init_io(&u->iomem, OBJECT(u), &ufs_mmio_ops, u, "ufs",
-                          u->reg_size);
+    ufs_init_mmio(u);
     pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &u->iomem);
     u->irq = pci_allocate_irq(pci_dev);
 }
@@ -2689,25 +2694,43 @@ static void ufs_init_hc(UfsHc *u)
     timer_mod(&u->idle_timer, now + UFS_IDLE_TIMER_TICK);
 }
 
-static void ufs_realize(PCIDevice *pci_dev, Error **errp)
+/*
+ * Frontend-agnostic realization: everything except the host-bus specific
+ * MMIO/IRQ/DMA plumbing, which the PCI or sysbus frontend sets up around
+ * this call.  The frontend must assign u->dma_as before calling.
+ */
+bool ufs_realize_core(UfsHc *u, Error **errp)
 {
-    UfsHc *u = UFS(pci_dev);
-
     if (!ufs_check_constraints(u, errp)) {
-        return;
+        return false;
     }
 
-    qbus_init(&u->bus, sizeof(UfsBus), TYPE_UFS_BUS, &pci_dev->qdev,
-              u->parent_obj.qdev.id);
+    qbus_init(&u->bus, sizeof(UfsBus), TYPE_UFS_BUS, DEVICE(u),
+              DEVICE(u)->id);
+    u->bus.hc = u;
 
     ufs_init_state(u);
     ufs_init_hc(u);
-    ufs_init_pci(u, pci_dev);
 
     ufs_init_wlu(&u->report_wlu, UFS_UPIU_REPORT_LUNS_WLUN);
     ufs_init_wlu(&u->dev_wlu, UFS_UPIU_UFS_DEVICE_WLUN);
     ufs_init_wlu(&u->boot_wlu, UFS_UPIU_BOOT_WLUN);
     ufs_init_wlu(&u->rpmb_wlu, UFS_UPIU_RPMB_WLUN);
+
+    return true;
+}
+
+static void ufs_realize(PCIDevice *pci_dev, Error **errp)
+{
+    UfsHc *u = UFS(pci_dev);
+
+    u->dma_as = pci_get_address_space(pci_dev);
+
+    if (!ufs_realize_core(u, errp)) {
+        return;
+    }
+
+    ufs_init_pci(u, pci_dev);
 }
 
 static void ufs_exit(PCIDevice *pci_dev)
-- 
2.53.0