[Qemu-devel] [PULL 00/32] SCSI patches for 2012-08-27

[Qemu-devel] [PULL 00/32] SCSI patches for 2012-08-27

[Paolo Bonzini]

Anthony,

The following changes since commit a21143486b9c6d7a50b7b62877c02b3c686943cb:

  Merge remote-tracking branch 'stefanha/net' into staging (2012-07-23 13:15:34 -0500)

are available in the git repository at:

  git://github.com/bonzini/qemu.git scsi-next

for you to fetch changes up to 4c205d0cb1c8bde5a53f6acceda74dae1043a197:

  virtio-scsi: enable MSI-X support (2012-07-27 15:47:30 +0200)

The main changes here are virtio-scsi event support (including
hotplug from Cong Meng and block_resize), cache toggle (and large
refactoring required for it), a few other scsi-disk improvements, and
two small preparatory patches for virtio-scsi multiqueue.

Paolo

----------------------------------------------------------------
Cong Meng (2):
      scsi: introduce hotplug() and hot_unplug() interfaces for SCSI bus
      virtio-scsi: Implement hotplug support for virtio-scsi

Dmitry Fleytman (2):
      cutils: add strpadcpy()
      scsi-disk: let the user customize vendor and product name

Hannes Reinecke (1):
      megasas: Replace trace_megasas_dcmd_dump_frame()

Paolo Bonzini (24):
      megasas: fix misuse of scsi_req_abort
      lsi: use qdev_reset_all
      lsi: introduce lsi_request_free
      lsi: avoid redundant tests of s->current != NULL
      scsi-block: remove properties that are not relevant for passthrough
      scsi-disk: make discard asynchronous
      scsi-disk: move all non-DMA commands to scsi_disk_emulate_command
      scsi-disk: split scsi-disk reqops
      scsi-disk: separate read_data/write_data implementation for emulate_reqops
      scsi-disk: support emulated TO_DEV requests
      scsi-disk: adjust offsets in MODE SENSE by 2
      scsi-disk: fix changeable values for MODE_PAGE_R_W_ERROR
      scsi-disk: parse MODE SELECT commands and parameters
      scsi-disk: support toggling the write cache
      scsi-disk: removable hard disks support load/eject
      scsi: add tracepoint for scsi_req_cancel
      scsi: establish precedence levels for unit attention
      scsi-disk: report resized disk via sense codes
      scsi: report parameter changes to HBA drivers
      virtio-scsi: Report missed events
      virtio-scsi: do not report dropped events after reset
      virtio-scsi: report parameter change events
      virtio-scsi: add ioeventfd support
      virtio-scsi: enable MSI-X support

Ronnie Sahlberg (3):
      scsi-disk: rd/wr/vr-protect !=0 is an error
      scsi-disk: improve the lba-out-of-range tests for read/write/verify
      scsi-disk: Fail medium writes with proper sense for readonly LUNs

 cutils.c         |    7 +
 hw/lsi53c895a.c  |   51 ++--
 hw/megasas.c     |   41 +--
 hw/scsi-bus.c    |  100 ++++++-
 hw/scsi-disk.c   |  774 +++++++++++++++++++++++++++++++++++++-----------------
 hw/scsi.h        |   13 +
 hw/virtio-pci.c  |    7 +-
 hw/virtio-scsi.c |  115 +++++++-
 qemu-common.h    |    1 +
 trace-events     |    4 +-
 10 files changed, 796 insertions(+), 317 deletions(-)
-- 
1.7.10.4

[Qemu-devel] [PATCH 01/32] megasas: Replace trace_megasas_dcmd_dump_frame()

[Paolo Bonzini]

From: Hannes Reinecke <hare@suse.de>

trace_megasas_dcmd_dump_frame() takes 9 arguments, which is
rather much. Plus the trace infrastructure doesn't support
it.  As we can get the information via other means it's pointless
to have it in the driver, so rather use some proper trace
point here and remove the old one.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Cc: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/megasas.c |   39 ++++++++++-----------------------------
 trace-events |    2 +-
 2 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/hw/megasas.c b/hw/megasas.c
index b99fa97..c389ddf 100644
--- a/hw/megasas.c
+++ b/hw/megasas.c
@@ -1290,35 +1290,16 @@ static int megasas_cluster_reset_ld(MegasasState *s, MegasasCmd *cmd)
 
 static int megasas_dcmd_set_properties(MegasasState *s, MegasasCmd *cmd)
 {
-    uint8_t *dummy = g_malloc(cmd->iov_size);
-
-    dma_buf_write(dummy, cmd->iov_size, &cmd->qsg);
-
-    trace_megasas_dcmd_dump_frame(0,
-            dummy[0x00], dummy[0x01], dummy[0x02], dummy[0x03],
-            dummy[0x04], dummy[0x05], dummy[0x06], dummy[0x07]);
-    trace_megasas_dcmd_dump_frame(1,
-            dummy[0x08], dummy[0x09], dummy[0x0a], dummy[0x0b],
-            dummy[0x0c], dummy[0x0d], dummy[0x0e], dummy[0x0f]);
-    trace_megasas_dcmd_dump_frame(2,
-            dummy[0x10], dummy[0x11], dummy[0x12], dummy[0x13],
-            dummy[0x14], dummy[0x15], dummy[0x16], dummy[0x17]);
-    trace_megasas_dcmd_dump_frame(3,
-            dummy[0x18], dummy[0x19], dummy[0x1a], dummy[0x1b],
-            dummy[0x1c], dummy[0x1d], dummy[0x1e], dummy[0x1f]);
-    trace_megasas_dcmd_dump_frame(4,
-            dummy[0x20], dummy[0x21], dummy[0x22], dummy[0x23],
-            dummy[0x24], dummy[0x25], dummy[0x26], dummy[0x27]);
-    trace_megasas_dcmd_dump_frame(5,
-            dummy[0x28], dummy[0x29], dummy[0x2a], dummy[0x2b],
-            dummy[0x2c], dummy[0x2d], dummy[0x2e], dummy[0x2f]);
-    trace_megasas_dcmd_dump_frame(6,
-            dummy[0x30], dummy[0x31], dummy[0x32], dummy[0x33],
-            dummy[0x34], dummy[0x35], dummy[0x36], dummy[0x37]);
-    trace_megasas_dcmd_dump_frame(7,
-            dummy[0x38], dummy[0x39], dummy[0x3a], dummy[0x3b],
-            dummy[0x3c], dummy[0x3d], dummy[0x3e], dummy[0x3f]);
-    g_free(dummy);
+    struct mfi_ctrl_props info;
+    size_t dcmd_size = sizeof(info);
+
+    if (cmd->iov_size < dcmd_size) {
+        trace_megasas_dcmd_invalid_xfer_len(cmd->index, cmd->iov_size,
+                                            dcmd_size);
+        return MFI_STAT_INVALID_PARAMETER;
+    }
+    dma_buf_write((uint8_t *)&info, cmd->iov_size, &cmd->qsg);
+    trace_megasas_dcmd_unsupported(cmd->index, cmd->iov_size);
     return MFI_STAT_OK;
 }
 
diff --git a/trace-events b/trace-events
index 2a5f074..6fb5eb5 100644
--- a/trace-events
+++ b/trace-events
@@ -595,7 +595,7 @@ megasas_dcmd_ld_get_list(int cmd, int num, int max) "scmd %d: DCMD LD get list:
 megasas_dcmd_ld_get_info(int cmd, int ld_id) "scmd %d: DCMD LD get info for dev %d"
 megasas_dcmd_pd_get_info(int cmd, int pd_id) "scmd %d: DCMD PD get info for dev %d"
 megasas_dcmd_pd_list_query(int cmd, int flags) "scmd %d: DCMD PD list query flags %x"
-megasas_dcmd_dump_frame(int offset, char f0, char f1, char f2, char f3, char f4, char f5, char f6, char f7) "0x%x: %02x %02x %02x %02x %02x %02x %02x %02x"
+megasas_dcmd_unsupported(int cmd, unsigned long size) "scmd %d: set properties len %ld"
 megasas_abort_frame(int cmd, int abort_cmd) "scmd %d: aborting frame %x"
 megasas_abort_no_cmd(int cmd, uint64_t context) "scmd %d: no active command for frame context %" PRIx64 ""
 megasas_abort_invalid_context(int cmd, uint64_t context, int abort_cmd) "scmd %d: invalid frame context %" PRIx64 " for abort frame %x"
-- 
1.7.10.4

[Qemu-devel] [PATCH 02/32] megasas: fix misuse of scsi_req_abort

[Paolo Bonzini]

scsi_req_abort is for terminating a command with a non-zero status.
The ABORT task management function is invoked by scsi_req_cancel.
In fact, ABORTED_COMMAND is a sense key, not a SAM status code.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/megasas.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/megasas.c b/hw/megasas.c
index c389ddf..9a0eab1 100644
--- a/hw/megasas.c
+++ b/hw/megasas.c
@@ -544,7 +544,7 @@ static void megasas_reset_frames(MegasasState *s)
 static void megasas_abort_command(MegasasCmd *cmd)
 {
     if (cmd->req) {
-        scsi_req_abort(cmd->req, ABORTED_COMMAND);
+        scsi_req_cancel(cmd->req);
         cmd->req = NULL;
     }
 }
-- 
1.7.10.4

[Qemu-devel] [PATCH 03/32] lsi: use qdev_reset_all

[Paolo Bonzini]

By first resetting the devices, lsi_soft_reset will find the queue
already cleared so there is no need to do that forcibly (which may also
leak SCSIRequests, and/or worse due to dangling references to the
lsi_request in the hba_private field).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/lsi53c895a.c |   15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 2fe141d..f04cc68 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -282,8 +282,6 @@ static inline int lsi_irq_on_rsl(LSIState *s)
 
 static void lsi_soft_reset(LSIState *s)
 {
-    lsi_request *p;
-
     DPRINTF("Reset\n");
     s->carry = 0;
 
@@ -350,15 +348,8 @@ static void lsi_soft_reset(LSIState *s)
     s->sbc = 0;
     s->csbc = 0;
     s->sbr = 0;
-    while (!QTAILQ_EMPTY(&s->queue)) {
-        p = QTAILQ_FIRST(&s->queue);
-        QTAILQ_REMOVE(&s->queue, p, next);
-        g_free(p);
-    }
-    if (s->current) {
-        g_free(s->current);
-        s->current = NULL;
-    }
+    assert(QTAILQ_EMPTY(&s->queue));
+    assert(!s->current);
 }
 
 static int lsi_dma_40bit(LSIState *s)
@@ -1738,7 +1729,7 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
             lsi_execute_script(s);
         }
         if (val & LSI_ISTAT0_SRST) {
-            lsi_soft_reset(s);
+            qdev_reset_all(&s->dev.qdev);
         }
         break;
     case 0x16: /* MBOX0 */
-- 
1.7.10.4

[Qemu-devel] [PATCH 04/32] lsi: introduce lsi_request_free

[Paolo Bonzini]

Split the common bits of lsi_request_cancelled and lsi_command_complete
out to a new function.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/lsi53c895a.c |   31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index f04cc68..078ab6d 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -641,23 +641,24 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t tag)
     return NULL;
 }
 
+static void lsi_request_free(LSIState *s, lsi_request *p)
+{
+    if (p == s->current) {
+        s->current = NULL;
+    } else {
+        QTAILQ_REMOVE(&s->queue, p, next);
+    }
+    g_free(p);
+}
+
 static void lsi_request_cancelled(SCSIRequest *req)
 {
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
     lsi_request *p = req->hba_private;
 
-    if (s->current && req == s->current->req) {
-        scsi_req_unref(req);
-        g_free(s->current);
-        s->current = NULL;
-        return;
-    }
-
-    if (p) {
-        QTAILQ_REMOVE(&s->queue, p, next);
-        scsi_req_unref(req);
-        g_free(p);
-    }
+    req->hba_private = NULL;
+    lsi_request_free(s, p);
+    scsi_req_unref(req);
 }
 
 /* Record that data is available for a queued command.  Returns zero if
@@ -706,9 +707,9 @@ static void lsi_command_complete(SCSIRequest *req, uint32_t status, size_t resid
     }
 
     if (s->current && req == s->current->req) {
-        scsi_req_unref(s->current->req);
-        g_free(s->current);
-        s->current = NULL;
+        req->hba_private = NULL;
+        lsi_request_free(s, s->current);
+        scsi_req_unref(req);
     }
     lsi_resume_script(s);
 }
-- 
1.7.10.4

[Qemu-devel] [PATCH 05/32] lsi: avoid redundant tests of s->current != NULL

[Paolo Bonzini]

Simplify the code by checking against req->hba_private directly,
and asserting that it is non-NULL before a command is completed
or canceled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/lsi53c895a.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 078ab6d..5f6cb17 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -706,7 +706,7 @@ static void lsi_command_complete(SCSIRequest *req, uint32_t status, size_t resid
         lsi_set_phase(s, PHASE_ST);
     }
 
-    if (s->current && req == s->current->req) {
+    if (req->hba_private == s->current) {
         req->hba_private = NULL;
         lsi_request_free(s, s->current);
         scsi_req_unref(req);
@@ -720,7 +720,8 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t len)
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
     int out;
 
-    if (s->waiting == 1 || !s->current || req->hba_private != s->current ||
+    assert(req->hba_private);
+    if (s->waiting == 1 || req->hba_private != s->current ||
         (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
         if (lsi_queue_req(s, req, len)) {
             return;
-- 
1.7.10.4

[Qemu-devel] [PATCH 06/32] scsi-block: remove properties that are not relevant for passthrough

[Paolo Bonzini]

scsi-block is a passthrough device and does not allow customization
of vendor, product, removable, DPOFUA, block size or any other piece of
information.  Thus, drop DEFINE_SCSI_DISK_PROPERTIES() from the
list of qdev properties.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 525816c..8907197 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -2040,7 +2040,7 @@ static TypeInfo scsi_cd_info = {
 
 #ifdef __linux__
 static Property scsi_block_properties[] = {
-    DEFINE_SCSI_DISK_PROPERTIES(),
+    DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.bs),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 07/32] cutils: add strpadcpy()

[Paolo Bonzini]

From: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>

Signed-off-by: Yan Vugenfirer <yan@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cutils.c      |    7 +++++++
 qemu-common.h |    1 +
 2 files changed, 8 insertions(+)

diff --git a/cutils.c b/cutils.c
index e2bc1b8..9d4c570 100644
--- a/cutils.c
+++ b/cutils.c
@@ -28,6 +28,13 @@
 #include "qemu_socket.h"
 #include "iov.h"
 
+void strpadcpy(char *buf, int buf_size, const char *str, char pad)
+{
+    int len = qemu_strnlen(str, buf_size);
+    memcpy(buf, str, len);
+    memset(buf + len, pad, buf_size - len);
+}
+
 void pstrcpy(char *buf, int buf_size, const char *str)
 {
     int c;
diff --git a/qemu-common.h b/qemu-common.h
index 7c8dac8..d26ff39 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -138,6 +138,7 @@ int qemu_timedate_diff(struct tm *tm);
 
 /* cutils.c */
 void pstrcpy(char *buf, int buf_size, const char *str);
