[Qemu-devel] [RFC PATCH v2 00/10] virtio-scsi: Dataplane on single iothread

[Qemu-devel] [RFC PATCH v2 00/10] virtio-scsi: Dataplane on single iothread

[Fam Zheng]

Hi all,

This series adds "iothread" property to virtio-scsi in a way just similar to
virtio-blk, and turns all scsi devices to run on top of it.

Example:

    -object iothread,id=iothread-1  \
    -device virtio-scsi-pci,id=virtio-scsi-bus-0,iothread=iothread-1  \
    -drive file=guest.img,id=scsi-disk-1,if=none,cache=none,aio=native  \
    -device scsi-disk,lun=1,drive=scsi-disk-1,id=scsi-disk-1

It uses irqfd, ioeventfd and vring in a way just like virtio-blk does now.

Please review the general approach and see if major points are missed in terms
of thread safety and completeness of the moved things from original context to
iothread context.

Note that the used vring is not function complete compared to virtqueue
implementation, because of its lacking of MMIO handling. So this is just an
RFC.

Migration hasn't been looked into yet, either.  Assigning multiple iothreads is
supposed to be worked on top of this as well, but it's not thoroughly planned
yet.

Thanks,
Fam


Fam Zheng (10):
  virtio: Compile vring code unconditionally
  virtio-scsi: Split virtio_scsi_handle_cmd_req from
    virtio_scsi_handle_cmd
  virtio-scsi: Split virtio_scsi_handle_ctrl_req from
    virtio_scsi_handle_ctrl
  virtio-scsi: Add VirtIOSCSIVring in VirtIOSCSIReq
  virtio-scsi: Make virtio_scsi_init_req public
  virtio-scsi: Make virtio_scsi_free_req public
  virtio-scsi: Make virtio_scsi_push_event public
  virtio-scsi: Add 'iothread' property to virtio-scsi-pci
  virtio-scsi-dataplane: Code to run virtio-scsi on iothread
  virtio-scsi: Hook up with dataplane

 hw/scsi/Makefile.objs           |   2 +-
 hw/scsi/virtio-scsi-dataplane.c | 219 +++++++++++++++++++++++++++++++++
 hw/scsi/virtio-scsi.c           | 260 +++++++++++++++++++++++++---------------
 hw/virtio/Makefile.objs         |   2 +-
 hw/virtio/virtio-pci.c          |   2 +
 include/hw/virtio/virtio-scsi.h |  65 ++++++++++
 6 files changed, 450 insertions(+), 100 deletions(-)
 create mode 100644 hw/scsi/virtio-scsi-dataplane.c

-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 01/10] virtio: Compile vring code unconditionally

[Fam Zheng]

Because virtio-scsi dataplane will also use it.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/virtio/Makefile.objs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index ec9e855..a92582f 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -2,7 +2,7 @@ common-obj-y += virtio-rng.o
 common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
 common-obj-y += virtio-bus.o
 common-obj-y += virtio-mmio.o
-common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += dataplane/
+common-obj-y += dataplane/
 
 obj-y += virtio.o virtio-balloon.o 
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 02/10] virtio-scsi: Split virtio_scsi_handle_cmd_req from virtio_scsi_handle_cmd

[Fam Zheng]

This is the "common part" to handle one cmd request. Refactor out for
later usage of dataplane iothread code.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/virtio-scsi.c           | 107 ++++++++++++++++------------------------
 include/hw/virtio/virtio-scsi.h |  26 ++++++++++
 2 files changed, 69 insertions(+), 64 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 0eb069a..bd5d789 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -21,31 +21,6 @@
 #include <hw/virtio/virtio-bus.h>
 #include "hw/virtio/virtio-access.h"
 
-typedef struct VirtIOSCSIReq {
-    VirtIOSCSI *dev;
-    VirtQueue *vq;
-    VirtQueueElement elem;
-    QEMUSGList qsgl;
-    SCSIRequest *sreq;
-    size_t resp_size;
-    enum SCSIXferMode mode;
-    QEMUIOVector resp_iov;
-    union {
-        VirtIOSCSICmdResp     cmd;
-        VirtIOSCSICtrlTMFResp tmf;
-        VirtIOSCSICtrlANResp  an;
-        VirtIOSCSIEvent       event;
-    } resp;
-    union {
-        struct {
-            VirtIOSCSICmdReq  cmd;
-            uint8_t           cdb[];
-        } QEMU_PACKED;
-        VirtIOSCSICtrlTMFReq  tmf;
-        VirtIOSCSICtrlANReq   an;
-    } req;
-} VirtIOSCSIReq;
-
 QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) !=
                   offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq));
 
@@ -434,52 +409,56 @@ static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req)
     virtio_scsi_complete_cmd_req(req);
 }
 
+void virtio_scsi_handle_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
+{
+    VirtIOSCSICommon *vs = &s->parent_obj;
+    int n;
+    SCSIDevice *d;
+    int rc;
+
+    rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
+                               sizeof(VirtIOSCSICmdResp) + vs->sense_size);
+    if (rc < 0) {
+        if (rc == -ENOTSUP) {
+            virtio_scsi_fail_cmd_req(req);
+        } else {
+            virtio_scsi_bad_req();
+        }
+        return;
+    }
+
+    d = virtio_scsi_device_find(s, req->req.cmd.lun);
+    if (!d) {
+        req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
+        virtio_scsi_complete_cmd_req(req);
+        return;
+    }
+    req->sreq = scsi_req_new(d, req->req.cmd.tag,
+                             virtio_scsi_get_lun(req->req.cmd.lun),
+                             req->req.cdb, req);
+
+    if (req->sreq->cmd.mode != SCSI_XFER_NONE
+        && (req->sreq->cmd.mode != req->mode ||
+            req->sreq->cmd.xfer > req->qsgl.size)) {
+        req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN;
+        virtio_scsi_complete_cmd_req(req);
+        return;
+    }
+
+    n = scsi_req_enqueue(req->sreq);
+    if (n) {
+        scsi_req_continue(req->sreq);
+    }
+}
+
 static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
 {
     /* use non-QOM casts in the data path */
     VirtIOSCSI *s = (VirtIOSCSI *)vdev;
-    VirtIOSCSICommon *vs = &s->parent_obj;
-
     VirtIOSCSIReq *req;
-    int n;
 
     while ((req = virtio_scsi_pop_req(s, vq))) {
-        SCSIDevice *d;
-        int rc;
-
-        rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
-                                   sizeof(VirtIOSCSICmdResp) + vs->sense_size);
-        if (rc < 0) {
-            if (rc == -ENOTSUP) {
-                virtio_scsi_fail_cmd_req(req);
-            } else {
-                virtio_scsi_bad_req();
-            }
-            continue;
-        }
-
-        d = virtio_scsi_device_find(s, req->req.cmd.lun);
-        if (!d) {
-            req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
-            virtio_scsi_complete_cmd_req(req);
-            continue;
-        }
-        req->sreq = scsi_req_new(d, req->req.cmd.tag,
-                                 virtio_scsi_get_lun(req->req.cmd.lun),
-                                 req->req.cdb, req);
-
-        if (req->sreq->cmd.mode != SCSI_XFER_NONE
-            && (req->sreq->cmd.mode != req->mode ||
-                req->sreq->cmd.xfer > req->qsgl.size)) {
-            req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN;
-            virtio_scsi_complete_cmd_req(req);
-            continue;
-        }
-
-        n = scsi_req_enqueue(req->sreq);
-        if (n) {
-            scsi_req_continue(req->sreq);
-        }
+        virtio_scsi_handle_cmd_req(s, req);
     }
 }
 
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 188a2d9..7349936 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -172,6 +172,31 @@ typedef struct {
     bool events_dropped;
 } VirtIOSCSI;
 
