All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3
@ 2014-08-13 21:56 John Snow
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 25/32] ahci: Adding basic functionality qtest John Snow
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: John Snow @ 2014-08-13 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru, stefanha, mst

This submission does not re-send patches 1-24, which are identical
to the v2 submission and have already been pulled onto
stefanha's block staging branch.

This patch series introduces a number of small fixes and tweaks to
help support an AHCI test suite that in the future I hope to expand
to a fuller regression suite to help guide the development of the
AHCI device support under, in particular, the Q35 machine type in QEMU.

Paolo Bonzini has contributed a number of cleanup and refactoring patches
that support changes to the PIO setup FIS packet construction code, which
is necessary for testing ths specification adherence of the IDENTIFY command,
which issues its data exclusively via PIO mechanisms.

The ahci-test code being checked in represents a minimum of functionality
needed in order to issue and receive commands from the AHCI HBA under the
libqos / qtest environment.

In V3, there is one assertion for incorrect behavior that is expected to
be fixed in a future patch set, where the Descriptor Processed interrupt
is not set or posted within the identify test.

V3:
    - All tests complete successfully.
    - Fixed the PCI capabilities ordering for AHCI.
    - Corrected the boot-time values of the PxTFD and PxSIG registers
    - Corrected PxTFD register being updated prior to PxCMD.FRE being set

V2:
"ide-test: add test for werror=stop"
    - changed filename variable name
    - altered the QMP event polling to avoid sleep
    - debug_path file cleanup on exit.
"ide: stop PIO transfer on errors"
    - Modified logic to be unconditional.
"ahci: construct PIO Setup FIS for PIO commands"
    - Added in dma_memory_map success checks and error pathways.
"libqtest: Correct small memory leak."
    - Corrected raw usage of free()
"ahci: Adding basic functionality qtest."
    - Removed HBA type.
    - Corrected cleanup order.
    - Corrected raw usage of free()
    - Removed needless conditional around g_free()
    - Altered ahci_boot to return by value instead of parameter.
"ahci: Add test_pci_spec to ahci-test."
    - Removed all ifdef logic in favor of runtime tuning. (--pedantic)
    - Removed all warnings, all infractions are now hard assertions.
    - Replaced g_assert with g_assert_cmphex in many cases
"ahci: add test_pci_enable to ahci-test."
    - Removed MSI codepaths, as it was incomplete and unused.
"ahci: Add test_hba_spec to ahci-test."
    - Removed unneeded macros
    - Added in an optional bar_size return parameter to qpci_iomap
"ahci: Add test_identify case to ahci-test."
    - Corrected raw usage of free()

For convenience; https://github.com/jnsnow/qemu/tree/ahci-test-v3

John Snow (15):
  q35: Enable the ioapic device to be seen by qtest.
  qtest: Adding qtest_memset and qmemset.
  libqos: Correct memory leak
  libqtest: Correct small memory leak.
  libqos: Fixes a small memory leak.
  libqos: allow qpci_iomap to return BAR mapping size
  qtest/ide: Fix small memory leak
  ahci: Adding basic functionality qtest.
  ahci: MSI capability should be at 0x80, not 0x50.
  ahci: Add test_pci_spec to ahci-test.
  ahci: add test_pci_enable to ahci-test.
  ahci: properly shadow the TFD register
  ahci: Add test_hba_spec to ahci-test.
  ahci: Add test_hba_enable to ahci-test.
  ahci: Add test_identify case to ahci-test.

Paolo Bonzini (17):
  blkdebug: report errors on flush too
  libqtest: add QTEST_LOG for debugging qtest testcases
  ide-test: add test for werror=stop
  ide: stash aiocb for flushes
  ide: simplify reset callbacks
  ide: simplify set_inactive callbacks
  ide: simplify async_cmd_done callbacks
  ide: simplify start_transfer callbacks
  ide: wrap start_dma callback
  ide: remove wrong setting of BM_STATUS_INT
  ide: fold add_status callback into set_inactive
  ide: move BM_STATUS bits to pci.[ch]
  ide: move retry constants out of BM_STATUS_* namespace
  ahci: remove duplicate PORT_IRQ_* constants
  ide: stop PIO transfer on errors
  ide: make all commands go through cmd_done
  ahci: construct PIO Setup FIS for PIO commands

 block/blkdebug.c          |   20 +
 hw/i386/pc_q35.c          |    2 +-
 hw/ide/ahci.c             |  155 +++--
 hw/ide/ahci.h             |   21 -
 hw/ide/atapi.c            |   11 +-
 hw/ide/core.c             |   94 ++-
 hw/ide/ich.c              |    5 +-
 hw/ide/internal.h         |   38 +-
 hw/ide/macio.c            |    9 -
 hw/ide/pci.c              |   45 +-
 hw/ide/pci.h              |    7 +
 tests/Makefile            |    2 +
 tests/ahci-test.c         | 1554 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ide-test.c          |   85 ++-
 tests/libqos/malloc-pc.c  |    3 +
 tests/libqos/pci-pc.c     |   12 +-
 tests/libqos/pci-pc.h     |    1 +
 tests/libqos/pci.c        |   10 +-
 tests/libqos/pci.h        |    4 +-
 tests/libqtest.c          |   20 +-
 tests/libqtest.h          |   24 +
 tests/usb-hcd-ehci-test.c |    2 +-
 22 files changed, 1922 insertions(+), 202 deletions(-)
 create mode 100644 tests/ahci-test.c

-- 
1.9.3

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v3 25/32] ahci: Adding basic functionality qtest.
  2014-08-13 21:56 [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3 John Snow
@ 2014-08-13 21:56 ` John Snow
  2014-08-14 15:34   ` Stefan Hajnoczi
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 26/32] ahci: MSI capability should be at 0x80, not 0x50 John Snow
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2014-08-13 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru, stefanha, mst

Currently, there is no qtest to test the functionality of
the AHCI functionality present within the Q35 machine type.

This patch adds a skeleton for an AHCI test suite,
and adds a simple sanity-check test case where we
identify that the AHCI device is present, then
disengage the virtual machine.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/Makefile    |   2 +
 tests/ahci-test.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 198 insertions(+)
 create mode 100644 tests/ahci-test.c

diff --git a/tests/Makefile b/tests/Makefile
index 4b2e1bb..0c61b22 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -132,6 +132,7 @@ check-qtest-i386-y = tests/endianness-test$(EXESUF)
 check-qtest-i386-y += tests/fdc-test$(EXESUF)
 gcov-files-i386-y = hw/block/fdc.c
 check-qtest-i386-y += tests/ide-test$(EXESUF)
+check-qtest-i386-y += tests/ahci-test$(EXESUF)
 check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
 gcov-files-i386-y += hw/block/hd-geometry.c
 check-qtest-i386-y += tests/boot-order-test$(EXESUF)
@@ -301,6 +302,7 @@ tests/endianness-test$(EXESUF): tests/endianness-test.o
 tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
 tests/fdc-test$(EXESUF): tests/fdc-test.o
 tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
+tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
 tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
 tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
 tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o $(libqos-obj-y)
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
new file mode 100644
index 0000000..cc49dba
--- /dev/null
+++ b/tests/ahci-test.c
@@ -0,0 +1,196 @@
+/*
+ * AHCI test cases
+ *
+ * Copyright (c) 2014 John Snow <jsnow@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include <stdint.h>
+#include <string.h>
+#include <stdio.h>
+
+#include <glib.h>
+
+#include "libqtest.h"
+#include "libqos/pci-pc.h"
+#include "libqos/malloc-pc.h"
+
+#include "qemu-common.h"
+#include "qemu/host-utils.h"
+
+#include "hw/pci/pci_ids.h"
+#include "hw/pci/pci_regs.h"
+
+/* Test-specific defines. */
+#define TEST_IMAGE_SIZE    (64 * 1024 * 1024)
+
+/*** Supplementary PCI Config Space IDs & Masks ***/
+#define PCI_DEVICE_ID_INTEL_Q35_AHCI   (0x2922)
+
+/*** Globals ***/
+static QGuestAllocator *guest_malloc;
+static QPCIBus *pcibus;
+static char tmp_path[] = "/tmp/qtest.XXXXXX";
+
+/*** Function Declarations ***/
+static QPCIDevice *get_ahci_device(void);
+static void free_ahci_device(QPCIDevice *dev);
+
+/*** Utilities ***/
+
+/**
+ * Locate, verify, and return a handle to the AHCI device.
+ */
+static QPCIDevice *get_ahci_device(void)
+{
+    QPCIDevice *ahci;
+    uint16_t vendor_id, device_id;
+
+    pcibus = qpci_init_pc();
+
+    /* Find the AHCI PCI device and verify it's the right one. */
+    ahci = qpci_device_find(pcibus, QPCI_DEVFN(0x1F, 0x02));
+    g_assert(ahci != NULL);
+
+    vendor_id = qpci_config_readw(ahci, PCI_VENDOR_ID);
+    device_id = qpci_config_readw(ahci, PCI_DEVICE_ID);
+
+    g_assert_cmphex(vendor_id, ==, PCI_VENDOR_ID_INTEL);
+    g_assert_cmphex(device_id, ==, PCI_DEVICE_ID_INTEL_Q35_AHCI);
+
+    return ahci;
+}
+
+static void free_ahci_device(QPCIDevice *ahci)
+{
+    /* libqos doesn't have a function for this, so free it manually */
+    g_free(ahci);
+
+    if (pcibus) {
+        qpci_free_pc(pcibus);
+        pcibus = NULL;
+    }
+}
+
+/*** Test Setup & Teardown ***/
+
+/**
+ * Launch QEMU with the given command line,
+ * and then set up interrupts and our guest malloc interface.
+ */
+static void qtest_boot(const char *cmdline_fmt, ...)
+{
+    va_list ap;
+    char *cmdline;
+
+    va_start(ap, cmdline_fmt);
+    cmdline = g_strdup_vprintf(cmdline_fmt, ap);
+    va_end(ap);
+
+    qtest_start(cmdline);
+    qtest_irq_intercept_in(global_qtest, "ioapic");
+    guest_malloc = pc_alloc_init();
+
+    g_free(cmdline);
+}
+
+/**
+ * Tear down the QEMU instance.
+ */
+static void qtest_shutdown(void)
+{
+    g_free(guest_malloc);
+    guest_malloc = NULL;
+    qtest_end();
+}
+
+/**
+ * Start a Q35 machine and bookmark a handle to the AHCI device.
+ */
+static QPCIDevice *ahci_boot(void)
+{
+    qtest_boot("-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
+               " -M q35 "
+               "-device ide-hd,drive=drive0 "
+               "-global ide-hd.ver=%s",
+               tmp_path, "testdisk", "version");
+
+    /* Verify that we have an AHCI device present. */
+    return get_ahci_device();
+}
+
+/**
+ * Clean up the PCI device, then terminate the QEMU instance.
+ */
+static void ahci_shutdown(QPCIDevice *ahci)
+{
+    free_ahci_device(ahci);
+    qtest_shutdown();
+}
+
+/******************************************************************************/
+/* Test Interfaces                                                            */
+/******************************************************************************/
+
+/**
+ * Basic sanity test to boot a machine, find an AHCI device, and shutdown.
+ */
+static void test_sanity(void)
+{
+    QPCIDevice *ahci;
+    ahci = ahci_boot();
+    ahci_shutdown(ahci);
+}
+
+/******************************************************************************/
+
+int main(int argc, char **argv)
+{
+    const char *arch;
+    int fd;
+    int ret;
+
+    /* Should be first to utilize g_test functionality, So we can see errors. */
+    g_test_init(&argc, &argv, NULL);
+
+    /* Check architecture */
+    arch = qtest_get_arch();
+    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
+        g_test_message("Skipping test for non-x86");
+        return 0;
+    }
+
+    /* Create a temporary raw image */
+    fd = mkstemp(tmp_path);
+    g_assert(fd >= 0);
+    ret = ftruncate(fd, TEST_IMAGE_SIZE);
+    g_assert(ret == 0);
+    close(fd);
+
+    /* Run the tests */
+    qtest_add_func("/ahci/sanity",     test_sanity);
+
+    ret = g_test_run();
+
+    /* Cleanup */
+    unlink(tmp_path);
+
+    return ret;
+}
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v3 26/32] ahci: MSI capability should be at 0x80, not 0x50.
  2014-08-13 21:56 [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3 John Snow
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 25/32] ahci: Adding basic functionality qtest John Snow
@ 2014-08-13 21:56 ` John Snow
  2014-08-14  7:15   ` Michael S. Tsirkin
  2014-08-14  7:19   ` Michael S. Tsirkin
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 27/32] ahci: Add test_pci_spec to ahci-test John Snow
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: John Snow @ 2014-08-13 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru, stefanha, mst

In the Intel ICH9 data sheet, the MSI capability offset
in the PCI configuration space for ICH9 AHCI devices is
specified to be 0x80.

Further, the PCI capability pointer should always point
to 0x80 in ICH9 devices, despite the fact that AHCI 1.3
specifies that it should be pointing to PMCAP (Which in
this instance would be 0x70) to maintain adherence to
the Intel data sheet specifications and real observed behavior.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ich.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index a2f1639..d2a3ac2 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -71,6 +71,7 @@
 #include <hw/ide/pci.h>
 #include <hw/ide/ahci.h>
 
+#define ICH9_MSI_CAP_OFFSET     0x80
 #define ICH9_SATA_CAP_OFFSET    0xA8
 
 #define ICH9_IDP_BAR            4
@@ -115,7 +116,6 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
     /* XXX Software should program this register */
     dev->config[0x90]   = 1 << 6; /* Address Map Register - AHCI mode */
 
-    msi_init(dev, 0x50, 1, true, false);
     d->ahci.irq = pci_allocate_irq(dev);
 
     pci_register_bar(dev, ICH9_IDP_BAR, PCI_BASE_ADDRESS_SPACE_IO,
@@ -135,6 +135,9 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
                  (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4));
     d->ahci.idp_offset = ICH9_IDP_INDEX;
 
+    /* MSI cap should be added last to be first. */
+    msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false);
+
     return 0;
 }
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v3 27/32] ahci: Add test_pci_spec to ahci-test.
  2014-08-13 21:56 [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3 John Snow
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 25/32] ahci: Adding basic functionality qtest John Snow
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 26/32] ahci: MSI capability should be at 0x80, not 0x50 John Snow
@ 2014-08-13 21:56 ` John Snow
  2014-08-14  7:18   ` Michael S. Tsirkin
  2014-08-14 16:03   ` Stefan Hajnoczi
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 28/32] ahci: add test_pci_enable " John Snow
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: John Snow @ 2014-08-13 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru, stefanha, mst

Adds a specification adherence test for AHCI
where the boot-up values for the PCI configuration space
are compared against the AHCI 1.3 specification.

This test does not itself attempt to engage the device.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index cc49dba..29ac0d0 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -25,7 +25,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <stdio.h>
-
+#include <getopt.h>
 #include <glib.h>
 
 #include "libqtest.h"
@@ -43,15 +43,36 @@
 
 /*** Supplementary PCI Config Space IDs & Masks ***/
 #define PCI_DEVICE_ID_INTEL_Q35_AHCI   (0x2922)