+void strpadcpy(char *buf, int buf_size, const char *str, char pad);
 char *pstrcat(char *buf, int buf_size, const char *s);
 int strstart(const char *str, const char *val, const char **ptr);
 int stristart(const char *str, const char *val, const char **ptr);
-- 
1.7.10.4

[Qemu-devel] [PATCH 08/32] scsi-disk: let the user customize vendor and product name

[Paolo Bonzini]

From: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>

This patch adds two new properties vendor and product to SCSI disks.
These options let the user customize the inquiry data returned by the
disk.

Signed-off-by: Yan Vugenfirer <yan@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
[ Use vendor and product property names, avoid "if" statements. - PB ]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |   31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 8907197..788fe86 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -72,6 +72,8 @@ struct SCSIDiskState
     QEMUBH *bh;
     char *version;
     char *serial;
+    char *vendor;
+    char *product;
     bool tray_open;
     bool tray_locked;
 };
@@ -669,12 +671,10 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
 
     outbuf[0] = s->qdev.type & 0x1f;
     outbuf[1] = (s->features & (1 << SCSI_DISK_F_REMOVABLE)) ? 0x80 : 0;
-    if (s->qdev.type == TYPE_ROM) {
-        memcpy(&outbuf[16], "QEMU CD-ROM     ", 16);
-    } else {
-        memcpy(&outbuf[16], "QEMU HARDDISK   ", 16);
-    }
-    memcpy(&outbuf[8], "QEMU    ", 8);
+
+    strpadcpy((char *) &outbuf[16], 16, s->product, ' ');
+    strpadcpy((char *) &outbuf[8], 8, s->vendor, ' ');
+
     memset(&outbuf[32], 0, 4);
     memcpy(&outbuf[32], s->version, MIN(4, strlen(s->version)));
     /*
@@ -1757,6 +1757,9 @@ static int scsi_initfn(SCSIDevice *dev)
     if (!s->version) {
         s->version = g_strdup(qemu_get_version());
     }
+    if (!s->vendor) {
+        s->vendor = g_strdup("QEMU");
+    }
 
     if (bdrv_is_sg(s->qdev.conf.bs)) {
         error_report("unwanted /dev/sg*");
@@ -1778,6 +1781,9 @@ static int scsi_hd_initfn(SCSIDevice *dev)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
     s->qdev.blocksize = s->qdev.conf.logical_block_size;
     s->qdev.type = TYPE_DISK;
+    if (!s->product) {
+        s->product = g_strdup("QEMU HARDDISK");
+    }
     return scsi_initfn(&s->qdev);
 }
 
@@ -1787,6 +1793,9 @@ static int scsi_cd_initfn(SCSIDevice *dev)
     s->qdev.blocksize = 2048;
     s->qdev.type = TYPE_ROM;
     s->features |= 1 << SCSI_DISK_F_REMOVABLE;
+    if (!s->product) {
+        s->product = g_strdup("QEMU CD-ROM");
+    }
     return scsi_initfn(&s->qdev);
 }
 
@@ -1954,10 +1963,12 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
 }
 #endif
 
-#define DEFINE_SCSI_DISK_PROPERTIES()                           \
-    DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),          \
-    DEFINE_PROP_STRING("ver",  SCSIDiskState, version),         \
-    DEFINE_PROP_STRING("serial",  SCSIDiskState, serial)
+#define DEFINE_SCSI_DISK_PROPERTIES()                                \
+    DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),               \
+    DEFINE_PROP_STRING("ver", SCSIDiskState, version),               \
+    DEFINE_PROP_STRING("serial", SCSIDiskState, serial),             \
+    DEFINE_PROP_STRING("vendor", SCSIDiskState, vendor),             \
+    DEFINE_PROP_STRING("product", SCSIDiskState, product)
 
 static Property scsi_hd_properties[] = {
     DEFINE_SCSI_DISK_PROPERTIES(),
-- 
1.7.10.4

[Qemu-devel] [PATCH 09/32] scsi-disk: make discard asynchronous

[Paolo Bonzini]

By making discard asynchronous, we can reuse all the error handling
code that is used for other commands.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |   22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 788fe86..f8e9dc1 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -169,7 +169,7 @@ static void scsi_disk_load_request(QEMUFile *f, SCSIRequest *req)
     qemu_iovec_init_external(&r->qiov, &r->iov, 1);
 }
 
-static void scsi_flush_complete(void * opaque, int ret)
+static void scsi_aio_complete(void *opaque, int ret)
 {
     SCSIDiskReq *r = (SCSIDiskReq *)opaque;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
@@ -222,7 +222,7 @@ static void scsi_write_do_fua(SCSIDiskReq *r)
 
     if (scsi_is_cmd_fua(&r->req.cmd)) {
         bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH);
-        r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_flush_complete, r);
+        r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r);
         return;
     }
 
@@ -1543,7 +1543,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
         /* The request is used as the AIO opaque value, so add a ref.  */
         scsi_req_ref(&r->req);
         bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH);
-        r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_flush_complete, r);
+        r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r);
         return 0;
     case READ_6:
     case READ_10:
@@ -1620,15 +1620,13 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
             goto fail;
         }
 
-        rc = bdrv_discard(s->qdev.conf.bs,
-                          r->req.cmd.lba * (s->qdev.blocksize / 512),
-                          len * (s->qdev.blocksize / 512));
-        if (rc < 0) {
-            /* XXX: better error code ?*/
-            goto fail;
-        }
-
-        break;
+        /* The request is used as the AIO opaque value, so add a ref.  */
+        scsi_req_ref(&r->req);
+        r->req.aiocb = bdrv_aio_discard(s->qdev.conf.bs,
+                                        r->req.cmd.lba * (s->qdev.blocksize / 512),
+                                        len * (s->qdev.blocksize / 512),
+                                        scsi_aio_complete, r);
+        return 0;
     default:
         DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
         scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
-- 
1.7.10.4

[Qemu-devel] [PATCH 10/32] scsi-disk: move all non-DMA commands to scsi_disk_emulate_command

[Paolo Bonzini]

We want to use separate SCSIReqOps for emulated commands needing an
allocated buffer vs. those that are zerocopy when the HBA supports
S/G lists.  Ensure that all of the former are in scsi_disk_emulate_command.

Commands that do not have any parameters are more similar to emulated
commands, so also move them, even if they do I/O.

Finally, MODE SELECT and MODE SELECT(10) are broken because we do not
yet support passing parameter data _to_ emulated commands, so disable
them.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |  152 ++++++++++++++++++++++++--------------------------------
 1 file changed, 66 insertions(+), 86 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f8e9dc1..9f7258d 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1448,10 +1448,65 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r)
         }
         DPRINTF("Unsupported Service Action In\n");
         goto illegal_request;
+    case SYNCHRONIZE_CACHE:
+        /* The request is used as the AIO opaque value, so add a ref.  */
+        scsi_req_ref(&r->req);
+        bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH);
+        r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r);
+        return 0;
+    case SEEK_10:
+        DPRINTF("Seek(10) (sector %" PRId64 ")\n", r->req.cmd.lba);
+        if (r->req.cmd.lba > s->qdev.max_lba) {
+            goto illegal_lba;
+        }
+        break;
+#if 0
+    case MODE_SELECT:
+        DPRINTF("Mode Select(6) (len %lu)\n", (long)r->req.cmd.xfer);
+        /* We don't support mode parameter changes.
+           Allow the mode parameter header + block descriptors only. */
+        if (r->req.cmd.xfer > 12) {
+            goto illegal_request;
+        }
+        break;
+    case MODE_SELECT_10:
+        DPRINTF("Mode Select(10) (len %lu)\n", (long)r->req.cmd.xfer);
+        /* We don't support mode parameter changes.
+           Allow the mode parameter header + block descriptors only. */
+        if (r->req.cmd.xfer > 16) {
+            goto illegal_request;
+        }
+        break;
+#endif
+    case WRITE_SAME_10:
+        nb_sectors = lduw_be_p(&req->cmd.buf[7]);
+        goto write_same;
+    case WRITE_SAME_16:
+        nb_sectors = ldl_be_p(&req->cmd.buf[10]) & 0xffffffffULL;
+    write_same:
+        if (r->req.cmd.lba > s->qdev.max_lba) {
+            goto illegal_lba;
+        }
+
+        /*
+         * We only support WRITE SAME with the unmap bit set for now.
+         */
+        if (!(req->cmd.buf[1] & 0x8)) {
+            goto illegal_request;
+        }
+
+        /* The request is used as the AIO opaque value, so add a ref.  */
+        scsi_req_ref(&r->req);
+        r->req.aiocb = bdrv_aio_discard(s->qdev.conf.bs,
+                                        r->req.cmd.lba * (s->qdev.blocksize / 512),
+                                        nb_sectors * (s->qdev.blocksize / 512),
+                                        scsi_aio_complete, r);
+        return 0;
     default:
         scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
         return -1;
     }
+    assert(r->sector_count == 0);
     buflen = MIN(buflen, req->cmd.xfer);
     return buflen;
 
@@ -1460,6 +1515,10 @@ illegal_request:
         scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
     }
     return -1;
+
+illegal_lba:
+    scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
+    return 0;
 }
 
 /* Execute a scsi command.  Returns the length of the data expected by the
@@ -1513,38 +1572,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
     }
 
     switch (command) {
-    case TEST_UNIT_READY:
-    case INQUIRY:
-    case MODE_SENSE:
-    case MODE_SENSE_10:
-    case RESERVE:
-    case RESERVE_10:
-    case RELEASE:
-    case RELEASE_10:
-    case START_STOP:
-    case ALLOW_MEDIUM_REMOVAL:
-    case READ_CAPACITY_10:
-    case READ_TOC:
-    case READ_DISC_INFORMATION:
-    case READ_DVD_STRUCTURE:
-    case GET_CONFIGURATION:
-    case GET_EVENT_STATUS_NOTIFICATION:
-    case MECHANISM_STATUS:
-    case SERVICE_ACTION_IN_16:
-    case REQUEST_SENSE:
-        rc = scsi_disk_emulate_command(r);
-        if (rc < 0) {
-            return 0;
-        }
-
-        r->iov.iov_len = rc;
-        break;
-    case SYNCHRONIZE_CACHE:
-        /* The request is used as the AIO opaque value, so add a ref.  */
-        scsi_req_ref(&r->req);
-        bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH);
-        r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r);
-        return 0;
     case READ_6:
     case READ_10:
     case READ_12:
@@ -1577,63 +1604,16 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
         r->sector = r->req.cmd.lba * (s->qdev.blocksize / 512);
         r->sector_count = len * (s->qdev.blocksize / 512);
         break;