+typedef struct VirtIOSCSIReq {
+    VirtIOSCSI *dev;
+    VirtQueue *vq;
+    VirtQueueElement elem;
+    QEMUSGList qsgl;
+    SCSIRequest *sreq;
+    size_t resp_size;
+    enum SCSIXferMode mode;
+    QEMUIOVector resp_iov;
+    union {
+        VirtIOSCSICmdResp     cmd;
+        VirtIOSCSICtrlTMFResp tmf;
+        VirtIOSCSICtrlANResp  an;
+        VirtIOSCSIEvent       event;
+    } resp;
+    union {
+        struct {
+            VirtIOSCSICmdReq  cmd;
+            uint8_t           cdb[];
+        } QEMU_PACKED;
+        VirtIOSCSICtrlTMFReq  tmf;
+        VirtIOSCSICtrlANReq   an;
+    } req;
+} VirtIOSCSIReq;
+
 #define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _conf_field)                     \
     DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1),       \
     DEFINE_PROP_UINT32("max_sectors", _state, _conf_field.max_sectors, 0xFFFF),\
@@ -192,5 +217,6 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp,
                                 HandleOutput cmd);
 
 void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp);
+void virtio_scsi_handle_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req);
 
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 03/10] virtio-scsi: Split virtio_scsi_handle_ctrl_req from virtio_scsi_handle_ctrl

[Fam Zheng]

To share with dataplane code.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/virtio-scsi.c           | 60 ++++++++++++++++++++++-------------------
 include/hw/virtio/virtio-scsi.h |  1 +
 2 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index bd5d789..cb2745d 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -308,40 +308,46 @@ fail:
     req->resp.tmf.response = VIRTIO_SCSI_S_BAD_TARGET;
 }
 
-static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
 {
-    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
-    VirtIOSCSIReq *req;
+    VirtIODevice *vdev = (VirtIODevice *)s;
+    int type;
 
-    while ((req = virtio_scsi_pop_req(s, vq))) {
-        int type;
+    if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0,
+                &type, sizeof(type)) < sizeof(type)) {
+        virtio_scsi_bad_req();
+        return;
+    }
 
-        if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0,
-                       &type, sizeof(type)) < sizeof(type)) {
+    virtio_tswap32s(vdev, &req->req.tmf.type);
+    if (req->req.tmf.type == VIRTIO_SCSI_T_TMF) {
+        if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
+                    sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
             virtio_scsi_bad_req();
-            continue;
+        } else {
+            virtio_scsi_do_tmf(s, req);
         }
 
-        virtio_tswap32s(vdev, &req->req.tmf.type);
-        if (req->req.tmf.type == VIRTIO_SCSI_T_TMF) {
-            if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
-                                      sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
-                virtio_scsi_bad_req();
-            } else {
-                virtio_scsi_do_tmf(s, req);
-            }
-
-        } else if (req->req.tmf.type == VIRTIO_SCSI_T_AN_QUERY ||
-                   req->req.tmf.type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
-            if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq),
-                                      sizeof(VirtIOSCSICtrlANResp)) < 0) {
-                virtio_scsi_bad_req();
-            } else {
-                req->resp.an.event_actual = 0;
-                req->resp.an.response = VIRTIO_SCSI_S_OK;
-            }
+    } else if (req->req.tmf.type == VIRTIO_SCSI_T_AN_QUERY ||
+            req->req.tmf.type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
+        if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq),
+                    sizeof(VirtIOSCSICtrlANResp)) < 0) {
+            virtio_scsi_bad_req();
+        } else {
+            req->resp.an.event_actual = 0;
+            req->resp.an.response = VIRTIO_SCSI_S_OK;
         }
-        virtio_scsi_complete_req(req);
+    }
+    virtio_scsi_complete_req(req);
+}
+
+static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+    VirtIOSCSIReq *req;
+
+    while ((req = virtio_scsi_pop_req(s, vq))) {
+        virtio_scsi_handle_ctrl_req(s, req);
     }
 }
 
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 7349936..fcd45e7 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -217,6 +217,7 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp,
                                 HandleOutput cmd);
 
 void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp);
+void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req);
 void virtio_scsi_handle_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req);
 
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 04/10] virtio-scsi: Add VirtIOSCSIVring in VirtIOSCSIReq

[Fam Zheng]

Move VirtIOSCSIReq to header and add one field "vring" as a wrapper
structure of Vring, VirtIOSCSIVring.

This is necessary for coming dataplane code that runs uses vring on
iothread.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/hw/virtio/virtio-scsi.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index fcd45e7..6138674 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -17,6 +17,8 @@
 #include "hw/virtio/virtio.h"
 #include "hw/pci/pci.h"
 #include "hw/scsi/scsi.h"
+#include "sysemu/iothread.h"
+#include "hw/virtio/dataplane/vring.h"
 
 #define TYPE_VIRTIO_SCSI_COMMON "virtio-scsi-common"
 #define VIRTIO_SCSI_COMMON(obj) \
@@ -153,6 +155,15 @@ struct VirtIOSCSIConf {
     char *wwpn;
 };
 
+struct VirtIOSCSICommon;
+
+typedef struct {
+    struct VirtIOSCSICommon *parent;
+    Vring vring;
+    EventNotifier host_notifier;
+    EventNotifier guest_notifier;
+} VirtIOSCSIVring;
+
 typedef struct VirtIOSCSICommon {
     VirtIODevice parent_obj;
     VirtIOSCSIConf conf;
@@ -175,6 +186,9 @@ typedef struct {
 typedef struct VirtIOSCSIReq {
     VirtIOSCSI *dev;
     VirtQueue *vq;
+    /* set if create by dataplane code */
+    VirtIOSCSIVring *vring;
+
     VirtQueueElement elem;
     QEMUSGList qsgl;
     SCSIRequest *sreq;
-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 05/10] virtio-scsi: Make virtio_scsi_init_req public

[Fam Zheng]

To share with datplane code later.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/virtio-scsi.c           | 2 +-
 include/hw/virtio/virtio-scsi.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index cb2745d..4a65653 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -40,7 +40,7 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
     return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
 }
 
-static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
+VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
 {
     VirtIOSCSIReq *req;
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 6138674..3f46493 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -233,5 +233,6 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp,
 void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp);
 void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req);
 void virtio_scsi_handle_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req);
+VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq);
 
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 06/10] virtio-scsi: Make virtio_scsi_free_req public

[Fam Zheng]

To share with dataplane code later.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/virtio-scsi.c           | 2 +-
 include/hw/virtio/virtio-scsi.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 4a65653..c5a6b63 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -55,7 +55,7 @@ VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
     return req;
 }
 
-static void virtio_scsi_free_req(VirtIOSCSIReq *req)
+void virtio_scsi_free_req(VirtIOSCSIReq *req)
 {
     qemu_iovec_destroy(&req->resp_iov);
     qemu_sglist_destroy(&req->qsgl);
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 3f46493..465d73d 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -234,5 +234,6 @@ void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp);
 void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req);
 void virtio_scsi_handle_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req);
 VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq);
+void virtio_scsi_free_req(VirtIOSCSIReq *req);
 
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 07/10] virtio-scsi: Make virtio_scsi_push_event public

[Fam Zheng]

Later this will be called by dataplane code.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/virtio-scsi.c           | 4 ++--
 include/hw/virtio/virtio-scsi.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index c5a6b63..8fa9bec 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -543,8 +543,8 @@ 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)
+void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
+                            uint32_t event, uint32_t reason)
 {
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
     VirtIOSCSIReq *req;
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 465d73d..4b6c266 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -235,5 +235,7 @@ void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req);
 void virtio_scsi_handle_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req);
 VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq);
 void virtio_scsi_free_req(VirtIOSCSIReq *req);
+void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
+                            uint32_t event, uint32_t reason);
 
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 08/10] virtio-scsi: Add 'iothread' property to virtio-scsi-pci

[Fam Zheng]

Similar to this property in virtio-blk for dataplane, add it as a QOM
link in virtio-scsi and an alias in virtio-scsi-pci, in order to assign
an iothread to the device.

Other bus types can be added later.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/virtio-scsi.c           | 11 +++++++++++
 hw/virtio/virtio-pci.c          |  2 ++
 include/hw/virtio/virtio-scsi.h |  1 +
 3 files changed, 14 insertions(+)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 8fa9bec..9e78e21 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -703,6 +703,16 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
                     virtio_scsi_save, virtio_scsi_load, s);
 }
 
