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

[Qemu-devel] [PATCH v2 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.

V2:
 (03) Added cpu_to_le16 into intermediary code.
 (04) Fixed assignment to be or-equals.
      ahci_set_command_header no longer modifies its argument.
 (09) Fixed assignment to be or-equals in the helper refactor.
 (10) New patch. Writes an FIS to guest memory, handling endianness.
 (11) Remove the property table sentinel
      update find_command to use the array size instead.
 (12) Remove cpu_to_le16 from ahci_command_commit,
      leave endianness to ahci_write_fis.
 (14) Remove cpu_to_le16 from ahci_set_sizes,
      leave endianness to ahci_write_fis.
 (19) qmemset the buffer to zeroes between IO calls,
      just to make sure.
 (--) Removed the ahci_macro_boot function patch
      (...for now...)
 (--) Retested entire stack of patches on F20/PPC.

~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.
  qtest/ahci: add ahci_write_fis
  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: Assert sector size in identify test
  qtest/ahci: Adding simple dma read-write test

 tests/ahci-test.c     | 230 ++++++++-------------
 tests/libqos/ahci.c   | 562 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/ahci.h   | 164 ++++++++++++---
 tests/libqos/malloc.c |   5 +
 tests/libqos/malloc.h |   1 +
 5 files changed, 794 insertions(+), 168 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v2 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>
Reviewed-by: Paolo Bonzini <pbonzini@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 v2 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>
Reviewed-by: Paolo Bonzini <pbonzini@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 v2 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   | 8 ++++----
 tests/libqos/ahci.h | 7 +++----
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 90647f2..d420e5f 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,9 +703,9 @@ 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.prdtl = cpu_to_le16(1); /* One PRD table entry. */
+    cmd.flags = cpu_to_le16(5);      /* reg_h2d_fis is 5 double-words long */
+    cmd.flags |= cpu_to_le16(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);
     cmd.ctbau = 0;
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

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

On Tue, Feb 03, 2015 at 04:46:23PM -0500, John Snow wrote:
> 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   | 8 ++++----
>  tests/libqos/ahci.h | 7 +++----
>  2 files changed, 7 insertions(+), 8 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 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 | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/ahci.h | 17 ++++++++----
 3 files changed, 110 insertions(+), 25 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index d420e5f..fbf329e 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 = cpu_to_le16(5);      /* reg_h2d_fis is 5 double-words long */
-    cmd.flags |= cpu_to_le16(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..139a366 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -310,3 +310,78 @@ 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)
+{
+    AHCICommandHeader tmp;
+    uint64_t ba = ahci->port[px].clb;
+    ba += cx * sizeof(AHCICommandHeader);
+
+    tmp.flags = cpu_to_le16(cmd->flags);
+    tmp.prdtl = cpu_to_le16(cmd->prdtl);
+    tmp.prdbc = cpu_to_le32(cmd->prdbc);
+    tmp.ctba = cpu_to_le64(cmd->ctba);
+
+    memwrite(ba, &tmp, 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

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

On Tue, Feb 03, 2015 at 04:46:24PM -0500, John Snow wrote:
> @@ -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);

"px" and "cx" are cryptic names.  "port" and "cmd_slot" would be
clearer.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 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>
Reviewed-by: Paolo Bonzini <pbonzini@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 fbf329e..a3e8f12 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 139a366..ef2b50e 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 v2 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>
Reviewed-by: Paolo Bonzini <pbonzini@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 a3e8f12..32b6be3 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 ef2b50e..d6e0c0b 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 v2 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>
Reviewed-by: Paolo Bonzini <pbonzini@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 32b6be3..4cc7e21 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 d6e0c0b..c9af691 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 v2 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>
Reviewed-by: Paolo Bonzini <pbonzini@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 4cc7e21..b67d935 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 c9af691..ec72627 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 v2 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 b67d935..211274e 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

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

On Tue, Feb 03, 2015 at 04:46:29PM -0500, John Snow wrote:
> 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(-)

I have not checked all the constants against the spec, but the patch
looks reasonable.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 10/19] qtest/ahci: add ahci_write_fis

[John Snow]

Similar to ahci_set_command_header, add a helper that takes an
in-memory representation of a command FIS and writes it to guest
memory, handling endianness as-needed.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 211274e..658956d 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -728,7 +728,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     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));
+    ahci_write_fis(ahci, &fis, table);
 
     /* Commit the PRD entry to the Command Table */
     memwrite(table + 0x80, &prd, sizeof(prd));
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index ec72627..7336781 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -464,6 +464,16 @@ void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx)
     ahci->port[px].prdtl[cx] = 0;
 }
 