-    case MODE_SELECT:
-        DPRINTF("Mode Select(6) (len %lu)\n", (long)r->req.cmd.xfer);
-        /* We don't support mode parameter changes.
-           Allow the mode parameter header + block descriptors only. */
-        if (r->req.cmd.xfer > 12) {
-            goto fail;
-        }
-        break;
-    case MODE_SELECT_10:
-        DPRINTF("Mode Select(10) (len %lu)\n", (long)r->req.cmd.xfer);
-        /* We don't support mode parameter changes.
-           Allow the mode parameter header + block descriptors only. */
-        if (r->req.cmd.xfer > 16) {
-            goto fail;
+    default:
+        rc = scsi_disk_emulate_command(r);
+        if (rc < 0) {
+            return 0;
         }
-        break;
-    case SEEK_10:
-        DPRINTF("Seek(10) (sector %" PRId64 ")\n", r->req.cmd.lba);
-        if (r->req.cmd.lba > s->qdev.max_lba) {
-            goto illegal_lba;
+        if (r->req.aiocb) {
+            return 0;
         }
+        r->iov.iov_len = rc;
         break;
-    case WRITE_SAME_10:
-        len = lduw_be_p(&buf[7]);
-        goto write_same;
-    case WRITE_SAME_16:
-        len = ldl_be_p(&buf[10]) & 0xffffffffULL;
-    write_same:
-
-        DPRINTF("WRITE SAME() (sector %" PRId64 ", count %d)\n",
-                r->req.cmd.lba, len);
-
-        if (r->req.cmd.lba > s->qdev.max_lba) {
-            goto illegal_lba;
-        }
-
-        /*
-         * We only support WRITE SAME with the unmap bit set for now.
-         */
-        if (!(buf[1] & 0x8)) {
-            goto fail;
-        }
-
-        /* The request is used as the AIO opaque value, so add a ref.  */
-        scsi_req_ref(&r->req);
-        r->req.aiocb = bdrv_aio_discard(s->qdev.conf.bs,
-                                        r->req.cmd.lba * (s->qdev.blocksize / 512),
-                                        len * (s->qdev.blocksize / 512),
-                                        scsi_aio_complete, r);
-        return 0;
-    default:
-        DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
-        scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
-        return 0;
-    fail:
-        scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
-        return 0;
     illegal_lba:
         scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
         return 0;
-- 
1.7.10.4

[Qemu-devel] [PATCH 11/32] scsi-disk: split scsi-disk reqops

[Paolo Bonzini]

Only checks for present medium were still done in scsi_send_command
for emulated commands.  So move those to scsi_disk_emulate_command
and return different SCSIReqOps depending on the kind of command.

Checks for present medium can be done unconditionally for the
scsi_disk_dma_reqops case.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |  191 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 123 insertions(+), 68 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 9f7258d..ea0b05e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1262,14 +1262,39 @@ static int scsi_disk_emulate_start_stop(SCSIDiskReq *r)
     return 0;
 }
 
-static int scsi_disk_emulate_command(SCSIDiskReq *r)
+static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
 {
-    SCSIRequest *req = &r->req;
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
     uint64_t nb_sectors;
     uint8_t *outbuf;
     int buflen = 0;
 
+    switch (req->cmd.buf[0]) {
+    case INQUIRY:
+    case MODE_SENSE:
+    case MODE_SENSE_10:
+    case RESERVE:
+    case RESERVE_10:
+    case RELEASE:
+    case RELEASE_10:
+    case START_STOP:
+    case ALLOW_MEDIUM_REMOVAL:
+    case GET_CONFIGURATION:
+    case GET_EVENT_STATUS_NOTIFICATION:
+    case MECHANISM_STATUS:
+    case REQUEST_SENSE:
+        break;
+
+    default:
+        if (s->tray_open || !bdrv_is_inserted(s->qdev.conf.bs)) {
+            scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
+            return 0;
+        }
+        break;
+    }
+
+    assert(req->cmd.mode != SCSI_XFER_TO_DEV);
     if (!r->iov.iov_base) {
         /*
          * FIXME: we shouldn't return anything bigger than 4k, but the code
@@ -1332,7 +1357,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r)
         break;
     case START_STOP:
         if (scsi_disk_emulate_start_stop(r) < 0) {
-            return -1;
+            return 0;
         }
         break;
     case ALLOW_MEDIUM_REMOVAL:
@@ -1503,18 +1528,23 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r)
                                         scsi_aio_complete, r);
         return 0;
     default:
+        DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
         scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
-        return -1;
+        return 0;
     }
-    assert(r->sector_count == 0);
-    buflen = MIN(buflen, req->cmd.xfer);
-    return buflen;
+    assert(!r->req.aiocb && r->sector_count == 0);
+    r->iov.iov_len = MIN(buflen, req->cmd.xfer);
+    r->sector_count = -1;
+    if (r->iov.iov_len == 0) {
+        scsi_req_complete(&r->req, GOOD);
+    }
+    return r->iov.iov_len;
 
 illegal_request:
     if (r->req.status == -1) {
         scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
     }
-    return -1;
+    return 0;
 
 illegal_lba:
     scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
@@ -1526,49 +1556,18 @@ illegal_lba:
    (eg. disk reads), negative for transfers to the device (eg. disk writes),
    and zero if the command does not transfer any data.  */
 
-static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
+static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
     int32_t len;
     uint8_t command;
-    int rc;
 
     command = buf[0];
-    DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", req->lun, req->tag, buf[0]);
-
-#ifdef DEBUG_SCSI
-    {
-        int i;
-        for (i = 1; i < r->req.cmd.len; i++) {
-            printf(" 0x%02x", buf[i]);
-        }
-        printf("\n");
-    }
-#endif
 
-    switch (command) {
-    case INQUIRY:
-    case MODE_SENSE:
-    case MODE_SENSE_10:
-    case RESERVE:
-    case RESERVE_10:
-    case RELEASE:
-    case RELEASE_10:
-    case START_STOP:
-    case ALLOW_MEDIUM_REMOVAL:
-    case GET_CONFIGURATION:
-    case GET_EVENT_STATUS_NOTIFICATION:
-    case MECHANISM_STATUS:
-    case REQUEST_SENSE:
-        break;
-
-    default:
-        if (s->tray_open || !bdrv_is_inserted(s->qdev.conf.bs)) {
-            scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
-            return 0;
-        }
-        break;
+    if (s->tray_open || !bdrv_is_inserted(s->qdev.conf.bs)) {
+        scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
+        return 0;
     }
 
     switch (command) {
@@ -1605,30 +1604,19 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
         r->sector_count = len * (s->qdev.blocksize / 512);
         break;
     default:
-        rc = scsi_disk_emulate_command(r);
-        if (rc < 0) {
-            return 0;
-        }
-        if (r->req.aiocb) {
-            return 0;
-        }
-        r->iov.iov_len = rc;
-        break;
+        abort();
     illegal_lba:
         scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
         return 0;
     }
-    if (r->sector_count == 0 && r->iov.iov_len == 0) {
+    if (r->sector_count == 0) {
         scsi_req_complete(&r->req, GOOD);
     }
-    len = r->sector_count * 512 + r->iov.iov_len;
+    assert(r->iov.iov_len == 0);
     if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
-        return -len;
+        return -r->sector_count * 512;
     } else {
-        if (!r->sector_count) {
-            r->sector_count = -1;
-        }
-        return len;
+        return r->sector_count * 512;
     }
 }
 
@@ -1793,10 +1781,19 @@ static int scsi_disk_initfn(SCSIDevice *dev)
     }
 }
 
-static const SCSIReqOps scsi_disk_reqops = {
+static const SCSIReqOps scsi_disk_emulate_reqops = {
     .size         = sizeof(SCSIDiskReq),
     .free_req     = scsi_free_request,
-    .send_command = scsi_send_command,
+    .send_command = scsi_disk_emulate_command,
+    .read_data    = scsi_read_data,
+    .write_data   = scsi_write_data,
+    .get_buf      = scsi_get_buf,
+};
+
+static const SCSIReqOps scsi_disk_dma_reqops = {
+    .size         = sizeof(SCSIDiskReq),
+    .free_req     = scsi_free_request,
+    .send_command = scsi_disk_dma_command,
     .read_data    = scsi_read_data,
     .write_data   = scsi_write_data,
     .cancel_io    = scsi_cancel_io,
@@ -1805,13 +1802,72 @@ static const SCSIReqOps scsi_disk_reqops = {
     .save_request = scsi_disk_save_request,
 };
 
+static const SCSIReqOps *const scsi_disk_reqops_dispatch[256] = {
+    [TEST_UNIT_READY]                 = &scsi_disk_emulate_reqops,
+    [INQUIRY]                         = &scsi_disk_emulate_reqops,
+    [MODE_SENSE]                      = &scsi_disk_emulate_reqops,
+    [MODE_SENSE_10]                   = &scsi_disk_emulate_reqops,
+    [START_STOP]                      = &scsi_disk_emulate_reqops,
+    [ALLOW_MEDIUM_REMOVAL]            = &scsi_disk_emulate_reqops,
+    [READ_CAPACITY_10]                = &scsi_disk_emulate_reqops,
+    [READ_TOC]                        = &scsi_disk_emulate_reqops,
+    [READ_DVD_STRUCTURE]              = &scsi_disk_emulate_reqops,
+    [READ_DISC_INFORMATION]           = &scsi_disk_emulate_reqops,
+    [GET_CONFIGURATION]               = &scsi_disk_emulate_reqops,
+    [GET_EVENT_STATUS_NOTIFICATION]   = &scsi_disk_emulate_reqops,
+    [MECHANISM_STATUS]                = &scsi_disk_emulate_reqops,
+    [SERVICE_ACTION_IN_16]            = &scsi_disk_emulate_reqops,
+    [REQUEST_SENSE]                   = &scsi_disk_emulate_reqops,
+    [SYNCHRONIZE_CACHE]               = &scsi_disk_emulate_reqops,
+    [SEEK_10]                         = &scsi_disk_emulate_reqops,
+#if 0
+    [MODE_SELECT]                     = &scsi_disk_emulate_reqops,
+    [MODE_SELECT_10]                  = &scsi_disk_emulate_reqops,
+#endif
+    [WRITE_SAME_10]                   = &scsi_disk_emulate_reqops,
+    [WRITE_SAME_16]                   = &scsi_disk_emulate_reqops,
+
+    [READ_6]                          = &scsi_disk_dma_reqops,
+    [READ_10]                         = &scsi_disk_dma_reqops,
+    [READ_12]                         = &scsi_disk_dma_reqops,
+    [READ_16]                         = &scsi_disk_dma_reqops,
+    [VERIFY_10]                       = &scsi_disk_dma_reqops,
+    [VERIFY_12]                       = &scsi_disk_dma_reqops,
+    [VERIFY_16]                       = &scsi_disk_dma_reqops,
+    [WRITE_6]                         = &scsi_disk_dma_reqops,
+    [WRITE_10]                        = &scsi_disk_dma_reqops,
+    [WRITE_12]                        = &scsi_disk_dma_reqops,
+    [WRITE_16]                        = &scsi_disk_dma_reqops,
+    [WRITE_VERIFY_10]                 = &scsi_disk_dma_reqops,
+    [WRITE_VERIFY_12]                 = &scsi_disk_dma_reqops,
+    [WRITE_VERIFY_16]                 = &scsi_disk_dma_reqops,
+};
+
 static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
                                      uint8_t *buf, void *hba_private)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
     SCSIRequest *req;
+    const SCSIReqOps *ops;
+    uint8_t command;
 
-    req = scsi_req_alloc(&scsi_disk_reqops, &s->qdev, tag, lun, hba_private);
+#ifdef DEBUG_SCSI
+    DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, buf[0]);
+    {
+        int i;
+        for (i = 1; i < r->req.cmd.len; i++) {
+            printf(" 0x%02x", buf[i]);
+        }
+        printf("\n");
+    }
+#endif
+
+    command = buf[0];
+    ops = scsi_disk_reqops_dispatch[command];
+    if (!ops) {
+        ops = &scsi_disk_emulate_reqops;
+    }
+    req = scsi_req_alloc(ops, &s->qdev, tag, lun, hba_private);
     return req;
 }
 
@@ -1925,15 +1981,14 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
          * unreliable, too.  It is even possible that reads deliver random data
          * from the host page cache (this is probably a Linux bug).
          *
-         * We might use scsi_disk_reqops as long as no writing commands are
+         * We might use scsi_disk_dma_reqops as long as no writing commands are
          * seen, but performance usually isn't paramount on optical media.  So,
          * just make scsi-block operate the same as scsi-generic for them.
          */
-        if (s->qdev.type == TYPE_ROM) {
-            break;
-	}
-        return scsi_req_alloc(&scsi_disk_reqops, &s->qdev, tag, lun,
-                              hba_private);
+        if (s->qdev.type != TYPE_ROM) {
+            return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun,
+                                  hba_private);
+        }
     }
 
     return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun,
-- 
1.7.10.4

[Qemu-devel] [PATCH 12/32] scsi-disk: separate read_data/write_data implementation for emulate_reqops

[Paolo Bonzini]

The previous patch only separated the send_command callback.
Use different implementations also for read_data and write_data.
The latter is still unreachable, so it aborts for now.

read_data passes the data buffer that was prepared and completes
the command.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |   36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index ea0b05e..1633177 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -343,13 +343,6 @@ static void scsi_read_data(SCSIRequest *req)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     bool first;
 
-    if (r->sector_count == (uint32_t)-1) {
-        DPRINTF("Read buf_len=%zd\n", r->iov.iov_len);
-        r->sector_count = 0;
-        r->started = true;
-        scsi_req_data(&r->req, r->iov.iov_len);
-        return;
-    }
     DPRINTF("Read sector_count=%d\n", r->sector_count);
     if (r->sector_count == 0) {
         /* This also clears the sense buffer for REQUEST SENSE.  */
@@ -1262,6 +1255,28 @@ static int scsi_disk_emulate_start_stop(SCSIDiskReq *r)
     return 0;
 }
 
+static void scsi_disk_emulate_read_data(SCSIRequest *req)
+{
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
+    int buflen = r->iov.iov_len;
+
+    if (buflen) {
+        DPRINTF("Read buf_len=%zd\n", buflen);
+        r->iov.iov_len = 0;
+        r->started = true;
+        scsi_req_data(&r->req, buflen);
+        return;
+    }
+
+    /* This also clears the sense buffer for REQUEST SENSE.  */
+    scsi_req_complete(&r->req, GOOD);
+}
+
+static void scsi_disk_emulate_write_data(SCSIRequest *req)
+{
+    abort();
+}
+
 static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
@@ -1532,9 +1547,8 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
         scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
         return 0;
     }
-    assert(!r->req.aiocb && r->sector_count == 0);
+    assert(!r->req.aiocb);
     r->iov.iov_len = MIN(buflen, req->cmd.xfer);
-    r->sector_count = -1;
     if (r->iov.iov_len == 0) {
         scsi_req_complete(&r->req, GOOD);
     }
@@ -1785,8 +1799,8 @@ static const SCSIReqOps scsi_disk_emulate_reqops = {
     .size         = sizeof(SCSIDiskReq),
     .free_req     = scsi_free_request,
     .send_command = scsi_disk_emulate_command,
-    .read_data    = scsi_read_data,
-    .write_data   = scsi_write_data,
+    .read_data    = scsi_disk_emulate_read_data,
+    .write_data   = scsi_disk_emulate_write_data,
     .get_buf      = scsi_get_buf,
 };
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 13/32] scsi-disk: support emulated TO_DEV requests

[Paolo Bonzini]

This adds the implementation of write_data for the emulated
command case.  The first time through it asks for more data,
the second time it finishes the processing of the command.

MODE SELECT and MODE SELECT(10) can now be re-enabled, but they
will not do much.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |   36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 1633177..b8d6086 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1274,7 +1274,26 @@ static void scsi_disk_emulate_read_data(SCSIRequest *req)
 
 static void scsi_disk_emulate_write_data(SCSIRequest *req)
 {
-    abort();
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
+
+    if (r->iov.iov_len) {
+        int buflen = r->iov.iov_len;
+        DPRINTF("Write buf_len=%zd\n", buflen);
+        r->iov.iov_len = 0;
+        scsi_req_data(&r->req, buflen);
+        return;
+    }
+
+    switch (req->cmd.buf[0]) {
+    case MODE_SELECT:
+    case MODE_SELECT_10:
+        /* This also clears the sense buffer for REQUEST SENSE.  */
+        scsi_req_complete(&r->req, GOOD);
+        break;
+
+    default:
+        abort();
+    }
 }
 
 static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
@@ -1283,7 +1302,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
     uint64_t nb_sectors;
     uint8_t *outbuf;
-    int buflen = 0;
+    int buflen;
 
     switch (req->cmd.buf[0]) {
     case INQUIRY:
@@ -1309,7 +1328,6 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
         break;
     }
 