+static void virtio_scsi_instance_init(Object *obj)
+{
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(obj);
+
+    object_property_add_link(obj, "iothread", TYPE_IOTHREAD,
+                             (Object **)&vs->conf.iothread,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);
+}
+
 void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -761,6 +771,7 @@ static const TypeInfo virtio_scsi_info = {
     .name = TYPE_VIRTIO_SCSI,
     .parent = TYPE_VIRTIO_SCSI_COMMON,
     .instance_size = sizeof(VirtIOSCSI),
+    .instance_init = virtio_scsi_instance_init,
     .class_init = virtio_scsi_class_init,
 };
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 3007319..176b021 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1179,6 +1179,8 @@ static void virtio_scsi_pci_instance_init(Object *obj)
     VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_SCSI);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+    object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev), "iothread",
+                              &error_abort);
 }
 
 static const TypeInfo virtio_scsi_pci_info = {
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 4b6c266..6f92c29 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -153,6 +153,7 @@ struct VirtIOSCSIConf {
     uint32_t cmd_per_lun;
     char *vhostfd;
     char *wwpn;
+    IOThread *iothread;
 };
 
 struct VirtIOSCSICommon;
-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread

[Fam Zheng]

This implements the core part of dataplane feature of virtio-scsi.

A few fields are added in VirtIOSCSICommon to maintain the dataplane
status. These fields are managed by a new source file:
virtio-scsi-dataplane.c.

Most code in this file will run on an iothread, unless otherwise
commented as in a global mutex context, such as those functions to
start, stop and setting the iothread property.

Upon start, we set up guest/host event notifiers, in a same way as
virtio-blk does. The handlers then pop request from vring and call into
virtio-scsi.c functions to process it. So we need to make sure make all
those called functions work with iothread, too.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/Makefile.objs           |   2 +-
 hw/scsi/virtio-scsi-dataplane.c | 219 ++++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-scsi.h |  19 ++++
 3 files changed, 239 insertions(+), 1 deletion(-)
 create mode 100644 hw/scsi/virtio-scsi-dataplane.c

diff --git a/hw/scsi/Makefile.objs b/hw/scsi/Makefile.objs
index 121ddc5..40c79d3 100644
--- a/hw/scsi/Makefile.objs
+++ b/hw/scsi/Makefile.objs
@@ -8,6 +8,6 @@ common-obj-$(CONFIG_ESP_PCI) += esp-pci.o
 obj-$(CONFIG_PSERIES) += spapr_vscsi.o
 
 ifeq ($(CONFIG_VIRTIO),y)
-obj-y += virtio-scsi.o
+obj-y += virtio-scsi.o virtio-scsi-dataplane.o
 obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
 endif
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
new file mode 100644
index 0000000..d077b67
--- /dev/null
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -0,0 +1,219 @@
+/*
+ * Virtio SCSI dataplane
+ *
+ * Copyright Red Hat, Inc. 2014
+ *
+ * Authors:
+ *   Fam Zheng <famz@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/virtio/virtio-scsi.h"
+#include "qemu/error-report.h"
+#include <hw/scsi/scsi.h>
+#include <block/scsi.h>
+#include <hw/virtio/virtio-bus.h>
+#include "hw/virtio/virtio-access.h"
+#include "stdio.h"
+
+/* Context: QEMU global mutex held */
+void virtio_scsi_set_iothread(VirtIOSCSICommon *s, IOThread *iothread)
+{
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    s->ctx = iothread_get_aio_context(s->conf.iothread);
+
+    /* Don't try if transport does not support notifiers. */
+    if (!k->set_guest_notifiers || !k->set_host_notifier) {
+        fprintf(stderr, "virtio-scsi: Failed to set iothread "
+                   "(transport does not support notifiers)");
+        exit(1);
+    }
+}
+
+static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSICommon *s,
+                                               VirtQueue *vq,
+                                               EventNotifierHandler *handler,
+                                               int n)
+{
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+    VirtIOSCSIVring *r = g_slice_new(VirtIOSCSIVring);
+
+    /* Set up virtqueue notify */
+    if (k->set_host_notifier(qbus->parent, n, true) != 0) {
+        fprintf(stderr, "virtio-scsi: Failed to set host notifier\n");
+        exit(1);
+    }
+    r->host_notifier = *virtio_queue_get_host_notifier(vq);
+    r->guest_notifier = *virtio_queue_get_guest_notifier(vq);
+    aio_set_event_notifier(s->ctx, &r->host_notifier, handler);
+
+    r->parent = s;
+
+    if (!vring_setup(&r->vring, VIRTIO_DEVICE(s), n)) {
+        fprintf(stderr, "virtio-scsi: VRing setup failed\n");
+        exit(1);
+    }
+    return r;
+}
+
+VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s,
+                                         VirtIOSCSIVring *vring)
+{
+    VirtIOSCSIReq *req = virtio_scsi_init_req(s, NULL);
+    int r;
+
+    req->vring = vring;
+    r = vring_pop((VirtIODevice *)s, &vring->vring, &req->elem);
+    if (r < 0) {
+        virtio_scsi_free_req(req);
+        req = NULL;
+    }
+    return req;
+}
+
+void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req)
+{
+    vring_push(&req->vring->vring, &req->elem,
+               req->qsgl.size + req->resp_iov.size);
+    event_notifier_set(&req->vring->guest_notifier);
+}
+
+static void virtio_scsi_iothread_handle_ctrl(EventNotifier *notifier)
+{
+    VirtIOSCSIVring *vring = container_of(notifier,
+                                          VirtIOSCSIVring, host_notifier);
+    VirtIOSCSI *s = VIRTIO_SCSI(vring->parent);
+    VirtIOSCSIReq *req;
+
+    event_notifier_test_and_clear(notifier);
+    while ((req = virtio_scsi_pop_req_vring(s, vring))) {
+        virtio_scsi_handle_ctrl_req(s, req);
+    }
+}
+
+static void virtio_scsi_iothread_handle_event(EventNotifier *notifier)
+{
+    VirtIOSCSIVring *vring = container_of(notifier,
+                                          VirtIOSCSIVring, host_notifier);
+    VirtIOSCSICommon *vs = vring->parent;
+    VirtIOSCSI *s = VIRTIO_SCSI(vs);
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    event_notifier_test_and_clear(notifier);
+
+    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+        return;
+    }
+
+    if (s->events_dropped) {
+        virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
+    }
+}
+
+static void virtio_scsi_iothread_handle_cmd(EventNotifier *notifier)
+{
+    VirtIOSCSIVring *vring = container_of(notifier,
+                                          VirtIOSCSIVring, host_notifier);
+    VirtIOSCSI *s = VIRTIO_SCSI(vring->parent);
+    VirtIOSCSIReq *req;
+
+    event_notifier_test_and_clear(notifier);
+    while ((req = virtio_scsi_pop_req_vring(s, vring))) {
+        virtio_scsi_handle_cmd_req(s, req);
+    }
+}
+
+/* Context: QEMU global mutex held */
+void virtio_scsi_dataplane_start(VirtIOSCSICommon *s)
+{
+    int i;
+    int rc;
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    if (s->dataplane_started ||
+        s->dataplane_starting ||
+        s->ctx != iothread_get_aio_context(s->conf.iothread)) {
+        return;
+    }
+
+    s->dataplane_starting = true;
+
+    /* Set up guest notifier (irq) */
+    rc = k->set_guest_notifiers(qbus->parent, s->conf.num_queues + 2, true);
+    if (rc != 0) {
+        fprintf(stderr, "virtio-scsi: Failed to set guest notifiers, "
+                "ensure -enable-kvm is set\n");
+        exit(1);
+    }
+
+    aio_context_acquire(s->ctx);
+    s->ctrl_vring = virtio_scsi_vring_init(s, s->ctrl_vq,
+                                           virtio_scsi_iothread_handle_ctrl,
+                                           0);
+    s->event_vring = virtio_scsi_vring_init(s, s->event_vq,
+                                            virtio_scsi_iothread_handle_event,
+                                            1);
+    s->cmd_vrings = g_malloc0(sizeof(VirtIOSCSIVring) * s->conf.num_queues);
+    for (i = 0; i < s->conf.num_queues; i++) {
+        s->cmd_vrings[i] =
+            virtio_scsi_vring_init(s, s->cmd_vqs[i],
+                                   virtio_scsi_iothread_handle_cmd,
+                                   i + 2);
+    }
+
+    aio_context_release(s->ctx);
+    s->dataplane_starting = false;
+    s->dataplane_started = true;
+}
+
+/* Context: QEMU global mutex held */
+void virtio_scsi_dataplane_stop(VirtIOSCSICommon *s)
+{
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+    int i;
+
+    if (!s->dataplane_started || s->dataplane_stopping) {
+        return;
+    }
+    s->dataplane_stopping = true;
+    assert(s->ctx == iothread_get_aio_context(s->conf.iothread));
+
+    aio_context_acquire(s->ctx);
+
+    aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier, NULL);
+    aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier, NULL);
+    for (i = 0; i < s->conf.num_queues; i++) {
+        aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier, NULL);
+    }
+
+    bdrv_drain_all(); /* ensure there are no in-flight requests */
+
+    aio_context_release(s->ctx);
+
+    /* Sync vring state back to virtqueue so that non-dataplane request
+     * processing can continue when we disable the host notifier below.
+     */
+    vring_teardown(&s->ctrl_vring->vring, vdev, 0);
+    vring_teardown(&s->event_vring->vring, vdev, 1);
+    for (i = 0; i < s->conf.num_queues; i++) {
+        vring_teardown(&s->cmd_vrings[i]->vring, vdev, 2 + i);
+    }
+
+    for (i = 0; i < s->conf.num_queues + 2; i++) {
+        k->set_host_notifier(qbus->parent, i, false);
+    }
+
+    /* Clean up guest notifier (irq) */
+    k->set_guest_notifiers(qbus->parent, s->conf.num_queues + 2, false);
+    s->dataplane_stopping = false;
+    s->dataplane_started = false;
+}
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 6f92c29..b9f2197 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -174,6 +174,18 @@ typedef struct VirtIOSCSICommon {
     VirtQueue *ctrl_vq;
     VirtQueue *event_vq;
     VirtQueue **cmd_vqs;
+
+    /* Fields for dataplane below */
+    AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
+
+    /* Vring is used instead of vq in dataplane code, because of the underlying
+     * memory layer thread safety */
+    VirtIOSCSIVring *ctrl_vring;
+    VirtIOSCSIVring *event_vring;
+    VirtIOSCSIVring **cmd_vrings;
+    bool dataplane_started;
+    bool dataplane_starting;
+    bool dataplane_stopping;
 } VirtIOSCSICommon;
 
 typedef struct {
@@ -239,4 +251,11 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req);
 void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
                             uint32_t event, uint32_t reason);
 