+#define PCI_MSI_FLAGS_RESERVED         (0xFF00)
+#define PCI_PM_CTRL_RESERVED             (0xFC)
+#define PCI_BCC(REG32)          ((REG32) >> 24)
+#define PCI_PI(REG32)   (((REG32) >> 8) & 0xFF)
+#define PCI_SCC(REG32) (((REG32) >> 16) & 0xFF)
+
+/*** Recognized AHCI Device Types ***/
+#define AHCI_INTEL_ICH9 (PCI_DEVICE_ID_INTEL_Q35_AHCI << 16 | \
+                         PCI_VENDOR_ID_INTEL)
 
 /*** Globals ***/
 static QGuestAllocator *guest_malloc;
 static QPCIBus *pcibus;
 static char tmp_path[] = "/tmp/qtest.XXXXXX";
+static bool ahci_pedantic;
+static uint32_t ahci_fingerprint;
+
+/*** Macro Utilities ***/
+#define assert_bit_set(data, mask) g_assert_cmphex((data) & (mask), ==, (mask))
+#define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0)
 
 /*** Function Declarations ***/
 static QPCIDevice *get_ahci_device(void);
 static void free_ahci_device(QPCIDevice *dev);
+static void ahci_test_pci_spec(QPCIDevice *ahci);
+static void ahci_test_pci_caps(QPCIDevice *ahci, uint16_t header,
+                               uint8_t offset);
+static void ahci_test_satacap(QPCIDevice *ahci, uint8_t offset);
+static void ahci_test_msicap(QPCIDevice *ahci, uint8_t offset);
+static void ahci_test_pmcap(QPCIDevice *ahci, uint8_t offset);
 
 /*** Utilities ***/
 
@@ -61,7 +82,6 @@ static void free_ahci_device(QPCIDevice *dev);
 static QPCIDevice *get_ahci_device(void)
 {
     QPCIDevice *ahci;
-    uint16_t vendor_id, device_id;
 
     pcibus = qpci_init_pc();
 
@@ -69,11 +89,15 @@ static QPCIDevice *get_ahci_device(void)
     ahci = qpci_device_find(pcibus, QPCI_DEVFN(0x1F, 0x02));
     g_assert(ahci != NULL);
 
-    vendor_id = qpci_config_readw(ahci, PCI_VENDOR_ID);
-    device_id = qpci_config_readw(ahci, PCI_DEVICE_ID);
+    ahci_fingerprint = qpci_config_readl(ahci, PCI_VENDOR_ID);
 
-    g_assert_cmphex(vendor_id, ==, PCI_VENDOR_ID_INTEL);
-    g_assert_cmphex(device_id, ==, PCI_DEVICE_ID_INTEL_Q35_AHCI);
+    switch (ahci_fingerprint) {
+    case AHCI_INTEL_ICH9:
+        break;
+    default:
+        /* Unknown device. */
+        g_assert_not_reached();
+    }
 
     return ahci;
 }
@@ -145,6 +169,239 @@ static void ahci_shutdown(QPCIDevice *ahci)
     qtest_shutdown();
 }
 
+/*** Specification Adherence Tests ***/
+
+/**
+ * Implementation for test_pci_spec. Ensures PCI configuration space is sane.
+ */
+static void ahci_test_pci_spec(QPCIDevice *ahci)
+{
+    uint8_t datab;
+    uint16_t data;
+    uint32_t datal;
+
+    /* Most of these bits should start cleared until we turn them on. */
+    data = qpci_config_readw(ahci, PCI_COMMAND);
+    assert_bit_clear(data, PCI_COMMAND_MEMORY);
+    assert_bit_clear(data, PCI_COMMAND_MASTER);
+    assert_bit_clear(data, PCI_COMMAND_SPECIAL);     /* Reserved */
+    assert_bit_clear(data, PCI_COMMAND_VGA_PALETTE); /* Reserved */
+    assert_bit_clear(data, PCI_COMMAND_PARITY);
+    assert_bit_clear(data, PCI_COMMAND_WAIT);        /* Reserved */
+    assert_bit_clear(data, PCI_COMMAND_SERR);
+    assert_bit_clear(data, PCI_COMMAND_FAST_BACK);
+    assert_bit_clear(data, PCI_COMMAND_INTX_DISABLE);
+    assert_bit_clear(data, 0xF800);                  /* Reserved */
+
+    data = qpci_config_readw(ahci, PCI_STATUS);
+    assert_bit_clear(data, 0x01 | 0x02 | 0x04);     /* Reserved */
+    assert_bit_clear(data, PCI_STATUS_INTERRUPT);
+    assert_bit_set(data, PCI_STATUS_CAP_LIST);      /* must be set */
+    assert_bit_clear(data, PCI_STATUS_UDF);         /* Reserved */
+    assert_bit_clear(data, PCI_STATUS_PARITY);
+    assert_bit_clear(data, PCI_STATUS_SIG_TARGET_ABORT);
+    assert_bit_clear(data, PCI_STATUS_REC_TARGET_ABORT);
+    assert_bit_clear(data, PCI_STATUS_REC_MASTER_ABORT);
+    assert_bit_clear(data, PCI_STATUS_SIG_SYSTEM_ERROR);
+    assert_bit_clear(data, PCI_STATUS_DETECTED_PARITY);
+
+    /* RID occupies the low byte, CCs occupy the high three. */
+    datal = qpci_config_readl(ahci, PCI_CLASS_REVISION);
+    if (ahci_pedantic) {
+        /* AHCI 1.3 specifies that at-boot, the RID should reset to 0x00,
+         * Though in practice this is likely seldom true. */
+        assert_bit_clear(datal, 0xFF);
+    }
+
+    /* BCC *must* equal 0x01. */
+    g_assert_cmphex(PCI_BCC(datal), ==, 0x01);
+    if (PCI_SCC(datal) == 0x01) {
+        /* IDE */
+        assert_bit_set(0x80000000, datal);
+        assert_bit_clear(0x60000000, datal);
+    } else if (PCI_SCC(datal) == 0x04) {
+        /* RAID */
+        g_assert_cmphex(PCI_PI(datal), ==, 0);
+    } else if (PCI_SCC(datal) == 0x06) {
+        /* AHCI */
+        g_assert_cmphex(PCI_PI(datal), ==, 0x01);
+    } else {
+        g_assert_not_reached();
+    }
+
+    datab = qpci_config_readb(ahci, PCI_CACHE_LINE_SIZE);
+    g_assert_cmphex(datab, ==, 0);
+
+    datab = qpci_config_readb(ahci, PCI_LATENCY_TIMER);
+    g_assert_cmphex(datab, ==, 0);
+
+    /* Only the bottom 7 bits must be off. */
+    datab = qpci_config_readb(ahci, PCI_HEADER_TYPE);
+    assert_bit_clear(datab, 0x7F);
+
+    /* BIST is optional, but the low 7 bits must always start off regardless. */
+    datab = qpci_config_readb(ahci, PCI_BIST);
+    assert_bit_clear(datab, 0x7F);
+
+    /* BARS 0-4 do not have a boot spec, but ABAR/BAR5 must be clean. */
+    datal = qpci_config_readl(ahci, PCI_BASE_ADDRESS_5);
+    g_assert_cmphex(datal, ==, 0);
+
+    qpci_config_writel(ahci, PCI_BASE_ADDRESS_5, 0xFFFFFFFF);
+    datal = qpci_config_readl(ahci, PCI_BASE_ADDRESS_5);
+    /* ABAR must be 32-bit, memory mapped, non-prefetchable and
+     * must be >= 512 bytes. To that end, bits 0-8 must be off. */
+    assert_bit_clear(datal, 0xFF);
+
+    /* Capability list MUST be present, */
+    datal = qpci_config_readl(ahci, PCI_CAPABILITY_LIST);
+    /* But these bits are reserved. */
+    assert_bit_clear(datal, ~0xFF);
+    g_assert_cmphex(datal, !=, 0);
+
+    /* Check specification adherence for capability extenstions. */
+    data = qpci_config_readw(ahci, datal);
+
+    switch (ahci_fingerprint) {
+    case AHCI_INTEL_ICH9:
+        /* Intel ICH9 Family Datasheet 14.1.19 p.550 */
+        g_assert_cmphex((data & 0xFF), ==, PCI_CAP_ID_MSI);
+        break;
+    default:
+        /* AHCI 1.3, Section 2.1.14 -- CAP must point to PMCAP. */
+        g_assert_cmphex((data & 0xFF), ==, PCI_CAP_ID_PM);
+    }
+
+    ahci_test_pci_caps(ahci, data, (uint8_t)datal);
+
+    /* Reserved. */
+    datal = qpci_config_readl(ahci, PCI_CAPABILITY_LIST + 4);
+    g_assert_cmphex(datal, ==, 0);
+
+    /* IPIN might vary, but ILINE must be off. */
+    datab = qpci_config_readb(ahci, PCI_INTERRUPT_LINE);
+    g_assert_cmphex(datab, ==, 0);
+}
+
+/**
+ * Test PCI capabilities for AHCI specification adherence.
+ */
+static void ahci_test_pci_caps(QPCIDevice *ahci, uint16_t header,
+                               uint8_t offset)
+{
+    uint8_t cid = header & 0xFF;
+    uint8_t next = header >> 8;
+
+    g_test_message("CID: %02x; next: %02x", cid, next);
+
+    switch (cid) {
+    case PCI_CAP_ID_PM:
+        ahci_test_pmcap(ahci, offset);
+        break;
+    case PCI_CAP_ID_MSI:
+        ahci_test_msicap(ahci, offset);
+        break;
+    case PCI_CAP_ID_SATA:
+        ahci_test_satacap(ahci, offset);
+        break;
+
+    default:
+        g_test_message("Unknown CAP 0x%02x", cid);
+    }
+
+    if (next) {
+        ahci_test_pci_caps(ahci, qpci_config_readw(ahci, next), next);
+    }
+}
+
+/**
+ * Test SATA PCI capabilitity for AHCI specification adherence.
+ */
+static void ahci_test_satacap(QPCIDevice *ahci, uint8_t offset)
+{
+    uint16_t dataw;
+    uint32_t datal;
+
+    g_test_message("Verifying SATACAP");
+
+    /* Assert that the SATACAP version is 1.0, And reserved bits are empty. */
+    dataw = qpci_config_readw(ahci, offset + 2);
+    g_assert_cmphex(dataw, ==, 0x10);
+
+    /* Grab the SATACR1 register. */
+    datal = qpci_config_readw(ahci, offset + 4);
+
+    switch (datal & 0x0F) {
+    case 0x04: /* BAR0 */
+    case 0x05: /* BAR1 */
+    case 0x06:
+    case 0x07:
+    case 0x08:
+    case 0x09: /* BAR5 */
+    case 0x0F: /* Immediately following SATACR1 in PCI config space. */
+        break;
+    default:
+        /* Invalid BARLOC for the Index Data Pair. */
+        g_assert_not_reached();
+    }
+
+    /* Reserved. */
+    g_assert_cmphex((datal >> 24), ==, 0x00);
+}
+
+/**
+ * Test MSI PCI capability for AHCI specification adherence.
+ */
+static void ahci_test_msicap(QPCIDevice *ahci, uint8_t offset)
+{
+    uint16_t dataw;
+    uint32_t datal;
+
+    g_test_message("Verifying MSICAP");
+
+    dataw = qpci_config_readw(ahci, offset + PCI_MSI_FLAGS);
+    assert_bit_clear(dataw, PCI_MSI_FLAGS_ENABLE);
+    assert_bit_clear(dataw, PCI_MSI_FLAGS_QSIZE);
+    assert_bit_clear(dataw, PCI_MSI_FLAGS_RESERVED);
+
+    datal = qpci_config_readl(ahci, offset + PCI_MSI_ADDRESS_LO);
+    g_assert_cmphex(datal, ==, 0);
+
+    if (dataw & PCI_MSI_FLAGS_64BIT) {
+        g_test_message("MSICAP is 64bit");
+        datal = qpci_config_readl(ahci, offset + PCI_MSI_ADDRESS_HI);
+        g_assert_cmphex(datal, ==, 0);
+        dataw = qpci_config_readw(ahci, offset + PCI_MSI_DATA_64);
+        g_assert_cmphex(dataw, ==, 0);
+    } else {
+        g_test_message("MSICAP is 32bit");
+        dataw = qpci_config_readw(ahci, offset + PCI_MSI_DATA_32);
+        g_assert_cmphex(dataw, ==, 0);
+    }
+}
+
+/**
+ * Test Power Management PCI capability for AHCI specification adherence.
+ */
+static void ahci_test_pmcap(QPCIDevice *ahci, uint8_t offset)
+{
+    uint16_t dataw;
+
+    g_test_message("Verifying PMCAP");
+
+    dataw = qpci_config_readw(ahci, offset + PCI_PM_PMC);
+    assert_bit_clear(dataw, PCI_PM_CAP_PME_CLOCK);
+    assert_bit_clear(dataw, PCI_PM_CAP_RESERVED);
+    assert_bit_clear(dataw, PCI_PM_CAP_D1);
+    assert_bit_clear(dataw, PCI_PM_CAP_D2);
+
+    dataw = qpci_config_readw(ahci, offset + PCI_PM_CTRL);
+    assert_bit_clear(dataw, PCI_PM_CTRL_STATE_MASK);
+    assert_bit_clear(dataw, PCI_PM_CTRL_RESERVED);
+    assert_bit_clear(dataw, PCI_PM_CTRL_DATA_SEL_MASK);
+    assert_bit_clear(dataw, PCI_PM_CTRL_DATA_SCALE_MASK);
+}
+
 /******************************************************************************/
 /* Test Interfaces                                                            */
 /******************************************************************************/
@@ -159,6 +416,18 @@ static void test_sanity(void)
     ahci_shutdown(ahci);
 }
 
+/**
+ * Ensure that the PCI configuration space for the AHCI device is in-line with
+ * the AHCI 1.3 specification for initial values.
+ */
+static void test_pci_spec(void)
+{
+    QPCIDevice *ahci;
+    ahci = ahci_boot();
+    ahci_test_pci_spec(ahci);
+    ahci_shutdown(ahci);
+}
+
 /******************************************************************************/
 
 int main(int argc, char **argv)
@@ -166,10 +435,33 @@ int main(int argc, char **argv)
     const char *arch;
     int fd;
     int ret;
+    int c;
+
+    static struct option long_options[] = {
+        {"pedantic", no_argument, 0, 'p' },
+        {0, 0, 0, 0},
+    };
 
     /* Should be first to utilize g_test functionality, So we can see errors. */
     g_test_init(&argc, &argv, NULL);
 
+    while (1) {
+        c = getopt_long(argc, argv, "", long_options, NULL);
+        if (c == -1) {
+            break;
+        }
+        switch (c) {
+        case -1:
+            break;
+        case 'p':
+            ahci_pedantic = 1;
+            break;
+        default:
+            fprintf(stderr, "Unrecognized ahci_test option.\n");
+            g_assert_not_reached();
+        }
+    }
+
     /* Check architecture */
     arch = qtest_get_arch();
     if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
@@ -186,6 +478,7 @@ int main(int argc, char **argv)
 
     /* Run the tests */
     qtest_add_func("/ahci/sanity",     test_sanity);