+void ahci_write_fis(AHCIQState *ahci, RegH2DFIS *fis, uint64_t addr)
+{
+    RegH2DFIS tmp = *fis;
+
+    /* All other FIS fields are 8 bit and do not need to be flipped. */
+    tmp.count = cpu_to_le16(tmp.count);
+
+    memwrite(addr, &tmp, sizeof(tmp));
+}
+
 unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t px)
 {
     unsigned i;
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 0d12582..d43210f 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -515,6 +515,7 @@ void ahci_get_command_header(AHCIQState *ahci, uint8_t px,
 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);
+void ahci_write_fis(AHCIQState *ahci, RegH2DFIS *fis, uint64_t addr);
 unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t px);
 
 #endif
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 10/19] qtest/ahci: add ahci_write_fis

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1896 bytes --]

On Tue, Feb 03, 2015 at 04:46:30PM -0500, John Snow wrote:
> Similar to ahci_set_command_header, add a helper that takes an
> in-memory representation of a command FIS and writes it to guest
> memory, handling endianness as-needed.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/ahci-test.c   |  2 +-
>  tests/libqos/ahci.c | 10 ++++++++++
>  tests/libqos/ahci.h |  1 +
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index 211274e..658956d 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -728,7 +728,7 @@ static void ahci_test_identify(AHCIQState *ahci)
>      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));
> +    ahci_write_fis(ahci, &fis, table);
>  
>      /* Commit the PRD entry to the Command Table */
>      memwrite(table + 0x80, &prd, sizeof(prd));
> diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
> index ec72627..7336781 100644
> --- a/tests/libqos/ahci.c
> +++ b/tests/libqos/ahci.c
> @@ -464,6 +464,16 @@ void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx)
>      ahci->port[px].prdtl[cx] = 0;
>  }
>  
> +void ahci_write_fis(AHCIQState *ahci, RegH2DFIS *fis, uint64_t addr)
> +{
> +    RegH2DFIS tmp = *fis;
> +
> +    /* All other FIS fields are 8 bit and do not need to be flipped. */
> +    tmp.count = cpu_to_le16(tmp.count);
> +
> +    memwrite(addr, &tmp, sizeof(tmp));
> +}

This patch looks wrong because tmp.count is byteswapped now but not
before.  It actually works because the value is 0 so we never bothered
to assign it explicitly.

I do wonder about the 'aux' field in the FIS struct.  It's uint32_t.
Although the tests never access it, should that field be byteswapped?

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 10/19] qtest/ahci: add ahci_write_fis

[John Snow]

On 02/05/2015 08:29 AM, Stefan Hajnoczi wrote:
> On Tue, Feb 03, 2015 at 04:46:30PM -0500, John Snow wrote:
>> Similar to ahci_set_command_header, add a helper that takes an
>> in-memory representation of a command FIS and writes it to guest
>> memory, handling endianness as-needed.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   tests/ahci-test.c   |  2 +-
>>   tests/libqos/ahci.c | 10 ++++++++++
>>   tests/libqos/ahci.h |  1 +
>>   3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
>> index 211274e..658956d 100644
>> --- a/tests/ahci-test.c
>> +++ b/tests/ahci-test.c
>> @@ -728,7 +728,7 @@ static void ahci_test_identify(AHCIQState *ahci)
>>       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));
>> +    ahci_write_fis(ahci, &fis, table);
>>
>>       /* Commit the PRD entry to the Command Table */
>>       memwrite(table + 0x80, &prd, sizeof(prd));
>> diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
>> index ec72627..7336781 100644
>> --- a/tests/libqos/ahci.c
>> +++ b/tests/libqos/ahci.c
>> @@ -464,6 +464,16 @@ void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx)
>>       ahci->port[px].prdtl[cx] = 0;
>>   }
>>
>> +void ahci_write_fis(AHCIQState *ahci, RegH2DFIS *fis, uint64_t addr)
>> +{
>> +    RegH2DFIS tmp = *fis;
>> +
>> +    /* All other FIS fields are 8 bit and do not need to be flipped. */
>> +    tmp.count = cpu_to_le16(tmp.count);
>> +
>> +    memwrite(addr, &tmp, sizeof(tmp));
>> +}
>
> This patch looks wrong because tmp.count is byteswapped now but not
> before.  It actually works because the value is 0 so we never bothered
> to assign it explicitly.
>
> I do wonder about the 'aux' field in the FIS struct.  It's uint32_t.
> Although the tests never access it, should that field be byteswapped?
>
> Stefan
>