+void virtio_scsi_set_iothread(VirtIOSCSICommon *s, IOThread *iothread);
+void virtio_scsi_dataplane_start(VirtIOSCSICommon *s);
+void virtio_scsi_dataplane_stop(VirtIOSCSICommon *s);
+void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req);
+VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s,
+                                         VirtIOSCSIVring *vring);
+
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
2.0.3

[Qemu-devel] [RFC PATCH v2 10/10] virtio-scsi: Hook up with dataplane

[Fam Zheng]

This enables the virtio-scsi-dataplane code by setting the iothread
in virtio-scsi device, and makes any function that is called by
back from dataplane to cooperate with the caller: they need to be
vring/iothread aware when handling the requests and using scsi devices
on the bus.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/virtio-scsi.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 9e78e21..1f2a9b6 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -62,6 +62,22 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req)
     g_free(req);
 }
 
+static void virtio_scsi_aio_acquire(VirtIOSCSICommon *vs)
+{
+    if (vs->dataplane_started) {
+        assert(vs->ctx);
+        aio_context_acquire(vs->ctx);
+    }
+}
+
+static void virtio_scsi_aio_release(VirtIOSCSICommon *vs)
+{
+    if (vs->dataplane_started) {
+        assert(vs->ctx);
+        aio_context_release(vs->ctx);
+    }
+}
+
 static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
 {
     VirtIOSCSI *s = req->dev;
@@ -69,13 +85,19 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
 
     qemu_iovec_from_buf(&req->resp_iov, 0, &req->resp, req->resp_size);
-    virtqueue_push(vq, &req->elem, req->qsgl.size + req->resp_iov.size);
+    if (req->vring) {
+        assert(req->vq == NULL);
+        virtio_scsi_vring_push_notify(req);
+    } else {
+        virtqueue_push(vq, &req->elem, req->qsgl.size + req->resp_iov.size);
+        virtio_notify(vdev, vq);
+    }
+
     if (req->sreq) {
         req->sreq->hba_private = NULL;
         scsi_req_unref(req->sreq);
     }
     virtio_scsi_free_req(req);
-    virtio_notify(vdev, vq);
 }
 
 static void virtio_scsi_bad_req(void)
@@ -204,10 +226,16 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
 static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
 {
     SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun);
+    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)s;
     SCSIRequest *r, *next;
     BusChild *kid;
     int target;
 
+    if (vs->dataplane_started && bdrv_get_aio_context(d->conf.bs) != vs->ctx) {
+        aio_context_acquire(vs->ctx);
+        bdrv_set_aio_context(d->conf.bs, vs->ctx);
+        aio_context_release(vs->ctx);
+    }
     /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE".  */
     req->resp.tmf.response = VIRTIO_SCSI_S_OK;
 
@@ -344,8 +372,13 @@ void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
 static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
     VirtIOSCSIReq *req;
 
+    if (vs->ctx) {
+        virtio_scsi_dataplane_start(vs);
+        return;
+    }
     while ((req = virtio_scsi_pop_req(s, vq))) {
         virtio_scsi_handle_ctrl_req(s, req);
     }
@@ -439,6 +472,11 @@ void virtio_scsi_handle_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
         virtio_scsi_complete_cmd_req(req);
         return;
     }
+    if (vs->dataplane_started && bdrv_get_aio_context(d->conf.bs) != vs->ctx) {
+        aio_context_acquire(vs->ctx);
+        bdrv_set_aio_context(d->conf.bs, vs->ctx);
+        aio_context_release(vs->ctx);
+    }
     req->sreq = scsi_req_new(d, req->req.cmd.tag,
                              virtio_scsi_get_lun(req->req.cmd.lun),
                              req->req.cdb, req);
@@ -461,8 +499,13 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
 {
     /* use non-QOM casts in the data path */
     VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
     VirtIOSCSIReq *req;
 
+    if (vs->ctx) {
+        virtio_scsi_dataplane_start(vs);
+        return;
+    }
     while ((req = virtio_scsi_pop_req(s, vq))) {
         virtio_scsi_handle_cmd_req(s, req);
     }
@@ -513,6 +556,9 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
     VirtIOSCSI *s = VIRTIO_SCSI(vdev);
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
 
+    if (vs->ctx) {
+        virtio_scsi_dataplane_stop(vs);
+    }
     s->resetting++;
     qbus_reset_all(&s->bus.qbus);
     s->resetting--;
@@ -555,7 +601,11 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
         return;
     }
 
-    req = virtio_scsi_pop_req(s, vs->event_vq);
+    if (vs->dataplane_started) {
+        req = virtio_scsi_pop_req_vring(s, vs->event_vring);
+    } else {
+        req = virtio_scsi_pop_req(s, vs->event_vq);
+    }
     if (!req) {
         s->events_dropped = true;
         return;
@@ -592,7 +642,12 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
 static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
 
+    if (vs->ctx) {
+        virtio_scsi_dataplane_start(vs);
+        return;
+    }
     if (s->events_dropped) {
         virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
     }
@@ -602,11 +657,14 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
 {
     VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
+    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
 
     if (((vdev->guest_features >> VIRTIO_SCSI_F_CHANGE) & 1) &&
         dev->type != TYPE_ROM) {
+        virtio_scsi_aio_acquire(vs);
         virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE,
                                sense.asc | (sense.ascq << 8));
+        virtio_scsi_aio_release(vs);
     }
 }
 
@@ -614,10 +672,13 @@ static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
 {
     VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
+    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
 
     if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
+        virtio_scsi_aio_acquire(vs);
         virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
                                VIRTIO_SCSI_EVT_RESET_RESCAN);
+        virtio_scsi_aio_release(vs);
     }
 }
 
@@ -625,10 +686,13 @@ static void virtio_scsi_hot_unplug(SCSIBus *bus, SCSIDevice *dev)
 {
     VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
+    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
 
     if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
+        virtio_scsi_aio_acquire(vs);
         virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
                                VIRTIO_SCSI_EVT_RESET_REMOVED);