-    assert(req->cmd.mode != SCSI_XFER_TO_DEV);
     if (!r->iov.iov_base) {
         /*
          * FIXME: we shouldn't return anything bigger than 4k, but the code
@@ -1326,6 +1344,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
         r->iov.iov_base = qemu_blockalign(s->qdev.conf.bs, r->buflen);
     }
 
+    buflen = req->cmd.xfer;
     outbuf = r->iov.iov_base;
     switch (req->cmd.buf[0]) {
     case TEST_UNIT_READY:
@@ -1500,7 +1519,6 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
             goto illegal_lba;
         }
         break;
-#if 0
     case MODE_SELECT:
         DPRINTF("Mode Select(6) (len %lu)\n", (long)r->req.cmd.xfer);
         /* We don't support mode parameter changes.
@@ -1517,7 +1535,6 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
             goto illegal_request;
         }
         break;
-#endif
     case WRITE_SAME_10:
         nb_sectors = lduw_be_p(&req->cmd.buf[7]);
         goto write_same;
@@ -1552,7 +1569,12 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
     if (r->iov.iov_len == 0) {
         scsi_req_complete(&r->req, GOOD);
     }
-    return r->iov.iov_len;
+    if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
+        assert(r->iov.iov_len == req->cmd.xfer);
+        return -r->iov.iov_len;
+    } else {
+        return r->iov.iov_len;
+    }
 
 illegal_request:
     if (r->req.status == -1) {
@@ -1834,10 +1856,8 @@ static const SCSIReqOps *const scsi_disk_reqops_dispatch[256] = {
     [REQUEST_SENSE]                   = &scsi_disk_emulate_reqops,
     [SYNCHRONIZE_CACHE]               = &scsi_disk_emulate_reqops,
     [SEEK_10]                         = &scsi_disk_emulate_reqops,
-#if 0
     [MODE_SELECT]                     = &scsi_disk_emulate_reqops,
     [MODE_SELECT_10]                  = &scsi_disk_emulate_reqops,
-#endif
     [WRITE_SAME_10]                   = &scsi_disk_emulate_reqops,
     [WRITE_SAME_16]                   = &scsi_disk_emulate_reqops,
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 14/32] scsi-disk: adjust offsets in MODE SENSE by 2

[Paolo Bonzini]

This will make offsets the same when implementing MODE SELECT.  This is
because MODE SELECT has to deal with both 2-byte and 4-byte headers.
Unfortunately, this means that the offsets are now off by two compared
to the descriptions in the SCSI specs, which include the header.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |  140 ++++++++++++++++++++++++++++++--------------------------
 1 file changed, 74 insertions(+), 66 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b8d6086..dca9866 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -959,148 +959,156 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         [MODE_PAGE_AUDIO_CTL]              = (1 << TYPE_ROM),
         [MODE_PAGE_CAPABILITIES]           = (1 << TYPE_ROM),
     };
-    uint8_t *p = *p_outbuf;
+
+    uint8_t *p = *p_outbuf + 2;
+    int length;
 
     if ((mode_sense_valid[page] & (1 << s->qdev.type)) == 0) {
         return -1;
     }
 
-    p[0] = page;
-
     /*
      * If Changeable Values are requested, a mask denoting those mode parameters
      * that are changeable shall be returned. As we currently don't support
      * parameter changes via MODE_SELECT all bits are returned set to zero.
      * The buffer was already menset to zero by the caller of this function.
+     *
+     * The offsets here are off by two compared to the descriptions in the
+     * SCSI specs, because those include a 2-byte header.  This is unfortunate,
+     * but it is done so that offsets are consistent within our implementation
+     * of MODE SENSE and MODE SELECT.  MODE SELECT has to deal with both
+     * 2-byte and 4-byte headers.
      */
     switch (page) {
     case MODE_PAGE_HD_GEOMETRY:
-        p[1] = 0x16;
+        length = 0x16;
         if (page_control == 1) { /* Changeable Values */
             break;
         }
         /* if a geometry hint is available, use it */
-        p[2] = (s->qdev.conf.cyls >> 16) & 0xff;
-        p[3] = (s->qdev.conf.cyls >> 8) & 0xff;
-        p[4] = s->qdev.conf.cyls & 0xff;
-        p[5] = s->qdev.conf.heads & 0xff;
+        p[0] = (s->qdev.conf.cyls >> 16) & 0xff;
+        p[1] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[2] = s->qdev.conf.cyls & 0xff;
+        p[3] = s->qdev.conf.heads & 0xff;
         /* Write precomp start cylinder, disabled */
-        p[6] = (s->qdev.conf.cyls >> 16) & 0xff;
-        p[7] = (s->qdev.conf.cyls >> 8) & 0xff;
-        p[8] = s->qdev.conf.cyls & 0xff;
+        p[4] = (s->qdev.conf.cyls >> 16) & 0xff;
+        p[5] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[6] = s->qdev.conf.cyls & 0xff;
         /* Reduced current start cylinder, disabled */
-        p[9] = (s->qdev.conf.cyls >> 16) & 0xff;
-        p[10] = (s->qdev.conf.cyls >> 8) & 0xff;
-        p[11] = s->qdev.conf.cyls & 0xff;
+        p[7] = (s->qdev.conf.cyls >> 16) & 0xff;
+        p[8] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[9] = s->qdev.conf.cyls & 0xff;
         /* Device step rate [ns], 200ns */
-        p[12] = 0;
-        p[13] = 200;
+        p[10] = 0;
+        p[11] = 200;
         /* Landing zone cylinder */
+        p[12] = 0xff;
+        p[13] =  0xff;
         p[14] = 0xff;
-        p[15] =  0xff;
-        p[16] = 0xff;
         /* Medium rotation rate [rpm], 5400 rpm */
-        p[20] = (5400 >> 8) & 0xff;
-        p[21] = 5400 & 0xff;
+        p[18] = (5400 >> 8) & 0xff;
+        p[19] = 5400 & 0xff;
         break;
 
     case MODE_PAGE_FLEXIBLE_DISK_GEOMETRY:
-        p[1] = 0x1e;
+        length = 0x1e;
         if (page_control == 1) { /* Changeable Values */
             break;
         }
         /* Transfer rate [kbit/s], 5Mbit/s */
-        p[2] = 5000 >> 8;
-        p[3] = 5000 & 0xff;
+        p[0] = 5000 >> 8;
+        p[1] = 5000 & 0xff;
         /* if a geometry hint is available, use it */
-        p[4] = s->qdev.conf.heads & 0xff;
-        p[5] = s->qdev.conf.secs & 0xff;
-        p[6] = s->qdev.blocksize >> 8;
+        p[2] = s->qdev.conf.heads & 0xff;
+        p[3] = s->qdev.conf.secs & 0xff;
+        p[4] = s->qdev.blocksize >> 8;
+        p[6] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[7] = s->qdev.conf.cyls & 0xff;
+        /* Write precomp start cylinder, disabled */
         p[8] = (s->qdev.conf.cyls >> 8) & 0xff;
         p[9] = s->qdev.conf.cyls & 0xff;
-        /* Write precomp start cylinder, disabled */
+        /* Reduced current start cylinder, disabled */
         p[10] = (s->qdev.conf.cyls >> 8) & 0xff;
         p[11] = s->qdev.conf.cyls & 0xff;
-        /* Reduced current start cylinder, disabled */
-        p[12] = (s->qdev.conf.cyls >> 8) & 0xff;
-        p[13] = s->qdev.conf.cyls & 0xff;
         /* Device step rate [100us], 100us */
-        p[14] = 0;
-        p[15] = 1;
+        p[12] = 0;
+        p[13] = 1;
         /* Device step pulse width [us], 1us */
-        p[16] = 1;
+        p[14] = 1;
         /* Device head settle delay [100us], 100us */
-        p[17] = 0;
-        p[18] = 1;
+        p[15] = 0;
+        p[16] = 1;
         /* Motor on delay [0.1s], 0.1s */
-        p[19] = 1;
+        p[17] = 1;
         /* Motor off delay [0.1s], 0.1s */
-        p[20] = 1;
+        p[18] = 1;
         /* Medium rotation rate [rpm], 5400 rpm */
-        p[28] = (5400 >> 8) & 0xff;
-        p[29] = 5400 & 0xff;
+        p[26] = (5400 >> 8) & 0xff;
+        p[27] = 5400 & 0xff;
         break;
 
     case MODE_PAGE_CACHING:
-        p[0] = 8;
-        p[1] = 0x12;
+        length = 0x12;
         if (page_control == 1) { /* Changeable Values */
             break;
         }
         if (bdrv_enable_write_cache(s->qdev.conf.bs)) {
-            p[2] = 4; /* WCE */
+            p[0] = 4; /* WCE */
         }
         break;
 
     case MODE_PAGE_R_W_ERROR:
-        p[1] = 10;
-        p[2] = 0x80; /* Automatic Write Reallocation Enabled */
+        length = 10;
+        p[0] = 0x80; /* Automatic Write Reallocation Enabled */
         if (s->qdev.type == TYPE_ROM) {
-            p[3] = 0x20; /* Read Retry Count */
+            p[1] = 0x20; /* Read Retry Count */
         }
         break;
 
     case MODE_PAGE_AUDIO_CTL:
-        p[1] = 14;
+        length = 14;
         break;
 
     case MODE_PAGE_CAPABILITIES:
-        p[1] = 0x14;
+        length = 0x14;
         if (page_control == 1) { /* Changeable Values */
             break;
         }
 
-        p[2] = 0x3b; /* CD-R & CD-RW read */
-        p[3] = 0; /* Writing not supported */
-        p[4] = 0x7f; /* Audio, composite, digital out,
+        p[0] = 0x3b; /* CD-R & CD-RW read */
+        p[1] = 0; /* Writing not supported */
+        p[2] = 0x7f; /* Audio, composite, digital out,
                         mode 2 form 1&2, multi session */
-        p[5] = 0xff; /* CD DA, DA accurate, RW supported,
+        p[3] = 0xff; /* CD DA, DA accurate, RW supported,
                         RW corrected, C2 errors, ISRC,
                         UPC, Bar code */
-        p[6] = 0x2d | (s->tray_locked ? 2 : 0);
+        p[4] = 0x2d | (s->tray_locked ? 2 : 0);
         /* Locking supported, jumper present, eject, tray */
-        p[7] = 0; /* no volume & mute control, no
+        p[5] = 0; /* no volume & mute control, no
                      changer */
-        p[8] = (50 * 176) >> 8; /* 50x read speed */
-        p[9] = (50 * 176) & 0xff;
-        p[10] = 2 >> 8; /* Two volume levels */
-        p[11] = 2 & 0xff;
-        p[12] = 2048 >> 8; /* 2M buffer */
-        p[13] = 2048 & 0xff;
-        p[14] = (16 * 176) >> 8; /* 16x read speed current */
-        p[15] = (16 * 176) & 0xff;
-        p[18] = (16 * 176) >> 8; /* 16x write speed */
+        p[6] = (50 * 176) >> 8; /* 50x read speed */
+        p[7] = (50 * 176) & 0xff;
+        p[8] = 2 >> 8; /* Two volume levels */
+        p[9] = 2 & 0xff;
+        p[10] = 2048 >> 8; /* 2M buffer */
+        p[11] = 2048 & 0xff;
+        p[12] = (16 * 176) >> 8; /* 16x read speed current */
+        p[13] = (16 * 176) & 0xff;
+        p[16] = (16 * 176) >> 8; /* 16x write speed */
+        p[17] = (16 * 176) & 0xff;
+        p[18] = (16 * 176) >> 8; /* 16x write speed current */
         p[19] = (16 * 176) & 0xff;
-        p[20] = (16 * 176) >> 8; /* 16x write speed current */
-        p[21] = (16 * 176) & 0xff;
         break;
 
     default:
         return -1;
     }
 
-    *p_outbuf += p[1] + 2;
-    return p[1] + 2;
+    assert(length < 256);
+    (*p_outbuf)[0] = page;
+    (*p_outbuf)[1] = length;
+    *p_outbuf += length + 2;
+    return length + 2;
 }
 
 static int scsi_disk_emulate_mode_sense(SCSIDiskReq *r, uint8_t *outbuf)
-- 
1.7.10.4

[Qemu-devel] [PATCH 15/32] scsi-disk: fix changeable values for MODE_PAGE_R_W_ERROR

[Paolo Bonzini]

The changeable values were not all-zeros for this mode page, fix it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index dca9866..7015420 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1059,6 +1059,9 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
 
     case MODE_PAGE_R_W_ERROR:
         length = 10;
