[Qemu-devel] [PATCH v2] scsi-bus: fix to allow some special SCSI commands

[TAMUKI Shoichi]

Currently, some special SCSI commands sent from the initiator in a
guest do not reach the target device.  To avoid this, extended (0x7e,)
variable length (0x7f,) and vendor specific (0xc0..0xff) opcodes are
now treated as valid CDBs.

Originally, the most significant 3 bits of a SCSI opcode specified the
length of the CDB.  However, when variable-length CDBs were created,
this correspondence was changed, and the entire opcode must be
examined to determine the CDB length.  The CDBs with the opcodes above
are done that way for now.

Signed-off-by: TAMUKI Shoichi <address@hidden>
---
v2: add a new argument to scsi_req_new(), and modify all invocations
in hw/{scsi,usb}, since this function is not called only for virtio-
scsi.

 hw/scsi/esp.c          | 20 ++++++++++----------
 hw/scsi/lsi53c895a.c   |  2 +-
 hw/scsi/megasas.c      | 12 +++++++-----
 hw/scsi/scsi-bus.c     | 26 ++++++++++++++++++++------
 hw/scsi/spapr_vscsi.c  |  3 ++-
 hw/scsi/virtio-scsi.c  |  2 +-
 hw/scsi/vmw_pvscsi.c   |  3 ++-
 hw/usb/dev-storage.c   |  3 ++-
 hw/usb/dev-uas.c       |  3 ++-
 include/hw/scsi/scsi.h |  2 +-
 10 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 5ab44d8..ad58d84 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -120,7 +120,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf)
     return dmalen;
 }
 
-static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
+static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid, uint32_t 
len)
 {
     int32_t datalen;
     int lun;
@@ -129,7 +129,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t 
busid)
     trace_esp_do_busid_cmd(busid);
     lun = busid & 7;
     current_lun = scsi_device_find(&s->bus, 0, s->current_dev->id, lun);
-    s->current_req = scsi_req_new(current_lun, 0, lun, buf, s);
+    s->current_req = scsi_req_new(current_lun, 0, lun, buf, s, len);
     datalen = scsi_req_enqueue(s->current_req);
     s->ti_size = datalen;
     if (datalen != 0) {
@@ -148,17 +148,17 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
uint8_t busid)
     esp_raise_irq(s);
 }
 
-static void do_cmd(ESPState *s, uint8_t *buf)
+static void do_cmd(ESPState *s, uint8_t *buf, uint32_t len)
 {
     uint8_t busid = buf[0];
 
-    do_busid_cmd(s, &buf[1], busid);
+    do_busid_cmd(s, &buf[1], busid, len);
 }
 
 static void handle_satn(ESPState *s)
 {
     uint8_t buf[32];
-    int len;
+    uint32_t len;
 
     if (s->dma && !s->dma_enabled) {
         s->dma_cb = handle_satn;
@@ -166,13 +166,13 @@ static void handle_satn(ESPState *s)
     }
     len = get_cmd(s, buf);
     if (len)
-        do_cmd(s, buf);
+        do_cmd(s, buf, len);
 }
 
 static void handle_s_without_atn(ESPState *s)
 {
     uint8_t buf[32];
-    int len;
+    uint32_t len;
 
     if (s->dma && !s->dma_enabled) {
         s->dma_cb = handle_s_without_atn;
@@ -180,7 +180,7 @@ static void handle_s_without_atn(ESPState *s)
     }
     len = get_cmd(s, buf);
     if (len) {
-        do_busid_cmd(s, buf, 0);
+        do_busid_cmd(s, buf, 0, len);
     }
 }
 
@@ -245,7 +245,7 @@ static void esp_do_dma(ESPState *s)
         s->ti_size = 0;
         s->cmdlen = 0;
         s->do_cmd = 0;
-        do_cmd(s, s->cmdbuf);
+        do_cmd(s, s->cmdbuf, len);
         return;
     }
     if (s->async_len == 0) {
@@ -355,7 +355,7 @@ static void handle_ti(ESPState *s)
         s->ti_size = 0;
         s->cmdlen = 0;
         s->do_cmd = 0;
-        do_cmd(s, s->cmdbuf);
+        do_cmd(s, s->cmdbuf, minlen);
         return;
     }
 }
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 786d848..1609721 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -783,7 +783,7 @@ static void lsi_do_command(LSIState *s)
     s->current = g_malloc0(sizeof(lsi_request));
     s->current->tag = s->select_tag;
     s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun, buf,