The Aux field(s) is/are used for some NCQ subcommands, and the 
formatting varies per-command, so it's not (at the moment) possible to 
byte swap it automatically ahead of time.

So the answer is "sometimes, maybe, but we're not using it right now."

Also, yes, count /was/ wrong before. It's right now :) since the 
IDENTIFY test as it currently stands is very literal and script-ish, 
there was no need to swap the bits there before. The DMA test requires 
this, though.

If you'd like, I can add a comment or a note that the AUX fields are 
currently ignored.

Re: [Qemu-devel] [PATCH v2 10/19] qtest/ahci: add ahci_write_fis

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3029 bytes --]

On Thu, Feb 05, 2015 at 11:19:16AM -0500, John Snow wrote:
> 
> 
> On 02/05/2015 08:29 AM, Stefan Hajnoczi wrote:
> >On Tue, Feb 03, 2015 at 04:46:30PM -0500, John Snow wrote:
> >>Similar to ahci_set_command_header, add a helper that takes an
> >>in-memory representation of a command FIS and writes it to guest
> >>memory, handling endianness as-needed.
> >>
> >>Signed-off-by: John Snow <jsnow@redhat.com>
> >>---
> >>  tests/ahci-test.c   |  2 +-
> >>  tests/libqos/ahci.c | 10 ++++++++++
> >>  tests/libqos/ahci.h |  1 +
> >>  3 files changed, 12 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> >>index 211274e..658956d 100644
> >>--- a/tests/ahci-test.c
> >>+++ b/tests/ahci-test.c
> >>@@ -728,7 +728,7 @@ static void ahci_test_identify(AHCIQState *ahci)
> >>      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));
> >>+    ahci_write_fis(ahci, &fis, table);
> >>
> >>      /* Commit the PRD entry to the Command Table */
> >>      memwrite(table + 0x80, &prd, sizeof(prd));
> >>diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
> >>index ec72627..7336781 100644
> >>--- a/tests/libqos/ahci.c
> >>+++ b/tests/libqos/ahci.c
> >>@@ -464,6 +464,16 @@ void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx)
> >>      ahci->port[px].prdtl[cx] = 0;
> >>  }
> >>
> >>+void ahci_write_fis(AHCIQState *ahci, RegH2DFIS *fis, uint64_t addr)
> >>+{
> >>+    RegH2DFIS tmp = *fis;
> >>+
> >>+    /* All other FIS fields are 8 bit and do not need to be flipped. */
> >>+    tmp.count = cpu_to_le16(tmp.count);
> >>+
> >>+    memwrite(addr, &tmp, sizeof(tmp));
> >>+}
> >
> >This patch looks wrong because tmp.count is byteswapped now but not
> >before.  It actually works because the value is 0 so we never bothered
> >to assign it explicitly.
> >
> >I do wonder about the 'aux' field in the FIS struct.  It's uint32_t.
> >Although the tests never access it, should that field be byteswapped?
> >
> >Stefan
> >
> 
> The Aux field(s) is/are used for some NCQ subcommands, and the formatting
> varies per-command, so it's not (at the moment) possible to byte swap it
> automatically ahead of time.
> 
> So the answer is "sometimes, maybe, but we're not using it right now."
> 
> Also, yes, count /was/ wrong before. It's right now :) since the IDENTIFY
> test as it currently stands is very literal and script-ish, there was no
> need to swap the bits there before. The DMA test requires this, though.
> 
> If you'd like, I can add a comment or a note that the AUX fields are
> currently ignored.