+        if (page_control == 1) { /* Changeable Values */
+            break;
+        }
         p[0] = 0x80; /* Automatic Write Reallocation Enabled */
         if (s->qdev.type == TYPE_ROM) {
             p[1] = 0x20; /* Read Retry Count */
-- 
1.7.10.4

[Qemu-devel] [PATCH 16/32] scsi-disk: parse MODE SELECT commands and parameters

[Paolo Bonzini]

This adds the bulk of the parsing code for MODE SELECT, including
breaking out changes to different mode pages, and checking that only
changeable values are modified.

In order to report errors correctly two passes are made through the
parameters; the first only looks for errors, the second actually
applies the changes to the mode page.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c  |   10 ++++
 hw/scsi-disk.c |  166 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 hw/scsi.h      |    4 ++
 3 files changed, 169 insertions(+), 11 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index dc74063..efbda6f 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1112,6 +1112,16 @@ const struct SCSISense sense_code_INVALID_FIELD = {
     .key = ILLEGAL_REQUEST, .asc = 0x24, .ascq = 0x00
 };
 
+/* Illegal request, Invalid field in parameter list */
+const struct SCSISense sense_code_INVALID_PARAM = {
+    .key = ILLEGAL_REQUEST, .asc = 0x26, .ascq = 0x00
+};
+
+/* Illegal request, Parameter list length error */
+const struct SCSISense sense_code_INVALID_PARAM_LEN = {
+    .key = ILLEGAL_REQUEST, .asc = 0x1a, .ascq = 0x00
+};
+
 /* Illegal request, LUN not supported */
 const struct SCSISense sense_code_LUN_NOT_SUPPORTED = {
     .key = ILLEGAL_REQUEST, .asc = 0x25, .ascq = 0x00
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 7015420..ad6c00d 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -43,6 +43,7 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 
 #define SCSI_DMA_BUF_SIZE    131072
 #define SCSI_MAX_INQUIRY_LEN 256
+#define SCSI_MAX_MODE_LEN    256
 
 typedef struct SCSIDiskState SCSIDiskState;
 
@@ -1283,6 +1284,159 @@ static void scsi_disk_emulate_read_data(SCSIRequest *req)
     scsi_req_complete(&r->req, GOOD);
 }
 
+static int scsi_disk_check_mode_select(SCSIDiskState *s, int page,
+                                       uint8_t *inbuf, int inlen)
+{
+    uint8_t mode_current[SCSI_MAX_MODE_LEN];
+    uint8_t mode_changeable[SCSI_MAX_MODE_LEN];
+    uint8_t *p;
+    int len, expected_len, changeable_len, i;
+
+    /* The input buffer does not include the page header, so it is
+     * off by 2 bytes.
+     */
+    expected_len = inlen + 2;
+    if (expected_len > SCSI_MAX_MODE_LEN) {
+        return -1;
+    }
+
+    p = mode_current;
+    memset(mode_current, 0, inlen + 2);
+    len = mode_sense_page(s, page, &p, 0);
+    if (len < 0 || len != expected_len) {
+        return -1;
+    }
+
+    p = mode_changeable;
+    memset(mode_changeable, 0, inlen + 2);
+    changeable_len = mode_sense_page(s, page, &p, 1);
+    assert(changeable_len == len);
+
+    /* Check that unchangeable bits are the same as what MODE SENSE
+     * would return.
+     */
+    for (i = 2; i < len; i++) {
+        if (((mode_current[i] ^ inbuf[i - 2]) & ~mode_changeable[i]) != 0) {
+            return -1;
+        }
+    }
+    return 0;
+}
+
+static void scsi_disk_apply_mode_select(SCSIDiskState *s, int page, uint8_t *p)
+{
+}
+
+static int mode_select_pages(SCSIDiskReq *r, uint8_t *p, int len, bool change)
+{
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
+    while (len > 0) {
+        int page, subpage, page_len;
+
+        /* Parse both possible formats for the mode page headers.  */
+        page = p[0] & 0x3f;
+        if (p[0] & 0x40) {
+            if (len < 4) {
+                goto invalid_param_len;
+            }
+            subpage = p[1];
+            page_len = lduw_be_p(&p[2]);
+            p += 4;
+            len -= 4;
+        } else {
+            if (len < 2) {
+                goto invalid_param_len;
+            }
+            subpage = 0;
+            page_len = p[1];
+            p += 2;
+            len -= 2;
+        }
+
+        if (subpage) {
+            goto invalid_param;
+        }
+        if (page_len > len) {
+            goto invalid_param_len;
+        }
+
+        if (!change) {
+            if (scsi_disk_check_mode_select(s, page, p, page_len) < 0) {
+                goto invalid_param;
+            }
+        } else {
+            scsi_disk_apply_mode_select(s, page, p);
+        }
+
+        p += page_len;
+        len -= page_len;
+    }
+    return 0;
+
+invalid_param:
+    scsi_check_condition(r, SENSE_CODE(INVALID_PARAM));
+    return -1;
+
+invalid_param_len:
+    scsi_check_condition(r, SENSE_CODE(INVALID_PARAM_LEN));
+    return -1;
+}
+
+static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
+{
+    uint8_t *p = inbuf;
+    int cmd = r->req.cmd.buf[0];
+    int len = r->req.cmd.xfer;
+    int hdr_len = (cmd == MODE_SELECT ? 4 : 8);
+    int bd_len;
+    int pass;
+
+    /* We only support PF=1, SP=0.  */
+    if ((r->req.cmd.buf[1] & 0x11) != 0x10) {
+        goto invalid_field;
+    }
+
+    if (len < hdr_len) {
+        goto invalid_param_len;
+    }
+
+    bd_len = (cmd == MODE_SELECT ? p[3] : lduw_be_p(&p[6]));
+    len -= hdr_len;
+    p += hdr_len;
+    if (len < bd_len) {
+        goto invalid_param_len;
+    }
+    if (bd_len != 0 && bd_len != 8) {
+        goto invalid_param;
+    }
+
+    len -= bd_len;
+    p += bd_len;
+
+    /* Ensure no change is made if there is an error!  */
+    for (pass = 0; pass < 2; pass++) {
+        if (mode_select_pages(r, p, len, pass == 1) < 0) {
+            assert(pass == 0);
+            return;
+        }
+    }
+    scsi_req_complete(&r->req, GOOD);
+    return;
+
+invalid_param:
+    scsi_check_condition(r, SENSE_CODE(INVALID_PARAM));
+    return;
+
+invalid_param_len:
+    scsi_check_condition(r, SENSE_CODE(INVALID_PARAM_LEN));
+    return;
+
+invalid_field:
+    scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
+    return;
+}
+
 static void scsi_disk_emulate_write_data(SCSIRequest *req)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
@@ -1299,7 +1453,7 @@ static void scsi_disk_emulate_write_data(SCSIRequest *req)
     case MODE_SELECT:
     case MODE_SELECT_10:
         /* This also clears the sense buffer for REQUEST SENSE.  */
-        scsi_req_complete(&r->req, GOOD);
+        scsi_disk_emulate_mode_select(r, r->iov.iov_base);
         break;
 
     default:
@@ -1532,19 +1686,9 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
         break;
     case MODE_SELECT:
         DPRINTF("Mode Select(6) (len %lu)\n", (long)r->req.cmd.xfer);
-        /* We don't support mode parameter changes.
-           Allow the mode parameter header + block descriptors only. */
-        if (r->req.cmd.xfer > 12) {
-            goto illegal_request;
-        }
         break;
     case MODE_SELECT_10:
         DPRINTF("Mode Select(10) (len %lu)\n", (long)r->req.cmd.xfer);
-        /* We don't support mode parameter changes.
-           Allow the mode parameter header + block descriptors only. */
-        if (r->req.cmd.xfer > 16) {
-            goto illegal_request;
-        }
         break;
     case WRITE_SAME_10:
         nb_sectors = lduw_be_p(&req->cmd.buf[7]);
diff --git a/hw/scsi.h b/hw/scsi.h
index ea8a155..e2fb8a4 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -180,6 +180,10 @@ extern const struct SCSISense sense_code_INVALID_OPCODE;
 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, Invalid field in parameter list */
+extern const struct SCSISense sense_code_INVALID_PARAM;
+/* Illegal request, Parameter list length error */
+extern const struct SCSISense sense_code_INVALID_PARAM_LEN;
 /* Illegal request, LUN not supported */
 extern const struct SCSISense sense_code_LUN_NOT_SUPPORTED;
 /* Illegal request, Saving parameters not supported */
-- 
1.7.10.4

[Qemu-devel] [PATCH 17/32] scsi-disk: support toggling the write cache

[Paolo Bonzini]

Finally, this uses the "plumbing" in the previous patch to
add support for toggling the WCE bit of the caching mode page.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index ad6c00d..fd5cd70 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1050,10 +1050,8 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
 
     case MODE_PAGE_CACHING:
         length = 0x12;
-        if (page_control == 1) { /* Changeable Values */
-            break;
-        }
-        if (bdrv_enable_write_cache(s->qdev.conf.bs)) {
+        if (page_control == 1 || /* Changeable Values */
+            bdrv_enable_write_cache(s->qdev.conf.bs)) {
             p[0] = 4; /* WCE */
         }
         break;
@@ -1325,6 +1323,14 @@ static int scsi_disk_check_mode_select(SCSIDiskState *s, int page,
 
 static void scsi_disk_apply_mode_select(SCSIDiskState *s, int page, uint8_t *p)
 {
+    switch (page) {
+    case MODE_PAGE_CACHING:
+        bdrv_set_enable_write_cache(s->qdev.conf.bs, (p[0] & 4) != 0);
+        break;
+
+    default:
+        break;
+    }
 }
 
 static int mode_select_pages(SCSIDiskReq *r, uint8_t *p, int len, bool change)
-- 
1.7.10.4

[Qemu-devel] [PATCH 18/32] scsi-disk: rd/wr/vr-protect !=0 is an error

[Paolo Bonzini]

From: Ronnie Sahlberg <ronniesahlberg@gmail.com>

The QEMU SCSI emulation does not support protection information,
so any READ/WRITE/VERIFY commands that has the protect bits set to
non-zero should fail with ILLEGAL_REQUEST/INVALID_FIELD_IN_CDB

>From SCSI SBC :
If the logical unit does not support protection information,
then the device server should terminate the command with CHECK CONDITION
status with the sense key set to ILLEGAL REQUEST and the additional sense
code set to INVALID FIELD IN CDB.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
[ Rebase after scsi_dma_reqops introduction - Paolo ]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index fd5cd70..526da4b 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1774,6 +1774,9 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
     case READ_16:
         len = r->req.cmd.xfer / s->qdev.blocksize;
         DPRINTF("Read (sector %" PRId64 ", count %d)\n", r->req.cmd.lba, len);
+        if (r->req.cmd.buf[1] & 0xe0) {
+            goto illegal_request;
+        }
         if (r->req.cmd.lba > s->qdev.max_lba) {
             goto illegal_lba;
         }
@@ -1794,6 +1797,9 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
         DPRINTF("Write %s(sector %" PRId64 ", count %d)\n",
                 (command & 0xe) == 0xe ? "And Verify " : "",
                 r->req.cmd.lba, len);
+        if (r->req.cmd.buf[1] & 0xe0) {
+            goto illegal_request;
+        }
         if (r->req.cmd.lba > s->qdev.max_lba) {
             goto illegal_lba;
         }
@@ -1802,6 +1808,9 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
         break;
     default:
         abort();
+    illegal_request:
+        scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
+        return 0;
     illegal_lba:
         scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
         return 0;
-- 
1.7.10.4

[Qemu-devel] [PATCH 19/32] scsi-disk: improve the lba-out-of-range tests for read/write/verify

[Paolo Bonzini]

From: Ronnie Sahlberg <ronniesahlberg@gmail.com>

Improve the tests for the LBA to cover more cases.

For the 16 byte opcodes, the lba is a uint64, so we need to check is to
make sure that we do not wrap.  For example if an opcode would specify
the LBA:0xffffffffffffffff and LEN:2 then lba+len would wrap to 1.

Also verify that ALL requested blocks are available, not just the first one.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 526da4b..3c03159 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1777,7 +1777,8 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
         if (r->req.cmd.buf[1] & 0xe0) {
             goto illegal_request;
         }
-        if (r->req.cmd.lba > s->qdev.max_lba) {
+        if (r->req.cmd.lba > r->req.cmd.lba + len ||
+            r->req.cmd.lba + len - 1 > s->qdev.max_lba) {
             goto illegal_lba;
         }
         r->sector = r->req.cmd.lba * (s->qdev.blocksize / 512);
@@ -1800,7 +1801,8 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
         if (r->req.cmd.buf[1] & 0xe0) {
             goto illegal_request;
         }
-        if (r->req.cmd.lba > s->qdev.max_lba) {
+        if (r->req.cmd.lba > r->req.cmd.lba + len ||
+            r->req.cmd.lba + len - 1 > s->qdev.max_lba) {
             goto illegal_lba;
         }
         r->sector = r->req.cmd.lba * (s->qdev.blocksize / 512);
-- 
1.7.10.4

[Qemu-devel] [PATCH 20/32] scsi-disk: Fail medium writes with proper sense for readonly LUNs

[Paolo Bonzini]

From: Ronnie Sahlberg <ronniesahlberg@gmail.com>

Add sense code for DATA_PROTECT/WRITE_PROTECTED and return this error
for any WRITE*/WRITE_VERIFY* calls if the device is readonly=on,
i.e. write-protected

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c  |    5 +++++
 hw/scsi-disk.c |   15 ++++++++++++---
 hw/scsi.h      |    2 ++
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index efbda6f..dd0cdd0 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1182,6 +1182,11 @@ const struct SCSISense sense_code_DEVICE_INTERNAL_RESET = {
     .key = UNIT_ATTENTION, .asc = 0x29, .ascq = 0x04
 };
 
+/* Data Protection, Write Protected */
+const struct SCSISense sense_code_WRITE_PROTECTED = {
+    .key = DATA_PROTECT, .asc = 0x27, .ascq = 0x00
+};
+
 /*
  * scsi_build_sense
  *
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 3c03159..fb0540a 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1702,6 +1702,10 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
     case WRITE_SAME_16:
         nb_sectors = ldl_be_p(&req->cmd.buf[10]) & 0xffffffffULL;
     write_same:
+        if (bdrv_is_read_only(s->qdev.conf.bs)) {
+            scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));
+            return 0;
+        }
         if (r->req.cmd.lba > s->qdev.max_lba) {
             goto illegal_lba;
         }
@@ -1784,9 +1788,6 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
         r->sector = r->req.cmd.lba * (s->qdev.blocksize / 512);
         r->sector_count = len * (s->qdev.blocksize / 512);
         break;
-    case VERIFY_10:
-    case VERIFY_12:
-    case VERIFY_16:
     case WRITE_6:
     case WRITE_10:
     case WRITE_12:
@@ -1794,6 +1795,14 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
     case WRITE_VERIFY_10:
     case WRITE_VERIFY_12:
     case WRITE_VERIFY_16:
+        if (bdrv_is_read_only(s->qdev.conf.bs)) {
+            scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));
+            return 0;
+        }
+        /* fallthrough */
+    case VERIFY_10:
+    case VERIFY_12:
+    case VERIFY_16:
         len = r->req.cmd.xfer / s->qdev.blocksize;
         DPRINTF("Write %s(sector %" PRId64 ", count %d)\n",
                 (command & 0xe) == 0xe ? "And Verify " : "",
diff --git a/hw/scsi.h b/hw/scsi.h
index e2fb8a4..fc9dc50 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -208,6 +208,8 @@ extern const struct SCSISense sense_code_MEDIUM_CHANGED;
 extern const struct SCSISense sense_code_REPORTED_LUNS_CHANGED;
 /* Unit attention, Device internal reset */
 extern const struct SCSISense sense_code_DEVICE_INTERNAL_RESET;
+/* Data Protection, Write Protected */
+extern const struct SCSISense sense_code_WRITE_PROTECTED;
 
 #define SENSE_CODE(x) sense_code_ ## x
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 21/32] scsi-disk: removable hard disks support load/eject

[Paolo Bonzini]

Support for the LOEJ bit of the START/STOP UNIT command right now is
limited to CD-ROMs.  This is wrong, since removable hard disks (in the
real world: SD card readers) also support it in pretty much the same way.

Without the LOEJ bit, START/STOP UNIT does nothing for all devices.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index fb0540a..b52e304 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1248,7 +1248,7 @@ static int scsi_disk_emulate_start_stop(SCSIDiskReq *r)
     bool start = req->cmd.buf[4] & 1;
     bool loej = req->cmd.buf[4] & 2; /* load on start, eject on !start */
 
-    if (s->qdev.type == TYPE_ROM && loej) {
+    if ((s->features & (1 << SCSI_DISK_F_REMOVABLE)) && loej) {
         if (!start && !s->tray_open && s->tray_locked) {
             scsi_check_condition(r,
                                  bdrv_is_inserted(s->qdev.conf.bs)
-- 
1.7.10.4

[Qemu-devel] [PATCH 22/32] scsi: add tracepoint for scsi_req_cancel

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c |    1 +
 trace-events  |    1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index dd0cdd0..543d9d2 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1496,6 +1496,7 @@ void scsi_req_complete(SCSIRequest *req, int status)
 
 void scsi_req_cancel(SCSIRequest *req)
 {
+    trace_scsi_req_cancel(req->dev->id, req->lun, req->tag);
     if (!req->enqueued) {
         return;
     }
diff --git a/trace-events b/trace-events
index 6fb5eb5..7aa28b1 100644
--- a/trace-events
+++ b/trace-events
@@ -403,6 +403,7 @@ usb_host_parse_error(int bus, int addr, const char *errmsg) "dev %d:%d, msg %s"
 
 # hw/scsi-bus.c
 scsi_req_alloc(int target, int lun, int tag) "target %d lun %d tag %d"
+scsi_req_cancel(int target, int lun, int tag) "target %d lun %d tag %d"
 scsi_req_data(int target, int lun, int tag, int len) "target %d lun %d tag %d len %d"
 scsi_req_data_canceled(int target, int lun, int tag, int len) "target %d lun %d tag %d len %d"
 scsi_req_dequeue(int target, int lun, int tag) "target %d lun %d tag %d"
-- 
1.7.10.4

[Qemu-devel] [PATCH 23/32] scsi: introduce hotplug() and hot_unplug() interfaces for SCSI bus

[Paolo Bonzini]

From: Cong Meng <mc@linux.vnet.ibm.com>

Add two interfaces hotplug() and hot_unplug() to scsi bus info.
The scsi bus can implement these two interfaces to signal the HBA driver
of guest kernel to add/remove the scsi device in question.

Signed-off-by: Sen Wang <senwang@linux.vnet.ibm.com>
Signed-off-by: Cong Meng <mc@linux.vnet.ibm.com>
[ Fixed braces and indentation - Paolo ]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c |   17 ++++++++++++++++-
 hw/scsi.h     |    2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 543d9d2..6ee0c10 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -186,6 +186,10 @@ static int scsi_qdev_init(DeviceState *qdev)
                                                          dev);
     }
 
+    if (bus->info->hotplug) {
+        bus->info->hotplug(bus, dev);
+    }
+
 err:
     return rc;
 }
@@ -1650,6 +1654,17 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
+static int scsi_qdev_unplug(DeviceState *qdev)
+{
+    SCSIDevice *dev = SCSI_DEVICE(qdev);
+    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
+
+    if (bus->info->hot_unplug) {
+        bus->info->hot_unplug(bus, dev);
+    }
+    return qdev_simple_unplug_cb(qdev);
+}
+
 static const VMStateInfo vmstate_info_scsi_requests = {
     .name = "scsi-requests",
     .get  = get_scsi_requests,
@@ -1686,7 +1701,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->bus_type = TYPE_SCSI_BUS;
     k->init     = scsi_qdev_init;
-    k->unplug   = qdev_simple_unplug_cb;
+    k->unplug   = scsi_qdev_unplug;
     k->exit     = scsi_qdev_exit;
     k->props    = scsi_props;
 }
diff --git a/hw/scsi.h b/hw/scsi.h
index fc9dc50..088d519 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -131,6 +131,8 @@ struct SCSIBusInfo {
     void (*transfer_data)(SCSIRequest *req, uint32_t arg);
     void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
     void (*cancel)(SCSIRequest *req);
+    void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
+    void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
     QEMUSGList *(*get_sg_list)(SCSIRequest *req);
 
     void (*save_request)(QEMUFile *f, SCSIRequest *req);
-- 
1.7.10.4

[Qemu-devel] [PATCH 24/32] scsi: establish precedence levels for unit attention

[Paolo Bonzini]

When a device is resized, we will report a unit attention condition
for CAPACITY DATA HAS CHANGED.  However, we should ensure that this
condition does not override a more important unit attention condition.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c  |   52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/scsi-disk.c |    4 ++--
 hw/scsi.h      |    1 +
 trace-events   |    1 +
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 6ee0c10..c38c0ec 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1531,6 +1531,55 @@ void scsi_req_abort(SCSIRequest *req, int status)
     scsi_req_unref(req);
 }
 
+static int scsi_ua_precedence(SCSISense sense)
+{
+    if (sense.key != UNIT_ATTENTION) {
+        return INT_MAX;
+    }
+    if (sense.asc == 0x29 && sense.ascq == 0x04) {
+        /* DEVICE INTERNAL RESET goes with POWER ON OCCURRED */
+        return 1;
+    } else if (sense.asc == 0x3F && sense.ascq == 0x01) {
+        /* MICROCODE HAS BEEN CHANGED goes with SCSI BUS RESET OCCURRED */
+        return 2;
+    } else if (sense.asc == 0x29 && (sense.ascq == 0x05 || sense.ascq == 0x06)) {
+        /* These two go with "all others". */
+        ;
+    } else if (sense.asc == 0x29 && sense.ascq <= 0x07) {
+        /* POWER ON, RESET OR BUS DEVICE RESET OCCURRED = 0
+         * POWER ON OCCURRED = 1
+         * SCSI BUS RESET OCCURRED = 2
+         * BUS DEVICE RESET FUNCTION OCCURRED = 3
+         * I_T NEXUS LOSS OCCURRED = 7
+         */
+        return sense.ascq;
+    } else if (sense.asc == 0x2F && sense.ascq == 0x01) {
+        /* COMMANDS CLEARED BY POWER LOSS NOTIFICATION  */
+        return 8;
+    }
+    return (sense.asc << 8) | sense.ascq;
+}
+
+void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense)
+{
+    int prec1, prec2;
+    if (sense.key != UNIT_ATTENTION) {
+        return;
+    }
+    trace_scsi_device_set_ua(sdev->id, sdev->lun, sense.key,
+                             sense.asc, sense.ascq);
+
+    /*
+     * Override a pre-existing unit attention condition, except for a more
+     * important reset condition.
+    */
+    prec1 = scsi_ua_precedence(sdev->unit_attention);
+    prec2 = scsi_ua_precedence(sense);
+    if (prec2 < prec1) {
+        sdev->unit_attention = sense;
+    }
+}
+
 void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense)
 {
     SCSIRequest *req;
@@ -1539,7 +1588,8 @@ void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense)
         req = QTAILQ_FIRST(&sdev->requests);
         scsi_req_cancel(req);
     }
-    sdev->unit_attention = sense;
+
+    scsi_device_set_ua(sdev, sense);
 }
 
 static char *scsibus_get_dev_path(DeviceState *dev)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b52e304..734fe13 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1876,7 +1876,7 @@ static void scsi_cd_change_media_cb(void *opaque, bool load)
      */
     s->media_changed = load;
     s->tray_open = !load;
-    s->qdev.unit_attention = SENSE_CODE(UNIT_ATTENTION_NO_MEDIUM);
+    scsi_device_set_ua(&s->qdev, SENSE_CODE(UNIT_ATTENTION_NO_MEDIUM));
     s->media_event = true;
     s->eject_request = false;
 }
@@ -1913,7 +1913,7 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
     if (s->media_changed) {
         s->media_changed = false;
-        s->qdev.unit_attention = SENSE_CODE(MEDIUM_CHANGED);
+        scsi_device_set_ua(&s->qdev, SENSE_CODE(MEDIUM_CHANGED));
     }
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index 088d519..f920208 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -239,6 +239,7 @@ void scsi_req_abort(SCSIRequest *req, int status);
 void scsi_req_cancel(SCSIRequest *req);
 void scsi_req_retry(SCSIRequest *req);
 void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense);
+void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense);
 int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed);
 SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int target, int lun);
 