-                                   s->current);
+                                   s->current, s->dbc);
 
     n = scsi_req_enqueue(s->current->req);
     if (n) {
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index c68a873..60ce4d1 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -990,7 +990,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int 
lun,
         info->inquiry_data[0] = 0x7f; /* Force PQual 0x3, PType 0x1f */
         info->vpd_page83[0] = 0x7f;
         megasas_setup_inquiry(cmdbuf, 0, sizeof(info->inquiry_data));
-        req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd);
+        req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd, 6);
         if (!req) {
             trace_megasas_dcmd_req_alloc_failed(cmd->index,
                                                 "PD get info std inquiry");
@@ -1008,7 +1008,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, 
int lun,
         return MFI_STAT_INVALID_STATUS;
     } else if (info->inquiry_data[0] != 0x7f && info->vpd_page83[0] == 0x7f) {
         megasas_setup_inquiry(cmdbuf, 0x83, sizeof(info->vpd_page83));
-        req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd);
+        req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd, 6);
         if (!req) {
             trace_megasas_dcmd_req_alloc_failed(cmd->index,
                                                 "PD get info vpd inquiry");
@@ -1153,7 +1153,7 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, 
int lun,
         memset(cmd->iov_buf, 0x0, dcmd_size);
         info = cmd->iov_buf;
         megasas_setup_inquiry(cdb, 0x83, sizeof(info->vpd_page83));
-        req = scsi_req_new(sdev, cmd->index, lun, cdb, cmd);
+        req = scsi_req_new(sdev, cmd->index, lun, cdb, cmd, 6);
         if (!req) {
             trace_megasas_dcmd_req_alloc_failed(cmd->index,
                                                 "LD get info vpd inquiry");
@@ -1610,7 +1610,8 @@ static int megasas_handle_scsi(MegasasState *s, 
MegasasCmd *cmd,
     }
 
     cmd->req = scsi_req_new(sdev, cmd->index,
-                            cmd->frame->header.lun_id, cdb, cmd);
+                            cmd->frame->header.lun_id, cdb, cmd,
+                            cmd->frame->header.cdb_len);
     if (!cmd->req) {
         trace_megasas_scsi_req_alloc_failed(
                 mfi_frame_desc[cmd->frame->header.frame_cmd],
@@ -1687,7 +1688,8 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd 
*cmd)
 
     megasas_encode_lba(cdb, lba_start, lba_count, is_write);
     cmd->req = scsi_req_new(sdev, cmd->index,
-                            cmd->frame->header.lun_id, cdb, cmd);
+                            cmd->frame->header.lun_id, cdb, cmd,
+                            cmd->frame->header.cdb_len);
     if (!cmd->req) {
         trace_megasas_scsi_req_alloc_failed(
             mfi_frame_desc[cmd->frame->header.frame_cmd],
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index ea1ac09..d039b6d 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -9,7 +9,8 @@
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
-static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
+static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf,
+        uint32_t len);
 static void scsi_req_dequeue(SCSIRequest *req);
 static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
 static void scsi_target_free_buf(SCSIRequest *req);
@@ -557,13 +558,13 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, 
SCSIDevice *d,
 }
 
 SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
-                          uint8_t *buf, void *hba_private)
+                          uint8_t *buf, void *hba_private, uint32_t len)
 {
     SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
     SCSIRequest *req;
     SCSICommand cmd;
 
-    if (scsi_req_parse(&cmd, d, buf) != 0) {
+    if (scsi_req_parse(&cmd, d, buf, len) != 0) {
         trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
         req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
     } else {
@@ -1181,7 +1182,8 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd)
     return lba;
 }
 
-static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
+static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf,
+        uint32_t len)
 {
     int rc;
 
@@ -1193,12 +1195,23 @@ static int scsi_req_parse(SCSICommand *cmd, SCSIDevice 
*dev, uint8_t *buf)
     case 2:
         cmd->len = 10;
         break;
+    case 3:
+        if (buf[0] == 0x7e || (buf[0] == 0x7f && len >= 16)) {
+            cmd->len = len;
+        } else {
+            return -1;
+        }
+        break;
     case 4:
         cmd->len = 16;
         break;
     case 5:
         cmd->len = 12;
         break;
+    case 6:
+    case 7:
+        cmd->len = len;
+        break;
     default:
         return -1;
     }
