[Qemu-devel] [PATCH 00/19] qtest/ahci: add dma test

[Qemu-devel] [PATCH 00/19] qtest/ahci: add dma test

[John Snow]

Add a simple DMA r/w test to ahci-test.

Oh, and for the first 18 patches, refactor everything into helpers so
that each ahci_test isn't a thousand lines long.

This patch depends upon the "ahci test preliminary refactoring" series
upstream, which shuffled a lot of libqos and malloc facilities to
support this series.

This patchset is a necessary step in checking in AHCI/DMA migration
tests that I will later use as proof as suitability of enabling the
ICH9 and AHCI migration flags.

~John

John Snow (19):
  libqos/ahci: Add ahci_port_select helper
  libqos/ahci: Add ahci_port_clear helper
  qtest/ahci: rename 'Command' to 'CommandHeader'
  libqos/ahci: Add command header helpers
  libqos/ahci: Add ahci_port_check_error helper
  libqos/ahci: Add ahci_port_check_interrupts helper
  libqos/ahci: Add port_check_nonbusy helper
  libqos/ahci: Add cmd response sanity check helpers
  qtest/ahci: Demagic ahci tests.
  libqos/ahci: Add ide cmd properties
  libqos/ahci: add ahci command functions
  libqos/ahci: add ahci command verify
  libqos/ahci: add ahci command size setters
  libqos/ahci: Add ahci_guest_io
  libqos/ahci: add ahci_io
  libqos/ahci: Add ahci_clean_mem
  qtest/ahci: Add a macro bootup routine
  qtest/ahci: Assert sector size in identify test
  qtest/ahci: Adding simple dma read-write test

 tests/ahci-test.c     | 246 +++++++++-------------
 tests/libqos/ahci.c   | 554 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/ahci.h   | 163 ++++++++++++---
 tests/libqos/malloc.c |   5 +
 tests/libqos/malloc.h |   1 +
 5 files changed, 798 insertions(+), 171 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 01/19] libqos/ahci: Add ahci_port_select helper

[John Snow]

This helper identifies which port of the
AHCI HBA has a device we may run tests on.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 19 ++-----------------
 tests/libqos/ahci.c | 27 +++++++++++++++++++++++++++
 tests/libqos/ahci.h |  1 +
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index fca33d2..c689b62 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -662,7 +662,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     RegH2DFIS fis;
     AHCICommand cmd;
     PRD prd;
-    uint32_t ports, reg, table, data_ptr;
+    uint32_t reg, table, data_ptr;
     uint16_t buff[256];
     unsigned i;
     int rc;
@@ -684,22 +684,7 @@ static void ahci_test_identify(AHCIQState *ahci)
      */
 
     /* Pick the first implemented and running port */
-    ports = ahci_rreg(ahci, AHCI_PI);
-    for (i = 0; i < 32; ports >>= 1, ++i) {
-        if (ports == 0) {
-            i = 32;
-        }
-
-        if (!(ports & 0x01)) {
-            continue;
-        }
-
-        reg = ahci_px_rreg(ahci, i, AHCI_PX_CMD);
-        if (BITSET(reg, AHCI_PX_CMD_ST)) {
-            break;
-        }
-    }
-    g_assert_cmphex(i, <, 32);
+    i = ahci_port_select(ahci);
     g_test_message("Selected port %u for test", i);
 
     /* Clear out this port's interrupts (ignore the init register d2h fis) */
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 3056fb1..8874790 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -267,3 +267,30 @@ void ahci_hba_enable(AHCIQState *ahci)
      * In the future, a small test-case to inspect the Register D2H FIS
      * and clear the initial interrupts might be good. */
 }
+
+/**
+ * Pick the first implemented and running port
+ */
+unsigned ahci_port_select(AHCIQState *ahci)
+{
+    uint32_t ports, reg;
+    unsigned i;
+
+    ports = ahci_rreg(ahci, AHCI_PI);
+    for (i = 0; i < 32; ports >>= 1, ++i) {
+        if (ports == 0) {
+            i = 32;
+        }
+
+        if (!(ports & 0x01)) {
+            continue;
+        }
+
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_CMD);
+        if (BITSET(reg, AHCI_PX_CMD_ST)) {
+            break;
+        }
+    }
+    g_assert(i < 32);
+    return i;
+}
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 77f2055..b3992e1 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -431,5 +431,6 @@ void free_ahci_device(QPCIDevice *dev);
 void ahci_pci_enable(AHCIQState *ahci);
 void start_ahci_device(AHCIQState *ahci);
 void ahci_hba_enable(AHCIQState *ahci);
+unsigned ahci_port_select(AHCIQState *ahci);
 
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH 02/19] libqos/ahci: Add ahci_port_clear helper

[John Snow]

Add a helper that assists in clearing out potentially old error and FIS
information from an AHCI port's data structures. This ensures we always
start with a blank slate for interrupt and FIS receipt information.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   |  9 ++-------
 tests/libqos/ahci.c | 16 ++++++++++++++++
 tests/libqos/ahci.h |  1 +
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index c689b62..90647f2 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -687,13 +687,8 @@ static void ahci_test_identify(AHCIQState *ahci)
     i = ahci_port_select(ahci);
     g_test_message("Selected port %u for test", i);
 
-    /* Clear out this port's interrupts (ignore the init register d2h fis) */
-    reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
-    ahci_px_wreg(ahci, i, AHCI_PX_IS, reg);
-    g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
-
-    /* Wipe the FIS-Receive Buffer */
-    qmemset(ahci->port[i].fb, 0x00, 0x100);
+    /* Clear out the FIS Receive area and any pending interrupts. */
+    ahci_port_clear(ahci, i);
 
     /* Create a Command Table buffer. 0x80 is the smallest with a PRDTL of 0. */
     /* We need at least one PRD, so round up to the nearest 0x80 multiple.    */
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 8874790..9195bd6 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -294,3 +294,19 @@ unsigned ahci_port_select(AHCIQState *ahci)
     g_assert(i < 32);
     return i;
 }
+
+/**
+ * Clear a port's interrupts and status information prior to a test.
+ */
+void ahci_port_clear(AHCIQState *ahci, uint8_t px)
+{
+    uint32_t reg;
+
+    /* Clear out this port's interrupts (ignore the init register d2h fis) */
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_IS);
+    ahci_px_wreg(ahci, px, AHCI_PX_IS, reg);
+    g_assert_cmphex(ahci_px_rreg(ahci, px, AHCI_PX_IS), ==, 0);
+
+    /* Wipe the FIS-Recieve Buffer */
+    qmemset(ahci->port[px].fb, 0x00, 0x100);
+}
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index b3992e1..49ddee5 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -432,5 +432,6 @@ void ahci_pci_enable(AHCIQState *ahci);
 void start_ahci_device(AHCIQState *ahci);
 void ahci_hba_enable(AHCIQState *ahci);
 unsigned ahci_port_select(AHCIQState *ahci);
+void ahci_port_clear(AHCIQState *ahci, uint8_t px);
 
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH 03/19] qtest/ahci: rename 'Command' to 'CommandHeader'

[John Snow]

The structure name is a bit of a misnomer; the structure currently named
command is actually the commandheader. A future patch in this series
will add an actual "Command" structure, so we'll rename it now before the
rest of the functions in this series try to use it.

In addition, rename the "b1" and "b2" fields
to be a unified uint16_t named "flags."

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 6 +++---
 tests/libqos/ahci.h | 7 +++----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 90647f2..85e5761 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -660,7 +660,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     RegD2HFIS *d2h = g_malloc0(0x20);
     RegD2HFIS *pio = g_malloc0(0x20);
     RegH2DFIS fis;
-    AHCICommand cmd;
+    AHCICommandHeader cmd;
     PRD prd;
     uint32_t reg, table, data_ptr;
     uint16_t buff[256];
@@ -703,8 +703,8 @@ static void ahci_test_identify(AHCIQState *ahci)
     /* Copy the existing Command #0 structure from the CLB into local memory,
      * and build a new command #0. */
     memread(ahci->port[i].clb, &cmd, sizeof(cmd));
-    cmd.b1 = 5;    /* reg_h2d_fis is 5 double-words long */
-    cmd.b2 = 0x04; /* clear PxTFD.STS.BSY when done */
+    cmd.flags = 5;    /* reg_h2d_fis is 5 double-words long */
+    cmd.flags |= 0x400; /* clear PxTFD.STS.BSY when done */
     cmd.prdtl = cpu_to_le16(1); /* One PRD table entry. */
     cmd.prdbc = 0;
     cmd.ctba = cpu_to_le32(table);
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 49ddee5..e3ada6e 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -329,15 +329,14 @@ typedef struct RegH2DFIS {
  * Command List entry structure.
  * The command list contains between 1-32 of these structures.
  */
-typedef struct AHCICommand {
-    uint8_t b1;
-    uint8_t b2;
+typedef struct AHCICommandHeader {
+    uint16_t flags; /* Cmd-Fis-Len, PMP#, and flags. */
     uint16_t prdtl; /* Phys Region Desc. Table Length */
     uint32_t prdbc; /* Phys Region Desc. Byte Count */
     uint32_t ctba;  /* Command Table Descriptor Base Address */
     uint32_t ctbau; /*                                    '' Upper */
     uint32_t res[4];
-} __attribute__((__packed__)) AHCICommand;
+} __attribute__((__packed__)) AHCICommandHeader;
 
 /**
  * Physical Region Descriptor; pointed to by the Command List Header,
-- 
1.9.3

[Qemu-devel] [PATCH 04/19] libqos/ahci: Add command header helpers

[John Snow]

Adds command header helper functions:
-ahci_command_header_set
-ahci_command_header_get,
-ahci_command_destroy, and
-ahci_cmd_pick

These helpers help to quickly manage the command header information in
the AHCI device.

ahci_command_header_set and get will store or retrieve an AHCI command
header, respectively.

ahci_cmd_pick chooses the first available but least recently used
command slot to allow us to cycle through the available command slots.

ahci_command_destroy obliterates all information contained within a
given slot's command header, and frees its associated command table,
but not its DMA buffer!

Lastly, the command table pointer fields (dba and dbau) are merged into
a single 64bit value to make managing 64bit tests simpler.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 43 ++++++++++++++++---------------
 tests/libqos/ahci.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/ahci.h | 17 ++++++++----
 3 files changed, 109 insertions(+), 25 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 85e5761..17aee37 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -662,10 +662,12 @@ static void ahci_test_identify(AHCIQState *ahci)
     RegH2DFIS fis;
     AHCICommandHeader cmd;
     PRD prd;
-    uint32_t reg, table, data_ptr;
+    uint32_t reg, data_ptr;
     uint16_t buff[256];
     unsigned i;
     int rc;
+    uint8_t cx;
+    uint64_t table;
 
     g_assert(ahci != NULL);
 
@@ -700,19 +702,19 @@ static void ahci_test_identify(AHCIQState *ahci)
     data_ptr = ahci_alloc(ahci, 512);
     g_assert(data_ptr);
 
-    /* Copy the existing Command #0 structure from the CLB into local memory,
-     * and build a new command #0. */
-    memread(ahci->port[i].clb, &cmd, sizeof(cmd));
-    cmd.flags = 5;    /* reg_h2d_fis is 5 double-words long */
-    cmd.flags |= 0x400; /* clear PxTFD.STS.BSY when done */
-    cmd.prdtl = cpu_to_le16(1); /* One PRD table entry. */
+    /* pick a command slot (should be 0!) */
+    cx = ahci_pick_cmd(ahci, i);
+
+    /* Construct our Command Header (set_command_header handles endianness.) */
+    memset(&cmd, 0x00, sizeof(cmd));
+    cmd.flags = 5;     /* reg_h2d_fis is 5 double-words long */
+    cmd.flags = 0x400; /* clear PxTFD.STS.BSY when done */
+    cmd.prdtl = 1;     /* One PRD table entry. */
     cmd.prdbc = 0;
-    cmd.ctba = cpu_to_le32(table);
-    cmd.ctbau = 0;
+    cmd.ctba = table;
 
     /* Construct our PRD, noting that DBC is 0-indexed. */
-    prd.dba = cpu_to_le32(data_ptr);
-    prd.dbau = 0;
+    prd.dba = cpu_to_le64(data_ptr);
     prd.res = 0;
     /* 511+1 bytes, request DPS interrupt */
     prd.dbc = cpu_to_le32(511 | 0x80000000);
@@ -733,14 +735,15 @@ static void ahci_test_identify(AHCIQState *ahci)
     /* Commit the PRD entry to the Command Table */
     memwrite(table + 0x80, &prd, sizeof(prd));
 
-    /* Commit Command #0, pointing to the Table, to the Command List Buffer. */
-    memwrite(ahci->port[i].clb, &cmd, sizeof(cmd));
+    /* Commit Command #cx, pointing to the Table, to the Command List Buffer. */
+    ahci_set_command_header(ahci, i, cx, &cmd);
 
-    /* Everything is in place, but we haven't given the go-ahead yet. */
+    /* Everything is in place, but we haven't given the go-ahead yet,
+     * so we should find that there are no pending interrupts yet. */
     g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
 
-    /* Issue Command #0 via PxCI */
-    ahci_px_wreg(ahci, i, AHCI_PX_CI, (1 << 0));
+    /* Issue Command #cx via PxCI */
+    ahci_px_wreg(ahci, i, AHCI_PX_CI, (1 << cx));
     while (BITSET(ahci_px_rreg(ahci, i, AHCI_PX_TFD), AHCI_PX_TFD_STS_BSY)) {
         usleep(50);
     }
@@ -764,9 +767,9 @@ static void ahci_test_identify(AHCIQState *ahci)
     ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_ERR);
     ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_ERR);
 