+        virtio_scsi_aio_release(vs);
     }
 }
 
@@ -671,6 +735,10 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp,
         s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE,
                                          cmd);
     }
+
+    if (s->conf.iothread) {
+        virtio_scsi_set_iothread(s, s->conf.iothread);
+    }
 }
 
 static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
-- 
2.0.3

Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread

[Paolo Bonzini]

Il 06/08/2014 07:35, Fam Zheng ha scritto:
>  ifeq ($(CONFIG_VIRTIO),y)
> -obj-y += virtio-scsi.o
> +obj-y += virtio-scsi.o virtio-scsi-dataplane.o
>  obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
>  endif

I first thought that this must be conditional on 
CONFIG_VIRTIO_BLK_DATA_PLANE.  However, CONFIG_VIRTIO_BLK_DATA_PLANE is 
itself obsolete:

##########################################
# adjust virtio-blk-data-plane based on linux-aio

if test "$virtio_blk_data_plane" = "yes" -a \
        "$linux_aio" != "yes" ; then
  error_exit "virtio-blk-data-plane requires Linux AIO, please try --enable-linux-aio"
elif test -z "$virtio_blk_data_plane" ; then
  virtio_blk_data_plane=$linux_aio
fi

and there's no requirement to have Linux AIO anymore.  Can you prepare a
patch to drop CONFIG_VIRTIO_BLK_DATA_PLANE, and replace patch 1 with it?

We can leave --disable-virtio-blk-data-plane and --enable-virtio-blk-data-plane
in for a couple of releases.

Paolo

Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread

[Fam Zheng]

On Wed, 08/06 10:45, Paolo Bonzini wrote:
> Il 06/08/2014 07:35, Fam Zheng ha scritto:
> >  ifeq ($(CONFIG_VIRTIO),y)
> > -obj-y += virtio-scsi.o
> > +obj-y += virtio-scsi.o virtio-scsi-dataplane.o
> >  obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
> >  endif
> 
> I first thought that this must be conditional on 
> CONFIG_VIRTIO_BLK_DATA_PLANE.  However, CONFIG_VIRTIO_BLK_DATA_PLANE is 
> itself obsolete:
> 
> ##########################################
> # adjust virtio-blk-data-plane based on linux-aio
> 
> if test "$virtio_blk_data_plane" = "yes" -a \
>         "$linux_aio" != "yes" ; then
>   error_exit "virtio-blk-data-plane requires Linux AIO, please try --enable-linux-aio"
> elif test -z "$virtio_blk_data_plane" ; then
>   virtio_blk_data_plane=$linux_aio
> fi
> 
> and there's no requirement to have Linux AIO anymore.  Can you prepare a
> patch to drop CONFIG_VIRTIO_BLK_DATA_PLANE, and replace patch 1 with it?
> 
> We can leave --disable-virtio-blk-data-plane and --enable-virtio-blk-data-plane
> in for a couple of releases.
> 

Yes. Sounds a good idea.

Fam

Re: [Qemu-devel] [RFC PATCH v2 00/10] virtio-scsi: Dataplane on single iothread

[Paolo Bonzini]

Il 06/08/2014 07:34, Fam Zheng ha scritto:
> Hi all,
> 
> This series adds "iothread" property to virtio-scsi in a way just similar to
> virtio-blk, and turns all scsi devices to run on top of it.
> 
> Example:
> 
>     -object iothread,id=iothread-1  \
>     -device virtio-scsi-pci,id=virtio-scsi-bus-0,iothread=iothread-1  \
>     -drive file=guest.img,id=scsi-disk-1,if=none,cache=none,aio=native  \
>     -device scsi-disk,lun=1,drive=scsi-disk-1,id=scsi-disk-1
> 
> It uses irqfd, ioeventfd and vring in a way just like virtio-blk does now.
> 
> Please review the general approach and see if major points are missed in terms
> of thread safety and completeness of the moved things from original context to
> iothread context.
> 
> Note that the used vring is not function complete compared to virtqueue
> implementation, because of its lacking of MMIO handling. So this is just an
> RFC.
> 
> Migration hasn't been looked into yet, either.  Assigning multiple iothreads is
> supposed to be worked on top of this as well, but it's not thoroughly planned
> yet.
> 
> Thanks,
> Fam
> 
> 
> Fam Zheng (10):
>   virtio: Compile vring code unconditionally
>   virtio-scsi: Split virtio_scsi_handle_cmd_req from
>     virtio_scsi_handle_cmd
>   virtio-scsi: Split virtio_scsi_handle_ctrl_req from
>     virtio_scsi_handle_ctrl
>   virtio-scsi: Add VirtIOSCSIVring in VirtIOSCSIReq
>   virtio-scsi: Make virtio_scsi_init_req public
>   virtio-scsi: Make virtio_scsi_free_req public
>   virtio-scsi: Make virtio_scsi_push_event public
>   virtio-scsi: Add 'iothread' property to virtio-scsi-pci
>   virtio-scsi-dataplane: Code to run virtio-scsi on iothread
>   virtio-scsi: Hook up with dataplane
> 
>  hw/scsi/Makefile.objs           |   2 +-
>  hw/scsi/virtio-scsi-dataplane.c | 219 +++++++++++++++++++++++++++++++++
>  hw/scsi/virtio-scsi.c           | 260 +++++++++++++++++++++++++---------------
>  hw/virtio/Makefile.objs         |   2 +-
>  hw/virtio/virtio-pci.c          |   2 +
>  include/hw/virtio/virtio-scsi.h |  65 ++++++++++
>  6 files changed, 450 insertions(+), 100 deletions(-)
>  create mode 100644 hw/scsi/virtio-scsi-dataplane.c
> 

I'm applying the patches to scsi-next.  However, please resubmit the
last two with the review comments addressed.  I'm only pushing them so
that we don't both spend time rebasing the branch.

Thanks,

Paolo

Thanks

Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread

[Paolo Bonzini]

