[Qemu-devel] [RFC 0/7] Convert SD card model to AIO

[Qemu-devel] [RFC 0/7] Convert SD card model to AIO

[Igor Mitsyanko]

This is an initial attempt to change our SD card model to use asynchronious
input/output API instead of synchronious one. This will require converting of
every user also. Right now I've converted only PL181 model, and I'll wait
for some feedback on taken approach before I'll continue with other users.

New async SD interface is built on two callbacks:
1 Callback which models arrival of start bit on DAT line from card (start bit)
2 Callback modeling deasserting of DAT0 line by card when it has finished
programming data (data busy end)

This is based on SD card specification.
Start bit is issued for every new block of data coming from card.
DAT0 line is kept low by card to signal busy state, while data block is being
programmed by card and it can't receive new data.

Furthermore, I decided to drop data buffering on SD controller side. We already
have two buffers in SD card model, no need to have another one in controller model.
We can avoid intermediate copying from card's buffer to controller's buffer, and
read/write directly from card.

Tested by running this Fedora image 
https://fedoraproject.org/wiki/Architectures/ARM/F18/Versatile_Express
on versatile board.

Igor Mitsyanko (7):
  sd.c: introduce AIO related members in SD state
  sd.c: introduce variable for trekking valid data
  sd.c: introduce "start bit" and "busy deasserted" callbacks
  sd.c: use callbacks as a flag to use async IO
  sd.c: introduce async read operation
  sd.c: introduce async write interface
  pl181.c: convert to async IO SD card interface

 hw/sd/omap_mmc.c    |   6 +-
 hw/sd/pl181.c       | 302 +++++++++++++++++++++++++++++++++-------------------
 hw/sd/pxa2xx_mmci.c |   2 +-
 hw/sd/sd.c          | 272 ++++++++++++++++++++++++++++++++++++++++++----
 hw/sd/sdhci.c       |   4 +-
 include/hw/sd.h     |   3 +-
 6 files changed, 449 insertions(+), 140 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [RFC 1/7] sd.c: introduce AIO related members in SD state

[Igor Mitsyanko]

New state members will be used for async IO implementation later.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---
 hw/sd/sd.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2e0ef3e..1dd1331 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -112,6 +112,10 @@ struct SDState {
     uint8_t *buf;
 
     bool enable;
+
+    QEMUIOVector qiov;
+    struct iovec iov;
+    BlockDriverAIOCB *aiocb;
 };
 
 static void sd_set_mode(SDState *sd)
@@ -403,6 +407,11 @@ static void sd_reset(SDState *sd, BlockDriverState *bdrv)
     uint64_t size;
     uint64_t sect;
 
+    if (sd->aiocb) {
+        bdrv_aio_cancel(sd->aiocb);
+        sd->aiocb = NULL;
+    }
+
     if (bdrv) {
         bdrv_get_geometry(bdrv, &sect);
     } else {
@@ -496,11 +505,15 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
     sd->buf = qemu_blockalign(bs, 512);
     sd->spi = is_spi;
     sd->enable = true;
+    sd->aiocb = NULL;
+    sd->iov.iov_base = NULL;
+    sd->iov.iov_len = BDRV_SECTOR_SIZE;
     sd_reset(sd, bs);
     if (sd->bdrv) {
         bdrv_attach_dev_nofail(sd->bdrv, sd);
         bdrv_set_dev_ops(sd->bdrv, &sd_block_ops, sd);
     }
+    qemu_iovec_init_external(&sd->qiov, &sd->iov, 1);
     vmstate_register(NULL, -1, &sd_vmstate, sd);
     return sd;
 }
-- 
1.8.1.4

[Qemu-devel] [RFC 2/7] sd.c: introduce variable for trekking valid data

[Igor Mitsyanko]

Initialize it appropriately when various commands are processed.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---
 hw/sd/sd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1dd1331..775a55c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -116,6 +116,7 @@ struct SDState {
     QEMUIOVector qiov;
     struct iovec iov;
     BlockDriverAIOCB *aiocb;
+    uint32_t transf_cnt;
 };
 
 static void sd_set_mode(SDState *sd)
-- 
1.8.1.4

[Qemu-devel] [RFC 3/7] sd.c: introduce "start bit" and "busy deasserted" callbacks

[Igor Mitsyanko]

Start bit callback models arrival of first bit of data on card's DAT line.

Busy deasserted callback models releasing of DAT0 line by card when it
transitions from a programming state to a writing data state.

Both of them will be used for async IO later.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---
 hw/sd/omap_mmc.c    |  6 +++---
 hw/sd/pl181.c       |  2 +-
 hw/sd/pxa2xx_mmci.c |  2 +-
 hw/sd/sd.c          | 24 +++++++++++++++++++++---
 hw/sd/sdhci.c       |  4 ++--
 include/hw/sd.h     |  3 ++-
 6 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index d4079cd..94580bb 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -620,7 +620,7 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
     s->card = sd_init(bd, 0);
 
     s->cdet = qemu_allocate_irqs(omap_mmc_cover_cb, s, 1)[0];
-    sd_set_cb(s->card, NULL, s->cdet);
+    sd_set_cb(s->card, NULL, s->cdet, NULL, NULL);
 
     return s;
 }
@@ -628,11 +628,11 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
 void omap_mmc_handlers(struct omap_mmc_s *s, qemu_irq ro, qemu_irq cover)
 {
     if (s->cdet) {
-        sd_set_cb(s->card, ro, s->cdet);
+        sd_set_cb(s->card, ro, s->cdet, NULL, NULL);
         s->coverswitch = cover;
         qemu_set_irq(cover, s->cdet_state);
     } else
-        sd_set_cb(s->card, ro, cover);
+        sd_set_cb(s->card, ro, cover, NULL, NULL);
 }
 
 void omap_mmc_enable(struct omap_mmc_s *s, int enable)
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 2527296..2caacc2 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -471,7 +471,7 @@ static void pl181_reset(DeviceState *d)
     s->mask[1] = 0;
 
     /* We can assume our GPIO outputs have been wired up now */
-    sd_set_cb(s->card, s->cardstatus[0], s->cardstatus[1]);
+    sd_set_cb(s->card, s->cardstatus[0], s->cardstatus[1], NULL, NULL);
 }
 
 static int pl181_init(SysBusDevice *dev)
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index 2db1cab..64a3050 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -549,5 +549,5 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
 void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly,
                 qemu_irq coverswitch)
 {
-    sd_set_cb(s->card, readonly, coverswitch);
+    sd_set_cb(s->card, readonly, coverswitch, NULL, NULL);
 }
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 775a55c..2e75201 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -108,6 +108,8 @@ struct SDState {
     uint8_t data[512];
     qemu_irq readonly_cb;
     qemu_irq inserted_cb;
+    qemu_irq start_bit_cb;
+    qemu_irq datbusy_cb;
     BlockDriverState *bdrv;
     uint8_t *buf;
 
@@ -519,10 +521,13 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
     return sd;
 }
 