-    /* Investigate CMD #0, assert that we read 512 bytes */
-    memread(ahci->port[i].clb, &cmd, sizeof(cmd));
-    g_assert_cmphex(512, ==, le32_to_cpu(cmd.prdbc));
+    /* Investigate the CMD, assert that we read 512 bytes */
+    ahci_get_command_header(ahci, i, cx, &cmd);
+    g_assert_cmphex(512, ==, cmd.prdbc);
 
     /* Investigate FIS responses */
     memread(ahci->port[i].fb + 0x20, pio, 0x20);
@@ -783,7 +786,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     /* The PIO Setup FIS contains a "bytes read" field, which is a
      * 16-bit value. The Physical Region Descriptor Byte Count is
      * 32-bit, but for small transfers using one PRD, it should match. */
-    g_assert_cmphex(le16_to_cpu(pio->res4), ==, le32_to_cpu(cmd.prdbc));
+    g_assert_cmphex(le16_to_cpu(pio->res4), ==, cmd.prdbc);
 
     /* Last, but not least: Investigate the IDENTIFY response data. */
     memread(data_ptr, &buff, 512);
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 9195bd6..f5e8864 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -310,3 +310,77 @@ void ahci_port_clear(AHCIQState *ahci, uint8_t px)
     /* Wipe the FIS-Recieve Buffer */
     qmemset(ahci->port[px].fb, 0x00, 0x100);
 }
+
+/* Get the #cx'th command of port #px. */
+void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
+                             uint8_t cx, AHCICommandHeader *cmd)
+{
+    uint64_t ba = ahci->port[px].clb;
+    ba += cx * sizeof(AHCICommandHeader);
+    memread(ba, cmd, sizeof(AHCICommandHeader));
+
+    cmd->flags = le16_to_cpu(cmd->flags);
+    cmd->prdtl = le16_to_cpu(cmd->prdtl);
+    cmd->prdbc = le32_to_cpu(cmd->prdbc);
+    cmd->ctba = le64_to_cpu(cmd->ctba);
+}
+
+/* Set the #cx'th command of port #px. */
+void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
+                             uint8_t cx, AHCICommandHeader *cmd)
+{
+    uint64_t ba = ahci->port[px].clb;
+    ba += cx * sizeof(AHCICommandHeader);
+
+    cmd->flags = cpu_to_le16(cmd->flags);
+    cmd->prdtl = cpu_to_le16(cmd->prdtl);
+    cmd->prdbc = cpu_to_le32(cmd->prdbc);
+    cmd->ctba = cpu_to_le64(cmd->ctba);
+
+    memwrite(ba, cmd, sizeof(AHCICommandHeader));
+}
+
+void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx)
+{
+    AHCICommandHeader cmd;
+
+    /* Obtain the Nth Command Header */
+    ahci_get_command_header(ahci, px, cx, &cmd);
+    if (cmd.ctba == 0) {
+        /* No address in it, so just return -- it's empty. */
+        goto tidy;
+    }
+
+    /* Free the Table */
+    ahci_free(ahci, cmd.ctba);
+
+ tidy:
+    /* NULL the header. */
+    memset(&cmd, 0x00, sizeof(cmd));
+    ahci_set_command_header(ahci, px, cx, &cmd);
+    ahci->port[px].ctba[cx] = 0;
+    ahci->port[px].prdtl[cx] = 0;
+}
+
+unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t px)
+{
+    unsigned i;
+    unsigned j;
+    uint32_t reg;
+
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_CI);
+
+    /* Pick the least recently used command slot that's available */
+    for (i = 0; i < 32; ++i) {
+        j = ((ahci->port[px].next + i) % 32);
+        if (reg & (1 << j)) {
+            continue;
+        }
+        ahci_destroy_command(ahci, px, i);
+        ahci->port[px].next = (j + 1) % 32;
+        return j;
+    }
+
+    g_test_message("All command slots were busy.");
+    g_assert_not_reached();
+}
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index e3ada6e..d27e75f 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -248,6 +248,9 @@
 typedef struct AHCIPortQState {
     uint64_t fb;
     uint64_t clb;
+    uint64_t ctba[32];
+    uint16_t prdtl[32];
+    uint8_t next; /** Next Command Slot to Use **/
 } AHCIPortQState;
 
 typedef struct AHCIQState {
@@ -333,8 +336,7 @@ typedef struct AHCICommandHeader {
     uint16_t flags; /* Cmd-Fis-Len, PMP#, and flags. */
     uint16_t prdtl; /* Phys Region Desc. Table Length */
     uint32_t prdbc; /* Phys Region Desc. Byte Count */
-    uint32_t ctba;  /* Command Table Descriptor Base Address */
-    uint32_t ctbau; /*                                    '' Upper */
+    uint64_t ctba;  /* Command Table Descriptor Base Address */
     uint32_t res[4];
 } __attribute__((__packed__)) AHCICommandHeader;
 
@@ -343,11 +345,10 @@ typedef struct AHCICommandHeader {
  * struct ahci_command.
  */
 typedef struct PRD {
-    uint32_t dba;  /* Data Base Address */
-    uint32_t dbau; /* Data Base Address Upper */
+    uint64_t dba;  /* Data Base Address */
     uint32_t res;  /* Reserved */
     uint32_t dbc;  /* Data Byte Count (0-indexed) & Interrupt Flag (bit 2^31) */
-} PRD;
+} __attribute__((__packed__)) PRD;
 
 /*** Macro Utilities ***/
 #define BITANY(data, mask) (((data) & (mask)) != 0)
@@ -432,5 +433,11 @@ void start_ahci_device(AHCIQState *ahci);
 void ahci_hba_enable(AHCIQState *ahci);
 unsigned ahci_port_select(AHCIQState *ahci);
 void ahci_port_clear(AHCIQState *ahci, uint8_t px);
+void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
+                             uint8_t cx, AHCICommandHeader *cmd);
+void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
+                             uint8_t cx, AHCICommandHeader *cmd);
+void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx);
+unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t px);
 
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH 05/19] libqos/ahci: Add ahci_port_check_error helper

[John Snow]

ahci_port_check_error checks a given port's error registers and asserts
that everything from the port-level view is still OK.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   |  8 +-------
 tests/libqos/ahci.c | 22 ++++++++++++++++++++++
 tests/libqos/ahci.h |  1 +
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 17aee37..63ecf73 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -747,6 +747,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     while (BITSET(ahci_px_rreg(ahci, i, AHCI_PX_TFD), AHCI_PX_TFD_STS_BSY)) {
         usleep(50);
     }
+    ahci_port_check_error(ahci, i);
 
     /* Check for expected interrupts */
     reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
@@ -760,13 +761,6 @@ static void ahci_test_identify(AHCIQState *ahci)
                  AHCI_PX_IS_DHRS | AHCI_PX_IS_PSS | AHCI_PX_IS_DPS);
     g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
 
-    /* Check for errors. */
-    reg = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
-    g_assert_cmphex(reg, ==, 0);
-    reg = ahci_px_rreg(ahci, i, AHCI_PX_TFD);
-    ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_ERR);
-    ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_ERR);
-
     /* Investigate the CMD, assert that we read 512 bytes */
     ahci_get_command_header(ahci, i, cx, &cmd);
     g_assert_cmphex(512, ==, cmd.prdbc);
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index f5e8864..35de9af 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -311,6 +311,28 @@ void ahci_port_clear(AHCIQState *ahci, uint8_t px)
     qmemset(ahci->port[px].fb, 0x00, 0x100);
 }
 
+/**
+ * Check a port for errors.
+ */
+void ahci_port_check_error(AHCIQState *ahci, uint8_t px)
+{
+    uint32_t reg;
+
+    /* The upper 9 bits of the IS register all indicate errors. */
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_IS);
+    reg >>= 23;
+    g_assert_cmphex(reg, ==, 0);
+
+    /* The Sata Error Register should be empty. */
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_SERR);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* The TFD also has two error sections. */
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_TFD);
+    ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_ERR);
+    ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_ERR);
+}
+
 /* Get the #cx'th command of port #px. */
 void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
                              uint8_t cx, AHCICommandHeader *cmd)
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index d27e75f..b384cbc 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -433,6 +433,7 @@ void start_ahci_device(AHCIQState *ahci);
 void ahci_hba_enable(AHCIQState *ahci);
 unsigned ahci_port_select(AHCIQState *ahci);
 void ahci_port_clear(AHCIQState *ahci, uint8_t px);
+void ahci_port_check_error(AHCIQState *ahci, uint8_t px);
 void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
                              uint8_t cx, AHCICommandHeader *cmd);
 void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
-- 
1.9.3

[Qemu-devel] [PATCH 06/19] libqos/ahci: Add ahci_port_check_interrupts helper

[John Snow]

A helper that compares a given port's current interrupts and checks them
against a supplied list of expected interrupt bits, and throws an error
if they do not match.

The helper then resets the requested interrupts on this port, and asserts
that the interrupt register is now empty.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 13 ++-----------
 tests/libqos/ahci.c | 14 ++++++++++++++
 tests/libqos/ahci.h |  2 ++
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 63ecf73..e744f44 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -747,19 +747,10 @@ static void ahci_test_identify(AHCIQState *ahci)
     while (BITSET(ahci_px_rreg(ahci, i, AHCI_PX_TFD), AHCI_PX_TFD_STS_BSY)) {
         usleep(50);
     }
+    /* Check registers for post-command consistency */
     ahci_port_check_error(ahci, i);
-
-    /* Check for expected interrupts */
-    reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
-    ASSERT_BIT_SET(reg, AHCI_PX_IS_DHRS);
-    ASSERT_BIT_SET(reg, AHCI_PX_IS_PSS);
     /* BUG: we expect AHCI_PX_IS_DPS to be set. */
-    ASSERT_BIT_CLEAR(reg, AHCI_PX_IS_DPS);
-
-    /* Clear expected interrupts and assert all interrupts now cleared. */
-    ahci_px_wreg(ahci, i, AHCI_PX_IS,
-                 AHCI_PX_IS_DHRS | AHCI_PX_IS_PSS | AHCI_PX_IS_DPS);
-    g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
+    ahci_port_check_interrupts(ahci, i, AHCI_PX_IS_DHRS | AHCI_PX_IS_PSS);
 
     /* Investigate the CMD, assert that we read 512 bytes */
     ahci_get_command_header(ahci, i, cx, &cmd);
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 35de9af..3318a54 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -333,6 +333,20 @@ void ahci_port_check_error(AHCIQState *ahci, uint8_t px)
     ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_ERR);
 }
 