Il 06/08/2014 07:35, Fam Zheng ha scritto:
> This implements the core part of dataplane feature of virtio-scsi.
> 
> A few fields are added in VirtIOSCSICommon to maintain the dataplane
> status. These fields are managed by a new source file:
> virtio-scsi-dataplane.c.
> 
> Most code in this file will run on an iothread, unless otherwise
> commented as in a global mutex context, such as those functions to
> start, stop and setting the iothread property.
> 
> Upon start, we set up guest/host event notifiers, in a same way as
> virtio-blk does. The handlers then pop request from vring and call into
> virtio-scsi.c functions to process it. So we need to make sure make all
> those called functions work with iothread, too.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/scsi/Makefile.objs           |   2 +-
>  hw/scsi/virtio-scsi-dataplane.c | 219 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-scsi.h |  19 ++++
>  3 files changed, 239 insertions(+), 1 deletion(-)
>  create mode 100644 hw/scsi/virtio-scsi-dataplane.c
> 
> diff --git a/hw/scsi/Makefile.objs b/hw/scsi/Makefile.objs
> index 121ddc5..40c79d3 100644
> --- a/hw/scsi/Makefile.objs
> +++ b/hw/scsi/Makefile.objs
> @@ -8,6 +8,6 @@ common-obj-$(CONFIG_ESP_PCI) += esp-pci.o
>  obj-$(CONFIG_PSERIES) += spapr_vscsi.o
>  
>  ifeq ($(CONFIG_VIRTIO),y)
> -obj-y += virtio-scsi.o
> +obj-y += virtio-scsi.o virtio-scsi-dataplane.o
>  obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
>  endif
> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> new file mode 100644
> index 0000000..d077b67
> --- /dev/null
> +++ b/hw/scsi/virtio-scsi-dataplane.c
> @@ -0,0 +1,219 @@
> +/*
> + * Virtio SCSI dataplane
> + *
> + * Copyright Red Hat, Inc. 2014
> + *
> + * Authors:
> + *   Fam Zheng <famz@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/virtio/virtio-scsi.h"
> +#include "qemu/error-report.h"
> +#include <hw/scsi/scsi.h>
> +#include <block/scsi.h>
> +#include <hw/virtio/virtio-bus.h>
> +#include "hw/virtio/virtio-access.h"
> +#include "stdio.h"
> +
> +/* Context: QEMU global mutex held */
> +void virtio_scsi_set_iothread(VirtIOSCSICommon *s, IOThread *iothread)
> +{
> +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    s->ctx = iothread_get_aio_context(s->conf.iothread);

assert that it's NULL?
> +
> +    /* Don't try if transport does not support notifiers. */
> +    if (!k->set_guest_notifiers || !k->set_host_notifier) {
> +        fprintf(stderr, "virtio-scsi: Failed to set iothread "
> +                   "(transport does not support notifiers)");
> +        exit(1);
> +    }
> +}
> +
> +static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSICommon *s,
> +                                               VirtQueue *vq,
> +                                               EventNotifierHandler *handler,
> +                                               int n)
> +{
> +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +    VirtIOSCSIVring *r = g_slice_new(VirtIOSCSIVring);
> +
> +    /* Set up virtqueue notify */
> +    if (k->set_host_notifier(qbus->parent, n, true) != 0) {
> +        fprintf(stderr, "virtio-scsi: Failed to set host notifier\n");
> +        exit(1);
> +    }
> +    r->host_notifier = *virtio_queue_get_host_notifier(vq);
> +    r->guest_notifier = *virtio_queue_get_guest_notifier(vq);
> +    aio_set_event_notifier(s->ctx, &r->host_notifier, handler);
> +
> +    r->parent = s;
> +
> +    if (!vring_setup(&r->vring, VIRTIO_DEVICE(s), n)) {
> +        fprintf(stderr, "virtio-scsi: VRing setup failed\n");
> +        exit(1);
> +    }
> +    return r;
> +}
> +
> +VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s,
> +                                         VirtIOSCSIVring *vring)
> +{
> +    VirtIOSCSIReq *req = virtio_scsi_init_req(s, NULL);
> +    int r;
> +
> +    req->vring = vring;
> +    r = vring_pop((VirtIODevice *)s, &vring->vring, &req->elem);
> +    if (r < 0) {
> +        virtio_scsi_free_req(req);
> +        req = NULL;
> +    }
> +    return req;
> +}
> +
> +void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req)
> +{
> +    vring_push(&req->vring->vring, &req->elem,
> +               req->qsgl.size + req->resp_iov.size);
> +    event_notifier_set(&req->vring->guest_notifier);
> +}
> +
> +static void virtio_scsi_iothread_handle_ctrl(EventNotifier *notifier)
> +{
> +    VirtIOSCSIVring *vring = container_of(notifier,
> +                                          VirtIOSCSIVring, host_notifier);
> +    VirtIOSCSI *s = VIRTIO_SCSI(vring->parent);
> +    VirtIOSCSIReq *req;
> +
> +    event_notifier_test_and_clear(notifier);
> +    while ((req = virtio_scsi_pop_req_vring(s, vring))) {
> +        virtio_scsi_handle_ctrl_req(s, req);
> +    }
> +}
> +
> +static void virtio_scsi_iothread_handle_event(EventNotifier *notifier)
> +{
> +    VirtIOSCSIVring *vring = container_of(notifier,
> +                                          VirtIOSCSIVring, host_notifier);
> +    VirtIOSCSICommon *vs = vring->parent;
> +    VirtIOSCSI *s = VIRTIO_SCSI(vs);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +
> +    event_notifier_test_and_clear(notifier);
> +
> +    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> +        return;
> +    }
> +
> +    if (s->events_dropped) {
> +        virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
> +    }
> +}
> +
> +static void virtio_scsi_iothread_handle_cmd(EventNotifier *notifier)
> +{
> +    VirtIOSCSIVring *vring = container_of(notifier,
> +                                          VirtIOSCSIVring, host_notifier);
> +    VirtIOSCSI *s = VIRTIO_SCSI(vring->parent);
> +    VirtIOSCSIReq *req;
> +
> +    event_notifier_test_and_clear(notifier);
> +    while ((req = virtio_scsi_pop_req_vring(s, vring))) {
> +        virtio_scsi_handle_cmd_req(s, req);
> +    }

Perhaps add bdrv_io_plug/bdrv_io_unplug?

> +}
> +
> +/* Context: QEMU global mutex held */

> +void virtio_scsi_dataplane_start(VirtIOSCSICommon *s)
> +{
> +    int i;
> +    int rc;
> +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    if (s->dataplane_started ||
> +        s->dataplane_starting ||
> +        s->ctx != iothread_get_aio_context(s->conf.iothread)) {
> +        return;
> +    }
> +
> +    s->dataplane_starting = true;

Please do a bdrv_drain_all() here.  Then you can set the contexts for
all children here too (for the hotplug case: in virtio_scsi_hotplug).

Then you do not have to do it in virtio_scsi_do_tmf and
virtio_scsi_handle_cmd_req.

> +    /* Set up guest notifier (irq) */
> +    rc = k->set_guest_notifiers(qbus->parent, s->conf.num_queues + 2, true);
> +    if (rc != 0) {
> +        fprintf(stderr, "virtio-scsi: Failed to set guest notifiers, "
> +                "ensure -enable-kvm is set\n");
> +        exit(1);
> +    }
> +
> +    aio_context_acquire(s->ctx);
> +    s->ctrl_vring = virtio_scsi_vring_init(s, s->ctrl_vq,
> +                                           virtio_scsi_iothread_handle_ctrl,
> +                                           0);
> +    s->event_vring = virtio_scsi_vring_init(s, s->event_vq,
> +                                            virtio_scsi_iothread_handle_event,
> +                                            1);
> +    s->cmd_vrings = g_malloc0(sizeof(VirtIOSCSIVring) * s->conf.num_queues);
> +    for (i = 0; i < s->conf.num_queues; i++) {
> +        s->cmd_vrings[i] =
> +            virtio_scsi_vring_init(s, s->cmd_vqs[i],
> +                                   virtio_scsi_iothread_handle_cmd,
> +                                   i + 2);
> +    }
> +
> +    aio_context_release(s->ctx);
> +    s->dataplane_starting = false;
> +    s->dataplane_started = true;
> +}
> +
> +/* Context: QEMU global mutex held */
> +void virtio_scsi_dataplane_stop(VirtIOSCSICommon *s)
> +{
> +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +    int i;
> +
> +    if (!s->dataplane_started || s->dataplane_stopping) {
> +        return;
> +    }
> +    s->dataplane_stopping = true;
> +    assert(s->ctx == iothread_get_aio_context(s->conf.iothread));
> +
> +    aio_context_acquire(s->ctx);
> +
> +    aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier, NULL);
> +    aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier, NULL);
> +    for (i = 0; i < s->conf.num_queues; i++) {
> +        aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier, NULL);
> +    }
> +
> +    bdrv_drain_all(); /* ensure there are no in-flight requests */
> +
> +    aio_context_release(s->ctx);
> +
> +    /* Sync vring state back to virtqueue so that non-dataplane request
> +     * processing can continue when we disable the host notifier below.
> +     */
> +    vring_teardown(&s->ctrl_vring->vring, vdev, 0);
> +    vring_teardown(&s->event_vring->vring, vdev, 1);
> +    for (i = 0; i < s->conf.num_queues; i++) {
> +        vring_teardown(&s->cmd_vrings[i]->vring, vdev, 2 + i);
> +    }
> +
> +    for (i = 0; i < s->conf.num_queues + 2; i++) {
> +        k->set_host_notifier(qbus->parent, i, false);
> +    }
> +
> +    /* Clean up guest notifier (irq) */
> +    k->set_guest_notifiers(qbus->parent, s->conf.num_queues + 2, false);
> +    s->dataplane_stopping = false;
> +    s->dataplane_started = false;
> +}
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index 6f92c29..b9f2197 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -174,6 +174,18 @@ typedef struct VirtIOSCSICommon {
>      VirtQueue *ctrl_vq;
>      VirtQueue *event_vq;
>      VirtQueue **cmd_vqs;
> +
> +    /* Fields for dataplane below */
> +    AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
> +
> +    /* Vring is used instead of vq in dataplane code, because of the underlying
> +     * memory layer thread safety */
> +    VirtIOSCSIVring *ctrl_vring;
> +    VirtIOSCSIVring *event_vring;
> +    VirtIOSCSIVring **cmd_vrings;
> +    bool dataplane_started;
> +    bool dataplane_starting;
> +    bool dataplane_stopping;

Please add these to VirtIOSCSI rather than VirtIOSCSICommon.  Same for
the new functions you declare below.

Paolo

>  } VirtIOSCSICommon;
>  
>  typedef struct {
> @@ -239,4 +251,11 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req);
>  void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
>                              uint32_t event, uint32_t reason);
>  
> +void virtio_scsi_set_iothread(VirtIOSCSICommon *s, IOThread *iothread);
> +void virtio_scsi_dataplane_start(VirtIOSCSICommon *s);
> +void virtio_scsi_dataplane_stop(VirtIOSCSICommon *s);
> +void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req);
> +VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s,
> +                                         VirtIOSCSIVring *vring);
> +
>  #endif /* _QEMU_VIRTIO_SCSI_H */
> 

