[Qemu-devel] [PATCH 0/3] ide: add TRIM support

[Qemu-devel] [PATCH 0/3] ide: add TRIM support

[Christoph Hellwig]

This patchset resurrects my older patches to add TRIM support.  The
old approach to pass an action handler through the IDE subsystem doesn't
work any more after the refactoring for the AHCI driver, so not we simple
switch on the type of command passed in.

[Qemu-devel] [PATCH 1/3] make dma_bdrv_io available to drivers

[Christoph Hellwig]

Make dma_bdrv_io available for drivers, and pass an explicit I/O function
instead of hardcoding bdrv_aio_readv/bdrv_aio_writev.  This is required
to implement non-READ/WRITE dma commands in the ide driver, e.g. the
upcoming TRIM support.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: qemu/dma-helpers.c
===================================================================
--- qemu.orig/dma-helpers.c	2010-12-16 17:40:25.422004054 +0100
+++ qemu/dma-helpers.c	2011-05-03 11:12:36.340387288 +0200
@@ -47,6 +47,7 @@ typedef struct {
     target_phys_addr_t sg_cur_byte;
     QEMUIOVector iov;
     QEMUBH *bh;
+    DMAIOFunc *io_func;
 } DMAAIOCB;
 
 static void dma_bdrv_cb(void *opaque, int ret);
@@ -116,13 +117,8 @@ static void dma_bdrv_cb(void *opaque, in
         return;
     }
 
-    if (dbs->is_write) {
-        dbs->acb = bdrv_aio_writev(dbs->bs, dbs->sector_num, &dbs->iov,
-                                   dbs->iov.size / 512, dma_bdrv_cb, dbs);
-    } else {
-        dbs->acb = bdrv_aio_readv(dbs->bs, dbs->sector_num, &dbs->iov,
-                                  dbs->iov.size / 512, dma_bdrv_cb, dbs);
-    }
+    dbs->acb = dbs->io_func(dbs->bs, dbs->sector_num, &dbs->iov,
+                            dbs->iov.size / 512, dma_bdrv_cb, dbs);
     if (!dbs->acb) {
         dma_bdrv_unmap(dbs);
         qemu_iovec_destroy(&dbs->iov);
@@ -144,12 +140,12 @@ static AIOPool dma_aio_pool = {
     .cancel             = dma_aio_cancel,
 };
 
-static BlockDriverAIOCB *dma_bdrv_io(
+BlockDriverAIOCB *dma_bdrv_io(
     BlockDriverState *bs, QEMUSGList *sg, uint64_t sector_num,
-    BlockDriverCompletionFunc *cb, void *opaque,
-    int is_write)
+    DMAIOFunc *io_func, BlockDriverCompletionFunc *cb,
+    void *opaque, int is_write)
 {
-    DMAAIOCB *dbs =  qemu_aio_get(&dma_aio_pool, bs, cb, opaque);
+    DMAAIOCB *dbs = qemu_aio_get(&dma_aio_pool, bs, cb, opaque);
 
     dbs->acb = NULL;
     dbs->bs = bs;
@@ -158,6 +154,7 @@ static BlockDriverAIOCB *dma_bdrv_io(
     dbs->sg_cur_index = 0;
     dbs->sg_cur_byte = 0;
     dbs->is_write = is_write;
+    dbs->io_func = io_func;
     dbs->bh = NULL;
     qemu_iovec_init(&dbs->iov, sg->nsg);
     dma_bdrv_cb(dbs, 0);
@@ -173,12 +170,12 @@ BlockDriverAIOCB *dma_bdrv_read(BlockDri
                                 QEMUSGList *sg, uint64_t sector,
                                 void (*cb)(void *opaque, int ret), void *opaque)
 {
-    return dma_bdrv_io(bs, sg, sector, cb, opaque, 0);
+    return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque, 0);
 }
 
 BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs,
                                  QEMUSGList *sg, uint64_t sector,
                                  void (*cb)(void *opaque, int ret), void *opaque)
 {
-    return dma_bdrv_io(bs, sg, sector, cb, opaque, 1);
+    return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, 1);
 }
Index: qemu/dma.h
===================================================================
--- qemu.orig/dma.h	2010-12-16 17:40:25.431007267 +0100
+++ qemu/dma.h	2011-05-03 11:12:36.343720604 +0200
@@ -32,6 +32,14 @@ void qemu_sglist_add(QEMUSGList *qsg, ta
                      target_phys_addr_t len);
 void qemu_sglist_destroy(QEMUSGList *qsg);
 
+typedef BlockDriverAIOCB *DMAIOFunc(BlockDriverState *bs, int64_t sector_num,
+                                 QEMUIOVector *iov, int nb_sectors,
+                                 BlockDriverCompletionFunc *cb, void *opaque);
+
+BlockDriverAIOCB *dma_bdrv_io(BlockDriverState *bs,
+                              QEMUSGList *sg, uint64_t sector_num,
+                              DMAIOFunc *io_func, BlockDriverCompletionFunc *cb,
+                              void *opaque, int is_write);
 BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
                                 QEMUSGList *sg, uint64_t sector,
                                 BlockDriverCompletionFunc *cb, void *opaque);

[Qemu-devel] [PATCH 2/3] ide: allow other dma comands than read and write

[Christoph Hellwig]

Replace the is_read flag with a dma_cmd flag to allow the dma and
restart logic to handler other commands like TRIM.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: qemu/hw/ide/core.c
===================================================================
--- qemu.orig/hw/ide/core.c	2011-05-03 11:32:14.727336751 +0200
+++ qemu/hw/ide/core.c	2011-05-03 11:35:56.259469943 +0200
@@ -473,7 +473,7 @@ handle_rw_error:
     if (ret < 0) {
         int op = BM_STATUS_DMA_RETRY;
 
-        if (s->is_read)
+        if (s->dma_cmd == IDE_DMA_READ)
             op |= BM_STATUS_RETRY_READ;
         if (ide_handle_rw_error(s, -ret, op)) {
             return;
@@ -483,7 +483,7 @@ handle_rw_error:
     n = s->io_buffer_size >> 9;
     sector_num = ide_get_sector(s);
     if (n > 0) {
-        dma_buf_commit(s, s->is_read);
+        dma_buf_commit(s, ide_cmd_is_read(s));
         sector_num += n;
         ide_set_sector(s, sector_num);
         s->nsector -= n;
@@ -500,20 +500,23 @@ handle_rw_error:
     n = s->nsector;
     s->io_buffer_index = 0;
     s->io_buffer_size = n * 512;
-    if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->is_read) == 0)
+    if (s->bus->dma->ops->prepare_buf(s->bus->dma, ide_cmd_is_read(s)) == 0)
         goto eot;
 
 #ifdef DEBUG_AIO
-    printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, is_read=%d\n",
-           sector_num, n, s->is_read);
+    printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, cmd_cmd=%d\n",
+           sector_num, n, s->dma_cmd);
 #endif
 
-    if (s->is_read) {
+    switch (s->dma_cmd) {
+    case IDE_DMA_READ:
         s->bus->dma->aiocb = dma_bdrv_read(s->bs, &s->sg, sector_num,
                                            ide_dma_cb, s);
-    } else {
+        break;
+    case IDE_DMA_WRITE:
         s->bus->dma->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
                                             ide_dma_cb, s);
+        break;
     }
 
     if (!s->bus->dma->aiocb) {
@@ -527,12 +530,12 @@ eot:
    ide_set_inactive(s);
 }
 
-static void ide_sector_start_dma(IDEState *s, int is_read)
+static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 {
     s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
     s->io_buffer_index = 0;
     s->io_buffer_size = 0;
-    s->is_read = is_read;
+    s->dma_cmd = dma_cmd;
     s->bus->dma->ops->start_dma(s->bus->dma, s, ide_dma_cb);
 }
 
@@ -915,7 +918,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t
         if (!s->bs)
             goto abort_cmd;
 	ide_cmd_lba48_transform(s, lba48);
-        ide_sector_start_dma(s, 1);
+        ide_sector_start_dma(s, IDE_DMA_READ);
         break;
 	case WIN_WRITEDMA_EXT:
 	lba48 = 1;
@@ -924,7 +927,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t
         if (!s->bs)
             goto abort_cmd;
 	ide_cmd_lba48_transform(s, lba48);
-        ide_sector_start_dma(s, 0);
+        ide_sector_start_dma(s, IDE_DMA_WRITE);
         s->media_changed = 1;
         break;
     case WIN_READ_NATIVE_MAX_EXT:
Index: qemu/hw/ide/internal.h
===================================================================
--- qemu.orig/hw/ide/internal.h	2011-05-03 11:32:13.884007986 +0200
+++ qemu/hw/ide/internal.h	2011-05-03 11:36:05.526086407 +0200
@@ -379,6 +379,14 @@ struct unreported_events {
     bool new_media;
 };
 
+enum ide_dma_cmd {
+    IDE_DMA_READ,
+    IDE_DMA_WRITE,
+};
+
+#define ide_cmd_is_read(s) \
+	((s)->dma_cmd == IDE_DMA_READ)
+
 /* NOTE: IDEState represents in fact one drive */
 struct IDEState {
     IDEBus *bus;
@@ -446,7 +454,7 @@ struct IDEState {
     uint32_t mdata_size;
     uint8_t *mdata_storage;
     int media_changed;
-    int is_read;
+    enum ide_dma_cmd dma_cmd;
     /* SMART */
     uint8_t smart_enabled;
     uint8_t smart_autosave;
Index: qemu/hw/ide/macio.c
===================================================================
--- qemu.orig/hw/ide/macio.c	2011-05-03 11:32:14.740670013 +0200
+++ qemu/hw/ide/macio.c	2011-05-03 11:35:56.302803041 +0200
@@ -145,12 +145,17 @@ static void pmac_ide_transfer_cb(void *o
     io->addr += io->len;
     io->len = 0;
 
-    if (s->is_read)
+    switch (s->dma_cmd) {
+    case IDE_DMA_READ:
         m->aiocb = dma_bdrv_read(s->bs, &s->sg, sector_num,
 		                 pmac_ide_transfer_cb, io);
-    else
+        break;
+    case IDE_DMA_WRITE:
         m->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
 		                  pmac_ide_transfer_cb, io);
+        break;
+    }
+
     if (!m->aiocb)
         pmac_ide_transfer_cb(io, -1);
 }
Index: qemu/hw/ide/pci.c
===================================================================
--- qemu.orig/hw/ide/pci.c	2011-05-03 11:32:13.854008149 +0200
+++ qemu/hw/ide/pci.c	2011-05-03 11:32:27.877265512 +0200
@@ -169,7 +169,7 @@ static int bmdma_set_inactive(IDEDMA *dm
     return 0;
 }
 
-static void bmdma_restart_dma(BMDMAState *bm, int is_read)
+static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd)
 {
     IDEState *s = bmdma_active_if(bm);
 
@@ -177,7 +177,7 @@ static void bmdma_restart_dma(BMDMAState
     s->io_buffer_index = 0;
     s->io_buffer_size = 0;
     s->nsector = bm->nsector;
-    s->is_read = is_read;
+    s->dma_cmd = dma_cmd;
     bm->cur_addr = bm->addr;
     bm->dma_cb = ide_dma_cb;
     bmdma_start_dma(&bm->dma, s, bm->dma_cb);
@@ -195,7 +195,7 @@ static void bmdma_restart_bh(void *opaqu
 
     if (bm->status & BM_STATUS_DMA_RETRY) {
         bm->status &= ~(BM_STATUS_DMA_RETRY | BM_STATUS_RETRY_READ);
-        bmdma_restart_dma(bm, is_read);
+        bmdma_restart_dma(bm, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
     } else if (bm->status & BM_STATUS_PIO_RETRY) {
         bm->status &= ~(BM_STATUS_PIO_RETRY | BM_STATUS_RETRY_READ);
         if (is_read) {

[Qemu-devel] [PATCH 3/3] ide: add TRIM support

[Christoph Hellwig]

Add support for TRIM sub function of the data set management command,
and wire it up to the qemu discard infrastructure.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: qemu/hw/ide/core.c
===================================================================
--- qemu.orig/hw/ide/core.c	2011-05-03 11:35:56.259469943 +0200
+++ qemu/hw/ide/core.c	2011-05-03 11:39:38.458266188 +0200
@@ -124,6 +124,9 @@ static void ide_identify(IDEState *s)
     put_le16(p + 66, 120);
     put_le16(p + 67, 120);
     put_le16(p + 68, 120);
+    if (dev && dev->conf.discard_granularity) {
+        put_le16(p + 69, (1 << 14)); /* determinate TRIM behavior */
+    }
 
     if (s->ncq_queues) {
         put_le16(p + 75, s->ncq_queues - 1);
@@ -157,6 +160,9 @@ static void ide_identify(IDEState *s)
     dev = s->unit ? s->bus->slave : s->bus->master;
     if (dev && dev->conf.physical_block_size)
         put_le16(p + 106, 0x6000 | get_physical_block_exp(&dev->conf));
+    if (dev && dev->conf.discard_granularity) {
+        put_le16(p + 169, 1); /* TRIM support */
+    }
 
     memcpy(s->identify_data, p, sizeof(s->identify_data));
     s->identify_set = 1;
@@ -299,6 +305,72 @@ static void ide_set_signature(IDEState *
     }
 }
 
+typedef struct TrimAIOCB {
+    BlockDriverAIOCB common;
+    QEMUBH *bh;
+    int ret;
+} TrimAIOCB;
+
+static void trim_aio_cancel(BlockDriverAIOCB *acb)
+{
+    TrimAIOCB *iocb = container_of(acb, TrimAIOCB, common);
+
+    qemu_bh_delete(iocb->bh);
+    iocb->bh = NULL;
+    qemu_aio_release(iocb);
+}
+
+static AIOPool trim_aio_pool = {
+    .aiocb_size         = sizeof(TrimAIOCB),
+    .cancel             = trim_aio_cancel,
+};
+
+static void ide_trim_bh_cb(void *opaque)
+{
+    TrimAIOCB *iocb = opaque;
+
+    iocb->common.cb(iocb->common.opaque, iocb->ret);
+
+    qemu_bh_delete(iocb->bh);
+    iocb->bh = NULL;
+
+    qemu_aio_release(iocb);
+}
+
+BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
+        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    TrimAIOCB *iocb;
+    int i, j, ret;
+
+    iocb = qemu_aio_get(&trim_aio_pool, bs, cb, opaque);
+    iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb);
+    iocb->ret = 0;
+
+    for (j = 0; j < qiov->niov; j++) {
+        uint64_t *buffer = qiov->iov[j].iov_base;
+
+        for (i = 0; i < qiov->iov[j].iov_len / 8; i++) {
+            /* 6-byte LBA + 2-byte range per entry */
+            uint64_t entry = le64_to_cpu(buffer[i]);
+            uint64_t sector = entry & 0x0000ffffffffffffULL;
+            uint16_t count = entry >> 48;
+
+            if (count == 0)
+                break;
+
+            ret = bdrv_discard(bs, sector * 512, count * 512);
+            if (!iocb->ret)
+                iocb->ret = ret;
+        }
+    }
+
+    qemu_bh_schedule(iocb->bh);
+
+    return &iocb->common;
+}
+
 static inline void ide_abort_command(IDEState *s)
 {
     s->status = READY_STAT | ERR_STAT;
@@ -517,6 +589,10 @@ handle_rw_error:
         s->bus->dma->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
                                             ide_dma_cb, s);
         break;
+    case IDE_DMA_TRIM:
+        s->bus->dma->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
+                                         ide_issue_trim, ide_dma_cb, s, 1);
+        break;
     }
 
     if (!s->bus->dma->aiocb) {
@@ -817,6 +893,17 @@ void ide_exec_cmd(IDEBus *bus, uint32_t
         return;
 
     switch(val) {
+    case WIN_DSM:
+        switch (s->feature) {
+        case DSM_TRIM:
+            if (!s->bs)
+                goto abort_cmd;
+            ide_sector_start_dma(s, IDE_DMA_TRIM);
+            break;
+        default:
+            goto abort_cmd;
+        }
+        break;
     case WIN_IDENTIFY:
         if (s->bs && s->drive_kind != IDE_CD) {
             if (s->drive_kind != IDE_CFATA)
Index: qemu/hw/ide/internal.h
===================================================================
--- qemu.orig/hw/ide/internal.h	2011-05-03 11:36:05.526086407 +0200
+++ qemu/hw/ide/internal.h	2011-05-03 11:39:50.264868893 +0200
@@ -62,7 +62,11 @@ typedef struct IDEDMAOps IDEDMAOps;
  */
 #define CFA_REQ_EXT_ERROR_CODE		0x03 /* CFA Request Extended Error Code */
 /*
- *	0x04->0x07 Reserved
+ *      0x04->0x05 Reserved
+ */
+#define WIN_DSM                         0x06
+/*
+ *      0x07 Reserved
  */
 #define WIN_SRST			0x08 /* ATAPI soft reset command */
 #define WIN_DEVICE_RESET		0x08
@@ -190,6 +194,9 @@ typedef struct IDEDMAOps IDEDMAOps;
 
 #define IDE_DMA_BUF_SECTORS 256
 
+/* feature values for Data Set Management */
+#define DSM_TRIM                        0x01
+
 #if (IDE_DMA_BUF_SECTORS < MAX_MULT_SECTORS)
 #error "IDE_DMA_BUF_SECTORS must be bigger or equal to MAX_MULT_SECTORS"
 #endif
@@ -382,6 +389,7 @@ struct unreported_events {
 enum ide_dma_cmd {
     IDE_DMA_READ,
     IDE_DMA_WRITE,
+    IDE_DMA_TRIM,
 };
 
 #define ide_cmd_is_read(s) \
@@ -583,6 +591,9 @@ void ide_transfer_start(IDEState *s, uin
                         EndTransferFunc *end_transfer_func);
 void ide_transfer_stop(IDEState *s);
 void ide_set_inactive(IDEState *s);
+BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
+        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque);
 
 /* hw/ide/atapi.c */
 void ide_atapi_cmd(IDEState *s);
Index: qemu/hw/ide/qdev.c
===================================================================
--- qemu.orig/hw/ide/qdev.c	2011-05-03 11:35:56.286136465 +0200
+++ qemu/hw/ide/qdev.c	2011-05-03 11:36:08.916068043 +0200
@@ -125,6 +125,11 @@ static int ide_drive_initfn(IDEDevice *d
     const char *serial;
     DriveInfo *dinfo;
 
+    if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) {
+        error_report("discard_granularity must be 512 for ide");
+        return -1;
+    }
+
     serial = dev->serial;
     if (!serial) {
         /* try to fall back to value set with legacy -drive serial=... */
Index: qemu/hw/ide/macio.c
===================================================================
--- qemu.orig/hw/ide/macio.c	2011-05-03 11:35:56.302803041 +0200
+++ qemu/hw/ide/macio.c	2011-05-03 11:39:19.788367333 +0200
@@ -154,6 +154,10 @@ static void pmac_ide_transfer_cb(void *o
         m->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
 		                  pmac_ide_transfer_cb, io);
         break;
+    case IDE_DMA_TRIM:
+        m->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
+                               ide_issue_trim,, pmac_ide_transfer_cb, s, 1);
+        break;
     }
 
     if (!m->aiocb)

Re: [Qemu-devel] [PATCH 3/3] ide: add TRIM support

[Paolo Bonzini]

On 05/03/2011 02:06 PM, Christoph Hellwig wrote:
> +    case IDE_DMA_TRIM:
> +        m->aiocb = dma_bdrv_io(s->bs,&s->sg, sector_num,
> +                               ide_issue_trim,, pmac_ide_transfer_cb, s, 1);
> +        break;

Not compile-tested?

Paolo

Re: [Qemu-devel] [PATCH 3/3] ide: add TRIM support

[Christoph Hellwig]

On Tue, May 03, 2011 at 03:08:43PM +0200, Paolo Bonzini wrote:
> On 05/03/2011 02:06 PM, Christoph Hellwig wrote:
>> +    case IDE_DMA_TRIM:
>> +        m->aiocb = dma_bdrv_io(s->bs,&s->sg, sector_num,
>> +                               ide_issue_trim,, pmac_ide_transfer_cb, s, 1);
>> +        break;
>
> Not compile-tested?

pmac emulation indeed wasn't in my usual compile setup.  Removing the comma
makes pmac compile, but I still can't finish a "full" qemu build:

hch@rocky:~/work/qemu$ make
  CC    arm-softmmu/gdbstub-xml.o
 cc1: warnings being treated as errors
 In file included from ../qemu-common.h:5:0,
                  from gdbstub-xml.c:2:
../config-host.h:2:0: error: "CONFIG_QEMU_PREFIX" redefined
config.h:2:0: note: this is the location of the previous definition
In file included from gdbstub-xml.c:140:0:
config.h:2:0: error: "CONFIG_QEMU_PREFIX" redefined
../config-host.h:2:0: note: this is the location of the previous definition
make[1]: *** [gdbstub-xml.o] Error 1
make: *** [subdir-arm-softmmu] Error 2

Re: [Qemu-devel] [PATCH 3/3] ide: add TRIM support

[Kevin Wolf]

Am 03.05.2011 14:06, schrieb Christoph Hellwig:
> Add support for TRIM sub function of the data set management command,
> and wire it up to the qemu discard infrastructure.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Don't failed trim requests turn into writes now?

Also, there are some braces missing.

Kevin

[Qemu-devel] [PATCH 2/3] ide: allow other dma comands than read and write

[Christoph Hellwig]

Replace the is_read flag with a dma_cmd flag to allow the dma and
restart logic to handler other commands like TRIM.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: qemu/hw/ide/core.c
===================================================================
--- qemu.orig/hw/ide/core.c	2011-05-18 20:24:04.318125593 +0200
+++ qemu/hw/ide/core.c	2011-05-18 20:30:20.855172933 +0200
@@ -473,7 +473,7 @@ handle_rw_error:
     if (ret < 0) {
         int op = BM_STATUS_DMA_RETRY;
 
-        if (s->is_read)
+        if (s->dma_cmd == IDE_DMA_READ)
             op |= BM_STATUS_RETRY_READ;
         if (ide_handle_rw_error(s, -ret, op)) {
             return;
@@ -483,7 +483,7 @@ handle_rw_error:
     n = s->io_buffer_size >> 9;
     sector_num = ide_get_sector(s);
     if (n > 0) {
-        dma_buf_commit(s, s->is_read);
+        dma_buf_commit(s, ide_cmd_is_read(s));
         sector_num += n;
         ide_set_sector(s, sector_num);
         s->nsector -= n;
@@ -500,20 +500,23 @@ handle_rw_error:
     n = s->nsector;
     s->io_buffer_index = 0;
     s->io_buffer_size = n * 512;
-    if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->is_read) == 0)
+    if (s->bus->dma->ops->prepare_buf(s->bus->dma, ide_cmd_is_read(s)) == 0)
         goto eot;
 
 #ifdef DEBUG_AIO
-    printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, is_read=%d\n",
-           sector_num, n, s->is_read);
+    printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, cmd_cmd=%d\n",
+           sector_num, n, s->dma_cmd);
 #endif
 
-    if (s->is_read) {
+    switch (s->dma_cmd) {
+    case IDE_DMA_READ:
         s->bus->dma->aiocb = dma_bdrv_read(s->bs, &s->sg, sector_num,
                                            ide_dma_cb, s);
-    } else {
+        break;
+    case IDE_DMA_WRITE:
         s->bus->dma->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
                                             ide_dma_cb, s);
+        break;
     }
 
     if (!s->bus->dma->aiocb) {
@@ -527,12 +530,12 @@ eot:
    ide_set_inactive(s);
 }
 
-static void ide_sector_start_dma(IDEState *s, int is_read)
+static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 {
     s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
     s->io_buffer_index = 0;
     s->io_buffer_size = 0;
-    s->is_read = is_read;
+    s->dma_cmd = dma_cmd;
     s->bus->dma->ops->start_dma(s->bus->dma, s, ide_dma_cb);
 }
 
@@ -915,7 +918,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t
         if (!s->bs)
             goto abort_cmd;
 	ide_cmd_lba48_transform(s, lba48);
-        ide_sector_start_dma(s, 1);
+        ide_sector_start_dma(s, IDE_DMA_READ);
         break;
 	case WIN_WRITEDMA_EXT:
 	lba48 = 1;
@@ -924,7 +927,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t
         if (!s->bs)
             goto abort_cmd;
 	ide_cmd_lba48_transform(s, lba48);
-        ide_sector_start_dma(s, 0);
+        ide_sector_start_dma(s, IDE_DMA_WRITE);
         s->media_changed = 1;
         break;
     case WIN_READ_NATIVE_MAX_EXT:
Index: qemu/hw/ide/internal.h
===================================================================
--- qemu.orig/hw/ide/internal.h	2011-05-18 20:24:04.334126497 +0200
+++ qemu/hw/ide/internal.h	2011-05-18 20:28:17.133626100 +0200
@@ -379,6 +379,14 @@ struct unreported_events {
     bool new_media;
 };
 
+enum ide_dma_cmd {
+    IDE_DMA_READ,
+    IDE_DMA_WRITE,
+};
+
+#define ide_cmd_is_read(s) \
+	((s)->dma_cmd == IDE_DMA_READ)
+
 /* NOTE: IDEState represents in fact one drive */
 struct IDEState {
     IDEBus *bus;
@@ -446,7 +454,7 @@ struct IDEState {
     uint32_t mdata_size;
     uint8_t *mdata_storage;
     int media_changed;
-    int is_read;
+    enum ide_dma_cmd dma_cmd;
     /* SMART */
     uint8_t smart_enabled;
     uint8_t smart_autosave;
Index: qemu/hw/ide/macio.c
===================================================================
--- qemu.orig/hw/ide/macio.c	2011-05-18 20:24:04.346126902 +0200
+++ qemu/hw/ide/macio.c	2011-05-18 20:28:17.145627191 +0200
@@ -145,12 +145,17 @@ static void pmac_ide_transfer_cb(void *o
     io->addr += io->len;
     io->len = 0;
 
-    if (s->is_read)
+    switch (s->dma_cmd) {
+    case IDE_DMA_READ:
         m->aiocb = dma_bdrv_read(s->bs, &s->sg, sector_num,
 		                 pmac_ide_transfer_cb, io);
-    else
+        break;
+    case IDE_DMA_WRITE:
         m->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
 		                  pmac_ide_transfer_cb, io);
+        break;
+    }
+
     if (!m->aiocb)
         pmac_ide_transfer_cb(io, -1);
 }
Index: qemu/hw/ide/pci.c
===================================================================
--- qemu.orig/hw/ide/pci.c	2011-05-18 20:24:04.362126865 +0200
+++ qemu/hw/ide/pci.c	2011-05-18 20:28:17.153625872 +0200
@@ -169,7 +169,7 @@ static int bmdma_set_inactive(IDEDMA *dm
     return 0;
 }
 
-static void bmdma_restart_dma(BMDMAState *bm, int is_read)
+static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd)
 {
     IDEState *s = bmdma_active_if(bm);
 
@@ -177,7 +177,7 @@ static void bmdma_restart_dma(BMDMAState
     s->io_buffer_index = 0;
     s->io_buffer_size = 0;
     s->nsector = bm->nsector;
-    s->is_read = is_read;
+    s->dma_cmd = dma_cmd;
     bm->cur_addr = bm->addr;
     bm->dma_cb = ide_dma_cb;
     bmdma_start_dma(&bm->dma, s, bm->dma_cb);
@@ -195,7 +195,7 @@ static void bmdma_restart_bh(void *opaqu
 
     if (bm->status & BM_STATUS_DMA_RETRY) {
         bm->status &= ~(BM_STATUS_DMA_RETRY | BM_STATUS_RETRY_READ);
-        bmdma_restart_dma(bm, is_read);
+        bmdma_restart_dma(bm, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
     } else if (bm->status & BM_STATUS_PIO_RETRY) {
         bm->status &= ~(BM_STATUS_PIO_RETRY | BM_STATUS_RETRY_READ);
         if (is_read) {