+void ahci_port_check_interrupts(AHCIQState *ahci, uint8_t px,
+                                uint32_t intr_mask)
+{
+    uint32_t reg;
+
+    /* Check for expected interrupts */
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_IS);
+    ASSERT_BIT_SET(reg, intr_mask);
+
+    /* Clear expected interrupts and assert all interrupts now cleared. */
+    ahci_px_wreg(ahci, px, AHCI_PX_IS, intr_mask);
+    g_assert_cmphex(ahci_px_rreg(ahci, px, AHCI_PX_IS), ==, 0);
+}
+
 /* Get the #cx'th command of port #px. */
 void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
                              uint8_t cx, AHCICommandHeader *cmd)
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index b384cbc..4b6696a 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -434,6 +434,8 @@ void ahci_hba_enable(AHCIQState *ahci);
 unsigned ahci_port_select(AHCIQState *ahci);
 void ahci_port_clear(AHCIQState *ahci, uint8_t px);
 void ahci_port_check_error(AHCIQState *ahci, uint8_t px);
+void ahci_port_check_interrupts(AHCIQState *ahci, uint8_t px,
+                                uint32_t intr_mask);
 void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
                              uint8_t cx, AHCICommandHeader *cmd);
 void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
-- 
1.9.3

[Qemu-devel] [PATCH 07/19] libqos/ahci: Add port_check_nonbusy helper

[John Snow]

A simple helper that asserts a given port is not busy processing any
commands via the TFD, Command Issue and SACT registers.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   |  1 +
 tests/libqos/ahci.c | 18 ++++++++++++++++++
 tests/libqos/ahci.h |  1 +
 3 files changed, 20 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index e744f44..77d0f97 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -751,6 +751,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     ahci_port_check_error(ahci, i);
     /* BUG: we expect AHCI_PX_IS_DPS to be set. */
     ahci_port_check_interrupts(ahci, i, AHCI_PX_IS_DHRS | AHCI_PX_IS_PSS);
+    ahci_port_check_nonbusy(ahci, i, cx);
 
     /* Investigate the CMD, assert that we read 512 bytes */
     ahci_get_command_header(ahci, i, cx, &cmd);
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 3318a54..0016aad 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -347,6 +347,24 @@ void ahci_port_check_interrupts(AHCIQState *ahci, uint8_t px,
     g_assert_cmphex(ahci_px_rreg(ahci, px, AHCI_PX_IS), ==, 0);
 }
 
+void ahci_port_check_nonbusy(AHCIQState *ahci, uint8_t px, uint8_t cx)
+{
+    uint32_t reg;
+
+    /* Assert that the command slot is no longer busy (NCQ) */
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_SACT);
+    ASSERT_BIT_CLEAR(reg, (1 << cx));
+
+    /* Non-NCQ */
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_CI);
+    ASSERT_BIT_CLEAR(reg, (1 << cx));
+
+    /* And assert that we are generally not busy. */
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_TFD);
+    ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_BSY);
+    ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_DRQ);
+}
+
 /* Get the #cx'th command of port #px. */
 void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
                              uint8_t cx, AHCICommandHeader *cmd)
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 4b6696a..7c504a5 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -436,6 +436,7 @@ void ahci_port_clear(AHCIQState *ahci, uint8_t px);
 void ahci_port_check_error(AHCIQState *ahci, uint8_t px);
 void ahci_port_check_interrupts(AHCIQState *ahci, uint8_t px,
                                 uint32_t intr_mask);
+void ahci_port_check_nonbusy(AHCIQState *ahci, uint8_t px, uint8_t cx);
 void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
                              uint8_t cx, AHCICommandHeader *cmd);
 void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
-- 
1.9.3

[Qemu-devel] [PATCH 08/19] libqos/ahci: Add cmd response sanity check helpers

[John Snow]

This patch adds a few helpers to help sanity-check the response of the
AHCI device after a command.

ahci_d2h_check_sanity inspects the D2H Register FIS,
ahci_pio_check_sanity inspects the PIO Setup FIS, and
ahci_cmd_check_sanity inspects the command header.

To support the PIO sanity check, a new structure is added for the
PIO Setup FIS type. Existing FIS types (H2D and D2H) have had their
members renamed slightly to condense reserved members into fewer
fields; and LBA fields are now represented by arrays of 8 byte chunks
instead of independent variables.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 29 ++++------------------------
 tests/libqos/ahci.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/ahci.h | 54 ++++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 88 insertions(+), 42 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 77d0f97..b21d4ad 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -657,12 +657,10 @@ static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port)
  */
 static void ahci_test_identify(AHCIQState *ahci)
 {
-    RegD2HFIS *d2h = g_malloc0(0x20);
-    RegD2HFIS *pio = g_malloc0(0x20);
     RegH2DFIS fis;
     AHCICommandHeader cmd;
     PRD prd;
-    uint32_t reg, data_ptr;
+    uint32_t data_ptr;
     uint16_t buff[256];
     unsigned i;
     int rc;
@@ -752,27 +750,11 @@ static void ahci_test_identify(AHCIQState *ahci)
     /* BUG: we expect AHCI_PX_IS_DPS to be set. */
     ahci_port_check_interrupts(ahci, i, AHCI_PX_IS_DHRS | AHCI_PX_IS_PSS);
     ahci_port_check_nonbusy(ahci, i, cx);
-
     /* Investigate the CMD, assert that we read 512 bytes */
-    ahci_get_command_header(ahci, i, cx, &cmd);
-    g_assert_cmphex(512, ==, cmd.prdbc);
-
+    ahci_port_check_cmd_sanity(ahci, i, cx, 512);
     /* Investigate FIS responses */
-    memread(ahci->port[i].fb + 0x20, pio, 0x20);
-    memread(ahci->port[i].fb + 0x40, d2h, 0x20);
-    g_assert_cmphex(pio->fis_type, ==, 0x5f);
-    g_assert_cmphex(d2h->fis_type, ==, 0x34);
-    g_assert_cmphex(pio->flags, ==, d2h->flags);
-    g_assert_cmphex(pio->status, ==, d2h->status);
-    g_assert_cmphex(pio->error, ==, d2h->error);
-
-    reg = ahci_px_rreg(ahci, i, AHCI_PX_TFD);
-    g_assert_cmphex((reg & AHCI_PX_TFD_ERR), ==, pio->error);
-    g_assert_cmphex((reg & AHCI_PX_TFD_STS), ==, pio->status);
-    /* The PIO Setup FIS contains a "bytes read" field, which is a
-     * 16-bit value. The Physical Region Descriptor Byte Count is
-     * 32-bit, but for small transfers using one PRD, it should match. */
-    g_assert_cmphex(le16_to_cpu(pio->res4), ==, cmd.prdbc);
+    ahci_port_check_d2h_sanity(ahci, i, cx);
+    ahci_port_check_pio_sanity(ahci, i, cx, 512);
 
     /* Last, but not least: Investigate the IDENTIFY response data. */
     memread(data_ptr, &buff, 512);
@@ -789,9 +771,6 @@ static void ahci_test_identify(AHCIQState *ahci)
     string_bswap16(&buff[23], 8);
     rc = memcmp(&buff[23], "version ", 8);
     g_assert_cmphex(rc, ==, 0);
-
-    g_free(d2h);
-    g_free(pio);
 }
 
 /******************************************************************************/
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 0016aad..924b9f0 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -365,6 +365,53 @@ void ahci_port_check_nonbusy(AHCIQState *ahci, uint8_t px, uint8_t cx)
     ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_DRQ);
 }
 
