[Qemu-devel] [RFC PATCH 4/9] scsi: always use get_sense

[Paolo Bonzini]

vscsi and vmw_pvscsi support autosensing by providing sense data
directly in the response.  Previous patches added usage of get_sense,
but kept the older state machine approach that sent REQUEST SENSE
commands separately.  Remove it, from now on all SCSIDevices will
have to support autosensing (as well as REQUEST SENSE of course).

Signed-off-by: Paolo Bonzini <address@hidden>
---
 hw/spapr_vscsi.c |   90 ++++++++++++-----------------------------------------
 hw/vmw_pvscsi.c  |   92 +++++++++++++----------------------------------------
 2 files changed, 43 insertions(+), 139 deletions(-)

diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 1c901ef..57b1c09 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -80,7 +80,6 @@ typedef struct vscsi_req {
     int                     active;
     long                    data_len;
     int                     writing;
-    int                     sensing;
     int                     senselen;
     uint8_t                 sense[SCSI_SENSE_BUF_SIZE];
 
@@ -445,40 +444,6 @@ static int vscsi_preprocess_desc(vscsi_req *req)
     return 0;
 }
 
-static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
-{
-    uint8_t *cdb = req->iu.srp.cmd.cdb;
-    int n;
-
-    n = scsi_req_get_sense(req->sreq, req->sense, sizeof(req->sense));
-    if (n) {
-        req->senselen = n;
-        vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-        vscsi_put_req(s, req);
-        return;
-    }
-
-    dprintf("VSCSI: Got CHECK_CONDITION, requesting sense...\n");
-    cdb[0] = 3;
-    cdb[1] = 0;
-    cdb[2] = 0;
-    cdb[3] = 0;
-    cdb[4] = 96;
-    cdb[5] = 0;
-    req->sensing = 1;
-    n = scsi_req_enqueue(req->sreq, cdb);
-    dprintf("VSCSI: Queued request sense tag 0x%x\n", req->qtag);
-    if (n < 0) {
-        fprintf(stderr, "VSCSI: REQUEST_SENSE wants write data !?!?!?\n");
-        vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
-        scsi_req_abort(req->sreq, CHECK_CONDITION);
-        return;
-    } else if (n == 0) {
-        return;
-    }
-    scsi_req_continue(req->sreq);
-}
-
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
 static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len)
 {
@@ -494,23 +459,6 @@ static void vscsi_transfer_data(SCSIRequest *sreq, 
uint32_t len)
         return;
     }
 
-    if (req->sensing) {
-        uint8_t *buf = scsi_req_get_buf(sreq);
-
-        len = MIN(len, SCSI_SENSE_BUF_SIZE);
-        dprintf("VSCSI: Sense data, %d bytes:\n", len);
-        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
-                buf[0], buf[1], buf[2], buf[3],
-                buf[4], buf[5], buf[6], buf[7]);
-        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
-                buf[8], buf[9], buf[10], buf[11],
-                buf[12], buf[13], buf[14], buf[15]);
-        memcpy(req->sense, buf, len);
-        req->senselen = len;
-        scsi_req_continue(req->sreq);
-        return;
-    }
-
     if (len) {
         buf = scsi_req_get_buf(sreq);
         rc = vscsi_srp_transfer_data(s, req, req->writing, buf, len);
@@ -541,28 +489,30 @@ static void vscsi_command_complete(SCSIRequest *sreq, 
uint32_t status)
         return;
     }
 
-    if (!req->sensing && status == CHECK_CONDITION) {
-        vscsi_send_request_sense(s, req);
-        return;
+    if (status == CHECK_CONDITION) {
+        req->senselen = scsi_req_get_sense(req->sreq, req->sense,
+                                          sizeof(req->sense));
+        dprintf("VSCSI: Sense data, %d bytes:\n", len);
+        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
+                req->sense[0], req->sense[1], req->sense[2], req->sense[3],
+                req->sense[4], req->sense[5], req->sense[6], req->sense[7]);
+        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
+                req->sense[8], req->sense[9], req->sense[10], req->sense[11],
+                req->sense[12], req->sense[13], req->sense[14], 
req->sense[15]);
     }
 