@@ -1861,14 +1874,15 @@ static int get_scsi_requests(QEMUFile *f, void *pv, 
size_t size)
 
     while ((sbyte = qemu_get_sbyte(f)) > 0) {
         uint8_t buf[SCSI_CMD_BUF_SIZE];
+        int len;
         uint32_t tag;
         uint32_t lun;
         SCSIRequest *req;
 
-        qemu_get_buffer(f, buf, sizeof(buf));
+        len = qemu_get_buffer(f, buf, sizeof(buf));
         qemu_get_be32s(f, &tag);
         qemu_get_be32s(f, &lun);
-        req = scsi_req_new(s, tag, lun, buf, NULL);
+        req = scsi_req_new(s, tag, lun, buf, NULL, len);
         req->retry = (sbyte == 1);
         if (bus->info->load_request) {
             req->hba_private = bus->info->load_request(f, req);
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 048cfc7..9151bf0 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -796,7 +796,8 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
         } return 1;
     }
 
-    req->sreq = scsi_req_new(sdev, req->qtag, lun, srp->cmd.cdb, req);
+    req->sreq = scsi_req_new(sdev, req->qtag, lun, srp->cmd.cdb, req,
+            16 + srp->cmd.add_cdb_len);
     n = scsi_req_enqueue(req->sreq);
 
     DPRINTF("VSCSI: Queued command tag 0x%x CMD 0x%x=%s LUN %d ret: %d\n",
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 0eb069a..2de9b28 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -466,7 +466,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, 
VirtQueue *vq)
         }
         req->sreq = scsi_req_new(d, req->req.cmd.tag,
                                  virtio_scsi_get_lun(req->req.cmd.lun),
-                                 req->req.cdb, req);
+                                 req->req.cdb, req, vs->cdb_size);
 
         if (req->sreq->cmd.mode != SCSI_XFER_NONE
             && (req->sreq->cmd.mode != req->mode ||
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index f9ed926..4925073 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -649,7 +649,8 @@ pvscsi_process_request_descriptor(PVSCSIState *s,
         r->sg.elemAddr = descr->dataAddr;
     }
 
-    r->sreq = scsi_req_new(d, descr->context, r->lun, descr->cdb, r);
+    r->sreq = scsi_req_new(d, descr->context, r->lun, descr->cdb, r,
+            descr->cdbLen);
     if (r->sreq->cmd.mode == SCSI_XFER_FROM_DEV &&
         (descr->flags & PVSCSI_FLAG_CMD_DIR_TODEVICE)) {
         r->cmp.hostStatus = BTSTAT_BADMSG;
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index ae4efcb..a930d6a 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -437,7 +437,8 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket 
*p)
                     tag, cbw.flags, cbw.cmd_len, s->data_len);
             assert(le32_to_cpu(s->csw.residue) == 0);
             s->scsi_len = 0;
-            s->req = scsi_req_new(scsi_dev, tag, cbw.lun, cbw.cmd, NULL);
+            s->req = scsi_req_new(scsi_dev, tag, cbw.lun, cbw.cmd, NULL,
+                    cbw.cmd_len);
 #ifdef DEBUG_MSD
             scsi_req_print(s->req);
 #endif
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 9832385..c6cb61b 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -714,7 +714,8 @@ static void usb_uas_command(UASDevice *uas, uas_iu *iu)
 
     req->req = scsi_req_new(req->dev, req->tag,
                             usb_uas_get_lun(req->lun),
-                            iu->command.cdb, req);
+                            iu->command.cdb, req,
+                            16 + iu->command.add_cdb_length);
     if (uas->requestlog) {
         scsi_req_print(req->req);
     }
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1adb549..76365c2 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -238,7 +238,7 @@ int scsi_build_sense(uint8_t *in_buf, int in_len,
 SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d,
                             uint32_t tag, uint32_t lun, void *hba_private);
 SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
-                          uint8_t *buf, void *hba_private);
+                          uint8_t *buf, void *hba_private, uint32_t len);
 int32_t scsi_req_enqueue(SCSIRequest *req);
 void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
-- 
1.9.0