+void ahci_port_check_d2h_sanity(AHCIQState *ahci, uint8_t px, uint8_t cx)
+{
+    RegD2HFIS *d2h = g_malloc0(0x20);
+    uint32_t reg;
+
+    memread(ahci->port[px].fb + 0x40, d2h, 0x20);
+    g_assert_cmphex(d2h->fis_type, ==, 0x34);
+
+    reg = ahci_px_rreg(ahci, px, AHCI_PX_TFD);
+    g_assert_cmphex((reg & AHCI_PX_TFD_ERR) >> 8, ==, d2h->error);
+    g_assert_cmphex((reg & AHCI_PX_TFD_STS), ==, d2h->status);
+
+    g_free(d2h);
+}
+
+void ahci_port_check_pio_sanity(AHCIQState *ahci, uint8_t px,
+                                uint8_t cx, size_t buffsize)
+{
+    PIOSetupFIS *pio = g_malloc0(0x20);
+
+    /* We cannot check the Status or E_Status registers, becuase
+     * the status may have again changed between the PIO Setup FIS
+     * and the conclusion of the command with the D2H Register FIS. */
+    memread(ahci->port[px].fb + 0x20, pio, 0x20);
+    g_assert_cmphex(pio->fis_type, ==, 0x5f);
+
+    /* BUG: PIO Setup FIS as utilized by QEMU tries to fit the entire
+     * transfer size in a uint16_t field. The maximum transfer size can
+     * eclipse this; the field is meant to convey the size of data per
+     * each Data FIS, not the entire operation as a whole. For now,
+     * we will sanity check the broken case where applicable. */
+    if (buffsize <= UINT16_MAX) {
+        g_assert_cmphex(le16_to_cpu(pio->tx_count), ==, buffsize);
+    }
+
+    g_free(pio);
+}
+
+void ahci_port_check_cmd_sanity(AHCIQState *ahci, uint8_t px,
+                                uint8_t cx, size_t buffsize)
+{
+    AHCICommandHeader cmd;
+
+    ahci_get_command_header(ahci, px, cx, &cmd);
+    g_assert_cmphex(buffsize, ==, cmd.prdbc);
+}
+
 /* Get the #cx'th command of port #px. */
 void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
                              uint8_t cx, AHCICommandHeader *cmd)
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 7c504a5..3b4e1ef 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -283,25 +283,44 @@ typedef struct RegD2HFIS {
     uint8_t status;
     uint8_t error;
     /* DW1 */
-    uint8_t lba_low;
-    uint8_t lba_mid;
-    uint8_t lba_high;
+    uint8_t lba_lo[3];
     uint8_t device;
     /* DW2 */
-    uint8_t lba3;
-    uint8_t lba4;
-    uint8_t lba5;
-    uint8_t res1;
+    uint8_t lba_hi[3];
+    uint8_t res0;
     /* DW3 */
     uint16_t count;
-    uint8_t res2;
-    uint8_t res3;
+    uint16_t res1;
     /* DW4 */
-    uint16_t res4;
-    uint16_t res5;
+    uint32_t res2;
 } __attribute__((__packed__)) RegD2HFIS;
 
 /**
+ * Register device-to-host FIS structure;
+ * PIO Setup variety.
+ */
+typedef struct PIOSetupFIS {
+    /* DW0 */
+    uint8_t fis_type;
+    uint8_t flags;
+    uint8_t status;
+    uint8_t error;
+    /* DW1 */
+    uint8_t lba_lo[3];
+    uint8_t device;
+    /* DW2 */
+    uint8_t lba_hi[3];
+    uint8_t res0;
+    /* DW3 */
+    uint16_t count;
+    uint8_t res1;
+    uint8_t e_status;
+    /* DW4 */
+    uint16_t tx_count;
+    uint16_t res2;
+} __attribute__((__packed__)) PIOSetupFIS;
+
+/**
  * Register host-to-device FIS structure.
  */
 typedef struct RegH2DFIS {
@@ -311,14 +330,10 @@ typedef struct RegH2DFIS {
     uint8_t command;
     uint8_t feature_low;
     /* DW1 */
-    uint8_t lba_low;
-    uint8_t lba_mid;
-    uint8_t lba_high;
+    uint8_t lba_lo[3];
     uint8_t device;
     /* DW2 */
-    uint8_t lba3;
-    uint8_t lba4;
-    uint8_t lba5;
+    uint8_t lba_hi[3];
     uint8_t feature_high;
     /* DW3 */
     uint16_t count;
@@ -437,6 +452,11 @@ void ahci_port_check_error(AHCIQState *ahci, uint8_t px);
 void ahci_port_check_interrupts(AHCIQState *ahci, uint8_t px,
                                 uint32_t intr_mask);
 void ahci_port_check_nonbusy(AHCIQState *ahci, uint8_t px, uint8_t cx);
+void ahci_port_check_d2h_sanity(AHCIQState *ahci, uint8_t px, uint8_t cx);
+void ahci_port_check_pio_sanity(AHCIQState *ahci, uint8_t px,
+                                uint8_t cx, size_t buffsize);
+void ahci_port_check_cmd_sanity(AHCIQState *ahci, uint8_t px,
+                                uint8_t cx, size_t buffsize);
 void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
                              uint8_t cx, AHCICommandHeader *cmd);
 void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
-- 
1.9.3

[Qemu-devel] [PATCH 09/19] qtest/ahci: Demagic ahci tests.

[John Snow]

Add human-readable command names and other miscellaneous #defines
to help make the code more readable.

Some of these definitions are not yet used in this current series,
but for convenience and sanity they have been lumped together here,
as it's more trouble than it is worth in a test suite to hand-pick,
one-by-one, which preprocessor definitions are useful per-each test.

These definitions include:

ATA Command Mnemonics
Current expected AHCI sector size
FIS magic bytes
REG_H2D_FIS flags
Command Header flags

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 12 ++++++------
 tests/libqos/ahci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index b21d4ad..c862525 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -705,9 +705,9 @@ static void ahci_test_identify(AHCIQState *ahci)
 
     /* Construct our Command Header (set_command_header handles endianness.) */
     memset(&cmd, 0x00, sizeof(cmd));
-    cmd.flags = 5;     /* reg_h2d_fis is 5 double-words long */
-    cmd.flags = 0x400; /* clear PxTFD.STS.BSY when done */
-    cmd.prdtl = 1;     /* One PRD table entry. */
+    cmd.flags = 5;            /* reg_h2d_fis is 5 double-words long */
+    cmd.flags = CMDH_CLR_BSY; /* clear PxTFD.STS.BSY when done */
+    cmd.prdtl = 1;            /* One PRD table entry. */
     cmd.prdbc = 0;
     cmd.ctba = table;
 
@@ -719,10 +719,10 @@ static void ahci_test_identify(AHCIQState *ahci)
 
     /* Construct our Command FIS, Based on http://wiki.osdev.org/AHCI */
     memset(&fis, 0x00, sizeof(fis));
-    fis.fis_type = 0x27; /* Register Host-to-Device FIS */
-    fis.command = 0xEC;  /* IDENTIFY */
+    fis.fis_type = REG_H2D_FIS;  /* Register Host-to-Device FIS */
+    fis.command = CMD_IDENTIFY;
     fis.device = 0;
-    fis.flags = 0x80;    /* Indicate this is a command FIS */
+    fis.flags = REG_H2D_FIS_CMD; /* Indicate this is a command FIS */
 
     /* We've committed nothing yet, no interrupts should be posted yet. */
     g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 3b4e1ef..0d12582 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -243,6 +243,59 @@
 #define AHCI_VERSION_1_2         (0x00010200)
 #define AHCI_VERSION_1_3         (0x00010300)
 
+#define AHCI_SECTOR_SIZE                (512)
+
+/* FIS types */
+enum {
+    REG_H2D_FIS = 0x27,
+    REG_D2H_FIS = 0x34,
+    DMA_ACTIVATE_FIS = 0x39,
+    DMA_SETUP_FIS = 0x41,
+    DATA_FIS = 0x46,
+    BIST_ACTIVATE_FIS = 0x58,
+    PIO_SETUP_FIS = 0x5F,
+    SDB_FIS = 0xA1
+};
+
+/* FIS flags */
+#define REG_H2D_FIS_CMD  0x80
+
+/* ATA Commands */
+enum {
+    /* DMA */
+    CMD_READ_DMA      = 0xC8,
+    CMD_READ_DMA_EXT  = 0x25,
+    CMD_WRITE_DMA     = 0xCA,
+    CMD_WRITE_DMA_EXT = 0x35,
+    /* PIO */
+    CMD_READ_PIO      = 0x20,
+    CMD_READ_PIO_EXT  = 0x24,
+    CMD_WRITE_PIO     = 0x30,
+    CMD_WRITE_PIO_EXT = 0x34,
+    /* Misc */
+    CMD_READ_MAX      = 0xF8,
+    CMD_READ_MAX_EXT  = 0x27,
+    CMD_FLUSH_CACHE   = 0xE7,
+    CMD_IDENTIFY      = 0xEC
+};
+
+/* AHCI Command Header Flags & Masks*/
+#define CMDH_CFL        (0x1F)
+#define CMDH_ATAPI      (0x20)
+#define CMDH_WRITE      (0x40)
+#define CMDH_PREFETCH   (0x80)
+#define CMDH_RESET     (0x100)
+#define CMDH_BIST      (0x200)
+#define CMDH_CLR_BSY   (0x400)
+#define CMDH_RES       (0x800)
+#define CMDH_PMP      (0xF000)
+
+/* ATA device register masks */
+#define ATA_DEVICE_MAGIC 0xA0
+#define ATA_DEVICE_LBA   0x40
+#define ATA_DEVICE_DRIVE 0x10
+#define ATA_DEVICE_HEAD  0x0F
+
 /*** Structures ***/
 
 typedef struct AHCIPortQState {
-- 
1.9.3

[Qemu-devel] [PATCH 10/19] libqos/ahci: Add ide cmd properties

[John Snow]

Add a structure that defines some properties of various IDE commands.
These will be used to simplify the interface to the libqos AHCI calls,
lessening the redundancy of specifying and respecifying properties of
commands to various helper functions.

The "Invalid Command Sentinel" here that caps the property array is an
invalid ATA command, namely 0x01. 0x00 is NOP and 0xFF is reserved for
vendor usage, so I chose the first invalid one instead.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/ahci.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 924b9f0..f0e2a27 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -34,6 +34,48 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/pci/pci_regs.h"
 
+typedef struct AHCICommandProp {
+    uint8_t  cmd;        /* Command Code */
+    bool     data;       /* Data transfer command? */
+    bool     pio;
+    bool     dma;
+    bool     lba28;
+    bool     lba48;
+    bool     read;
+    bool     write;
+    bool     atapi;
+    bool     ncq;
+    uint64_t size;       /* Static transfer size, for commands like IDENTIFY. */
+    uint32_t interrupts; /* Expected interrupts for this command. */
+} AHCICommandProp;
+
+#define CMD_INVALID_SENTINEL 0x01
+
+AHCICommandProp ahci_command_properties[] = {
+    { .cmd = CMD_READ_PIO,      .data = true,  .pio = true,
+                                .lba28 = true, .read = true },
+    { .cmd = CMD_WRITE_PIO,     .data = true,  .pio = true,
+                                .lba28 = true, .write = true },
+    { .cmd = CMD_READ_PIO_EXT,  .data = true,  .pio = true,
+                                .lba48 = true, .read = true },
+    { .cmd = CMD_WRITE_PIO_EXT, .data = true,  .pio = true,
+                                .lba48 = true, .write = true },
+    { .cmd = CMD_READ_DMA,      .data = true,  .dma = true,
+                                .lba28 = true, .read = true },
+    { .cmd = CMD_WRITE_DMA,     .data = true,  .dma = true,
+                                .lba28 = true, .write = true },
+    { .cmd = CMD_READ_DMA_EXT,  .data = true,  .dma = true,
+                                .lba48 = true, .read = true },
+    { .cmd = CMD_WRITE_DMA_EXT, .data = true,  .dma = true,
+                                .lba48 = true, .write = true },
+    { .cmd = CMD_IDENTIFY,      .data = true,  .pio = true,
+                                .size = 512,   .read = true },
+    { .cmd = CMD_READ_MAX,      .lba28 = true },
+    { .cmd = CMD_READ_MAX_EXT,  .lba48 = true },
+    { .cmd = CMD_FLUSH_CACHE,   .data = false },
+    { .cmd = CMD_INVALID_SENTINEL }
+};
+
 /**
  * Allocate space in the guest using information in the AHCIQState object.
  */
-- 
1.9.3

[Qemu-devel] [PATCH 11/19] libqos/ahci: add ahci command functions

[John Snow]

This patch adds the AHCICommand structure, and a set of functions to
operate on the structure.

ahci_command_create - Initialize and create a new AHCICommand in memory
ahci_command_free - Destroy this object.
ahci_command_set_buffer - Set where the guest memory DMA buffer is.
ahci_command_commit - Write this command to the AHCI HBA.
ahci_command_issue - Issue the committed command synchronously.
ahci_command_issue_async - Issue the committed command asynchronously.
ahci_command_wait - Wait for an asynchronous command to finish.
ahci_command_slot - Get the number of the command slot we committed to.

Helpers:
size_to_prdtl       - Calculate the required minimum PRDTL size from
                      a buffer size.
ahci_command_find   - Given an ATA command mnemonic, look it up in the
                      properties table to obtain info about the command.
command_header_init - Initialize the command header with sane values.
command_table_init  - Initialize the command table with sane values.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   |  73 +++++--------------
 tests/libqos/ahci.c | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/ahci.h |  15 ++++
 3 files changed, 234 insertions(+), 56 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index c862525..0834020 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -657,30 +657,28 @@ static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port)
  */
 static void ahci_test_identify(AHCIQState *ahci)
 {
-    RegH2DFIS fis;
-    AHCICommandHeader cmd;
-    PRD prd;
     uint32_t data_ptr;
     uint16_t buff[256];
     unsigned i;
     int rc;
+    AHCICommand *cmd;
     uint8_t cx;
-    uint64_t table;
 
     g_assert(ahci != NULL);
 
     /* We need to:
-     * (1) Create a Command Table Buffer and update the Command List Slot #0
-     *     to point to this buffer.
-     * (2) Construct an FIS host-to-device command structure, and write it to
+     * (1) Create a data buffer for the IDENTIFY response to be sent to,
+     * (2) Create a Command Table Buffer
+     * (3) Construct an FIS host-to-device command structure, and write it to
      *     the top of the command table buffer.
-     * (3) Create a data buffer for the IDENTIFY response to be sent to
      * (4) Create a Physical Region Descriptor that points to the data buffer,
      *     and write it to the bottom (offset 0x80) of the command table.
-     * (5) Now, PxCLB points to the command list, command 0 points to
+     * (5) Obtain a Command List slot, and update this header to point to
+     *     the Command Table we built above.
+     * (6) Now, PxCLB points to the command list, command 0 points to
      *     our table, and our table contains an FIS instruction and a
      *     PRD that points to our rx buffer.
-     * (6) We inform the HBA via PxCI that there is a command ready in slot #0.
+     * (7) We inform the HBA via PxCI that there is a command ready in slot #0.
      */
 
     /* Pick the first implemented and running port */
@@ -690,61 +688,24 @@ static void ahci_test_identify(AHCIQState *ahci)
     /* Clear out the FIS Receive area and any pending interrupts. */
     ahci_port_clear(ahci, i);
 
-    /* Create a Command Table buffer. 0x80 is the smallest with a PRDTL of 0. */
-    /* We need at least one PRD, so round up to the nearest 0x80 multiple.    */
-    table = ahci_alloc(ahci, CMD_TBL_SIZ(1));
-    g_assert(table);
-    ASSERT_BIT_CLEAR(table, 0x7F);
-
-    /* Create a data buffer ... where we will dump the IDENTIFY data to. */
+    /* Create a data buffer where we will dump the IDENTIFY data to. */
     data_ptr = ahci_alloc(ahci, 512);
     g_assert(data_ptr);
 
-    /* pick a command slot (should be 0!) */
-    cx = ahci_pick_cmd(ahci, i);
-
-    /* Construct our Command Header (set_command_header handles endianness.) */
-    memset(&cmd, 0x00, sizeof(cmd));
-    cmd.flags = 5;            /* reg_h2d_fis is 5 double-words long */
-    cmd.flags = CMDH_CLR_BSY; /* clear PxTFD.STS.BSY when done */
-    cmd.prdtl = 1;            /* One PRD table entry. */
-    cmd.prdbc = 0;
-    cmd.ctba = table;
-
-    /* Construct our PRD, noting that DBC is 0-indexed. */
-    prd.dba = cpu_to_le64(data_ptr);
-    prd.res = 0;
-    /* 511+1 bytes, request DPS interrupt */
-    prd.dbc = cpu_to_le32(511 | 0x80000000);
-
-    /* Construct our Command FIS, Based on http://wiki.osdev.org/AHCI */
-    memset(&fis, 0x00, sizeof(fis));
-    fis.fis_type = REG_H2D_FIS;  /* Register Host-to-Device FIS */
-    fis.command = CMD_IDENTIFY;
-    fis.device = 0;
-    fis.flags = REG_H2D_FIS_CMD; /* Indicate this is a command FIS */
-
-    /* We've committed nothing yet, no interrupts should be posted yet. */
-    g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
-
-    /* Commit the Command FIS to the Command Table */
-    memwrite(table, &fis, sizeof(fis));
-
-    /* Commit the PRD entry to the Command Table */
-    memwrite(table + 0x80, &prd, sizeof(prd));
-
-    /* Commit Command #cx, pointing to the Table, to the Command List Buffer. */
-    ahci_set_command_header(ahci, i, cx, &cmd);
+    /* Construct the Command Table (FIS and PRDT) and Command Header */
+    cmd = ahci_command_create(CMD_IDENTIFY);
+    ahci_command_set_buffer(cmd, data_ptr);
+    /* Write the command header and PRDT to guest memory */
+    ahci_command_commit(ahci, cmd, i);
 
     /* Everything is in place, but we haven't given the go-ahead yet,
      * so we should find that there are no pending interrupts yet. */
     g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
 
     /* Issue Command #cx via PxCI */
-    ahci_px_wreg(ahci, i, AHCI_PX_CI, (1 << cx));
-    while (BITSET(ahci_px_rreg(ahci, i, AHCI_PX_TFD), AHCI_PX_TFD_STS_BSY)) {
-        usleep(50);
-    }
+    ahci_command_issue(ahci, cmd);
+    cx = ahci_command_slot(cmd);
+
     /* Check registers for post-command consistency */
     ahci_port_check_error(ahci, i);
     /* BUG: we expect AHCI_PX_IS_DPS to be set. */
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index f0e2a27..87cc9dc 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -527,3 +527,205 @@ unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t px)
     g_test_message("All command slots were busy.");
     g_assert_not_reached();
 }