diff --git a/trace-events b/trace-events
index 7aa28b1..6b12f83 100644
--- a/trace-events
+++ b/trace-events
@@ -412,6 +412,7 @@ scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, int xfer) "targ
 scsi_req_parsed_lba(int target, int lun, int tag, int cmd, uint64_t lba) "target %d lun %d tag %d command %d lba %"PRIu64
 scsi_req_parse_bad(int target, int lun, int tag, int cmd) "target %d lun %d tag %d command %d"
 scsi_req_build_sense(int target, int lun, int tag, int key, int asc, int ascq) "target %d lun %d tag %d key %#02x asc %#02x ascq %#02x"
+scsi_device_set_ua(int target, int lun, int key, int asc, int ascq) "target %d lun %d key %#02x asc %#02x ascq %#02x"
 scsi_report_luns(int target, int lun, int tag) "target %d lun %d tag %d"
 scsi_inquiry(int target, int lun, int tag, int cdb1, int cdb2) "target %d lun %d tag %d page %#02x/%#02x"
 scsi_test_unit_ready(int target, int lun, int tag) "target %d lun %d tag %d"
-- 
1.7.10.4

[Qemu-devel] [PATCH 25/32] scsi-disk: report resized disk via sense codes

[Paolo Bonzini]

Linux will not use these, but a very similar mechanism will be used to
report the condition via virtio-scsi events.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c  |    5 +++++
 hw/scsi-disk.c |   24 ++++++++++++++++++++++--
 hw/scsi.h      |    2 ++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index c38c0ec..04262de 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1161,6 +1161,11 @@ const struct SCSISense sense_code_LUN_FAILURE = {
     .key = ABORTED_COMMAND, .asc = 0x3e, .ascq = 0x01
 };
 
+/* Unit attention, Capacity data has changed */
+const struct SCSISense sense_code_CAPACITY_CHANGED = {
+    .key = UNIT_ATTENTION, .asc = 0x2a, .ascq = 0x09
+};
+
 /* Unit attention, Power on, reset or bus device reset occurred */
 const struct SCSISense sense_code_RESET = {
     .key = UNIT_ATTENTION, .asc = 0x29, .ascq = 0x00
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 734fe13..9ba405f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1860,6 +1860,18 @@ static void scsi_destroy(SCSIDevice *dev)
     blockdev_mark_auto_del(s->qdev.conf.bs);
 }
 
+static void scsi_disk_resize_cb(void *opaque)
+{
+    SCSIDiskState *s = opaque;
+
+    /* SPC lists this sense code as available only for
+     * direct-access devices.
+     */
+    if (s->qdev.type == TYPE_DISK) {
+        scsi_device_set_ua(&s->qdev, SENSE_CODE(CAPACITY_CHANGED));
+    }
+}
+
 static void scsi_cd_change_media_cb(void *opaque, bool load)
 {
     SCSIDiskState *s = opaque;
@@ -1901,11 +1913,17 @@ static bool scsi_cd_is_medium_locked(void *opaque)
     return ((SCSIDiskState *)opaque)->tray_locked;
 }
 
-static const BlockDevOps scsi_cd_block_ops = {
+static const BlockDevOps scsi_disk_removable_block_ops = {
     .change_media_cb = scsi_cd_change_media_cb,
     .eject_request_cb = scsi_cd_eject_request_cb,
     .is_tray_open = scsi_cd_is_tray_open,
     .is_medium_locked = scsi_cd_is_medium_locked,
+
+    .resize_cb = scsi_disk_resize_cb,
+};
+
+static const BlockDevOps scsi_disk_block_ops = {
+    .resize_cb = scsi_disk_resize_cb,
 };
 
 static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
@@ -1950,7 +1968,9 @@ static int scsi_initfn(SCSIDevice *dev)
     }
 
     if (s->features & (1 << SCSI_DISK_F_REMOVABLE)) {
-        bdrv_set_dev_ops(s->qdev.conf.bs, &scsi_cd_block_ops, s);
+        bdrv_set_dev_ops(s->qdev.conf.bs, &scsi_disk_removable_block_ops, s);
+    } else {
+        bdrv_set_dev_ops(s->qdev.conf.bs, &scsi_disk_block_ops, s);
     }
     bdrv_set_buffer_alignment(s->qdev.conf.bs, s->qdev.blocksize);
 
diff --git a/hw/scsi.h b/hw/scsi.h
index f920208..82a5fc8 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -200,6 +200,8 @@ extern const struct SCSISense sense_code_IO_ERROR;
 extern const struct SCSISense sense_code_I_T_NEXUS_LOSS;
 /* Command aborted, Logical Unit failure */
 extern const struct SCSISense sense_code_LUN_FAILURE;
+/* LUN not ready, Capacity data has changed */
+extern const struct SCSISense sense_code_CAPACITY_CHANGED;
 /* LUN not ready, Medium not present */
 extern const struct SCSISense sense_code_UNIT_ATTENTION_NO_MEDIUM;
 /* Unit attention, Power on, reset or bus device reset occurred */
-- 
1.7.10.4

[Qemu-devel] [PATCH 26/32] scsi: report parameter changes to HBA drivers

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c  |   10 ++++++++++
 hw/scsi-disk.c |    1 +
 hw/scsi.h      |    2 ++
 3 files changed, 13 insertions(+)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 04262de..e4ec19e 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1072,6 +1072,16 @@ int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
     return 0;
 }
 
+void scsi_device_report_change(SCSIDevice *dev, SCSISense sense)
+{
+    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
+
+    scsi_device_set_ua(dev, sense);
+    if (bus->info->change) {
+        bus->info->change(bus, dev, sense);
+    }
+}
+
 /*
  * Predefined sense codes
  */
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 9ba405f..84b63ff 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1869,6 +1869,7 @@ static void scsi_disk_resize_cb(void *opaque)
      */
     if (s->qdev.type == TYPE_DISK) {
         scsi_device_set_ua(&s->qdev, SENSE_CODE(CAPACITY_CHANGED));
+        scsi_device_report_change(&s->qdev, SENSE_CODE(CAPACITY_CHANGED));
     }
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index 82a5fc8..1aeee46 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -133,6 +133,7 @@ struct SCSIBusInfo {
     void (*cancel)(SCSIRequest *req);
     void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
     void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
+    void (*change)(SCSIBus *bus, SCSIDevice *dev, SCSISense sense);
     QEMUSGList *(*get_sg_list)(SCSIRequest *req);
 
     void (*save_request)(QEMUFile *f, SCSIRequest *req);
@@ -242,6 +243,7 @@ void scsi_req_cancel(SCSIRequest *req);
 void scsi_req_retry(SCSIRequest *req);
 void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense);
 void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense);
+void scsi_device_report_change(SCSIDevice *dev, SCSISense sense);
 int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed);
 SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int target, int lun);
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 27/32] virtio-scsi: Implement hotplug support for virtio-scsi

[Paolo Bonzini]

From: Cong Meng <mc@linux.vnet.ibm.com>

Implement the hotplug() and hot_unplug() interfaces in virtio-scsi, by signal
the virtio_scsi.ko in guest kernel via event virtual queue.

