scsi: Update sense code handling

The SCSI spec has a quite detailed list of sense codes available.
It even mandates the use of specific ones for some failure cases.
The current implementation just has one type of generic error
which is actually a violation of the spec in certain cases.
This patch introduces various predefined sense codes to have the
sense code reporting more in line with the spec.

On top of Hannes's patch I fixed the reply to REQUEST SENSE commands
with DESC=0 and a small (<18) length.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>

hw/scsi-bus.c

@@ -154,7 +154,7 @@ void scsi_req_enqueue(SCSIRequest *req)
     QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
 }
 
-void scsi_req_dequeue(SCSIRequest *req)
+static void scsi_req_dequeue(SCSIRequest *req)
 {
     trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag);
     if (req->enqueued) {
@@ -391,6 +391,95 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
     return 0;
 }
 
+/*
+ * Predefined sense codes
+ */
+
+/* No sense data available */
+const struct SCSISense sense_code_NO_SENSE = {
+    .key = NO_SENSE , .asc = 0x00 , .ascq = 0x00
+};
+
+/* LUN not ready, Manual intervention required */
+const struct SCSISense sense_code_LUN_NOT_READY = {
+    .key = NOT_READY, .asc = 0x04, .ascq = 0x03
+};
+
+/* LUN not ready, Medium not present */
+const struct SCSISense sense_code_NO_MEDIUM = {
+    .key = NOT_READY, .asc = 0x3a, .ascq = 0x00
+};
+
+/* Hardware error, internal target failure */
+const struct SCSISense sense_code_TARGET_FAILURE = {
+    .key = HARDWARE_ERROR, .asc = 0x44, .ascq = 0x00
+};
+
+/* Illegal request, invalid command operation code */
+const struct SCSISense sense_code_INVALID_OPCODE = {
+    .key = ILLEGAL_REQUEST, .asc = 0x20, .ascq = 0x00
+};
+
+/* Illegal request, LBA out of range */
+const struct SCSISense sense_code_LBA_OUT_OF_RANGE = {
+    .key = ILLEGAL_REQUEST, .asc = 0x21, .ascq = 0x00
+};
+
+/* Illegal request, Invalid field in CDB */
+const struct SCSISense sense_code_INVALID_FIELD = {
+    .key = ILLEGAL_REQUEST, .asc = 0x24, .ascq = 0x00
+};
+
+/* Illegal request, LUN not supported */
+const struct SCSISense sense_code_LUN_NOT_SUPPORTED = {
+    .key = ILLEGAL_REQUEST, .asc = 0x25, .ascq = 0x00
+};
+
+/* Command aborted, I/O process terminated */
+const struct SCSISense sense_code_IO_ERROR = {
+    .key = ABORTED_COMMAND, .asc = 0x00, .ascq = 0x06
+};
+
+/* Command aborted, I_T Nexus loss occurred */
+const struct SCSISense sense_code_I_T_NEXUS_LOSS = {
+    .key = ABORTED_COMMAND, .asc = 0x29, .ascq = 0x07
+};
+
+/* Command aborted, Logical Unit failure */
+const struct SCSISense sense_code_LUN_FAILURE = {
+    .key = ABORTED_COMMAND, .asc = 0x3e, .ascq = 0x01
+};
+
+/*
+ * scsi_build_sense
+ *
+ * Build a sense buffer
+ */
+int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed)
+{
+    if (!fixed && len < 8) {
+        return 0;
+    }
+
+    memset(buf, 0, len);
+    if (fixed) {
+        /* Return fixed format sense buffer */
+        buf[0] = 0xf0;
+        buf[2] = sense.key;
+        buf[7] = 7;
+        buf[12] = sense.asc;
+        buf[13] = sense.ascq;
+        return MIN(len, 18);
+    } else {
+        /* Return descriptor format sense buffer */
+        buf[0] = 0x72;
+        buf[1] = sense.key;
+        buf[2] = sense.asc;
+        buf[3] = sense.ascq;
+        return 8;
+    }
+}
+
 static const char *scsi_command_name(uint8_t cmd)
 {
     static const char *names[] = {

hw/scsi-disk.c

@@ -49,10 +49,6 @@ do { fprintf(stderr, "scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 
 typedef struct SCSIDiskState SCSIDiskState;
 
-typedef struct SCSISense {
-    uint8_t key;
-} SCSISense;
-
 typedef struct SCSIDiskReq {
     SCSIRequest req;
     /* ??? We should probably keep track of whether the data transfer is
@@ -111,24 +107,19 @@ static void scsi_disk_clear_sense(SCSIDiskState *s)
     memset(&s->sense, 0, sizeof(s->sense));
 }
 
-static void scsi_disk_set_sense(SCSIDiskState *s, uint8_t key)
-{
-    s->sense.key = key;
-}
-
-static void scsi_req_set_status(SCSIDiskReq *r, int status, int sense_code)
+static void scsi_req_set_status(SCSIDiskReq *r, int status, SCSISense sense)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
     r->req.status = status;
-    scsi_disk_set_sense(s, sense_code);
+    s->sense = sense;
 }
 
 /* Helper function for command completion.  */
-static void scsi_command_complete(SCSIDiskReq *r, int status, int sense)
+static void scsi_command_complete(SCSIDiskReq *r, int status, SCSISense sense)
 {
-    DPRINTF("Command complete tag=0x%x status=%d sense=%d\n",
-            r->req.tag, status, sense);
+    DPRINTF("Command complete tag=0x%x status=%d sense=%d/%d/%d\n",
+            r->req.tag, status, sense.key, sense.asc, sense.ascq);
     scsi_req_set_status(r, status, sense);
     scsi_req_complete(&r->req);
 }
@@ -182,7 +173,7 @@ static void scsi_read_data(SCSIRequest *req)
     }
     DPRINTF("Read sector_count=%d\n", r->sector_count);
     if (r->sector_count == 0) {
-        scsi_command_complete(r, GOOD, NO_SENSE);
+        scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
         return;
     }
 
@@ -225,8 +216,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
         if (type == SCSI_REQ_STATUS_RETRY_READ) {
             scsi_req_data(&r->req, 0);
         }
-        scsi_command_complete(r, CHECK_CONDITION,
-                HARDWARE_ERROR);
+        if (error == ENOMEM) {
+            scsi_command_complete(r, CHECK_CONDITION,
+                                  SENSE_CODE(TARGET_FAILURE));
+        } else {
+            scsi_command_complete(r, CHECK_CONDITION,
+                                  SENSE_CODE(IO_ERROR));
+        }
         bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
     }
 
@@ -251,7 +247,7 @@ static void scsi_write_complete(void * opaque, int ret)
     r->sector += n;
     r->sector_count -= n;
     if (r->sector_count == 0) {
-        scsi_command_complete(r, GOOD, NO_SENSE);
+        scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
     } else {
         len = r->sector_count * 512;
         if (len > SCSI_DMA_BUF_SIZE) {
@@ -278,7 +274,7 @@ static int scsi_write_data(SCSIRequest *req)
         r->req.aiocb = bdrv_aio_writev(s->bs, r->sector, &r->qiov, n,
                                    scsi_write_complete, r);
         if (r->req.aiocb == NULL) {
-            scsi_write_complete(r, -EIO);
+            scsi_write_complete(r, -ENOMEM);
         }
     } else {
         /* Invoke completion routine to fetch data from host.  */
@@ -316,7 +312,7 @@ static void scsi_dma_restart_bh(void *opaque)
             case SCSI_REQ_STATUS_RETRY_FLUSH:
                 ret = scsi_disk_emulate_command(r, r->iov.iov_base);
                 if (ret == 0) {
-                    scsi_command_complete(r, GOOD, NO_SENSE);
+                    scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
                 }
             }
         }
@@ -815,19 +811,8 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
     case REQUEST_SENSE:
         if (req->cmd.xfer < 4)
             goto illegal_request;
-        memset(outbuf, 0, 4);
-        buflen = 4;
-        if (s->sense.key == NOT_READY && req->cmd.xfer >= 18) {
-            memset(outbuf, 0, 18);
-            buflen = 18;
-            outbuf[7] = 10;
-            /* asc 0x3a, ascq 0: Medium not present */
-            outbuf[12] = 0x3a;
-            outbuf[13] = 0;
-        }
-        outbuf[0] = 0xf0;
-        outbuf[1] = 0;
-        outbuf[2] = s->sense.key;
+        buflen = scsi_build_sense(s->sense, outbuf, req->cmd.xfer,
+                                  req->cmd.xfer > 13);
         scsi_disk_clear_sense(s);
         break;
     case INQUIRY:
@@ -965,17 +950,22 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
         }
         break;
     default:
-        goto illegal_request;
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+        return -1;
     }
-    scsi_req_set_status(r, GOOD, NO_SENSE);
+    scsi_req_set_status(r, GOOD, SENSE_CODE(NO_SENSE));
     return buflen;
 
 not_ready:
-    scsi_command_complete(r, CHECK_CONDITION, NOT_READY);
+    if (!bdrv_is_inserted(s->bs)) {
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(NO_MEDIUM));
+    } else {
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(LUN_NOT_READY));
+    }
     return -1;
 
 illegal_request:
-    scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST);
+    scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_FIELD));
     return -1;
 }
 
@@ -1002,7 +992,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
 
     if (scsi_req_parse(&r->req, buf) != 0) {
         BADF("Unsupported command length, command %x\n", command);
-        goto fail;
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+        return 0;
     }
 #ifdef DEBUG_SCSI
     {
@@ -1017,8 +1008,11 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
     if (req->lun || buf[1] >> 5) {
         /* Only LUN 0 supported.  */
         DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : buf[1] >> 5);
-        if (command != REQUEST_SENSE && command != INQUIRY)
-            goto fail;
+        if (command != REQUEST_SENSE && command != INQUIRY) {
+            scsi_command_complete(r, CHECK_CONDITION,
+                                  SENSE_CODE(LUN_NOT_SUPPORTED));
+            return 0;
+        }
     }
     switch (command) {
     case TEST_UNIT_READY:
@@ -1126,15 +1120,17 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
         break;
     default:
         DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+        return 0;
     fail:
-        scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST);
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_FIELD));
         return 0;
     illegal_lba:
-        scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR);
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(LBA_OUT_OF_RANGE));
         return 0;
     }
     if (r->sector_count == 0 && r->iov.iov_len == 0) {
-        scsi_command_complete(r, GOOD, NO_SENSE);
+        scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
     }
     len = r->sector_count * 512 + r->iov.iov_len;
     if (is_write) {

hw/scsi-generic.c

@@ -66,6 +66,19 @@ struct SCSIGenericState
     uint8_t senselen;
 };
 
+static void scsi_set_sense(SCSIGenericState *s, SCSISense sense)
+{
+    s->senselen = scsi_build_sense(sense, s->sensebuf, SCSI_SENSE_BUF_SIZE, 0);
+    s->driver_status = SG_ERR_DRIVER_SENSE;
+}
+
+static void scsi_clear_sense(SCSIGenericState *s)
+{
+    memset(s->sensebuf, 0, SCSI_SENSE_BUF_SIZE);
+    s->senselen = 0;
+    s->driver_status = 0;
+}
+
 static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
 {
     SCSIRequest *req;
@@ -92,9 +105,22 @@ static void scsi_command_complete(void *opaque, int ret)
     if (s->driver_status & SG_ERR_DRIVER_SENSE)
         s->senselen = r->io_header.sb_len_wr;
 
-    if (ret != 0)
-        r->req.status = BUSY;
-    else {
+    if (ret != 0) {
+        switch (ret) {
+        case -EINVAL:
+            r->req.status = CHECK_CONDITION;
+            scsi_set_sense(s, SENSE_CODE(INVALID_FIELD));
+            break;
+        case -ENOMEM:
+            r->req.status = CHECK_CONDITION;
+            scsi_set_sense(s, SENSE_CODE(TARGET_FAILURE));
+            break;
+        default:
+            r->req.status = CHECK_CONDITION;
+            scsi_set_sense(s, SENSE_CODE(IO_ERROR));
+            break;
+        }
+    } else {
         if (s->driver_status & SG_ERR_DRIVER_TIMEOUT) {
             r->req.status = BUSY;
             BADF("Driver Timeout\n");
@@ -144,7 +170,7 @@ static int execute_command(BlockDriverState *bdrv,
     r->req.aiocb = bdrv_aio_ioctl(bdrv, SG_IO, &r->io_header, complete, r);
     if (r->req.aiocb == NULL) {
         BADF("execute_command: read failed !\n");
-        return -1;
+        return -ENOMEM;
     }
 
     return 0;
@@ -198,12 +224,14 @@ static void scsi_read_data(SCSIRequest *req)
                 r->buf[0], r->buf[1], r->buf[2], r->buf[3],
                 r->buf[4], r->buf[5], r->buf[6], r->buf[7]);
         scsi_req_data(&r->req, s->senselen);
+        /* Clear sensebuf after REQUEST_SENSE */
+        scsi_clear_sense(s);
         return;
     }
 
     ret = execute_command(s->bs, r, SG_DXFER_FROM_DEV, scsi_read_complete);
-    if (ret == -1) {
-        scsi_command_complete(r, -EINVAL);
+    if (ret < 0) {
+        scsi_command_complete(r, ret);
         return;
     }
 }
@@ -246,8 +274,8 @@ static int scsi_write_data(SCSIRequest *req)
     }
 
     ret = execute_command(s->bs, r, SG_DXFER_TO_DEV, scsi_write_complete);
-    if (ret == -1) {
-        scsi_command_complete(r, -EINVAL);
+    if (ret < 0) {
+        scsi_command_complete(r, ret);
         return 1;
     }
 
@@ -296,16 +324,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
     if (cmd[0] != REQUEST_SENSE &&
         (req->lun != s->lun || (cmd[1] >> 5) != s->lun)) {
         DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : cmd[1] >> 5);
-
-        s->sensebuf[0] = 0x70;
-        s->sensebuf[1] = 0x00;
-        s->sensebuf[2] = ILLEGAL_REQUEST;
-        s->sensebuf[3] = 0x00;
-        s->sensebuf[4] = 0x00;
-        s->sensebuf[5] = 0x00;
-        s->sensebuf[6] = 0x00;
-        s->senselen = 7;
-        s->driver_status = SG_ERR_DRIVER_SENSE;
+        scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
         r->req.status = CHECK_CONDITION;
         scsi_req_complete(&r->req);
         return 0;
@@ -313,8 +332,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
 
     if (-1 == scsi_req_parse(&r->req, cmd)) {
         BADF("Unsupported command length, command %x\n", cmd[0]);
-        scsi_req_dequeue(&r->req);
-        scsi_req_unref(&r->req);
+        scsi_command_complete(r, -EINVAL);
         return 0;
     }
     scsi_req_fixup(&r->req);
@@ -338,8 +356,9 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
         r->buflen = 0;
         r->buf = NULL;
         ret = execute_command(s->bs, r, SG_DXFER_NONE, scsi_command_complete);
-        if (ret == -1) {
-            scsi_command_complete(r, -EINVAL);
+        if (ret < 0) {
+            scsi_command_complete(r, ret);
+            return 0;
         }
         return 0;
     }

hw/scsi.h

@@ -27,6 +27,12 @@ enum SCSIXferMode {
     SCSI_XFER_TO_DEV,    /*  WRITE, MODE_SELECT, ...         */
 };
 
+typedef struct SCSISense {
+    uint8_t key;
+    uint8_t asc;
+    uint8_t ascq;
+} SCSISense;
+
 struct SCSIRequest {
     SCSIBus           *bus;
     SCSIDevice        *dev;
@@ -104,10 +110,41 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
                                       int unit, bool removable);
 int scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
 
+/*
+ * Predefined sense codes
+ */
+
+/* No sense data available */
+extern const struct SCSISense sense_code_NO_SENSE;
+/* LUN not ready, Manual intervention required */
+extern const struct SCSISense sense_code_LUN_NOT_READY;
+/* LUN not ready, Medium not present */
+extern const struct SCSISense sense_code_NO_MEDIUM;
+/* Hardware error, internal target failure */
+extern const struct SCSISense sense_code_TARGET_FAILURE;
+/* Illegal request, invalid command operation code */
+extern const struct SCSISense sense_code_INVALID_OPCODE;
+/* Illegal request, LBA out of range */
+extern const struct SCSISense sense_code_LBA_OUT_OF_RANGE;
+/* Illegal request, Invalid field in CDB */
+extern const struct SCSISense sense_code_INVALID_FIELD;
+/* Illegal request, LUN not supported */
+extern const struct SCSISense sense_code_LUN_NOT_SUPPORTED;
+/* Command aborted, I/O process terminated */
+extern const struct SCSISense sense_code_IO_ERROR;
+/* Command aborted, I_T Nexus loss occurred */
+extern const struct SCSISense sense_code_I_T_NEXUS_LOSS;
+/* Command aborted, Logical Unit failure */
+extern const struct SCSISense sense_code_LUN_FAILURE;
+
+#define SENSE_CODE(x) sense_code_ ## x
+
+int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
+int scsi_sense_valid(SCSISense sense);
+
 SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
 void scsi_req_enqueue(SCSIRequest *req);
 void scsi_req_free(SCSIRequest *req);
-void scsi_req_dequeue(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
 void scsi_req_unref(SCSIRequest *req);