Re: [Qemu-devel] [RFC PATCH v2 10/10] virtio-scsi: Hook up with dataplane

[Paolo Bonzini]

Il 06/08/2014 07:35, Fam Zheng ha scritto:
> This enables the virtio-scsi-dataplane code by setting the iothread
> in virtio-scsi device, and makes any function that is called by
> back from dataplane to cooperate with the caller: they need to be
> vring/iothread aware when handling the requests and using scsi devices
> on the bus.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/scsi/virtio-scsi.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 71 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 9e78e21..1f2a9b6 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -62,6 +62,22 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req)
>      g_free(req);
>  }
>  
> +static void virtio_scsi_aio_acquire(VirtIOSCSICommon *vs)
> +{
> +    if (vs->dataplane_started) {
> +        assert(vs->ctx);
> +        aio_context_acquire(vs->ctx);
> +    }
> +}
> +
> +static void virtio_scsi_aio_release(VirtIOSCSICommon *vs)
> +{
> +    if (vs->dataplane_started) {
> +        assert(vs->ctx);
> +        aio_context_release(vs->ctx);
> +    }
> +}

These are not needed if you do the acquire/release in
virtio_scsi_push_event.

>  static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
>  {
>      VirtIOSCSI *s = req->dev;
> @@ -69,13 +85,19 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
>      VirtIODevice *vdev = VIRTIO_DEVICE(s);
>  
>      qemu_iovec_from_buf(&req->resp_iov, 0, &req->resp, req->resp_size);
> -    virtqueue_push(vq, &req->elem, req->qsgl.size + req->resp_iov.size);
> +    if (req->vring) {
> +        assert(req->vq == NULL);
> +        virtio_scsi_vring_push_notify(req);
> +    } else {
> +        virtqueue_push(vq, &req->elem, req->qsgl.size + req->resp_iov.size);
> +        virtio_notify(vdev, vq);
> +    }
> +
>      if (req->sreq) {
>          req->sreq->hba_private = NULL;
>          scsi_req_unref(req->sreq);
>      }
>      virtio_scsi_free_req(req);
> -    virtio_notify(vdev, vq);
>  }
>  
>  static void virtio_scsi_bad_req(void)
> @@ -204,10 +226,16 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
>  static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
>  {
>      SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun);
> +    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)s;
>      SCSIRequest *r, *next;
>      BusChild *kid;
>      int target;
>  
> +    if (vs->dataplane_started && bdrv_get_aio_context(d->conf.bs) != vs->ctx) {
> +        aio_context_acquire(vs->ctx);
> +        bdrv_set_aio_context(d->conf.bs, vs->ctx);
> +        aio_context_release(vs->ctx);
> +    }
>      /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE".  */
>      req->resp.tmf.response = VIRTIO_SCSI_S_OK;
>  
> @@ -344,8 +372,13 @@ void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
>  static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>  {
>      VirtIOSCSI *s = (VirtIOSCSI *)vdev;
> +    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
>      VirtIOSCSIReq *req;
>  
> +    if (vs->ctx) {
> +        virtio_scsi_dataplane_start(vs);
> +        return;
> +    }
>      while ((req = virtio_scsi_pop_req(s, vq))) {
>          virtio_scsi_handle_ctrl_req(s, req);
>      }
> @@ -439,6 +472,11 @@ void virtio_scsi_handle_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
>          virtio_scsi_complete_cmd_req(req);
>          return;
>      }
> +    if (vs->dataplane_started && bdrv_get_aio_context(d->conf.bs) != vs->ctx) {
> +        aio_context_acquire(vs->ctx);
> +        bdrv_set_aio_context(d->conf.bs, vs->ctx);
> +        aio_context_release(vs->ctx);
> +    }
>      req->sreq = scsi_req_new(d, req->req.cmd.tag,
>                               virtio_scsi_get_lun(req->req.cmd.lun),
>                               req->req.cdb, req);
> @@ -461,8 +499,13 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
>  {
>      /* use non-QOM casts in the data path */
>      VirtIOSCSI *s = (VirtIOSCSI *)vdev;
> +    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
>      VirtIOSCSIReq *req;
>  
> +    if (vs->ctx) {
> +        virtio_scsi_dataplane_start(vs);
> +        return;
> +    }

A migration state change notifier (like in virtio-blk-dataplane) is missing.

>      while ((req = virtio_scsi_pop_req(s, vq))) {
>          virtio_scsi_handle_cmd_req(s, req);
>      }
> @@ -513,6 +556,9 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
>      VirtIOSCSI *s = VIRTIO_SCSI(vdev);
>      VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
>  
> +    if (vs->ctx) {
> +        virtio_scsi_dataplane_stop(vs);
> +    }
>      s->resetting++;
>      qbus_reset_all(&s->bus.qbus);
>      s->resetting--;
> @@ -555,7 +601,11 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
>          return;
>      }
>  
> -    req = virtio_scsi_pop_req(s, vs->event_vq);
> +    if (vs->dataplane_started) {
> +        req = virtio_scsi_pop_req_vring(s, vs->event_vring);
> +    } else {
> +        req = virtio_scsi_pop_req(s, vs->event_vq);
> +    }
>      if (!req) {
>          s->events_dropped = true;
>          return;
> @@ -592,7 +642,12 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
>  static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
>  {
>      VirtIOSCSI *s = VIRTIO_SCSI(vdev);
> +    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
>  
> +    if (vs->ctx) {
> +        virtio_scsi_dataplane_start(vs);
> +        return;
> +    }
>      if (s->events_dropped) {
>          virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
>      }
> @@ -602,11 +657,14 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
>  {
>      VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
>      VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
>  
>      if (((vdev->guest_features >> VIRTIO_SCSI_F_CHANGE) & 1) &&
>          dev->type != TYPE_ROM) {
> +        virtio_scsi_aio_acquire(vs);
>          virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE,
>                                 sense.asc | (sense.ascq << 8));
> +        virtio_scsi_aio_release(vs);
>      }
>  }
>  
> @@ -614,10 +672,13 @@ static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
>  {
>      VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
>      VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
>  
>      if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
> +        virtio_scsi_aio_acquire(vs);
>          virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
>                                 VIRTIO_SCSI_EVT_RESET_RESCAN);
> +        virtio_scsi_aio_release(vs);
>      }
>  }
>  
> @@ -625,10 +686,13 @@ static void virtio_scsi_hot_unplug(SCSIBus *bus, SCSIDevice *dev)
>  {
>      VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
>      VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vdev;
>  
>      if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
> +        virtio_scsi_aio_acquire(vs);
>          virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
>                                 VIRTIO_SCSI_EVT_RESET_REMOVED);
> +        virtio_scsi_aio_release(vs);
>      }
>  }
>  
> @@ -671,6 +735,10 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp,
>          s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE,
>                                           cmd);
>      }
> +
> +    if (s->conf.iothread) {
> +        virtio_scsi_set_iothread(s, s->conf.iothread);
> +    }
>  }
>  
>  static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
> 

Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread

[Fam Zheng]