+    qtest_add_func("/ahci/pci_spec",   test_pci_spec);
 
     ret = g_test_run();
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v3 28/32] ahci: add test_pci_enable to ahci-test.
  2014-08-13 21:56 [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3 John Snow
                   ` (2 preceding siblings ...)
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 27/32] ahci: Add test_pci_spec to ahci-test John Snow
@ 2014-08-13 21:56 ` John Snow
  2014-08-14 16:05   ` Stefan Hajnoczi
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 29/32] ahci: properly shadow the TFD register John Snow
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2014-08-13 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru, stefanha, mst

This adds a test wherein we engage the PCI AHCI
device and ensure that the memory region for the
HBA functionality is now accessible.

Under Q35 environments, additional PCI configuration
is performed to ensure that the HBA functionality
will become usable.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 29ac0d0..0ca4a20 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -56,6 +56,7 @@
 /*** Globals ***/
 static QGuestAllocator *guest_malloc;
 static QPCIBus *pcibus;
+static uint64_t barsize;
 static char tmp_path[] = "/tmp/qtest.XXXXXX";
 static bool ahci_pedantic;
 static uint32_t ahci_fingerprint;
@@ -66,6 +67,7 @@ static uint32_t ahci_fingerprint;
 
 /*** Function Declarations ***/
 static QPCIDevice *get_ahci_device(void);
+static QPCIDevice *start_ahci_device(QPCIDevice *dev, void **hba_base);
 static void free_ahci_device(QPCIDevice *dev);
 static void ahci_test_pci_spec(QPCIDevice *ahci);
 static void ahci_test_pci_caps(QPCIDevice *ahci, uint16_t header,
@@ -111,6 +113,9 @@ static void free_ahci_device(QPCIDevice *ahci)
         qpci_free_pc(pcibus);
         pcibus = NULL;
     }
+
+    /* Clear our cached barsize information. */
+    barsize = 0;
 }
 
 /*** Test Setup & Teardown ***/
@@ -169,6 +174,44 @@ static void ahci_shutdown(QPCIDevice *ahci)
     qtest_shutdown();
 }
 
+/*** Logical Device Initialization ***/
+
+/**
+ * Start the PCI device and sanity-check default operation.
+ */
+static void ahci_pci_enable(QPCIDevice *ahci, void **hba_base)
+{
+    uint8_t reg;
+
+    start_ahci_device(ahci, hba_base);
+
+    switch(ahci_fingerprint) {
+    case AHCI_INTEL_ICH9:
+        /* ICH9 has a register at PCI 0x92 that
+         * acts as a master port enabler mask. */
+        reg = qpci_config_readb(ahci, 0x92);
+        reg |= 0x3F;
+        qpci_config_writeb(ahci, 0x92, reg);
+        assert_bit_set(qpci_config_readb(ahci, 0x92), 0x3F);
+        break;
+    }
+
+}
+
+/**
+ * Map BAR5/ABAR, and engage the PCI device.
+ */
+static QPCIDevice *start_ahci_device(QPCIDevice *ahci, void **hba_base)
+{
+    /* Map AHCI's ABAR (BAR5) */
+    *hba_base = qpci_iomap(ahci, 5, &barsize);
+
+    /* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */
+    qpci_device_enable(ahci);
+
+    return ahci;
+}
+
 /*** Specification Adherence Tests ***/
 
 /**
@@ -428,6 +471,19 @@ static void test_pci_spec(void)
     ahci_shutdown(ahci);
 }
 
+/**
+ * Engage the PCI AHCI device and sanity check the response.
+ * Perform additional PCI config space bringup for the HBA.
+ */
+static void test_pci_enable(void)
+{
+    QPCIDevice *ahci;
+    void *hba_base;
+    ahci = ahci_boot();
+    ahci_pci_enable(ahci, &hba_base);
+    ahci_shutdown(ahci);
+}
+
 /******************************************************************************/
 
 int main(int argc, char **argv)
@@ -479,6 +535,7 @@ int main(int argc, char **argv)
     /* Run the tests */
     qtest_add_func("/ahci/sanity",     test_sanity);
     qtest_add_func("/ahci/pci_spec",   test_pci_spec);
+    qtest_add_func("/ahci/pci_enable", test_pci_enable);
 
     ret = g_test_run();
 
diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
index ce0b308..b244689 100644
--- a/tests/libqos/pci.c
+++ b/tests/libqos/pci.c
@@ -73,6 +73,12 @@ void qpci_device_enable(QPCIDevice *dev)
     cmd = qpci_config_readw(dev, PCI_COMMAND);
     cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
     qpci_config_writew(dev, PCI_COMMAND, cmd);
+
+    /* Verify the bits are now set. */
+    cmd = qpci_config_readw(dev, PCI_COMMAND);
+    g_assert_cmphex(cmd & PCI_COMMAND_IO, ==, PCI_COMMAND_IO);
+    g_assert_cmphex(cmd & PCI_COMMAND_MEMORY, ==, PCI_COMMAND_MEMORY);
+    g_assert_cmphex(cmd & PCI_COMMAND_MASTER, ==, PCI_COMMAND_MASTER);
 }
 
 uint8_t qpci_config_readb(QPCIDevice *dev, uint8_t offset)
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v3 29/32] ahci: properly shadow the TFD register
  2014-08-13 21:56 [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3 John Snow
                   ` (3 preceding siblings ...)
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 28/32] ahci: add test_pci_enable " John Snow
@ 2014-08-13 21:56 ` John Snow
  2014-08-14 16:09   ` Stefan Hajnoczi
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 30/32] ahci: Add test_hba_spec to ahci-test John Snow
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2014-08-13 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru, stefanha, mst

In a real AHCI device, several S/ATA registers are mirrored or shadowed
within the AHCI register set. These registers are not updated
synchronously for each read access, but rather after a Device-to-Host
Register FIS packet is received, which contains the values from these
registers on the device.

In QEMU, by reaching directly into the device to grab these bits before
they are "sent," we may introduce race conditions where unexpected
values are present "before they are sent" which could cause issues for
some guests, particularly if an attempt is made to read the PxTFD
register prior to enabling the port, where incorrect values will be read.

This patch also addresses the boot-time values for the PxTFD and PxSIG
registers to bring them in line with the AHCI 1.3 specification.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ahci.c | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 4cda0d0..00d8a7a 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -78,8 +78,7 @@ static uint32_t  ahci_port_read(AHCIState *s, int port, int offset)
         val = pr->cmd;
         break;
     case PORT_TFDATA:
-        val = ((uint16_t)s->dev[port].port.ifs[0].error << 8) |
-              s->dev[port].port.ifs[0].status;
+        val = pr->tfdata;
         break;
     case PORT_SIG:
         val = pr->sig;
@@ -251,14 +250,13 @@ static void  ahci_port_write(AHCIState *s, int port, int offset, uint32_t val)
             check_cmd(s, port);
             break;
         case PORT_TFDATA:
-            s->dev[port].port.ifs[0].error = (val >> 8) & 0xff;
-            s->dev[port].port.ifs[0].status = val & 0xff;
+            /* Read Only. */
             break;
         case PORT_SIG:
-            pr->sig = val;
+            /* Read Only */
             break;
         case PORT_SCR_STAT:
-            pr->scr_stat = val;
+            /* Read Only */
             break;
         case PORT_SCR_CTL:
             if (((pr->scr_ctl & AHCI_SCR_SCTL_DET) == 1) &&
@@ -497,6 +495,8 @@ static void ahci_reset_port(AHCIState *s, int port)
     pr->scr_stat = 0;
     pr->scr_err = 0;
     pr->scr_act = 0;
+    pr->tfdata = 0x7F;
+    pr->sig = 0xFFFFFFFF;
     d->busy_slot = -1;
     d->init_d2h_sent = false;
 
@@ -528,16 +528,16 @@ static void ahci_reset_port(AHCIState *s, int port)
 
     s->dev[port].port_state = STATE_RUN;
     if (!ide_state->bs) {
-        s->dev[port].port_regs.sig = 0;
+        pr->sig = 0;
         ide_state->status = SEEK_STAT | WRERR_STAT;
     } else if (ide_state->drive_kind == IDE_CD) {
-        s->dev[port].port_regs.sig = SATA_SIGNATURE_CDROM;
+        pr->sig = SATA_SIGNATURE_CDROM;
         ide_state->lcyl = 0x14;
         ide_state->hcyl = 0xeb;
         DPRINTF(port, "set lcyl = %d\n", ide_state->lcyl);
         ide_state->status = SEEK_STAT | WRERR_STAT | READY_STAT;
     } else {
-        s->dev[port].port_regs.sig = SATA_SIGNATURE_DISK;
+        pr->sig = SATA_SIGNATURE_DISK;
         ide_state->status = SEEK_STAT | WRERR_STAT;
     }
 
@@ -563,7 +563,8 @@ static void debug_print_fis(uint8_t *fis, int cmd_len)
 
 static void ahci_write_fis_sdb(AHCIState *s, int port, uint32_t finished)
 {
-    AHCIPortRegs *pr = &s->dev[port].port_regs;
+    AHCIDevice *ad = &s->dev[port];
+    AHCIPortRegs *pr = &ad->port_regs;
     IDEState *ide_state;
     uint8_t *sdb_fis;
 
@@ -572,8 +573,8 @@ static void ahci_write_fis_sdb(AHCIState *s, int port, uint32_t finished)
         return;
     }
 
-    sdb_fis = &s->dev[port].res_fis[RES_FIS_SDBFIS];
-    ide_state = &s->dev[port].port.ifs[0];
+    sdb_fis = &ad->res_fis[RES_FIS_SDBFIS];
+    ide_state = &ad->port.ifs[0];
 
     /* clear memory */
     *(uint32_t*)sdb_fis = 0;
@@ -582,9 +583,14 @@ static void ahci_write_fis_sdb(AHCIState *s, int port, uint32_t finished)
     sdb_fis[0] = ide_state->error;
     sdb_fis[2] = ide_state->status & 0x77;
     s->dev[port].finished |= finished;
-    *(uint32_t*)(sdb_fis + 4) = cpu_to_le32(s->dev[port].finished);
+    *(uint32_t*)(sdb_fis + 4) = cpu_to_le32(ad->finished);
 
-    ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_SDB_FIS);
+    /* Update shadow registers (except BSY 0x80 and DRQ 0x08) */
+    pr->tfdata = (ad->port.ifs[0].error << 8) |
+        (ad->port.ifs[0].status & 0x77) |
+        (pr->tfdata & 0x88);
+
+    ahci_trigger_irq(s, ad, PORT_IRQ_SDB_FIS);
 }
 
 static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
@@ -642,6 +648,10 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
     pio_fis[18] = 0;
     pio_fis[19] = 0;
 
+    /* Update shadow registers: */
+    pr->tfdata = (ad->port.ifs[0].error << 8) |
+        ad->port.ifs[0].status;
+
     if (pio_fis[2] & ERR_STAT) {
         ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
     }
@@ -693,6 +703,10 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
         d2h_fis[i] = 0;
     }
 
+    /* Update shadow registers: */
+    pr->tfdata = (ad->port.ifs[0].error << 8) |
+        ad->port.ifs[0].status;
+
     if (d2h_fis[2] & ERR_STAT) {
         ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
     }
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v3 30/32] ahci: Add test_hba_spec to ahci-test.
  2014-08-13 21:56 [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3 John Snow
                   ` (4 preceding siblings ...)
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 29/32] ahci: properly shadow the TFD register John Snow
@ 2014-08-13 21:56 ` John Snow
  2014-08-14 16:25   ` Stefan Hajnoczi
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 31/32] ahci: Add test_hba_enable " John Snow
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 32/32] ahci: Add test_identify case " John Snow
  7 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2014-08-13 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru, stefanha, mst

Add a test routine that checks the boot-up values of the HBA
configuration memory space against the AHCI 1.3 specification
and Intel ICH9 data sheet (for Q35 machines) for adherence and
sane values.

The HBA is not yet engaged or put into the idle state.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 0ca4a20..85f8661 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -53,6 +53,210 @@
 #define AHCI_INTEL_ICH9 (PCI_DEVICE_ID_INTEL_Q35_AHCI << 16 | \
                          PCI_VENDOR_ID_INTEL)
 
+/*** AHCI/HBA Register Offsets and Bitmasks ***/
+#define AHCI_CAP                          (0)
+#define AHCI_CAP_NP                    (0x1F)
+#define AHCI_CAP_SXS                   (0x20)
+#define AHCI_CAP_EMS                   (0x40)
+#define AHCI_CAP_CCCS                  (0x80)
+#define AHCI_CAP_NCS                 (0x1F00)
+#define AHCI_CAP_PSC                 (0x2000)
+#define AHCI_CAP_SSC                 (0x4000)
+#define AHCI_CAP_PMD                 (0x8000)
+#define AHCI_CAP_FBSS               (0x10000)
+#define AHCI_CAP_SPM                (0x20000)
+#define AHCI_CAP_SAM                (0x40000)
+#define AHCI_CAP_RESERVED           (0x80000)
+#define AHCI_CAP_ISS               (0xF00000)
+#define AHCI_CAP_SCLO             (0x1000000)
+#define AHCI_CAP_SAL              (0x2000000)
+#define AHCI_CAP_SALP             (0x4000000)
+#define AHCI_CAP_SSS              (0x8000000)
+#define AHCI_CAP_SMPS            (0x10000000)
+#define AHCI_CAP_SSNTF           (0x20000000)
+#define AHCI_CAP_SNCQ            (0x40000000)
+#define AHCI_CAP_S64A            (0x80000000)
+
+#define AHCI_GHC                          (1)
+#define AHCI_GHC_HR                    (0x01)
+#define AHCI_GHC_IE                    (0x02)
+#define AHCI_GHC_MRSM                  (0x04)
+#define AHCI_GHC_RESERVED        (0x7FFFFFF8)
+#define AHCI_GHC_AE              (0x80000000)
+
+#define AHCI_IS                           (2)
+#define AHCI_PI                           (3)
+#define AHCI_VS                           (4)
+
+#define AHCI_CCCCTL                       (5)
+#define AHCI_CCCCTL_EN                 (0x01)
+#define AHCI_CCCCTL_RESERVED           (0x06)
+#define AHCI_CCCCTL_CC               (0xFF00)
+#define AHCI_CCCCTL_TV           (0xFFFF0000)
+
+#define AHCI_CCCPORTS                     (6)
+#define AHCI_EMLOC                        (7)
+
+#define AHCI_EMCTL                        (8)
+#define AHCI_EMCTL_STSMR               (0x01)
+#define AHCI_EMCTL_CTLTM              (0x100)
+#define AHCI_EMCTL_CTLRST             (0x200)
+#define AHCI_EMCTL_RESERVED      (0xF0F0FCFE)
+
+#define AHCI_CAP2                         (9)
+#define AHCI_CAP2_BOH                  (0x01)
+#define AHCI_CAP2_NVMP                 (0x02)
+#define AHCI_CAP2_APST                 (0x04)
+#define AHCI_CAP2_RESERVED       (0xFFFFFFF8)
+
+#define AHCI_BOHC                        (10)
+#define AHCI_RESERVED                    (11)
+#define AHCI_NVMHCI                      (24)
+#define AHCI_VENDOR                      (40)
+#define AHCI_PORTS                       (64)
+
+/*** Port Memory Offsets & Bitmasks ***/
+#define AHCI_PX_CLB                       (0)
+#define AHCI_PX_CLB_RESERVED          (0x1FF)
+
+#define AHCI_PX_CLBU                      (1)
+
+#define AHCI_PX_FB                        (2)
+#define AHCI_PX_FB_RESERVED            (0xFF)
+
+#define AHCI_PX_FBU                       (3)
+
+#define AHCI_PX_IS                        (4)
+#define AHCI_PX_IS_DHRS                 (0x1)
+#define AHCI_PX_IS_PSS                  (0x2)
+#define AHCI_PX_IS_DSS                  (0x4)
+#define AHCI_PX_IS_SDBS                 (0x8)
+#define AHCI_PX_IS_UFS                 (0x10)
+#define AHCI_PX_IS_DPS                 (0x20)
+#define AHCI_PX_IS_PCS                 (0x40)
+#define AHCI_PX_IS_DMPS                (0x80)
+#define AHCI_PX_IS_RESERVED       (0x23FFF00)
+#define AHCI_PX_IS_PRCS            (0x400000)
+#define AHCI_PX_IS_IPMS            (0x800000)
+#define AHCI_PX_IS_OFS            (0x1000000)
+#define AHCI_PX_IS_INFS           (0x4000000)
+#define AHCI_PX_IS_IFS            (0x8000000)
+#define AHCI_PX_IS_HBDS          (0x10000000)
+#define AHCI_PX_IS_HBFS          (0x20000000)
+#define AHCI_PX_IS_TFES          (0x40000000)
+#define AHCI_PX_IS_CPDS          (0x80000000)
+
+#define AHCI_PX_IE                        (5)
+#define AHCI_PX_IE_DHRE                 (0x1)
+#define AHCI_PX_IE_PSE                  (0x2)
+#define AHCI_PX_IE_DSE                  (0x4)
+#define AHCI_PX_IE_SDBE                 (0x8)
+#define AHCI_PX_IE_UFE                 (0x10)
+#define AHCI_PX_IE_DPE                 (0x20)
+#define AHCI_PX_IE_PCE                 (0x40)
+#define AHCI_PX_IE_DMPE                (0x80)
+#define AHCI_PX_IE_RESERVED       (0x23FFF00)
+#define AHCI_PX_IE_PRCE            (0x400000)
+#define AHCI_PX_IE_IPME            (0x800000)
+#define AHCI_PX_IE_OFE            (0x1000000)
+#define AHCI_PX_IE_INFE           (0x4000000)
+#define AHCI_PX_IE_IFE            (0x8000000)
+#define AHCI_PX_IE_HBDE          (0x10000000)
+#define AHCI_PX_IE_HBFE          (0x20000000)
+#define AHCI_PX_IE_TFEE          (0x40000000)
+#define AHCI_PX_IE_CPDE          (0x80000000)
+
+#define AHCI_PX_CMD                       (6)
+#define AHCI_PX_CMD_ST                  (0x1)
+#define AHCI_PX_CMD_SUD                 (0x2)
+#define AHCI_PX_CMD_POD                 (0x4)
+#define AHCI_PX_CMD_CLO                 (0x8)
+#define AHCI_PX_CMD_FRE                (0x10)
+#define AHCI_PX_CMD_RESERVED           (0xE0)
+#define AHCI_PX_CMD_CCS              (0x1F00)
+#define AHCI_PX_CMD_MPSS             (0x2000)
+#define AHCI_PX_CMD_FR               (0x4000)
+#define AHCI_PX_CMD_CR               (0x8000)
+#define AHCI_PX_CMD_CPS             (0x10000)
+#define AHCI_PX_CMD_PMA             (0x20000)
+#define AHCI_PX_CMD_HPCP            (0x40000)
+#define AHCI_PX_CMD_MPSP            (0x80000)
+#define AHCI_PX_CMD_CPD            (0x100000)
+#define AHCI_PX_CMD_ESP            (0x200000)
+#define AHCI_PX_CMD_FBSCP          (0x400000)
+#define AHCI_PX_CMD_APSTE          (0x800000)
+#define AHCI_PX_CMD_ATAPI         (0x1000000)
+#define AHCI_PX_CMD_DLAE          (0x2000000)
+#define AHCI_PX_CMD_ALPE          (0x4000000)
+#define AHCI_PX_CMD_ASP           (0x8000000)
+#define AHCI_PX_CMD_ICC          (0xF0000000)
+
+#define AHCI_PX_RES1                      (7)
+
+#define AHCI_PX_TFD                       (8)
+#define AHCI_PX_TFD_STS                (0xFF)
+#define AHCI_PX_TFD_STS_ERR            (0x01)
+#define AHCI_PX_TFD_STS_CS1            (0x06)
+#define AHCI_PX_TFD_STS_DRQ            (0x08)
+#define AHCI_PX_TFD_STS_CS2            (0x70)
+#define AHCI_PX_TFD_STS_BSY            (0x80)
+#define AHCI_PX_TFD_ERR              (0xFF00)
+#define AHCI_PX_TFD_RESERVED     (0xFFFF0000)
+
+#define AHCI_PX_SIG                       (9)
+#define AHCI_PX_SIG_SECTOR_COUNT       (0xFF)
+#define AHCI_PX_SIG_LBA_LOW          (0xFF00)
+#define AHCI_PX_SIG_LBA_MID        (0xFF0000)
+#define AHCI_PX_SIG_LBA_HIGH     (0xFF000000)
+
+#define AHCI_PX_SSTS                     (10)
+#define AHCI_PX_SSTS_DET               (0x0F)
+#define AHCI_PX_SSTS_SPD               (0xF0)
+#define AHCI_PX_SSTS_IPM              (0xF00)
+#define AHCI_PX_SSTS_RESERVED    (0xFFFFF000)
+#define SSTS_DET_NO_DEVICE             (0x00)
+#define SSTS_DET_PRESENT               (0x01)
+#define SSTS_DET_ESTABLISHED           (0x03)
+#define SSTS_DET_OFFLINE               (0x04)
+
+#define AHCI_PX_SCTL                     (11)
+
+#define AHCI_PX_SERR                     (12)
+#define AHCI_PX_SERR_ERR             (0xFFFF)
+#define AHCI_PX_SERR_DIAG        (0xFFFF0000)
+#define  AHCI_PX_SERR_DIAG_X     (0x04000000)
+
+#define AHCI_PX_SACT                     (13)
+#define AHCI_PX_CI                       (14)
+#define AHCI_PX_SNTF                     (15)
+
+#define AHCI_PX_FBS                      (16)
+#define AHCI_PX_FBS_EN                  (0x1)
+#define AHCI_PX_FBS_DEC                 (0x2)
+#define AHCI_PX_FBS_SDE                 (0x4)
+#define AHCI_PX_FBS_DEV               (0xF00)
+#define AHCI_PX_FBS_ADO              (0xF000)
+#define AHCI_PX_FBS_DWE             (0xF0000)
+#define AHCI_PX_FBS_RESERVED     (0xFFF000F8)
+
+#define AHCI_PX_RES2                     (17)
+#define AHCI_PX_VS                       (28)
+
+#define HBA_DATA_REGION_SIZE            (256)
+#define HBA_PORT_DATA_SIZE              (128)
+#define HBA_PORT_NUM_REG (HBA_PORT_DATA_SIZE/4)
+
+#define AHCI_VERSION_0_95        (0x00000905)
+#define AHCI_VERSION_1_0         (0x00010000)
+#define AHCI_VERSION_1_1         (0x00010100)
+#define AHCI_VERSION_1_2         (0x00010200)
+#define AHCI_VERSION_1_3         (0x00010300)
+
+typedef struct hba_cap {
+    uint32_t cap;
+    uint32_t cap2;
+} hba_cap;
+
 /*** Globals ***/
 static QGuestAllocator *guest_malloc;
 static QPCIBus *pcibus;
@@ -62,13 +266,29 @@ static bool ahci_pedantic;
 static uint32_t ahci_fingerprint;
 
 /*** Macro Utilities ***/
+#define bitany(data, mask) (((data) & (mask)) != 0)
+#define bitset(data, mask) (((data) & (mask)) == (mask))
+#define bitclr(data, mask) (((data) & (mask)) == 0)
 #define assert_bit_set(data, mask) g_assert_cmphex((data) & (mask), ==, (mask))
 #define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0)
 
+/*** IO macros for the AHCI memory registers. ***/
+#define ahci_read(OFST) qpci_io_readl(ahci, hba_base + (OFST))
+#define ahci_write(OFST, VAL) qpci_io_writel(ahci, hba_base + (OFST), (VAL))
+#define ahci_rreg(regno)      ahci_read(4 * (regno))
+#define ahci_wreg(regno, val) ahci_write(4 * (regno), (val))
+
+/*** IO macros for port-specific offsets inside of AHCI memory. ***/
+#define px_ofst(port, regno) (HBA_PORT_NUM_REG * (port) + AHCI_PORTS + (regno))
+#define px_rreg(port, regno)      ahci_rreg(px_ofst((port), (regno)))
+#define px_wreg(port, regno, val) ahci_wreg(px_ofst((port), (regno)), (val))
+
 /*** Function Declarations ***/
 static QPCIDevice *get_ahci_device(void);
 static QPCIDevice *start_ahci_device(QPCIDevice *dev, void **hba_base);
 static void free_ahci_device(QPCIDevice *dev);
+static void ahci_test_port_spec(QPCIDevice *ahci, void *hba_base,
+                                hba_cap *hcap, uint8_t port);
 static void ahci_test_pci_spec(QPCIDevice *ahci);
 static void ahci_test_pci_caps(QPCIDevice *ahci, uint16_t header,
                                uint8_t offset);
@@ -445,6 +665,325 @@ static void ahci_test_pmcap(QPCIDevice *ahci, uint8_t offset)
     assert_bit_clear(dataw, PCI_PM_CTRL_DATA_SCALE_MASK);
 }
 
+static void ahci_test_hba_spec(QPCIDevice *ahci, void *hba_base)
+{
+    struct hba_cap hcap;
+    unsigned i;
+    uint32_t cap, cap2, reg;
+    uint32_t ports;
+    uint8_t nports_impl;
+    uint8_t maxports;
+
+    g_assert(ahci != 0);
+    g_assert(hba_base != 0);
+
+    /*
+     * Note that the AHCI spec does expect the BIOS to set up a few things:
+     * CAP.SSS    - Support for staggered spin-up            (t/f)
+     * CAP.SMPS   - Support for mechanical presence switches (t/f)
+     * PI         - Ports Implemented                        (1-32)
+     * PxCMD.HPCP - Hot Plug Capable Port
+     * PxCMD.MPSP - Mechanical Presence Switch Present
+     * PxCMD.CPD  - Cold Presence Detection support
+     *
+     * Additional items are touched if CAP.SSS is on, see AHCI 10.1.1 p.97:
+     * Foreach Port Implemented:
+     * -PxCMD.ST, PxCMD.CR, PxCMD.FRE, PxCMD.FR, PxSCTL.DET are 0
+     * -PxCLB/U and PxFB/U are set to valid regions in memory
+     * -PxSUD is set to 1.
+     * -PxSSTS.DET is polled for presence; if detected, we continue:
+     * -PxSERR is cleared with 1's.
+     * -If PxTFD.STS.BSY, PxTFD.STS.DRQ, and PxTFD.STS.ERR are all zero,
+     *  the device is ready.
+     */
+
+    /* 1 CAP - Capabilities Register */
+    cap = ahci_rreg(AHCI_CAP);
+    assert_bit_clear(cap, AHCI_CAP_RESERVED);
+
+    /* 2 GHC - Global Host Control */
+    reg = ahci_rreg(AHCI_GHC);
+    assert_bit_clear(reg, AHCI_GHC_HR);
+    assert_bit_clear(reg, AHCI_GHC_IE);
+    assert_bit_clear(reg, AHCI_GHC_MRSM);
+    if (bitset(cap, AHCI_CAP_SAM)) {
+        g_test_message("Supports AHCI-Only Mode: GHC_AE is Read-Only.");
+        assert_bit_set(reg, AHCI_GHC_AE);
+    } else {
+        g_test_message("Supports AHCI/Legacy mix.");
+        assert_bit_clear(reg, AHCI_GHC_AE);
+    }
+
+    /* 3 IS - Interrupt Status */
+    reg = ahci_rreg(AHCI_IS);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* 4 PI - Ports Implemented */
+    ports = ahci_rreg(AHCI_PI);
+    /* Ports Implemented must be non-zero. */
+    g_assert_cmphex(ports, !=, 0);
+    /* Ports Implemented must be <= Number of Ports. */
+    nports_impl = ctpopl(ports);
+    g_assert_cmpuint(((AHCI_CAP_NP & cap) + 1), >=, nports_impl);
+
+    g_assert_cmphex(barsize, >, 0);
+    /* Ports must be within the proper range. Given a mapping of SIZE,
+     * 256 bytes are used for global HBA control, and the rest is used
+     * for ports data, at 0x80 bytes each. */
+    maxports = (barsize - HBA_DATA_REGION_SIZE) / HBA_PORT_DATA_SIZE;
+    /* e.g, 30 ports for 4K of memory. (4096 - 256) / 128 = 30 */
+    g_assert_cmphex((reg >> maxports), ==, 0);
+
+    /* 5 AHCI Version */
+    reg = ahci_rreg(AHCI_VS);
+    switch (reg) {
+    case AHCI_VERSION_0_95:
+    case AHCI_VERSION_1_0:
+    case AHCI_VERSION_1_1:
+    case AHCI_VERSION_1_2:
+    case AHCI_VERSION_1_3:
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    /* 6 Command Completion Coalescing Control: depends on CAP.CCCS. */
+    reg = ahci_rreg(AHCI_CCCCTL);
+    if (bitset(cap, AHCI_CAP_CCCS)) {
+        assert_bit_clear(reg, AHCI_CCCCTL_EN);
+        assert_bit_clear(reg, AHCI_CCCCTL_RESERVED);
+        assert_bit_set(reg, AHCI_CCCCTL_CC);
+        assert_bit_set(reg, AHCI_CCCCTL_TV);
+    } else {
+        g_assert_cmphex(reg, ==, 0);
+    }
+
+    /* 7 CCC_PORTS */
+    reg = ahci_rreg(AHCI_CCCPORTS);
+    /* Must be zeroes initially regardless of CAP.CCCS */
+    g_assert_cmphex(reg, ==, 0);
+
+    /* 8 EM_LOC */
+    reg = ahci_rreg(AHCI_EMLOC);
+    if (bitclr(cap, AHCI_CAP_EMS)) {
+        g_assert_cmphex(reg, ==, 0);
+    }
+
+    /* 9 EM_CTL */
+    reg = ahci_rreg(AHCI_EMCTL);
+    if (bitset(cap, AHCI_CAP_EMS)) {
+        assert_bit_clear(reg, AHCI_EMCTL_STSMR);
+        assert_bit_clear(reg, AHCI_EMCTL_CTLTM);
+        assert_bit_clear(reg, AHCI_EMCTL_CTLRST);
+        assert_bit_clear(reg, AHCI_EMCTL_RESERVED);
+    } else {
+        g_assert_cmphex(reg, ==, 0);
+    }
+
+    /* 10 CAP2 -- Capabilities Extended */
+    cap2 = ahci_rreg(AHCI_CAP2);
+    assert_bit_clear(cap2, AHCI_CAP2_RESERVED);
+
+    /* 11 BOHC -- Bios/OS Handoff Control */
+    reg = ahci_rreg(AHCI_BOHC);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* 12 -- 23: Reserved */
+    g_test_message("Verifying HBA reserved area is empty.");
+    for (i = AHCI_RESERVED; i < AHCI_NVMHCI; ++i) {
+        reg = ahci_rreg(i);
+        g_assert_cmphex(reg, ==, 0);
+    }
+
+    /* 24 -- 39: NVMHCI */
+    if (bitclr(cap2, AHCI_CAP2_NVMP)) {
+        g_test_message("Verifying HBA/NVMHCI area is empty.");
+        for (i = AHCI_NVMHCI; i < AHCI_VENDOR; ++i) {
+            reg = ahci_rreg(i);
+            g_assert_cmphex(reg, ==, 0);
+        }
+    }
+
+    /* 40 -- 63: Vendor */
+    g_test_message("Verifying HBA/Vendor area is empty.");
+    for (i = AHCI_VENDOR; i < AHCI_PORTS; ++i) {
+        reg = ahci_rreg(i);
+        g_assert_cmphex(reg, ==, 0);
+    }
+
+    /* 64 -- XX: Port Space */
+    hcap.cap = cap;
+    hcap.cap2 = cap2;
+    for (i = 0; ports || (i < maxports); ports >>= 1, ++i) {
+        if (bitset(ports, 0x1)) {
+            g_test_message("Testing port %u for spec", i);
+            ahci_test_port_spec(ahci, hba_base, &hcap, i);
+        } else {
+            uint16_t j;
+            uint16_t low = AHCI_PORTS + (32 * i);
+            uint16_t high = AHCI_PORTS + (32 * (i + 1));
+            g_test_message("Asserting unimplemented port %u "
+                           "(reg [%u-%u]) is empty.",
+                           i, low, high - 1);
+            for (j = low; j < high; ++j) {
+                reg = ahci_rreg(j);
+                g_assert_cmphex(reg, ==, 0);
+            }
+        }
+    }
+}
+
+/**
+ * Test the memory space for one port for specification adherence.
+ */
+static void ahci_test_port_spec(QPCIDevice *ahci, void *hba_base,
+                                hba_cap *hcap, uint8_t port)
+{
+    uint32_t reg;
+    unsigned i;
+
+    /* (0) CLB */
+    reg = px_rreg(port, AHCI_PX_CLB);
+    assert_bit_clear(reg, AHCI_PX_CLB_RESERVED);
+
+    /* (1) CLBU */
+    if (bitclr(hcap->cap, AHCI_CAP_S64A)) {
+        reg = px_rreg(port, AHCI_PX_CLBU);
+        g_assert_cmphex(reg, ==, 0);
+    }
+
+    /* (2) FB */
+    reg = px_rreg(port, AHCI_PX_FB);
+    assert_bit_clear(reg, AHCI_PX_FB_RESERVED);
+
+    /* (3) FBU */
+    if (bitclr(hcap->cap, AHCI_CAP_S64A)) {
+        reg = px_rreg(port, AHCI_PX_FBU);
+        g_assert_cmphex(reg, ==, 0);
+    }
+
+    /* (4) IS */
+    reg = px_rreg(port, AHCI_PX_IS);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* (5) IE */
+    reg = px_rreg(port, AHCI_PX_IE);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* (6) CMD */
+    reg = px_rreg(port, AHCI_PX_CMD);
+    assert_bit_clear(reg, AHCI_PX_CMD_FRE);
+    assert_bit_clear(reg, AHCI_PX_CMD_RESERVED);
+    assert_bit_clear(reg, AHCI_PX_CMD_CCS);
+    assert_bit_clear(reg, AHCI_PX_CMD_FR);
+    assert_bit_clear(reg, AHCI_PX_CMD_CR);
+    assert_bit_clear(reg, AHCI_PX_CMD_PMA); /* And RW only if CAP.SPM */
+    assert_bit_clear(reg, AHCI_PX_CMD_APSTE); /* RW only if CAP2.APST */
+    assert_bit_clear(reg, AHCI_PX_CMD_ATAPI);
+    assert_bit_clear(reg, AHCI_PX_CMD_DLAE);
+    assert_bit_clear(reg, AHCI_PX_CMD_ALPE);  /* RW only if CAP.SALP */
+    assert_bit_clear(reg, AHCI_PX_CMD_ASP);   /* RW only if CAP.SALP */
+    assert_bit_clear(reg, AHCI_PX_CMD_ICC);
+    /* If CPDetect support does not exist, CPState must be off. */
+    if (bitclr(reg, AHCI_PX_CMD_CPD)) {
+        assert_bit_clear(reg, AHCI_PX_CMD_CPS);
+    }
+    /* If MPSPresence is not set, MPSState must be off. */
+    if (bitclr(reg, AHCI_PX_CMD_MPSP)) {
+        assert_bit_clear(reg, AHCI_PX_CMD_MPSS);
+    }
+    /* If we do not support MPS, MPSS and MPSP must be off. */
+    if (bitclr(hcap->cap, AHCI_CAP_SMPS)) {
+        assert_bit_clear(reg, AHCI_PX_CMD_MPSS);
+        assert_bit_clear(reg, AHCI_PX_CMD_MPSP);
+    }
+    /* If, via CPD or MPSP we detect a drive, HPCP must be on. */
+    if (bitany(reg, AHCI_PX_CMD_CPD || AHCI_PX_CMD_MPSP)) {
+        assert_bit_set(reg, AHCI_PX_CMD_HPCP);
+    }
+    /* HPCP and ESP cannot both be active. */
+    g_assert_false(bitset(reg, AHCI_PX_CMD_HPCP | AHCI_PX_CMD_ESP));
+    /* If CAP.FBSS is not set, FBSCP must not be set. */
+    if (bitclr(hcap->cap, AHCI_CAP_FBSS)) {
+        assert_bit_clear(reg, AHCI_PX_CMD_FBSCP);
+    }
+
+    /* (7) RESERVED */
+    reg = px_rreg(port, AHCI_PX_RES1);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* (8) TFD */
+    reg = px_rreg(port, AHCI_PX_TFD);
+    /* At boot, prior to an FIS being received, the TFD register should be 0x7F,
+     * which breaks down as follows, as seen in AHCI 1.3 sec 3.3.8, p. 27. */
+    assert_bit_set(reg, AHCI_PX_TFD_STS_ERR);
+    assert_bit_set(reg, AHCI_PX_TFD_STS_CS1);
+    assert_bit_set(reg, AHCI_PX_TFD_STS_DRQ);
+    assert_bit_set(reg, AHCI_PX_TFD_STS_CS2);
+    assert_bit_clear(reg, AHCI_PX_TFD_STS_BSY);
+    assert_bit_clear(reg, AHCI_PX_TFD_ERR);
+    assert_bit_clear(reg, AHCI_PX_TFD_RESERVED);
+
+    /* (9) SIG */
+    /* Though AHCI specifies the boot value should be 0xFFFFFFFF,
+     * Even when GHC.ST is zero, the AHCI HBA may receive the initial
+     * D2H register FIS and update the signature asynchronously,
+     * so we cannot expect a value here. AHCI 1.3, sec 3.3.9, pp 27-28 */
+
+    /* (10) SSTS / SCR0: SStatus */
+    reg = px_rreg(port, AHCI_PX_SSTS);
+    assert_bit_clear(reg, AHCI_PX_SSTS_RESERVED);
+    /* Even though the register should be 0 at boot, it is asynchronous and
+     * prone to change, so we cannot test any well known value. */
+
+    /* (11) SCTL / SCR2: SControl */
+    reg = px_rreg(port, AHCI_PX_SCTL);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* (12) SERR / SCR1: SError */
+    reg = px_rreg(port, AHCI_PX_SERR);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* (13) SACT / SCR3: SActive */
+    reg = px_rreg(port, AHCI_PX_SACT);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* (14) CI */
+    reg = px_rreg(port, AHCI_PX_CI);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* (15) SNTF */
+    reg = px_rreg(port, AHCI_PX_SNTF);
+    g_assert_cmphex(reg, ==, 0);
+
+    /* (16) FBS */
+    reg = px_rreg(port, AHCI_PX_FBS);
+    assert_bit_clear(reg, AHCI_PX_FBS_EN);
+    assert_bit_clear(reg, AHCI_PX_FBS_DEC);
+    assert_bit_clear(reg, AHCI_PX_FBS_SDE);
+    assert_bit_clear(reg, AHCI_PX_FBS_DEV);
+    assert_bit_clear(reg, AHCI_PX_FBS_DWE);
+    assert_bit_clear(reg, AHCI_PX_FBS_RESERVED);
+    if (bitset(hcap->cap, AHCI_CAP_FBSS)) {
+        /* if Port-Multiplier FIS-based switching avail, ADO must >= 2 */
+        g_assert((reg & AHCI_PX_FBS_ADO) >> ctzl(AHCI_PX_FBS_ADO) >= 2);
+    }
+
+    /* [17 -- 27] RESERVED */
+    for (i = AHCI_PX_RES2; i < AHCI_PX_VS; ++i) {
+        reg = px_rreg(port, i);
+        g_assert_cmphex(reg, ==, 0);
+    }
+
+    /* [28 -- 31] Vendor-Specific */
+    for (i = AHCI_PX_VS; i < 32; ++i) {
+        reg = px_rreg(port, i);
+        if (reg) {
+            g_test_message("INFO: Vendor register %u non-empty", i);
+        }
+    }
+}
+
 /******************************************************************************/
 /* Test Interfaces                                                            */
 /******************************************************************************/
@@ -484,6 +1023,21 @@ static void test_pci_enable(void)
     ahci_shutdown(ahci);
 }
 
+/**
+ * Investigate the memory mapped regions of the HBA,
+ * and test them for AHCI specification adherence.
+ */
+static void test_hba_spec(void)
+{
+    QPCIDevice *ahci;
+    void *hba_base;
+
+    ahci = ahci_boot();
+    ahci_pci_enable(ahci, &hba_base);
+    ahci_test_hba_spec(ahci, hba_base);
+    ahci_shutdown(ahci);
+}
+
 /******************************************************************************/
 
 int main(int argc, char **argv)
@@ -536,6 +1090,7 @@ int main(int argc, char **argv)
     qtest_add_func("/ahci/sanity",     test_sanity);
     qtest_add_func("/ahci/pci_spec",   test_pci_spec);
     qtest_add_func("/ahci/pci_enable", test_pci_enable);
+    qtest_add_func("/ahci/hba_spec",   test_hba_spec);
 
     ret = g_test_run();
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v3 31/32] ahci: Add test_hba_enable to ahci-test.
  2014-08-13 21:56 [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3 John Snow
                   ` (5 preceding siblings ...)
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 30/32] ahci: Add test_hba_spec to ahci-test John Snow
@ 2014-08-13 21:56 ` John Snow
  2014-08-14 16:46   ` Stefan Hajnoczi
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 32/32] ahci: Add test_identify case " John Snow
  7 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2014-08-13 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru, stefanha, mst

This test engages the HBA functionality and initializes
values to sane defaults to allow for minimal HBA functionality.

Buffers are allocated and pointers are updated to allow minimal
I/O commands to complete as expected. Error registers and responses
are sanity checked for specification adherence.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 85f8661..31880ed 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -277,11 +277,17 @@ static uint32_t ahci_fingerprint;
 #define ahci_write(OFST, VAL) qpci_io_writel(ahci, hba_base + (OFST), (VAL))
 #define ahci_rreg(regno)      ahci_read(4 * (regno))
 #define ahci_wreg(regno, val) ahci_write(4 * (regno), (val))
+#define ahci_set(regno, mask) ahci_wreg((regno), ahci_rreg(regno) | (mask))
+#define ahci_clr(regno, mask) ahci_wreg((regno), ahci_rreg(regno) & ~(mask))
 
 /*** IO macros for port-specific offsets inside of AHCI memory. ***/
 #define px_ofst(port, regno) (HBA_PORT_NUM_REG * (port) + AHCI_PORTS + (regno))
 #define px_rreg(port, regno)      ahci_rreg(px_ofst((port), (regno)))
 #define px_wreg(port, regno, val) ahci_wreg(px_ofst((port), (regno)), (val))
+#define px_set(port, reg, mask)   px_wreg((port), (reg),                \
+                                          px_rreg((port), (reg)) | (mask));
+#define px_clr(port, reg, mask)   px_wreg((port), (reg),                \
+                                          px_rreg((port), (reg)) & ~(mask));
 
 /*** Function Declarations ***/
 static QPCIDevice *get_ahci_device(void);
@@ -432,6 +438,140 @@ static QPCIDevice *start_ahci_device(QPCIDevice *ahci, void **hba_base)
     return ahci;
 }
 
+/**
+ * Test and initialize the AHCI's HBA memory areas.
+ * Initialize and start any ports with devices attached.
+ * Bring the HBA into the idle state.
+ */
+static void ahci_hba_enable(QPCIDevice *ahci, void *hba_base)
+{
+    /* Bits of interest in this section:
+     * GHC.AE     Global Host Control / AHCI Enable
+     * PxCMD.ST   Port Command: Start
+     * PxCMD.SUD  "Spin Up Device"
+     * PxCMD.POD  "Power On Device"
+     * PxCMD.FRE  "FIS Receive Enable"
+     * PxCMD.FR   "FIS Receive Running"
+     * PxCMD.CR   "Command List Running"
+     */
+
+    g_assert(ahci != NULL);
+    g_assert(hba_base != NULL);
+
+    uint32_t reg, ports_impl, clb, fb;
+    uint16_t i;
+    uint8_t num_cmd_slots;
+
+    g_assert(hba_base != 0);
+
+    /* Set GHC.AE to 1 */
+    ahci_set(AHCI_GHC, AHCI_GHC_AE);
+    reg = ahci_rreg(AHCI_GHC);
+    assert_bit_set(reg, AHCI_GHC_AE);
+
+    /* Read CAP.NCS, how many command slots do we have? */
+    reg = ahci_rreg(AHCI_CAP);
+    num_cmd_slots = ((reg & AHCI_CAP_NCS) >> ctzl(AHCI_CAP_NCS)) + 1;
+    g_test_message("Number of Command Slots: %u", num_cmd_slots);
+
+    /* Determine which ports are implemented. */
+    ports_impl = ahci_rreg(AHCI_PI);
+
+    for (i = 0; ports_impl; ports_impl >>= 1, ++i) {
+        if (!(ports_impl & 0x01)) {
+            continue;
+        }
+
+        g_test_message("Initializing port %u", i);
+
+        reg = px_rreg(i, AHCI_PX_CMD);
+        if (bitclr(reg, AHCI_PX_CMD_ST | AHCI_PX_CMD_CR |
+                   AHCI_PX_CMD_FRE | AHCI_PX_CMD_FR)) {
+            g_test_message("port is idle");
+        } else {
+            g_test_message("port needs to be idled");
+            px_clr(i, AHCI_PX_CMD, (AHCI_PX_CMD_ST | AHCI_PX_CMD_FRE));
+            /* The port has 500ms to disengage. */
+            usleep(500000);
+            reg = px_rreg(i, AHCI_PX_CMD);
+            assert_bit_clear(reg, AHCI_PX_CMD_CR);
+            assert_bit_clear(reg, AHCI_PX_CMD_FR);
+            g_test_message("port is now idle");
+            /* The spec does allow for possibly needing a PORT RESET
+             * or HBA reset if we fail to idle the port. */
+        }
+
+        /* Allocate Memory for the Command List Buffer & FIS Buffer */
+        /* PxCLB space ... 0x20 per command, as in 4.2.2 p 36 */
+        clb = guest_alloc(guest_malloc, num_cmd_slots * 0x20);
+        g_test_message("CLB: 0x%08x", clb);
+        px_wreg(i, AHCI_PX_CLB, clb);
+        g_assert_cmphex(clb, ==, px_rreg(i, AHCI_PX_CLB));
+
+        /* PxFB space ... 0x100, as in 4.2.1 p 35 */
+        fb = guest_alloc(guest_malloc, 0x100);
+        g_test_message("FB: 0x%08x", fb);
+        px_wreg(i, AHCI_PX_FB, fb);
+        g_assert_cmphex(fb, ==, px_rreg(i, AHCI_PX_FB));
+
+        /* Clear PxSERR, PxIS, then IS.IPS[x] by writing '1's. */
+        px_wreg(i, AHCI_PX_SERR, 0xFFFFFFFF);
+        px_wreg(i, AHCI_PX_IS, 0xFFFFFFFF);
+        ahci_wreg(AHCI_IS, (1 << i));
+
+        /* Verify Interrupts Cleared */
+        reg = px_rreg(i, AHCI_PX_SERR);
+        g_assert_cmphex(reg, ==, 0);
+
+        reg = px_rreg(i, AHCI_PX_IS);
+        g_assert_cmphex(reg, ==, 0);
+
+        reg = ahci_rreg(AHCI_IS);
+        assert_bit_clear(reg, (1 << i));
+
+        /* Enable All Interrupts: */
+        px_wreg(i, AHCI_PX_IE, 0xFFFFFFFF);
+        reg = px_rreg(i, AHCI_PX_IE);
+        g_assert_cmphex(reg, ==, ~((uint32_t)AHCI_PX_IE_RESERVED));
+
+        /* Enable the FIS Receive Engine. */
+        px_set(i, AHCI_PX_CMD, AHCI_PX_CMD_FRE);
+        reg = px_rreg(i, AHCI_PX_CMD);
+        assert_bit_set(reg, AHCI_PX_CMD_FR);
+
+        /* AHCI 1.3 spec: if !STS.BSY, !STS.DRQ and PxSSTS.DET indicates
+         * physical presence, a device is present and may be started. However,
+         * PxSERR.DIAG.X /may/ need to be cleared a priori. */
+        reg = px_rreg(i, AHCI_PX_SERR);
+        if (bitset(reg, AHCI_PX_SERR_DIAG_X)) {
+            px_set(i, AHCI_PX_SERR, AHCI_PX_SERR_DIAG_X);
+        }
+
+        reg = px_rreg(i, AHCI_PX_TFD);
+        if (bitclr(reg, AHCI_PX_TFD_STS_BSY | AHCI_PX_TFD_STS_DRQ)) {
+            reg = px_rreg(i, AHCI_PX_SSTS);
+            if ((reg & AHCI_PX_SSTS_DET) == SSTS_DET_ESTABLISHED) {
+                /* Device Found: set PxCMD.ST := 1 */
+                px_set(i, AHCI_PX_CMD, AHCI_PX_CMD_ST);
+                assert_bit_set(px_rreg(i, AHCI_PX_CMD), AHCI_PX_CMD_CR);
+                g_test_message("Started Device %u", i);
+            } else if ((reg & AHCI_PX_SSTS_DET)) {
+                /* Device present, but in some unknown state. */
+                g_assert_not_reached();
+            }
+        }
+    }
+
+    /* Enable GHC.IE */
+    ahci_set(AHCI_GHC, AHCI_GHC_IE);
+    reg = ahci_rreg(AHCI_GHC);
+    assert_bit_set(reg, AHCI_GHC_IE);
+
+    /* TODO: The device should now be idling and waiting for commands.
+     * In the future, a small test-case to inspect the Register D2H FIS
+     * and clear the initial interrupts might be good. */
+}
+
 /*** Specification Adherence Tests ***/
 
 /**
@@ -1038,6 +1178,21 @@ static void test_hba_spec(void)
     ahci_shutdown(ahci);
 }
 
+/**
+ * Engage the HBA functionality of the AHCI PCI device,
+ * and bring it into a functional idle state.
+ */
+static void test_hba_enable(void)
+{
+    QPCIDevice *ahci;
+    void *hba_base;
+
+    ahci = ahci_boot();
+    ahci_pci_enable(ahci, &hba_base);
+    ahci_hba_enable(ahci, hba_base);
+    ahci_shutdown(ahci);
+}
+
 /******************************************************************************/
 
 int main(int argc, char **argv)