The counterpart patch of virtio_scsi.ko will be sent soon in another thread.

Signed-off-by: Sen Wang <senwang@linux.vnet.ibm.com>
Signed-off-by: Cong Meng <mc@linux.vnet.ibm.com>
[ Add memset, fix LUN field, placate checkpatch - Paolo ]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-scsi.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 70 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index 0a5ac40..4f02195 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -24,6 +24,10 @@
 #define VIRTIO_SCSI_MAX_TARGET  255
 #define VIRTIO_SCSI_MAX_LUN     16383
 
+/* Feature Bits */
+#define VIRTIO_SCSI_F_INOUT                    0
+#define VIRTIO_SCSI_F_HOTPLUG                  1
+
 /* Response codes */
 #define VIRTIO_SCSI_S_OK                       0
 #define VIRTIO_SCSI_S_OVERRUN                  1
@@ -60,6 +64,11 @@
 #define VIRTIO_SCSI_T_TRANSPORT_RESET          1
 #define VIRTIO_SCSI_T_ASYNC_NOTIFY             2
 
+/* Reasons for transport reset event */
+#define VIRTIO_SCSI_EVT_RESET_HARD             0
+#define VIRTIO_SCSI_EVT_RESET_RESCAN           1
+#define VIRTIO_SCSI_EVT_RESET_REMOVED          2
+
 /* SCSI command request, followed by data-out */
 typedef struct {
     uint8_t lun[8];              /* Logical Unit Number */
@@ -206,11 +215,13 @@ static void qemu_sgl_init_external(QEMUSGList *qsgl, struct iovec *sg,
 static void virtio_scsi_parse_req(VirtIOSCSI *s, VirtQueue *vq,
                                   VirtIOSCSIReq *req)
 {
-    assert(req->elem.out_num && req->elem.in_num);
+    assert(req->elem.in_num);
     req->vq = vq;
     req->dev = s;
     req->sreq = NULL;
-    req->req.buf = req->elem.out_sg[0].iov_base;
+    if (req->elem.out_num) {
+        req->req.buf = req->elem.out_sg[0].iov_base;
+    }
     req->resp.buf = req->elem.in_sg[0].iov_base;
 
     if (req->elem.out_num > 1) {
@@ -545,6 +556,7 @@ static void virtio_scsi_set_config(VirtIODevice *vdev,
 static uint32_t virtio_scsi_get_features(VirtIODevice *vdev,
                                          uint32_t requested_features)
 {
+    requested_features |= (1UL << VIRTIO_SCSI_F_HOTPLUG);
     return requested_features;
 }
 
@@ -577,6 +589,60 @@ static int virtio_scsi_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
+                                   uint32_t event, uint32_t reason)
+{
+    VirtIOSCSIReq *req = virtio_scsi_pop_req(s, s->event_vq);
+    VirtIOSCSIEvent *evt;
+
+    if (req) {
+        int in_size;
+        if (req->elem.out_num || req->elem.in_num != 1) {
+            virtio_scsi_bad_req();
+        }
+
+        in_size = req->elem.in_sg[0].iov_len;
+        if (in_size < sizeof(VirtIOSCSIEvent)) {
+            virtio_scsi_bad_req();
+        }
+
+        evt = req->resp.event;
+        memset(evt, 0, sizeof(VirtIOSCSIEvent));
+        evt->event = event;
+        evt->reason = reason;
+        evt->lun[0] = 1;
+        evt->lun[1] = dev->id;
+
+        /* Linux wants us to keep the same encoding we use for REPORT LUNS.  */
+        if (dev->lun >= 256) {
+            evt->lun[2] = (dev->lun >> 8) | 0x40;
+        }
+        evt->lun[3] = dev->lun & 0xFF;
+        virtio_scsi_complete_req(req);
+    }
+}
+
+static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
+{
+    VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
+
+    if (((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) &&
+        (s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+        virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
+                               VIRTIO_SCSI_EVT_RESET_RESCAN);
+    }
+}
+
+static void virtio_scsi_hot_unplug(SCSIBus *bus, SCSIDevice *dev)
+{
+    VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
+
+    if ((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
+        virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
+                               VIRTIO_SCSI_EVT_RESET_REMOVED);
+    }
+}
+
 static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .tcq = true,
     .max_channel = VIRTIO_SCSI_MAX_CHANNEL,
@@ -585,6 +651,8 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
 
     .complete = virtio_scsi_command_complete,
     .cancel = virtio_scsi_request_cancelled,
+    .hotplug = virtio_scsi_hotplug,
+    .hot_unplug = virtio_scsi_hot_unplug,
     .get_sg_list = virtio_scsi_get_sg_list,
     .save_request = virtio_scsi_save_request,
     .load_request = virtio_scsi_load_request,
-- 
1.7.10.4

[Qemu-devel] [PATCH 28/32] virtio-scsi: Report missed events

[Paolo Bonzini]

When an event is reported but no buffers are present in the event vq,
we can set a flag and report a dummy event as soon as one is added.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-scsi.c |   54 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index 4f02195..d5c5984 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -141,6 +141,7 @@ typedef struct {
     uint32_t sense_size;
     uint32_t cdb_size;
     int resetting;
+    bool events_dropped;
     VirtQueue *ctrl_vq;
     VirtQueue *event_vq;
     VirtQueue *cmd_vqs[0];
@@ -416,10 +417,6 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
-static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
-{
-}
-
 static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
                                          size_t resid)
 {
@@ -594,22 +591,34 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
 {
     VirtIOSCSIReq *req = virtio_scsi_pop_req(s, s->event_vq);
     VirtIOSCSIEvent *evt;
+    int in_size;
 
-    if (req) {
-        int in_size;
-        if (req->elem.out_num || req->elem.in_num != 1) {
-            virtio_scsi_bad_req();
-        }
+    if (!req) {
+        s->events_dropped = true;
+        return;
+    }
 
-        in_size = req->elem.in_sg[0].iov_len;
-        if (in_size < sizeof(VirtIOSCSIEvent)) {
-            virtio_scsi_bad_req();
-        }
+    if (req->elem.out_num || req->elem.in_num != 1) {
+        virtio_scsi_bad_req();
+    }
 
-        evt = req->resp.event;
-        memset(evt, 0, sizeof(VirtIOSCSIEvent));
-        evt->event = event;
-        evt->reason = reason;
+    if (s->events_dropped) {
+        event |= VIRTIO_SCSI_T_EVENTS_MISSED;
+        s->events_dropped = false;
+    }
+
+    in_size = req->elem.in_sg[0].iov_len;
+    if (in_size < sizeof(VirtIOSCSIEvent)) {
+        virtio_scsi_bad_req();
+    }
+
+    evt = req->resp.event;
+    memset(evt, 0, sizeof(VirtIOSCSIEvent));
+    evt->event = event;
+    evt->reason = reason;
+    if (!dev) {
+        assert(event == VIRTIO_SCSI_T_NO_EVENT);
+    } else {
         evt->lun[0] = 1;
         evt->lun[1] = dev->id;
 
@@ -618,7 +627,16 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
             evt->lun[2] = (dev->lun >> 8) | 0x40;
         }
         evt->lun[3] = dev->lun & 0xFF;
-        virtio_scsi_complete_req(req);
+    }
+    virtio_scsi_complete_req(req);
+}
+
+static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+
+    if (s->events_dropped) {
+        virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
     }
 }
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 29/32] virtio-scsi: do not report dropped events after reset

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-scsi.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index d5c5984..bf18861 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -563,6 +563,7 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
 
     s->sense_size = VIRTIO_SCSI_SENSE_SIZE;
     s->cdb_size = VIRTIO_SCSI_CDB_SIZE;
+    s->events_dropped = false;
 }
 
 /* The device does not have anything to save beyond the virtio data.
-- 
1.7.10.4

[Qemu-devel] [PATCH 30/32] virtio-scsi: report parameter change events

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-scsi.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index bf18861..c4a5b22 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -27,6 +27,7 @@
 /* Feature Bits */
 #define VIRTIO_SCSI_F_INOUT                    0
 #define VIRTIO_SCSI_F_HOTPLUG                  1
+#define VIRTIO_SCSI_F_CHANGE                   2
 
 /* Response codes */
 #define VIRTIO_SCSI_S_OK                       0
@@ -63,6 +64,7 @@
 #define VIRTIO_SCSI_T_NO_EVENT                 0
 #define VIRTIO_SCSI_T_TRANSPORT_RESET          1
 #define VIRTIO_SCSI_T_ASYNC_NOTIFY             2
+#define VIRTIO_SCSI_T_PARAM_CHANGE             3
 
 /* Reasons for transport reset event */
 #define VIRTIO_SCSI_EVT_RESET_HARD             0
@@ -554,6 +556,7 @@ static uint32_t virtio_scsi_get_features(VirtIODevice *vdev,
                                          uint32_t requested_features)
 {
     requested_features |= (1UL << VIRTIO_SCSI_F_HOTPLUG);
+    requested_features |= (1UL << VIRTIO_SCSI_F_CHANGE);
     return requested_features;
 }
 
@@ -641,6 +644,18 @@ static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
+{
+    VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
+
+    if (((s->vdev.guest_features >> VIRTIO_SCSI_F_CHANGE) & 1) &&
+        (s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+        dev->type != TYPE_ROM) {
+        virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE,
+                               sense.asc | (sense.ascq << 8));
+    }
+}
+
 static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
 {
     VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
@@ -670,6 +685,7 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
 
     .complete = virtio_scsi_command_complete,
     .cancel = virtio_scsi_request_cancelled,
+    .change = virtio_scsi_change,
     .hotplug = virtio_scsi_hotplug,
     .hot_unplug = virtio_scsi_hot_unplug,
     .get_sg_list = virtio_scsi_get_sg_list,
-- 
1.7.10.4

[Qemu-devel] [PATCH 31/32] virtio-scsi: add ioeventfd support

[Paolo Bonzini]

Probably due to bad merge months ago, virtio-scsi-pci did not have
ioeventfd support.  Fix this and enable it by default, as is the
case for other virtio-pci devices.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-pci.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 4e03f0b..82859a5 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -1040,6 +1040,7 @@ static int virtio_scsi_exit_pci(PCIDevice *pci_dev)
 }
 
 static Property virtio_scsi_properties[] = {
+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
     DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOPCIProxy, host_features, scsi),
     DEFINE_PROP_END_OF_LIST(),
-- 
1.7.10.4

[Qemu-devel] [PATCH 32/32] virtio-scsi: enable MSI-X support

[Paolo Bonzini]

While virtio-scsi does support multiqueue, the default number of
interrupt vectors is not enough to actually enable usage of
multiple queues in the driver; this is because with only 2
vectors the driver will not be able to use a separate
interrupt for each request queue.  Derive the desired number
of vectors from the number of request queues.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-pci.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 82859a5..1109467 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -1023,7 +1023,9 @@ static int virtio_scsi_init_pci(PCIDevice *pci_dev)
         return -EINVAL;
     }
 
-    vdev->nvectors = proxy->nvectors;
+    vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED
+                                        ? proxy->scsi.num_queues + 3
+                                        : proxy->nvectors;
     virtio_init_pci(proxy, vdev);
 
     /* make the actual value visible */
@@ -1041,7 +1043,7 @@ static int virtio_scsi_exit_pci(PCIDevice *pci_dev)
 
 static Property virtio_scsi_properties[] = {
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
-    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED),
     DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOPCIProxy, host_features, scsi),
     DEFINE_PROP_END_OF_LIST(),
 };
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 01/32] megasas: Replace trace_megasas_dcmd_dump_frame()

[Stefan Hajnoczi]

On Fri, Jul 27, 2012 at 4:02 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Hannes Reinecke <hare@suse.de>
>
> trace_megasas_dcmd_dump_frame() takes 9 arguments, which is
> rather much. Plus the trace infrastructure doesn't support
> it.  As we can get the information via other means it's pointless
> to have it in the driver, so rather use some proper trace
> point here and remove the old one.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Cc: Stefan Weil <sw@weilnetz.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/megasas.c |   39 ++++++++++-----------------------------
>  trace-events |    2 +-
>  2 files changed, 11 insertions(+), 30 deletions(-)

The simple trace backend now supports arbitrary argument counts :).

I don't think you need to change this patch but I'm just spreading the
word since this limitation has now been lifted.

Stefan

Re: [Qemu-devel] [PATCH 03/32] lsi: use qdev_reset_all

[Jan Kiszka]

On 2012-07-27 17:02, Paolo Bonzini wrote:
> By first resetting the devices, lsi_soft_reset will find the queue
> already cleared so there is no need to do that forcibly (which may also
> leak SCSIRequests, and/or worse due to dangling references to the
> lsi_request in the hba_private field).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/lsi53c895a.c |   15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index 2fe141d..f04cc68 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -282,8 +282,6 @@ static inline int lsi_irq_on_rsl(LSIState *s)
>  
>  static void lsi_soft_reset(LSIState *s)
>  {
> -    lsi_request *p;
> -
>      DPRINTF("Reset\n");
>      s->carry = 0;
>  
> @@ -350,15 +348,8 @@ static void lsi_soft_reset(LSIState *s)
>      s->sbc = 0;
>      s->csbc = 0;
>      s->sbr = 0;
> -    while (!QTAILQ_EMPTY(&s->queue)) {
> -        p = QTAILQ_FIRST(&s->queue);
> -        QTAILQ_REMOVE(&s->queue, p, next);
> -        g_free(p);
> -    }
> -    if (s->current) {
> -        g_free(s->current);
> -        s->current = NULL;
> -    }
> +    assert(QTAILQ_EMPTY(&s->queue));
> +    assert(!s->current);
>  }
>  
>  static int lsi_dma_40bit(LSIState *s)
> @@ -1738,7 +1729,7 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
>              lsi_execute_script(s);
>          }
>          if (val & LSI_ISTAT0_SRST) {
> -            lsi_soft_reset(s);
> +            qdev_reset_all(&s->dev.qdev);

What should ensure that all device resets are performed before the host
reset? I'm asking as we just ran into the assert(!s->current) over
1.3.1. Or is there some patch from a later version missing (not seeing
anything suspicious for lsi at least)?

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 03/32] lsi: use qdev_reset_all

[Paolo Bonzini]