-    if (req->sensing) {
-        dprintf("VSCSI: Sense done !\n");
-        status = CHECK_CONDITION;
-    } else {
-        dprintf("VSCSI: Command complete err=%d\n", status);
-        if (status == 0) {
-            /* We handle overflows, not underflows for normal commands,
-             * but hopefully nobody cares
-             */
-            if (req->writing) {
-                res_out = req->data_len;
-            } else {
-                res_in = req->data_len;
-            }
+    dprintf("VSCSI: Command complete err=%d\n", status);
+    if (status == 0) {
+        /* We handle overflows, not underflows for normal commands,
+         * but hopefully nobody cares
+         */
+        if (req->writing) {
+            res_out = req->data_len;
+        } else {
+            res_in = req->data_len;
         }
     }
-    vscsi_send_rsp(s, req, 0, res_in, res_out);
+    vscsi_send_rsp(s, req, status, res_in, res_out);
     vscsi_put_req(s, req);
 }
 
diff --git a/hw/vmw_pvscsi.c b/hw/vmw_pvscsi.c
index 84bcfe6..1cb6715 100644
--- a/hw/vmw_pvscsi.c
+++ b/hw/vmw_pvscsi.c
@@ -31,7 +31,6 @@ typedef struct PVSCSISGState {
 typedef struct PVSCSIRequest {
     SCSIDevice *sdev;
     SCSIRequest *sreq;
-    uint8_t sensing;
     uint8_t sense_key;
     uint8_t completed;
     int lun;
@@ -293,7 +292,6 @@ static void pvscsi_complete_req(PVSCSIState *s, 
PVSCSIRequest *p)
     qemu_bh_schedule(s->complete_reqs_bh);
 }
 
-/* Fetch sense data for a completed request.  */
 /* Write sense data for a completed request.  */
 static void pvscsi_write_sense(PVSCSIRequest *p, uint8_t *buf, int len)
 {
@@ -302,29 +300,6 @@ static void pvscsi_write_sense(PVSCSIRequest *p, uint8_t 
*buf, int len)
     cpu_physical_memory_write(p->req.senseAddr, buf, p->cmp.senseLen);
 }
 
-static bool pvscsi_send_request_sense(PVSCSIRequest *p)
-{
-    uint8_t cdb[6] = { 3, p->lun << 5, 0, 0, 96, 0 };
-    uint8_t sense[96];
-    int n;
-
-    n = scsi_req_get_sense(p->sreq, sense, sizeof(sense));
-    if (n) {
-        pvscsi_write_sense(p, sense, n);
-        return false;
-    }
-
-    trace_pvscsi_request_sense(p->sreq->tag, p->lun);
-    n = scsi_req_enqueue(p->sreq, cdb);
-    if (n < 0) {
-        /* should not happen, just leave sense data empty in this case. */
-        scsi_req_cancel(p->sreq);
-    } else if (n > 0) {
-        scsi_req_continue(p->sreq);
-    }
-    return true;
-}
-
 static void pvscsi_transfer_data_with_buffer(PVSCSIRequest *p, bool to_host,
                                              uint8_t *buf, int len)
 {
@@ -418,48 +393,33 @@ static void pvscsi_transfer_data(SCSIRequest *req, 
uint32_t len)
     PVSCSIState *s = DO_UPCAST(PVSCSIState, dev.qdev, req->bus->qbus.parent);
     PVSCSIRequest *p = pvscsi_find_request(s, req);
     uint8_t *buf = scsi_req_get_buf(req);
+    int to_host = (p->req.flags & PVSCSI_FLAG_CMD_DIR_TOHOST) != 0;
 
     if (!p) {
         fprintf(stderr, "PVSCSI: Can't find request for tag 0x%x\n", req->tag);
         return;
     }
 
-    if (!p->sensing) {
-        int to_host = (p->req.flags & PVSCSI_FLAG_CMD_DIR_TOHOST) != 0;
-
-        assert(p->resid);
-        trace_pvscsi_transfer_data(p->req.context, len);
-        if (!len) {
-            /* Short transfer.  */
-            p->cmp.hostStatus = BTSTAT_DATARUN;
-            scsi_req_cancel(req);
-            return;
-        }
-
-        if (len > p->resid) {
-            /* Small buffer.  */
-            p->cmp.hostStatus = BTSTAT_DATARUN;
-            scsi_req_cancel(req);
-            return;
-        }
+    assert(p->resid);
+    trace_pvscsi_transfer_data(p->req.context, len);
+    if (!len) {
+        /* Short transfer.  */
+        p->cmp.hostStatus = BTSTAT_DATARUN;
+        scsi_req_cancel(req);
+        return;
+    }
 
-        if (p->req.flags & PVSCSI_FLAG_CMD_WITH_SG_LIST) {
-            pvscsi_transfer_data_with_sg_list(p, to_host, buf, len);
-        } else {
-            pvscsi_transfer_data_with_buffer(p, to_host, buf, len);
-        }
+    if (len > p->resid) {
+        /* Small buffer.  */
+        p->cmp.hostStatus = BTSTAT_DATARUN;
+        scsi_req_cancel(req);
+        return;
     }
 
-    else if (p->sensing == 1) {
-        /* Got sense data.  Write it back and kick the device to complete
-         * the request.  */
-        if (len) {
-            pvscsi_write_sense(p, buf, len);
-            if (buf[2] == NO_SENSE) {
-                p->cmp.scsiStatus = GOOD;
-            }
-        }
-        p->sensing = 2;
+    if (p->req.flags & PVSCSI_FLAG_CMD_WITH_SG_LIST) {
+        pvscsi_transfer_data_with_sg_list(p, to_host, buf, len);
+    } else {
+        pvscsi_transfer_data_with_buffer(p, to_host, buf, len);
     }
 
     scsi_req_continue(req);
@@ -476,18 +436,12 @@ static void pvscsi_command_complete(SCSIRequest *req, 
uint32_t status)
         return;
     }
 
-    /* Here to complete the request.  */
-    if (!p->sensing) {
-        p->cmp.scsiStatus = status;
-
-        if (p->cmp.scsiStatus == CHECK_CONDITION) {
-            p->sensing = 1;
-            if (pvscsi_send_request_sense(p)) {
-                return;
-            }
-        }
+    p->cmp.scsiStatus = status;
+    if (p->cmp.scsiStatus == CHECK_CONDITION) {
+       uint8_t sense[96];
+        int n = scsi_req_get_sense(p->sreq, sense, sizeof(sense));
+        pvscsi_write_sense(p, sense, n);
     }
-
     pvscsi_complete_req(s, p);
 }
 
-- 
1.7.4.4