Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging

# gpg: Signature made Fri 14 Nov 2014 11:05:54 GMT using RSA key ID 81AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>"
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>"

* remotes/stefanha/tags/block-pull-request:
  vmdk: Leave bdi intact if -ENOTSUP in vmdk_get_info
  block: Fix max nb_sectors in bdrv_make_zero
  ahci: factor out FIS decomposition from handle_cmd
  ahci: Check cmd_fis[1] more explicitly
  ahci: Reorder error cases in handle_cmd
  ahci: Fix FIS decomposition
  ahci: add is_ncq predicate helper
  ide: Correct handling of malformed/short PRDTs
  ahci: unify sglist preparation
  ide: repair PIO transfers for cases where nsector > 1
  ahci: Fix byte count regression for ATAPI/PIO
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>

block.c

@@ -2790,8 +2790,8 @@ int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
         if (nb_sectors <= 0) {
             return 0;
         }
-        if (nb_sectors > INT_MAX) {
-            nb_sectors = INT_MAX;
+        if (nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
+            nb_sectors = INT_MAX / BDRV_SECTOR_SIZE;
         }
         ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n);
         if (ret < 0) {

block/vmdk.c

@@ -2137,23 +2137,29 @@ static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs)
     return spec_info;
 }
 
+static bool vmdk_extents_type_eq(const VmdkExtent *a, const VmdkExtent *b)
+{
+    return a->flat == b->flat &&
+           a->compressed == b->compressed &&
+           (a->flat || a->cluster_sectors == b->cluster_sectors);
+}
+
 static int vmdk_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
     int i;
     BDRVVmdkState *s = bs->opaque;
     assert(s->num_extents);
-    bdi->needs_compressed_writes = s->extents[0].compressed;
-    if (!s->extents[0].flat) {
-        bdi->cluster_size = s->extents[0].cluster_sectors << BDRV_SECTOR_BITS;
-    }
+
     /* See if we have multiple extents but they have different cases */
     for (i = 1; i < s->num_extents; i++) {
-        if (bdi->needs_compressed_writes != s->extents[i].compressed ||
-            (bdi->cluster_size && bdi->cluster_size !=
-                s->extents[i].cluster_sectors << BDRV_SECTOR_BITS)) {
+        if (!vmdk_extents_type_eq(&s->extents[0], &s->extents[i])) {
             return -ENOTSUP;
         }
     }
+    bdi->needs_compressed_writes = s->extents[0].compressed;
+    if (!s->extents[0].flat) {
+        bdi->cluster_size = s->extents[0].cluster_sectors << BDRV_SECTOR_BITS;
+    }
     return 0;
 }
 

hw/ide/ahci.c

@@ -730,7 +730,8 @@ static int prdt_tbl_entry_size(const AHCI_SG *tbl)
     return (le32_to_cpu(tbl->flags_size) & AHCI_PRDT_SIZE_MASK) + 1;
 }
 
-static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int offset)
+static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist,
+                                int32_t offset)
 {
     AHCICmdHdr *cmd = ad->cur_cmd;
     uint32_t opts = le32_to_cpu(cmd->opts);
@@ -741,13 +742,21 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int offset)
     uint8_t *prdt;
     int i;
     int r = 0;
-    int sum = 0;
+    uint64_t sum = 0;
     int off_idx = -1;
-    int off_pos = -1;
+    int64_t off_pos = -1;
     int tbl_entry_size;
     IDEBus *bus = &ad->port;
     BusState *qbus = BUS(bus);
 
+    /*
+     * Note: AHCI PRDT can describe up to 256GiB. SATA/ATA only support
+     * transactions of up to 32MiB as of ATA8-ACS3 rev 1b, assuming a
+     * 512 byte sector size. We limit the PRDT in this implementation to
+     * a reasonably large 2GiB, which can accommodate the maximum transfer
+     * request for sector sizes up to 32K.
+     */
+
     if (!sglist_alloc_hint) {
         DPRINTF(ad->port_no, "no sg list given by guest: 0x%08x\n", opts);
         return -1;
@@ -782,7 +791,7 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int offset)
         }
         if ((off_idx == -1) || (off_pos < 0) || (off_pos > tbl_entry_size)) {
             DPRINTF(ad->port_no, "%s: Incorrect offset! "
-                            "off_idx: %d, off_pos: %d\n",
+                            "off_idx: %d, off_pos: %"PRId64"\n",
                             __func__, off_idx, off_pos);
             r = -1;
             goto out;
@@ -797,6 +806,13 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int offset)
             /* flags_size is zero-based */
             qemu_sglist_add(sglist, le64_to_cpu(tbl[i].addr),
                             prdt_tbl_entry_size(&tbl[i]));
+            if (sglist->size > INT32_MAX) {
+                error_report("AHCI Physical Region Descriptor Table describes "
+                             "more than 2 GiB.\n");
+                qemu_sglist_destroy(sglist);
+                r = -1;
+                goto out;
+            }
         }
     }
 
@@ -838,6 +854,21 @@ static void ncq_cb(void *opaque, int ret)
     ncq_tfs->used = 0;
 }
 
+static int is_ncq(uint8_t ata_cmd)
+{
+    /* Based on SATA 3.2 section 13.6.3.2 */
+    switch (ata_cmd) {
+    case READ_FPDMA_QUEUED:
+    case WRITE_FPDMA_QUEUED:
+    case NCQ_NON_DATA:
+    case RECEIVE_FPDMA_QUEUED:
+    case SEND_FPDMA_QUEUED:
+        return 1;
+    default:
+        return 0;
+    }
+}
+
 static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
                                 int slot)
 {
@@ -903,84 +934,40 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
                                            ncq_cb, ncq_tfs);
             break;
         default:
-            DPRINTF(port, "error: tried to process non-NCQ command as NCQ\n");
+            if (is_ncq(cmd_fis[2])) {
+                DPRINTF(port,
+                        "error: unsupported NCQ command (0x%02x) received\n",
+                        cmd_fis[2]);
+            } else {
+                DPRINTF(port,
+                        "error: tried to process non-NCQ command as NCQ\n");
+            }
             qemu_sglist_destroy(&ncq_tfs->sglist);
-            break;
     }
 }
 
-static int handle_cmd(AHCIState *s, int port, int slot)
+static void handle_reg_h2d_fis(AHCIState *s, int port,
+                               int slot, uint8_t *cmd_fis)
 {
-    IDEState *ide_state;
-    uint32_t opts;
-    uint64_t tbl_addr;
-    AHCICmdHdr *cmd;
-    uint8_t *cmd_fis;
-    dma_addr_t cmd_len;
-
-    if (s->dev[port].port.ifs[0].status & (BUSY_STAT|DRQ_STAT)) {
-        /* Engine currently busy, try again later */
-        DPRINTF(port, "engine busy\n");
-        return -1;
-    }
-
-    cmd = &((AHCICmdHdr *)s->dev[port].lst)[slot];
-
-    if (!s->dev[port].lst) {
-        DPRINTF(port, "error: lst not given but cmd handled");
-        return -1;
-    }
-
-    /* remember current slot handle for later */
-    s->dev[port].cur_cmd = cmd;
-
-    opts = le32_to_cpu(cmd->opts);
-    tbl_addr = le64_to_cpu(cmd->tbl_addr);
-
-    cmd_len = 0x80;
-    cmd_fis = dma_memory_map(s->as, tbl_addr, &cmd_len,
-                             DMA_DIRECTION_FROM_DEVICE);
-
-    if (!cmd_fis) {
-        DPRINTF(port, "error: guest passed us an invalid cmd fis\n");
-        return -1;
-    }
-
-    /* The device we are working for */
-    ide_state = &s->dev[port].port.ifs[0];
-
-    if (!ide_state->blk) {
-        DPRINTF(port, "error: guest accessed unused port");
-        goto out;
-    }
-
-    debug_print_fis(cmd_fis, 0x90);
-    //debug_print_fis(cmd_fis, (opts & AHCI_CMD_HDR_CMD_FIS_LEN) * 4);
+    IDEState *ide_state = &s->dev[port].port.ifs[0];
+    AHCICmdHdr *cmd = s->dev[port].cur_cmd;
+    uint32_t opts = le32_to_cpu(cmd->opts);
 
-    switch (cmd_fis[0]) {
-        case SATA_FIS_TYPE_REGISTER_H2D:
-            break;
-        default:
-            DPRINTF(port, "unknown command cmd_fis[0]=%02x cmd_fis[1]=%02x "
-                          "cmd_fis[2]=%02x\n", cmd_fis[0], cmd_fis[1],
-                          cmd_fis[2]);
-            goto out;
-            break;
+    if (cmd_fis[1] & 0x0F) {
+        DPRINTF(port, "Port Multiplier not supported."
+                " cmd_fis[0]=%02x cmd_fis[1]=%02x cmd_fis[2]=%02x\n",
+                cmd_fis[0], cmd_fis[1], cmd_fis[2]);
+        return;
     }
 
-    switch (cmd_fis[1]) {
-        case SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER:
-            break;
-        case 0:
-            break;
-        default:
-            DPRINTF(port, "unknown command cmd_fis[0]=%02x cmd_fis[1]=%02x "
-                          "cmd_fis[2]=%02x\n", cmd_fis[0], cmd_fis[1],
-                          cmd_fis[2]);
-            goto out;
-            break;
+    if (cmd_fis[1] & 0x70) {
+        DPRINTF(port, "Reserved flags set in H2D Register FIS."
+                " cmd_fis[0]=%02x cmd_fis[1]=%02x cmd_fis[2]=%02x\n",
+                cmd_fis[0], cmd_fis[1], cmd_fis[2]);
+        return;
     }
 
+    if (!(cmd_fis[1] & SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER)) {
         switch (s->dev[port].port_state) {
         case STATE_RUN:
             if (cmd_fis[15] & ATA_SRST) {
@@ -993,60 +980,44 @@ static int handle_cmd(AHCIState *s, int port, int slot)
             }
             break;
         }
-
-    if (cmd_fis[1] == SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER) {
+        return;
+    }
 
     /* Check for NCQ command */
-        if ((cmd_fis[2] == READ_FPDMA_QUEUED) ||
-            (cmd_fis[2] == WRITE_FPDMA_QUEUED)) {
+    if (is_ncq(cmd_fis[2])) {
         process_ncq_command(s, port, cmd_fis, slot);
-            goto out;
-        }
-
-        /* Decompose the FIS  */
-        ide_state->nsector = (int64_t)((cmd_fis[13] << 8) | cmd_fis[12]);
-        ide_state->feature = cmd_fis[3];
-        if (!ide_state->nsector) {
-            ide_state->nsector = 256;
+        return;
     }
 
-        if (ide_state->drive_kind != IDE_CD) {
-            /*
-             * We set the sector depending on the sector defined in the FIS.
-             * Unfortunately, the spec isn't exactly obvious on this one.
+    /* Decompose the FIS:
+     * AHCI does not interpret FIS packets, it only forwards them.
+     * SATA 1.0 describes how to decode LBA28 and CHS FIS packets.
+     * Later specifications, e.g, SATA 3.2, describe LBA48 FIS packets.
      *
-             * Apparently LBA48 commands set fis bytes 10,9,8,6,5,4 to the
-             * 48 bit sector number. ATA_CMD_READ_DMA_EXT is an example for
-             * such a command.
+     * ATA4 describes sector number for LBA28/CHS commands.
+     * ATA6 describes sector number for LBA48 commands.
+     * ATA8 deprecates CHS fully, describing only LBA28/48.
      *
-             * Non-LBA48 commands however use 7[lower 4 bits],6,5,4 to define a
-             * 28-bit sector number. ATA_CMD_READ_DMA is an example for such
-             * a command.
-             *
-             * Since the spec doesn't explicitly state what each field should
-             * do, I simply assume non-used fields as reserved and OR everything
-             * together, independent of the command.
-             */
-            ide_set_sector(ide_state, ((uint64_t)cmd_fis[10] << 40)
-                                    | ((uint64_t)cmd_fis[9] << 32)
-                                    /* This is used for LBA48 commands */
-                                    | ((uint64_t)cmd_fis[8] << 24)
-                                    /* This is used for non-LBA48 commands */
-                                    | ((uint64_t)(cmd_fis[7] & 0xf) << 24)
-                                    | ((uint64_t)cmd_fis[6] << 16)
-                                    | ((uint64_t)cmd_fis[5] << 8)
-                                    | cmd_fis[4]);
-        }
+     * We dutifully convert the FIS into IDE registers, and allow the
+     * core layer to interpret them as needed. */
+    ide_state->feature = cmd_fis[3];
+    ide_state->sector = cmd_fis[4];      /* LBA 7:0 */
+    ide_state->lcyl = cmd_fis[5];        /* LBA 15:8  */
+    ide_state->hcyl = cmd_fis[6];        /* LBA 23:16 */
+    ide_state->select = cmd_fis[7];      /* LBA 27:24 (LBA28) */
+    ide_state->hob_sector = cmd_fis[8];  /* LBA 31:24 */
+    ide_state->hob_lcyl = cmd_fis[9];    /* LBA 39:32 */
+    ide_state->hob_hcyl = cmd_fis[10];   /* LBA 47:40 */
+    ide_state->hob_feature = cmd_fis[11];
+    ide_state->nsector = (int64_t)((cmd_fis[13] << 8) | cmd_fis[12]);
+    /* 14, 16, 17, 18, 19: Reserved (SATA 1.0) */
+    /* 15: Only valid when UPDATE_COMMAND not set. */
 
     /* Copy the ACMD field (ATAPI packet, if any) from the AHCI command
-         * table to ide_state->io_buffer
-         */
+     * table to ide_state->io_buffer */
     if (opts & AHCI_CMD_ATAPI) {
         memcpy(ide_state->io_buffer, &cmd_fis[AHCI_COMMAND_TABLE_ACMD], 0x10);
-            ide_state->lcyl = 0x14;
-            ide_state->hcyl = 0xeb;
         debug_print_fis(ide_state->io_buffer, 0x10);
-            ide_state->feature = IDE_FEATURE_DMA;
         s->dev[port].done_atapi_packet = false;
         /* XXX send PIO setup FIS */
     }
@@ -1058,6 +1029,62 @@ static int handle_cmd(AHCIState *s, int port, int slot)
 
     /* We're ready to process the command in FIS byte 2. */
     ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
+}
+
+static int handle_cmd(AHCIState *s, int port, int slot)
+{
+    IDEState *ide_state;
+    uint64_t tbl_addr;
+    AHCICmdHdr *cmd;
+    uint8_t *cmd_fis;
+    dma_addr_t cmd_len;
+
+    if (s->dev[port].port.ifs[0].status & (BUSY_STAT|DRQ_STAT)) {
+        /* Engine currently busy, try again later */
+        DPRINTF(port, "engine busy\n");
+        return -1;
+    }
+
+    if (!s->dev[port].lst) {
+        DPRINTF(port, "error: lst not given but cmd handled");
+        return -1;
+    }
+    cmd = &((AHCICmdHdr *)s->dev[port].lst)[slot];
+    /* remember current slot handle for later */
+    s->dev[port].cur_cmd = cmd;
+
+    /* The device we are working for */
+    ide_state = &s->dev[port].port.ifs[0];
+    if (!ide_state->blk) {
+        DPRINTF(port, "error: guest accessed unused port");
+        return -1;
+    }
+
+    tbl_addr = le64_to_cpu(cmd->tbl_addr);
+    cmd_len = 0x80;
+    cmd_fis = dma_memory_map(s->as, tbl_addr, &cmd_len,
+                             DMA_DIRECTION_FROM_DEVICE);
+    if (!cmd_fis) {
+        DPRINTF(port, "error: guest passed us an invalid cmd fis\n");
+        return -1;
+    } else if (cmd_len != 0x80) {
+        ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_HBUS_ERR);
+        DPRINTF(port, "error: dma_memory_map failed: "
+                "(len(%02"PRIx64") != 0x80)\n",
+                cmd_len);
+        goto out;
+    }
+    debug_print_fis(cmd_fis, 0x80);
+
+    switch (cmd_fis[0]) {
+        case SATA_FIS_TYPE_REGISTER_H2D:
+            handle_reg_h2d_fis(s, port, slot, cmd_fis);
+            break;
+        default:
+            DPRINTF(port, "unknown command cmd_fis[0]=%02x cmd_fis[1]=%02x "
+                          "cmd_fis[2]=%02x\n", cmd_fis[0], cmd_fis[1],
+                          cmd_fis[2]);
+            break;
     }
 
 out:
@@ -1089,10 +1116,11 @@ static void ahci_start_transfer(IDEDMA *dma)
     if (is_atapi && !ad->done_atapi_packet) {
         /* already prepopulated iobuffer */
         ad->done_atapi_packet = true;
+        size = 0;
         goto out;
     }
 
-    if (!ahci_populate_sglist(ad, &s->sg, 0)) {
+    if (ahci_dma_prepare_buf(dma, is_write)) {
         has_sglist = 1;
     }
 
@@ -1139,16 +1167,19 @@ static void ahci_start_dma(IDEDMA *dma, IDEState *s,
  * Not currently invoked by PIO R/W chains,
  * which invoke ahci_populate_sglist via ahci_start_transfer.
  */
-static int ahci_dma_prepare_buf(IDEDMA *dma, int is_write)
+static int32_t ahci_dma_prepare_buf(IDEDMA *dma, int is_write)
 {
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
     IDEState *s = &ad->port.ifs[0];
 
-    ahci_populate_sglist(ad, &s->sg, 0);
+    if (ahci_populate_sglist(ad, &s->sg, s->io_buffer_offset) == -1) {
+        DPRINTF(ad->port_no, "ahci_dma_prepare_buf failed.\n");
+        return -1;
+    }
     s->io_buffer_size = s->sg.size;
 
     DPRINTF(ad->port_no, "len=%#x\n", s->io_buffer_size);
-    return s->io_buffer_size != 0;
+    return s->io_buffer_size;
 }
 
 /**

hw/ide/ahci.h

@@ -186,6 +186,9 @@
 
 #define READ_FPDMA_QUEUED                  0x60
 #define WRITE_FPDMA_QUEUED                 0x61
+#define NCQ_NON_DATA                       0x63
+#define RECEIVE_FPDMA_QUEUED               0x65
+#define SEND_FPDMA_QUEUED                  0x64
 
 #define RES_FIS_DSFIS                      0x00
 #define RES_FIS_PSFIS                      0x20

hw/ide/core.c

@@ -592,6 +592,7 @@ static void ide_sector_read_cb(void *opaque, int ret)
 
     ide_set_sector(s, ide_get_sector(s) + n);
     s->nsector -= n;
+    s->io_buffer_offset += 512 * n;
 }
 
 void ide_sector_read(IDEState *s)
@@ -730,10 +731,11 @@ void ide_dma_cb(void *opaque, int ret)
     n = s->nsector;
     s->io_buffer_index = 0;
     s->io_buffer_size = n * 512;
-    if (s->bus->dma->ops->prepare_buf(s->bus->dma, ide_cmd_is_read(s)) == 0) {
+    if (s->bus->dma->ops->prepare_buf(s->bus->dma, ide_cmd_is_read(s)) < 512) {
         /* The PRDs were too short. Reset the Active bit, but don't raise an
          * interrupt. */
         s->status = READY_STAT | SEEK_STAT;
+        dma_buf_commit(s, 0);
         goto eot;
     }
 
@@ -832,6 +834,8 @@ static void ide_sector_write_cb(void *opaque, int ret)
         n = s->req_nb_sectors;
     }
     s->nsector -= n;
+    s->io_buffer_offset += 512 * n;
+
     if (s->nsector == 0) {
         /* no more sectors to write */
         ide_transfer_stop(s);
@@ -1824,6 +1828,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 
     s->status = READY_STAT | BUSY_STAT;
     s->error = 0;
+    s->io_buffer_offset = 0;
 
     complete = ide_cmd_table[val].handler(s, val);
     if (complete) {
@@ -2309,12 +2314,17 @@ static int ide_nop_int(IDEDMA *dma, int x)
     return 0;
 }
 
+static int32_t ide_nop_int32(IDEDMA *dma, int x)
+{
+    return 0;
+}
+
 static void ide_nop_restart(void *opaque, int x, RunState y)
 {
 }
 
 static const IDEDMAOps ide_dma_nop_ops = {
-    .prepare_buf    = ide_nop_int,
+    .prepare_buf    = ide_nop_int32,
     .rw_buf         = ide_nop_int,
     .set_unit       = ide_nop_int,
     .restart_cb     = ide_nop_restart,

hw/ide/internal.h

@@ -322,6 +322,7 @@ typedef void EndTransferFunc(IDEState *);
 typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockCompletionFunc *);
 typedef void DMAVoidFunc(IDEDMA *);
 typedef int DMAIntFunc(IDEDMA *, int);
+typedef int32_t DMAInt32Func(IDEDMA *, int);
 typedef void DMAu32Func(IDEDMA *, uint32_t);
 typedef void DMAStopFunc(IDEDMA *, bool);
 typedef void DMARestartFunc(void *, int, RunState);
@@ -385,7 +386,7 @@ struct IDEState {
     uint8_t cdrom_changed;
     int packet_transfer_size;
     int elementary_transfer_size;
-    int io_buffer_index;
+    int32_t io_buffer_index;
     int lba;
     int cd_sector_size;
     int atapi_dma; /* true if dma is requested for the packet cmd */
@@ -394,8 +395,8 @@ struct IDEState {
     struct iovec iov;
     QEMUIOVector qiov;
     /* ATA DMA state */
-    int io_buffer_offset;
-    int io_buffer_size;
+    int32_t io_buffer_offset;
+    int32_t io_buffer_size;
     QEMUSGList sg;
     /* PIO transfer handling */
     int req_nb_sectors; /* number of sectors per interrupt */
@@ -405,8 +406,8 @@ struct IDEState {
     uint8_t *io_buffer;
     /* PIO save/restore */
     int32_t io_buffer_total_len;
-    int cur_io_buffer_offset;
-    int cur_io_buffer_len;
+    int32_t cur_io_buffer_offset;
+    int32_t cur_io_buffer_len;
     uint8_t end_transfer_fn_idx;
     QEMUTimer *sector_write_timer; /* only used for win2k install hack */
     uint32_t irq_count; /* counts IRQs when using win2k install hack */
@@ -430,7 +431,7 @@ struct IDEState {
 struct IDEDMAOps {
     DMAStartFunc *start_dma;
     DMAVoidFunc *start_transfer;
-    DMAIntFunc *prepare_buf;
+    DMAInt32Func *prepare_buf;
     DMAu32Func *commit_buf;
     DMAIntFunc *rw_buf;
     DMAIntFunc *set_unit;

hw/ide/macio.c

@@ -553,6 +553,11 @@ static int ide_nop_int(IDEDMA *dma, int x)
     return 0;
 }
 
+static int32_t ide_nop_int32(IDEDMA *dma, int x)
+{
+    return 0;
+}
+
 static void ide_nop_restart(void *opaque, int x, RunState y)
 {
 }
@@ -569,7 +574,7 @@ static void ide_dbdma_start(IDEDMA *dma, IDEState *s,
 
 static const IDEDMAOps dbdma_ops = {
     .start_dma      = ide_dbdma_start,
-    .prepare_buf    = ide_nop_int,
+    .prepare_buf    = ide_nop_int32,
     .rw_buf         = ide_nop_int,
     .set_unit       = ide_nop_int,
     .restart_cb     = ide_nop_restart,

hw/ide/pci.c

@@ -28,7 +28,7 @@
 #include <hw/isa/isa.h>
 #include "sysemu/block-backend.h"
 #include "sysemu/dma.h"
-
+#include "qemu/error-report.h"
 #include <hw/ide/pci.h>
 
 #define BMDMA_PAGE_SIZE 4096
@@ -55,8 +55,11 @@ static void bmdma_start_dma(IDEDMA *dma, IDEState *s,
     }
 }
 
-/* return 0 if buffer completed */
-static int bmdma_prepare_buf(IDEDMA *dma, int is_write)
+/**
+ * Return the number of bytes successfully prepared.
+ * -1 on error.
+ */
+static int32_t bmdma_prepare_buf(IDEDMA *dma, int is_write)
 {
     BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
     IDEState *s = bmdma_active_if(bm);
@@ -74,8 +77,9 @@ static int bmdma_prepare_buf(IDEDMA *dma, int is_write)
         if (bm->cur_prd_len == 0) {
             /* end of table (with a fail safe of one page) */
             if (bm->cur_prd_last ||
-                (bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE)
-                return s->io_buffer_size != 0;
+                (bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE) {
+                return s->io_buffer_size;
+            }
             pci_dma_read(pci_dev, bm->cur_addr, &prd, 8);
             bm->cur_addr += 8;
             prd.addr = le32_to_cpu(prd.addr);
@@ -90,12 +94,23 @@ static int bmdma_prepare_buf(IDEDMA *dma, int is_write)
         l = bm->cur_prd_len;
         if (l > 0) {
             qemu_sglist_add(&s->sg, bm->cur_prd_addr, l);
+
+            /* Note: We limit the max transfer to be 2GiB.
+             * This should accommodate the largest ATA transaction
+             * for LBA48 (65,536 sectors) and 32K sector sizes. */
+            if (s->sg.size > INT32_MAX) {
+                error_report("IDE: sglist describes more than 2GiB.\n");
+                break;
+            }
             bm->cur_prd_addr += l;
             bm->cur_prd_len -= l;
             s->io_buffer_size += l;
         }
     }
-    return 1;
+
+    qemu_sglist_destroy(&s->sg);
+    s->io_buffer_size = 0;
+    return -1;
 }
 
 /* return 0 if buffer completed */