[Qemu-devel] [PULL 00/12] Ide patches

[Qemu-devel] [PULL 00/12] Ide patches

[John Snow]

The following changes since commit 42d58e7c6760cb9c55627c28ae538e27dcf2f144:

  Merge remote-tracking branch 'remotes/sstabellini/tags/xen-15-06-02-tag' into staging (2015-06-02 16:47:31 +0100)

are available in the git repository at:

  https://github.com/jnsnow/qemu.git tags/ide-pull-request

for you to fetch changes up to 0ba98885a0e965a17df214ab12b819ef630d8a14:

  macio: remove remainder_len DBDMA_io property (2015-06-04 20:25:39 -0400)

----------------------------------------------------------------

----------------------------------------------------------------

Kevin Wolf (8):
  fdc: Rename fdctrl_reset_fifo() to fdctrl_to_command_phase()
  fdc: Rename fdctrl_set_fifo() to fdctrl_to_result_phase()
  fdc: Introduce fdctrl->phase
  fdc: Use phase in fdctrl_write_data()
  fdc: Code cleanup in fdctrl_write_data()
  fdc: Disentangle phases in fdctrl_read_data()
  fdc: Fix MSR.RQM flag
  fdc-test: Test state for existing cases more thoroughly

Mark Cave-Ayland (4):
  macio: switch pmac_dma_read() over to new offset/len implementation
  macio: switch pmac_dma_write() over to new offset/len implementation
  macio: update comment/constants to reflect the new code
  macio: remove remainder_len DBDMA_io property

 hw/block/fdc.c             | 296 +++++++++++++++++++++++++++++++++------------
 hw/ide/macio.c             | 279 ++++++++++++++++--------------------------
 include/hw/ppc/mac_dbdma.h |   4 +-
 tests/fdc-test.c           |  34 ++++++
 4 files changed, 362 insertions(+), 251 deletions(-)

-- 
2.1.0

[Qemu-devel] [PULL 01/12] fdc: Rename fdctrl_reset_fifo() to fdctrl_to_command_phase()

[John Snow]

From: Kevin Wolf <kwolf@redhat.com>

What all callers of fdctrl_reset_fifo() really want to do is to start
the command phase, where writes to the data port initiate a new command.

The function doesn't only clear the FIFO, but also sets up the state so
that a new command can be received. Rename it to reflect this.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-2-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/block/fdc.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index d8a8edd..9f95ace 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -324,7 +324,7 @@ static void fd_revalidate(FDrive *drv)
 /* Intel 82078 floppy disk controller emulation          */
 
 static void fdctrl_reset(FDCtrl *fdctrl, int do_irq);
-static void fdctrl_reset_fifo(FDCtrl *fdctrl);
+static void fdctrl_to_command_phase(FDCtrl *fdctrl);
 static int fdctrl_transfer_handler (void *opaque, int nchan,
                                     int dma_pos, int dma_len);
 static void fdctrl_raise_irq(FDCtrl *fdctrl);
@@ -918,7 +918,7 @@ static void fdctrl_reset(FDCtrl *fdctrl, int do_irq)
     fdctrl->data_dir = FD_DIR_WRITE;
     for (i = 0; i < MAX_FD; i++)
         fd_recalibrate(&fdctrl->drives[i]);
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
     if (do_irq) {
         fdctrl->status0 |= FD_SR0_RDYCHG;
         fdctrl_raise_irq(fdctrl);
@@ -1134,8 +1134,8 @@ static uint32_t fdctrl_read_dir(FDCtrl *fdctrl)
     return retval;
 }
 
-/* FIFO state control */
-static void fdctrl_reset_fifo(FDCtrl *fdctrl)
+/* Clear the FIFO and update the state for receiving the next command */
+static void fdctrl_to_command_phase(FDCtrl *fdctrl)
 {
     fdctrl->data_dir = FD_DIR_WRITE;
     fdctrl->data_pos = 0;
@@ -1533,7 +1533,7 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
         if (fdctrl->msr & FD_MSR_NONDMA) {
             fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
         } else {
-            fdctrl_reset_fifo(fdctrl);
+            fdctrl_to_command_phase(fdctrl);
             fdctrl_reset_irq(fdctrl);
         }
     }
@@ -1667,7 +1667,7 @@ static void fdctrl_handle_restore(FDCtrl *fdctrl, int direction)
     fdctrl->config = fdctrl->fifo[11];
     fdctrl->precomp_trk = fdctrl->fifo[12];
     fdctrl->pwrd = fdctrl->fifo[13];
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
 }
 
 static void fdctrl_handle_save(FDCtrl *fdctrl, int direction)
@@ -1746,7 +1746,7 @@ static void fdctrl_handle_specify(FDCtrl *fdctrl, int direction)
     else
         fdctrl->dor |= FD_DOR_DMAEN;
     /* No result back */
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
 }
 
 static void fdctrl_handle_sense_drive_status(FDCtrl *fdctrl, int direction)
@@ -1772,7 +1772,7 @@ static void fdctrl_handle_recalibrate(FDCtrl *fdctrl, int direction)
     SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
     cur_drv = get_cur_drv(fdctrl);
     fd_recalibrate(cur_drv);
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
     /* Raise Interrupt */
     fdctrl->status0 |= FD_SR0_SEEK;
     fdctrl_raise_irq(fdctrl);
@@ -1808,7 +1808,7 @@ static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction)
 
     SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
     cur_drv = get_cur_drv(fdctrl);
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
     /* The seek command just sends step pulses to the drive and doesn't care if
      * there is a medium inserted of if it's banging the head against the drive.
      */
@@ -1825,7 +1825,7 @@ static void fdctrl_handle_perpendicular_mode(FDCtrl *fdctrl, int direction)
     if (fdctrl->fifo[1] & 0x80)
         cur_drv->perpendicular = fdctrl->fifo[1] & 0x7;
     /* No result back */
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
 }
 
 static void fdctrl_handle_configure(FDCtrl *fdctrl, int direction)
@@ -1833,7 +1833,7 @@ static void fdctrl_handle_configure(FDCtrl *fdctrl, int direction)
     fdctrl->config = fdctrl->fifo[2];
     fdctrl->precomp_trk =  fdctrl->fifo[3];
     /* No result back */
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
 }
 
 static void fdctrl_handle_powerdown_mode(FDCtrl *fdctrl, int direction)
@@ -1846,7 +1846,7 @@ static void fdctrl_handle_powerdown_mode(FDCtrl *fdctrl, int direction)
 static void fdctrl_handle_option(FDCtrl *fdctrl, int direction)
 {
     /* No result back */
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
 }
 
 static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direction)
@@ -1864,7 +1864,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct
             fdctrl->fifo[3] = 0;
             fdctrl_set_fifo(fdctrl, 4);
         } else {
-            fdctrl_reset_fifo(fdctrl);
+            fdctrl_to_command_phase(fdctrl);
         }
     } else if (fdctrl->data_len > 7) {
         /* ERROR */
@@ -1887,7 +1887,7 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)
         fd_seek(cur_drv, cur_drv->head,
                 cur_drv->track + fdctrl->fifo[2], cur_drv->sect, 1);
     }
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
     /* Raise Interrupt */
     fdctrl->status0 |= FD_SR0_SEEK;
     fdctrl_raise_irq(fdctrl);
@@ -1905,7 +1905,7 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
         fd_seek(cur_drv, cur_drv->head,
                 cur_drv->track - fdctrl->fifo[2], cur_drv->sect, 1);
     }
-    fdctrl_reset_fifo(fdctrl);
+    fdctrl_to_command_phase(fdctrl);
     /* Raise Interrupt */
     fdctrl->status0 |= FD_SR0_SEEK;
     fdctrl_raise_irq(fdctrl);
-- 
2.1.0

[Qemu-devel] [PULL 02/12] fdc: Rename fdctrl_set_fifo() to fdctrl_to_result_phase()

[John Snow]

From: Kevin Wolf <kwolf@redhat.com>

What callers really do with this function is to switch from execution
phase (including data transfers) to result phase where the guest can
read out one or more status bytes from the FIFO (the number depends on
the command).

Rename the function accordingly.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-3-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/block/fdc.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 9f95ace..8c41434 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1142,8 +1142,9 @@ static void fdctrl_to_command_phase(FDCtrl *fdctrl)
     fdctrl->msr &= ~(FD_MSR_CMDBUSY | FD_MSR_DIO);
 }
 
-/* Set FIFO status for the host to read */
-static void fdctrl_set_fifo(FDCtrl *fdctrl, int fifo_len)
+/* Update the state to allow the guest to read out the command status.
+ * @fifo_len is the number of result bytes to be read out. */
+static void fdctrl_to_result_phase(FDCtrl *fdctrl, int fifo_len)
 {
     fdctrl->data_dir = FD_DIR_READ;
     fdctrl->data_len = fifo_len;
@@ -1157,7 +1158,7 @@ static void fdctrl_unimplemented(FDCtrl *fdctrl, int direction)
     qemu_log_mask(LOG_UNIMP, "fdc: unimplemented command 0x%02x\n",
                   fdctrl->fifo[0]);
     fdctrl->fifo[0] = FD_SR0_INVCMD;
-    fdctrl_set_fifo(fdctrl, 1);
+    fdctrl_to_result_phase(fdctrl, 1);
 }
 
 /* Seek to next sector
@@ -1238,7 +1239,7 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0,
     fdctrl->msr |= FD_MSR_RQM | FD_MSR_DIO;
     fdctrl->msr &= ~FD_MSR_NONDMA;
 
-    fdctrl_set_fifo(fdctrl, 7);
+    fdctrl_to_result_phase(fdctrl, 7);
     fdctrl_raise_irq(fdctrl);
 }
 
@@ -1606,7 +1607,7 @@ static void fdctrl_handle_lock(FDCtrl *fdctrl, int direction)
 {
     fdctrl->lock = (fdctrl->fifo[0] & 0x80) ? 1 : 0;
     fdctrl->fifo[0] = fdctrl->lock << 4;
-    fdctrl_set_fifo(fdctrl, 1);
+    fdctrl_to_result_phase(fdctrl, 1);
 }
 
 static void fdctrl_handle_dumpreg(FDCtrl *fdctrl, int direction)
@@ -1631,20 +1632,20 @@ static void fdctrl_handle_dumpreg(FDCtrl *fdctrl, int direction)
         (cur_drv->perpendicular << 2);
     fdctrl->fifo[8] = fdctrl->config;
     fdctrl->fifo[9] = fdctrl->precomp_trk;
-    fdctrl_set_fifo(fdctrl, 10);
+    fdctrl_to_result_phase(fdctrl, 10);
 }
 
 static void fdctrl_handle_version(FDCtrl *fdctrl, int direction)
 {
     /* Controller's version */
     fdctrl->fifo[0] = fdctrl->version;
-    fdctrl_set_fifo(fdctrl, 1);
+    fdctrl_to_result_phase(fdctrl, 1);
 }
 
 static void fdctrl_handle_partid(FDCtrl *fdctrl, int direction)
 {
     fdctrl->fifo[0] = 0x41; /* Stepping 1 */
-    fdctrl_set_fifo(fdctrl, 1);
+    fdctrl_to_result_phase(fdctrl, 1);
 }
 
 static void fdctrl_handle_restore(FDCtrl *fdctrl, int direction)
@@ -1697,7 +1698,7 @@ static void fdctrl_handle_save(FDCtrl *fdctrl, int direction)
     fdctrl->fifo[12] = fdctrl->pwrd;
     fdctrl->fifo[13] = 0;
     fdctrl->fifo[14] = 0;
-    fdctrl_set_fifo(fdctrl, 15);
+    fdctrl_to_result_phase(fdctrl, 15);
 }
 
 static void fdctrl_handle_readid(FDCtrl *fdctrl, int direction)
@@ -1762,7 +1763,7 @@ static void fdctrl_handle_sense_drive_status(FDCtrl *fdctrl, int direction)
         (cur_drv->head << 2) |
         GET_CUR_DRV(fdctrl) |
         0x28;
-    fdctrl_set_fifo(fdctrl, 1);
+    fdctrl_to_result_phase(fdctrl, 1);
 }
 
 static void fdctrl_handle_recalibrate(FDCtrl *fdctrl, int direction)
@@ -1788,7 +1789,7 @@ static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction)
         fdctrl->reset_sensei--;
     } else if (!(fdctrl->sra & FD_SRA_INTPEND)) {
         fdctrl->fifo[0] = FD_SR0_INVCMD;
-        fdctrl_set_fifo(fdctrl, 1);
+        fdctrl_to_result_phase(fdctrl, 1);
         return;
     } else {
         fdctrl->fifo[0] =
@@ -1797,7 +1798,7 @@ static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction)
     }
 
     fdctrl->fifo[1] = cur_drv->track;
-    fdctrl_set_fifo(fdctrl, 2);
+    fdctrl_to_result_phase(fdctrl, 2);
     fdctrl_reset_irq(fdctrl);
     fdctrl->status0 = FD_SR0_RDYCHG;
 }
@@ -1840,7 +1841,7 @@ static void fdctrl_handle_powerdown_mode(FDCtrl *fdctrl, int direction)
 {
     fdctrl->pwrd = fdctrl->fifo[1];
     fdctrl->fifo[0] = fdctrl->fifo[1];
-    fdctrl_set_fifo(fdctrl, 1);
+    fdctrl_to_result_phase(fdctrl, 1);
 }
 
 static void fdctrl_handle_option(FDCtrl *fdctrl, int direction)
@@ -1862,7 +1863,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct
             fdctrl->fifo[0] = fdctrl->fifo[1];
             fdctrl->fifo[2] = 0;
             fdctrl->fifo[3] = 0;
-            fdctrl_set_fifo(fdctrl, 4);
+            fdctrl_to_result_phase(fdctrl, 4);
         } else {
             fdctrl_to_command_phase(fdctrl);
         }
@@ -1870,7 +1871,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct
         /* ERROR */
         fdctrl->fifo[0] = 0x80 |
             (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
-        fdctrl_set_fifo(fdctrl, 1);
+        fdctrl_to_result_phase(fdctrl, 1);
     }
 }
 
-- 
2.1.0

[Qemu-devel] [PULL 03/12] fdc: Introduce fdctrl->phase

[John Snow]

From: Kevin Wolf <kwolf@redhat.com>

The floppy controller spec describes three different controller phases,
which are currently not explicitly modelled in our emulation. Instead,
each phase is represented by a combination of flags in registers.

This patch makes explicit in which phase the controller currently is.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-4-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/block/fdc.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 8c41434..f5bcf0b 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -495,6 +495,33 @@ enum {
     FD_DIR_DSKCHG   = 0x80,
 };
 
+/*
+ * See chapter 5.0 "Controller phases" of the spec:
+ *
+ * Command phase:
+ * The host writes a command and its parameters into the FIFO. The command
+ * phase is completed when all parameters for the command have been supplied,
+ * and execution phase is entered.
+ *
+ * Execution phase:
+ * Data transfers, either DMA or non-DMA. For non-DMA transfers, the FIFO
+ * contains the payload now, otherwise it's unused. When all bytes of the
+ * required data have been transferred, the state is switched to either result
+ * phase (if the command produces status bytes) or directly back into the
+ * command phase for the next command.
+ *
+ * Result phase:
+ * The host reads out the FIFO, which contains one or more result bytes now.
+ */
+enum {
+    /* Only for migration: reconstruct phase from registers like qemu 2.3 */
+    FD_PHASE_RECONSTRUCT    = 0,
+
+    FD_PHASE_COMMAND        = 1,
+    FD_PHASE_EXECUTION      = 2,
+    FD_PHASE_RESULT         = 3,
+};
+
 #define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
 #define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
 
@@ -504,6 +531,7 @@ struct FDCtrl {
     /* Controller state */
     QEMUTimer *result_timer;
     int dma_chann;
+    uint8_t phase;
     /* Controller's identification */
     uint8_t version;
     /* HW */
@@ -744,6 +772,28 @@ static const VMStateDescription vmstate_fdrive = {
     }
 };
 
+/*
+ * Reconstructs the phase from register values according to the logic that was
+ * implemented in qemu 2.3. This is the default value that is used if the phase
+ * subsection is not present on migration.
+ *
+ * Don't change this function to reflect newer qemu versions, it is part of
+ * the migration ABI.
+ */
+static int reconstruct_phase(FDCtrl *fdctrl)
+{
+    if (fdctrl->msr & FD_MSR_NONDMA) {
+        return FD_PHASE_EXECUTION;
+    } else if ((fdctrl->msr & FD_MSR_RQM) == 0) {
+        /* qemu 2.3 disabled RQM only during DMA transfers */
+        return FD_PHASE_EXECUTION;
+    } else if (fdctrl->msr & FD_MSR_DIO) {
+        return FD_PHASE_RESULT;
+    } else {
+        return FD_PHASE_COMMAND;
+    }
+}
+
 static void fdc_pre_save(void *opaque)
 {
     FDCtrl *s = opaque;
@@ -751,12 +801,24 @@ static void fdc_pre_save(void *opaque)
     s->dor_vmstate = s->dor | GET_CUR_DRV(s);
 }
 
+static int fdc_pre_load(void *opaque)
+{
+    FDCtrl *s = opaque;
+    s->phase = FD_PHASE_RECONSTRUCT;
+    return 0;
+}
+
 static int fdc_post_load(void *opaque, int version_id)
 {
     FDCtrl *s = opaque;
 
     SET_CUR_DRV(s, s->dor_vmstate & FD_DOR_SELMASK);
     s->dor = s->dor_vmstate & ~FD_DOR_SELMASK;
+
+    if (s->phase == FD_PHASE_RECONSTRUCT) {
+        s->phase = reconstruct_phase(s);
+    }
+
     return 0;
 }
 
@@ -794,11 +856,29 @@ static const VMStateDescription vmstate_fdc_result_timer = {
     }
 };
 
+static bool fdc_phase_needed(void *opaque)
+{
+    FDCtrl *fdctrl = opaque;
+
+    return reconstruct_phase(fdctrl) != fdctrl->phase;
+}
+
+static const VMStateDescription vmstate_fdc_phase = {
+    .name = "fdc/phase",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(phase, FDCtrl),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_fdc = {
     .name = "fdc",
     .version_id = 2,
     .minimum_version_id = 2,
     .pre_save = fdc_pre_save,
+    .pre_load = fdc_pre_load,
     .post_load = fdc_post_load,
     .fields = (VMStateField[]) {
         /* Controller State */
@@ -839,6 +919,9 @@ static const VMStateDescription vmstate_fdc = {
             .vmsd = &vmstate_fdc_result_timer,
             .needed = fdc_result_timer_needed,
         } , {
+            .vmsd = &vmstate_fdc_phase,
+            .needed = fdc_phase_needed,
+        } , {
             /* empty */
         }
     }
@@ -1137,6 +1220,7 @@ static uint32_t fdctrl_read_dir(FDCtrl *fdctrl)
 /* Clear the FIFO and update the state for receiving the next command */
 static void fdctrl_to_command_phase(FDCtrl *fdctrl)
 {
+    fdctrl->phase = FD_PHASE_COMMAND;
     fdctrl->data_dir = FD_DIR_WRITE;
     fdctrl->data_pos = 0;
     fdctrl->msr &= ~(FD_MSR_CMDBUSY | FD_MSR_DIO);
@@ -1146,6 +1230,7 @@ static void fdctrl_to_command_phase(FDCtrl *fdctrl)
  * @fifo_len is the number of result bytes to be read out. */
 static void fdctrl_to_result_phase(FDCtrl *fdctrl, int fifo_len)
 {
+    fdctrl->phase = FD_PHASE_RESULT;
     fdctrl->data_dir = FD_DIR_READ;
     fdctrl->data_len = fifo_len;
     fdctrl->data_pos = 0;
@@ -1912,6 +1997,9 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
     fdctrl_raise_irq(fdctrl);
 }
 
+/*
+ * Handlers for the execution phase of each command
+ */
 static const struct {
     uint8_t value;
     uint8_t mask;
@@ -2015,6 +2103,7 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
         /* We now have all parameters
          * and will be able to treat the command
          */
+        fdctrl->phase = FD_PHASE_EXECUTION;
         if (fdctrl->data_state & FD_STATE_FORMAT) {
             fdctrl_format_sector(fdctrl);
             return;
-- 
2.1.0

[Qemu-devel] [PULL 04/12] fdc: Use phase in fdctrl_write_data()

[John Snow]

From: Kevin Wolf <kwolf@redhat.com>

Instead of relying on a flag in the MSR to distinguish controller phases,
use the explicit phase that we store now. Assertions of the right MSR
flags are added.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-5-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/block/fdc.c | 69 ++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index f5bcf0b..e3515a1 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2059,8 +2059,12 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
         return;
     }
     fdctrl->dsr &= ~FD_DSR_PWRDOWN;
-    /* Is it write command time ? */
-    if (fdctrl->msr & FD_MSR_NONDMA) {
+
+    switch (fdctrl->phase) {
+    case FD_PHASE_EXECUTION:
+        /* For DMA requests, RQM should be cleared during execution phase, so
+         * we would have errored out above. */
+        assert(fdctrl->msr & FD_MSR_NONDMA);
         /* FIFO data write */
         pos = fdctrl->data_pos++;
         pos %= FD_SECTOR_LEN;
@@ -2072,12 +2076,12 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
                 < 0) {
                 FLOPPY_DPRINTF("error writing sector %d\n",
                                fd_sector(cur_drv));
-                return;
+                break;
             }
             if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
                 FLOPPY_DPRINTF("error seeking to next sector %d\n",
                                fd_sector(cur_drv));
-                return;
+                break;
             }
         }
         /* Switch from transfer mode to status mode
@@ -2085,33 +2089,42 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
          */
         if (fdctrl->data_pos == fdctrl->data_len)
             fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
-        return;
-    }
-    if (fdctrl->data_pos == 0) {
-        /* Command */
-        pos = command_to_handler[value & 0xff];
-        FLOPPY_DPRINTF("%s command\n", handlers[pos].name);
-        fdctrl->data_len = handlers[pos].parameters + 1;
-        fdctrl->msr |= FD_MSR_CMDBUSY;
-    }
+        break;
 
-    FLOPPY_DPRINTF("%s: %02x\n", __func__, value);
-    pos = fdctrl->data_pos++;
-    pos %= FD_SECTOR_LEN;
-    fdctrl->fifo[pos] = value;
-    if (fdctrl->data_pos == fdctrl->data_len) {
-        /* We now have all parameters
-         * and will be able to treat the command
-         */
-        fdctrl->phase = FD_PHASE_EXECUTION;
-        if (fdctrl->data_state & FD_STATE_FORMAT) {
-            fdctrl_format_sector(fdctrl);
-            return;
+    case FD_PHASE_COMMAND:
+        assert(!(fdctrl->msr & FD_MSR_NONDMA));
+
+        if (fdctrl->data_pos == 0) {
+            /* Command */
+            pos = command_to_handler[value & 0xff];
+            FLOPPY_DPRINTF("%s command\n", handlers[pos].name);
+            fdctrl->data_len = handlers[pos].parameters + 1;
+            fdctrl->msr |= FD_MSR_CMDBUSY;
+        }
+
+        FLOPPY_DPRINTF("%s: %02x\n", __func__, value);
+        pos = fdctrl->data_pos++;
+        pos %= FD_SECTOR_LEN;
+        fdctrl->fifo[pos] = value;
+        if (fdctrl->data_pos == fdctrl->data_len) {
+            /* We now have all parameters
+             * and will be able to treat the command
+             */
+            fdctrl->phase = FD_PHASE_EXECUTION;
+            if (fdctrl->data_state & FD_STATE_FORMAT) {
+                fdctrl_format_sector(fdctrl);
+                break;
+            }
+
+            pos = command_to_handler[fdctrl->fifo[0] & 0xff];
+            FLOPPY_DPRINTF("treat %s command\n", handlers[pos].name);
+            (*handlers[pos].handler)(fdctrl, handlers[pos].direction);
         }
+        break;
 
-        pos = command_to_handler[fdctrl->fifo[0] & 0xff];
-        FLOPPY_DPRINTF("treat %s command\n", handlers[pos].name);
-        (*handlers[pos].handler)(fdctrl, handlers[pos].direction);
+    case FD_PHASE_RESULT:
+    default:
+        abort();
     }
 }
 
-- 
2.1.0

[Qemu-devel] [PULL 05/12] fdc: Code cleanup in fdctrl_write_data()

[John Snow]

From: Kevin Wolf <kwolf@redhat.com>

Factor out a few common lines of code, reformat, improve comments.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-6-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/block/fdc.c | 63 ++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index e3515a1..1cc1e3a 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2000,14 +2000,16 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
 /*
  * Handlers for the execution phase of each command
  */
-static const struct {
+typedef struct FDCtrlCommand {
     uint8_t value;
     uint8_t mask;
     const char* name;
     int parameters;
     void (*handler)(FDCtrl *fdctrl, int direction);
     int direction;
-} handlers[] = {
+} FDCtrlCommand;
+
+static const FDCtrlCommand handlers[] = {
     { FD_CMD_READ, 0x1f, "READ", 8, fdctrl_start_transfer, FD_DIR_READ },
     { FD_CMD_WRITE, 0x3f, "WRITE", 8, fdctrl_start_transfer, FD_DIR_WRITE },
     { FD_CMD_SEEK, 0xff, "SEEK", 2, fdctrl_handle_seek },
@@ -2044,9 +2046,19 @@ static const struct {
 /* Associate command to an index in the 'handlers' array */
 static uint8_t command_to_handler[256];
 
+static const FDCtrlCommand *get_command(uint8_t cmd)
+{
+    int idx;
+
+    idx = command_to_handler[cmd];
+    FLOPPY_DPRINTF("%s command\n", handlers[idx].name);
+    return &handlers[idx];
+}
+
 static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
 {
     FDrive *cur_drv;
+    const FDCtrlCommand *cmd;
     uint32_t pos;
 
     /* Reset mode */
@@ -2060,15 +2072,22 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
     }
     fdctrl->dsr &= ~FD_DSR_PWRDOWN;
 
+    FLOPPY_DPRINTF("%s: %02x\n", __func__, value);
+
+    /* If data_len spans multiple sectors, the current position in the FIFO
+     * wraps around while fdctrl->data_pos is the real position in the whole
+     * request. */
+    pos = fdctrl->data_pos++;
+    pos %= FD_SECTOR_LEN;
+    fdctrl->fifo[pos] = value;
+
     switch (fdctrl->phase) {
     case FD_PHASE_EXECUTION:
         /* For DMA requests, RQM should be cleared during execution phase, so
          * we would have errored out above. */
         assert(fdctrl->msr & FD_MSR_NONDMA);
+
         /* FIFO data write */
-        pos = fdctrl->data_pos++;
-        pos %= FD_SECTOR_LEN;
-        fdctrl->fifo[pos] = value;
         if (pos == FD_SECTOR_LEN - 1 ||
             fdctrl->data_pos == fdctrl->data_len) {
             cur_drv = get_cur_drv(fdctrl);
@@ -2084,41 +2103,37 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
                 break;
             }
         }
-        /* Switch from transfer mode to status mode
-         * then from status mode to command mode
-         */
-        if (fdctrl->data_pos == fdctrl->data_len)
+
+        /* Switch to result phase when done with the transfer */
+        if (fdctrl->data_pos == fdctrl->data_len) {
             fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
+        }
         break;
 
     case FD_PHASE_COMMAND:
         assert(!(fdctrl->msr & FD_MSR_NONDMA));
+        assert(fdctrl->data_pos < FD_SECTOR_LEN);
 
-        if (fdctrl->data_pos == 0) {
-            /* Command */
-            pos = command_to_handler[value & 0xff];
-            FLOPPY_DPRINTF("%s command\n", handlers[pos].name);
-            fdctrl->data_len = handlers[pos].parameters + 1;
+        if (pos == 0) {
+            /* The first byte specifies the command. Now we start reading
+             * as many parameters as this command requires. */
+            cmd = get_command(value);
+            fdctrl->data_len = cmd->parameters + 1;
             fdctrl->msr |= FD_MSR_CMDBUSY;
         }
 
-        FLOPPY_DPRINTF("%s: %02x\n", __func__, value);
-        pos = fdctrl->data_pos++;
-        pos %= FD_SECTOR_LEN;
-        fdctrl->fifo[pos] = value;
         if (fdctrl->data_pos == fdctrl->data_len) {
-            /* We now have all parameters
-             * and will be able to treat the command
-             */
+            /* We have all parameters now, execute the command */
             fdctrl->phase = FD_PHASE_EXECUTION;
+
             if (fdctrl->data_state & FD_STATE_FORMAT) {
                 fdctrl_format_sector(fdctrl);
                 break;
             }
 
-            pos = command_to_handler[fdctrl->fifo[0] & 0xff];
-            FLOPPY_DPRINTF("treat %s command\n", handlers[pos].name);
-            (*handlers[pos].handler)(fdctrl, handlers[pos].direction);
+            cmd = get_command(fdctrl->fifo[0]);
+            FLOPPY_DPRINTF("Calling handler for '%s'\n", cmd->name);
+            cmd->handler(fdctrl, cmd->direction);
         }
         break;
 
-- 
2.1.0

[Qemu-devel] [PULL 06/12] fdc: Disentangle phases in fdctrl_read_data()

[John Snow]

From: Kevin Wolf <kwolf@redhat.com>

This commit makes similar improvements as have already been made to the
write function: Instead of relying on a flag in the MSR to distinguish
controller phases, use the explicit phase that we store now. Assertions
of the right MSR flags are added.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-7-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/block/fdc.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 1cc1e3a..3c64194 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1591,9 +1591,16 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
         FLOPPY_DPRINTF("error: controller not ready for reading\n");
         return 0;
     }
+
+    /* If data_len spans multiple sectors, the current position in the FIFO
+     * wraps around while fdctrl->data_pos is the real position in the whole
+     * request. */
     pos = fdctrl->data_pos;
     pos %= FD_SECTOR_LEN;
-    if (fdctrl->msr & FD_MSR_NONDMA) {
+
+    switch (fdctrl->phase) {
+    case FD_PHASE_EXECUTION:
+        assert(fdctrl->msr & FD_MSR_NONDMA);
         if (pos == 0) {
             if (fdctrl->data_pos != 0)
                 if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
@@ -1609,20 +1616,26 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
                 memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
             }
         }
-    }
-    retval = fdctrl->fifo[pos];
-    if (++fdctrl->data_pos == fdctrl->data_len) {
-        fdctrl->data_pos = 0;
-        /* Switch from transfer mode to status mode
-         * then from status mode to command mode
-         */
-        if (fdctrl->msr & FD_MSR_NONDMA) {
+
+        if (++fdctrl->data_pos == fdctrl->data_len) {
             fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
-        } else {
+        }
+        break;
+
+    case FD_PHASE_RESULT:
+        assert(!(fdctrl->msr & FD_MSR_NONDMA));
+        if (++fdctrl->data_pos == fdctrl->data_len) {
             fdctrl_to_command_phase(fdctrl);
             fdctrl_reset_irq(fdctrl);
         }
+        break;
+
+    case FD_PHASE_COMMAND:
+    default:
+        abort();
     }
+
+    retval = fdctrl->fifo[pos];
     FLOPPY_DPRINTF("data register: 0x%02x\n", retval);
 
     return retval;
-- 
2.1.0

[Qemu-devel] [PULL 07/12] fdc: Fix MSR.RQM flag

[John Snow]

From: Kevin Wolf <kwolf@redhat.com>

The RQM bit in MSR should be set whenever the guest is supposed to
access the FIFO, and it should be cleared in all other cases. This is
important so the guest can't continue writing/reading the FIFO beyond
the length that it's suppossed to access (see CVE-2015-3456).

Commit e9077462 fixed the CVE by adding code that avoids the buffer
overflow; however it doesn't correct the wrong behaviour of the floppy
controller which should already have cleared RQM.

Currently, RQM stays set all the time and during all phases while a
command is being processed. This is error-prone because the command has
to explicitly clear the flag if it doesn't need data (and indeed, the
two buggy commands that are the culprits for the CVE just forgot to do
that).

This patch clears RQM immediately as soon as all bytes that are expected
have been received. If the the FIFO is used in the next phase, the flag
has to be set explicitly there.

It also clear RQM after receiving all bytes even if the phase transition
immediately sets it again. While it's technically not necessary at the
moment because the state between clearing and setting RQM is not
observable by the guest, this is more explicit and matches how real
hardware works. It will actually become necessary in qemu once
asynchronous code paths are introduced.

This alone should have been enough to fix the CVE, but now we have two
lines of defense - even better.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-8-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/block/fdc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 3c64194..6e79459 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1223,7 +1223,9 @@ static void fdctrl_to_command_phase(FDCtrl *fdctrl)
     fdctrl->phase = FD_PHASE_COMMAND;
     fdctrl->data_dir = FD_DIR_WRITE;
     fdctrl->data_pos = 0;
+    fdctrl->data_len = 1; /* Accept command byte, adjust for params later */
     fdctrl->msr &= ~(FD_MSR_CMDBUSY | FD_MSR_DIO);
+    fdctrl->msr |= FD_MSR_RQM;
 }
 
 /* Update the state to allow the guest to read out the command status.
@@ -1438,7 +1440,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
         }
     }
     FLOPPY_DPRINTF("start non-DMA transfer\n");
-    fdctrl->msr |= FD_MSR_NONDMA;
+    fdctrl->msr |= FD_MSR_NONDMA | FD_MSR_RQM;
     if (direction != FD_DIR_WRITE)
         fdctrl->msr |= FD_MSR_DIO;
     /* IO based transfer: calculate len */
@@ -1618,6 +1620,7 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
         }
 
         if (++fdctrl->data_pos == fdctrl->data_len) {
+            fdctrl->msr &= ~FD_MSR_RQM;
             fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
         }
         break;
@@ -1625,6 +1628,7 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
     case FD_PHASE_RESULT:
         assert(!(fdctrl->msr & FD_MSR_NONDMA));
         if (++fdctrl->data_pos == fdctrl->data_len) {
+            fdctrl->msr &= ~FD_MSR_RQM;
             fdctrl_to_command_phase(fdctrl);
             fdctrl_reset_irq(fdctrl);
         }
@@ -2094,6 +2098,10 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
     pos %= FD_SECTOR_LEN;
     fdctrl->fifo[pos] = value;
 
+    if (fdctrl->data_pos == fdctrl->data_len) {
+        fdctrl->msr &= ~FD_MSR_RQM;
+    }
+
     switch (fdctrl->phase) {
     case FD_PHASE_EXECUTION:
         /* For DMA requests, RQM should be cleared during execution phase, so
@@ -2132,6 +2140,9 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
              * as many parameters as this command requires. */
             cmd = get_command(value);
             fdctrl->data_len = cmd->parameters + 1;
+            if (cmd->parameters) {
+                fdctrl->msr |= FD_MSR_RQM;
+            }
             fdctrl->msr |= FD_MSR_CMDBUSY;
         }
 
-- 
2.1.0

[Qemu-devel] [PULL 08/12] fdc-test: Test state for existing cases more thoroughly

[John Snow]

From: Kevin Wolf <kwolf@redhat.com>

This just adds a few additional checks of the MSR and interrupt pin to
the already existing test cases.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-9-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/fdc-test.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 3c6c83c..416394f 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -218,6 +218,10 @@ static uint8_t send_read_no_dma_command(int nb_sect, uint8_t expected_st0)
         inb(FLOPPY_BASE + reg_fifo);
     }
 
+    msr = inb(FLOPPY_BASE + reg_msr);
+    assert_bit_set(msr, BUSY | RQM | DIO);
+    g_assert(get_irq(FLOPPY_IRQ));
+
     st0 = floppy_recv();
     if (st0 != expected_st0) {
         ret = 1;
@@ -228,8 +232,15 @@ static uint8_t send_read_no_dma_command(int nb_sect, uint8_t expected_st0)
     floppy_recv();
     floppy_recv();
     floppy_recv();
+    g_assert(get_irq(FLOPPY_IRQ));
     floppy_recv();
 
+    /* Check that we're back in command phase */
+    msr = inb(FLOPPY_BASE + reg_msr);
+    assert_bit_clear(msr, BUSY | DIO);
+    assert_bit_set(msr, RQM);
+    g_assert(!get_irq(FLOPPY_IRQ));
+
     return ret;
 }
 
@@ -403,6 +414,7 @@ static void test_read_id(void)
     uint8_t head = 0;
     uint8_t cyl;
     uint8_t st0;
+    uint8_t msr;
 
     /* Seek to track 0 and check with READ ID */
     send_seek(0);
@@ -411,18 +423,29 @@ static void test_read_id(void)
     g_assert(!get_irq(FLOPPY_IRQ));
     floppy_send(head << 2 | drive);
 
+    msr = inb(FLOPPY_BASE + reg_msr);
+    if (!get_irq(FLOPPY_IRQ)) {
+        assert_bit_set(msr, BUSY);
+        assert_bit_clear(msr, RQM);
+    }
+
     while (!get_irq(FLOPPY_IRQ)) {
         /* qemu involves a timer with READ ID... */
         clock_step(1000000000LL / 50);
     }
 
+    msr = inb(FLOPPY_BASE + reg_msr);
+    assert_bit_set(msr, BUSY | RQM | DIO);
+
     st0 = floppy_recv();
     floppy_recv();
     floppy_recv();
     cyl = floppy_recv();
     head = floppy_recv();
     floppy_recv();
+    g_assert(get_irq(FLOPPY_IRQ));
     floppy_recv();
+    g_assert(!get_irq(FLOPPY_IRQ));
 
     g_assert_cmpint(cyl, ==, 0);
     g_assert_cmpint(head, ==, 0);
@@ -443,18 +466,29 @@ static void test_read_id(void)
     g_assert(!get_irq(FLOPPY_IRQ));
     floppy_send(head << 2 | drive);
 
+    msr = inb(FLOPPY_BASE + reg_msr);
+    if (!get_irq(FLOPPY_IRQ)) {
+        assert_bit_set(msr, BUSY);
+        assert_bit_clear(msr, RQM);
+    }
+
     while (!get_irq(FLOPPY_IRQ)) {
         /* qemu involves a timer with READ ID... */
         clock_step(1000000000LL / 50);
     }
 
+    msr = inb(FLOPPY_BASE + reg_msr);
+    assert_bit_set(msr, BUSY | RQM | DIO);
+
     st0 = floppy_recv();
     floppy_recv();
     floppy_recv();
     cyl = floppy_recv();
     head = floppy_recv();
     floppy_recv();
+    g_assert(get_irq(FLOPPY_IRQ));
     floppy_recv();
+    g_assert(!get_irq(FLOPPY_IRQ));
 
     g_assert_cmpint(cyl, ==, 8);
     g_assert_cmpint(head, ==, 1);
-- 
2.1.0

[Qemu-devel] [PULL 09/12] macio: switch pmac_dma_read() over to new offset/len implementation

[John Snow]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

For better handling of unaligned block device accesses.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1433455177-21243-2-git-send-email-mark.cave-ayland@ilande.co.uk
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/macio.c | 106 +++++++++++++++++++++++----------------------------------
 1 file changed, 42 insertions(+), 64 deletions(-)

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 585a27b..52ee4ac 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -52,7 +52,7 @@ static const int debug_macio = 0;
 #define MACIO_PAGE_SIZE 4096
 
 static void pmac_dma_read(BlockBackend *blk,
-                          int64_t sector_num, int nb_sectors,
+                          int64_t offset, unsigned int bytes,
                           void (*cb)(void *opaque, int ret), void *opaque)
 {
     DBDMA_io *io = opaque;
@@ -60,76 +60,48 @@ static void pmac_dma_read(BlockBackend *blk,
     IDEState *s = idebus_active_if(&m->bus);
     dma_addr_t dma_addr, dma_len;
     void *mem;
-    int nsector, remainder;
+    int64_t sector_num;
+    int nsector;
+    uint64_t align = BDRV_SECTOR_SIZE;
+    size_t head_bytes, tail_bytes;
 
     qemu_iovec_destroy(&io->iov);
     qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1);
 
-    if (io->remainder_len > 0) {
-        /* Return remainder of request */
-        int transfer = MIN(io->remainder_len, io->len);
+    sector_num = (offset >> 9);
+    nsector = (io->len >> 9);
 
-        MACIO_DPRINTF("--- DMA read pop     - bounce addr: %p addr: %"
-                      HWADDR_PRIx " remainder_len: %x\n",
-                      &io->remainder + (0x200 - transfer), io->addr,
-                      io->remainder_len);
-
-        cpu_physical_memory_write(io->addr,
-                                  &io->remainder + (0x200 - transfer),
-                                  transfer);
-
-        io->remainder_len -= transfer;
-        io->len -= transfer;
-        io->addr += transfer;
-
-        s->io_buffer_index += transfer;
-        s->io_buffer_size -= transfer;
-
-        if (io->remainder_len != 0) {
-            /* Still waiting for remainder */
-            return;
-        }
-
-        if (io->len == 0) {
-            MACIO_DPRINTF("--- finished all read processing; go and finish\n");
-            cb(opaque, 0);
-            return;
-        }
-    }
-
-    if (s->drive_kind == IDE_CD) {
-        sector_num = (int64_t)(s->lba << 2) + (s->io_buffer_index >> 9);
-    } else {
-        sector_num = ide_get_sector(s) + (s->io_buffer_index >> 9);
-    }
-
-    nsector = ((io->len + 0x1ff) >> 9);
-    remainder = (nsector << 9) - io->len;
-
-    MACIO_DPRINTF("--- DMA read transfer - addr: %" HWADDR_PRIx " len: %x\n",
-                  io->addr, io->len);
+    MACIO_DPRINTF("--- DMA read transfer (0x%" HWADDR_PRIx ",0x%x): "
+                  "sector_num: %" PRId64 ", nsector: %d\n", io->addr, io->len,
+                  sector_num, nsector);
 
     dma_addr = io->addr;
     dma_len = io->len;
     mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len,
                          DMA_DIRECTION_FROM_DEVICE);
 
-    if (!remainder) {
-        MACIO_DPRINTF("--- DMA read aligned - addr: %" HWADDR_PRIx
-                      " len: %x\n", io->addr, io->len);
-        qemu_iovec_add(&io->iov, mem, io->len);
-    } else {
-        MACIO_DPRINTF("--- DMA read unaligned - addr: %" HWADDR_PRIx
-                      " len: %x\n", io->addr, io->len);
-        qemu_iovec_add(&io->iov, mem, io->len);
+    if (offset & (align - 1)) {
+        head_bytes = offset & (align - 1);
 
-        MACIO_DPRINTF("--- DMA read push    - bounce addr: %p "
-                      "remainder_len: %x\n",
-                      &io->remainder + 0x200 - remainder, remainder);
-        qemu_iovec_add(&io->iov, &io->remainder + 0x200 - remainder,
-                       remainder);
+        MACIO_DPRINTF("--- DMA unaligned head: sector %" PRId64 ", "
+                      "discarding %zu bytes\n", sector_num, head_bytes);
 
-        io->remainder_len = remainder;
+        qemu_iovec_add(&io->iov, &io->remainder, head_bytes);
+
+        bytes += offset & (align - 1);
+        offset = offset & ~(align - 1);
+    }
+
+    qemu_iovec_add(&io->iov, mem, io->len);
+
+    if ((offset + bytes) & (align - 1)) {
+        tail_bytes = (offset + bytes) & (align - 1);
+
+        MACIO_DPRINTF("--- DMA unaligned tail: sector %" PRId64 ", "
+                      "discarding bytes %zu\n", sector_num, tail_bytes);
+
+        qemu_iovec_add(&io->iov, &io->remainder, align - tail_bytes);
+        bytes = ROUND_UP(bytes, align);
     }
 
     s->io_buffer_size -= io->len;
@@ -137,11 +109,11 @@ static void pmac_dma_read(BlockBackend *blk,
 
     io->len = 0;
 
-    MACIO_DPRINTF("--- Block read transfer   - sector_num: %"PRIx64"  "
-                  "nsector: %x\n",
-                  sector_num, nsector);
+    MACIO_DPRINTF("--- Block read transfer - sector_num: %" PRIx64 "  "
+                  "nsector: %x\n", (offset >> 9), (bytes >> 9));
 
-    m->aiocb = blk_aio_readv(blk, sector_num, &io->iov, nsector, cb, io);
+    m->aiocb = blk_aio_readv(blk, (offset >> 9), &io->iov, (bytes >> 9),
+                             cb, io);
 }
 
 static void pmac_dma_write(BlockBackend *blk,
@@ -246,6 +218,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
     IDEState *s = idebus_active_if(&m->bus);
     int64_t sector_num;
     int nsector, remainder;
+    int64_t offset;
 
     MACIO_DPRINTF("\ns is %p\n", s);
     MACIO_DPRINTF("io_buffer_index: %x\n", s->io_buffer_index);
@@ -294,10 +267,13 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
     nsector = (io->len + 0x1ff) >> 9;
     remainder = io->len & 0x1ff;
 
+    /* Calculate current offset */
+    offset = (int64_t)(s->lba << 11) + s->io_buffer_index;
+
     MACIO_DPRINTF("nsector: %d   remainder: %x\n", nsector, remainder);
     MACIO_DPRINTF("sector: %"PRIx64"   %zx\n", sector_num, io->iov.size / 512);
 
-    pmac_dma_read(s->blk, sector_num, nsector, pmac_ide_atapi_transfer_cb, io);
+    pmac_dma_read(s->blk, offset, io->len, pmac_ide_atapi_transfer_cb, io);
     return;
 
 done:
@@ -315,6 +291,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
     IDEState *s = idebus_active_if(&m->bus);
     int64_t sector_num;
     int nsector, remainder;
+    int64_t offset;
 
     MACIO_DPRINTF("pmac_ide_transfer_cb\n");
 
@@ -349,6 +326,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
 
     /* Calculate number of sectors */
     sector_num = ide_get_sector(s) + (s->io_buffer_index >> 9);
+    offset = (ide_get_sector(s) << 9) + s->io_buffer_index;
     nsector = (io->len + 0x1ff) >> 9;
     remainder = io->len & 0x1ff;
 
@@ -359,7 +337,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
 
     switch (s->dma_cmd) {
     case IDE_DMA_READ:
-        pmac_dma_read(s->blk, sector_num, nsector, pmac_ide_transfer_cb, io);
+        pmac_dma_read(s->blk, offset, io->len, pmac_ide_transfer_cb, io);
         break;
     case IDE_DMA_WRITE:
         pmac_dma_write(s->blk, sector_num, nsector, pmac_ide_transfer_cb, io);
-- 
2.1.0

[Qemu-devel] [PULL 10/12] macio: switch pmac_dma_write() over to new offset/len implementation

[John Snow]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

In particular, this fixes a bug whereby chains of overlapping head/tail chains
would incorrectly write over each other's remainder cache. This is the access
pattern used by OS X/Darwin and fixes an issue with a corrupt Darwin
installation in my local tests.

While we are here, rename the DBDMA_io struct property remainder to
head_remainder for clarification.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1433455177-21243-3-git-send-email-mark.cave-ayland@ilande.co.uk
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/macio.c             | 120 +++++++++++++++++++++------------------------
 include/hw/ppc/mac_dbdma.h |   3 +-
 2 files changed, 57 insertions(+), 66 deletions(-)

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 52ee4ac..85e315f 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -86,7 +86,7 @@ static void pmac_dma_read(BlockBackend *blk,
         MACIO_DPRINTF("--- DMA unaligned head: sector %" PRId64 ", "
                       "discarding %zu bytes\n", sector_num, head_bytes);
 
-        qemu_iovec_add(&io->iov, &io->remainder, head_bytes);
+        qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes);
 
         bytes += offset & (align - 1);
         offset = offset & ~(align - 1);
@@ -100,7 +100,7 @@ static void pmac_dma_read(BlockBackend *blk,
         MACIO_DPRINTF("--- DMA unaligned tail: sector %" PRId64 ", "
                       "discarding bytes %zu\n", sector_num, tail_bytes);
 
-        qemu_iovec_add(&io->iov, &io->remainder, align - tail_bytes);
+        qemu_iovec_add(&io->iov, &io->tail_remainder, align - tail_bytes);
         bytes = ROUND_UP(bytes, align);
     }
 
@@ -117,7 +117,7 @@ static void pmac_dma_read(BlockBackend *blk,
 }
 
 static void pmac_dma_write(BlockBackend *blk,
-                         int64_t sector_num, int nb_sectors,
+                         int64_t offset, int bytes,
                          void (*cb)(void *opaque, int ret), void *opaque)
 {
     DBDMA_io *io = opaque;
@@ -125,53 +125,20 @@ static void pmac_dma_write(BlockBackend *blk,
     IDEState *s = idebus_active_if(&m->bus);
     dma_addr_t dma_addr, dma_len;
     void *mem;
-    int nsector, remainder;
-    int extra = 0;
+    int64_t sector_num;
+    int nsector;
+    uint64_t align = BDRV_SECTOR_SIZE;
+    size_t head_bytes, tail_bytes;
+    bool unaligned_head = false, unaligned_tail = false;
 
     qemu_iovec_destroy(&io->iov);
     qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1);
 
-    if (io->remainder_len > 0) {
-        /* Return remainder of request */
-        int transfer = MIN(io->remainder_len, io->len);
-
-        MACIO_DPRINTF("--- processing write remainder %x\n", transfer);
-        cpu_physical_memory_read(io->addr,
-                                 &io->remainder + (0x200 - transfer),
-                                 transfer);
-
-        io->remainder_len -= transfer;
-        io->len -= transfer;
-        io->addr += transfer;
-
-        s->io_buffer_index += transfer;
-        s->io_buffer_size -= transfer;
-
-        if (io->remainder_len != 0) {
-            /* Still waiting for remainder */
-            return;
-        }
-
-        MACIO_DPRINTF("--> prepending bounce buffer with size 0x200\n");
-
-        /* Sector transfer complete - prepend to request */
-        qemu_iovec_add(&io->iov, &io->remainder, 0x200);
-        extra = 1;
-    }
-
-    if (s->drive_kind == IDE_CD) {
-        sector_num = (int64_t)(s->lba << 2) + (s->io_buffer_index >> 9);
-    } else {
-        sector_num = ide_get_sector(s) + (s->io_buffer_index >> 9);
-    }
-
+    sector_num = (offset >> 9);
     nsector = (io->len >> 9);
-    remainder = io->len - (nsector << 9);
 
-    MACIO_DPRINTF("--- DMA write transfer - addr: %" HWADDR_PRIx " len: %x\n",
-                  io->addr, io->len);
-    MACIO_DPRINTF("xxx remainder: %x\n", remainder);
-    MACIO_DPRINTF("xxx sector_num: %"PRIx64"   nsector: %x\n",
+    MACIO_DPRINTF("--- DMA write transfer (0x%" HWADDR_PRIx ",0x%x): "
+                  "sector_num: %" PRId64 ", nsector: %d\n", io->addr, io->len,
                   sector_num, nsector);
 
     dma_addr = io->addr;
@@ -179,36 +146,59 @@ static void pmac_dma_write(BlockBackend *blk,
     mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len,
                          DMA_DIRECTION_TO_DEVICE);
 
-    if (!remainder) {
-        MACIO_DPRINTF("--- DMA write aligned - addr: %" HWADDR_PRIx
-                      " len: %x\n", io->addr, io->len);
+    if (offset & (align - 1)) {
+        head_bytes = offset & (align - 1);
+        sector_num = ((offset & ~(align - 1)) >> 9);
+
+        MACIO_DPRINTF("--- DMA unaligned head: pre-reading head sector %"
+                      PRId64 "\n", sector_num);
+
+        blk_pread(s->blk, (sector_num << 9), &io->head_remainder, align);
+
+        qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes);
         qemu_iovec_add(&io->iov, mem, io->len);
-    } else {
-        /* Write up to last complete sector */
-        MACIO_DPRINTF("--- DMA write unaligned - addr: %" HWADDR_PRIx
-                      " len: %x\n", io->addr, (nsector << 9));
-        qemu_iovec_add(&io->iov, mem, (nsector << 9));
 
-        MACIO_DPRINTF("--- DMA write read    - bounce addr: %p "
-                      "remainder_len: %x\n", &io->remainder, remainder);
-        cpu_physical_memory_read(io->addr + (nsector << 9), &io->remainder,
-                                 remainder);
+        bytes += offset & (align - 1);
+        offset = offset & ~(align - 1);
+
+        unaligned_head = true;
+    }
+
+    if ((offset + bytes) & (align - 1)) {
+        tail_bytes = (offset + bytes) & (align - 1);
+        sector_num = (((offset + bytes) & ~(align - 1)) >> 9);
+
+        MACIO_DPRINTF("--- DMA unaligned tail: pre-reading tail sector %"
+                      PRId64 "\n", sector_num);
 
-        io->remainder_len = 0x200 - remainder;
+        blk_pread(s->blk, (sector_num << 9), &io->tail_remainder, align);
 
-        MACIO_DPRINTF("xxx remainder_len: %x\n", io->remainder_len);
+        if (!unaligned_head) {
+            qemu_iovec_add(&io->iov, mem, io->len);
+        }
+
+        qemu_iovec_add(&io->iov, &io->tail_remainder + tail_bytes,
+                       align - tail_bytes);
+
+        bytes = ROUND_UP(bytes, align);
+
+        unaligned_tail = true;
+    }
+
+    if (!unaligned_head && !unaligned_tail) {
+        qemu_iovec_add(&io->iov, mem, io->len);
     }
 
-    s->io_buffer_size -= ((nsector + extra) << 9);
-    s->io_buffer_index += ((nsector + extra) << 9);
+    s->io_buffer_size -= io->len;
+    s->io_buffer_index += io->len;
 
     io->len = 0;
 
-    MACIO_DPRINTF("--- Block write transfer   - sector_num: %"PRIx64"  "
-                  "nsector: %x\n", sector_num, nsector + extra);
+    MACIO_DPRINTF("--- Block write transfer - sector_num: %" PRIx64 "  "
+                  "nsector: %x\n", (offset >> 9), (bytes >> 9));
 
-    m->aiocb = blk_aio_writev(blk, sector_num, &io->iov, nsector + extra, cb,
-                              io);
+    m->aiocb = blk_aio_writev(blk, (offset >> 9), &io->iov, (bytes >> 9),
+                              cb, io);
 }
 
 static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
@@ -340,7 +330,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
         pmac_dma_read(s->blk, offset, io->len, pmac_ide_transfer_cb, io);
         break;
     case IDE_DMA_WRITE:
-        pmac_dma_write(s->blk, sector_num, nsector, pmac_ide_transfer_cb, io);
+        pmac_dma_write(s->blk, offset, io->len, pmac_ide_transfer_cb, io);
         break;
     case IDE_DMA_TRIM:
         MACIO_DPRINTF("TRIM command issued!");
diff --git a/include/hw/ppc/mac_dbdma.h b/include/hw/ppc/mac_dbdma.h
index c580327..7f247fa 100644
--- a/include/hw/ppc/mac_dbdma.h
+++ b/include/hw/ppc/mac_dbdma.h
@@ -40,7 +40,8 @@ struct DBDMA_io {
     /* DMA is in progress, don't start another one */
     bool processing;
     /* unaligned last sector of a request */
-    uint8_t remainder[0x200];
+    uint8_t head_remainder[0x200];
+    uint8_t tail_remainder[0x200];
     int remainder_len;
     QEMUIOVector iov;
 };
-- 
2.1.0

[Qemu-devel] [PULL 11/12] macio: update comment/constants to reflect the new code

[John Snow]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

With the offset/len functions taking care of all of the alignment mapping
in isolation from the DMA tranasaction, many comments are now unnecessary.
Remove these and tidy up a few constants at the same time.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1433455177-21243-4-git-send-email-mark.cave-ayland@ilande.co.uk
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/macio.c | 46 +++++++++++++---------------------------------
 1 file changed, 13 insertions(+), 33 deletions(-)

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 85e315f..d353bd2 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -51,6 +51,13 @@ static const int debug_macio = 0;
 
 #define MACIO_PAGE_SIZE 4096
 
+/*
+ * Unaligned DMA read/write access functions required for OS X/Darwin which
+ * don't perform DMA transactions on sector boundaries. These functions are
+ * modelled on bdrv_co_do_preadv()/bdrv_co_do_pwritev() and so should be
+ * easy to remove if the unaligned block APIs are ever exposed.
+ */
+
 static void pmac_dma_read(BlockBackend *blk,
                           int64_t offset, unsigned int bytes,
                           void (*cb)(void *opaque, int ret), void *opaque)
@@ -206,20 +213,12 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
     DBDMA_io *io = opaque;
     MACIOIDEState *m = io->opaque;
     IDEState *s = idebus_active_if(&m->bus);
-    int64_t sector_num;
-    int nsector, remainder;
     int64_t offset;
 
-    MACIO_DPRINTF("\ns is %p\n", s);
-    MACIO_DPRINTF("io_buffer_index: %x\n", s->io_buffer_index);
-    MACIO_DPRINTF("io_buffer_size: %x   packet_transfer_size: %x\n",
-                  s->io_buffer_size, s->packet_transfer_size);
-    MACIO_DPRINTF("lba: %x\n", s->lba);
-    MACIO_DPRINTF("io_addr: %" HWADDR_PRIx "  io_len: %x\n", io->addr,
-                  io->len);
+    MACIO_DPRINTF("pmac_ide_atapi_transfer_cb\n");
 
     if (ret < 0) {
-        MACIO_DPRINTF("THERE WAS AN ERROR!  %d\n", ret);
+        MACIO_DPRINTF("DMA error: %d\n", ret);
         ide_atapi_io_error(s, ret);
         goto done;
     }
@@ -233,6 +232,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
     }
 
     if (s->io_buffer_size <= 0) {
+        MACIO_DPRINTF("End of IDE transfer\n");
         ide_atapi_cmd_ok(s);
         m->dma_active = false;
         goto done;
@@ -252,22 +252,13 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
         goto done;
     }
 
-    /* Calculate number of sectors */
-    sector_num = (int64_t)(s->lba << 2) + (s->io_buffer_index >> 9);
-    nsector = (io->len + 0x1ff) >> 9;
-    remainder = io->len & 0x1ff;
-
     /* Calculate current offset */
     offset = (int64_t)(s->lba << 11) + s->io_buffer_index;
 
-    MACIO_DPRINTF("nsector: %d   remainder: %x\n", nsector, remainder);
-    MACIO_DPRINTF("sector: %"PRIx64"   %zx\n", sector_num, io->iov.size / 512);
-
     pmac_dma_read(s->blk, offset, io->len, pmac_ide_atapi_transfer_cb, io);
     return;
 
 done:
-    MACIO_DPRINTF("done DMA\n\n");
     block_acct_done(blk_get_stats(s->blk), &s->acct);
     io->dma_end(opaque);
 
@@ -279,14 +270,12 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
     DBDMA_io *io = opaque;
     MACIOIDEState *m = io->opaque;
     IDEState *s = idebus_active_if(&m->bus);
-    int64_t sector_num;
-    int nsector, remainder;
     int64_t offset;
 
     MACIO_DPRINTF("pmac_ide_transfer_cb\n");
 
     if (ret < 0) {
-        MACIO_DPRINTF("DMA error\n");
+        MACIO_DPRINTF("DMA error: %d\n", ret);
         m->aiocb = NULL;
         ide_dma_error(s);
         io->remainder_len = 0;
@@ -302,7 +291,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
     }
 
     if (s->io_buffer_size <= 0) {
-        MACIO_DPRINTF("end of transfer\n");
+        MACIO_DPRINTF("End of IDE transfer\n");
         s->status = READY_STAT | SEEK_STAT;
         ide_set_irq(s->bus);
         m->dma_active = false;
@@ -315,15 +304,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
     }
 
     /* Calculate number of sectors */
-    sector_num = ide_get_sector(s) + (s->io_buffer_index >> 9);
     offset = (ide_get_sector(s) << 9) + s->io_buffer_index;
-    nsector = (io->len + 0x1ff) >> 9;
-    remainder = io->len & 0x1ff;
-
-    s->nsector -= nsector;
-
-    MACIO_DPRINTF("nsector: %d   remainder: %x\n", nsector, remainder);
-    MACIO_DPRINTF("sector: %"PRIx64"   %x\n", sector_num, nsector);
 
     switch (s->dma_cmd) {
     case IDE_DMA_READ:
@@ -333,7 +314,6 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
         pmac_dma_write(s->blk, offset, io->len, pmac_ide_transfer_cb, io);
         break;
     case IDE_DMA_TRIM:
-        MACIO_DPRINTF("TRIM command issued!");
         break;
     }
 
@@ -537,7 +517,7 @@ static void ide_dbdma_start(IDEDMA *dma, IDEState *s,
     if (s->drive_kind == IDE_CD) {
         s->io_buffer_size = s->packet_transfer_size;
     } else {
-        s->io_buffer_size = s->nsector * 0x200;
+        s->io_buffer_size = s->nsector * BDRV_SECTOR_SIZE;
     }
 
     MACIO_DPRINTF("\n\n------------ IDE transfer\n");
-- 
2.1.0

[Qemu-devel] [PULL 12/12] macio: remove remainder_len DBDMA_io property

[John Snow]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Since the block alignment code is now effectively independent of the DMA
implementation, this variable is no longer required and can be removed.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1433455177-21243-5-git-send-email-mark.cave-ayland@ilande.co.uk
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/macio.c             | 13 -------------
 include/hw/ppc/mac_dbdma.h |  1 -
 2 files changed, 14 deletions(-)

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index d353bd2..dd52d50 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -278,7 +278,6 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
         MACIO_DPRINTF("DMA error: %d\n", ret);
         m->aiocb = NULL;
         ide_dma_error(s);
-        io->remainder_len = 0;
         goto done;
     }
 
@@ -509,9 +508,6 @@ static void ide_dbdma_start(IDEDMA *dma, IDEState *s,
                             BlockCompletionFunc *cb)
 {
     MACIOIDEState *m = container_of(dma, MACIOIDEState, dma);
-    DBDMAState *dbdma = m->dbdma;
-    DBDMA_io *io;
-    int i;
 
     s->io_buffer_index = 0;
     if (s->drive_kind == IDE_CD) {
@@ -526,15 +522,6 @@ static void ide_dbdma_start(IDEDMA *dma, IDEState *s,
     MACIO_DPRINTF("lba: %x    size: %x\n", s->lba, s->io_buffer_size);
     MACIO_DPRINTF("-------------------------\n");
 
-    for (i = 0; i < DBDMA_CHANNELS; i++) {
-        io = &dbdma->channels[i].io;
-
-        if (io->opaque == m) {
-            io->remainder_len = 0;
-        }
-    }
-
-    MACIO_DPRINTF("\n");
     m->dma_active = true;
     DBDMA_kick(m->dbdma);
 }
diff --git a/include/hw/ppc/mac_dbdma.h b/include/hw/ppc/mac_dbdma.h
index 7f247fa..c687021 100644
--- a/include/hw/ppc/mac_dbdma.h
+++ b/include/hw/ppc/mac_dbdma.h
@@ -42,7 +42,6 @@ struct DBDMA_io {
     /* unaligned last sector of a request */
     uint8_t head_remainder[0x200];
     uint8_t tail_remainder[0x200];
-    int remainder_len;
     QEMUIOVector iov;
 };
 
-- 
2.1.0

Re: [Qemu-devel] [PULL 00/12] Ide patches

[Peter Maydell]

On 5 June 2015 at 21:00, John Snow <jsnow@redhat.com> wrote:
> The following changes since commit 42d58e7c6760cb9c55627c28ae538e27dcf2f144:
>
>   Merge remote-tracking branch 'remotes/sstabellini/tags/xen-15-06-02-tag' into staging (2015-06-02 16:47:31 +0100)
>
> are available in the git repository at:
>
>   https://github.com/jnsnow/qemu.git tags/ide-pull-request
>
> for you to fetch changes up to 0ba98885a0e965a17df214ab12b819ef630d8a14:
>
>   macio: remove remainder_len DBDMA_io property (2015-06-04 20:25:39 -0400)
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM