qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: John Snow
Subject: Re: [Qemu-devel] [PATCH v2 10/19] qtest/ahci: add ahci_write_fis
Date: Thu, 05 Feb 2015 11:19:16 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0



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 <address@hidden>
---
  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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]