Il 23/04/2013 10:47, Jan Kiszka ha scritto:
>> >          if (val & LSI_ISTAT0_SRST) {
>> > -            lsi_soft_reset(s);
>> > +            qdev_reset_all(&s->dev.qdev);
> What should ensure that all device resets are performed before the host
> reset? I'm asking as we just ran into the assert(!s->current) over
> 1.3.1. Or is there some patch from a later version missing (not seeing
> anything suspicious for lsi at least)?

qdev should, but it looks like it uses pre-order instead of post-order.

Paolo

Re: [Qemu-devel] [PATCH 03/32] lsi: use qdev_reset_all

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

Il 23/04/2013 18:13, Paolo Bonzini ha scritto:
> Il 23/04/2013 10:47, Jan Kiszka ha scritto:
>>>>          if (val & LSI_ISTAT0_SRST) {
>>>> -            lsi_soft_reset(s);
>>>> +            qdev_reset_all(&s->dev.qdev);
>> What should ensure that all device resets are performed before the host
>> reset? I'm asking as we just ran into the assert(!s->current) over
>> 1.3.1. Or is there some patch from a later version missing (not seeing
>> anything suspicious for lsi at least)?
> 
> qdev should, but it looks like it uses pre-order instead of post-order.

Something like this series (based on 1.3.0), totally untested.

Paolo

[-- Attachment #2: reset-for-jan.patch --]
[-- Type: text/x-patch, Size: 13873 bytes --]

>From ec9930ec271a29041275284127d58104d58d4264 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 23 Apr 2013 18:40:25 +0200
Subject: [PATCH 0/4] *** SUBJECT HERE ***

*** BLURB HERE ***

Paolo Bonzini (4):
  qdev: add qbus_reset_all
  pci: do not export pci_bus_reset
  qdev: allow both pre- and post-order vists in qdev walking functions
  qdev: switch reset to post-order

 hw/pci.c        | 39 +++++++++++++++++++--------------------
 hw/pci.h        |  1 -
 hw/pci_bridge.c |  2 +-
 hw/qdev-core.h  | 27 ++++++++++++++++++++++-----
 hw/qdev.c       | 52 +++++++++++++++++++++++++++++++++++++++-------------
 5 files changed, 81 insertions(+), 40 deletions(-)

-- 
1.8.2

>From 483bc3eddd7804c1b101982fa6bb8150efcdf448 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Thu, 10 Jan 2013 15:49:07 +0100
Subject: [PATCH 1/4] qdev: add qbus_reset_all

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev-core.h | 12 ++++++++++++
 hw/qdev.c      |  7 ++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index fff7f0f..7dcf836 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -191,6 +191,18 @@ int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
 int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
                        qbus_walkerfn *busfn, void *opaque);
 void qdev_reset_all(DeviceState *dev);
+
+/**
+ * @qbus_reset_all:
+ * @bus: Bus to be reset.
+ *
+ * Reset @bus and perform a bus-level ("hard") reset of all devices connected
+ * to it, including recursive processing of all buses below @bus itself.  A
+ * hard reset means that qbus_reset_all will reset all state of the device.
+ * For PCI devices, for example, this will include the base address registers
+ * or configuration space.
+ */
+void qbus_reset_all(BusState *bus);
 void qbus_reset_all_fn(void *opaque);
 
 void qbus_free(BusState *bus);
diff --git a/hw/qdev.c b/hw/qdev.c
index 788b4da..d017782 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -227,10 +227,15 @@ void qdev_reset_all(DeviceState *dev)
     qdev_walk_children(dev, qdev_reset_one, qbus_reset_one, NULL);
 }
 
+void qbus_reset_all(BusState *bus)
+{
+    qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL);
+}
+
 void qbus_reset_all_fn(void *opaque)
 {
     BusState *bus = opaque;
-    qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL);
+    qbus_reset_all(bus);
 }
 
 /* can be used as ->unplug() callback for the simple cases */
-- 
1.8.2


>From 03a8c5e5fe2ff16b6048b49112b69802779de47b Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Mon, 17 Dec 2012 11:02:25 +0100
Subject: [PATCH 2/4] pci: do not export pci_bus_reset

qbus_reset_all can be used instead.  There is no semantic change
because pcibus_reset returns 1 and takes care of the device
tree traversal.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pci.c        | 8 ++------
 hw/pci.h        | 1 -
 hw/pci_bridge.c | 2 +-
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 97a0cd7..916f83c 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -209,8 +209,9 @@ void pci_device_reset(PCIDevice *dev)
  * Trigger pci bus reset under a given bus.
  * To be called on RST# assert.
  */
-void pci_bus_reset(PCIBus *bus)
+static int pcibus_reset(BusState *qbus)
 {
+    PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
     int i;
 
     for (i = 0; i < bus->nirq; i++) {
@@ -221,11 +222,6 @@ void pci_bus_reset(PCIBus *bus)
             pci_device_reset(bus->devices[i]);
         }
     }
-}
-
-static int pcibus_reset(BusState *qbus)
-{
-    pci_bus_reset(DO_UPCAST(PCIBus, qbus, qbus));
 
     /* topology traverse is done by pci_bus_reset().
        Tell qbus/qdev walker not to traverse the tree */
diff --git a/hw/pci.h b/hw/pci.h
index 4da0c2a..88fd4a3 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -334,7 +334,6 @@ void pci_bus_fire_intx_routing_notifier(PCIBus *bus);
 void pci_device_set_intx_routing_notifier(PCIDevice *dev,
                                           PCIINTxRoutingNotifier notifier);
 void pci_device_reset(PCIDevice *dev);
-void pci_bus_reset(PCIBus *bus);
 
 PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
                         const char *default_devaddr);
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 4680501..30ed34e 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -234,7 +234,7 @@ void pci_bridge_write_config(PCIDevice *d,
     newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
     if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
         /* Trigger hot reset on 0->1 transition. */
-        pci_bus_reset(&s->sec_bus);
+        qbus_reset_all(&s->sec_bus.qbus);
     }
 }
 
-- 
1.8.2


>From 7d76cfc73da60bd2ea225a7df8b5a1c6e4cfa799 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Mon, 17 Dec 2012 10:37:02 +0100
Subject: [PATCH 3/4] qdev: allow both pre- and post-order vists in qdev
 walking functions

Resetting should be done in post-order, not pre-order.  However,
qdev_walk_children and qbus_walk_children do not allow this.  Fix
it by adding two extra arguments to the functions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev-core.h | 13 +++++++++----
 hw/qdev.c      | 45 +++++++++++++++++++++++++++++++++------------
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index 7dcf836..3d81969 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -186,10 +186,15 @@ BusState *qbus_create(const char *typename, DeviceState *parent, const char *nam
 /* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
  *         < 0 if either devfn or busfn terminate walk somewhere in cursion,
  *           0 otherwise. */
-int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
-                       qbus_walkerfn *busfn, void *opaque);
-int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
-                       qbus_walkerfn *busfn, void *opaque);
+int qbus_walk_children(BusState *bus,
+                       qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
+                       qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
+                       void *opaque);
+int qdev_walk_children(DeviceState *dev,
+                       qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
+                       qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
+                       void *opaque);
+
 void qdev_reset_all(DeviceState *dev);
 
 /**
diff --git a/hw/qdev.c b/hw/qdev.c
index d017782..3090135 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -224,12 +224,12 @@ static int qbus_reset_one(BusState *bus, void *opaque)
 
 void qdev_reset_all(DeviceState *dev)
 {
-    qdev_walk_children(dev, qdev_reset_one, qbus_reset_one, NULL);
+    qdev_walk_children(dev, qdev_reset_one, qbus_reset_one, NULL, NULL, NULL);
 }
 
 void qbus_reset_all(BusState *bus)
 {
-    qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL);
+    qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL, NULL, NULL);
 }
 
 void qbus_reset_all_fn(void *opaque)
@@ -339,49 +339,70 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
     return NULL;
 }
 
-int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
-                       qbus_walkerfn *busfn, void *opaque)
+int qbus_walk_children(BusState *bus,
+                       qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
+                       qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
+                       void *opaque)
 {
     BusChild *kid;
     int err;
 
-    if (busfn) {
-        err = busfn(bus, opaque);
+    if (pre_busfn) {
+        err = pre_busfn(bus, opaque);
         if (err) {
             return err;
         }
     }
 
     QTAILQ_FOREACH(kid, &bus->children, sibling) {
-        err = qdev_walk_children(kid->child, devfn, busfn, opaque);
+        err = qdev_walk_children(kid->child,
+                                 pre_devfn, pre_busfn,
+                                 post_devfn, post_busfn, opaque);
         if (err < 0) {
             return err;
         }
     }
 
+    if (post_busfn) {
+        err = post_busfn(bus, opaque);
+        if (err) {
+            return err;
+        }
+    }
+
     return 0;
 }
 
-int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
-                       qbus_walkerfn *busfn, void *opaque)
+int qdev_walk_children(DeviceState *dev,
+                       qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
+                       qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
+                       void *opaque)
 {
     BusState *bus;
     int err;
 
-    if (devfn) {
-        err = devfn(dev, opaque);
+    if (pre_devfn) {
+        err = pre_devfn(dev, opaque);
         if (err) {
             return err;
         }
     }
 
     QLIST_FOREACH(bus, &dev->child_bus, sibling) {
-        err = qbus_walk_children(bus, devfn, busfn, opaque);
+        err = qbus_walk_children(bus, pre_devfn, pre_busfn,
+                                 post_devfn, post_busfn, opaque);
         if (err < 0) {
             return err;
         }
     }
 
+    if (post_devfn) {
+        err = post_devfn(dev, opaque);
+        if (err) {
+            return err;
+        }
+    }
+
     return 0;
 }
 
-- 
1.8.2


>From ec9930ec271a29041275284127d58104d58d4264 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Mon, 17 Dec 2012 10:50:16 +0100
Subject: [PATCH 4/4] qdev: switch reset to post-order

Post-order is the only sensible direction for the reset signals.
For example, suppose pre-order is used and the parent has some data
structures that cache children state (for example a list of active
requests).  When the reset method is invoked on the parent, these caches
could be in any state.

If post-order is used, on the other hand, these will be in a known state
when the reset method is invoked on the parent.

This change means that it is no longer possible to block the visit of
the devices, so the callback is changed to return void.  This is not
a problem, because PCI was returning 1 exactly in order to achieve the
same ordering that this patch implements.

PCI can then rely on the qdev core having sent a "reset signal" (whatever
that means) to the device, and only do the PCI-specific initialization
with pci_do_device_reset.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pci.c       | 33 ++++++++++++++++++---------------
 hw/qdev-core.h |  2 +-
 hw/qdev.c      |  6 +++---
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 916f83c..b195d67 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -45,7 +45,7 @@
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
-static int pcibus_reset(BusState *qbus);
+static void pcibus_reset(BusState *qbus);
 
 static Property pci_props[] = {
     DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
@@ -164,16 +164,10 @@ void pci_device_deassert_intx(PCIDevice *dev)
     }
 }
 
-/*
- * This function is called on #RST and FLR.
- * FLR if PCI_EXP_DEVCTL_BCR_FLR is set
- */
-void pci_device_reset(PCIDevice *dev)
+static void pci_do_device_reset(PCIDevice *dev)
 {
     int r;
 
-    qdev_reset_all(&dev->qdev);
-
     dev->irq_state = 0;
     pci_update_irq_status(dev);
     pci_device_deassert_intx(dev);
@@ -206,10 +200,23 @@ void pci_device_reset(PCIDevice *dev)
 }
 
 /*
+ * This function is called on #RST and FLR.
+ * FLR if PCI_EXP_DEVCTL_BCR_FLR is set
+ */
+void pci_device_reset(PCIDevice *dev)
+{
+    int r;
+
+    qdev_reset_all(&dev->qdev);
+    pci_do_device_reset(dev);
+}
+
+/*
  * Trigger pci bus reset under a given bus.
- * To be called on RST# assert.
+ * Called via qbus_reset_all on RST# assert, after the devices
+ * have been reset qdev_reset_all-ed already.
  */
-static int pcibus_reset(BusState *qbus)
+static void pcibus_reset(BusState *qbus)
 {
     PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
     int i;
@@ -219,13 +226,9 @@ static int pcibus_reset(BusState *qbus)
     }
     for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
         if (bus->devices[i]) {
-            pci_device_reset(bus->devices[i]);
+            pci_do_device_reset(bus->devices[i]);
         }
     }
-
-    /* topology traverse is done by pci_bus_reset().
-       Tell qbus/qdev walker not to traverse the tree */
-    return 1;
 }
 
 static void pci_host_bus_register(int domain, PCIBus *bus)
diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index 3d81969..e6197ee 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -95,7 +95,7 @@ struct BusClass {
      * bindings can be found at http://playground.sun.com/1275/bindings/.
      */
     char *(*get_fw_dev_path)(DeviceState *dev);
-    int (*reset)(BusState *bus);
+    void (*reset)(BusState *bus);
 };
 
 typedef struct BusChild {
diff --git a/hw/qdev.c b/hw/qdev.c
index 3090135..a1c1ffa 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -217,19 +217,19 @@ static int qbus_reset_one(BusState *bus, void *opaque)
 {
     BusClass *bc = BUS_GET_CLASS(bus);
     if (bc->reset) {
-        return bc->reset(bus);
+        bc->reset(bus);
     }
     return 0;
 }
 
 void qdev_reset_all(DeviceState *dev)
 {
-    qdev_walk_children(dev, qdev_reset_one, qbus_reset_one, NULL, NULL, NULL);
+    qdev_walk_children(dev, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL);
 }
 
 void qbus_reset_all(BusState *bus)
 {
-    qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL, NULL, NULL);
+    qbus_walk_children(bus, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL);
 }
 
 void qbus_reset_all_fn(void *opaque)
-- 
1.8.2

Re: [Qemu-devel] [PATCH 03/32] lsi: use qdev_reset_all

[Jan Kiszka]

On 2013-04-23 18:43, Paolo Bonzini wrote:
> Il 23/04/2013 18:13, Paolo Bonzini ha scritto:
>> Il 23/04/2013 10:47, Jan Kiszka ha scritto:
>>>>>          if (val & LSI_ISTAT0_SRST) {
>>>>> -            lsi_soft_reset(s);
>>>>> +            qdev_reset_all(&s->dev.qdev);
>>> What should ensure that all device resets are performed before the host
>>> reset? I'm asking as we just ran into the assert(!s->current) over
>>> 1.3.1. Or is there some patch from a later version missing (not seeing
>>> anything suspicious for lsi at least)?
>>
>> qdev should, but it looks like it uses pre-order instead of post-order.
> 
> Something like this series (based on 1.3.0), totally untested.

Forwarded to the folks who managed to trigger this. Will let you know.

Many thanks!
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux