Merge remote-tracking branch 'remotes/jnsnow/tags/ide-pull-request' into staging

# gpg: Signature made Fri 18 Sep 2015 15:59:02 BST using RSA key ID AAFC390E
# gpg: Good signature from "John Snow (John Huston) <jsnow@redhat.com>"

* remotes/jnsnow/tags/ide-pull-request:
  ahci: clean up initial d2h semantics
  ahci: remove cmd_fis argument from write_fis_d2h
  ahci: fix signature generation
  ahci: remove dead reset code
  atapi: abort transfers with 0 byte limits
  ide: fix ATAPI command permissions
  ide-test: add cdrom dma test
  ide-test: add cdrom pio test
  qtest/ahci: export generate_pattern
  qtest/ahci: use generate_pattern everywhere
  ide: unify io_buffer_offset increments

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

8 files changed, 342 insertions, 119 deletions

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 44f6e27398..796be15635 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -46,10 +46,9 @@ do { \
 static void check_cmd(AHCIState *s, int port);
 static int handle_cmd(AHCIState *s, int port, uint8_t slot);
 static void ahci_reset_port(AHCIState *s, int port);
-static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis);
+static bool ahci_write_fis_d2h(AHCIDevice *ad);
 static void ahci_init_d2h(AHCIDevice *ad);
 static int ahci_dma_prepare_buf(IDEDMA *dma, int32_t limit);
-static void ahci_commit_buf(IDEDMA *dma, uint32_t tx_bytes);
 static bool ahci_map_clb_address(AHCIDevice *ad);
 static bool ahci_map_fis_address(AHCIDevice *ad);
 static void ahci_unmap_clb_address(AHCIDevice *ad);
@@ -296,7 +295,6 @@ static void  ahci_port_write(AHCIState *s, int port, int offset, uint32_t val)
             if ((pr->cmd & PORT_CMD_FIS_ON) &&
                 !s->dev[port].init_d2h_sent) {
                 ahci_init_d2h(&s->dev[port]);
-                s->dev[port].init_d2h_sent = true;
             }
 
             check_cmd(s, port);
@@ -539,20 +537,33 @@ static void ahci_check_cmd_bh(void *opaque)
 
 static void ahci_init_d2h(AHCIDevice *ad)
 {
-    uint8_t init_fis[20];
     IDEState *ide_state = &ad->port.ifs[0];
+    AHCIPortRegs *pr = &ad->port_regs;
 
-    memset(init_fis, 0, sizeof(init_fis));
-
-    init_fis[4] = 1;
-    init_fis[12] = 1;
+    if (ad->init_d2h_sent) {
+        return;
+    }
 
-    if (ide_state->drive_kind == IDE_CD) {
-        init_fis[5] = ide_state->lcyl;
-        init_fis[6] = ide_state->hcyl;
+    if (ahci_write_fis_d2h(ad)) {
+        ad->init_d2h_sent = true;
+        /* We're emulating receiving the first Reg H2D Fis from the device;
+         * Update the SIG register, but otherwise proceed as normal. */
+        pr->sig = (ide_state->hcyl << 24) |
+            (ide_state->lcyl << 16) |
+            (ide_state->sector << 8) |
+            (ide_state->nsector & 0xFF);
     }
+}
 
-    ahci_write_fis_d2h(ad, init_fis);
+static void ahci_set_signature(AHCIDevice *ad, uint32_t sig)
+{
+    IDEState *s = &ad->port.ifs[0];
+    s->hcyl = sig >> 24 & 0xFF;
+    s->lcyl = sig >> 16 & 0xFF;
+    s->sector = sig >> 8 & 0xFF;
+    s->nsector = sig & 0xFF;
+
+    DPRINTF(ad->port_no, "set hcyl:lcyl:sect:nsect = 0x%08x\n", sig);
 }
 
 static void ahci_reset_port(AHCIState *s, int port)
@@ -603,17 +614,11 @@ static void ahci_reset_port(AHCIState *s, int port)
     }
 
     s->dev[port].port_state = STATE_RUN;
-    if (!ide_state->blk) {
-        pr->sig = 0;
-        ide_state->status = SEEK_STAT | WRERR_STAT;
-    } else if (ide_state->drive_kind == IDE_CD) {
-        pr->sig = SATA_SIGNATURE_CDROM;
-        ide_state->lcyl = 0x14;
-        ide_state->hcyl = 0xeb;
-        DPRINTF(port, "set lcyl = %d\n", ide_state->lcyl);
+    if (ide_state->drive_kind == IDE_CD) {
+        ahci_set_signature(d, SATA_SIGNATURE_CDROM);\
         ide_state->status = SEEK_STAT | WRERR_STAT | READY_STAT;
     } else {
-        pr->sig = SATA_SIGNATURE_DISK;
+        ahci_set_signature(d, SATA_SIGNATURE_DISK);
         ide_state->status = SEEK_STAT | WRERR_STAT;
     }
 
@@ -749,7 +754,7 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
     ahci_trigger_irq(ad->hba, ad, PORT_IRQ_PIOS_FIS);
 }
 
-static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
+static bool ahci_write_fis_d2h(AHCIDevice *ad)
 {
     AHCIPortRegs *pr = &ad->port_regs;
     uint8_t *d2h_fis;
@@ -757,7 +762,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
     IDEState *s = &ad->port.ifs[0];
 
     if (!ad->res_fis || !(pr->cmd & PORT_CMD_FIS_RX)) {
-        return;
+        return false;
     }
 
     d2h_fis = &ad->res_fis[RES_FIS_RFIS];
@@ -790,6 +795,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
     }
 
     ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
+    return true;
 }
 
 static int prdt_tbl_entry_size(const AHCI_SG *tbl)
@@ -1289,7 +1295,7 @@ out:
     s->data_ptr = s->data_end;
 
     /* Update number of transferred bytes, destroy sglist */
-    ahci_commit_buf(dma, size);
+    dma_buf_commit(s, size);
 
     s->end_transfer_func(s);
 
@@ -1331,9 +1337,8 @@ static void ahci_restart(IDEDMA *dma)
 }
 
 /**
- * Called in DMA R/W chains to read the PRDT, utilizing ahci_populate_sglist.
- * Not currently invoked by PIO R/W chains,
- * which invoke ahci_populate_sglist via ahci_start_transfer.
+ * Called in DMA and PIO R/W chains to read the PRDT.
+ * Not shared with NCQ pathways.
  */
 static int32_t ahci_dma_prepare_buf(IDEDMA *dma, int32_t limit)
 {
@@ -1352,21 +1357,16 @@ static int32_t ahci_dma_prepare_buf(IDEDMA *dma, int32_t limit)
 }
 
 /**
- * Destroys the scatter-gather list,
- * and updates the command header with a bytes-read value.
- * called explicitly via ahci_dma_rw_buf (ATAPI DMA),
- * and ahci_start_transfer (PIO R/W),
- * and called via callback from ide_dma_cb for DMA R/W paths.
+ * Updates the command header with a bytes-read value.
+ * Called via dma_buf_commit, for both DMA and PIO paths.
+ * sglist destruction is handled within dma_buf_commit.
  */
 static void ahci_commit_buf(IDEDMA *dma, uint32_t tx_bytes)
 {
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
-    IDEState *s = &ad->port.ifs[0];
 
     tx_bytes += le32_to_cpu(ad->cur_cmd->status);
     ad->cur_cmd->status = cpu_to_le32(tx_bytes);
-
-    qemu_sglist_destroy(&s->sg);
 }
 
 static int ahci_dma_rw_buf(IDEDMA *dma, int is_write)
@@ -1387,10 +1387,9 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_write)
     }
 
     /* free sglist, update byte count */
-    ahci_commit_buf(dma, l);
+    dma_buf_commit(s, l);
 
     s->io_buffer_index += l;
-    s->io_buffer_offset += l;
 
     DPRINTF(ad->port_no, "len=%#x\n", l);
 
@@ -1404,7 +1403,7 @@ static void ahci_cmd_done(IDEDMA *dma)
     DPRINTF(ad->port_no, "cmd done\n");
 
     /* update d2h status */
-    ahci_write_fis_d2h(ad, NULL);
+    ahci_write_fis_d2h(ad);
 
     if (!ad->check_bh) {
         /* maybe we still have something to process, check later */
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 79dd167107..747f46611e 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -1169,20 +1169,28 @@ enum {
      * 4.1.8)
      */
     CHECK_READY = 0x02,
+
+    /*
+     * Commands flagged with NONDATA do not in any circumstances return
+     * any data via ide_atapi_cmd_reply. These commands are exempt from
+     * the normal byte_count_limit constraints.
+     * See ATA8-ACS3 "7.21.5 Byte Count Limit"
+     */
+    NONDATA = 0x04,
 };
 
 static const struct {
     void (*handler)(IDEState *s, uint8_t *buf);
     int flags;
 } atapi_cmd_table[0x100] = {
-    [ 0x00 ] = { cmd_test_unit_ready,               CHECK_READY },
+    [ 0x00 ] = { cmd_test_unit_ready,               CHECK_READY | NONDATA },
     [ 0x03 ] = { cmd_request_sense,                 ALLOW_UA },
     [ 0x12 ] = { cmd_inquiry,                       ALLOW_UA },
-    [ 0x1b ] = { cmd_start_stop_unit,               0 }, /* [1] */
-    [ 0x1e ] = { cmd_prevent_allow_medium_removal,  0 },
+    [ 0x1b ] = { cmd_start_stop_unit,               NONDATA }, /* [1] */
+    [ 0x1e ] = { cmd_prevent_allow_medium_removal,  NONDATA },
     [ 0x25 ] = { cmd_read_cdvd_capacity,            CHECK_READY },
     [ 0x28 ] = { cmd_read, /* (10) */               CHECK_READY },
-    [ 0x2b ] = { cmd_seek,                          CHECK_READY },
+    [ 0x2b ] = { cmd_seek,                          CHECK_READY | NONDATA },
     [ 0x43 ] = { cmd_read_toc_pma_atip,             CHECK_READY },
     [ 0x46 ] = { cmd_get_configuration,             ALLOW_UA },
     [ 0x4a ] = { cmd_get_event_status_notification, ALLOW_UA },
@@ -1190,7 +1198,7 @@ static const struct {
     [ 0x5a ] = { cmd_mode_sense, /* (10) */         0 },
     [ 0xa8 ] = { cmd_read, /* (12) */               CHECK_READY },
     [ 0xad ] = { cmd_read_dvd_structure,            CHECK_READY },
-    [ 0xbb ] = { cmd_set_speed,                     0 },
+    [ 0xbb ] = { cmd_set_speed,                     NONDATA },
     [ 0xbd ] = { cmd_mechanism_status,              0 },
     [ 0xbe ] = { cmd_read_cd,                       CHECK_READY },
     /* [1] handler detects and reports not ready condition itself */
@@ -1251,6 +1259,20 @@ void ide_atapi_cmd(IDEState *s)
         return;
     }
 
+    /* Nondata commands permit the byte_count_limit to be 0.
+     * If this is a data-transferring PIO command and BCL is 0,
+     * we abort at the /ATA/ level, not the ATAPI level.
+     * See ATA8 ACS3 section 7.17.6.49 and 7.21.5 */
+    if (!(atapi_cmd_table[s->io_buffer[0]].flags & NONDATA)) {
+        /* TODO: Check IDENTIFY data word 125 for default BCL (currently 0) */
+        uint16_t byte_count_limit = s->lcyl | (s->hcyl << 8);
+        if (!(byte_count_limit || s->atapi_dma)) {
+            /* TODO: Move abort back into core.c and make static inline again */
+            ide_abort_command(s);
+            return;
+        }
+    }
+
     /* Execute the command */
     if (atapi_cmd_table[s->io_buffer[0]].handler) {
         atapi_cmd_table[s->io_buffer[0]].handler(s, buf);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 50449cae09..317406dca3 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -457,7 +457,7 @@ BlockAIOCB *ide_issue_trim(BlockBackend *blk,
     return &iocb->common;
 }
 
-static inline void ide_abort_command(IDEState *s)
+void ide_abort_command(IDEState *s)
 {
     ide_transfer_stop(s);
     s->status = READY_STAT | ERR_STAT;
@@ -591,7 +591,6 @@ static void ide_sector_read_cb(void *opaque, int ret)
     s->nsector -= n;
     /* Allow the guest to read the io_buffer */
     ide_transfer_start(s, s->io_buffer, n * BDRV_SECTOR_SIZE, ide_sector_read);
-    s->io_buffer_offset += 512 * n;
     ide_set_irq(s->bus);
 }
 
@@ -635,11 +634,12 @@ static void ide_sector_read(IDEState *s)
                                  ide_sector_read_cb, s);
 }
 
-static void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
+void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
 {
     if (s->bus->dma->ops->commit_buf) {
         s->bus->dma->ops->commit_buf(s->bus->dma, tx_bytes);
     }
+    s->io_buffer_offset += tx_bytes;
     qemu_sglist_destroy(&s->sg);
 }
 
@@ -842,7 +842,6 @@ static void ide_sector_write_cb(void *opaque, int ret)
         n = s->req_nb_sectors;
     }
     s->nsector -= n;
-    s->io_buffer_offset += 512 * n;
 
     ide_set_sector(s, ide_get_sector(s) + n);
     if (s->nsector == 0) {
@@ -1747,11 +1746,11 @@ static const struct {
 } ide_cmd_table[0x100] = {
     /* NOP not implemented, mandatory for CD */
     [CFA_REQ_EXT_ERROR_CODE]      = { cmd_cfa_req_ext_error_code, CFA_OK },
-    [WIN_DSM]                     = { cmd_data_set_management, ALL_OK },
+    [WIN_DSM]                     = { cmd_data_set_management, HD_CFA_OK },
     [WIN_DEVICE_RESET]            = { cmd_device_reset, CD_OK },
     [WIN_RECAL]                   = { cmd_nop, HD_CFA_OK | SET_DSC},
     [WIN_READ]                    = { cmd_read_pio, ALL_OK },
-    [WIN_READ_ONCE]               = { cmd_read_pio, ALL_OK },
+    [WIN_READ_ONCE]               = { cmd_read_pio, HD_CFA_OK },
     [WIN_READ_EXT]                = { cmd_read_pio, HD_CFA_OK },
     [WIN_READDMA_EXT]             = { cmd_read_dma, HD_CFA_OK },
     [WIN_READ_NATIVE_MAX_EXT]     = { cmd_read_native_max, HD_CFA_OK | SET_DSC },
@@ -1770,12 +1769,12 @@ static const struct {
     [CFA_TRANSLATE_SECTOR]        = { cmd_cfa_translate_sector, CFA_OK },
     [WIN_DIAGNOSE]                = { cmd_exec_dev_diagnostic, ALL_OK },
     [WIN_SPECIFY]                 = { cmd_nop, HD_CFA_OK | SET_DSC },
-    [WIN_STANDBYNOW2]             = { cmd_nop, ALL_OK },
-    [WIN_IDLEIMMEDIATE2]          = { cmd_nop, ALL_OK },
-    [WIN_STANDBY2]                = { cmd_nop, ALL_OK },
-    [WIN_SETIDLE2]                = { cmd_nop, ALL_OK },
-    [WIN_CHECKPOWERMODE2]         = { cmd_check_power_mode, ALL_OK | SET_DSC },
-    [WIN_SLEEPNOW2]               = { cmd_nop, ALL_OK },
+    [WIN_STANDBYNOW2]             = { cmd_nop, HD_CFA_OK },
+    [WIN_IDLEIMMEDIATE2]          = { cmd_nop, HD_CFA_OK },
+    [WIN_STANDBY2]                = { cmd_nop, HD_CFA_OK },
+    [WIN_SETIDLE2]                = { cmd_nop, HD_CFA_OK },
+    [WIN_CHECKPOWERMODE2]         = { cmd_check_power_mode, HD_CFA_OK | SET_DSC },
+    [WIN_SLEEPNOW2]               = { cmd_nop, HD_CFA_OK },
     [WIN_PACKETCMD]               = { cmd_packet, CD_OK },
     [WIN_PIDENTIFY]               = { cmd_identify_packet, CD_OK },
     [WIN_SMART]                   = { cmd_smart, HD_CFA_OK | SET_DSC },
@@ -1789,19 +1788,19 @@ static const struct {
     [WIN_WRITEDMA]                = { cmd_write_dma, HD_CFA_OK },
     [WIN_WRITEDMA_ONCE]           = { cmd_write_dma, HD_CFA_OK },
     [CFA_WRITE_MULTI_WO_ERASE]    = { cmd_write_multiple, CFA_OK },
-    [WIN_STANDBYNOW1]             = { cmd_nop, ALL_OK },
-    [WIN_IDLEIMMEDIATE]           = { cmd_nop, ALL_OK },
-    [WIN_STANDBY]                 = { cmd_nop, ALL_OK },
-    [WIN_SETIDLE1]                = { cmd_nop, ALL_OK },
-    [WIN_CHECKPOWERMODE1]         = { cmd_check_power_mode, ALL_OK | SET_DSC },
-    [WIN_SLEEPNOW1]               = { cmd_nop, ALL_OK },
+    [WIN_STANDBYNOW1]             = { cmd_nop, HD_CFA_OK },
+    [WIN_IDLEIMMEDIATE]           = { cmd_nop, HD_CFA_OK },
+    [WIN_STANDBY]                 = { cmd_nop, HD_CFA_OK },
+    [WIN_SETIDLE1]                = { cmd_nop, HD_CFA_OK },
+    [WIN_CHECKPOWERMODE1]         = { cmd_check_power_mode, HD_CFA_OK | SET_DSC },
+    [WIN_SLEEPNOW1]               = { cmd_nop, HD_CFA_OK },
     [WIN_FLUSH_CACHE]             = { cmd_flush_cache, ALL_OK },
     [WIN_FLUSH_CACHE_EXT]         = { cmd_flush_cache, HD_CFA_OK },
     [WIN_IDENTIFY]                = { cmd_identify, ALL_OK },
     [WIN_SETFEATURES]             = { cmd_set_features, ALL_OK | SET_DSC },
     [IBM_SENSE_CONDITION]         = { cmd_ibm_sense_condition, CFA_OK | SET_DSC },
     [CFA_WEAR_LEVEL]              = { cmd_cfa_erase_sectors, HD_CFA_OK | SET_DSC },
-    [WIN_READ_NATIVE_MAX]         = { cmd_read_native_max, ALL_OK | SET_DSC },
+    [WIN_READ_NATIVE_MAX]         = { cmd_read_native_max, HD_CFA_OK | SET_DSC },
 };
 
 static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 30fdcbc5fa..05e93ffe3b 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -536,7 +536,9 @@ int64_t ide_get_sector(IDEState *s);
 void ide_set_sector(IDEState *s, int64_t sector_num);
 
 void ide_start_dma(IDEState *s, BlockCompletionFunc *cb);
+void dma_buf_commit(IDEState *s, uint32_t tx_bytes);
 void ide_dma_error(IDEState *s);
+void ide_abort_command(IDEState *s);
 
 void ide_atapi_cmd_ok(IDEState *s);
 void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc);
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 87d7691861..59d387c6d0 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -71,32 +71,6 @@ static void string_bswap16(uint16_t *s, size_t bytes)
     }
 }
 
-static void generate_pattern(void *buffer, size_t len, size_t cycle_len)
-{
-    int i, j;
-    unsigned char *tx = (unsigned char *)buffer;
-    unsigned char p;
-    size_t *sx;
-
-    /* Write an indicative pattern that varies and is unique per-cycle */
-    p = rand() % 256;
-    for (i = j = 0; i < len; i++, j++) {
-        tx[i] = p;
-        if (j % cycle_len == 0) {
-            p = rand() % 256;
-        }
-    }
-
-    /* force uniqueness by writing an id per-cycle */
-    for (i = 0; i < len / cycle_len; i++) {
-        j = i * cycle_len;
-        if (j + sizeof(*sx) <= len) {
-            sx = (size_t *)&tx[j];
-            *sx = i;
-        }
-    }
-}
-
 /**
  * Verify that the transfer did not corrupt our state at all.
  */
@@ -1155,7 +1129,6 @@ static void ahci_migrate_simple(uint8_t cmd_read, uint8_t cmd_write)
     size_t bufsize = 4096;
     unsigned char *tx = g_malloc(bufsize);
     unsigned char *rx = g_malloc0(bufsize);
-    unsigned i;
     const char *uri = "tcp:127.0.0.1:1234";
 
     src = ahci_boot_and_enable("-m 1024 -M q35 "
@@ -1171,9 +1144,7 @@ static void ahci_migrate_simple(uint8_t cmd_read, uint8_t cmd_write)
     ahci_port_clear(src, px);
 
     /* create pattern */
-    for (i = 0; i < bufsize; i++) {
-        tx[i] = (bufsize - i);
-    }
+    generate_pattern(tx, bufsize, AHCI_SECTOR_SIZE);
 
     /* Write, migrate, then read. */
     ahci_io(src, px, cmd_write, tx, bufsize, 0);
@@ -1213,7 +1184,6 @@ static void ahci_halted_io_test(uint8_t cmd_read, uint8_t cmd_write)
     size_t bufsize = 4096;
     unsigned char *tx = g_malloc(bufsize);
     unsigned char *rx = g_malloc0(bufsize);
-    unsigned i;
     uint64_t ptr;
     AHCICommand *cmd;
 
@@ -1231,11 +1201,8 @@ static void ahci_halted_io_test(uint8_t cmd_read, uint8_t cmd_write)
     port = ahci_port_select(ahci);
     ahci_port_clear(ahci, port);
 
-    for (i = 0; i < bufsize; i++) {
-        tx[i] = (bufsize - i);
-    }
-
     /* create DMA source buffer and write pattern */
+    generate_pattern(tx, bufsize, AHCI_SECTOR_SIZE);
     ptr = ahci_alloc(ahci, bufsize);
     g_assert(ptr);
     memwrite(ptr, tx, bufsize);
@@ -1282,7 +1249,6 @@ static void ahci_migrate_halted_io(uint8_t cmd_read, uint8_t cmd_write)
     size_t bufsize = 4096;
     unsigned char *tx = g_malloc(bufsize);
     unsigned char *rx = g_malloc0(bufsize);
-    unsigned i;
     uint64_t ptr;
     AHCICommand *cmd;
     const char *uri = "tcp:127.0.0.1:1234";
@@ -1310,10 +1276,7 @@ static void ahci_migrate_halted_io(uint8_t cmd_read, uint8_t cmd_write)
     /* Initialize and prepare */
     port = ahci_port_select(src);
     ahci_port_clear(src, port);
-
-    for (i = 0; i < bufsize; i++) {
-        tx[i] = (bufsize - i);
-    }
+    generate_pattern(tx, bufsize, AHCI_SECTOR_SIZE);
 
     /* create DMA source buffer and write pattern */
     ptr = ahci_alloc(src, bufsize);
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 4a07e3a9d0..559473812c 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -45,8 +45,15 @@
 #define IDE_BASE 0x1f0
 #define IDE_PRIMARY_IRQ 14
 
+#define ATAPI_BLOCK_SIZE 2048
+
+/* How many bytes to receive via ATAPI PIO at one time.
+ * Must be less than 0xFFFF. */
+#define BYTE_COUNT_LIMIT 5120
+
 enum {
     reg_data        = 0x0,
+    reg_feature     = 0x1,
     reg_nsectors    = 0x2,
     reg_lba_low     = 0x3,
     reg_lba_middle  = 0x4,
@@ -80,6 +87,7 @@ enum {
     CMD_WRITE_DMA   = 0xca,
     CMD_FLUSH_CACHE = 0xe7,
     CMD_IDENTIFY    = 0xec,
+    CMD_PACKET      = 0xa0,
 
     CMDF_ABORT      = 0x100,
     CMDF_NO_BM      = 0x200,
@@ -172,7 +180,8 @@ typedef struct PrdtEntry {
 #define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0)
 
 static int send_dma_request(int cmd, uint64_t sector, int nb_sectors,
-                            PrdtEntry *prdt, int prdt_entries)
+                            PrdtEntry *prdt, int prdt_entries,
+                            void(*post_exec)(uint64_t sector, int nb_sectors))
 {
     QPCIDevice *dev;
     uint16_t bmdma_base;
@@ -189,6 +198,9 @@ static int send_dma_request(int cmd, uint64_t sector, int nb_sectors,
 
     switch (cmd) {
     case CMD_READ_DMA:
+    case CMD_PACKET:
+        /* Assuming we only test data reads w/ ATAPI, otherwise we need to know
+         * the SCSI command being sent in the packet, too. */
         from_dev = true;
         break;
     case CMD_WRITE_DMA:
@@ -217,14 +229,22 @@ static int send_dma_request(int cmd, uint64_t sector, int nb_sectors,
     outl(bmdma_base + bmreg_prdt, guest_prdt);
 
     /* ATA DMA command */
-    outb(IDE_BASE + reg_nsectors, nb_sectors);
-
-    outb(IDE_BASE + reg_lba_low,    sector & 0xff);
-    outb(IDE_BASE + reg_lba_middle, (sector >> 8) & 0xff);
-    outb(IDE_BASE + reg_lba_high,   (sector >> 16) & 0xff);
+    if (cmd == CMD_PACKET) {
+        /* Enables ATAPI DMA; otherwise PIO is attempted */
+        outb(IDE_BASE + reg_feature, 0x01);
+    } else {
+        outb(IDE_BASE + reg_nsectors, nb_sectors);
+        outb(IDE_BASE + reg_lba_low,    sector & 0xff);
+        outb(IDE_BASE + reg_lba_middle, (sector >> 8) & 0xff);
+        outb(IDE_BASE + reg_lba_high,   (sector >> 16) & 0xff);
+    }
 
     outb(IDE_BASE + reg_command, cmd);
 
+    if (post_exec) {
+        post_exec(sector, nb_sectors);
+    }
+
     /* Start DMA transfer */
     outb(bmdma_base + bmreg_cmd, BM_CMD_START | (from_dev ? BM_CMD_WRITE : 0));
 
@@ -278,7 +298,8 @@ static void test_bmdma_simple_rw(void)
     memset(buf, 0x55, len);
     memwrite(guest_buf, buf, len);
 
-    status = send_dma_request(CMD_WRITE_DMA, 0, 1, prdt, ARRAY_SIZE(prdt));
+    status = send_dma_request(CMD_WRITE_DMA, 0, 1, prdt,
+                              ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, BM_STS_INTR);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 
@@ -286,14 +307,15 @@ static void test_bmdma_simple_rw(void)
     memset(buf, 0xaa, len);
     memwrite(guest_buf, buf, len);
 
-    status = send_dma_request(CMD_WRITE_DMA, 1, 1, prdt, ARRAY_SIZE(prdt));
+    status = send_dma_request(CMD_WRITE_DMA, 1, 1, prdt,
+                              ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, BM_STS_INTR);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 
     /* Read and verify 0x55 pattern in sector 0 */
     memset(cmpbuf, 0x55, len);
 
-    status = send_dma_request(CMD_READ_DMA, 0, 1, prdt, ARRAY_SIZE(prdt));
+    status = send_dma_request(CMD_READ_DMA, 0, 1, prdt, ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, BM_STS_INTR);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 
@@ -303,7 +325,7 @@ static void test_bmdma_simple_rw(void)
     /* Read and verify 0xaa pattern in sector 1 */
     memset(cmpbuf, 0xaa, len);
 
-    status = send_dma_request(CMD_READ_DMA, 1, 1, prdt, ARRAY_SIZE(prdt));
+    status = send_dma_request(CMD_READ_DMA, 1, 1, prdt, ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, BM_STS_INTR);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 
@@ -328,13 +350,13 @@ static void test_bmdma_short_prdt(void)
 
     /* Normal request */
     status = send_dma_request(CMD_READ_DMA, 0, 1,
-                              prdt, ARRAY_SIZE(prdt));
+                              prdt, ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, 0);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 
     /* Abort the request before it completes */
     status = send_dma_request(CMD_READ_DMA | CMDF_ABORT, 0, 1,
-                              prdt, ARRAY_SIZE(prdt));
+                              prdt, ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, 0);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 }
@@ -353,13 +375,13 @@ static void test_bmdma_one_sector_short_prdt(void)
 
     /* Normal request */
     status = send_dma_request(CMD_READ_DMA, 0, 2,
-                              prdt, ARRAY_SIZE(prdt));
+                              prdt, ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, 0);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 
     /* Abort the request before it completes */
     status = send_dma_request(CMD_READ_DMA | CMDF_ABORT, 0, 2,
-                              prdt, ARRAY_SIZE(prdt));
+                              prdt, ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, 0);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 }
@@ -377,13 +399,13 @@ static void test_bmdma_long_prdt(void)
 
     /* Normal request */
     status = send_dma_request(CMD_READ_DMA, 0, 1,
-                              prdt, ARRAY_SIZE(prdt));
+                              prdt, ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, BM_STS_ACTIVE | BM_STS_INTR);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 
     /* Abort the request before it completes */
     status = send_dma_request(CMD_READ_DMA | CMDF_ABORT, 0, 1,
-                              prdt, ARRAY_SIZE(prdt));
+                              prdt, ARRAY_SIZE(prdt), NULL);
     g_assert_cmphex(status, ==, BM_STS_INTR);
     assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
 }
@@ -399,7 +421,7 @@ static void test_bmdma_no_busmaster(void)
     PrdtEntry prdt[4096] = { };
 
     status = send_dma_request(CMD_READ_DMA | CMDF_NO_BM, 0, 512,
-                              prdt, ARRAY_SIZE(prdt));
+                              prdt, ARRAY_SIZE(prdt), NULL);
 
     /* Not entirely clear what the expected result is, but this is what we get
      * in practice. At least we want to be aware of any changes. */
@@ -585,6 +607,191 @@ static void test_isa_retry_flush(const char *machine)
     test_retry_flush("isapc");
 }
 
+typedef struct Read10CDB {
+    uint8_t opcode;
+    uint8_t flags;
+    uint32_t lba;
+    uint8_t reserved;
+    uint16_t nblocks;
+    uint8_t control;
+    uint16_t padding;
+} __attribute__((__packed__)) Read10CDB;
+
+static void send_scsi_cdb_read10(uint64_t lba, int nblocks)
+{
+    Read10CDB pkt = { .padding = 0 };
+    int i;
+
+    g_assert_cmpint(lba, <=, UINT32_MAX);
+    g_assert_cmpint(nblocks, <=, UINT16_MAX);
+    g_assert_cmpint(nblocks, >=, 0);
+
+    /* Construct SCSI CDB packet */
+    pkt.opcode = 0x28;
+    pkt.lba = cpu_to_be32(lba);
+    pkt.nblocks = cpu_to_be16(nblocks);
+
+    /* Send Packet */
+    for (i = 0; i < sizeof(Read10CDB)/2; i++) {
+        outw(IDE_BASE + reg_data, ((uint16_t *)&pkt)[i]);
+    }
+}
+
+static void nsleep(int64_t nsecs)
+{
+    const struct timespec val = { .tv_nsec = nsecs };
+    nanosleep(&val, NULL);
+    clock_set(nsecs);
+}
+
+static uint8_t ide_wait_clear(uint8_t flag)
+{
+    int i;
+    uint8_t data;
+
+    /* Wait with a 5 second timeout */
+    for (i = 0; i <= 12500000; i++) {
+        data = inb(IDE_BASE + reg_status);
+        if (!(data & flag)) {
+            return data;
+        }
+        nsleep(400);
+    }
+    g_assert_not_reached();
+}
+
+static void ide_wait_intr(int irq)
+{
+    int i;
+    bool intr;
+
+    for (i = 0; i <= 12500000; i++) {
+        intr = get_irq(irq);
+        if (intr) {
+            return;
+        }
+        nsleep(400);
+    }
+
+    g_assert_not_reached();
+}
+
+static void cdrom_pio_impl(int nblocks)
+{
+    FILE *fh;
+    int patt_blocks = MAX(16, nblocks);
+    size_t patt_len = ATAPI_BLOCK_SIZE * patt_blocks;
+    char *pattern = g_malloc(patt_len);
+    size_t rxsize = ATAPI_BLOCK_SIZE * nblocks;
+    uint16_t *rx = g_malloc0(rxsize);
+    int i, j;
+    uint8_t data;
+    uint16_t limit;
+
+    /* Prepopulate the CDROM with an interesting pattern */
+    generate_pattern(pattern, patt_len, ATAPI_BLOCK_SIZE);
+    fh = fopen(tmp_path, "w+");
+    fwrite(pattern, ATAPI_BLOCK_SIZE, patt_blocks, fh);
+    fclose(fh);
+
+    ide_test_start("-drive if=none,file=%s,media=cdrom,format=raw,id=sr0,index=0 "
+                   "-device ide-cd,drive=sr0,bus=ide.0", tmp_path);
+    qtest_irq_intercept_in(global_qtest, "ioapic");
+
+    /* PACKET command on device 0 */
+    outb(IDE_BASE + reg_device, 0);
+    outb(IDE_BASE + reg_lba_middle, BYTE_COUNT_LIMIT & 0xFF);
+    outb(IDE_BASE + reg_lba_high, (BYTE_COUNT_LIMIT >> 8 & 0xFF));
+    outb(IDE_BASE + reg_command, CMD_PACKET);
+    /* HPD0: Check_Status_A State */
+    nsleep(400);
+    data = ide_wait_clear(BSY);
+    /* HPD1: Send_Packet State */
+    assert_bit_set(data, DRQ | DRDY);
+    assert_bit_clear(data, ERR | DF | BSY);
+
+    /* SCSI CDB (READ10) -- read n*2048 bytes from block 0 */
+    send_scsi_cdb_read10(0, nblocks);
+
+    /* HPD3: INTRQ_Wait */
+    ide_wait_intr(IDE_PRIMARY_IRQ);
+
+    /* HPD2: Check_Status_B */
+    data = ide_wait_clear(BSY);
+    assert_bit_set(data, DRQ | DRDY);
+    assert_bit_clear(data, ERR | DF | BSY);
+
+    /* Read data back: occurs in bursts of 'BYTE_COUNT_LIMIT' bytes.
+     * If BYTE_COUNT_LIMIT is odd, we transfer BYTE_COUNT_LIMIT - 1 bytes.
+     * We allow an odd limit only when the remaining transfer size is
+     * less than BYTE_COUNT_LIMIT. However, SCSI's read10 command can only
+     * request n blocks, so our request size is always even.
+     * For this reason, we assume there is never a hanging byte to fetch. */
+    g_assert(!(rxsize & 1));
+    limit = BYTE_COUNT_LIMIT & ~1;
+    for (i = 0; i < DIV_ROUND_UP(rxsize, limit); i++) {
+        size_t offset = i * (limit / 2);
+        size_t rem = (rxsize / 2) - offset;
+        for (j = 0; j < MIN((limit / 2), rem); j++) {
+            rx[offset + j] = inw(IDE_BASE + reg_data);
+        }
+        ide_wait_intr(IDE_PRIMARY_IRQ);
+    }
+    data = ide_wait_clear(DRQ);
+    assert_bit_set(data, DRDY);
+    assert_bit_clear(data, DRQ | ERR | DF | BSY);
+
+    g_assert_cmpint(memcmp(pattern, rx, rxsize), ==, 0);
+    g_free(pattern);
+    g_free(rx);
+    test_bmdma_teardown();
+}
+
+static void test_cdrom_pio(void)
+{
+    cdrom_pio_impl(1);
+}
+
+static void test_cdrom_pio_large(void)
+{
+    /* Test a few loops of the PIO DRQ mechanism. */
+    cdrom_pio_impl(BYTE_COUNT_LIMIT * 4 / ATAPI_BLOCK_SIZE);
+}
+
+
+static void test_cdrom_dma(void)
+{
+    static const size_t len = ATAPI_BLOCK_SIZE;
+    char *pattern = g_malloc(ATAPI_BLOCK_SIZE * 16);
+    char *rx = g_malloc0(len);
+    uintptr_t guest_buf;
+    PrdtEntry prdt[1];
+    FILE *fh;
+
+    ide_test_start("-drive if=none,file=%s,media=cdrom,format=raw,id=sr0,index=0 "
+                   "-device ide-cd,drive=sr0,bus=ide.0", tmp_path);
+    qtest_irq_intercept_in(global_qtest, "ioapic");
+
+    guest_buf = guest_alloc(guest_malloc, len);
+    prdt[0].addr = cpu_to_le32(guest_buf);
+    prdt[0].size = cpu_to_le32(len | PRDT_EOT);
+
+    generate_pattern(pattern, ATAPI_BLOCK_SIZE * 16, ATAPI_BLOCK_SIZE);
+    fh = fopen(tmp_path, "w+");
+    fwrite(pattern, ATAPI_BLOCK_SIZE, 16, fh);
+    fclose(fh);
+
+    send_dma_request(CMD_PACKET, 0, 1, prdt, 1, send_scsi_cdb_read10);
+
+    /* Read back data from guest memory into local qtest memory */
+    memread(guest_buf, rx, len);
+    g_assert_cmpint(memcmp(pattern, rx, len), ==, 0);
+
+    g_free(pattern);
+    g_free(rx);
+    test_bmdma_teardown();
+}
+
 int main(int argc, char **argv)
 {
     const char *arch = qtest_get_arch();
@@ -628,6 +835,10 @@ int main(int argc, char **argv)
     qtest_add_func("/ide/flush/retry_pci", test_pci_retry_flush);
     qtest_add_func("/ide/flush/retry_isa", test_isa_retry_flush);
 
+    qtest_add_func("/ide/cdrom/pio", test_cdrom_pio);
+    qtest_add_func("/ide/cdrom/pio_large", test_cdrom_pio_large);
+    qtest_add_func("/ide/cdrom/dma", test_cdrom_dma);
+
     ret = g_test_run();
 
     /* Cleanup */
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index fce625b18a..8d7c5a9db8 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -212,3 +212,29 @@ void prepare_blkdebug_script(const char *debug_fn, const char *event)
     ret = fclose(debug_file);
     g_assert(ret == 0);
 }
+
+void generate_pattern(void *buffer, size_t len, size_t cycle_len)
+{
+    int i, j;
+    unsigned char *tx = (unsigned char *)buffer;
+    unsigned char p;
+    size_t *sx;
+
+    /* Write an indicative pattern that varies and is unique per-cycle */
+    p = rand() % 256;
+    for (i = 0; i < len; i++) {
+        tx[i] = p++ % 256;
+        if (i % cycle_len == 0) {
+            p = rand() % 256;
+        }
+    }
+
+    /* force uniqueness by writing an id per-cycle */
+    for (i = 0; i < len / cycle_len; i++) {
+        j = i * cycle_len;
+        if (j + sizeof(*sx) <= len) {
+            sx = (size_t *)&tx[j];
+            *sx = i;
+        }
+    }
+}
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index e1f14ea6fb..492a651f5b 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -24,6 +24,7 @@ void mkqcow2(const char *file, unsigned size_mb);
 void set_context(QOSState *s);
 void migrate(QOSState *from, QOSState *to, const char *uri);
 void prepare_blkdebug_script(const char *debug_fn, const char *event);
+void generate_pattern(void *buffer, size_t len, size_t cycle_len);
 
 static inline uint64_t qmalloc(QOSState *q, size_t bytes)
 {