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

2015-02-02 Thread John Snow



On 02/02/2015 05:27 AM, Paolo Bonzini wrote:



On 30/01/2015 19:41, John Snow wrote:

+/* Set the #cx'th command of port #px. */
+void ahci_set_command_header(AHCIQState *ahci, uint8_t px,
+ uint8_t cx, AHCICommandHeader *cmd)
+{
+uint64_t ba = ahci-port[px].clb;
+ba += cx * sizeof(AHCICommandHeader);
+
+cmd-flags = cpu_to_le16(cmd-flags);
+cmd-prdtl = cpu_to_le16(cmd-prdtl);
+cmd-prdbc = cpu_to_le32(cmd-prdbc);
+cmd-ctba = cpu_to_le64(cmd-ctba);


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

Paolo


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




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


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




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

2015-02-02 Thread John Snow



On 02/02/2015 05:25 AM, Paolo Bonzini wrote:



On 30/01/2015 19:41, John Snow wrote:

+/* Construct our Command Header (set_command_header handles endianness.) */
+memset(cmd, 0x00, sizeof(cmd));
+cmd.flags = 5; /* reg_h2d_fis is 5 double-words long */
+cmd.flags = 0x400; /* clear PxTFD.STS.BSY when done */


And here |= became = ?

Paolo


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




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



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

2015-02-02 Thread Paolo Bonzini


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

And here |= became = ?

Paolo

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



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

2015-02-02 Thread Paolo Bonzini


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

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

Paolo

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



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

2015-01-30 Thread 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 js...@redhat.com
---
 tests/ahci-test.c   | 43 ---
 tests/libqos/ahci.c | 74 +
 tests/libqos/ahci.h | 17 
 3 files changed, 109 insertions(+), 25 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 85e5761..17aee37 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -662,10 +662,12 @@ static void ahci_test_identify(AHCIQState *ahci)
 RegH2DFIS fis;
 AHCICommandHeader cmd;
 PRD prd;
-uint32_t reg, table, data_ptr;
+uint32_t reg, data_ptr;
 uint16_t buff[256];
 unsigned i;
 int rc;
+uint8_t cx;
+uint64_t table;
 
 g_assert(ahci != NULL);
 
@@ -700,19 +702,19 @@ static void ahci_test_identify(AHCIQState *ahci)
 data_ptr = ahci_alloc(ahci, 512);
 g_assert(data_ptr);
 
-/* Copy the existing Command #0 structure from the CLB into local memory,
- * and build a new command #0. */
-memread(ahci-port[i].clb, cmd, sizeof(cmd));
-cmd.flags = 5;/* reg_h2d_fis is 5 double-words long */
-cmd.flags |= 0x400; /* clear PxTFD.STS.BSY when done */
-cmd.prdtl = cpu_to_le16(1); /* One PRD table entry. */
+/* pick a command slot (should be 0!) */
+cx = ahci_pick_cmd(ahci, i);
+
+/* Construct our Command Header (set_command_header handles endianness.) */
+memset(cmd, 0x00, sizeof(cmd));
+cmd.flags = 5; /* reg_h2d_fis is 5 double-words long */
+cmd.flags = 0x400; /* clear PxTFD.STS.BSY when done */
+cmd.prdtl = 1; /* One PRD table entry. */
 cmd.prdbc = 0;
-cmd.ctba = cpu_to_le32(table);
-cmd.ctbau = 0;
+cmd.ctba = table;
 
 /* Construct our PRD, noting that DBC is 0-indexed. */
-prd.dba = cpu_to_le32(data_ptr);
-prd.dbau = 0;
+prd.dba = cpu_to_le64(data_ptr);
 prd.res = 0;
 /* 511+1 bytes, request DPS interrupt */
 prd.dbc = cpu_to_le32(511 | 0x8000);
@@ -733,14 +735,15 @@ static void ahci_test_identify(AHCIQState *ahci)
 /* Commit the PRD entry to the Command Table */
 memwrite(table + 0x80, prd, sizeof(prd));
 
-/* Commit Command #0, pointing to the Table, to the Command List Buffer. */
-memwrite(ahci-port[i].clb, cmd, sizeof(cmd));
+/* Commit Command #cx, pointing to the Table, to the Command List Buffer. 
*/
+ahci_set_command_header(ahci, i, cx, cmd);
 
-/* Everything is in place, but we haven't given the go-ahead yet. */
+/* Everything is in place, but we haven't given the go-ahead yet,
+ * so we should find that there are no pending interrupts yet. */
 g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
 
-/* Issue Command #0 via PxCI */
-ahci_px_wreg(ahci, i, AHCI_PX_CI, (1  0));
+/* Issue Command #cx via PxCI */
+ahci_px_wreg(ahci, i, AHCI_PX_CI, (1  cx));
 while (BITSET(ahci_px_rreg(ahci, i, AHCI_PX_TFD), AHCI_PX_TFD_STS_BSY)) {
 usleep(50);
 }
@@ -764,9 +767,9 @@ static void ahci_test_identify(AHCIQState *ahci)
 ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_ERR);
 ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_ERR);
 
-/* Investigate CMD #0, assert that we read 512 bytes */
-memread(ahci-port[i].clb, cmd, sizeof(cmd));
-g_assert_cmphex(512, ==, le32_to_cpu(cmd.prdbc));
+/* Investigate the CMD, assert that we read 512 bytes */
+ahci_get_command_header(ahci, i, cx, cmd);
+g_assert_cmphex(512, ==, cmd.prdbc);
 
 /* Investigate FIS responses */
 memread(ahci-port[i].fb + 0x20, pio, 0x20);
@@ -783,7 +786,7 @@ static void ahci_test_identify(AHCIQState *ahci)
 /* The PIO Setup FIS contains a bytes read field, which is a
  * 16-bit value. The Physical Region Descriptor Byte Count is
  * 32-bit, but for small transfers using one PRD, it should match. */
-g_assert_cmphex(le16_to_cpu(pio-res4), ==, le32_to_cpu(cmd.prdbc));
+g_assert_cmphex(le16_to_cpu(pio-res4), ==, cmd.prdbc);
 
 /* Last, but not least: Investigate the IDENTIFY response data. */
 memread(data_ptr, buff, 512);
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 9195bd6..f5e8864 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -310,3 +310,77 @@ void ahci_port_clear(AHCIQState *ahci,