[PATCH v4 5/7] ufs: Make the logical block size configurable and answer absent LUNs

Mikail Sadic <[email protected]>
Newsgroups org.nongnu.qemu-devel,org.nongnu.qemu-arm
Message-ID <[email protected]>
Two things the ufs-lu model hardcodes that hardware does not.

The logical block size is fixed at UFS_BLOCK_SIZE (4096), both in the unit
descriptor the logical unit reports and in the block size of the scsi-hd
backing it. UFS does not require that: the unit descriptor carries a
base-2 exponent, and 512-byte blocks are common on real hardware and in
images built for it. Add a 'logical-block-size' property that sets both,
defaulting to UFS_BLOCK_SIZE so the ufs PCI device keeps the geometry and
the property set it has today. Restrict it to powers of two from 512 bytes
to UFS_BLOCK_SIZE, and reject a drive shorter than one block, which would
otherwise realize a unit reporting zero blocks.

An INQUIRY addressed to a logical unit that is not mapped fails the
request outright. SPC has a specific answer for this case: return the
standard INQUIRY data with the peripheral qualifier and device type saying
that no device is present on that logical unit, with GOOD status, so that
a host enumerating logical units can tell an absent unit from a transport
error. hw/scsi/scsi-bus.c already answers this way for a target's
unsupported logical units. Do the same for an unmapped ufs-lu, using
TYPE_NO_LUN. Commands other than a standard INQUIRY still fail, now with
LOGICAL UNIT NOT SUPPORTED sense data rather than an unadorned request
failure. The invalid-LUN trace point is unchanged.

Both are needed by the AST2700 UFS controller added later in this series:
the OpenBMC images it boots are laid out for 512-byte sectors, and U-Boot
logs an OCS failure for every unpopulated logical unit while probing it.

Signed-off-by: Mikail Sadic <[email protected]>
---
 hw/ufs/ufs.h |  3 +++
 hw/ufs/lu.c  | 67 +++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/ufs/ufs.c |  2 +-
 3 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/hw/ufs/ufs.h b/hw/ufs/ufs.h
index aa8361d93d..47d1c72ef3 100644
--- a/hw/ufs/ufs.h
+++ b/hw/ufs/ufs.h
@@ -21,6 +21,7 @@
 #define UFS_MAX_MCQ_QNUM 32
 #define UFS_BLOCK_SIZE_SHIFT 12
 #define UFS_BLOCK_SIZE (1 << UFS_BLOCK_SIZE_SHIFT)
+#define UFS_MIN_BLOCK_SIZE 512
 
 typedef struct UfsBusClass {
     BusClass parent_class;
@@ -80,6 +81,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;
@@ -303,6 +305,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);
 bool ufs_realize(UfsHc *u, DeviceState *dev, AddressSpace *dma_as,
                  Error **errp);
 void ufs_unrealize(UfsHc *u);
diff --git a/hw/ufs/lu.c b/hw/ufs/lu.c
index eeca865eb5..b1aba79a53 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;
 }
 
+/*
+ * A logical unit that is not mapped answers a standard INQUIRY as "not
+ * connected" with GOOD status, as hardware does, so that a host bus scan
+ * skips it instead of reporting a controller error. Any other command is
+ * rejected.
+ */
+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)) {
+        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));
@@ -455,6 +495,25 @@ static bool ufs_lu_check_constraints(UfsLu *lu, Error **errp)
         return false;
     }
 
+    if (!is_power_of_2(lu->logical_block_size)) {
+        error_setg(errp, "logical-block-size must be a power of 2, not %"
+                   PRIu32, lu->logical_block_size);
+        return false;
+    }
+
+    if (lu->logical_block_size < UFS_MIN_BLOCK_SIZE ||
+        lu->logical_block_size > UFS_BLOCK_SIZE) {
+        error_setg(errp, "logical-block-size must be between %d and %d bytes",
+                   UFS_MIN_BLOCK_SIZE, UFS_BLOCK_SIZE);
+        return false;
+    }
+
+    if (blk_getlength(lu->conf.blk) < lu->logical_block_size) {
+        error_setg(errp, "drive is smaller than one %" PRIu32 "-byte block",
+                   lu->logical_block_size);
+        return false;
+    }
+
     return true;
 }
 
@@ -475,8 +534,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)) {
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index 36c674af32..016876eb63 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -1083,7 +1083,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) {
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.