On Fri, 09/19 11:29, Paolo Bonzini wrote:
> Il 06/08/2014 07:35, Fam Zheng ha scritto:
> > diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> > index 6f92c29..b9f2197 100644
> > --- a/include/hw/virtio/virtio-scsi.h
> > +++ b/include/hw/virtio/virtio-scsi.h
> > @@ -174,6 +174,18 @@ typedef struct VirtIOSCSICommon {
> >      VirtQueue *ctrl_vq;
> >      VirtQueue *event_vq;
> >      VirtQueue **cmd_vqs;
> > +
> > +    /* Fields for dataplane below */
> > +    AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
> > +
> > +    /* Vring is used instead of vq in dataplane code, because of the underlying
> > +     * memory layer thread safety */
> > +    VirtIOSCSIVring *ctrl_vring;
> > +    VirtIOSCSIVring *event_vring;
> > +    VirtIOSCSIVring **cmd_vrings;
> > +    bool dataplane_started;
> > +    bool dataplane_starting;
> > +    bool dataplane_stopping;
> 
> Please add these to VirtIOSCSI rather than VirtIOSCSICommon.  Same for
> the new functions you declare below.

What's the rationale, please? Asking because especially the VirtIOSCSIVring
fields are the dataplane counterparts of VirtQueue fields, so putting in
VirtIOSCSI seems unnatural for me.

Fam

Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread

[Fam Zheng]

On Fri, 09/19 11:29, Paolo Bonzini wrote:
> Il 06/08/2014 07:35, Fam Zheng ha scritto:
> > +void virtio_scsi_dataplane_start(VirtIOSCSICommon *s)
> > +{
> > +    int i;
> > +    int rc;
> > +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> > +
> > +    if (s->dataplane_started ||
> > +        s->dataplane_starting ||
> > +        s->ctx != iothread_get_aio_context(s->conf.iothread)) {
> > +        return;
> > +    }
> > +
> > +    s->dataplane_starting = true;
> 
> Please do a bdrv_drain_all() here.  Then you can set the contexts for
> all children here too (for the hotplug case: in virtio_scsi_hotplug).
> 
> Then you do not have to do it in virtio_scsi_do_tmf and
> virtio_scsi_handle_cmd_req.

Do we want multiple iothreads in the future? If yes, the aio context will need
to be checked/set for each command, that's why I put it in virtio_scsi_do_tmf
and virtio_scsi_handle_cmd_req.

Fam

> 
> > +    /* Set up guest notifier (irq) */
> > +    rc = k->set_guest_notifiers(qbus->parent, s->conf.num_queues + 2, true);
> > +    if (rc != 0) {
> > +        fprintf(stderr, "virtio-scsi: Failed to set guest notifiers, "
> > +                "ensure -enable-kvm is set\n");
> > +        exit(1);
> > +    }
> > +
> > +    aio_context_acquire(s->ctx);
> > +    s->ctrl_vring = virtio_scsi_vring_init(s, s->ctrl_vq,
> > +                                           virtio_scsi_iothread_handle_ctrl,
> > +                                           0);
> > +    s->event_vring = virtio_scsi_vring_init(s, s->event_vq,
> > +                                            virtio_scsi_iothread_handle_event,
> > +                                            1);
> > +    s->cmd_vrings = g_malloc0(sizeof(VirtIOSCSIVring) * s->conf.num_queues);
> > +    for (i = 0; i < s->conf.num_queues; i++) {
> > +        s->cmd_vrings[i] =
> > +            virtio_scsi_vring_init(s, s->cmd_vqs[i],
> > +                                   virtio_scsi_iothread_handle_cmd,
> > +                                   i + 2);
> > +    }
> > +
> > +    aio_context_release(s->ctx);
> > +    s->dataplane_starting = false;
> > +    s->dataplane_started = true;
> > +}
> > +
> > +/* Context: QEMU global mutex held */
> > +void virtio_scsi_dataplane_stop(VirtIOSCSICommon *s)
> > +{
> > +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
> > +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> > +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> > +    int i;
> > +
> > +    if (!s->dataplane_started || s->dataplane_stopping) {
> > +        return;
> > +    }
> > +    s->dataplane_stopping = true;
> > +    assert(s->ctx == iothread_get_aio_context(s->conf.iothread));
> > +
> > +    aio_context_acquire(s->ctx);
> > +
> > +    aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier, NULL);
> > +    aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier, NULL);
> > +    for (i = 0; i < s->conf.num_queues; i++) {
> > +        aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier, NULL);
> > +    }
> > +
> > +    bdrv_drain_all(); /* ensure there are no in-flight requests */
> > +
> > +    aio_context_release(s->ctx);
> > +
> > +    /* Sync vring state back to virtqueue so that non-dataplane request
> > +     * processing can continue when we disable the host notifier below.
> > +     */
> > +    vring_teardown(&s->ctrl_vring->vring, vdev, 0);
> > +    vring_teardown(&s->event_vring->vring, vdev, 1);
> > +    for (i = 0; i < s->conf.num_queues; i++) {
> > +        vring_teardown(&s->cmd_vrings[i]->vring, vdev, 2 + i);
> > +    }
> > +
> > +    for (i = 0; i < s->conf.num_queues + 2; i++) {
> > +        k->set_host_notifier(qbus->parent, i, false);
> > +    }
> > +
> > +    /* Clean up guest notifier (irq) */
> > +    k->set_guest_notifiers(qbus->parent, s->conf.num_queues + 2, false);
> > +    s->dataplane_stopping = false;
> > +    s->dataplane_started = false;
> > +}
> > diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> > index 6f92c29..b9f2197 100644
> > --- a/include/hw/virtio/virtio-scsi.h
> > +++ b/include/hw/virtio/virtio-scsi.h
> > @@ -174,6 +174,18 @@ typedef struct VirtIOSCSICommon {
> >      VirtQueue *ctrl_vq;
> >      VirtQueue *event_vq;
> >      VirtQueue **cmd_vqs;
> > +
> > +    /* Fields for dataplane below */
> > +    AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
> > +
> > +    /* Vring is used instead of vq in dataplane code, because of the underlying
> > +     * memory layer thread safety */
> > +    VirtIOSCSIVring *ctrl_vring;
> > +    VirtIOSCSIVring *event_vring;
> > +    VirtIOSCSIVring **cmd_vrings;
> > +    bool dataplane_started;
> > +    bool dataplane_starting;
> > +    bool dataplane_stopping;
> 
> Please add these to VirtIOSCSI rather than VirtIOSCSICommon.  Same for
> the new functions you declare below.
> 
> Paolo
> 
> >  } VirtIOSCSICommon;
> >  
> >  typedef struct {
> > @@ -239,4 +251,11 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req);
> >  void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
> >                              uint32_t event, uint32_t reason);
> >  
> > +void virtio_scsi_set_iothread(VirtIOSCSICommon *s, IOThread *iothread);
> > +void virtio_scsi_dataplane_start(VirtIOSCSICommon *s);
> > +void virtio_scsi_dataplane_stop(VirtIOSCSICommon *s);
> > +void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req);
> > +VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s,
> > +                                         VirtIOSCSIVring *vring);
> > +
> >  #endif /* _QEMU_VIRTIO_SCSI_H */
> > 
> 

Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread

[Paolo Bonzini]

Il 22/09/2014 07:56, Fam Zheng ha scritto:
>> > 
>> > Please add these to VirtIOSCSI rather than VirtIOSCSICommon.  Same for
>> > the new functions you declare below.
> What's the rationale, please? Asking because especially the VirtIOSCSIVring
> fields are the dataplane counterparts of VirtQueue fields, so putting in
> VirtIOSCSI seems unnatural for me.

Because everything you put in VirtIOSCSICommon will be shared between
virtio-scsi and vhost-scsi, and vhost-scsi need not use neither vring
nor dataplane.

Paolo

Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread

[Fam Zheng]

On Mon, 09/22 10:09, Paolo Bonzini wrote:
> Il 22/09/2014 07:56, Fam Zheng ha scritto:
> >> > 
> >> > Please add these to VirtIOSCSI rather than VirtIOSCSICommon.  Same for
> >> > the new functions you declare below.
> > What's the rationale, please? Asking because especially the VirtIOSCSIVring
> > fields are the dataplane counterparts of VirtQueue fields, so putting in
> > VirtIOSCSI seems unnatural for me.
> 
> Because everything you put in VirtIOSCSICommon will be shared between
> virtio-scsi and vhost-scsi, and vhost-scsi need not use neither vring
> nor dataplane.
> 

I see, I'll move it into VirtIOSCSI then!

Fam