@@ -1091,6 +1246,7 @@ int main(int argc, char **argv)
     qtest_add_func("/ahci/pci_spec",   test_pci_spec);
     qtest_add_func("/ahci/pci_enable", test_pci_enable);
     qtest_add_func("/ahci/hba_spec",   test_hba_spec);
+    qtest_add_func("/ahci/hba_enable", test_hba_enable);
 
     ret = g_test_run();
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v3 32/32] ahci: Add test_identify case to ahci-test.
  2014-08-13 21:56 [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3 John Snow
                   ` (6 preceding siblings ...)
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 31/32] ahci: Add test_hba_enable " John Snow
@ 2014-08-13 21:56 ` John Snow
  2014-08-14 16:52   ` Stefan Hajnoczi
  7 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2014-08-13 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru, stefanha, mst

Utilizing all of the bring-up code in pci_enable and hba_enable,
this test issues a simple IDENTIFY command via the HBA and retrieves
the response via the PIO receive mechanisms of the HBA.

Bugs: The DPS interrupt (Descriptor Processed Status) does not
currently get set. This will need to be adjusted in a future
patch series when the AHCI DMA pathways are reworked to allow
the feature, which may be utilized by OSX guests.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 31880ed..3c51d27 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -252,6 +252,97 @@
 #define AHCI_VERSION_1_2         (0x00010200)
 #define AHCI_VERSION_1_3         (0x00010300)
 
+/*** Structures ***/
+
+/**
+ * Generic FIS structure.
+ */
+struct fis {
+    uint8_t fis_type;
+    uint8_t flags;
+    char data[0];
+} __attribute__((__packed__));
+
+/**
+ * Register device-to-host FIS structure.
+ */
+struct reg_d2h_fis {
+    /* DW0 */
+    uint8_t fis_type;
+    uint8_t flags;
+    uint8_t status;
+    uint8_t error;
+    /* DW1 */
+    uint8_t lba_low;
+    uint8_t lba_mid;
+    uint8_t lba_high;
+    uint8_t device;
+    /* DW2 */
+    uint8_t lba3;
+    uint8_t lba4;
+    uint8_t lba5;
+    uint8_t res1;
+    /* DW3 */
+    uint16_t count;
+    uint8_t res2;
+    uint8_t res3;
+    /* DW4 */
+    uint16_t res4;
+    uint16_t res5;
+} __attribute__((__packed__));
+
+/**
+ * Register host-to-device FIS structure.
+ */
+struct reg_h2d_fis {
+    /* DW0 */
+    uint8_t fis_type;
+    uint8_t flags;
+    uint8_t command;
+    uint8_t feature_low;
+    /* DW1 */
+    uint8_t lba_low;
+    uint8_t lba_mid;
+    uint8_t lba_high;
+    uint8_t device;
+    /* DW2 */
+    uint8_t lba3;
+    uint8_t lba4;
+    uint8_t lba5;
+    uint8_t feature_high;
+    /* DW3 */
+    uint16_t count;
+    uint8_t icc;
+    uint8_t control;
+    /* DW4 */
+    uint32_t aux;
+} __attribute__((__packed__));
+
+/**
+ * Command List entry structure.
+ * The command list contains between 1-32 of these structures.
+ */
+struct ahci_command {
+    uint8_t b1;
+    uint8_t b2;
+    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__));
+
+/**
+ * Physical Region Descriptor; pointed to by the Command List Header,
+ * struct ahci_command.
+ */
+struct prd {
+    uint32_t dba;  /* Data Base Address */
+    uint32_t dbau; /* Data Base Address Upper */
+    uint32_t res;  /* Reserved */
+    uint32_t dbc;  /* Data Byte Count (0-indexed) & Interrupt Flag (bit 2^31) */
+};
+
 typedef struct hba_cap {
     uint32_t cap;
     uint32_t cap2;
@@ -289,6 +380,10 @@ static uint32_t ahci_fingerprint;
 #define px_clr(port, reg, mask)   px_wreg((port), (reg),                \
                                           px_rreg((port), (reg)) & ~(mask));
 
+/* For calculating how big the PRD table needs to be: */
+#define cmd_tbl_siz(n) ((0x80 + ((n) * sizeof(struct prd)) + 0x7F) & ~0x7F)
+
+
 /*** Function Declarations ***/
 static QPCIDevice *get_ahci_device(void);
 static QPCIDevice *start_ahci_device(QPCIDevice *dev, void **hba_base);
@@ -304,6 +399,17 @@ static void ahci_test_pmcap(QPCIDevice *ahci, uint8_t offset);
 
 /*** Utilities ***/
 
+static void string_cpu_to_be16(uint16_t *s, size_t bytes)
+{
+    g_assert_cmphex((bytes & 1), ==, 0);
+    bytes /= 2;
+
+    while (bytes--) {
+        *s = cpu_to_be16(*s);
+        s++;
+    }
+}
+
 /**
  * Locate, verify, and return a handle to the AHCI device.
  */
@@ -1124,6 +1230,180 @@ static void ahci_test_port_spec(QPCIDevice *ahci, void *hba_base,
     }
 }
 
+/**
+ * Utilizing an initialized AHCI HBA, issue an IDENTIFY command to the first
+ * device we see, then read and check the response.
+ */
+static void ahci_test_identify(QPCIDevice *ahci, void *hba_base)
+{
+    struct reg_d2h_fis *d2h = g_malloc0(0x20);
+    struct reg_d2h_fis *pio = g_malloc0(0x20);
+    struct reg_h2d_fis fis;
+    struct ahci_command cmd;
+    struct prd prd;
+    uint32_t ports, reg, clb, table, fb, data_ptr;
+    uint16_t buff[256];
+    unsigned i;
+    int rc;
+
+    g_assert(ahci != NULL);
+    g_assert(hba_base != 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
+     *     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
+     *     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.
+     */
+
+    /* Pick the first implemented and running port */
+    ports = ahci_rreg(AHCI_PI);
+    for (i = 0; i < 32; ports >>= 1, ++i) {
+        if (ports == 0) {
+            i = 32;
+        }
+
+        if (!(ports & 0x01)) {
+            continue;
+        }
+
+        reg = px_rreg(i, AHCI_PX_CMD);
+        if (bitset(reg, AHCI_PX_CMD_ST)) {
+            break;
+        }
+    }
+    g_assert_cmphex(i, <, 32);
+    g_test_message("Selected port %u for test", i);
+
+    /* Clear out this port's interrupts (ignore the init register d2h fis) */
+    reg = px_rreg(i, AHCI_PX_IS);
+    px_wreg(i, AHCI_PX_IS, reg);
+    g_assert_cmphex(px_rreg(i, AHCI_PX_IS), ==, 0);
+
+    /* Wipe the FIS-Recieve Buffer */
+    fb = px_rreg(i, AHCI_PX_FB);
+    g_assert_cmphex(fb, !=, 0);
+    qmemset(fb, 0x00, 0x100);
+
+    /* 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 = guest_alloc(guest_malloc, cmd_tbl_siz(1));
+    g_assert(table);
+    assert_bit_clear(table, 0x7F);
+
+    /* Create a data buffer ... where we will dump the IDENTIFY data to. */
+    data_ptr = guest_alloc(guest_malloc, 512);
+    g_assert(data_ptr);
+
+    /* Grab the Command List Buffer pointer */
+    clb = px_rreg(i, AHCI_PX_CLB);
+    g_assert(clb);
+
+    /* Copy the existing Command #0 structure from the CLB into local memory,
+     * and build a new command #0. */
+    memread(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 = 1; /* One PRD table entry. */
+    cmd.prdbc = 0;
+    cmd.ctba = table;
+    cmd.ctbau = 0;
+
+    /* Construct our PRD, noting that DBC is 0-indexed. */
+    prd.dba = data_ptr;
+    prd.dbau = 0;
+    prd.res = 0;
+    prd.dbc = 511;
+    prd.dbc |= 0x80000000; /* Request DPS Interrupt */
+
+    /* 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.device = 0;
+    fis.flags = 0x80;    /* Indicate this is a command FIS */
+
+    /* We've committed nothing yet, no interrupts should be posted yet. */
+    g_assert_cmphex(px_rreg(i, AHCI_PX_IS), ==, 0);
+
+    /* Commit the Command FIS to the Command Table */
+    memwrite(table, &fis, sizeof(fis));
+
+    /* Commit the PRD entry to the Command Table */
+    memwrite(table + 0x80, &prd, sizeof(prd));
+
+    /* Commit Command #0, pointing to the Table, to the Command List Buffer. */
+    memwrite(clb, &cmd, sizeof(cmd));
+
+    /* Everything is in place, but we haven't given the go-ahead yet. */
+    g_assert_cmphex(px_rreg(i, AHCI_PX_IS), ==, 0);
+
+    /* Issue Command #0 via PxCI */
+    px_wreg(i, AHCI_PX_CI, (1 << 0));
+    while (bitset(px_rreg(i, AHCI_PX_TFD), AHCI_PX_TFD_STS_BSY)) {
+        usleep(50);
+    }
+
+    /* Check for expected interrupts */
+    reg = px_rreg(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. */
+    px_wreg(i, AHCI_PX_IS, AHCI_PX_IS_DHRS | AHCI_PX_IS_PSS | AHCI_PX_IS_DPS);
+    g_assert_cmphex(px_rreg(i, AHCI_PX_IS), ==, 0);
+
+    /* Check for errors. */
+    reg = px_rreg(i, AHCI_PX_SERR);
+    g_assert_cmphex(reg, ==, 0);
+    reg = px_rreg(i, AHCI_PX_TFD);
+    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(clb, &cmd, sizeof(cmd));
+    g_assert_cmphex(512, ==, cmd.prdbc);
+
+    /* Investigate FIS responses */
+    memread(fb + 0x20, pio, 0x20);
+    memread(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 = px_rreg(i, AHCI_PX_TFD);
+    g_assert_cmphex((reg & AHCI_PX_TFD_ERR), ==, pio->error);
+    g_assert_cmphex((reg & AHCI_PX_TFD_STS), ==, pio->status);
+    /* PIO FIS contains a "bytes read" field, it should match up. */
+    g_assert_cmphex(pio->res4, ==, cmd.prdbc);
+
+    /* Last, but not least: Investigate the IDENTIFY response data. */
+    memread(data_ptr, &buff, 512);
+
+    /* Check serial number/version in the buffer */
+    string_cpu_to_be16(&buff[10], 20);
+    rc = memcmp(&buff[10], "testdisk            ", 20);
+    g_assert_cmphex(rc, ==, 0);
+
+    string_cpu_to_be16(&buff[23], 8);
+    rc = memcmp(&buff[23], "version ", 8);
+    g_assert_cmphex(rc, ==, 0);
+
+    g_free(d2h);
+    g_free(pio);
+}
+
 /******************************************************************************/
 /* Test Interfaces                                                            */
 /******************************************************************************/
@@ -1193,6 +1473,22 @@ static void test_hba_enable(void)
     ahci_shutdown(ahci);
 }
 
+/**
+ * Bring up the device and issue an IDENTIFY command.
+ * Inspect the state of the HBA device and the data returned.
+ */
+static void test_identify(void)
+{
+    QPCIDevice *ahci;
+    void *hba_base;
+
+    ahci = ahci_boot();
+    ahci_pci_enable(ahci, &hba_base);
+    ahci_hba_enable(ahci, hba_base);
+    ahci_test_identify(ahci, hba_base);
+    ahci_shutdown(ahci);
+}
+
 /******************************************************************************/
 
 int main(int argc, char **argv)
@@ -1247,6 +1543,7 @@ int main(int argc, char **argv)
     qtest_add_func("/ahci/pci_enable", test_pci_enable);
     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);
 
     ret = g_test_run();
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 26/32] ahci: MSI capability should be at 0x80, not 0x50.
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 26/32] ahci: MSI capability should be at 0x80, not 0x50 John Snow
@ 2014-08-14  7:15   ` Michael S. Tsirkin
  2014-08-14  7:19   ` Michael S. Tsirkin
  1 sibling, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2014-08-14  7:15 UTC (permalink / raw)
  To: John Snow; +Cc: qemu-devel, stefanha, armbru

On Wed, Aug 13, 2014 at 05:56:09PM -0400, John Snow wrote:
> In the Intel ICH9 data sheet, the MSI capability offset
> in the PCI configuration space for ICH9 AHCI devices is
> specified to be 0x80.
> 
> Further, the PCI capability pointer should always point
> to 0x80 in ICH9 devices, despite the fact that AHCI 1.3
> specifies that it should be pointing to PMCAP (Which in
> this instance would be 0x70) to maintain adherence to
> the Intel data sheet specifications and real observed behavior.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  hw/ide/ich.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ide/ich.c b/hw/ide/ich.c
> index a2f1639..d2a3ac2 100644
> --- a/hw/ide/ich.c
> +++ b/hw/ide/ich.c
> @@ -71,6 +71,7 @@
>  #include <hw/ide/pci.h>
>  #include <hw/ide/ahci.h>
>  
> +#define ICH9_MSI_CAP_OFFSET     0x80
>  #define ICH9_SATA_CAP_OFFSET    0xA8
>  
>  #define ICH9_IDP_BAR            4
> @@ -115,7 +116,6 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>      /* XXX Software should program this register */
>      dev->config[0x90]   = 1 << 6; /* Address Map Register - AHCI mode */
>  
> -    msi_init(dev, 0x50, 1, true, false);
>      d->ahci.irq = pci_allocate_irq(dev);
>  
>      pci_register_bar(dev, ICH9_IDP_BAR, PCI_BASE_ADDRESS_SPACE_IO,
> @@ -135,6 +135,9 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>                   (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4));
>      d->ahci.idp_offset = ICH9_IDP_INDEX;
>  
> +    /* MSI cap should be added last to be first. */

I would put the explanation from the commit log
somewhere around this comment otherwise we risk
someone noting ahci spec says otherwise and
sending a patch "fixing" it.

> +    msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false);
> +
>      return 0;
>  }
>  
> -- 
> 1.9.3

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 27/32] ahci: Add test_pci_spec to ahci-test.
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 27/32] ahci: Add test_pci_spec to ahci-test John Snow
@ 2014-08-14  7:18   ` Michael S. Tsirkin
  2014-08-14 16:03   ` Stefan Hajnoczi
  1 sibling, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2014-08-14  7:18 UTC (permalink / raw)
  To: John Snow; +Cc: qemu-devel, stefanha, armbru

On Wed, Aug 13, 2014 at 05:56:10PM -0400, John Snow wrote:
> Adds a specification adherence test for AHCI
> where the boot-up values for the PCI configuration space
> are compared against the AHCI 1.3 specification.
> 
> This test does not itself attempt to engage the device.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  tests/ahci-test.c | 305 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 299 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index cc49dba..29ac0d0 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -25,7 +25,7 @@
>  #include <stdint.h>
>  #include <string.h>
>  #include <stdio.h>
> -
> +#include <getopt.h>
>  #include <glib.h>
>  
>  #include "libqtest.h"
> @@ -43,15 +43,36 @@
>  
>  /*** Supplementary PCI Config Space IDs & Masks ***/
>  #define PCI_DEVICE_ID_INTEL_Q35_AHCI   (0x2922)
> +#define PCI_MSI_FLAGS_RESERVED         (0xFF00)
> +#define PCI_PM_CTRL_RESERVED             (0xFC)
> +#define PCI_BCC(REG32)          ((REG32) >> 24)
> +#define PCI_PI(REG32)   (((REG32) >> 8) & 0xFF)
> +#define PCI_SCC(REG32) (((REG32) >> 16) & 0xFF)
> +
> +/*** Recognized AHCI Device Types ***/
> +#define AHCI_INTEL_ICH9 (PCI_DEVICE_ID_INTEL_Q35_AHCI << 16 | \
> +                         PCI_VENDOR_ID_INTEL)
>  
>  /*** Globals ***/
>  static QGuestAllocator *guest_malloc;
>  static QPCIBus *pcibus;
>  static char tmp_path[] = "/tmp/qtest.XXXXXX";
> +static bool ahci_pedantic;
> +static uint32_t ahci_fingerprint;
> +
> +/*** Macro Utilities ***/
> +#define assert_bit_set(data, mask) g_assert_cmphex((data) & (mask), ==, (mask))
> +#define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0)
>  
>  /*** Function Declarations ***/
>  static QPCIDevice *get_ahci_device(void);
>  static void free_ahci_device(QPCIDevice *dev);
> +static void ahci_test_pci_spec(QPCIDevice *ahci);
> +static void ahci_test_pci_caps(QPCIDevice *ahci, uint16_t header,
> +                               uint8_t offset);
> +static void ahci_test_satacap(QPCIDevice *ahci, uint8_t offset);
> +static void ahci_test_msicap(QPCIDevice *ahci, uint8_t offset);
> +static void ahci_test_pmcap(QPCIDevice *ahci, uint8_t offset);
>  
>  /*** Utilities ***/
>  
> @@ -61,7 +82,6 @@ static void free_ahci_device(QPCIDevice *dev);
>  static QPCIDevice *get_ahci_device(void)
>  {
>      QPCIDevice *ahci;
> -    uint16_t vendor_id, device_id;
>  
>      pcibus = qpci_init_pc();
>  
> @@ -69,11 +89,15 @@ static QPCIDevice *get_ahci_device(void)
>      ahci = qpci_device_find(pcibus, QPCI_DEVFN(0x1F, 0x02));
>      g_assert(ahci != NULL);
>  
> -    vendor_id = qpci_config_readw(ahci, PCI_VENDOR_ID);
> -    device_id = qpci_config_readw(ahci, PCI_DEVICE_ID);
> +    ahci_fingerprint = qpci_config_readl(ahci, PCI_VENDOR_ID);
>  
> -    g_assert_cmphex(vendor_id, ==, PCI_VENDOR_ID_INTEL);
> -    g_assert_cmphex(device_id, ==, PCI_DEVICE_ID_INTEL_Q35_AHCI);
> +    switch (ahci_fingerprint) {
> +    case AHCI_INTEL_ICH9:
> +        break;
> +    default:
> +        /* Unknown device. */
> +        g_assert_not_reached();
> +    }
>  
>      return ahci;
>  }
> @@ -145,6 +169,239 @@ static void ahci_shutdown(QPCIDevice *ahci)
>      qtest_shutdown();
>  }
>  
> +/*** Specification Adherence Tests ***/
> +
> +/**
> + * Implementation for test_pci_spec. Ensures PCI configuration space is sane.
> + */
> +static void ahci_test_pci_spec(QPCIDevice *ahci)
> +{
> +    uint8_t datab;
> +    uint16_t data;
> +    uint32_t datal;
> +
> +    /* Most of these bits should start cleared until we turn them on. */
> +    data = qpci_config_readw(ahci, PCI_COMMAND);
> +    assert_bit_clear(data, PCI_COMMAND_MEMORY);
> +    assert_bit_clear(data, PCI_COMMAND_MASTER);
> +    assert_bit_clear(data, PCI_COMMAND_SPECIAL);     /* Reserved */
> +    assert_bit_clear(data, PCI_COMMAND_VGA_PALETTE); /* Reserved */
> +    assert_bit_clear(data, PCI_COMMAND_PARITY);
> +    assert_bit_clear(data, PCI_COMMAND_WAIT);        /* Reserved */
> +    assert_bit_clear(data, PCI_COMMAND_SERR);
> +    assert_bit_clear(data, PCI_COMMAND_FAST_BACK);
> +    assert_bit_clear(data, PCI_COMMAND_INTX_DISABLE);
> +    assert_bit_clear(data, 0xF800);                  /* Reserved */
> +
> +    data = qpci_config_readw(ahci, PCI_STATUS);
> +    assert_bit_clear(data, 0x01 | 0x02 | 0x04);     /* Reserved */
> +    assert_bit_clear(data, PCI_STATUS_INTERRUPT);
> +    assert_bit_set(data, PCI_STATUS_CAP_LIST);      /* must be set */
> +    assert_bit_clear(data, PCI_STATUS_UDF);         /* Reserved */
> +    assert_bit_clear(data, PCI_STATUS_PARITY);
> +    assert_bit_clear(data, PCI_STATUS_SIG_TARGET_ABORT);
> +    assert_bit_clear(data, PCI_STATUS_REC_TARGET_ABORT);
> +    assert_bit_clear(data, PCI_STATUS_REC_MASTER_ABORT);
> +    assert_bit_clear(data, PCI_STATUS_SIG_SYSTEM_ERROR);
> +    assert_bit_clear(data, PCI_STATUS_DETECTED_PARITY);
> +
> +    /* RID occupies the low byte, CCs occupy the high three. */
> +    datal = qpci_config_readl(ahci, PCI_CLASS_REVISION);
> +    if (ahci_pedantic) {
> +        /* AHCI 1.3 specifies that at-boot, the RID should reset to 0x00,
> +         * Though in practice this is likely seldom true. */
> +        assert_bit_clear(datal, 0xFF);
> +    }
> +
> +    /* BCC *must* equal 0x01. */
> +    g_assert_cmphex(PCI_BCC(datal), ==, 0x01);
> +    if (PCI_SCC(datal) == 0x01) {
> +        /* IDE */
> +        assert_bit_set(0x80000000, datal);
> +        assert_bit_clear(0x60000000, datal);
> +    } else if (PCI_SCC(datal) == 0x04) {
> +        /* RAID */
> +        g_assert_cmphex(PCI_PI(datal), ==, 0);
> +    } else if (PCI_SCC(datal) == 0x06) {
> +        /* AHCI */
> +        g_assert_cmphex(PCI_PI(datal), ==, 0x01);
> +    } else {
> +        g_assert_not_reached();
> +    }
> +
> +    datab = qpci_config_readb(ahci, PCI_CACHE_LINE_SIZE);
> +    g_assert_cmphex(datab, ==, 0);
> +
> +    datab = qpci_config_readb(ahci, PCI_LATENCY_TIMER);
> +    g_assert_cmphex(datab, ==, 0);
> +
> +    /* Only the bottom 7 bits must be off. */
> +    datab = qpci_config_readb(ahci, PCI_HEADER_TYPE);
> +    assert_bit_clear(datab, 0x7F);
> +
> +    /* BIST is optional, but the low 7 bits must always start off regardless. */
> +    datab = qpci_config_readb(ahci, PCI_BIST);
> +    assert_bit_clear(datab, 0x7F);
> +
> +    /* BARS 0-4 do not have a boot spec, but ABAR/BAR5 must be clean. */
> +    datal = qpci_config_readl(ahci, PCI_BASE_ADDRESS_5);
> +    g_assert_cmphex(datal, ==, 0);
> +
> +    qpci_config_writel(ahci, PCI_BASE_ADDRESS_5, 0xFFFFFFFF);
> +    datal = qpci_config_readl(ahci, PCI_BASE_ADDRESS_5);
> +    /* ABAR must be 32-bit, memory mapped, non-prefetchable and
> +     * must be >= 512 bytes. To that end, bits 0-8 must be off. */
> +    assert_bit_clear(datal, 0xFF);
> +
> +    /* Capability list MUST be present, */
> +    datal = qpci_config_readl(ahci, PCI_CAPABILITY_LIST);
> +    /* But these bits are reserved. */
> +    assert_bit_clear(datal, ~0xFF);
> +    g_assert_cmphex(datal, !=, 0);
> +
> +    /* Check specification adherence for capability extenstions. */
> +    data = qpci_config_readw(ahci, datal);
> +
> +    switch (ahci_fingerprint) {
> +    case AHCI_INTEL_ICH9:
> +        /* Intel ICH9 Family Datasheet 14.1.19 p.550 */
> +        g_assert_cmphex((data & 0xFF), ==, PCI_CAP_ID_MSI);
> +        break;
> +    default:
> +        /* AHCI 1.3, Section 2.1.14 -- CAP must point to PMCAP. */
> +        g_assert_cmphex((data & 0xFF), ==, PCI_CAP_ID_PM);
> +    }
> +
> +    ahci_test_pci_caps(ahci, data, (uint8_t)datal);
> +
> +    /* Reserved. */
> +    datal = qpci_config_readl(ahci, PCI_CAPABILITY_LIST + 4);
> +    g_assert_cmphex(datal, ==, 0);
> +
> +    /* IPIN might vary, but ILINE must be off. */
> +    datab = qpci_config_readb(ahci, PCI_INTERRUPT_LINE);
> +    g_assert_cmphex(datab, ==, 0);
> +}
> +
> +/**
> + * Test PCI capabilities for AHCI specification adherence.
> + */
> +static void ahci_test_pci_caps(QPCIDevice *ahci, uint16_t header,
> +                               uint8_t offset)
> +{
> +    uint8_t cid = header & 0xFF;
> +    uint8_t next = header >> 8;
> +
> +    g_test_message("CID: %02x; next: %02x", cid, next);
> +
> +    switch (cid) {
> +    case PCI_CAP_ID_PM:
> +        ahci_test_pmcap(ahci, offset);
> +        break;
> +    case PCI_CAP_ID_MSI:
> +        ahci_test_msicap(ahci, offset);
> +        break;
> +    case PCI_CAP_ID_SATA:
> +        ahci_test_satacap(ahci, offset);
> +        break;
> +
> +    default:
> +        g_test_message("Unknown CAP 0x%02x", cid);
> +    }
> +
> +    if (next) {
> +        ahci_test_pci_caps(ahci, qpci_config_readw(ahci, next), next);
> +    }
> +}
> +
> +/**
> + * Test SATA PCI capabilitity for AHCI specification adherence.
> + */
> +static void ahci_test_satacap(QPCIDevice *ahci, uint8_t offset)
> +{
> +    uint16_t dataw;
> +    uint32_t datal;
> +
> +    g_test_message("Verifying SATACAP");
> +
> +    /* Assert that the SATACAP version is 1.0, And reserved bits are empty. */
> +    dataw = qpci_config_readw(ahci, offset + 2);
> +    g_assert_cmphex(dataw, ==, 0x10);
> +
> +    /* Grab the SATACR1 register. */
> +    datal = qpci_config_readw(ahci, offset + 4);
> +
> +    switch (datal & 0x0F) {
> +    case 0x04: /* BAR0 */
> +    case 0x05: /* BAR1 */
> +    case 0x06:
> +    case 0x07:
> +    case 0x08:
> +    case 0x09: /* BAR5 */
> +    case 0x0F: /* Immediately following SATACR1 in PCI config space. */
> +        break;
> +    default:
> +        /* Invalid BARLOC for the Index Data Pair. */
> +        g_assert_not_reached();
> +    }
> +
> +    /* Reserved. */
> +    g_assert_cmphex((datal >> 24), ==, 0x00);
> +}
> +
> +/**
> + * Test MSI PCI capability for AHCI specification adherence.
> + */
> +static void ahci_test_msicap(QPCIDevice *ahci, uint8_t offset)
> +{
> +    uint16_t dataw;
> +    uint32_t datal;
> +
> +    g_test_message("Verifying MSICAP");
> +
> +    dataw = qpci_config_readw(ahci, offset + PCI_MSI_FLAGS);
> +    assert_bit_clear(dataw, PCI_MSI_FLAGS_ENABLE);
> +    assert_bit_clear(dataw, PCI_MSI_FLAGS_QSIZE);
> +    assert_bit_clear(dataw, PCI_MSI_FLAGS_RESERVED);
> +
> +    datal = qpci_config_readl(ahci, offset + PCI_MSI_ADDRESS_LO);
> +    g_assert_cmphex(datal, ==, 0);
> +
> +    if (dataw & PCI_MSI_FLAGS_64BIT) {
> +        g_test_message("MSICAP is 64bit");
> +        datal = qpci_config_readl(ahci, offset + PCI_MSI_ADDRESS_HI);
> +        g_assert_cmphex(datal, ==, 0);
> +        dataw = qpci_config_readw(ahci, offset + PCI_MSI_DATA_64);
> +        g_assert_cmphex(dataw, ==, 0);
> +    } else {
> +        g_test_message("MSICAP is 32bit");
> +        dataw = qpci_config_readw(ahci, offset + PCI_MSI_DATA_32);
> +        g_assert_cmphex(dataw, ==, 0);
> +    }
> +}
> +
> +/**
> + * Test Power Management PCI capability for AHCI specification adherence.
> + */
> +static void ahci_test_pmcap(QPCIDevice *ahci, uint8_t offset)
> +{
> +    uint16_t dataw;
> +
> +    g_test_message("Verifying PMCAP");
> +
> +    dataw = qpci_config_readw(ahci, offset + PCI_PM_PMC);
> +    assert_bit_clear(dataw, PCI_PM_CAP_PME_CLOCK);
> +    assert_bit_clear(dataw, PCI_PM_CAP_RESERVED);
> +    assert_bit_clear(dataw, PCI_PM_CAP_D1);
> +    assert_bit_clear(dataw, PCI_PM_CAP_D2);
> +
> +    dataw = qpci_config_readw(ahci, offset + PCI_PM_CTRL);
> +    assert_bit_clear(dataw, PCI_PM_CTRL_STATE_MASK);
> +    assert_bit_clear(dataw, PCI_PM_CTRL_RESERVED);
> +    assert_bit_clear(dataw, PCI_PM_CTRL_DATA_SEL_MASK);
> +    assert_bit_clear(dataw, PCI_PM_CTRL_DATA_SCALE_MASK);
> +}
> +
>  /******************************************************************************/
>  /* Test Interfaces                                                            */
>  /******************************************************************************/
> @@ -159,6 +416,18 @@ static void test_sanity(void)
>      ahci_shutdown(ahci);
>  }
>  
> +/**
> + * Ensure that the PCI configuration space for the AHCI device is in-line with
> + * the AHCI 1.3 specification for initial values.
> + */
> +static void test_pci_spec(void)
> +{
> +    QPCIDevice *ahci;
> +    ahci = ahci_boot();
> +    ahci_test_pci_spec(ahci);
> +    ahci_shutdown(ahci);
> +}
> +
>  /******************************************************************************/
>  
>  int main(int argc, char **argv)
> @@ -166,10 +435,33 @@ int main(int argc, char **argv)
>      const char *arch;
>      int fd;
>      int ret;
> +    int c;
> +
> +    static struct option long_options[] = {
> +        {"pedantic", no_argument, 0, 'p' },
> +        {0, 0, 0, 0},
> +    };
>  
>      /* Should be first to utilize g_test functionality, So we can see errors. */
>      g_test_init(&argc, &argv, NULL);
>  
> +    while (1) {
> +        c = getopt_long(argc, argv, "", long_options, NULL);
> +        if (c == -1) {
> +            break;
> +        }
> +        switch (c) {
> +        case -1:
> +            break;
> +        case 'p':
> +            ahci_pedantic = 1;
> +            break;
> +        default:
> +            fprintf(stderr, "Unrecognized ahci_test option.\n");
> +            g_assert_not_reached();
> +        }
> +    }
> +
>      /* Check architecture */
>      arch = qtest_get_arch();
>      if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
> @@ -186,6 +478,7 @@ int main(int argc, char **argv)
>  
>      /* Run the tests */
>      qtest_add_func("/ahci/sanity",     test_sanity);
> +    qtest_add_func("/ahci/pci_spec",   test_pci_spec);
>  
>      ret = g_test_run();
>  
> -- 
> 1.9.3

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 26/32] ahci: MSI capability should be at 0x80, not 0x50.
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 26/32] ahci: MSI capability should be at 0x80, not 0x50 John Snow
  2014-08-14  7:15   ` Michael S. Tsirkin
@ 2014-08-14  7:19   ` Michael S. Tsirkin
  1 sibling, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2014-08-14  7:19 UTC (permalink / raw)
  To: John Snow; +Cc: qemu-devel, stefanha, armbru

On Wed, Aug 13, 2014 at 05:56:09PM -0400, John Snow wrote:
> In the Intel ICH9 data sheet, the MSI capability offset
> in the PCI configuration space for ICH9 AHCI devices is
> specified to be 0x80.
> 
> Further, the PCI capability pointer should always point
> to 0x80 in ICH9 devices, despite the fact that AHCI 1.3
> specifies that it should be pointing to PMCAP (Which in
> this instance would be 0x70) to maintain adherence to
> the Intel data sheet specifications and real observed behavior.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>


Besides the comment I sent previously:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


> ---
>  hw/ide/ich.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ide/ich.c b/hw/ide/ich.c
> index a2f1639..d2a3ac2 100644
> --- a/hw/ide/ich.c
> +++ b/hw/ide/ich.c
> @@ -71,6 +71,7 @@
>  #include <hw/ide/pci.h>
>  #include <hw/ide/ahci.h>
>  
> +#define ICH9_MSI_CAP_OFFSET     0x80
>  #define ICH9_SATA_CAP_OFFSET    0xA8
>  
>  #define ICH9_IDP_BAR            4
> @@ -115,7 +116,6 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>      /* XXX Software should program this register */
>      dev->config[0x90]   = 1 << 6; /* Address Map Register - AHCI mode */
>  
> -    msi_init(dev, 0x50, 1, true, false);
>      d->ahci.irq = pci_allocate_irq(dev);
>  
>      pci_register_bar(dev, ICH9_IDP_BAR, PCI_BASE_ADDRESS_SPACE_IO,
> @@ -135,6 +135,9 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>                   (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4));
>      d->ahci.idp_offset = ICH9_IDP_INDEX;
>  
> +    /* MSI cap should be added last to be first. */
> +    msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false);
> +
>      return 0;
>  }
>  
> -- 
> 1.9.3

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 25/32] ahci: Adding basic functionality qtest.
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 25/32] ahci: Adding basic functionality qtest John Snow
@ 2014-08-14 15:34   ` Stefan Hajnoczi
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2014-08-14 15:34 UTC (permalink / raw)
  To: John Snow; +Cc: mst, qemu-devel, armbru

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

On Wed, Aug 13, 2014 at 05:56:08PM -0400, John Snow wrote:
> Currently, there is no qtest to test the functionality of
> the AHCI functionality present within the Q35 machine type.
> 
> This patch adds a skeleton for an AHCI test suite,
> and adds a simple sanity-check test case where we
> identify that the AHCI device is present, then
> disengage the virtual machine.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/Makefile    |   2 +
>  tests/ahci-test.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 198 insertions(+)
>  create mode 100644 tests/ahci-test.c

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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 27/32] ahci: Add test_pci_spec to ahci-test.
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 27/32] ahci: Add test_pci_spec to ahci-test John Snow
  2014-08-14  7:18   ` Michael S. Tsirkin
@ 2014-08-14 16:03   ` Stefan Hajnoczi
  1 sibling, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2014-08-14 16:03 UTC (permalink / raw)
  To: John Snow; +Cc: mst, qemu-devel, armbru

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