-void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
+void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert,
+               qemu_irq startbit, qemu_irq endbusy)
 {
     sd->readonly_cb = readonly;
     sd->inserted_cb = insert;
+    sd->start_bit_cb = startbit;
+    sd->datbusy_cb = endbusy;
     qemu_set_irq(readonly, sd->bdrv ? bdrv_is_read_only(sd->bdrv) : 0);
     qemu_set_irq(insert, sd->bdrv ? bdrv_is_inserted(sd->bdrv) : 0);
 }
@@ -762,6 +767,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
+            qemu_irq_raise(sd->start_bit_cb);
             return sd_r1;
 
         default:
@@ -841,6 +847,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             memcpy(sd->data, sd->csd, 16);
             sd->data_start = addr;
             sd->data_offset = 0;
+            qemu_irq_raise(sd->start_bit_cb);
             return sd_r1;
 
         default:
@@ -863,6 +870,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             memcpy(sd->data, sd->cid, 16);
             sd->data_start = addr;
             sd->data_offset = 0;
+            qemu_irq_raise(sd->start_bit_cb);
             return sd_r1;
 
         default:
@@ -1111,6 +1119,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             *(uint32_t *) sd->data = sd_wpbits(sd, req.arg);
             sd->data_start = addr;
             sd->data_offset = 0;
+            qemu_irq_raise(sd->start_bit_cb);
             return sd_r1b;
 
         default:
@@ -1201,10 +1210,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         switch (sd->state) {
         case sd_transfer_state:
             sd->data_offset = 0;
-            if (req.arg & 1)
+            if (req.arg & 1) {
                 sd->state = sd_sendingdata_state;
-            else
+                qemu_irq_raise(sd->start_bit_cb);
+            } else {
                 sd->state = sd_receivingdata_state;
+            }
             return sd_r1;
 
         default:
@@ -1251,6 +1262,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
+            qemu_irq_raise(sd->start_bit_cb);
             return sd_r1;
 
         default:
@@ -1266,6 +1278,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
+            qemu_irq_raise(sd->start_bit_cb);
             return sd_r1;
 
         default:
@@ -1319,6 +1332,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
+            qemu_irq_raise(sd->start_bit_cb);
             return sd_r1;
 
         default:
@@ -1596,6 +1610,7 @@ void sd_write_data(SDState *sd, uint8_t value)
                 }
             /* Bzzzzzzztt .... Operation complete.  */
             sd->state = sd_transfer_state;
+            qemu_irq_raise(sd->datbusy_cb);
         }
         break;
 
@@ -1620,6 +1635,7 @@ void sd_write_data(SDState *sd, uint8_t value)
                 }
             /* Bzzzzzzztt .... Operation complete.  */
             sd->state = sd_transfer_state;
+            qemu_irq_raise(sd->datbusy_cb);
         }
         break;
 
@@ -1631,6 +1647,7 @@ void sd_write_data(SDState *sd, uint8_t value)
             sd_lock_command(sd);
             /* Bzzzzzzztt .... Operation complete.  */
             sd->state = sd_transfer_state;
+            qemu_irq_raise(sd->datbusy_cb);
         }
         break;
 
@@ -1639,6 +1656,7 @@ void sd_write_data(SDState *sd, uint8_t value)
         if (sd->data_offset >= sd->blk_len) {
             APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
             sd->state = sd_transfer_state;
+            qemu_irq_raise(sd->datbusy_cb);
         }
         break;
 
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 91dc9b0..bb33368 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -193,7 +193,7 @@ static void sdhci_reset(SDHCIState *s)
      * initialization */
     memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
 
-    sd_set_cb(s->card, s->ro_cb, s->eject_cb);
+    sd_set_cb(s->card, s->ro_cb, s->eject_cb, NULL, NULL);
     s->data_count = 0;
     s->stopped_state = sdhc_not_stopped;
 }
@@ -1168,7 +1168,7 @@ static void sdhci_initfn(Object *obj)
     s->card = sd_init(di ? di->bdrv : NULL, 0);
     s->eject_cb = qemu_allocate_irqs(sdhci_insert_eject_cb, s, 1)[0];
     s->ro_cb = qemu_allocate_irqs(sdhci_card_readonly_cb, s, 1)[0];
-    sd_set_cb(s->card, s->ro_cb, s->eject_cb);
+    sd_set_cb(s->card, s->ro_cb, s->eject_cb, NULL, NULL);
 
     s->insert_timer = qemu_new_timer_ns(vm_clock, sdhci_raise_insertion_irq, s);
     s->transfer_timer = qemu_new_timer_ns(vm_clock, sdhci_do_data_transfer, s);
diff --git a/include/hw/sd.h b/include/hw/sd.h
index d9b97e4..a7024d4 100644
--- a/include/hw/sd.h
+++ b/include/hw/sd.h
@@ -73,7 +73,8 @@ int sd_do_command(SDState *sd, SDRequest *req,
                   uint8_t *response);
 void sd_write_data(SDState *sd, uint8_t value);
 uint8_t sd_read_data(SDState *sd);
-void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert);
+void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert,
+               qemu_irq startbit, qemu_irq endbusy);
 bool sd_data_ready(SDState *sd);
 void sd_enable(SDState *sd, bool enable);
 
-- 
1.8.1.4

[Qemu-devel] [RFC 4/7] sd.c: use callbacks as a flag to use async IO

[Igor Mitsyanko]

This is temporary to distinguish between sync and async users.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---
 hw/sd/sd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2e75201..a0bbbaa 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -530,6 +530,10 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert,
     sd->datbusy_cb = endbusy;
     qemu_set_irq(readonly, sd->bdrv ? bdrv_is_read_only(sd->bdrv) : 0);
     qemu_set_irq(insert, sd->bdrv ? bdrv_is_inserted(sd->bdrv) : 0);
+
+    if (sd->start_bit_cb && sd->datbusy_cb) {
+        sd->iov.iov_base = sd->buf;
+    }
 }
 
 static void sd_erase(SDState *sd)
-- 
1.8.1.4

[Qemu-devel] [RFC 5/7] sd.c: introduce async read operation

[Igor Mitsyanko]

It will only be used if start bit and databusy callbacks were initialized
by user of SD card model.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---
 hw/sd/sd.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 77 insertions(+), 8 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a0bbbaa..659ec56 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -685,6 +685,44 @@ static void sd_lock_command(SDState *sd)
         sd->card_status &= ~CARD_IS_LOCKED;
 }
 