+
+inline unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
+{
+    /* Each PRD can describe up to 4MiB */
+    g_assert_cmphex(bytes_per_prd, <=, 4096 * 1024);
+    g_assert_cmphex(bytes_per_prd & 0x01, ==, 0x00);
+    return (bytes + bytes_per_prd - 1) / bytes_per_prd;
+}
+
+typedef struct AHCICommand {
+    /* Test Management Data */
+    uint8_t name;
+    uint8_t px;
+    uint8_t cx;
+    uint32_t interrupts;
+    uint64_t xbytes;
+    uint32_t prd_size;
+    uint64_t buffer;
+    AHCICommandProp *props;
+    /* Data to be transferred to the guest */
+    AHCICommandHeader header;
+    RegH2DFIS fis;
+    void *atapi_cmd;
+} AHCICommand;
+
+static AHCICommandProp *ahci_command_find(uint8_t command_name)
+{
+    int i;
+
+    for (i = 0; ; i++) {
+        if (ahci_command_properties[i].cmd == CMD_INVALID_SENTINEL) {
+            return NULL;
+        } else if (ahci_command_properties[i].cmd == command_name) {
+            return &ahci_command_properties[i];
+        }
+    }
+}
+
+/**
+ * Initializes a basic command header in memory.
+ * We assume that this is for an ATA command using RegH2DFIS.
+ */
+static void command_header_init(AHCICommand *cmd)
+{
+    AHCICommandHeader *hdr = &cmd->header;
+    AHCICommandProp *props = cmd->props;
+
+    hdr->flags = 5;             /* RegH2DFIS is 5 DW long. Must be < 32 */
+    hdr->flags |= CMDH_CLR_BSY; /* Clear the BSY bit when done */
+    if (props->write) {
+        hdr->flags |= CMDH_WRITE;
+    }
+    if (props->atapi) {
+        hdr->flags |= CMDH_ATAPI;
+    }
+    /* Other flags: PREFETCH, RESET, and BIST */
+    hdr->prdtl = size_to_prdtl(cmd->xbytes, cmd->prd_size);
+    hdr->prdbc = 0;
+    hdr->ctba = 0;
+}
+
+static void command_table_init(AHCICommand *cmd)
+{
+    RegH2DFIS *fis = &(cmd->fis);
+
+    fis->fis_type = REG_H2D_FIS;
+    fis->flags = REG_H2D_FIS_CMD; /* "Command" bit */
+    fis->command = cmd->name;
+    cmd->fis.feature_low = 0x00;
+    cmd->fis.feature_high = 0x00;
+    if (cmd->props->lba28 || cmd->props->lba48) {
+        cmd->fis.device = ATA_DEVICE_LBA;
+    }
+    cmd->fis.count = cpu_to_le16(cmd->xbytes / AHCI_SECTOR_SIZE);
+    cmd->fis.icc = 0x00;
+    cmd->fis.control = 0x00;
+    cmd->fis.aux = 0x00;
+}
+
+AHCICommand *ahci_command_create(uint8_t command_name)
+{
+    AHCICommandProp *props = ahci_command_find(command_name);
+    AHCICommand *cmd;
+
+    g_assert(props);
+    cmd = g_malloc0(sizeof(AHCICommand));
+    g_assert(!(props->dma && props->pio));
+    g_assert(!(props->lba28 && props->lba48));
+    g_assert(!(props->read && props->write));
+    g_assert(!props->size || props->data);
+
+    /* Defaults and book-keeping */
+    cmd->props = props;
+    cmd->name = command_name;
+    cmd->xbytes = props->size;
+    cmd->prd_size = 4096;
+    cmd->buffer = 0xabad1dea;
+
+    cmd->interrupts = AHCI_PX_IS_DHRS;
+    /* BUG: We expect the DPS interrupt for data commands */
+    /* cmd->interrupts |= props->data ? AHCI_PX_IS_DPS : 0; */
+    /* BUG: We expect the DMA Setup interrupt for DMA commands */
+    /* cmd->interrupts |= props->dma ? AHCI_PX_IS_DSS : 0; */
+    cmd->interrupts |= props->pio ? AHCI_PX_IS_PSS : 0;
+
+    command_header_init(cmd);
+    command_table_init(cmd);
+
+    return cmd;
+}
+
+void ahci_command_free(AHCICommand *cmd)
+{
+    g_free(cmd);
+}
+
+void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer)
+{
+    cmd->buffer = buffer;
+}
+
+void ahci_command_commit(AHCIQState *ahci, AHCICommand *cmd, uint8_t px)
+{
+    uint16_t i, prdtl;
+    uint64_t table_size, table_ptr, remaining;
+    PRD prd;
+
+    /* This command is now tied to this port/command slot */
+    cmd->px = px;
+    cmd->cx = ahci_pick_cmd(ahci, px);
+
+    /* Create a buffer for the command table */
+    prdtl = size_to_prdtl(cmd->xbytes, cmd->prd_size);
+    table_size = CMD_TBL_SIZ(prdtl);
+    table_ptr = ahci_alloc(ahci, table_size);
+    g_assert(table_ptr);
+    /* AHCI 1.3: Must be aligned to 0x80 */
+    g_assert((table_ptr & 0x7F) == 0x00);
+    cmd->header.ctba = table_ptr;
+
+    /* Commit the command header and command FIS */
+    ahci_set_command_header(ahci, px, cmd->cx, &(cmd->header));
+    memwrite(table_ptr, &(cmd->fis), sizeof(cmd->fis));
+
+    /* Construct and write the PRDs to the command table */
+    g_assert_cmphex(prdtl, ==, cmd->header.prdtl);
+    remaining = cmd->xbytes;
+    for (i = 0; i < prdtl; ++i) {
+        prd.dba = cpu_to_le64(cmd->buffer + (cmd->prd_size * i));
+        prd.res = 0;
+        if (remaining > cmd->prd_size) {
+            /* Note that byte count is 0-based. */
+            prd.dbc = cpu_to_le32(cmd->prd_size - 1);
+            remaining -= cmd->prd_size;
+        } else {
+            /* Again, dbc is 0-based. */
+            prd.dbc = cpu_to_le32(remaining - 1);
+            remaining = 0;
+        }
+        prd.dbc |= cpu_to_le32(0x80000000); /* Request DPS Interrupt */
+
+        /* Commit the PRD entry to the Command Table */
+        memwrite(table_ptr + 0x80 + (i * sizeof(PRD)),
+                 &prd, sizeof(PRD));
+    }
+
+    /* Bookmark the PRDTL and CTBA values */
+    ahci->port[px].ctba[cmd->cx] = table_ptr;
+    ahci->port[px].prdtl[cmd->cx] = prdtl;
+}
+
+void ahci_command_issue_async(AHCIQState *ahci, AHCICommand *cmd)
+{
+    if (cmd->props->ncq) {
+        ahci_px_wreg(ahci, cmd->px, AHCI_PX_SACT, (1 << cmd->cx));
+    }
+
+    ahci_px_wreg(ahci, cmd->px, AHCI_PX_CI, (1 << cmd->cx));
+}
+
+void ahci_command_wait(AHCIQState *ahci, AHCICommand *cmd)
+{
+    /* We can't rely on STS_BSY until the command has started processing.
+     * Therefore, we also use the Command Issue bit as indication of
+     * a command in-flight. */
+    while (BITSET(ahci_px_rreg(ahci, cmd->px, AHCI_PX_TFD),
+                  AHCI_PX_TFD_STS_BSY) ||
+           BITSET(ahci_px_rreg(ahci, cmd->px, AHCI_PX_CI), (1 << cmd->cx))) {
+        usleep(50);
+    }
+}
+
+void ahci_command_issue(AHCIQState *ahci, AHCICommand *cmd)
+{
+    ahci_command_issue_async(ahci, cmd);
+    ahci_command_wait(ahci, cmd);
+}
+
+uint8_t ahci_command_slot(AHCICommand *cmd)
+{
+    return cmd->cx;
+}
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 0d12582..99ac820 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -418,6 +418,9 @@ typedef struct PRD {
     uint32_t dbc;  /* Data Byte Count (0-indexed) & Interrupt Flag (bit 2^31) */
 } __attribute__((__packed__)) PRD;
 
+/* Opaque, defined within ahci.c */
+typedef struct AHCICommand AHCICommand;
+
 /*** Macro Utilities ***/
 #define BITANY(data, mask) (((data) & (mask)) != 0)
 #define BITSET(data, mask) (((data) & (mask)) == (mask))
@@ -516,5 +519,17 @@ void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
                              uint8_t cx, AHCICommandHeader *cmd);
 void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx);
 unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t px);
+unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd);
 
+/* Command Lifecycle */
+AHCICommand *ahci_command_create(uint8_t command_name);
+void ahci_command_commit(AHCIQState *ahci, AHCICommand *cmd, uint8_t px);
+void ahci_command_issue(AHCIQState *ahci, AHCICommand *cmd);
+void ahci_command_issue_async(AHCIQState *ahci, AHCICommand *cmd);
+void ahci_command_wait(AHCIQState *ahci, AHCICommand *cmd);
+void ahci_command_free(AHCICommand *cmd);
+/* Command adjustments */
+void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer);
+
+uint8_t ahci_command_slot(AHCICommand *cmd);
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH 12/19] libqos/ahci: add ahci command verify

[John Snow]

Helps to verify that a command completed successfully.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 16 ++--------------
 tests/libqos/ahci.c | 12 ++++++++++++
 tests/libqos/ahci.h |  1 +
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 0834020..6e7b765 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -662,7 +662,6 @@ static void ahci_test_identify(AHCIQState *ahci)
     unsigned i;
     int rc;
     AHCICommand *cmd;
-    uint8_t cx;
 
     g_assert(ahci != NULL);
 
@@ -702,20 +701,9 @@ static void ahci_test_identify(AHCIQState *ahci)
      * so we should find that there are no pending interrupts yet. */
     g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
 
-    /* Issue Command #cx via PxCI */
+    /* Issue command and sanity check response. */
     ahci_command_issue(ahci, cmd);
-    cx = ahci_command_slot(cmd);
-
-    /* Check registers for post-command consistency */
-    ahci_port_check_error(ahci, i);
-    /* BUG: we expect AHCI_PX_IS_DPS to be set. */
-    ahci_port_check_interrupts(ahci, i, AHCI_PX_IS_DHRS | AHCI_PX_IS_PSS);
-    ahci_port_check_nonbusy(ahci, i, cx);
-    /* Investigate the CMD, assert that we read 512 bytes */
-    ahci_port_check_cmd_sanity(ahci, i, cx, 512);
-    /* Investigate FIS responses */
-    ahci_port_check_d2h_sanity(ahci, i, cx);
-    ahci_port_check_pio_sanity(ahci, i, cx, 512);
+    ahci_command_verify(ahci, cmd);
 
     /* Last, but not least: Investigate the IDENTIFY response data. */
     memread(data_ptr, &buff, 512);
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 87cc9dc..594c821 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -725,6 +725,18 @@ void ahci_command_issue(AHCIQState *ahci, AHCICommand *cmd)
     ahci_command_wait(ahci, cmd);
 }
 
+void ahci_command_verify(AHCIQState *ahci, AHCICommand *cmd)
+{
+    ahci_port_check_error(ahci, cmd->px);
+    ahci_port_check_interrupts(ahci, cmd->px, cmd->interrupts);
+    ahci_port_check_nonbusy(ahci, cmd->px, cmd->cx);
+    ahci_port_check_cmd_sanity(ahci, cmd->px, cmd->cx, cmd->xbytes);
+    ahci_port_check_d2h_sanity(ahci, cmd->px, cmd->cx);
+    if (cmd->props->pio) {
+        ahci_port_check_pio_sanity(ahci, cmd->px, cmd->cx, cmd->xbytes);
+    }
+}
+
 uint8_t ahci_command_slot(AHCICommand *cmd)
 {
     return cmd->cx;
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 99ac820..46b7bf3 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -527,6 +527,7 @@ void ahci_command_commit(AHCIQState *ahci, AHCICommand *cmd, uint8_t px);
 void ahci_command_issue(AHCIQState *ahci, AHCICommand *cmd);
 void ahci_command_issue_async(AHCIQState *ahci, AHCICommand *cmd);
 void ahci_command_wait(AHCIQState *ahci, AHCICommand *cmd);
+void ahci_command_verify(AHCIQState *ahci, AHCICommand *cmd);
 void ahci_command_free(AHCICommand *cmd);
 /* Command adjustments */
 void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer);
-- 
1.9.3

[Qemu-devel] [PATCH 13/19] libqos/ahci: add ahci command size setters

[John Snow]

Adds setters for size, prd_size and both via set_sizes.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/ahci.c | 22 ++++++++++++++++++++++
 tests/libqos/ahci.h |  5 +++++
 2 files changed, 27 insertions(+)

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 594c821..3375d54 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -648,6 +648,28 @@ void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer)
     cmd->buffer = buffer;
 }
 
+void ahci_command_set_sizes(AHCICommand *cmd, uint64_t xbytes,
+                            unsigned prd_size)
+{
+    /* Each PRD can describe up to 4MiB, and must not be odd. */
+    g_assert_cmphex(prd_size, <=, 4096 * 1024);
+    g_assert_cmphex(prd_size & 0x01, ==, 0x00);
+    cmd->prd_size = prd_size;
+    cmd->xbytes = xbytes;
+    cmd->fis.count = cpu_to_le16(cmd->xbytes / AHCI_SECTOR_SIZE);
+    cmd->header.prdtl = size_to_prdtl(cmd->xbytes, cmd->prd_size);
+}
+
+void ahci_command_set_size(AHCICommand *cmd, uint64_t xbytes)
+{
+    ahci_command_set_sizes(cmd, xbytes, cmd->prd_size);
+}
+
+void ahci_command_set_prd_size(AHCICommand *cmd, unsigned prd_size)
+{
+    ahci_command_set_sizes(cmd, cmd->xbytes, prd_size);
+}
+
 void ahci_command_commit(AHCIQState *ahci, AHCICommand *cmd, uint8_t px)
 {
     uint16_t i, prdtl;
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 46b7bf3..1300967 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -531,6 +531,11 @@ void ahci_command_verify(AHCIQState *ahci, AHCICommand *cmd);
 void ahci_command_free(AHCICommand *cmd);
 /* Command adjustments */
 void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer);
+void ahci_command_set_size(AHCICommand *cmd, uint64_t xbytes);
+void ahci_command_set_prd_size(AHCICommand *cmd, unsigned prd_size);
+void ahci_command_set_sizes(AHCICommand *cmd, uint64_t xbytes,
+                            unsigned prd_size);
 
 uint8_t ahci_command_slot(AHCICommand *cmd);
+
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH 14/19] libqos/ahci: Add ahci_guest_io

[John Snow]

ahci_guest_io is a shorthand function that will, in one shot,
execute a data command on the guest to the specified guest buffer
location, in the requested amount.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/ahci.c | 15 +++++++++++++++
 tests/libqos/ahci.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 3375d54..d55dab8 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -536,6 +536,21 @@ inline unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
     return (bytes + bytes_per_prd - 1) / bytes_per_prd;
 }
 
+/* Given a guest buffer address, perform an IO operation */
+void ahci_guest_io(AHCIQState *ahci, uint8_t px, uint8_t ide_cmd,
+                   uint64_t buffer, size_t bufsize)
+{
+    AHCICommand *cmd;
+
+    cmd = ahci_command_create(ide_cmd);
+    ahci_command_set_buffer(cmd, buffer);
+    ahci_command_set_size(cmd, bufsize);
+    ahci_command_commit(ahci, cmd, px);
+    ahci_command_issue(ahci, cmd);
+    ahci_command_verify(ahci, cmd);
+    ahci_command_free(cmd);
+}
+
 typedef struct AHCICommand {
     /* Test Management Data */
     uint8_t name;
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 1300967..cbde141 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -520,6 +520,8 @@ void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
 void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx);
 unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t px);
 unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd);
+void ahci_guest_io(AHCIQState *ahci, uint8_t px, uint8_t ide_cmd,
+                   uint64_t gbuffer, size_t size);
 
 /* Command Lifecycle */
 AHCICommand *ahci_command_create(uint8_t command_name);
-- 
1.9.3

[Qemu-devel] [PATCH 15/19] libqos/ahci: add ahci_io

[John Snow]

ahci_io is a wrapper around ahci_guest_io that takes a pointer to host
memory instead, and will create a guest memory buffer and copy the data
to/from as needed and as appropriate for a read/write command, such that
after a read, the guest data will be in a host buffer, and for a write,
the data will be transmitted to guest memory prior to the block operation.

Now that we have all the syntactic sugar functions in place for AHCI,
we can convert the identify test to be very, very short.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c   | 63 +++++++++++++++++++++--------------------------------
 tests/libqos/ahci.c | 25 +++++++++++++++++++++
 tests/libqos/ahci.h |  2 ++
 3 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 6e7b765..47491fe 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -657,56 +657,43 @@ static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port)
  */
 static void ahci_test_identify(AHCIQState *ahci)
 {
-    uint32_t data_ptr;
     uint16_t buff[256];
-    unsigned i;
+    unsigned px;
     int rc;
-    AHCICommand *cmd;
+    const size_t buffsize = 512;
 
     g_assert(ahci != NULL);
 
-    /* We need to:
-     * (1) Create a data buffer for the IDENTIFY response to be sent to,
-     * (2) Create a Command Table Buffer
+    /**
+     * This serves as a bit of a tutorial on AHCI device programming:
+     *
+     * (1) Create a data buffer for the IDENTIFY response to be sent to
+     * (2) Create a Command Table buffer, where we will store the
+     *     command and PRDT (Physical Region Descriptor Table)
      * (3) Construct an FIS host-to-device command structure, and write it to
-     *     the top of the command table buffer.
-     * (4) Create a Physical Region Descriptor that points to the data buffer,
-     *     and write it to the bottom (offset 0x80) of the command table.
-     * (5) Obtain a Command List slot, and update this header to point to
-     *     the Command Table we built above.
-     * (6) Now, PxCLB points to the command list, command 0 points to
-     *     our table, and our table contains an FIS instruction and a
-     *     PRD that points to our rx buffer.
-     * (7) We inform the HBA via PxCI that there is a command ready in slot #0.
+     *     the top of the Command Table buffer.
+     * (4) Create one or more Physical Region Descriptors (PRDs) that describe
+     *     a location in memory where data may be stored/retrieved.
+     * (5) Write these PRDTs to the bottom (offset 0x80) of the Command Table.
+     * (6) Each AHCI port has up to 32 command slots. Each slot contains a
+     *     header that points to a Command Table buffer. Pick an unused slot
+     *     and update it to point to the Command Table we have built.
+     * (7) Now: Command #n points to our Command Table, and our Command Table
+     *     contains the FIS (that describes our command) and the PRDTL, which
+     *     describes our buffer.
+     * (8) We inform the HBA via PxCI (Command Issue) that the command in slot
+     *     #n is ready for processing.
      */
 
     /* Pick the first implemented and running port */
-    i = ahci_port_select(ahci);
-    g_test_message("Selected port %u for test", i);
+    px = ahci_port_select(ahci);
+    g_test_message("Selected port %u for test", px);
 
     /* Clear out the FIS Receive area and any pending interrupts. */
-    ahci_port_clear(ahci, i);
+    ahci_port_clear(ahci, px);
 
-    /* Create a data buffer where we will dump the IDENTIFY data to. */
-    data_ptr = ahci_alloc(ahci, 512);
-    g_assert(data_ptr);
-
-    /* Construct the Command Table (FIS and PRDT) and Command Header */
-    cmd = ahci_command_create(CMD_IDENTIFY);
-    ahci_command_set_buffer(cmd, data_ptr);
-    /* Write the command header and PRDT to guest memory */
-    ahci_command_commit(ahci, cmd, i);
-
-    /* Everything is in place, but we haven't given the go-ahead yet,
-     * so we should find that there are no pending interrupts yet. */
-    g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
-
-    /* Issue command and sanity check response. */
-    ahci_command_issue(ahci, cmd);
-    ahci_command_verify(ahci, cmd);
-
-    /* Last, but not least: Investigate the IDENTIFY response data. */
-    memread(data_ptr, &buff, 512);
+    /* "Read" 512 bytes using CMD_IDENTIFY into the host buffer. */
+    ahci_io(ahci, px, CMD_IDENTIFY, &buff, buffsize);
 
     /* Check serial number/version in the buffer */
     /* NB: IDENTIFY strings are packed in 16bit little endian chunks.
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index d55dab8..e73205b 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -580,6 +580,31 @@ static AHCICommandProp *ahci_command_find(uint8_t command_name)
     }
 }
 
+/* Given a HOST buffer, create a buffer address and perform an IO operation. */
+void ahci_io(AHCIQState *ahci, uint8_t px, uint8_t ide_cmd,
+             void *buffer, size_t bufsize)
+{
+    uint64_t ptr;
+    AHCICommandProp *props;
+
+    props = ahci_command_find(ide_cmd);
+    g_assert(props);
+    ptr = ahci_alloc(ahci, bufsize);
+    g_assert(ptr);
+
+    if (props->write) {
+        memwrite(ptr, buffer, bufsize);
+    }
+
+    ahci_guest_io(ahci, px, ide_cmd, ptr, bufsize);
+
+    if (props->read) {
+        memread(ptr, buffer, bufsize);
+    }
+
+    ahci_free(ahci, ptr);
+}
+
 /**
  * Initializes a basic command header in memory.
  * We assume that this is for an ATA command using RegH2DFIS.
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index cbde141..3c4f478 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -522,6 +522,8 @@ unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t px);
 unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd);
 void ahci_guest_io(AHCIQState *ahci, uint8_t px, uint8_t ide_cmd,
                    uint64_t gbuffer, size_t size);
+void ahci_io(AHCIQState *ahci, uint8_t px, uint8_t ide_cmd,
+             void *buffer, size_t bufsize);
 
 /* Command Lifecycle */
 AHCICommand *ahci_command_create(uint8_t command_name);
-- 
1.9.3

[Qemu-devel] [PATCH 16/19] libqos/ahci: Add ahci_clean_mem

[John Snow]

Clean up guest memory being used in ahci_clean_mem, to be
called during ahci_shutdown. With all guest memory leaks removed,
add an option to the allocator to throw an assertion if a leak
occurs.

This test adds some sanity to both the AHCI library and the
allocator.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c     |  3 +++
 tests/libqos/ahci.c   | 18 ++++++++++++++++++
 tests/libqos/ahci.h   |  1 +
 tests/libqos/malloc.c |  5 +++++
 tests/libqos/malloc.h |  1 +
 5 files changed, 28 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 47491fe..3a0131a 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -86,6 +86,7 @@ static AHCIQState *ahci_boot(void)
         "-device ide-hd,drive=drive0 "
         "-global ide-hd.ver=%s";
     s->parent = qtest_pc_boot(cli, tmp_path, "testdisk", "version");
+    alloc_set_flags(s->parent->alloc, ALLOC_LEAK_ASSERT);
 
     /* Verify that we have an AHCI device present. */
     s->dev = get_ahci_device(&s->fingerprint);
@@ -99,6 +100,8 @@ static AHCIQState *ahci_boot(void)
 static void ahci_shutdown(AHCIQState *ahci)
 {
     QOSState *qs = ahci->parent;
+
+    ahci_clean_mem(ahci);
     free_ahci_device(ahci->dev);
     g_free(ahci);
     qtest_shutdown(qs);
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index e73205b..87a9e62 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -133,6 +133,24 @@ void free_ahci_device(QPCIDevice *dev)
     qpci_free_pc(pcibus);
 }
 
+/* Free all memory in-use by the AHCI device. */
+void ahci_clean_mem(AHCIQState *ahci)
+{
+    uint8_t px, cx;
+
+    for (px = 0; px < 32; ++px) {
+        if (ahci->port[px].fb) {
+            ahci_free(ahci, ahci->port[px].fb);
+        }
+        if (ahci->port[px].clb) {
+            for (cx = 0; cx < 32; cx++) {
+                ahci_destroy_command(ahci, px, cx);
+            }
+            ahci_free(ahci, ahci->port[px].clb);
+        }
+    }
+}
+
 /*** Logical Device Initialization ***/
 
 /**
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 3c4f478..2d25957 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -499,6 +499,7 @@ uint64_t ahci_alloc(AHCIQState *ahci, size_t bytes);
 void ahci_free(AHCIQState *ahci, uint64_t addr);
 QPCIDevice *get_ahci_device(uint32_t *fingerprint);
 void free_ahci_device(QPCIDevice *dev);
+void ahci_clean_mem(AHCIQState *ahci);
 void ahci_pci_enable(AHCIQState *ahci);
 void start_ahci_device(AHCIQState *ahci);
 void ahci_hba_enable(AHCIQState *ahci);
diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
index 8cce1ba..ad283f6 100644
--- a/tests/libqos/malloc.c
+++ b/tests/libqos/malloc.c
@@ -324,3 +324,8 @@ void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size)
     g_assert(is_power_of_2(page_size));
     allocator->page_size = page_size;
 }
+
+void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts)
+{
+    allocator->opts |= opts;
+}
diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index a39dba4..71ac407 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -36,5 +36,6 @@ QGuestAllocator *alloc_init(uint64_t start, uint64_t end);
 QGuestAllocator *alloc_init_flags(QAllocOpts flags,
                                   uint64_t start, uint64_t end);
 void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size);
+void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts);
 
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH 17/19] qtest/ahci: Add a macro bootup routine

[John Snow]

Add a routine that can be used to engage the AHCI
device at a not-granular level so that bringing up
the functionality of the HBA is easy in future tests
that are not concerned with testing the bring-up process.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 3a0131a..9207e73 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -107,6 +107,21 @@ static void ahci_shutdown(AHCIQState *ahci)
     qtest_shutdown(qs);
 }
 
+/**
+ * Boot and fully enable the HBA device.
+ * @see ahci_boot, ahci_pci_enable and ahci_hba_enable.
+ */
+static AHCIQState *ahci_macro_bootup(void)
+{
+    AHCIQState *ahci;
+    ahci = ahci_boot();
+
+    ahci_pci_enable(ahci);
+    ahci_hba_enable(ahci);
+
+    return ahci;
+}
+
 /*** Specification Adherence Tests ***/
 
 /**
@@ -787,9 +802,7 @@ static void test_identify(void)
 {
     AHCIQState *ahci;
 
-    ahci = ahci_boot();
-    ahci_pci_enable(ahci);
-    ahci_hba_enable(ahci);
+    ahci = ahci_macro_bootup();
     ahci_test_identify(ahci);
     ahci_shutdown(ahci);
 }
-- 
1.9.3

[Qemu-devel] [PATCH 18/19] qtest/ahci: Assert sector size in identify test

[John Snow]

A minor sanity check to assert that the sector size is 512.
The current block layer code deeply assumes that the IDE
sector size will be 512 bytes, so we carry forward that assumption
here.

This is useful for the DMA tests, which currently assume that
a sector will always be 512 bytes.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 9207e73..cae94b5 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -678,6 +678,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     uint16_t buff[256];
     unsigned px;
     int rc;
+    uint16_t sect_size;
     const size_t buffsize = 512;
 
     g_assert(ahci != NULL);
@@ -725,6 +726,9 @@ static void ahci_test_identify(AHCIQState *ahci)
     string_bswap16(&buff[23], 8);
     rc = memcmp(&buff[23], "version ", 8);
     g_assert_cmphex(rc, ==, 0);
+
+    sect_size = le16_to_cpu(*((uint16_t *)(&buff[5])));
+    g_assert_cmphex(sect_size, ==, 0x200);
 }
 
 /******************************************************************************/
-- 
1.9.3

[Qemu-devel] [PATCH 19/19] qtest/ahci: Adding simple dma read-write test

[John Snow]

Adds a test case for AHCI wherein we write a 4K
block of a changing pattern to sector 0, then
read back that 4K and compare the transmit and
receive buffers.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ahci-test.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index cae94b5..1f02fa8 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -731,6 +731,45 @@ static void ahci_test_identify(AHCIQState *ahci)
     g_assert_cmphex(sect_size, ==, 0x200);
 }
 
+static void ahci_test_dma_rw_simple(AHCIQState *ahci)
+{
+    uint64_t ptr;
+    uint8_t px;
+    unsigned i;
+    const unsigned bufsize = 4096;
+    unsigned char *tx = g_malloc(bufsize);
+    unsigned char *rx = g_malloc0(bufsize);
+
+    g_assert(ahci != NULL);
+
+    /* Pick the first running port and clear it. */
+    px = ahci_port_select(ahci);
+    ahci_port_clear(ahci, px);
+
+    /*** Create pattern and transfer to guest ***/
+    /* Data buffer in the guest */
+    ptr = ahci_alloc(ahci, bufsize);
+    g_assert(ptr);
+
+    /* Write some indicative pattern to our 4K buffer. */
+    for (i = 0; i < bufsize; i++) {
+        tx[i] = (bufsize - i);
+    }
+    memwrite(ptr, tx, bufsize);
+
+    /* Write this buffer to disk, then read it back to the DMA buffer. */
+    ahci_guest_io(ahci, px, CMD_WRITE_DMA, ptr, bufsize);
+    ahci_guest_io(ahci, px, CMD_READ_DMA, ptr, bufsize);
+
+    /*** Read back the Data ***/
+    memread(ptr, rx, bufsize);
+    g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0);
+
+    ahci_free(ahci, ptr);
+    g_free(tx);
+    g_free(rx);
+}
+
 /******************************************************************************/
 /* Test Interfaces                                                            */
 /******************************************************************************/
@@ -811,6 +850,18 @@ static void test_identify(void)
     ahci_shutdown(ahci);
 }
 
+/**
+ * Perform a simple DMA R/W test, using a single PRD and non-NCQ commands.
+ */
+static void test_dma_rw_simple(void)
+{
+    AHCIQState *ahci;
+
+    ahci = ahci_macro_bootup();
+    ahci_test_dma_rw_simple(ahci);
+    ahci_shutdown(ahci);
+}
+
 /******************************************************************************/
 
 int main(int argc, char **argv)
@@ -866,6 +917,7 @@ int main(int argc, char **argv)
     qtest_add_func("/ahci/hba_spec",   test_hba_spec);
     qtest_add_func("/ahci/hba_enable", test_hba_enable);
     qtest_add_func("/ahci/identify",   test_identify);
+    qtest_add_func("/ahci/dma/simple", test_dma_rw_simple);
 
     ret = g_test_run();
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH 03/19] qtest/ahci: rename 'Command' to 'CommandHeader'

[Paolo Bonzini]

On 30/01/2015 19:41, John Snow wrote:
> @@ -703,8 +703,8 @@ static void ahci_test_identify(AHCIQState *ahci)
>      /* Copy the existing Command #0 structure from the CLB into local memory,
>       * and build a new command #0. */
>      memread(ahci->port[i].clb, &cmd, sizeof(cmd));
> -    cmd.b1 = 5;    /* reg_h2d_fis is 5 double-words long */
> -    cmd.b2 = 0x04; /* clear PxTFD.STS.BSY when done */
> +    cmd.flags = 5;    /* reg_h2d_fis is 5 double-words long */
> +    cmd.flags |= 0x400; /* clear PxTFD.STS.BSY when done */

Missing cpu_to_le16, I think.

Paolo

Re: [Qemu-devel] [PATCH 04/19] libqos/ahci: Add command header helpers

[Paolo Bonzini]

On 30/01/2015 19:41, John Snow wrote:
> +    /* Construct our Command Header (set_command_header handles endianness.) */
> +    memset(&cmd, 0x00, sizeof(cmd));
> +    cmd.flags = 5;     /* reg_h2d_fis is 5 double-words long */
> +    cmd.flags = 0x400; /* clear PxTFD.STS.BSY when done */

And here |= became = ?

Paolo

> +    cmd.prdtl = 1;     /* One PRD table entry. */

Re: [Qemu-devel] [PATCH 04/19] libqos/ahci: Add command header helpers

[Paolo Bonzini]

On 30/01/2015 19:41, John Snow wrote:
> +/* Set the #cx'th command of port #px. */
> +void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
> +                             uint8_t cx, AHCICommandHeader *cmd)
> +{
> +    uint64_t ba = ahci->port[px].clb;
> +    ba += cx * sizeof(AHCICommandHeader);
> +
> +    cmd->flags = cpu_to_le16(cmd->flags);
> +    cmd->prdtl = cpu_to_le16(cmd->prdtl);
> +    cmd->prdbc = cpu_to_le32(cmd->prdbc);
> +    cmd->ctba = cpu_to_le64(cmd->ctba);

Modifying cmd is uglyish, and especially it might hide bugs that only
happen on big endian system.  Please copy to a local AHCICommandHeader
variable before adjusting for host endianness.

Paolo

> +    memwrite(ba, cmd, sizeof(AHCICommandHeader));
> +}

Re: [Qemu-devel] [PATCH 09/19] qtest/ahci: Demagic ahci tests.

[Paolo Bonzini]

On 30/01/2015 19:42, John Snow wrote:
>      memset(&cmd, 0x00, sizeof(cmd));
> -    cmd.flags = 5;     /* reg_h2d_fis is 5 double-words long */
> -    cmd.flags = 0x400; /* clear PxTFD.STS.BSY when done */
> -    cmd.prdtl = 1;     /* One PRD table entry. */
> +    cmd.flags = 5;            /* reg_h2d_fis is 5 double-words long */
> +    cmd.flags = CMDH_CLR_BSY; /* clear PxTFD.STS.BSY when done */

Same = vs. |= note as before.

Paolo

Re: [Qemu-devel] [PATCH 10/19] libqos/ahci: Add ide cmd properties

[Paolo Bonzini]

On 30/01/2015 19:42, John Snow wrote:
> The "Invalid Command Sentinel" here that caps the property array is an
> invalid ATA command, namely 0x01. 0x00 is NOP and 0xFF is reserved for
> vendor usage, so I chose the first invalid one instead.

You can use ARRAY_SIZE instead.

Paolo

Re: [Qemu-devel] [PATCH 13/19] libqos/ahci: add ahci command size setters

[Paolo Bonzini]

On 30/01/2015 19:42, John Snow wrote:
> +void ahci_command_set_sizes(AHCICommand *cmd, uint64_t xbytes,
> +                            unsigned prd_size)
> +{
> +    /* Each PRD can describe up to 4MiB, and must not be odd. */
> +    g_assert_cmphex(prd_size, <=, 4096 * 1024);
> +    g_assert_cmphex(prd_size & 0x01, ==, 0x00);
> +    cmd->prd_size = prd_size;
> +    cmd->xbytes = xbytes;
> +    cmd->fis.count = cpu_to_le16(cmd->xbytes / AHCI_SECTOR_SIZE);

Why do you need cpu_to_le16 here, instead of having it in the function
that writes the command to guest memory?

Paolo

> +    cmd->header.prdtl = size_to_prdtl(cmd->xbytes, cmd->prd_size);
> +}
> +

Re: [Qemu-devel] [PATCH 17/19] qtest/ahci: Add a macro bootup routine

[Paolo Bonzini]

On 30/01/2015 19:42, John Snow wrote:
> +/**
> + * Boot and fully enable the HBA device.
> + * @see ahci_boot, ahci_pci_enable and ahci_hba_enable.
> + */
> +static AHCIQState *ahci_macro_bootup(void)

Ugly name...  I would just leave out this patch.

Paolo

> +{
> +    AHCIQState *ahci;
> +    ahci = ahci_boot();
> +
> +    ahci_pci_enable(ahci);
> +    ahci_hba_enable(ahci);
> +
> +    return ahci;
> +}

Re: [Qemu-devel] [PATCH 19/19] qtest/ahci: Adding simple dma read-write test

[Paolo Bonzini]

On 30/01/2015 19:42, John Snow wrote:
> +    memwrite(ptr, tx, bufsize);
> +
> +    /* Write this buffer to disk, then read it back to the DMA buffer. */
> +    ahci_guest_io(ahci, px, CMD_WRITE_DMA, ptr, bufsize);

I would qmemset the buffer here.

Paolo

> +    ahci_guest_io(ahci, px, CMD_READ_DMA, ptr, bufsize);
> +
> +    /*** Read back the Data ***/
> +    memread(ptr, rx, bufsize);
> +    g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0);

Re: [Qemu-devel] [PATCH 00/19] qtest/ahci: add dma test

[Paolo Bonzini]

On 30/01/2015 19:41, John Snow wrote:
> Add a simple DMA r/w test to ahci-test.
> 
> Oh, and for the first 18 patches, refactor everything into helpers so
> that each ahci_test isn't a thousand lines long.
> 
> This patch depends upon the "ahci test preliminary refactoring" series
> upstream, which shuffled a lot of libqos and malloc facilities to
> support this series.
> 
> This patchset is a necessary step in checking in AHCI/DMA migration
> tests that I will later use as proof as suitability of enabling the
> ICH9 and AHCI migration flags.

Sent a few comments, everything that I didn't reply to has my Reviewed-by.

Paolo

> John Snow (19):
>   libqos/ahci: Add ahci_port_select helper
>   libqos/ahci: Add ahci_port_clear helper
>   qtest/ahci: rename 'Command' to 'CommandHeader'
>   libqos/ahci: Add command header helpers
>   libqos/ahci: Add ahci_port_check_error helper
>   libqos/ahci: Add ahci_port_check_interrupts helper
>   libqos/ahci: Add port_check_nonbusy helper
>   libqos/ahci: Add cmd response sanity check helpers
>   qtest/ahci: Demagic ahci tests.
>   libqos/ahci: Add ide cmd properties
>   libqos/ahci: add ahci command functions
>   libqos/ahci: add ahci command verify
>   libqos/ahci: add ahci command size setters
>   libqos/ahci: Add ahci_guest_io
>   libqos/ahci: add ahci_io
>   libqos/ahci: Add ahci_clean_mem
>   qtest/ahci: Add a macro bootup routine
>   qtest/ahci: Assert sector size in identify test
>   qtest/ahci: Adding simple dma read-write test
> 
>  tests/ahci-test.c     | 246 +++++++++-------------
>  tests/libqos/ahci.c   | 554 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/ahci.h   | 163 ++++++++++++---
>  tests/libqos/malloc.c |   5 +
>  tests/libqos/malloc.h |   1 +
>  5 files changed, 798 insertions(+), 171 deletions(-)
> 

Re: [Qemu-devel] [PATCH 04/19] libqos/ahci: Add command header helpers

[John Snow]

On 02/02/2015 05:25 AM, Paolo Bonzini wrote:
>
>
> On 30/01/2015 19:41, John Snow wrote:
>> +    /* Construct our Command Header (set_command_header handles endianness.) */
>> +    memset(&cmd, 0x00, sizeof(cmd));
>> +    cmd.flags = 5;     /* reg_h2d_fis is 5 double-words long */
>> +    cmd.flags = 0x400; /* clear PxTFD.STS.BSY when done */
>
> And here |= became = ?
>
> Paolo
>
>> +    cmd.prdtl = 1;     /* One PRD table entry. */
>

A typo that testing didn't pick up on. (I wonder how?)
Thank you.

Re: [Qemu-devel] [PATCH 04/19] libqos/ahci: Add command header helpers

[John Snow]

On 02/02/2015 05:27 AM, Paolo Bonzini wrote:
>
>
> On 30/01/2015 19:41, John Snow wrote:
>> +/* Set the #cx'th command of port #px. */
>> +void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
>> +                             uint8_t cx, AHCICommandHeader *cmd)
>> +{
>> +    uint64_t ba = ahci->port[px].clb;
>> +    ba += cx * sizeof(AHCICommandHeader);
>> +
>> +    cmd->flags = cpu_to_le16(cmd->flags);
>> +    cmd->prdtl = cpu_to_le16(cmd->prdtl);
>> +    cmd->prdbc = cpu_to_le32(cmd->prdbc);
>> +    cmd->ctba = cpu_to_le64(cmd->ctba);
>
> Modifying cmd is uglyish, and especially it might hide bugs that only
> happen on big endian system.  Please copy to a local AHCICommandHeader
> variable before adjusting for host endianness.
>
> Paolo
>
>> +    memwrite(ba, cmd, sizeof(AHCICommandHeader));
>> +}
>

You're right. I had just never actually thought to reference the command 
after committing it before, so I hadn't considered it in that light.

If I intend to share goodies with other tests it would be nice if they 
didn't do secretly evil things :)

Re: [Qemu-devel] [PATCH 13/19] libqos/ahci: add ahci command size setters

[John Snow]

On 02/02/2015 05:35 AM, Paolo Bonzini wrote:
>
>
> On 30/01/2015 19:42, John Snow wrote:
>> +void ahci_command_set_sizes(AHCICommand *cmd, uint64_t xbytes,
>> +                            unsigned prd_size)
>> +{
>> +    /* Each PRD can describe up to 4MiB, and must not be odd. */
>> +    g_assert_cmphex(prd_size, <=, 4096 * 1024);
>> +    g_assert_cmphex(prd_size & 0x01, ==, 0x00);
>> +    cmd->prd_size = prd_size;
>> +    cmd->xbytes = xbytes;
>> +    cmd->fis.count = cpu_to_le16(cmd->xbytes / AHCI_SECTOR_SIZE);
>
> Why do you need cpu_to_le16 here, instead of having it in the function
> that writes the command to guest memory?
>
> Paolo
>
>> +    cmd->header.prdtl = size_to_prdtl(cmd->xbytes, cmd->prd_size);
>> +}
>> +
>

In this case, only the command header had a utility written for it to 
flip the bits for me. This is part of the FIS, instead, which has no 
explicit flip-on-write mechanism inside of commit.

So, it's correct, but not terribly consistent.

I can write a fis write helper to make this more internally consistent 
about when we handle it for the user and when we don't.

Re: [Qemu-devel] [PATCH 17/19] qtest/ahci: Add a macro bootup routine

[John Snow]

On 02/02/2015 05:37 AM, Paolo Bonzini wrote:
>
>
> On 30/01/2015 19:42, John Snow wrote:
>> +/**
>> + * Boot and fully enable the HBA device.
>> + * @see ahci_boot, ahci_pci_enable and ahci_hba_enable.
>> + */
>> +static AHCIQState *ahci_macro_bootup(void)
>
> Ugly name...  I would just leave out this patch.
>
> Paolo
>
>> +{
>> +    AHCIQState *ahci;
>> +    ahci = ahci_boot();
>> +
>> +    ahci_pci_enable(ahci);
>> +    ahci_hba_enable(ahci);
>> +
>> +    return ahci;
>> +}
>

It comes in handy later for testing migration so I don't have to do a 
lot of boilerplate for each instance, though it is just a convenience 
subroutine with no logic of its own.

I like to cut down on boilerplate as much as possible to expose the 
logic of the test as much as possible.

Have a suggestion for a better name, or are you very adamant about 
culling it?

--js

Re: [Qemu-devel] [PATCH 13/19] libqos/ahci: add ahci command size setters

[Paolo Bonzini]

On 02/02/2015 22:09, John Snow wrote:
> 
> In this case, only the command header had a utility written for it to
> flip the bits for me. This is part of the FIS, instead, which has no
> explicit flip-on-write mechanism inside of commit.
> 
> So, it's correct, but not terribly consistent.
> 
> I can write a fis write helper to make this more internally consistent
> about when we handle it for the user and when we don't.

Please do. :)

Paolo

Re: [Qemu-devel] [PATCH 17/19] qtest/ahci: Add a macro bootup routine

[Paolo Bonzini]

On 02/02/2015 22:12, John Snow wrote:
> 
> It comes in handy later for testing migration so I don't have to do a
> lot of boilerplate for each instance, though it is just a convenience
> subroutine with no logic of its own.
> 
> I like to cut down on boilerplate as much as possible to expose the
> logic of the test as much as possible.
> 
> Have a suggestion for a better name, or are you very adamant about
> culling it?

I'm adamant about culling it because I don't have a suggestion for a
better name.

In the long run, I think we should just have a qos_boot function that
does everything including PCI scanning, mapping BARs and initializing
devices.  But we're of course very far from that.

Paolo