On Wed, Aug 13, 2014 at 05:56:10PM -0400, John Snow wrote:
> Adds a specification adherence test for AHCI
> where the boot-up values for the PCI configuration space
> are compared against the AHCI 1.3 specification.
> 
> This test does not itself attempt to engage the device.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/ahci-test.c | 305 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 299 insertions(+), 6 deletions(-)

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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 28/32] ahci: add test_pci_enable to ahci-test.
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 28/32] ahci: add test_pci_enable " John Snow
@ 2014-08-14 16:05   ` Stefan Hajnoczi
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2014-08-14 16:05 UTC (permalink / raw)
  To: John Snow; +Cc: mst, qemu-devel, armbru

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

On Wed, Aug 13, 2014 at 05:56:11PM -0400, John Snow wrote:
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index 29ac0d0..0ca4a20 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -56,6 +56,7 @@
>  /*** Globals ***/
>  static QGuestAllocator *guest_malloc;
>  static QPCIBus *pcibus;
> +static uint64_t barsize;

This variable is not used so it would be nicer to move it to the
relevant later patch.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 29/32] ahci: properly shadow the TFD register
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 29/32] ahci: properly shadow the TFD register John Snow
@ 2014-08-14 16:09   ` Stefan Hajnoczi
  2014-08-14 16:13     ` John Snow
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Hajnoczi @ 2014-08-14 16:09 UTC (permalink / raw)
  To: John Snow; +Cc: mst, qemu-devel, armbru

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

On Wed, Aug 13, 2014 at 05:56:12PM -0400, John Snow wrote:
> @@ -497,6 +495,8 @@ static void ahci_reset_port(AHCIState *s, int port)
>      pr->scr_stat = 0;
>      pr->scr_err = 0;
>      pr->scr_act = 0;
> +    pr->tfdata = 0x7F;

Is it possible to avoid the magic number?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 29/32] ahci: properly shadow the TFD register
  2014-08-14 16:09   ` Stefan Hajnoczi
@ 2014-08-14 16:13     ` John Snow
  2014-08-14 17:09       ` Stefan Hajnoczi
  0 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2014-08-14 16:13 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: mst, qemu-devel, armbru



On 08/14/2014 12:09 PM, Stefan Hajnoczi wrote:
> On Wed, Aug 13, 2014 at 05:56:12PM -0400, John Snow wrote:
>> @@ -497,6 +495,8 @@ static void ahci_reset_port(AHCIState *s, int port)
>>       pr->scr_stat = 0;
>>       pr->scr_err = 0;
>>       pr->scr_act = 0;
>> +    pr->tfdata = 0x7F;
>
> Is it possible to avoid the magic number?
>

I can name it as AHCI_TFD_BOOT_VALUE or AHCI_TFD_INIT_VALUE or so. Same 
for the PxSIG boot value too. I don't think I can give any greater 
meaning to the value because it's just a boot signature that means "We 
haven't received an FIS yet."

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 30/32] ahci: Add test_hba_spec to ahci-test.
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 30/32] ahci: Add test_hba_spec to ahci-test John Snow
@ 2014-08-14 16:25   ` Stefan Hajnoczi
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2014-08-14 16:25 UTC (permalink / raw)
  To: John Snow; +Cc: mst, qemu-devel, armbru

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

On Wed, Aug 13, 2014 at 05:56:13PM -0400, John Snow wrote:
> Add a test routine that checks the boot-up values of the HBA
> configuration memory space against the AHCI 1.3 specification
> and Intel ICH9 data sheet (for Q35 machines) for adherence and
> sane values.
> 
> The HBA is not yet engaged or put into the idle state.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/ahci-test.c | 555 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 555 insertions(+)

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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 31/32] ahci: Add test_hba_enable to ahci-test.
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 31/32] ahci: Add test_hba_enable " John Snow
@ 2014-08-14 16:46   ` Stefan Hajnoczi
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2014-08-14 16:46 UTC (permalink / raw)
  To: John Snow; +Cc: mst, qemu-devel, armbru

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

On Wed, Aug 13, 2014 at 05:56:14PM -0400, John Snow wrote:
> This test engages the HBA functionality and initializes
> values to sane defaults to allow for minimal HBA functionality.
> 
> Buffers are allocated and pointers are updated to allow minimal
> I/O commands to complete as expected. Error registers and responses
> are sanity checked for specification adherence.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/ahci-test.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 156 insertions(+)

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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 32/32] ahci: Add test_identify case to ahci-test.
  2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 32/32] ahci: Add test_identify case " John Snow
@ 2014-08-14 16:52   ` Stefan Hajnoczi
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2014-08-14 16:52 UTC (permalink / raw)
  To: John Snow; +Cc: mst, qemu-devel, armbru

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

On Wed, Aug 13, 2014 at 05:56:15PM -0400, John Snow wrote:
> +    /* Copy the existing Command #0 structure from the CLB into local memory,
> +     * and build a new command #0. */
> +    memread(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 = 1; /* One PRD table entry. */

What about endianness?  This will be copied into guest memory, we should
use cpu_to_X() to ensure the correct endianness.

> +    cmd.prdbc = 0;
> +    cmd.ctba = table;
> +    cmd.ctbau = 0;
> +
> +    /* Construct our PRD, noting that DBC is 0-indexed. */
> +    prd.dba = data_ptr;

Endianness

> +    reg = px_rreg(i, AHCI_PX_TFD);
> +    g_assert_cmphex((reg & AHCI_PX_TFD_ERR), ==, pio->error);
> +    g_assert_cmphex((reg & AHCI_PX_TFD_STS), ==, pio->status);
> +    /* PIO FIS contains a "bytes read" field, it should match up. */
> +    g_assert_cmphex(pio->res4, ==, cmd.prdbc);

Endianness

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v3 29/32] ahci: properly shadow the TFD register
  2014-08-14 16:13     ` John Snow
@ 2014-08-14 17:09       ` Stefan Hajnoczi
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2014-08-14 17:09 UTC (permalink / raw)
  To: John Snow; +Cc: mst, qemu-devel, armbru

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

On Thu, Aug 14, 2014 at 12:13:51PM -0400, John Snow wrote:
> 
> 
> On 08/14/2014 12:09 PM, Stefan Hajnoczi wrote:
> >On Wed, Aug 13, 2014 at 05:56:12PM -0400, John Snow wrote:
> >>@@ -497,6 +495,8 @@ static void ahci_reset_port(AHCIState *s, int port)
> >>      pr->scr_stat = 0;
> >>      pr->scr_err = 0;
> >>      pr->scr_act = 0;
> >>+    pr->tfdata = 0x7F;
> >
> >Is it possible to avoid the magic number?
> >
> 
> I can name it as AHCI_TFD_BOOT_VALUE or AHCI_TFD_INIT_VALUE or so. Same for
> the PxSIG boot value too. I don't think I can give any greater meaning to
> the value because it's just a boot signature that means "We haven't received
> an FIS yet."

In that case, don't bother.  I thought this was error and status fields
shifted.

Stefan

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2014-08-14 17:10 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-13 21:56 [Qemu-devel] [PATCH v3 00/32] AHCI test suite framework v3 John Snow
2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 25/32] ahci: Adding basic functionality qtest John Snow
2014-08-14 15:34   ` Stefan Hajnoczi
2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 26/32] ahci: MSI capability should be at 0x80, not 0x50 John Snow
2014-08-14  7:15   ` Michael S. Tsirkin
2014-08-14  7:19   ` Michael S. Tsirkin
2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 27/32] ahci: Add test_pci_spec to ahci-test John Snow
2014-08-14  7:18   ` Michael S. Tsirkin
2014-08-14 16:03   ` Stefan Hajnoczi
2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 28/32] ahci: add test_pci_enable " John Snow
2014-08-14 16:05   ` Stefan Hajnoczi
2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 29/32] ahci: properly shadow the TFD register John Snow
2014-08-14 16:09   ` Stefan Hajnoczi
2014-08-14 16:13     ` John Snow
2014-08-14 17:09       ` Stefan Hajnoczi
2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 30/32] ahci: Add test_hba_spec to ahci-test John Snow
2014-08-14 16:25   ` Stefan Hajnoczi
2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 31/32] ahci: Add test_hba_enable " John Snow
2014-08-14 16:46   ` Stefan Hajnoczi
2014-08-13 21:56 ` [Qemu-devel] [PATCH v3 32/32] ahci: Add test_identify case " John Snow
2014-08-14 16:52   ` Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.