We discussed the aux field on IRC.  Since its structure depends on the
command, it would be more appropraite to make it uint8_t aux[4] and
perhaps introduce a union if we actually start using that field.  That
way it's clear that this is not a uint32_t that needs byteswapping.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 11/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.

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

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 7336781..c2d16e2 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -34,6 +34,45 @@
 #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;
+
+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 }
+};
+
 /**
  * Allocate space in the guest using information in the AHCIQState object.
  */
-- 
1.9.3

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

On Tue, Feb 03, 2015 at 04:46:31PM -0500, John Snow wrote:
> 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.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/libqos/ahci.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 12/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 658956d..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 */
-    ahci_write_fis(ahci, &fis, table);
-
-    /* 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 c2d16e2..4750680 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -535,3 +535,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 < ARRAY_SIZE(ahci_command_properties); i++) {
+        if (ahci_command_properties[i].cmd == command_name) {
+            return &ahci_command_properties[i];
+        }
+    }
+
+    return NULL;
+}
+
+/**
+ * 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 = (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));
+    ahci_write_fis(ahci, &(cmd->fis), table_ptr);
+
+    /* 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 d43210f..aba27df 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))
@@ -517,5 +520,17 @@ void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
 void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx);
 void ahci_write_fis(AHCIQState *ahci, RegH2DFIS *fis, uint64_t addr);
 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

Re: [Qemu-devel] [PATCH v2 12/19] libqos/ahci: add ahci command functions

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]

On Tue, Feb 03, 2015 at 04:46:32PM -0500, John Snow wrote:
> 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(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 13/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>
Reviewed-by: Paolo Bonzini <pbonzini@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 4750680..d1e511a 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -733,6 +733,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 aba27df..500319a 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -528,6 +528,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 v2 14/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 d1e511a..60849ea 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -656,6 +656,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 = (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 500319a..1f09ef1 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -532,6 +532,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

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

On Tue, Feb 03, 2015 at 04:46:34PM -0500, John Snow wrote:
> 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(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 15/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>
Reviewed-by: Paolo Bonzini <pbonzini@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 60849ea..99a7218 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -544,6 +544,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 1f09ef1..dcb6c84 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -521,6 +521,8 @@ void ahci_destroy_command(AHCIQState *ahci, uint8_t px, uint8_t cx);
 void ahci_write_fis(AHCIQState *ahci, RegH2DFIS *fis, uint64_t addr);
 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 v2 16/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>
Reviewed-by: Paolo Bonzini <pbonzini@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 99a7218..7b6072b 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -588,6 +588,31 @@ static AHCICommandProp *ahci_command_find(uint8_t command_name)
     return NULL;
 }
 
+/* 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 dcb6c84..e69775d 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -523,6 +523,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 v2 17/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>
Reviewed-by: Paolo Bonzini <pbonzini@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 7b6072b..c97acc2 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -130,6 +130,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 e69775d..8c315c8 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 v2 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>
Reviewed-by: Paolo Bonzini <pbonzini@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 3a0131a..f595b96 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -663,6 +663,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);
@@ -710,6 +711,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 v2 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 | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index f595b96..ad02f33 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -716,6 +716,46 @@ 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);
+    qmemset(ptr, 0x00, 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                                                            */
 /******************************************************************************/
@@ -798,6 +838,20 @@ 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_boot();
+    ahci_pci_enable(ahci);
+    ahci_hba_enable(ahci);
+    ahci_test_dma_rw_simple(ahci);
+    ahci_shutdown(ahci);
+}
+
 /******************************************************************************/
 
 int main(int argc, char **argv)
@@ -853,6 +907,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 v2 19/19] qtest/ahci: Adding simple dma read-write test

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Tue, Feb 03, 2015 at 04:46:39PM -0500, John Snow wrote:
> 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 | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]