diff mbox

[3/4] scsi-disk: introduce dma_readv and dma_writev

Message ID 1462966873-30473-4-git-send-email-pbonzini@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paolo Bonzini May 11, 2016, 11:41 a.m. UTC 
These are replacements for blk_aio_readv and blk_aio_writev that allow
customization of the data path.  They reuse the DMA helpers' DMAIOFunc
callback type, so that the same function can be used in either the
QEMUSGList or the bounce-buffered case.

This customization will be needed in the next patch to do zero-copy
SG_IO on scsi-block.

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

Comments

Eric Blake May 19, 2016, 3:11 p.m. UTC | #1 
On 05/11/2016 05:41 AM, Paolo Bonzini wrote:
> These are replacements for blk_aio_readv and blk_aio_writev that allow
> customization of the data path.  They reuse the DMA helpers' DMAIOFunc
> callback type, so that the same function can be used in either the
> QEMUSGList or the bounce-buffered case.
> 
> This customization will be needed in the next patch to do zero-copy
> SG_IO on scsi-block.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/scsi/scsi-disk.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 56 insertions(+), 8 deletions(-)
> 

> @@ -339,14 +354,16 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
>      if (r->req.sg) {
>          dma_acct_start(s->qdev.conf.blk, &r->acct, r->req.sg, BLOCK_ACCT_READ);
>          r->req.resid -= r->req.sg->size;
> -        r->req.aiocb = dma_blk_read(s->qdev.conf.blk, r->req.sg, r->sector,
> -                                    scsi_dma_complete, r);
> +        r->req.aiocb = dma_blk_io(blk_get_aio_context(s->qdev.conf.blk),
> +                                  r->req.sg, r->sector,
> +                                  sdc->dma_readv, r, scsi_dma_complete, r,
> +                                  DMA_DIRECTION_FROM_DEVICE);

Is it worth considering byte-based rather than sector-based interfaces,
as part of this series?

> +static
> +BlockAIOCB *scsi_dma_readv(int64_t sector_num,
> +                           QEMUIOVector *iov, int nb_sectors,
> +                           BlockCompletionFunc *cb, void *cb_opaque,
> +                           void *opaque)
> +{
> +    SCSIDiskReq *r = opaque;
> +    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
> +    return blk_aio_readv(s->qdev.conf.blk, sector_num, iov, nb_sectors,
> +                         cb, cb_opaque);
> +}

Especially since commit 7b1deac killed blk_aio_readv() in favor of
byte-based blk_aio_preadv().
Paolo Bonzini May 19, 2016, 3:13 p.m. UTC | #2 
On 19/05/2016 17:11, Eric Blake wrote:
>> > @@ -339,14 +354,16 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
>> >      if (r->req.sg) {
>> >          dma_acct_start(s->qdev.conf.blk, &r->acct, r->req.sg, BLOCK_ACCT_READ);
>> >          r->req.resid -= r->req.sg->size;
>> > -        r->req.aiocb = dma_blk_read(s->qdev.conf.blk, r->req.sg, r->sector,
>> > -                                    scsi_dma_complete, r);
>> > +        r->req.aiocb = dma_blk_io(blk_get_aio_context(s->qdev.conf.blk),
>> > +                                  r->req.sg, r->sector,
>> > +                                  sdc->dma_readv, r, scsi_dma_complete, r,
>> > +                                  DMA_DIRECTION_FROM_DEVICE);
> 
> Is it worth considering byte-based rather than sector-based interfaces,
> as part of this series?

I think it's a separate change.  I'm okay with doing the switch to
byte-based interfaces first.  Another related change would be to add a
"mask" argument, so that I could generalize this:

    if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
        qemu_iovec_discard_back(&dbs->iov,
                                dbs->iov.size & ~BDRV_SECTOR_MASK);
    }

For this series the mask needs to be the logical block size.

Thanks,

Paolo
diff mbox

Patch

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 45b604f..3ecfc46 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -55,7 +55,21 @@  do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 
 #define TYPE_SCSI_DISK_BASE         "scsi-disk-base"
 
+#define SCSI_DISK_BASE(obj) \
+     OBJECT_CHECK(SCSIDiskState, (obj), TYPE_SCSI_DISK_BASE)
+#define SCSI_DISK_BASE_CLASS(klass) \
+     OBJECT_CLASS_CHECK(SCSIDiskClass, (klass), TYPE_SCSI_DISK_BASE)
+#define SCSI_DISK_BASE_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(SCSIDiskClass, (obj), TYPE_SCSI_DISK_BASE)
+
 typedef struct SCSIDiskState SCSIDiskState;
+typedef struct SCSIDiskClass SCSIDiskClass;
+
+typedef struct SCSIDiskClass {
+    SCSIDeviceClass parent_class;
+    DMAIOFunc       *dma_readv;
+    DMAIOFunc       *dma_writev;
+} SCSIDiskClass;
 
 typedef struct SCSIDiskReq {
     SCSIRequest req;
@@ -318,6 +332,7 @@  done:
 static void scsi_do_read(SCSIDiskReq *r, int ret)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+    SCSIDiskClass *sdc = (SCSIDiskClass *) object_get_class(OBJECT(s));
     uint32_t n;
 
     assert (r->req.aiocb == NULL);
@@ -339,14 +354,16 @@  static void scsi_do_read(SCSIDiskReq *r, int ret)
     if (r->req.sg) {
         dma_acct_start(s->qdev.conf.blk, &r->acct, r->req.sg, BLOCK_ACCT_READ);
         r->req.resid -= r->req.sg->size;
-        r->req.aiocb = dma_blk_read(s->qdev.conf.blk, r->req.sg, r->sector,
-                                    scsi_dma_complete, r);
+        r->req.aiocb = dma_blk_io(blk_get_aio_context(s->qdev.conf.blk),
+                                  r->req.sg, r->sector,
+                                  sdc->dma_readv, r, scsi_dma_complete, r,
+                                  DMA_DIRECTION_FROM_DEVICE);
     } else {
         n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
         block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
                          n * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
-        r->req.aiocb = blk_aio_readv(s->qdev.conf.blk, r->sector, &r->qiov, n,
-                                     scsi_read_complete, r);
+        r->req.aiocb = sdc->dma_readv(r->sector, &r->qiov, n,
+                                      scsi_read_complete, r, r);
     }
 
 done:
@@ -506,6 +523,7 @@  static void scsi_write_data(SCSIRequest *req)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+    SCSIDiskClass *sdc = (SCSIDiskClass *) object_get_class(OBJECT(s));
     uint32_t n;
 
     /* No data transfer may already be in progress */
@@ -543,14 +561,16 @@  static void scsi_write_data(SCSIRequest *req)
     if (r->req.sg) {
         dma_acct_start(s->qdev.conf.blk, &r->acct, r->req.sg, BLOCK_ACCT_WRITE);
         r->req.resid -= r->req.sg->size;
-        r->req.aiocb = dma_blk_write(s->qdev.conf.blk, r->req.sg, r->sector,
-                                     scsi_dma_complete, r);
+        r->req.aiocb = dma_blk_io(blk_get_aio_context(s->qdev.conf.blk),
+                                  r->req.sg, r->sector,
+                                  sdc->dma_writev, r, scsi_dma_complete, r,
+                                  DMA_DIRECTION_TO_DEVICE);
     } else {
         n = r->qiov.size / 512;
         block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
                          n * BDRV_SECTOR_SIZE, BLOCK_ACCT_WRITE);
-        r->req.aiocb = blk_aio_writev(s->qdev.conf.blk, r->sector, &r->qiov, n,
-                                      scsi_write_complete, r);
+        r->req.aiocb = sdc->dma_writev(r->sector, &r->qiov, n,
+                                       scsi_write_complete, r, r);
     }
 }
 
@@ -2657,12 +2677,39 @@  static int scsi_block_parse_cdb(SCSIDevice *d, SCSICommand *cmd,
 
 #endif
 
+static
+BlockAIOCB *scsi_dma_readv(int64_t sector_num,
+                           QEMUIOVector *iov, int nb_sectors,
+                           BlockCompletionFunc *cb, void *cb_opaque,
+                           void *opaque)
+{
+    SCSIDiskReq *r = opaque;
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+    return blk_aio_readv(s->qdev.conf.blk, sector_num, iov, nb_sectors,
+                         cb, cb_opaque);
+}
+
+static
+BlockAIOCB *scsi_dma_writev(int64_t sector_num,
+                           QEMUIOVector *iov, int nb_sectors,
+                           BlockCompletionFunc *cb, void *cb_opaque,
+                           void *opaque)
+{
+    SCSIDiskReq *r = opaque;
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+    return blk_aio_writev(s->qdev.conf.blk, sector_num, iov, nb_sectors,
+                          cb, cb_opaque);
+}
+
 static void scsi_disk_base_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
 
     dc->fw_name = "disk";
     dc->reset = scsi_disk_reset;
+    sdc->dma_readv = scsi_dma_readv;
+    sdc->dma_writev = scsi_dma_writev;
 }
 
 static const TypeInfo scsi_disk_base_info = {
@@ -2670,6 +2717,7 @@  static const TypeInfo scsi_disk_base_info = {
     .parent        = TYPE_SCSI_DEVICE,
     .class_init    = scsi_disk_base_class_initfn,
     .instance_size = sizeof(SCSIDiskState),
+    .class_size    = sizeof(SCSIDiskClass),
 };
 
 #define DEFINE_SCSI_DISK_PROPERTIES()                                \