+static void sd_bdrv_read_done(void *opaque, int ret)
+{
+    SDState *sd = opaque;
+    uint32_t io_len, offset;
+    uint64_t end_sector;
+
+    DPRINTF("sd_bdrv_read_done ret = %d, \n", ret);
+    sd->aiocb = NULL;
+
+    if (ret != 0) {
+        return;
+    }
+
+    if (sd->state != sd_sendingdata_state) {
+        DPRINTF("Transfer was aborted\n");
+        return;
+    }
+
+    io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
+    end_sector = (sd->data_start + io_len - 1) >> BDRV_SECTOR_BITS;
+    offset = (sd->data_start + sd->transf_cnt) & ~BDRV_SECTOR_MASK;
+
+    if (end_sector >
+           (sd->data_start + sd->transf_cnt) >> BDRV_SECTOR_BITS) {
+        memcpy(sd->data, sd->buf + offset, BDRV_SECTOR_SIZE - offset);
+        sd->transf_cnt += BDRV_SECTOR_SIZE - offset;
+        sd->aiocb = bdrv_aio_readv(sd->bdrv, end_sector, &sd->qiov, 1,
+                sd_bdrv_read_done, sd);
+        return;
+    } else {
+        memcpy(sd->data + sd->transf_cnt, sd->buf + offset,
+                io_len - sd->transf_cnt);
+        sd->transf_cnt += io_len - sd->transf_cnt;
+    }
+
+    qemu_irq_raise(sd->start_bit_cb);
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd,
                                        SDRequest req)
 {
@@ -891,8 +929,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             sd->data_start = req.arg;
             sd->data_offset = 0;
 
-            if (sd->data_start + sd->blk_len > sd->size)
+            if (sd->data_start + sd->blk_len > sd->size) {
                 sd->card_status |= ADDRESS_ERROR;
+            } else if (sd->iov.iov_base) {
+                sd->aiocb = bdrv_aio_readv(sd->bdrv,
+                                           sd->data_start >> BDRV_SECTOR_BITS,
+                                           &sd->qiov, 1, sd_bdrv_read_done, sd);
+            }
             return sd_r0;
 
         default:
@@ -904,6 +947,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         switch (sd->state) {
         case sd_sendingdata_state:
             sd->state = sd_transfer_state;
+            sd->transf_cnt = 0;
+            if (sd->aiocb) {
+                bdrv_aio_cancel(sd->aiocb);
+                sd->aiocb = NULL;
+            }
             return sd_r1b;
 
         case sd_receivingdata_state:
@@ -969,8 +1017,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             sd->data_start = addr;
             sd->data_offset = 0;
 
-            if (sd->data_start + sd->blk_len > sd->size)
+            if (sd->data_start + sd->blk_len > sd->size) {
                 sd->card_status |= ADDRESS_ERROR;
+            } else {
+                sd->aiocb = bdrv_aio_readv(sd->bdrv,
+                                           sd->data_start >> BDRV_SECTOR_BITS,
+                                           &sd->qiov, 1, sd_bdrv_read_done, sd);
+            }
             return sd_r1;
 
         default:
@@ -985,8 +1038,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             sd->data_start = addr;
             sd->data_offset = 0;
 
-            if (sd->data_start + sd->blk_len > sd->size)
+            if (sd->data_start + sd->blk_len > sd->size) {
                 sd->card_status |= ADDRESS_ERROR;
+            } else {
+                sd->aiocb = bdrv_aio_readv(sd->bdrv,
+                                           sd->data_start >> BDRV_SECTOR_BITS,
+                                           &sd->qiov, 1, sd_bdrv_read_done, sd);
+            }
             return sd_r1;
 
         default:
@@ -1706,16 +1764,21 @@ uint8_t sd_read_data(SDState *sd)
         break;
 
     case 11:	/* CMD11:  READ_DAT_UNTIL_STOP */
-        if (sd->data_offset == 0)
+        if (sd->data_offset == 0 && sd->iov.iov_base == NULL) {
             BLK_READ_BLOCK(sd->data_start, io_len);
+        }
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= io_len) {
             sd->data_start += io_len;
             sd->data_offset = 0;
+            sd->transf_cnt = 0;
             if (sd->data_start + io_len > sd->size) {
                 sd->card_status |= ADDRESS_ERROR;
-                break;
+            } else if (sd->iov.iov_base) {
+                sd->aiocb = bdrv_aio_readv(sd->bdrv,
+                                           sd->data_start >> BDRV_SECTOR_BITS,
+                                           &sd->qiov, 1, sd_bdrv_read_done, sd);
             }
         }
         break;
@@ -1728,8 +1791,9 @@ uint8_t sd_read_data(SDState *sd)
         break;
 
     case 17:	/* CMD17:  READ_SINGLE_BLOCK */
-        if (sd->data_offset == 0)
+        if (sd->data_offset == 0 && sd->iov.iov_base == NULL) {
             BLK_READ_BLOCK(sd->data_start, io_len);
+        }
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= io_len)
@@ -1737,16 +1801,21 @@ uint8_t sd_read_data(SDState *sd)
         break;
 
     case 18:	/* CMD18:  READ_MULTIPLE_BLOCK */
-        if (sd->data_offset == 0)
+        if (sd->data_offset == 0 && sd->iov.iov_base == NULL) {
             BLK_READ_BLOCK(sd->data_start, io_len);
+        }
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= io_len) {
             sd->data_start += io_len;
             sd->data_offset = 0;
+            sd->transf_cnt = 0;
             if (sd->data_start + io_len > sd->size) {
                 sd->card_status |= ADDRESS_ERROR;
-                break;
+            } else if (sd->iov.iov_base) {
+                sd->aiocb = bdrv_aio_readv(sd->bdrv,
+                                           sd->data_start >> BDRV_SECTOR_BITS,
+                                           &sd->qiov, 1, sd_bdrv_read_done, sd);
             }
         }
         break;
-- 
1.8.1.4

[Qemu-devel] [RFC 6/7] sd.c: introduce async write interface

[Igor Mitsyanko]

Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---
 hw/sd/sd.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 133 insertions(+), 12 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 659ec56..615ab61 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -958,6 +958,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             sd->state = sd_programming_state;
             /* Bzzzzzzztt .... Operation complete.  */
             sd->state = sd_transfer_state;
+            sd->transf_cnt = 0;
+            if (sd->aiocb) {
+                bdrv_aio_cancel(sd->aiocb);
+                sd->aiocb = NULL;
+            }
             return sd_r1b;
 
         default:
@@ -1600,6 +1605,90 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 #define APP_READ_BLOCK(a, len)	memset(sd->data, 0xec, len)
 #define APP_WRITE_BLOCK(a, len)
 
+static void sd_write_done(void *opaque, int ret)
+{
+    SDState *sd = opaque;
+
+    DPRINTF("sd_write_done: ret = %d\n", ret);
+    sd->aiocb = NULL;
+
+    if (ret != 0) {
+        return;
+    }
+
+    switch (sd->current_cmd) {
+    case 24:    /* CMD24:  WRITE_SINGLE_BLOCK */
+        sd->blk_written++;
+        sd->csd[14] |= 0x40;
+        /* Bzzzzzzztt .... Operation complete.  */
+        sd->state = sd_transfer_state;
+        qemu_irq_raise(sd->datbusy_cb);
+        break;
+    case 25:    /* CMD25:  WRITE_MULTIPLE_BLOCK */
+        sd->blk_written++;
+        sd->data_start += sd->blk_len;
+        sd->data_offset = 0;
+        sd->transf_cnt = 0;
+        sd->csd[14] |= 0x40;
+        /* Bzzzzzzztt .... Operation complete.  */
+        sd->state = sd_receivingdata_state;
+        qemu_irq_raise(sd->datbusy_cb);
+        break;
+    default:
+        DPRINTF("unknown command\n");
+        break;
+    }
+}
+
+static void sd_read_next_sector(void *opaque, int ret);
+
+static void sd_sector_read_done(void *opaque, int ret)
+{
+    SDState *sd = opaque;
+    uint64_t end;
+    unsigned offset, start;
+
+    DPRINTF("sd_sector_read_done ret = %d\n", ret);
+    sd->aiocb = NULL;
+
+    if (ret != 0) {
+        return;
+    }
+
+    start = sd->data_start + sd->transf_cnt;
+    end = sd->data_start + sd->blk_len;
+    offset = start & ~BDRV_SECTOR_MASK;
+
+    if (end > ((start & BDRV_SECTOR_MASK) + BDRV_SECTOR_SIZE)) {
+        memcpy(sd->buf + offset, sd->data, BDRV_SECTOR_SIZE - offset);
+        sd->transf_cnt += BDRV_SECTOR_SIZE - offset;
+        sd->aiocb = bdrv_aio_writev(sd->bdrv, start >> BDRV_SECTOR_BITS,
+                                    &sd->qiov, 1, sd_read_next_sector, sd);
+    } else {
+        memcpy(sd->buf + offset, sd->data + sd->transf_cnt,
+            sd->blk_len - sd->transf_cnt);
+        sd->transf_cnt += sd->blk_len - sd->transf_cnt;
+        sd->aiocb = bdrv_aio_writev(sd->bdrv, start >> BDRV_SECTOR_BITS,
+                                    &sd->qiov, 1, sd_write_done, sd);
+    }
+}
+
+static void sd_read_next_sector(void *opaque, int ret)
+{
+    SDState *sd = opaque;
+
+    DPRINTF("sd_read_next_sector ret = %d\n", ret);
+    sd->aiocb = NULL;
+
+    if (ret != 0) {
+        return;
+    }
+
+    sd->aiocb = bdrv_aio_readv(sd->bdrv,
+                               (sd->data_start >> BDRV_SECTOR_BITS) + 1,
+                               &sd->qiov, 1, sd_sector_read_done, sd);
+}
+
 void sd_write_data(SDState *sd, uint8_t value)
 {
     int i;
@@ -1621,11 +1710,27 @@ void sd_write_data(SDState *sd, uint8_t value)
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
             sd->state = sd_programming_state;
-            BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
-            sd->blk_written ++;
-            sd->csd[14] |= 0x40;
-            /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_transfer_state;
+
+            if (sd->iov.iov_base) {
+                if ((sd->data_start & ~BDRV_SECTOR_MASK) ||
+                    sd->blk_len < BDRV_SECTOR_SIZE) {
+                    sd->aiocb = bdrv_aio_readv(sd->bdrv,
+                                   sd->data_start >> BDRV_SECTOR_BITS,
+                                   &sd->qiov, 1, sd_sector_read_done, sd);
+                    break;
+                }
+
+                memcpy(sd->buf, sd->data, sd->blk_len);
+                sd->aiocb = bdrv_aio_writev(sd->bdrv,
+                                            sd->data_start >> BDRV_SECTOR_BITS,
+                                            &sd->qiov, 1, sd_write_done, sd);
+            } else {
+                BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
+                sd->blk_written ++;
+                sd->csd[14] |= 0x40;
+                /* Bzzzzzzztt .... Operation complete.  */
+                sd->state = sd_transfer_state;
+            }
         }
         break;
 
@@ -1645,14 +1750,30 @@ void sd_write_data(SDState *sd, uint8_t value)
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
             sd->state = sd_programming_state;
-            BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
-            sd->blk_written++;
-            sd->data_start += sd->blk_len;
-            sd->data_offset = 0;
-            sd->csd[14] |= 0x40;
 
-            /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_receivingdata_state;
+            if (sd->iov.iov_base) {
+                if ((sd->data_start & ~BDRV_SECTOR_MASK) ||
+                    sd->blk_len < BDRV_SECTOR_SIZE) {
+                    sd->aiocb = bdrv_aio_readv(sd->bdrv,
+                                   sd->data_start >> BDRV_SECTOR_BITS,
+                                   &sd->qiov, 1, sd_sector_read_done, sd);
+                    break;
+                }
+
+                memcpy(sd->buf, sd->data, sd->blk_len);
+                sd->aiocb = bdrv_aio_writev(sd->bdrv,
+                                            sd->data_start >> BDRV_SECTOR_BITS,
+                                            &sd->qiov, 1, sd_write_done, sd);
+            } else {
+                BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
+                sd->blk_written++;
+                sd->data_start += sd->blk_len;
+                sd->data_offset = 0;
+                sd->csd[14] |= 0x40;
+
+                /* Bzzzzzzztt .... Operation complete.  */
+                sd->state = sd_receivingdata_state;
+            }
         }
         break;
 
-- 
1.8.1.4

[Qemu-devel] [RFC 7/7] pl181.c: convert to async IO SD card interface

[Igor Mitsyanko]

Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
---
 hw/sd/pl181.c | 302 +++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 192 insertions(+), 110 deletions(-)

diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 2caacc2..a8a3510 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -20,7 +20,7 @@ do { printf("pl181: " fmt , ## __VA_ARGS__); } while (0)
 #define DPRINTF(fmt, ...) do {} while(0)
 #endif
 
-#define PL181_FIFO_LEN 16
+#define PL181_FIFO_LEN 64
 
 typedef struct {
     SysBusDevice busdev;
@@ -39,23 +39,23 @@ typedef struct {
     uint32_t status;
     uint32_t mask[2];
     int32_t fifo_pos;
-    int32_t fifo_len;
     /* The linux 2.6.21 driver is buggy, and misbehaves if new data arrives
        while it is reading the FIFO.  We hack around this be defering
        subsequent transfers until after the driver polls the status word.
        http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4446/1
      */
     int32_t linux_hack;
-    uint32_t fifo[PL181_FIFO_LEN];
     qemu_irq irq[2];
     /* GPIO outputs for 'card is readonly' and 'card inserted' */
     qemu_irq cardstatus[2];
+    qemu_irq sbit_received;
+    qemu_irq busy_deasrt;
 } pl181_state;
 
 static const VMStateDescription vmstate_pl181 = {
     .name = "pl181",
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(clock, pl181_state),
         VMSTATE_UINT32(power, pl181_state),
@@ -70,9 +70,7 @@ static const VMStateDescription vmstate_pl181 = {
         VMSTATE_UINT32(status, pl181_state),
         VMSTATE_UINT32_ARRAY(mask, pl181_state, 2),
         VMSTATE_INT32(fifo_pos, pl181_state),
-        VMSTATE_INT32(fifo_len, pl181_state),
         VMSTATE_INT32(linux_hack, pl181_state),
-        VMSTATE_UINT32_ARRAY(fifo, pl181_state, PL181_FIFO_LEN),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -133,32 +131,110 @@ static void pl181_update(pl181_state *s)
     }
 }
 
+static inline unsigned int pl181_get_blk_size(pl181_state *s)
+{
+    if (s->datactrl & PL181_DATA_MODE) {
+        return 1;
+    } else {
+        return 1 << ((s->datactrl >> 4) & 0xf);
+    }
+}
+
 static void pl181_fifo_push(pl181_state *s, uint32_t value)
 {
-    int n;
+    unsigned i;
+    unsigned int blkdat_left = s->datacnt & (pl181_get_blk_size(s) - 1);
 
-    if (s->fifo_len == PL181_FIFO_LEN) {
-        fprintf(stderr, "pl181: FIFO overflow\n");
-        return;
+    if (blkdat_left == 0 && s->fifo_pos == 0) {
+        /* Start of block */
+        blkdat_left = pl181_get_blk_size(s);
+    }
+
+    for (i = 0; i < sizeof(uint32_t); ++i) {
+        if (s->fifo_pos >= PL181_FIFO_LEN) {
+            qemu_log_mask(LOG_GUEST_ERROR, "pl181_fifo_push: fifo overflow\n");
+            break;
+        }
+
+        sd_write_data(s->card, value >> i * 8);
+
+        if (blkdat_left > PL181_FIFO_LEN) {
+            --blkdat_left;
+            --s->datacnt;
+        } else {
+            ++s->fifo_pos;
+        }
+    }
+
+    if (s->fifo_pos == 0) {
+        s->status &= ~(PL181_STATUS_TXDATAAVLBL | PL181_STATUS_TXFIFOFULL);
+        s->status |= PL181_STATUS_TXFIFOEMPTY | PL181_STATUS_TXFIFOHALFEMPTY;
+    } else {
+        s->status &= ~(PL181_STATUS_TXFIFOEMPTY | PL181_STATUS_TXFIFOHALFEMPTY |
+            PL181_STATUS_TXFIFOFULL);
+        s->status |= PL181_STATUS_TXDATAAVLBL;
+
+        if (s->fifo_pos == PL181_FIFO_LEN) {
+            s->status |= PL181_STATUS_TXFIFOFULL;
+        } else if (s->fifo_pos <= (PL181_FIFO_LEN / 2)) {
+            s->status |= PL181_STATUS_TXFIFOHALFEMPTY;
+        }
     }
-    n = (s->fifo_pos + s->fifo_len) & (PL181_FIFO_LEN - 1);
-    s->fifo_len++;
-    s->fifo[n] = value;
-    DPRINTF("FIFO push %08x\n", (int)value);
+
+    DPRINTF("FIFO push %08x: datacnt=%u, fifo_pos=%u\n",
+        (int)value, s->datacnt, s->fifo_pos);
+    pl181_update(s);
 }
 
 static uint32_t pl181_fifo_pop(pl181_state *s)
 {
-    uint32_t value;
+    uint32_t value = 0;
+    uint32_t blksz_mask = pl181_get_blk_size(s) - 1;
+    unsigned i;
 
-    if (s->fifo_len == 0) {
-        fprintf(stderr, "pl181: FIFO underflow\n");
-        return 0;
+    for (i = 0; i < sizeof(uint32_t); ++i) {
+        if (s->fifo_pos == 0) {
+            qemu_log_mask(LOG_GUEST_ERROR, "pl181_fifo_pop: fifo underflow\n");
+            break;
+        }
+
+        value |= sd_read_data(s->card) << i * 8;
+
+        if (s->datacnt & blksz_mask) {
+            --s->datacnt;
+        } else {
+            --s->fifo_pos;
+        }
+    }
+
+    if (s->fifo_pos == 0) {
+        s->status |= PL181_STATUS_RXFIFOEMPTY | PL181_STATUS_DATABLOCKEND;
+        s->status &= ~(PL181_STATUS_RXDATAAVLBL | PL181_STATUS_RXFIFOFULL |
+            PL181_STATUS_RXFIFOHALFFULL);
+
+        if (s->datacnt == 0) {
+            s->status |= PL181_STATUS_DATAEND;
+            s->status &= ~PL181_STATUS_RX_FIFO;
+            s->datactrl &= ~PL181_DATA_ENABLE;
+            DPRINTF("Data engine idle\n");
+        }
+    } else {
+        s->status |= PL181_STATUS_RXDATAAVLBL;
+        s->status &= ~PL181_STATUS_RXFIFOEMPTY;
+
+        if (s->fifo_pos == PL181_FIFO_LEN) {
+            s->status |= PL181_STATUS_RXFIFOFULL;
+        }
+
+        if (s->fifo_pos >= (PL181_FIFO_LEN / 2)) {
+            s->status |= PL181_STATUS_RXFIFOHALFFULL;
+        }
     }
-    value = s->fifo[s->fifo_pos];
-    s->fifo_len--;
-    s->fifo_pos = (s->fifo_pos + 1) & (PL181_FIFO_LEN - 1);
-    DPRINTF("FIFO pop %08x\n", (int)value);
+
+    pl181_update(s);
+
+    DPRINTF("FIFO pop %08x: datacnt=%u\n", (int)value, s->datacnt);
+
     return value;
 }
 
@@ -207,82 +283,29 @@ error:
    the FIFO holding 32-bit words and the card taking data in single byte
    chunks.  FIFO bytes are transferred in little-endian order.  */
 
-static void pl181_fifo_run(pl181_state *s)
+static void pl181_transfer_start(pl181_state *s)
 {
-    uint32_t bits;
-    uint32_t value = 0;
-    int n;
-    int is_read;
-
-    is_read = (s->datactrl & PL181_DATA_DIRECTION) != 0;
-    if (s->datacnt != 0 && (!is_read || sd_data_ready(s->card))
-            && !s->linux_hack) {
-        if (is_read) {
-            n = 0;
-            while (s->datacnt && s->fifo_len < PL181_FIFO_LEN) {
-                value |= (uint32_t)sd_read_data(s->card) << (n * 8);
-                s->datacnt--;
-                n++;
-                if (n == 4) {
-                    pl181_fifo_push(s, value);
-                    n = 0;
-                    value = 0;
-                }
-            }
-            if (n != 0) {
-                pl181_fifo_push(s, value);
-            }
-        } else { /* write */
-            n = 0;
-            while (s->datacnt > 0 && (s->fifo_len > 0 || n > 0)) {
-                if (n == 0) {
-                    value = pl181_fifo_pop(s);
-                    n = 4;
-                }
-                n--;
-                s->datacnt--;
-                sd_write_data(s->card, value & 0xff);
-                value >>= 8;
-            }
-        }
-    }
+    s->datacnt = s->datalength;
+    s->fifo_pos = 0;
     s->status &= ~(PL181_STATUS_RX_FIFO | PL181_STATUS_TX_FIFO);
-    if (s->datacnt == 0) {
-        s->status |= PL181_STATUS_DATAEND;
-        /* HACK: */
-        s->status |= PL181_STATUS_DATABLOCKEND;
-        DPRINTF("Transfer Complete\n");
-    }
-    if (s->datacnt == 0 && s->fifo_len == 0) {
-        s->datactrl &= ~PL181_DATA_ENABLE;
-        DPRINTF("Data engine idle\n");
-    } else {
-        /* Update FIFO bits.  */
-        bits = PL181_STATUS_TXACTIVE | PL181_STATUS_RXACTIVE;
-        if (s->fifo_len == 0) {
-            bits |= PL181_STATUS_TXFIFOEMPTY;
-            bits |= PL181_STATUS_RXFIFOEMPTY;
-        } else {
-            bits |= PL181_STATUS_TXDATAAVLBL;
-            bits |= PL181_STATUS_RXDATAAVLBL;
-        }
-        if (s->fifo_len == 16) {
-            bits |= PL181_STATUS_TXFIFOFULL;
-            bits |= PL181_STATUS_RXFIFOFULL;
-        }
-        if (s->fifo_len <= 8) {
-            bits |= PL181_STATUS_TXFIFOHALFEMPTY;
-        }
-        if (s->fifo_len >= 8) {
-            bits |= PL181_STATUS_RXFIFOHALFFULL;
-        }
+
+    if (s->datacnt) {
         if (s->datactrl & PL181_DATA_DIRECTION) {
-            bits &= PL181_STATUS_RX_FIFO;
+            s->status |= PL181_STATUS_RXFIFOEMPTY | PL181_STATUS_RXACTIVE;
+            DPRINTF("Reading %u bytes (blksz=%u): waiting for start bit\n",
+                s->datacnt, pl181_get_blk_size(s));
         } else {
-            bits &= PL181_STATUS_TX_FIFO;
+            s->status |= PL181_STATUS_TXFIFOEMPTY |
+                PL181_STATUS_TXFIFOHALFEMPTY | PL181_STATUS_TXACTIVE;
+            DPRINTF("Writing %u bytes (blksize=%u)\n", s->datacnt,
+                pl181_get_blk_size(s));
         }
-        s->status |= bits;
+    } else {
+        s->status |= PL181_STATUS_DATAEND;
+        s->datactrl &= ~PL181_DATA_ENABLE;
     }
+
+    pl181_update(s);
 }
 
 static uint64_t pl181_read(void *opaque, hwaddr offset,
@@ -323,11 +346,11 @@ static uint64_t pl181_read(void *opaque, hwaddr offset,
         return s->datacnt;
     case 0x34: /* Status */
         tmp = s->status;
-        if (s->linux_hack) {
+        if (s->linux_hack == 2) {
             s->linux_hack = 0;
-            pl181_fifo_run(s);
-            pl181_update(s);
+            qemu_irq_raise(s->sbit_received);
         }
+        s->linux_hack = 0;
         return tmp;
     case 0x3c: /* Mask0 */
         return s->mask[0];
@@ -341,26 +364,22 @@ static uint64_t pl181_read(void *opaque, hwaddr offset,
            transferred between the serial engine and the card.
            We don't emulate this level of detail, so both can be the same.  */
         tmp = (s->datacnt + 3) >> 2;
-        if (s->linux_hack) {
+        if (s->linux_hack == 2) {
             s->linux_hack = 0;
-            pl181_fifo_run(s);
-            pl181_update(s);
+            qemu_irq_raise(s->sbit_received);
         }
+        s->linux_hack = 0;
         return tmp;
     case 0x80: case 0x84: case 0x88: case 0x8c: /* FifoData */
     case 0x90: case 0x94: case 0x98: case 0x9c:
     case 0xa0: case 0xa4: case 0xa8: case 0xac:
     case 0xb0: case 0xb4: case 0xb8: case 0xbc:
-        if (s->fifo_len == 0) {
+        if (s->fifo_pos == 0) {
             qemu_log_mask(LOG_GUEST_ERROR, "pl181: Unexpected FIFO read\n");
             return 0;
         } else {
-            uint32_t value;
-            value = pl181_fifo_pop(s);
             s->linux_hack = 1;
-            pl181_fifo_run(s);
-            pl181_update(s);
-            return value;
+            return pl181_fifo_pop(s);
         }
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
@@ -395,7 +414,6 @@ static void pl181_write(void *opaque, hwaddr offset,
                               "pl181: Pending commands not implemented\n");
             } else {
                 pl181_send_command(s);
-                pl181_fifo_run(s);
             }
             /* The command has completed one way or the other.  */
             s->cmd &= ~PL181_CMD_ENABLE;
@@ -410,8 +428,7 @@ static void pl181_write(void *opaque, hwaddr offset,
     case 0x2c: /* DataCtrl */
         s->datactrl = value & 0xff;
         if (value & PL181_DATA_ENABLE) {
-            s->datacnt = s->datalength;
-            pl181_fifo_run(s);
+            pl181_transfer_start(s);
         }
         break;
     case 0x38: /* Clear */
@@ -431,7 +448,6 @@ static void pl181_write(void *opaque, hwaddr offset,
             qemu_log_mask(LOG_GUEST_ERROR, "pl181: Unexpected FIFO write\n");
         } else {
             pl181_fifo_push(s, value);
-            pl181_fifo_run(s);
         }
         break;
     default:
@@ -447,6 +463,68 @@ static const MemoryRegionOps pl181_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void pl181_start_bit_received(void *opaque, int irq, int level)
+{
+    pl181_state *s = (pl181_state *)opaque;
+    unsigned int blksz = pl181_get_blk_size(s);
+
+    if (!(s->status & PL181_STATUS_RXACTIVE) || s->datacnt == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "pl181: unexpected start bit\n");
+        return;
+    }
+
+    if (s->linux_hack) {
+        s->linux_hack = 2;
+        return;
+    }
+
+    s->status &= ~(PL181_STATUS_RXFIFOHALFFULL | PL181_STATUS_RXFIFOFULL |
+        PL181_STATUS_RXFIFOEMPTY);
+    s->status |= PL181_STATUS_RXDATAAVLBL;
+    s->fifo_pos = MIN(blksz, PL181_FIFO_LEN);
+    s->fifo_pos = MIN(s->fifo_pos, s->datacnt);
+    s->datacnt -= s->fifo_pos;
+
+    if (s->fifo_pos == PL181_FIFO_LEN) {
+        s->status |= PL181_STATUS_RXFIFOFULL;
+    }
+
+    if (s->fifo_pos >= (PL181_FIFO_LEN / 2)) {
+        s->status |= PL181_STATUS_RXFIFOHALFFULL;
+    }
+
+    DPRINTF("Start bit received: datacnt=%u, fifo_pos=%u\n",
+        s->datacnt, s->fifo_pos);
+    pl181_update(s);
+}
+
+static void pl181_busy_deasserted(void *opaque, int irq, int level)
+{
+    pl181_state *s = (pl181_state *)opaque;
+
+    if (!(s->status & PL181_STATUS_TXACTIVE)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "pl181: unexpected busy end\n");
+        return;
+    }
+
+    s->datacnt -= s->fifo_pos;
+    s->fifo_pos = 0;
+
+    if (s->datacnt == 0) {
+        s->datactrl &= ~PL181_DATA_ENABLE;
+        s->status |= PL181_STATUS_DATAEND | PL181_STATUS_DATABLOCKEND;
+        s->status &= ~PL181_STATUS_TX_FIFO;
+        DPRINTF("Data engine idle\n");
+    } else {
+        s->status &= ~PL181_STATUS_TXDATAAVLBL;
+        s->status |= PL181_STATUS_TXFIFOEMPTY | PL181_STATUS_TXFIFOHALFEMPTY |
+            PL181_STATUS_DATABLOCKEND;
+    }
+
+    pl181_update(s);
+    DPRINTF("Busy deasserted: datacnt=%u\n", s->datacnt);
+}
+
 static void pl181_reset(DeviceState *d)
 {
     pl181_state *s = DO_UPCAST(pl181_state, busdev.qdev, d);
@@ -469,9 +547,11 @@ static void pl181_reset(DeviceState *d)
     s->linux_hack = 0;
     s->mask[0] = 0;
     s->mask[1] = 0;
+    s->fifo_pos = 0;
 
     /* We can assume our GPIO outputs have been wired up now */
-    sd_set_cb(s->card, s->cardstatus[0], s->cardstatus[1], NULL, NULL);
+    sd_set_cb(s->card, s->cardstatus[0], s->cardstatus[1],
+              s->sbit_received, s->busy_deasrt);
 }
 
 static int pl181_init(SysBusDevice *dev)
@@ -486,6 +566,8 @@ static int pl181_init(SysBusDevice *dev)
     qdev_init_gpio_out(&s->busdev.qdev, s->cardstatus, 2);
     dinfo = drive_get_next(IF_SD);
     s->card = sd_init(dinfo ? dinfo->bdrv : NULL, 0);
+    s->sbit_received = qemu_allocate_irqs(pl181_start_bit_received, s, 1)[0];
+    s->busy_deasrt = qemu_allocate_irqs(pl181_busy_deasserted, s, 1)[0];
     return 0;
 }
 
-- 
1.8.1.4

Re: [Qemu-devel] [RFC 5/7] sd.c: introduce async read operation

[Stefan Hajnoczi]

On Fri, May 10, 2013 at 08:10:23PM +0400, Igor Mitsyanko wrote:
> +    if (sd->state != sd_sendingdata_state) {
> +        DPRINTF("Transfer was aborted\n");
> +        return;
> +    }
> +
> +    io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;

If you respin, a constant would be nice instead of the 1 << 30 magic
number.

Re: [Qemu-devel] [RFC 0/7] Convert SD card model to AIO

[Stefan Hajnoczi]

On Fri, May 10, 2013 at 08:10:18PM +0400, Igor Mitsyanko wrote:
> This is an initial attempt to change our SD card model to use asynchronious
> input/output API instead of synchronious one. This will require converting of
> every user also. Right now I've converted only PL181 model, and I'll wait
> for some feedback on taken approach before I'll continue with other users.
> 
> New async SD interface is built on two callbacks:
> 1 Callback which models arrival of start bit on DAT line from card (start bit)
> 2 Callback modeling deasserting of DAT0 line by card when it has finished
> programming data (data busy end)
> 
> This is based on SD card specification.
> Start bit is issued for every new block of data coming from card.
> DAT0 line is kept low by card to signal busy state, while data block is being
> programmed by card and it can't receive new data.
> 
> Furthermore, I decided to drop data buffering on SD controller side. We already
> have two buffers in SD card model, no need to have another one in controller model.
> We can avoid intermediate copying from card's buffer to controller's buffer, and
> read/write directly from card.
> 
> Tested by running this Fedora image 
> https://fedoraproject.org/wiki/Architectures/ARM/F18/Versatile_Express
> on versatile board.
> 
> Igor Mitsyanko (7):
>   sd.c: introduce AIO related members in SD state
>   sd.c: introduce variable for trekking valid data
>   sd.c: introduce "start bit" and "busy deasserted" callbacks
>   sd.c: use callbacks as a flag to use async IO
>   sd.c: introduce async read operation
>   sd.c: introduce async write interface
>   pl181.c: convert to async IO SD card interface
> 
>  hw/sd/omap_mmc.c    |   6 +-
>  hw/sd/pl181.c       | 302 +++++++++++++++++++++++++++++++++-------------------
>  hw/sd/pxa2xx_mmci.c |   2 +-
>  hw/sd/sd.c          | 272 ++++++++++++++++++++++++++++++++++++++++++----
>  hw/sd/sdhci.c       |   4 +-
>  include/hw/sd.h     |   3 +-
>  6 files changed, 449 insertions(+), 140 deletions(-)

Thanks for doing this!  It makes me happy when we replace synchronous
block layer callers with asynchronous ones.

I didn't review each line but overall it seems like the right direction.

Stefan

Re: [Qemu-devel] [RFC 2/7] sd.c: introduce variable for trekking valid data

[Peter Maydell]

On 10 May 2013 17:10, Igor Mitsyanko <i.mitsyanko@gmail.com> wrote:
> Initialize it appropriately when various commands are processed.

"tracking", but the commit message doesn't match the
patch contents anyway -- should this patch have more
content from later patches, or be squashed into one of
them?

>
> Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
> ---
>  hw/sd/sd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 1dd1331..775a55c 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -116,6 +116,7 @@ struct SDState {
>      QEMUIOVector qiov;
>      struct iovec iov;
>      BlockDriverAIOCB *aiocb;
> +    uint32_t transf_cnt;

How does this work for migration -- are we guaranteed that
all outstanding AIO requests complete before we try to
migrate?

>  };
>
>  static void sd_set_mode(SDState *sd)
> --
> 1.8.1.4
>


thanks
-- PMM

Re: [Qemu-devel] [RFC 3/7] sd.c: introduce "start bit" and "busy deasserted" callbacks

[Peter Maydell]

On 10 May 2013 17:10, Igor Mitsyanko <i.mitsyanko@gmail.com> wrote:
> Start bit callback models arrival of first bit of data on card's DAT line.
>
> Busy deasserted callback models releasing of DAT0 line by card when it
> transitions from a programming state to a writing data state.
>
> Both of them will be used for async IO later.

This patch could really use some documentation comments that
clarify the interface between the SD controller and the card
model, so people writing SD controllers know what they have
to implement and what the expected semantics of the callbacks
are.

thanks
-- PMM

Re: [Qemu-devel] [RFC 7/7] pl181.c: convert to async IO SD card interface

[Peter Maydell]

On 10 May 2013 17:10, Igor Mitsyanko <i.mitsyanko@gmail.com> wrote:
> Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>

(not a complete review, just a couple of things I noticed on a
first pass.)

> ---
>  hw/sd/pl181.c | 302 +++++++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 192 insertions(+), 110 deletions(-)
>
> diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
> index 2caacc2..a8a3510 100644
> --- a/hw/sd/pl181.c
> +++ b/hw/sd/pl181.c
> @@ -20,7 +20,7 @@ do { printf("pl181: " fmt , ## __VA_ARGS__); } while (0)
>  #define DPRINTF(fmt, ...) do {} while(0)
>  #endif
>
> -#define PL181_FIFO_LEN 16
> +#define PL181_FIFO_LEN 64
>
>  typedef struct {
>      SysBusDevice busdev;
> @@ -39,23 +39,23 @@ typedef struct {
>      uint32_t status;
>      uint32_t mask[2];
>      int32_t fifo_pos;
> -    int32_t fifo_len;
>      /* The linux 2.6.21 driver is buggy, and misbehaves if new data arrives
>         while it is reading the FIFO.  We hack around this be defering
>         subsequent transfers until after the driver polls the status word.
>         http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4446/1
>       */
>      int32_t linux_hack;
> -    uint32_t fifo[PL181_FIFO_LEN];

This looks really odd. Why has the fifo gone away?

>      qemu_irq irq[2];
>      /* GPIO outputs for 'card is readonly' and 'card inserted' */
>      qemu_irq cardstatus[2];
> +    qemu_irq sbit_received;
> +    qemu_irq busy_deasrt;

No real need to abbreviate here, just call it 'busy_deassert'.

>  } pl181_state;

> @@ -395,7 +414,6 @@ static void pl181_write(void *opaque, hwaddr offset,
>                                "pl181: Pending commands not implemented\n");
>              } else {
>                  pl181_send_command(s);
> -                pl181_fifo_run(s);
>              }
>              /* The command has completed one way or the other.  */
>              s->cmd &= ~PL181_CMD_ENABLE;

This doesn't look right. As the comment notes, previously we could
assume (because we were strictly synchronous) that at this point the
command had completed and move the command state machine straight
back to idle. However now things are asynchronous this isn't true
and you need extra logic to change the cmd enabled bit only when we've
actually finished whatever it was we were doing.

thanks
-- PMM

Re: [Qemu-devel] [RFC 6/7] sd.c: introduce async write interface

[Peter Maydell]

On 10 May 2013 17:10, Igor Mitsyanko <i.mitsyanko@gmail.com> wrote:
> Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
> ---
>  hw/sd/sd.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 133 insertions(+), 12 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 659ec56..615ab61 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -958,6 +958,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
>              sd->state = sd_programming_state;
>              /* Bzzzzzzztt .... Operation complete.  */
>              sd->state = sd_transfer_state;
> +            sd->transf_cnt = 0;
> +            if (sd->aiocb) {
> +                bdrv_aio_cancel(sd->aiocb);
> +                sd->aiocb = NULL;
> +            }
>              return sd_r1b;
>
>          default:
> @@ -1600,6 +1605,90 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
>  #define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len)
>  #define APP_WRITE_BLOCK(a, len)
>
> +static void sd_write_done(void *opaque, int ret)
> +{
> +    SDState *sd = opaque;
> +
> +    DPRINTF("sd_write_done: ret = %d\n", ret);
> +    sd->aiocb = NULL;
> +
> +    if (ret != 0) {
> +        return;
> +    }
> +
> +    switch (sd->current_cmd) {
> +    case 24:    /* CMD24:  WRITE_SINGLE_BLOCK */
> +        sd->blk_written++;
> +        sd->csd[14] |= 0x40;
> +        /* Bzzzzzzztt .... Operation complete.  */

These 'bzzzzt' comments don't really apply to the async
interface, I think -- they're a kind of joke about the
fact that some of these commands which take a long time
on hardware are (were) completely synchronous in the
QEMU model and so the 'operation' is indicated as taking
place via the 'bzzzt' sound effect.

-- PMM

Re: [Qemu-devel] [RFC 2/7] sd.c: introduce variable for trekking valid data

[Stefan Hajnoczi]

On Fri, Jun 14, 2013 at 12:49:52PM +0100, Peter Maydell wrote:
> On 10 May 2013 17:10, Igor Mitsyanko <i.mitsyanko@gmail.com> wrote:
> > Initialize it appropriately when various commands are processed.
> 
> "tracking", but the commit message doesn't match the
> patch contents anyway -- should this patch have more
> content from later patches, or be squashed into one of
> them?
> 
> >
> > Signed-off-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
> > ---
> >  hw/sd/sd.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> > index 1dd1331..775a55c 100644
> > --- a/hw/sd/sd.c
> > +++ b/hw/sd/sd.c
> > @@ -116,6 +116,7 @@ struct SDState {
> >      QEMUIOVector qiov;
> >      struct iovec iov;
> >      BlockDriverAIOCB *aiocb;
> > +    uint32_t transf_cnt;
> 
> How does this work for migration -- are we guaranteed that
> all outstanding AIO requests complete before we try to
> migrate?

Migration does bdrv_drain_all() to complete all pending requests
(indirectly by pausing the guest in do_vm_stop()).

Stefan

Re: [Qemu-devel] [RFC 2/7] sd.c: introduce variable for trekking valid data

[Paolo Bonzini]

Il 17/06/2013 14:30, Stefan Hajnoczi ha scritto:
>>> > > @@ -116,6 +116,7 @@ struct SDState {
>>> > >      QEMUIOVector qiov;
>>> > >      struct iovec iov;
>>> > >      BlockDriverAIOCB *aiocb;
>>> > > +    uint32_t transf_cnt;
>> > 
>> > How does this work for migration -- are we guaranteed that
>> > all outstanding AIO requests complete before we try to
>> > migrate?
> Migration does bdrv_drain_all() to complete all pending requests
> (indirectly by pausing the guest in do_vm_stop()).

... and sd.c AFAIR doesn't support rerror/werror so it doesn't have to
migrate pending requests.

Paolo