qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring
@ 2015-01-19 20:15 John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 01/15] libqos: Split apart pc_alloc_init John Snow
                   ` (16 more replies)
  0 siblings, 17 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

This series aims to do two main things:

(1) Eliminate global state out of the ahci-test file so that
    the tests are more functional. This will allow me to write
    migration tests more easily. These tests are already written
    and will be submitted upstream after these refactor series.

(2) With global state removed from ahci-test, factor out functions
    that would be useful to manipulate ahci devices into libqos/ahci.o,
    to allow other testwriters to capitalize on the functional
    refactoring.

So, as an overview, we do a few things:

 - Clean up the malloc interface to have a clear pc-specific interface
   that uses an architecture independent core.

 - Add some new structures to help keep track of AHCI and libqos state.

 - Rewrite existing AHCI helpers and routines to use the new structures
   and helper routines.

 - Excise any commonly valuable code to libqos/ahci.h and libqos/ahci.c.

This series therefore introduces no new functionality itself, but I was
trying to keep the series small and reviewable. The real necessity here
in jumbling malloc and ahci functions around is to create a functional
interface that can be bundled into a single structure to help facilitate
migration testing inside of qtests, for which I intend to export to all
of qtests to be re-used for other tests.

>From this point forward, the churn to my AHCI refactor series should be
significantly reduced and more pleasant to read.

Thanks,
John.

V2:
 - #6  Made QGuestAllocator object opaque, in a new patch.
   - Created page_size setter method.
 - #7  Renamed "QOSOperations" to "QOSOps" and added Paolo's R-b.
 - #12 Dropped GCC attributes from static inline helpers (ahci.h)
 - #15 Adjusted assertion to not compare void ptr to int 0
 - #15 Share ahci.o with libqos-obj-pc-y instead of only ahci-test
   * ahci.o is pc-only for now.

John Snow (15):
  libqos: Split apart pc_alloc_init
  qtest/ahci: Create ahci.h
  libqos: create libqos.c
  libqos: add qtest_vboot
  libqos: add alloc_init_flags
  libqos: Update QGuestAllocator to be opaque
  libqos: add pc specific interface
  qtest/ahci: Store hba_base in AHCIQState
  qtest/ahci: finalize AHCIQState consolidation
  qtest/ahci: remove pcibus global
  qtest/ahci: remove guest_malloc global
  libqos/ahci: Functional register helpers
  qtest/ahci: remove getter/setter macros
  qtest/ahci: Bookmark FB and CLB pointers
  libqos/ahci: create libqos/ahci.c

 tests/Makefile           |   5 +-
 tests/ahci-test.c        | 894 ++++++++---------------------------------------
 tests/libqos/ahci.c      | 269 ++++++++++++++
 tests/libqos/ahci.h      | 435 +++++++++++++++++++++++
 tests/libqos/libqos-pc.c |  24 ++
 tests/libqos/libqos-pc.h |   9 +
 tests/libqos/libqos.c    |  63 ++++
 tests/libqos/libqos.h    |  33 ++
 tests/libqos/malloc-pc.c |  20 +-
 tests/libqos/malloc.c    |  86 ++++-
 tests/libqos/malloc.h    |  25 +-
 11 files changed, 1068 insertions(+), 795 deletions(-)
 create mode 100644 tests/libqos/ahci.c
 create mode 100644 tests/libqos/ahci.h
 create mode 100644 tests/libqos/libqos-pc.c
 create mode 100644 tests/libqos/libqos-pc.h
 create mode 100644 tests/libqos/libqos.c
 create mode 100644 tests/libqos/libqos.h

-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 01/15] libqos: Split apart pc_alloc_init
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 02/15] qtest/ahci: Create ahci.h John Snow
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Move the list-specific initialization over into
malloc.c, to keep all of the list implementation
details within the same file.

The allocation and freeing of these structures are
now both back within the same layer.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/libqos/malloc-pc.c | 20 ++++----------------
 tests/libqos/malloc.c    | 17 +++++++++++++++++
 tests/libqos/malloc.h    |  1 +
 3 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index c9c48fd..36a0740 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -32,31 +32,19 @@ void pc_alloc_uninit(QGuestAllocator *allocator)
 
 QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags)
 {
-    QGuestAllocator *s = g_malloc0(sizeof(*s));
+    QGuestAllocator *s;
     uint64_t ram_size;
     QFWCFG *fw_cfg = pc_fw_cfg_init();
-    MemBlock *node;
+
+    ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
+    s = alloc_init(1 << 20, MIN(ram_size, 0xE0000000));
 
     s->opts = flags;
     s->page_size = PAGE_SIZE;
 
-    ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
-
-    /* Start at 1MB */
-    s->start = 1 << 20;
-
-    /* Respect PCI hole */
-    s->end = MIN(ram_size, 0xE0000000);
-
     /* clean-up */
     g_free(fw_cfg);
 
-    QTAILQ_INIT(&s->used);
-    QTAILQ_INIT(&s->free);
-
-    node = mlist_new(s->start, s->end - s->start);
-    QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
-
     return s;
 }
 
diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
index 5debf18..0d34ecd 100644
--- a/tests/libqos/malloc.c
+++ b/tests/libqos/malloc.c
@@ -268,3 +268,20 @@ void guest_free(QGuestAllocator *allocator, uint64_t addr)
         mlist_check(allocator);
     }
 }
+
+QGuestAllocator *alloc_init(uint64_t start, uint64_t end)
+{
+    QGuestAllocator *s = g_malloc0(sizeof(*s));
+    MemBlock *node;
+
+    s->start = start;
+    s->end = end;
+
+    QTAILQ_INIT(&s->used);
+    QTAILQ_INIT(&s->free);
+
+    node = mlist_new(s->start, s->end - s->start);
+    QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
+
+    return s;
+}
diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 465efeb..677db77 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -50,4 +50,5 @@ void alloc_uninit(QGuestAllocator *allocator);
 uint64_t guest_alloc(QGuestAllocator *allocator, size_t size);
 void guest_free(QGuestAllocator *allocator, uint64_t addr);
 
+QGuestAllocator *alloc_init(uint64_t start, uint64_t end);
 #endif
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 02/15] qtest/ahci: Create ahci.h
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 01/15] libqos: Split apart pc_alloc_init John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 03/15] libqos: create libqos.c John Snow
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Extract defines and other information to ahci.h, to be shared with other
tests if they so please.

At the very least, reduce clutter in the test file itself.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/ahci-test.c   | 319 +----------------------------------------------
 tests/libqos/ahci.h | 352 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 353 insertions(+), 318 deletions(-)
 create mode 100644 tests/libqos/ahci.h

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index b1a59f2..5c9da12 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -29,6 +29,7 @@
 #include <glib.h>
 
 #include "libqtest.h"
+#include "libqos/ahci.h"
 #include "libqos/pci-pc.h"
 #include "libqos/malloc-pc.h"
 
@@ -41,313 +42,6 @@
 /* Test-specific defines. */
 #define TEST_IMAGE_SIZE    (64 * 1024 * 1024)
 
-/*** 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)
-
-/*** 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)
-
-/*** Structures ***/
-
-/**
- * Generic FIS structure.
- */
-typedef struct FIS {
-    uint8_t fis_type;
-    uint8_t flags;
-    char data[0];
-} __attribute__((__packed__)) FIS;
-
-/**
- * Register device-to-host FIS structure.
- */
-typedef struct RegD2HFIS {
-    /* 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__)) RegD2HFIS;
-
-/**
- * Register host-to-device FIS structure.
- */
-typedef struct RegH2DFIS {
-    /* 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__)) RegH2DFIS;
-
-/**
- * Command List entry structure.
- * The command list contains between 1-32 of these structures.
- */
-typedef struct AHCICommand {
-    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__)) AHCICommand;
-
-/**
- * Physical Region Descriptor; pointed to by the Command List Header,
- * struct ahci_command.
- */
-typedef 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) */
-} PRD;
-
-typedef struct HBACap {
-    uint32_t cap;
-    uint32_t cap2;
-} HBACap;
-
 /*** Globals ***/
 static QGuestAllocator *guest_malloc;
 static QPCIBus *pcibus;
@@ -356,13 +50,6 @@ static char tmp_path[] = "/tmp/qtest.XXXXXX";
 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))
@@ -380,10 +67,6 @@ 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(PRD)) + 0x7F) & ~0x7F)
-
-
 /*** Function Declarations ***/
 static QPCIDevice *get_ahci_device(void);
 static QPCIDevice *start_ahci_device(QPCIDevice *dev, void **hba_base);
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
new file mode 100644
index 0000000..6564c5a
--- /dev/null
+++ b/tests/libqos/ahci.h
@@ -0,0 +1,352 @@
+#ifndef __libqos_ahci_h
+#define __libqos_ahci_h
+
+/*
+ * AHCI qtest library functions and definitions
+ *
+ * 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 <stdlib.h>
+#include <stdbool.h>
+
+#include "libqos/pci.h"
+#include "libqos/malloc-pc.h"
+
+/*** 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)
+
+/*** 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)
+
+/*** Structures ***/
+
+/**
+ * Generic FIS structure.
+ */
+typedef struct FIS {
+    uint8_t fis_type;
+    uint8_t flags;
+    char data[0];
+} __attribute__((__packed__)) FIS;
+
+/**
+ * Register device-to-host FIS structure.
+ */
+typedef struct RegD2HFIS {
+    /* 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__)) RegD2HFIS;
+
+/**
+ * Register host-to-device FIS structure.
+ */
+typedef struct RegH2DFIS {
+    /* 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__)) RegH2DFIS;
+
+/**
+ * Command List entry structure.
+ * The command list contains between 1-32 of these structures.
+ */
+typedef struct AHCICommand {
+    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__)) AHCICommand;
+
+/**
+ * Physical Region Descriptor; pointed to by the Command List Header,
+ * struct ahci_command.
+ */
+typedef 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) */
+} PRD;
+
+typedef struct HBACap {
+    uint32_t cap;
+    uint32_t cap2;
+} HBACap;
+
+/*** 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)
+
+/* For calculating how big the PRD table needs to be: */
+#define CMD_TBL_SIZ(n) ((0x80 + ((n) * sizeof(PRD)) + 0x7F) & ~0x7F)
+
+#endif
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 03/15] libqos: create libqos.c
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 01/15] libqos: Split apart pc_alloc_init John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 02/15] qtest/ahci: Create ahci.h John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 04/15] libqos: add qtest_vboot John Snow
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

The intent of this file is to serve as a misc. utilities file to be
shared amongst tests that are utilizing libqos facilities.

In a later patch, migration test helpers will be added to libqos.c that
will allow simplified testing of migration cases where libqos is
"Just Enough OS" for migrations testing.

The addition of the AHCIQState structure will also allow us to eliminate
global variables inside of qtests to manage allocators and test instances
in a better, more functional way.

libqos.c:
        - Add qtest_boot
        - Add qtest_shutdown

libqos.h:
        - Create QOSState structure for allocator and QTestState.

ahci-test.c:
        - Move qtest_boot and qtest_shutdown to libqos.c/h
        - Create AHCIQState to interface with new qtest_boot/shutdown prototypes
        - Modify tests slightly to use new types.

For now, the new object file is only linked to ahci-test, because it still
relies on pc architecture specific code in libqos. The next two patches will
reorganize the code to be more general.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/Makefile        |  2 +-
 tests/ahci-test.c     | 95 +++++++++++++++++++++------------------------------
 tests/libqos/ahci.h   |  5 +++
 tests/libqos/libqos.c | 48 ++++++++++++++++++++++++++
 tests/libqos/libqos.h | 26 ++++++++++++++
 5 files changed, 119 insertions(+), 57 deletions(-)
 create mode 100644 tests/libqos/libqos.c
 create mode 100644 tests/libqos/libqos.h

diff --git a/tests/Makefile b/tests/Makefile
index c2e2e52..c07d840 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -312,7 +312,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/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y) tests/libqos/libqos.o
 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
index 5c9da12..15542b9 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -29,6 +29,7 @@
 #include <glib.h>
 
 #include "libqtest.h"
+#include "libqos/libqos.h"
 #include "libqos/ahci.h"
 #include "libqos/pci-pc.h"
 #include "libqos/malloc-pc.h"
@@ -136,58 +137,40 @@ static void free_ahci_device(QPCIDevice *ahci)
 /*** 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)
+static AHCIQState *ahci_boot(void)
 {
-    qtest_boot("-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s,"
-               "format=raw"
-               " -M q35 "
-               "-device ide-hd,drive=drive0 "
-               "-global ide-hd.ver=%s",
-               tmp_path, "testdisk", "version");
+    AHCIQState *s;
+    const char *cli;
+
+    s = g_malloc0(sizeof(AHCIQState));
+
+    cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
+        ",format=raw"
+        " -M q35 "
+        "-device ide-hd,drive=drive0 "
+        "-global ide-hd.ver=%s";
+    s->parent = qtest_boot(cli, tmp_path, "testdisk", "version");
 
     /* Verify that we have an AHCI device present. */
-    return get_ahci_device();
+    s->dev = get_ahci_device();
+
+    /* Stopgap: Copy the allocator reference */
+    guest_malloc = s->parent->alloc;
+
+    return s;
 }
 
 /**
  * Clean up the PCI device, then terminate the QEMU instance.
  */
-static void ahci_shutdown(QPCIDevice *ahci)
+static void ahci_shutdown(AHCIQState *ahci)
 {
-    free_ahci_device(ahci);
-    qtest_shutdown();
+    QOSState *qs = ahci->parent;
+    free_ahci_device(ahci->dev);
+    g_free(ahci);
+    qtest_shutdown(qs);
 }
 
 /*** Logical Device Initialization ***/
@@ -1104,7 +1087,7 @@ static void ahci_test_identify(QPCIDevice *ahci, void *hba_base)
  */
 static void test_sanity(void)
 {
-    QPCIDevice *ahci;
+    AHCIQState *ahci;
     ahci = ahci_boot();
     ahci_shutdown(ahci);
 }
@@ -1115,9 +1098,9 @@ static void test_sanity(void)
  */
 static void test_pci_spec(void)
 {
-    QPCIDevice *ahci;
+    AHCIQState *ahci;
     ahci = ahci_boot();
-    ahci_test_pci_spec(ahci);
+    ahci_test_pci_spec(ahci->dev);
     ahci_shutdown(ahci);
 }
 
@@ -1127,10 +1110,10 @@ static void test_pci_spec(void)
  */
 static void test_pci_enable(void)
 {
-    QPCIDevice *ahci;
+    AHCIQState *ahci;
     void *hba_base;
     ahci = ahci_boot();
-    ahci_pci_enable(ahci, &hba_base);
+    ahci_pci_enable(ahci->dev, &hba_base);
     ahci_shutdown(ahci);
 }
 
@@ -1140,12 +1123,12 @@ static void test_pci_enable(void)
  */
 static void test_hba_spec(void)
 {
-    QPCIDevice *ahci;
+    AHCIQState *ahci;
     void *hba_base;
 
     ahci = ahci_boot();
-    ahci_pci_enable(ahci, &hba_base);
-    ahci_test_hba_spec(ahci, hba_base);
+    ahci_pci_enable(ahci->dev, &hba_base);
+    ahci_test_hba_spec(ahci->dev, hba_base);
     ahci_shutdown(ahci);
 }
 
@@ -1155,12 +1138,12 @@ static void test_hba_spec(void)
  */
 static void test_hba_enable(void)
 {
-    QPCIDevice *ahci;
+    AHCIQState *ahci;
     void *hba_base;
 
     ahci = ahci_boot();
-    ahci_pci_enable(ahci, &hba_base);
-    ahci_hba_enable(ahci, hba_base);
+    ahci_pci_enable(ahci->dev, &hba_base);
+    ahci_hba_enable(ahci->dev, hba_base);
     ahci_shutdown(ahci);
 }
 
@@ -1170,13 +1153,13 @@ static void test_hba_enable(void)
  */
 static void test_identify(void)
 {
-    QPCIDevice *ahci;
+    AHCIQState *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_pci_enable(ahci->dev, &hba_base);
+    ahci_hba_enable(ahci->dev, hba_base);
+    ahci_test_identify(ahci->dev, hba_base);
     ahci_shutdown(ahci);
 }
 
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 6564c5a..bc5f45d 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -245,6 +245,11 @@
 
 /*** Structures ***/
 
+typedef struct AHCIQState {
+    QOSState *parent;
+    QPCIDevice *dev;
+} AHCIQState;
+
 /**
  * Generic FIS structure.
  */
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
new file mode 100644
index 0000000..c478bc9
--- /dev/null
+++ b/tests/libqos/libqos.c
@@ -0,0 +1,48 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <glib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/wait.h>
+
+#include "libqtest.h"
+#include "libqos/libqos.h"
+#include "libqos/pci.h"
+#include "libqos/malloc-pc.h"
+
+/*** Test Setup & Teardown ***/
+
+/**
+ * Launch QEMU with the given command line,
+ * and then set up interrupts and our guest malloc interface.
+ */
+QOSState *qtest_boot(const char *cmdline_fmt, ...)
+{
+    QOSState *qs = g_malloc(sizeof(QOSState));
+    char *cmdline;
+    va_list ap;
+
+    va_start(ap, cmdline_fmt);
+    cmdline = g_strdup_vprintf(cmdline_fmt, ap);
+    va_end(ap);
+
+    qs->qts = qtest_start(cmdline);
+    qtest_irq_intercept_in(global_qtest, "ioapic");
+    qs->alloc = pc_alloc_init();
+
+    g_free(cmdline);
+    return qs;
+}
+
+/**
+ * Tear down the QEMU instance.
+ */
+void qtest_shutdown(QOSState *qs)
+{
+    if (qs->alloc) {
+        pc_alloc_uninit(qs->alloc);
+        qs->alloc = NULL;
+    }
+    qtest_quit(qs->qts);
+    g_free(qs);
+}
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
new file mode 100644
index 0000000..7a106f2
--- /dev/null
+++ b/tests/libqos/libqos.h
@@ -0,0 +1,26 @@
+#ifndef __libqos_h
+#define __libqos_h
+
+#include "libqtest.h"
+#include "libqos/pci.h"
+#include "libqos/malloc-pc.h"
+
+typedef struct QOSState {
+    QTestState *qts;
+    QGuestAllocator *alloc;
+} QOSState;
+
+QOSState *qtest_boot(const char *cmdline_fmt, ...);
+void qtest_shutdown(QOSState *qs);
+
+static inline uint64_t qmalloc(QOSState *q, size_t bytes)
+{
+    return guest_alloc(q->alloc, bytes);
+}
+
+static inline void qfree(QOSState *q, uint64_t addr)
+{
+    guest_free(q->alloc, addr);
+}
+
+#endif
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 04/15] libqos: add qtest_vboot
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (2 preceding siblings ...)
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 03/15] libqos: create libqos.c John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 05/15] libqos: add alloc_init_flags John Snow
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Add a va_list variant of the qtest_boot function.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/libqos/libqos.c | 25 +++++++++++++++++++------
 tests/libqos/libqos.h |  1 +
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index c478bc9..c8b3ef0 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -16,16 +16,13 @@
  * Launch QEMU with the given command line,
  * and then set up interrupts and our guest malloc interface.
  */
-QOSState *qtest_boot(const char *cmdline_fmt, ...)
+QOSState *qtest_vboot(const char *cmdline_fmt, va_list ap)
 {
-    QOSState *qs = g_malloc(sizeof(QOSState));
     char *cmdline;
-    va_list ap;
 
-    va_start(ap, cmdline_fmt);
+    struct QOSState *qs = g_malloc(sizeof(QOSState));
+
     cmdline = g_strdup_vprintf(cmdline_fmt, ap);
-    va_end(ap);
-
     qs->qts = qtest_start(cmdline);
     qtest_irq_intercept_in(global_qtest, "ioapic");
     qs->alloc = pc_alloc_init();
@@ -35,6 +32,22 @@ QOSState *qtest_boot(const char *cmdline_fmt, ...)
 }
 
 /**
+ * Launch QEMU with the given command line,
+ * and then set up interrupts and our guest malloc interface.
+ */
+QOSState *qtest_boot(const char *cmdline_fmt, ...)
+{
+    QOSState *qs;
+    va_list ap;
+
+    va_start(ap, cmdline_fmt);
+    qs = qtest_vboot(cmdline_fmt, ap);
+    va_end(ap);
+
+    return qs;
+}
+
+/**
  * Tear down the QEMU instance.
  */
 void qtest_shutdown(QOSState *qs)
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index 7a106f2..7ae0a8d 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -10,6 +10,7 @@ typedef struct QOSState {
     QGuestAllocator *alloc;
 } QOSState;
 
+QOSState *qtest_vboot(const char *cmdline_fmt, va_list ap);
 QOSState *qtest_boot(const char *cmdline_fmt, ...);
 void qtest_shutdown(QOSState *qs);
 
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 05/15] libqos: add alloc_init_flags
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (3 preceding siblings ...)
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 04/15] libqos: add qtest_vboot John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 06/15] libqos: Update QGuestAllocator to be opaque John Snow
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Allow a generic interface to alloc_init_flags,
not just through pc_alloc_init_flags.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/libqos/malloc-pc.c | 4 +---
 tests/libqos/malloc.c    | 8 ++++++++
 tests/libqos/malloc.h    | 2 ++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index 36a0740..6a5fdf3 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -37,9 +37,7 @@ QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags)
     QFWCFG *fw_cfg = pc_fw_cfg_init();
 
     ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
-    s = alloc_init(1 << 20, MIN(ram_size, 0xE0000000));
-
-    s->opts = flags;
+    s = alloc_init_flags(flags, 1 << 20, MIN(ram_size, 0xE0000000));
     s->page_size = PAGE_SIZE;
 
     /* clean-up */
diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
index 0d34ecd..4ff260f 100644
--- a/tests/libqos/malloc.c
+++ b/tests/libqos/malloc.c
@@ -285,3 +285,11 @@ QGuestAllocator *alloc_init(uint64_t start, uint64_t end)
 
     return s;
 }
+
+QGuestAllocator *alloc_init_flags(QAllocOpts opts,
+                                  uint64_t start, uint64_t end)
+{
+    QGuestAllocator *s = alloc_init(start, end);
+    s->opts = opts;
+    return s;
+}
diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 677db77..7b29547 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -51,4 +51,6 @@ uint64_t guest_alloc(QGuestAllocator *allocator, size_t size);
 void guest_free(QGuestAllocator *allocator, uint64_t addr);
 
 QGuestAllocator *alloc_init(uint64_t start, uint64_t end);
+QGuestAllocator *alloc_init_flags(QAllocOpts flags,
+                                  uint64_t start, uint64_t end);
 #endif
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 06/15] libqos: Update QGuestAllocator to be opaque
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (4 preceding siblings ...)
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 05/15] libqos: add alloc_init_flags John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-27 21:22   ` Marc Marí
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 07/15] libqos: add pc specific interface John Snow
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

To avoid the architecture-specific implementations of the generic qtest
allocator having to know about fields within the allocator, add a
page_size setter method for users or arch specializations to use.
The allocator will assume a default page_size for general use, but it
can always be overridden.

Since this was the last instance of code directly using properties of the
QGuestAllocator object directly, modify the type to be opaque and move
the structure inside of malloc.c.

mlist_new, which was previously exported, is made static local to malloc.c,
as it has no external users.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/malloc-pc.c |  2 +-
 tests/libqos/malloc.c    | 61 ++++++++++++++++++++++++++++++++++++------------
 tests/libqos/malloc.h    | 22 +++--------------
 3 files changed, 50 insertions(+), 35 deletions(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index 6a5fdf3..6e253b6 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -38,7 +38,7 @@ QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags)
 
     ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
     s = alloc_init_flags(flags, 1 << 20, MIN(ram_size, 0xE0000000));
-    s->page_size = PAGE_SIZE;
+    alloc_set_page_size(s, PAGE_SIZE);
 
     /* clean-up */
     g_free(fw_cfg);
diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
index 4ff260f..8cce1ba 100644
--- a/tests/libqos/malloc.c
+++ b/tests/libqos/malloc.c
@@ -16,6 +16,26 @@
 #include <inttypes.h>
 #include <glib.h>
 
+typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
+
+typedef struct MemBlock {
+    QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
+    uint64_t size;
+    uint64_t addr;
+} MemBlock;
+
+typedef struct QGuestAllocator {
+    QAllocOpts opts;
+    uint64_t start;
+    uint64_t end;
+    uint32_t page_size;
+
+    MemList used;
+    MemList free;
+} QGuestAllocator;
+
+#define DEFAULT_PAGE_SIZE 4096
+
 static void mlist_delete(MemList *list, MemBlock *node)
 {
     g_assert(list && node);
@@ -103,6 +123,21 @@ static void mlist_coalesce(MemList *head, MemBlock *node)
     } while (merge);
 }
 
+static MemBlock *mlist_new(uint64_t addr, uint64_t size)
+{
+    MemBlock *block;
+
+    if (!size) {
+        return NULL;
+    }
+    block = g_malloc0(sizeof(MemBlock));
+
+    block->addr = addr;
+    block->size = size;
+
+    return block;
+}
+
 static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode,
                                                                 uint64_t size)
 {
@@ -187,21 +222,6 @@ static void mlist_free(QGuestAllocator *s, uint64_t addr)
     mlist_coalesce(&s->free, node);
 }
 
-MemBlock *mlist_new(uint64_t addr, uint64_t size)
-{
-    MemBlock *block;
-
-    if (!size) {
-        return NULL;
-    }
-    block = g_malloc0(sizeof(MemBlock));
-
-    block->addr = addr;
-    block->size = size;
-
-    return block;
-}
-
 /*
  * Mostly for valgrind happiness, but it does offer
  * a chokepoint for debugging guest memory leaks, too.
@@ -283,6 +303,8 @@ QGuestAllocator *alloc_init(uint64_t start, uint64_t end)
     node = mlist_new(s->start, s->end - s->start);
     QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
 
+    s->page_size = DEFAULT_PAGE_SIZE;
+
     return s;
 }
 
@@ -293,3 +315,12 @@ QGuestAllocator *alloc_init_flags(QAllocOpts opts,
     s->opts = opts;
     return s;
 }
+
+void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size)
+{
+    /* Can't alter the page_size for an allocator in-use */
+    g_assert(QTAILQ_EMPTY(&allocator->used));
+
+    g_assert(is_power_of_2(page_size));
+    allocator->page_size = page_size;
+}
diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 7b29547..a39dba4 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -17,8 +17,6 @@
 #include <sys/types.h>
 #include "qemu/queue.h"
 
-#define MLIST_ENTNAME entries
-
 typedef enum {
     ALLOC_NO_FLAGS    = 0x00,
     ALLOC_LEAK_WARN   = 0x01,
@@ -26,24 +24,8 @@ typedef enum {
     ALLOC_PARANOID    = 0x04
 } QAllocOpts;
 
-typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
-typedef struct MemBlock {
-    QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
-    uint64_t size;
-    uint64_t addr;
-} MemBlock;
+typedef struct QGuestAllocator QGuestAllocator;
 
-typedef struct QGuestAllocator {
-    QAllocOpts opts;
-    uint64_t start;
-    uint64_t end;
-    uint32_t page_size;
-
-    MemList used;
-    MemList free;
-} QGuestAllocator;
-
-MemBlock *mlist_new(uint64_t addr, uint64_t size);
 void alloc_uninit(QGuestAllocator *allocator);
 
 /* Always returns page aligned values */
@@ -53,4 +35,6 @@ void guest_free(QGuestAllocator *allocator, uint64_t addr);
 QGuestAllocator *alloc_init(uint64_t start, uint64_t end);
 QGuestAllocator *alloc_init_flags(QAllocOpts flags,
                                   uint64_t start, uint64_t end);
+void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size);
+
 #endif
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 07/15] libqos: add pc specific interface
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (5 preceding siblings ...)
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 06/15] libqos: Update QGuestAllocator to be opaque John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 08/15] qtest/ahci: Store hba_base in AHCIQState John Snow
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Create an operations structure so that the libqos interface can be
architecture agnostic, and create a pc-specific interface to functions
like qtest_boot.

Move the libqos object in the Makefile from being ahci-test only to
being linked with all tests that utilize the libqos features.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/Makefile           |  6 +++---
 tests/ahci-test.c        |  4 ++--
 tests/libqos/libqos-pc.c | 24 ++++++++++++++++++++++++
 tests/libqos/libqos-pc.h |  9 +++++++++
 tests/libqos/libqos.c    | 16 +++++++++-------
 tests/libqos/libqos.h    | 10 ++++++++--
 6 files changed, 55 insertions(+), 14 deletions(-)
 create mode 100644 tests/libqos/libqos-pc.c
 create mode 100644 tests/libqos/libqos-pc.h

diff --git a/tests/Makefile b/tests/Makefile
index c07d840..d8ef64f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -299,9 +299,9 @@ tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
 tests/test-bitops$(EXESUF): tests/test-bitops.o libqemuutil.a
 
 libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
-libqos-obj-y += tests/libqos/i2c.o
+libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
-libqos-pc-obj-y += tests/libqos/malloc-pc.o
+libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
 libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o
 libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o
@@ -312,7 +312,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/libqos/libqos.o
+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
index 15542b9..3bc9772 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -29,7 +29,7 @@
 #include <glib.h>
 
 #include "libqtest.h"
-#include "libqos/libqos.h"
+#include "libqos/libqos-pc.h"
 #include "libqos/ahci.h"
 #include "libqos/pci-pc.h"
 #include "libqos/malloc-pc.h"
@@ -151,7 +151,7 @@ static AHCIQState *ahci_boot(void)
         " -M q35 "
         "-device ide-hd,drive=drive0 "
         "-global ide-hd.ver=%s";
-    s->parent = qtest_boot(cli, tmp_path, "testdisk", "version");
+    s->parent = qtest_pc_boot(cli, tmp_path, "testdisk", "version");
 
     /* Verify that we have an AHCI device present. */
     s->dev = get_ahci_device();
diff --git a/tests/libqos/libqos-pc.c b/tests/libqos/libqos-pc.c
new file mode 100644
index 0000000..bbace89
--- /dev/null
+++ b/tests/libqos/libqos-pc.c
@@ -0,0 +1,24 @@
+#include "libqos/libqos-pc.h"
+#include "libqos/malloc-pc.h"
+
+static QOSOps qos_ops = {
+    .init_allocator = pc_alloc_init_flags,
+    .uninit_allocator = pc_alloc_uninit
+};
+
+QOSState *qtest_pc_boot(const char *cmdline_fmt, ...)
+{
+    QOSState *qs;
+    va_list ap;
+
+    va_start(ap, cmdline_fmt);
+    qs = qtest_vboot(&qos_ops, cmdline_fmt, ap);
+    va_end(ap);
+
+    return qs;
+}
+
+void qtest_pc_shutdown(QOSState *qs)
+{
+    return qtest_shutdown(qs);
+}
diff --git a/tests/libqos/libqos-pc.h b/tests/libqos/libqos-pc.h
new file mode 100644
index 0000000..316857d
--- /dev/null
+++ b/tests/libqos/libqos-pc.h
@@ -0,0 +1,9 @@
+#ifndef __libqos_pc_h
+#define __libqos_pc_h
+
+#include "libqos/libqos.h"
+
+QOSState *qtest_pc_boot(const char *cmdline_fmt, ...);
+void qtest_pc_shutdown(QOSState *qs);
+
+#endif
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index c8b3ef0..bc8beb2 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -8,7 +8,6 @@
 #include "libqtest.h"
 #include "libqos/libqos.h"
 #include "libqos/pci.h"
-#include "libqos/malloc-pc.h"
 
 /*** Test Setup & Teardown ***/
 
@@ -16,7 +15,7 @@
  * Launch QEMU with the given command line,
  * and then set up interrupts and our guest malloc interface.
  */
-QOSState *qtest_vboot(const char *cmdline_fmt, va_list ap)
+QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap)
 {
     char *cmdline;
 
@@ -24,8 +23,11 @@ QOSState *qtest_vboot(const char *cmdline_fmt, va_list ap)
 
     cmdline = g_strdup_vprintf(cmdline_fmt, ap);
     qs->qts = qtest_start(cmdline);
+    qs->ops = ops;
     qtest_irq_intercept_in(global_qtest, "ioapic");
-    qs->alloc = pc_alloc_init();
+    if (ops && ops->init_allocator) {
+        qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
+    }
 
     g_free(cmdline);
     return qs;
@@ -35,13 +37,13 @@ QOSState *qtest_vboot(const char *cmdline_fmt, va_list ap)
  * Launch QEMU with the given command line,
  * and then set up interrupts and our guest malloc interface.
  */
-QOSState *qtest_boot(const char *cmdline_fmt, ...)
+QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...)
 {
     QOSState *qs;
     va_list ap;
 
     va_start(ap, cmdline_fmt);
-    qs = qtest_vboot(cmdline_fmt, ap);
+    qs = qtest_vboot(ops, cmdline_fmt, ap);
     va_end(ap);
 
     return qs;
@@ -52,8 +54,8 @@ QOSState *qtest_boot(const char *cmdline_fmt, ...)
  */
 void qtest_shutdown(QOSState *qs)
 {
-    if (qs->alloc) {
-        pc_alloc_uninit(qs->alloc);
+    if (qs->alloc && qs->ops && qs->ops->uninit_allocator) {
+        qs->ops->uninit_allocator(qs->alloc);
         qs->alloc = NULL;
     }
     qtest_quit(qs->qts);
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index 7ae0a8d..612d41e 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -5,13 +5,19 @@
 #include "libqos/pci.h"
 #include "libqos/malloc-pc.h"
 
+typedef struct QOSOps {
+    QGuestAllocator *(*init_allocator)(QAllocOpts);
+    void (*uninit_allocator)(QGuestAllocator *);
+} QOSOps;
+
 typedef struct QOSState {
     QTestState *qts;
     QGuestAllocator *alloc;
+    QOSOps *ops;
 } QOSState;
 
-QOSState *qtest_vboot(const char *cmdline_fmt, va_list ap);
-QOSState *qtest_boot(const char *cmdline_fmt, ...);
+QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap);
+QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...);
 void qtest_shutdown(QOSState *qs);
 
 static inline uint64_t qmalloc(QOSState *q, size_t bytes)
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 08/15] qtest/ahci: Store hba_base in AHCIQState
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (6 preceding siblings ...)
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 07/15] libqos: add pc specific interface John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 09/15] qtest/ahci: finalize AHCIQState consolidation John Snow
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Store the HBA memory base address in the new state object, to simplify
function prototypes and encourage a more functional testing style.

This causes a lot of churn, but this patch is as "simplified" as I could
get it to be. This patch is therefore fairly mechanical and straightforward:
Any case where we pass "hba_base" has been consolidated into the AHCIQState
object and we pass the one unified parameter.

Any case where we reference "ahci" and "hba_state" have been modified to use
"ahci->dev" for the PCIDevice and "ahci->hba_state" to get at the base memory
address, accordingly.

Notes:

 - A needless return is removed from start_ahci_device.

 - For ease of reviewing, this patch can be reproduced (mostly) by:
   # Replace (ahci, hba_base) prototypes with unified parameter
   's/(QPCIDevice \*ahci, void \*\?\*hba_base/(AHCIQState *ahci/'

   # Replace (ahci->dev, hba_base) calls with unified parameter
   's/(ahci->dev, &\?hba_base)/(ahci)/'

   # Replace calls to PCI config space using "ahci" with "ahci->dev"
   's/qpci_config_\(read\|write\)\(.\)(ahci,/qpci_config_\1\2(ahci->dev,/'

   After these, the remaining differences are easy to review by hand.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/ahci-test.c   | 136 +++++++++++++++++++++++++---------------------------
 tests/libqos/ahci.h |   1 +
 2 files changed, 65 insertions(+), 72 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 3bc9772..a6e507f 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -52,8 +52,9 @@ static bool ahci_pedantic;
 static uint32_t ahci_fingerprint;
 
 /*** 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_READ(OFST) qpci_io_readl(ahci->dev, ahci->hba_base + (OFST))
+#define AHCI_WRITE(OFST, VAL) qpci_io_writel(ahci->dev,                 \
+                                             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))
@@ -70,16 +71,17 @@ 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 start_ahci_device(AHCIQState *ahci);
 static void free_ahci_device(QPCIDevice *dev);
-static void ahci_test_port_spec(QPCIDevice *ahci, void *hba_base,
+
+static void ahci_test_port_spec(AHCIQState *ahci,
                                 HBACap *hcap, uint8_t port);
-static void ahci_test_pci_spec(QPCIDevice *ahci);
-static void ahci_test_pci_caps(QPCIDevice *ahci, uint16_t header,
+static void ahci_test_pci_spec(AHCIQState *ahci);
+static void ahci_test_pci_caps(AHCIQState *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);
+static void ahci_test_satacap(AHCIQState *ahci, uint8_t offset);
+static void ahci_test_msicap(AHCIQState *ahci, uint8_t offset);
+static void ahci_test_pmcap(AHCIQState *ahci, uint8_t offset);
 
 /*** Utilities ***/
 
@@ -178,21 +180,21 @@ static void ahci_shutdown(AHCIQState *ahci)
 /**
  * Start the PCI device and sanity-check default operation.
  */
-static void ahci_pci_enable(QPCIDevice *ahci, void **hba_base)
+static void ahci_pci_enable(AHCIQState *ahci)
 {
     uint8_t reg;
 
-    start_ahci_device(ahci, hba_base);
+    start_ahci_device(ahci);
 
     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 = qpci_config_readb(ahci->dev, 0x92);
         reg |= 0x3F;
-        qpci_config_writeb(ahci, 0x92, reg);
+        qpci_config_writeb(ahci->dev, 0x92, reg);
         /* 0...0111111b -- bit significant, ports 0-5 enabled. */
-        ASSERT_BIT_SET(qpci_config_readb(ahci, 0x92), 0x3F);
+        ASSERT_BIT_SET(qpci_config_readb(ahci->dev, 0x92), 0x3F);
         break;
     }
 
@@ -201,15 +203,13 @@ static void ahci_pci_enable(QPCIDevice *ahci, void **hba_base)
 /**
  * Map BAR5/ABAR, and engage the PCI device.
  */
-static QPCIDevice *start_ahci_device(QPCIDevice *ahci, void **hba_base)
+static void start_ahci_device(AHCIQState *ahci)
 {
     /* Map AHCI's ABAR (BAR5) */
-    *hba_base = qpci_iomap(ahci, 5, &barsize);
+    ahci->hba_base = qpci_iomap(ahci->dev, 5, &barsize);
 
     /* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */
-    qpci_device_enable(ahci);
-
-    return ahci;
+    qpci_device_enable(ahci->dev);
 }
 
 /**
@@ -217,7 +217,7 @@ static QPCIDevice *start_ahci_device(QPCIDevice *ahci, void **hba_base)
  * 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)
+static void ahci_hba_enable(AHCIQState *ahci)
 {
     /* Bits of interest in this section:
      * GHC.AE     Global Host Control / AHCI Enable
@@ -230,14 +230,11 @@ static void ahci_hba_enable(QPCIDevice *ahci, void *hba_base)
      */
 
     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);
@@ -351,14 +348,14 @@ static void ahci_hba_enable(QPCIDevice *ahci, void *hba_base)
 /**
  * Implementation for test_pci_spec. Ensures PCI configuration space is sane.
  */
-static void ahci_test_pci_spec(QPCIDevice *ahci)
+static void ahci_test_pci_spec(AHCIQState *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);
+    data = qpci_config_readw(ahci->dev, PCI_COMMAND);
     ASSERT_BIT_CLEAR(data, PCI_COMMAND_MEMORY);
     ASSERT_BIT_CLEAR(data, PCI_COMMAND_MASTER);
     ASSERT_BIT_CLEAR(data, PCI_COMMAND_SPECIAL);     /* Reserved */
@@ -370,7 +367,7 @@ static void ahci_test_pci_spec(QPCIDevice *ahci)
     ASSERT_BIT_CLEAR(data, PCI_COMMAND_INTX_DISABLE);
     ASSERT_BIT_CLEAR(data, 0xF800);                  /* Reserved */
 
-    data = qpci_config_readw(ahci, PCI_STATUS);
+    data = qpci_config_readw(ahci->dev, 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 */
@@ -383,7 +380,7 @@ static void ahci_test_pci_spec(QPCIDevice *ahci)
     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);
+    datal = qpci_config_readl(ahci->dev, 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. */
@@ -406,38 +403,38 @@ static void ahci_test_pci_spec(QPCIDevice *ahci)
         g_assert_not_reached();
     }
 
-    datab = qpci_config_readb(ahci, PCI_CACHE_LINE_SIZE);
+    datab = qpci_config_readb(ahci->dev, PCI_CACHE_LINE_SIZE);
     g_assert_cmphex(datab, ==, 0);
 
-    datab = qpci_config_readb(ahci, PCI_LATENCY_TIMER);
+    datab = qpci_config_readb(ahci->dev, PCI_LATENCY_TIMER);
     g_assert_cmphex(datab, ==, 0);
 
     /* Only the bottom 7 bits must be off. */
-    datab = qpci_config_readb(ahci, PCI_HEADER_TYPE);
+    datab = qpci_config_readb(ahci->dev, 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);
+    datab = qpci_config_readb(ahci->dev, 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);
+    datal = qpci_config_readl(ahci->dev, 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);
+    qpci_config_writel(ahci->dev, PCI_BASE_ADDRESS_5, 0xFFFFFFFF);
+    datal = qpci_config_readl(ahci->dev, 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);
+    datal = qpci_config_readl(ahci->dev, 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);
+    data = qpci_config_readw(ahci->dev, datal);
 
     switch (ahci_fingerprint) {
     case AHCI_INTEL_ICH9:
@@ -452,18 +449,18 @@ static void ahci_test_pci_spec(QPCIDevice *ahci)
     ahci_test_pci_caps(ahci, data, (uint8_t)datal);
 
     /* Reserved. */
-    datal = qpci_config_readl(ahci, PCI_CAPABILITY_LIST + 4);
+    datal = qpci_config_readl(ahci->dev, 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);
+    datab = qpci_config_readb(ahci->dev, 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,
+static void ahci_test_pci_caps(AHCIQState *ahci, uint16_t header,
                                uint8_t offset)
 {
     uint8_t cid = header & 0xFF;
@@ -487,14 +484,14 @@ static void ahci_test_pci_caps(QPCIDevice *ahci, uint16_t header,
     }
 
     if (next) {
-        ahci_test_pci_caps(ahci, qpci_config_readw(ahci, next), next);
+        ahci_test_pci_caps(ahci, qpci_config_readw(ahci->dev, next), next);
     }
 }
 
 /**
  * Test SATA PCI capabilitity for AHCI specification adherence.
  */
-static void ahci_test_satacap(QPCIDevice *ahci, uint8_t offset)
+static void ahci_test_satacap(AHCIQState *ahci, uint8_t offset)
 {
     uint16_t dataw;
     uint32_t datal;
@@ -502,11 +499,11 @@ static void ahci_test_satacap(QPCIDevice *ahci, uint8_t offset)
     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);
+    dataw = qpci_config_readw(ahci->dev, offset + 2);
     g_assert_cmphex(dataw, ==, 0x10);
 
     /* Grab the SATACR1 register. */
-    datal = qpci_config_readw(ahci, offset + 4);
+    datal = qpci_config_readw(ahci->dev, offset + 4);
 
     switch (datal & 0x0F) {
     case 0x04: /* BAR0 */
@@ -529,30 +526,30 @@ static void ahci_test_satacap(QPCIDevice *ahci, uint8_t offset)
 /**
  * Test MSI PCI capability for AHCI specification adherence.
  */
-static void ahci_test_msicap(QPCIDevice *ahci, uint8_t offset)
+static void ahci_test_msicap(AHCIQState *ahci, uint8_t offset)
 {
     uint16_t dataw;
     uint32_t datal;
 
     g_test_message("Verifying MSICAP");
 
-    dataw = qpci_config_readw(ahci, offset + PCI_MSI_FLAGS);
+    dataw = qpci_config_readw(ahci->dev, 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);
+    datal = qpci_config_readl(ahci->dev, 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);
+        datal = qpci_config_readl(ahci->dev, offset + PCI_MSI_ADDRESS_HI);
         g_assert_cmphex(datal, ==, 0);
-        dataw = qpci_config_readw(ahci, offset + PCI_MSI_DATA_64);
+        dataw = qpci_config_readw(ahci->dev, 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);
+        dataw = qpci_config_readw(ahci->dev, offset + PCI_MSI_DATA_32);
         g_assert_cmphex(dataw, ==, 0);
     }
 }
@@ -560,26 +557,26 @@ static void ahci_test_msicap(QPCIDevice *ahci, uint8_t offset)
 /**
  * Test Power Management PCI capability for AHCI specification adherence.
  */
-static void ahci_test_pmcap(QPCIDevice *ahci, uint8_t offset)
+static void ahci_test_pmcap(AHCIQState *ahci, uint8_t offset)
 {
     uint16_t dataw;
 
     g_test_message("Verifying PMCAP");
 
-    dataw = qpci_config_readw(ahci, offset + PCI_PM_PMC);
+    dataw = qpci_config_readw(ahci->dev, 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);
+    dataw = qpci_config_readw(ahci->dev, 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);
 }
 
-static void ahci_test_hba_spec(QPCIDevice *ahci, void *hba_base)
+static void ahci_test_hba_spec(AHCIQState *ahci)
 {
     HBACap hcap;
     unsigned i;
@@ -588,8 +585,7 @@ static void ahci_test_hba_spec(QPCIDevice *ahci, void *hba_base)
     uint8_t nports_impl;
     uint8_t maxports;
 
-    g_assert(ahci != 0);
-    g_assert(hba_base != 0);
+    g_assert(ahci != NULL);
 
     /*
      * Note that the AHCI spec does expect the BIOS to set up a few things:
@@ -731,7 +727,7 @@ static void ahci_test_hba_spec(QPCIDevice *ahci, void *hba_base)
     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);
+            ahci_test_port_spec(ahci, &hcap, i);
         } else {
             uint16_t j;
             uint16_t low = AHCI_PORTS + (32 * i);
@@ -750,7 +746,7 @@ static void ahci_test_hba_spec(QPCIDevice *ahci, void *hba_base)
 /**
  * Test the memory space for one port for specification adherence.
  */
-static void ahci_test_port_spec(QPCIDevice *ahci, void *hba_base,
+static void ahci_test_port_spec(AHCIQState *ahci,
                                 HBACap *hcap, uint8_t port)
 {
     uint32_t reg;
@@ -902,7 +898,7 @@ 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)
+static void ahci_test_identify(AHCIQState *ahci)
 {
     RegD2HFIS *d2h = g_malloc0(0x20);
     RegD2HFIS *pio = g_malloc0(0x20);
@@ -915,7 +911,6 @@ static void ahci_test_identify(QPCIDevice *ahci, void *hba_base)
     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
@@ -1100,7 +1095,7 @@ static void test_pci_spec(void)
 {
     AHCIQState *ahci;
     ahci = ahci_boot();
-    ahci_test_pci_spec(ahci->dev);
+    ahci_test_pci_spec(ahci);
     ahci_shutdown(ahci);
 }
 
@@ -1111,9 +1106,9 @@ static void test_pci_spec(void)
 static void test_pci_enable(void)
 {
     AHCIQState *ahci;
-    void *hba_base;
+
     ahci = ahci_boot();
-    ahci_pci_enable(ahci->dev, &hba_base);
+    ahci_pci_enable(ahci);
     ahci_shutdown(ahci);
 }
 
@@ -1124,11 +1119,10 @@ static void test_pci_enable(void)
 static void test_hba_spec(void)
 {
     AHCIQState *ahci;
-    void *hba_base;
 
     ahci = ahci_boot();
-    ahci_pci_enable(ahci->dev, &hba_base);
-    ahci_test_hba_spec(ahci->dev, hba_base);
+    ahci_pci_enable(ahci);
+    ahci_test_hba_spec(ahci);
     ahci_shutdown(ahci);
 }
 
@@ -1139,11 +1133,10 @@ static void test_hba_spec(void)
 static void test_hba_enable(void)
 {
     AHCIQState *ahci;
-    void *hba_base;
 
     ahci = ahci_boot();
-    ahci_pci_enable(ahci->dev, &hba_base);
-    ahci_hba_enable(ahci->dev, hba_base);
+    ahci_pci_enable(ahci);
+    ahci_hba_enable(ahci);
     ahci_shutdown(ahci);
 }
 
@@ -1154,12 +1147,11 @@ static void test_hba_enable(void)
 static void test_identify(void)
 {
     AHCIQState *ahci;
-    void *hba_base;
 
     ahci = ahci_boot();
-    ahci_pci_enable(ahci->dev, &hba_base);
-    ahci_hba_enable(ahci->dev, hba_base);
-    ahci_test_identify(ahci->dev, hba_base);
+    ahci_pci_enable(ahci);
+    ahci_hba_enable(ahci);
+    ahci_test_identify(ahci);
     ahci_shutdown(ahci);
 }
 
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index bc5f45d..e9e0206 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -248,6 +248,7 @@
 typedef struct AHCIQState {
     QOSState *parent;
     QPCIDevice *dev;
+    void *hba_base;
 } AHCIQState;
 
 /**
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 09/15] qtest/ahci: finalize AHCIQState consolidation
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (7 preceding siblings ...)
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 08/15] qtest/ahci: Store hba_base in AHCIQState John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 10/15] qtest/ahci: remove pcibus global John Snow
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Move barsize, ahci_fingerprint and capabilities registers into
the AHCIQState object, removing global ahci-related state
from the ahci-test.c file.

More churn, less globals.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/ahci-test.c   | 80 +++++++++++++++++++++++++----------------------------
 tests/libqos/ahci.h |  9 +++---
 2 files changed, 42 insertions(+), 47 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index a6e507f..96fb45c 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -46,10 +46,8 @@
 /*** 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;
 
 /*** IO macros for the AHCI memory registers. ***/
 #define AHCI_READ(OFST) qpci_io_readl(ahci->dev, ahci->hba_base + (OFST))
@@ -70,12 +68,11 @@ static uint32_t ahci_fingerprint;
                                           PX_RREG((port), (reg)) & ~(mask));
 
 /*** Function Declarations ***/
-static QPCIDevice *get_ahci_device(void);
+static QPCIDevice *get_ahci_device(uint32_t *fingerprint);
 static void start_ahci_device(AHCIQState *ahci);
 static void free_ahci_device(QPCIDevice *dev);
 
-static void ahci_test_port_spec(AHCIQState *ahci,
-                                HBACap *hcap, uint8_t port);
+static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port);
 static void ahci_test_pci_spec(AHCIQState *ahci);
 static void ahci_test_pci_caps(AHCIQState *ahci, uint16_t header,
                                uint8_t offset);
@@ -99,9 +96,10 @@ static void string_bswap16(uint16_t *s, size_t bytes)
 /**
  * Locate, verify, and return a handle to the AHCI device.
  */
-static QPCIDevice *get_ahci_device(void)
+static QPCIDevice *get_ahci_device(uint32_t *fingerprint)
 {
     QPCIDevice *ahci;
+    uint32_t ahci_fingerprint;
 
     pcibus = qpci_init_pc();
 
@@ -119,6 +117,9 @@ static QPCIDevice *get_ahci_device(void)
         g_assert_not_reached();
     }
 
+    if (fingerprint) {
+        *fingerprint = ahci_fingerprint;
+    }
     return ahci;
 }
 
@@ -131,9 +132,6 @@ static void free_ahci_device(QPCIDevice *ahci)
         qpci_free_pc(pcibus);
         pcibus = NULL;
     }
-
-    /* Clear our cached barsize information. */
-    barsize = 0;
 }
 
 /*** Test Setup & Teardown ***/
@@ -156,7 +154,7 @@ static AHCIQState *ahci_boot(void)
     s->parent = qtest_pc_boot(cli, tmp_path, "testdisk", "version");
 
     /* Verify that we have an AHCI device present. */
-    s->dev = get_ahci_device();
+    s->dev = get_ahci_device(&s->fingerprint);
 
     /* Stopgap: Copy the allocator reference */
     guest_malloc = s->parent->alloc;
@@ -186,7 +184,7 @@ static void ahci_pci_enable(AHCIQState *ahci)
 
     start_ahci_device(ahci);
 
-    switch (ahci_fingerprint) {
+    switch (ahci->fingerprint) {
     case AHCI_INTEL_ICH9:
         /* ICH9 has a register at PCI 0x92 that
          * acts as a master port enabler mask. */
@@ -206,7 +204,7 @@ static void ahci_pci_enable(AHCIQState *ahci)
 static void start_ahci_device(AHCIQState *ahci)
 {
     /* Map AHCI's ABAR (BAR5) */
-    ahci->hba_base = qpci_iomap(ahci->dev, 5, &barsize);
+    ahci->hba_base = qpci_iomap(ahci->dev, 5, &ahci->barsize);
 
     /* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */
     qpci_device_enable(ahci->dev);
@@ -228,21 +226,23 @@ static void ahci_hba_enable(AHCIQState *ahci)
      * PxCMD.FR   "FIS Receive Running"
      * PxCMD.CR   "Command List Running"
      */
-
-    g_assert(ahci != NULL);
-
     uint32_t reg, ports_impl, clb, fb;
     uint16_t i;
     uint8_t num_cmd_slots;
 
+    g_assert(ahci != NULL);
+
     /* Set GHC.AE to 1 */
     AHCI_SET(AHCI_GHC, AHCI_GHC_AE);
     reg = AHCI_RREG(AHCI_GHC);
     ASSERT_BIT_SET(reg, AHCI_GHC_AE);
 
+    /* Cache CAP and CAP2. */
+    ahci->cap = AHCI_RREG(AHCI_CAP);
+    ahci->cap2 = AHCI_RREG(AHCI_CAP2);
+
     /* 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;
+    num_cmd_slots = ((ahci->cap & AHCI_CAP_NCS) >> ctzl(AHCI_CAP_NCS)) + 1;
     g_test_message("Number of Command Slots: %u", num_cmd_slots);
 
     /* Determine which ports are implemented. */
@@ -436,7 +436,7 @@ static void ahci_test_pci_spec(AHCIQState *ahci)
     /* Check specification adherence for capability extenstions. */
     data = qpci_config_readw(ahci->dev, datal);
 
-    switch (ahci_fingerprint) {
+    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);
@@ -578,9 +578,8 @@ static void ahci_test_pmcap(AHCIQState *ahci, uint8_t offset)
 
 static void ahci_test_hba_spec(AHCIQState *ahci)
 {
-    HBACap hcap;
     unsigned i;
-    uint32_t cap, cap2, reg;
+    uint32_t reg;
     uint32_t ports;
     uint8_t nports_impl;
     uint8_t maxports;
@@ -608,15 +607,15 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
      */
 
     /* 1 CAP - Capabilities Register */
-    cap = AHCI_RREG(AHCI_CAP);
-    ASSERT_BIT_CLEAR(cap, AHCI_CAP_RESERVED);
+    ahci->cap = AHCI_RREG(AHCI_CAP);
+    ASSERT_BIT_CLEAR(ahci->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)) {
+    if (BITSET(ahci->cap, AHCI_CAP_SAM)) {
         g_test_message("Supports AHCI-Only Mode: GHC_AE is Read-Only.");
         ASSERT_BIT_SET(reg, AHCI_GHC_AE);
     } else {
@@ -634,13 +633,13 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     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_cmpuint(((AHCI_CAP_NP & ahci->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;
+    g_assert_cmphex(ahci->barsize, >, 0);
+    maxports = (ahci->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);
 
@@ -659,7 +658,7 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
 
     /* 6 Command Completion Coalescing Control: depends on CAP.CCCS. */
     reg = AHCI_RREG(AHCI_CCCCTL);
-    if (BITSET(cap, AHCI_CAP_CCCS)) {
+    if (BITSET(ahci->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);
@@ -675,13 +674,13 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
 
     /* 8 EM_LOC */
     reg = AHCI_RREG(AHCI_EMLOC);
-    if (BITCLR(cap, AHCI_CAP_EMS)) {
+    if (BITCLR(ahci->cap, AHCI_CAP_EMS)) {
         g_assert_cmphex(reg, ==, 0);
     }
 
     /* 9 EM_CTL */
     reg = AHCI_RREG(AHCI_EMCTL);
-    if (BITSET(cap, AHCI_CAP_EMS)) {
+    if (BITSET(ahci->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);
@@ -691,8 +690,8 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     }
 
     /* 10 CAP2 -- Capabilities Extended */
-    cap2 = AHCI_RREG(AHCI_CAP2);
-    ASSERT_BIT_CLEAR(cap2, AHCI_CAP2_RESERVED);
+    ahci->cap2 = AHCI_RREG(AHCI_CAP2);
+    ASSERT_BIT_CLEAR(ahci->cap2, AHCI_CAP2_RESERVED);
 
     /* 11 BOHC -- Bios/OS Handoff Control */
     reg = AHCI_RREG(AHCI_BOHC);
@@ -706,7 +705,7 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     }
 
     /* 24 -- 39: NVMHCI */
-    if (BITCLR(cap2, AHCI_CAP2_NVMP)) {
+    if (BITCLR(ahci->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);
@@ -722,12 +721,10 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     }
 
     /* 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, &hcap, i);
+            ahci_test_port_spec(ahci, i);
         } else {
             uint16_t j;
             uint16_t low = AHCI_PORTS + (32 * i);
@@ -746,8 +743,7 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
 /**
  * Test the memory space for one port for specification adherence.
  */
-static void ahci_test_port_spec(AHCIQState *ahci,
-                                HBACap *hcap, uint8_t port)
+static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port)
 {
     uint32_t reg;
     unsigned i;
@@ -757,7 +753,7 @@ static void ahci_test_port_spec(AHCIQState *ahci,
     ASSERT_BIT_CLEAR(reg, AHCI_PX_CLB_RESERVED);
 
     /* (1) CLBU */
-    if (BITCLR(hcap->cap, AHCI_CAP_S64A)) {
+    if (BITCLR(ahci->cap, AHCI_CAP_S64A)) {
         reg = PX_RREG(port, AHCI_PX_CLBU);
         g_assert_cmphex(reg, ==, 0);
     }
@@ -767,7 +763,7 @@ static void ahci_test_port_spec(AHCIQState *ahci,
     ASSERT_BIT_CLEAR(reg, AHCI_PX_FB_RESERVED);
 
     /* (3) FBU */
-    if (BITCLR(hcap->cap, AHCI_CAP_S64A)) {
+    if (BITCLR(ahci->cap, AHCI_CAP_S64A)) {
         reg = PX_RREG(port, AHCI_PX_FBU);
         g_assert_cmphex(reg, ==, 0);
     }
@@ -803,7 +799,7 @@ static void ahci_test_port_spec(AHCIQState *ahci,
         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)) {
+    if (BITCLR(ahci->cap, AHCI_CAP_SMPS)) {
         ASSERT_BIT_CLEAR(reg, AHCI_PX_CMD_MPSS);
         ASSERT_BIT_CLEAR(reg, AHCI_PX_CMD_MPSP);
     }
@@ -814,7 +810,7 @@ static void ahci_test_port_spec(AHCIQState *ahci,
     /* HPCP and ESP cannot both be active. */
     g_assert(!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)) {
+    if (BITCLR(ahci->cap, AHCI_CAP_FBSS)) {
         ASSERT_BIT_CLEAR(reg, AHCI_PX_CMD_FBSCP);
     }
 
@@ -874,7 +870,7 @@ static void ahci_test_port_spec(AHCIQState *ahci,
     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 (BITSET(ahci->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);
     }
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index e9e0206..8e92385 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -249,6 +249,10 @@ typedef struct AHCIQState {
     QOSState *parent;
     QPCIDevice *dev;
     void *hba_base;
+    uint64_t barsize;
+    uint32_t fingerprint;
+    uint32_t cap;
+    uint32_t cap2;
 } AHCIQState;
 
 /**
@@ -340,11 +344,6 @@ typedef struct PRD {
     uint32_t dbc;  /* Data Byte Count (0-indexed) & Interrupt Flag (bit 2^31) */
 } PRD;
 
-typedef struct HBACap {
-    uint32_t cap;
-    uint32_t cap2;
-} HBACap;
-
 /*** Macro Utilities ***/
 #define BITANY(data, mask) (((data) & (mask)) != 0)
 #define BITSET(data, mask) (((data) & (mask)) == (mask))
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 10/15] qtest/ahci: remove pcibus global
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (8 preceding siblings ...)
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 09/15] qtest/ahci: finalize AHCIQState consolidation John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 11/15] qtest/ahci: remove guest_malloc global John Snow
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Rely on the PCI Device's bus pointer instead.
One less global to worry about.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/ahci-test.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 96fb45c..0cc56ab 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -45,7 +45,6 @@
 
 /*** Globals ***/
 static QGuestAllocator *guest_malloc;
-static QPCIBus *pcibus;
 static char tmp_path[] = "/tmp/qtest.XXXXXX";
 static bool ahci_pedantic;
 
@@ -100,6 +99,7 @@ static QPCIDevice *get_ahci_device(uint32_t *fingerprint)
 {
     QPCIDevice *ahci;
     uint32_t ahci_fingerprint;
+    QPCIBus *pcibus;
 
     pcibus = qpci_init_pc();
 
@@ -123,15 +123,13 @@ static QPCIDevice *get_ahci_device(uint32_t *fingerprint)
     return ahci;
 }
 
-static void free_ahci_device(QPCIDevice *ahci)
+static void free_ahci_device(QPCIDevice *dev)
 {
+    QPCIBus *pcibus = dev ? dev->bus : NULL;
+
     /* libqos doesn't have a function for this, so free it manually */
-    g_free(ahci);
-
-    if (pcibus) {
-        qpci_free_pc(pcibus);
-        pcibus = NULL;
-    }
+    g_free(dev);
+    qpci_free_pc(pcibus);
 }
 
 /*** Test Setup & Teardown ***/
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 11/15] qtest/ahci: remove guest_malloc global
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (9 preceding siblings ...)
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 10/15] qtest/ahci: remove pcibus global John Snow
@ 2015-01-19 20:15 ` John Snow
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 12/15] libqos/ahci: Functional register helpers John Snow
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Make helper routines rely on the earmarked
guest allocator object with AHCIQState/QOSSTate instead.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/ahci-test.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 0cc56ab..bb98968 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -32,7 +32,6 @@
 #include "libqos/libqos-pc.h"
 #include "libqos/ahci.h"
 #include "libqos/pci-pc.h"
-#include "libqos/malloc-pc.h"
 
 #include "qemu-common.h"
 #include "qemu/host-utils.h"
@@ -44,7 +43,6 @@
 #define TEST_IMAGE_SIZE    (64 * 1024 * 1024)
 
 /*** Globals ***/
-static QGuestAllocator *guest_malloc;
 static char tmp_path[] = "/tmp/qtest.XXXXXX";
 static bool ahci_pedantic;
 
@@ -92,6 +90,11 @@ static void string_bswap16(uint16_t *s, size_t bytes)
     }
 }
 
+static uint64_t ahci_alloc(AHCIQState *ahci, size_t bytes)
+{
+    return qmalloc(ahci->parent, bytes);
+}
+
 /**
  * Locate, verify, and return a handle to the AHCI device.
  */
@@ -154,9 +157,6 @@ static AHCIQState *ahci_boot(void)
     /* Verify that we have an AHCI device present. */
     s->dev = get_ahci_device(&s->fingerprint);
 
-    /* Stopgap: Copy the allocator reference */
-    guest_malloc = s->parent->alloc;
-
     return s;
 }
 
@@ -272,13 +272,13 @@ static void ahci_hba_enable(AHCIQState *ahci)
 
         /* 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);
+        clb = ahci_alloc(ahci, 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);
+        fb = ahci_alloc(ahci, 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));
@@ -951,12 +951,12 @@ static void ahci_test_identify(AHCIQState *ahci)
 
     /* 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));
+    table = ahci_alloc(ahci, CMD_TBL_SIZ(1));
     g_assert(table);
     ASSERT_BIT_CLEAR(table, 0x7F);
 
     /* Create a data buffer ... where we will dump the IDENTIFY data to. */
-    data_ptr = guest_alloc(guest_malloc, 512);
+    data_ptr = ahci_alloc(ahci, 512);
     g_assert(data_ptr);
 
     /* Grab the Command List Buffer pointer */
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 12/15] libqos/ahci: Functional register helpers
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (10 preceding siblings ...)
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 11/15] qtest/ahci: remove guest_malloc global John Snow
@ 2015-01-19 20:16 ` John Snow
  2015-01-28 10:26   ` Paolo Bonzini
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 13/15] qtest/ahci: remove getter/setter macros John Snow
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2015-01-19 20:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Introduce a set of "static inline" register helpers that are intended to
replace the current set of macros with more functional versions that are
better suited to inclusion in libqos than porcelain macros.

As a stopgap measure before eliminating the porcelain macros, define them
to use the new functions defined in the ahci.h header.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index bb98968..25e54b8 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -47,22 +47,19 @@ static char tmp_path[] = "/tmp/qtest.XXXXXX";
 static bool ahci_pedantic;
 
 /*** IO macros for the AHCI memory registers. ***/
-#define AHCI_READ(OFST) qpci_io_readl(ahci->dev, ahci->hba_base + (OFST))
-#define AHCI_WRITE(OFST, VAL) qpci_io_writel(ahci->dev,                 \
-                                             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))
+#define AHCI_READ(OFST)       ahci_mread(ahci, (OFST))
+#define AHCI_WRITE(OFST, VAL) ahci_mwrite(ahci, (OFST), (VAL))
+#define AHCI_RREG(regno)      ahci_rreg(ahci, (regno))
+#define AHCI_WREG(regno, val) ahci_wreg(ahci, (regno), (val))
+#define AHCI_SET(regno, mask) ahci_set(ahci, (regno), (mask))
+#define AHCI_CLR(regno, mask) ahci_clr(ahci, (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));
+#define PX_OFST(port, regno)      ahci_px_ofst((port), (regno))
+#define PX_RREG(port, regno)      ahci_px_rreg(ahci, (port), (regno))
+#define PX_WREG(port, regno, val) ahci_px_wreg(ahci, (port), (regno), (val))
+#define PX_SET(port, reg, mask)   ahci_px_set(ahci, (port), (reg), (mask))
+#define PX_CLR(port, reg, mask)   ahci_px_clr(ahci, (port), (reg), (mask))
 
 /*** Function Declarations ***/
 static QPCIDevice *get_ahci_device(uint32_t *fingerprint);
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 8e92385..645f05b 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -354,4 +354,67 @@ typedef struct PRD {
 /* For calculating how big the PRD table needs to be: */
 #define CMD_TBL_SIZ(n) ((0x80 + ((n) * sizeof(PRD)) + 0x7F) & ~0x7F)
 
+/* Helpers for reading/writing AHCI HBA register values */
+
+static inline uint32_t ahci_mread(AHCIQState *ahci, size_t offset)
+{
+    return qpci_io_readl(ahci->dev, ahci->hba_base + offset);
+}
+
+static inline void ahci_mwrite(AHCIQState *ahci, size_t offset, uint32_t value)
+{
+    qpci_io_writel(ahci->dev, ahci->hba_base + offset, value);
+}
+
+static inline uint32_t ahci_rreg(AHCIQState *ahci, uint32_t reg_num)
+{
+    return ahci_mread(ahci, 4 * reg_num);
+}
+
+static inline void ahci_wreg(AHCIQState *ahci, uint32_t reg_num, uint32_t value)
+{
+    ahci_mwrite(ahci, 4 * reg_num, value);
+}
+
+static inline void ahci_set(AHCIQState *ahci, uint32_t reg_num, uint32_t mask)
+{
+    ahci_wreg(ahci, reg_num, ahci_rreg(ahci, reg_num) | mask);
+}
+
+static inline void ahci_clr(AHCIQState *ahci, uint32_t reg_num, uint32_t mask)
+{
+    ahci_wreg(ahci, reg_num, ahci_rreg(ahci, reg_num) & ~mask);
+}
+
+static inline size_t ahci_px_offset(uint8_t port, uint32_t reg_num)
+{
+    return AHCI_PORTS + (HBA_PORT_NUM_REG * port) + reg_num;
+}
+
+static inline uint32_t ahci_px_rreg(AHCIQState *ahci, uint8_t port,
+                                    uint32_t reg_num)
+{
+    return ahci_rreg(ahci, ahci_px_offset(port, reg_num));
+}
+
+static inline void ahci_px_wreg(AHCIQState *ahci, uint8_t port,
+                                uint32_t reg_num, uint32_t value)
+{
+    ahci_wreg(ahci, ahci_px_offset(port, reg_num), value);
+}
+
+static inline void ahci_px_set(AHCIQState *ahci, uint8_t port,
+                               uint32_t reg_num, uint32_t mask)
+{
+    ahci_px_wreg(ahci, port, reg_num,
+                 ahci_px_rreg(ahci, port, reg_num) | mask);
+}
+
+static inline void ahci_px_clr(AHCIQState *ahci, uint8_t port,
+                               uint32_t reg_num, uint32_t mask)
+{
+    ahci_px_wreg(ahci, port, reg_num,
+                 ahci_px_rreg(ahci, port, reg_num) & ~mask);
+}
+
 #endif
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 13/15] qtest/ahci: remove getter/setter macros
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (11 preceding siblings ...)
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 12/15] libqos/ahci: Functional register helpers John Snow
@ 2015-01-19 20:16 ` John Snow
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 14/15] qtest/ahci: Bookmark FB and CLB pointers John Snow
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

These macros were a bad idea: They relied upon certain arguments being
present locally with a specific name.

With the endgoal being to factor out AHCI helper functions outside of
the test file itself, these have to be replaced by more explicit helper
setter/getter functions.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/ahci-test.c | 178 +++++++++++++++++++++++++-----------------------------
 1 file changed, 83 insertions(+), 95 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 25e54b8..aa1f66f 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -46,21 +46,6 @@
 static char tmp_path[] = "/tmp/qtest.XXXXXX";
 static bool ahci_pedantic;
 
-/*** IO macros for the AHCI memory registers. ***/
-#define AHCI_READ(OFST)       ahci_mread(ahci, (OFST))
-#define AHCI_WRITE(OFST, VAL) ahci_mwrite(ahci, (OFST), (VAL))
-#define AHCI_RREG(regno)      ahci_rreg(ahci, (regno))
-#define AHCI_WREG(regno, val) ahci_wreg(ahci, (regno), (val))
-#define AHCI_SET(regno, mask) ahci_set(ahci, (regno), (mask))
-#define AHCI_CLR(regno, mask) ahci_clr(ahci, (regno), (mask))
-
-/*** IO macros for port-specific offsets inside of AHCI memory. ***/
-#define PX_OFST(port, regno)      ahci_px_ofst((port), (regno))
-#define PX_RREG(port, regno)      ahci_px_rreg(ahci, (port), (regno))
-#define PX_WREG(port, regno, val) ahci_px_wreg(ahci, (port), (regno), (val))
-#define PX_SET(port, reg, mask)   ahci_px_set(ahci, (port), (reg), (mask))
-#define PX_CLR(port, reg, mask)   ahci_px_clr(ahci, (port), (reg), (mask))
-
 /*** Function Declarations ***/
 static QPCIDevice *get_ahci_device(uint32_t *fingerprint);
 static void start_ahci_device(AHCIQState *ahci);
@@ -228,20 +213,20 @@ static void ahci_hba_enable(AHCIQState *ahci)
     g_assert(ahci != NULL);
 
     /* Set GHC.AE to 1 */
-    AHCI_SET(AHCI_GHC, AHCI_GHC_AE);
-    reg = AHCI_RREG(AHCI_GHC);
+    ahci_set(ahci, AHCI_GHC, AHCI_GHC_AE);
+    reg = ahci_rreg(ahci, AHCI_GHC);
     ASSERT_BIT_SET(reg, AHCI_GHC_AE);
 
     /* Cache CAP and CAP2. */
-    ahci->cap = AHCI_RREG(AHCI_CAP);
-    ahci->cap2 = AHCI_RREG(AHCI_CAP2);
+    ahci->cap = ahci_rreg(ahci, AHCI_CAP);
+    ahci->cap2 = ahci_rreg(ahci, AHCI_CAP2);
 
     /* Read CAP.NCS, how many command slots do we have? */
     num_cmd_slots = ((ahci->cap & 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);
+    ports_impl = ahci_rreg(ahci, AHCI_PI);
 
     for (i = 0; ports_impl; ports_impl >>= 1, ++i) {
         if (!(ports_impl & 0x01)) {
@@ -250,16 +235,17 @@ static void ahci_hba_enable(AHCIQState *ahci)
 
         g_test_message("Initializing port %u", i);
 
-        reg = PX_RREG(i, AHCI_PX_CMD);
+        reg = ahci_px_rreg(ahci, 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));
+            ahci_px_clr(ahci, 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);
+            reg = ahci_px_rreg(ahci, 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");
@@ -271,55 +257,56 @@ static void ahci_hba_enable(AHCIQState *ahci)
         /* PxCLB space ... 0x20 per command, as in 4.2.2 p 36 */
         clb = ahci_alloc(ahci, 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));
+        ahci_px_wreg(ahci, i, AHCI_PX_CLB, clb);
+        g_assert_cmphex(clb, ==, ahci_px_rreg(ahci, i, AHCI_PX_CLB));
 
         /* PxFB space ... 0x100, as in 4.2.1 p 35 */
         fb = ahci_alloc(ahci, 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));
+        ahci_px_wreg(ahci, i, AHCI_PX_FB, fb);
+        g_assert_cmphex(fb, ==, ahci_px_rreg(ahci, 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));
+        ahci_px_wreg(ahci, i, AHCI_PX_SERR, 0xFFFFFFFF);
+        ahci_px_wreg(ahci, i, AHCI_PX_IS, 0xFFFFFFFF);
+        ahci_wreg(ahci, AHCI_IS, (1 << i));
 
         /* Verify Interrupts Cleared */
-        reg = PX_RREG(i, AHCI_PX_SERR);
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
         g_assert_cmphex(reg, ==, 0);
 
-        reg = PX_RREG(i, AHCI_PX_IS);
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
         g_assert_cmphex(reg, ==, 0);
 
-        reg = AHCI_RREG(AHCI_IS);
+        reg = ahci_rreg(ahci, 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);
+        ahci_px_wreg(ahci, i, AHCI_PX_IE, 0xFFFFFFFF);
+        reg = ahci_px_rreg(ahci, 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);
+        ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_FRE);
+        reg = ahci_px_rreg(ahci, 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);
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
         if (BITSET(reg, AHCI_PX_SERR_DIAG_X)) {
-            PX_SET(i, AHCI_PX_SERR, AHCI_PX_SERR_DIAG_X);
+            ahci_px_set(ahci, i, AHCI_PX_SERR, AHCI_PX_SERR_DIAG_X);
         }
 
-        reg = PX_RREG(i, AHCI_PX_TFD);
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_TFD);
         if (BITCLR(reg, AHCI_PX_TFD_STS_BSY | AHCI_PX_TFD_STS_DRQ)) {
-            reg = PX_RREG(i, AHCI_PX_SSTS);
+            reg = ahci_px_rreg(ahci, 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);
+                ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_ST);
+                ASSERT_BIT_SET(ahci_px_rreg(ahci, 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. */
@@ -329,8 +316,8 @@ static void ahci_hba_enable(AHCIQState *ahci)
     }
 
     /* Enable GHC.IE */
-    AHCI_SET(AHCI_GHC, AHCI_GHC_IE);
-    reg = AHCI_RREG(AHCI_GHC);
+    ahci_set(ahci, AHCI_GHC, AHCI_GHC_IE);
+    reg = ahci_rreg(ahci, AHCI_GHC);
     ASSERT_BIT_SET(reg, AHCI_GHC_IE);
 
     /* TODO: The device should now be idling and waiting for commands.
@@ -602,11 +589,11 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
      */
 
     /* 1 CAP - Capabilities Register */
-    ahci->cap = AHCI_RREG(AHCI_CAP);
+    ahci->cap = ahci_rreg(ahci, AHCI_CAP);
     ASSERT_BIT_CLEAR(ahci->cap, AHCI_CAP_RESERVED);
 
     /* 2 GHC - Global Host Control */
-    reg = AHCI_RREG(AHCI_GHC);
+    reg = ahci_rreg(ahci, AHCI_GHC);
     ASSERT_BIT_CLEAR(reg, AHCI_GHC_HR);
     ASSERT_BIT_CLEAR(reg, AHCI_GHC_IE);
     ASSERT_BIT_CLEAR(reg, AHCI_GHC_MRSM);
@@ -619,11 +606,11 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     }
 
     /* 3 IS - Interrupt Status */
-    reg = AHCI_RREG(AHCI_IS);
+    reg = ahci_rreg(ahci, AHCI_IS);
     g_assert_cmphex(reg, ==, 0);
 
     /* 4 PI - Ports Implemented */
-    ports = AHCI_RREG(AHCI_PI);
+    ports = ahci_rreg(ahci, AHCI_PI);
     /* Ports Implemented must be non-zero. */
     g_assert_cmphex(ports, !=, 0);
     /* Ports Implemented must be <= Number of Ports. */
@@ -639,7 +626,7 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     g_assert_cmphex((reg >> maxports), ==, 0);
 
     /* 5 AHCI Version */
-    reg = AHCI_RREG(AHCI_VS);
+    reg = ahci_rreg(ahci, AHCI_VS);
     switch (reg) {
     case AHCI_VERSION_0_95:
     case AHCI_VERSION_1_0:
@@ -652,7 +639,7 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     }
 
     /* 6 Command Completion Coalescing Control: depends on CAP.CCCS. */
-    reg = AHCI_RREG(AHCI_CCCCTL);
+    reg = ahci_rreg(ahci, AHCI_CCCCTL);
     if (BITSET(ahci->cap, AHCI_CAP_CCCS)) {
         ASSERT_BIT_CLEAR(reg, AHCI_CCCCTL_EN);
         ASSERT_BIT_CLEAR(reg, AHCI_CCCCTL_RESERVED);
@@ -663,18 +650,18 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     }
 
     /* 7 CCC_PORTS */
-    reg = AHCI_RREG(AHCI_CCCPORTS);
+    reg = ahci_rreg(ahci, AHCI_CCCPORTS);
     /* Must be zeroes initially regardless of CAP.CCCS */
     g_assert_cmphex(reg, ==, 0);
 
     /* 8 EM_LOC */
-    reg = AHCI_RREG(AHCI_EMLOC);
+    reg = ahci_rreg(ahci, AHCI_EMLOC);
     if (BITCLR(ahci->cap, AHCI_CAP_EMS)) {
         g_assert_cmphex(reg, ==, 0);
     }
 
     /* 9 EM_CTL */
-    reg = AHCI_RREG(AHCI_EMCTL);
+    reg = ahci_rreg(ahci, AHCI_EMCTL);
     if (BITSET(ahci->cap, AHCI_CAP_EMS)) {
         ASSERT_BIT_CLEAR(reg, AHCI_EMCTL_STSMR);
         ASSERT_BIT_CLEAR(reg, AHCI_EMCTL_CTLTM);
@@ -685,17 +672,17 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     }
 
     /* 10 CAP2 -- Capabilities Extended */
-    ahci->cap2 = AHCI_RREG(AHCI_CAP2);
+    ahci->cap2 = ahci_rreg(ahci, AHCI_CAP2);
     ASSERT_BIT_CLEAR(ahci->cap2, AHCI_CAP2_RESERVED);
 
     /* 11 BOHC -- Bios/OS Handoff Control */
-    reg = AHCI_RREG(AHCI_BOHC);
+    reg = ahci_rreg(ahci, 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);
+        reg = ahci_rreg(ahci, i);
         g_assert_cmphex(reg, ==, 0);
     }
 
@@ -703,7 +690,7 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     if (BITCLR(ahci->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);
+            reg = ahci_rreg(ahci, i);
             g_assert_cmphex(reg, ==, 0);
         }
     }
@@ -711,7 +698,7 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
     /* 40 -- 63: Vendor */
     g_test_message("Verifying HBA/Vendor area is empty.");
     for (i = AHCI_VENDOR; i < AHCI_PORTS; ++i) {
-        reg = AHCI_RREG(i);
+        reg = ahci_rreg(ahci, i);
         g_assert_cmphex(reg, ==, 0);
     }
 
@@ -728,7 +715,7 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
                            "(reg [%u-%u]) is empty.",
                            i, low, high - 1);
             for (j = low; j < high; ++j) {
-                reg = AHCI_RREG(j);
+                reg = ahci_rreg(ahci, j);
                 g_assert_cmphex(reg, ==, 0);
             }
         }
@@ -744,35 +731,35 @@ static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port)
     unsigned i;
 
     /* (0) CLB */
-    reg = PX_RREG(port, AHCI_PX_CLB);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_CLB);
     ASSERT_BIT_CLEAR(reg, AHCI_PX_CLB_RESERVED);
 
     /* (1) CLBU */
     if (BITCLR(ahci->cap, AHCI_CAP_S64A)) {
-        reg = PX_RREG(port, AHCI_PX_CLBU);
+        reg = ahci_px_rreg(ahci, port, AHCI_PX_CLBU);
         g_assert_cmphex(reg, ==, 0);
     }
 
     /* (2) FB */
-    reg = PX_RREG(port, AHCI_PX_FB);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_FB);
     ASSERT_BIT_CLEAR(reg, AHCI_PX_FB_RESERVED);
 
     /* (3) FBU */
     if (BITCLR(ahci->cap, AHCI_CAP_S64A)) {
-        reg = PX_RREG(port, AHCI_PX_FBU);
+        reg = ahci_px_rreg(ahci, port, AHCI_PX_FBU);
         g_assert_cmphex(reg, ==, 0);
     }
 
     /* (4) IS */
-    reg = PX_RREG(port, AHCI_PX_IS);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_IS);
     g_assert_cmphex(reg, ==, 0);
 
     /* (5) IE */
-    reg = PX_RREG(port, AHCI_PX_IE);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_IE);
     g_assert_cmphex(reg, ==, 0);
 
     /* (6) CMD */
-    reg = PX_RREG(port, AHCI_PX_CMD);
+    reg = ahci_px_rreg(ahci, 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);
@@ -810,11 +797,11 @@ static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port)
     }
 
     /* (7) RESERVED */
-    reg = PX_RREG(port, AHCI_PX_RES1);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_RES1);
     g_assert_cmphex(reg, ==, 0);
 
     /* (8) TFD */
-    reg = PX_RREG(port, AHCI_PX_TFD);
+    reg = ahci_px_rreg(ahci, 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);
@@ -832,33 +819,33 @@ static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port)
      * 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);
+    reg = ahci_px_rreg(ahci, 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);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_SCTL);
     g_assert_cmphex(reg, ==, 0);
 
     /* (12) SERR / SCR1: SError */
-    reg = PX_RREG(port, AHCI_PX_SERR);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_SERR);
     g_assert_cmphex(reg, ==, 0);
 
     /* (13) SACT / SCR3: SActive */
-    reg = PX_RREG(port, AHCI_PX_SACT);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_SACT);
     g_assert_cmphex(reg, ==, 0);
 
     /* (14) CI */
-    reg = PX_RREG(port, AHCI_PX_CI);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_CI);
     g_assert_cmphex(reg, ==, 0);
 
     /* (15) SNTF */
-    reg = PX_RREG(port, AHCI_PX_SNTF);
+    reg = ahci_px_rreg(ahci, port, AHCI_PX_SNTF);
     g_assert_cmphex(reg, ==, 0);
 
     /* (16) FBS */
-    reg = PX_RREG(port, AHCI_PX_FBS);
+    reg = ahci_px_rreg(ahci, 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);
@@ -872,13 +859,13 @@ static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port)
 
     /* [17 -- 27] RESERVED */
     for (i = AHCI_PX_RES2; i < AHCI_PX_VS; ++i) {
-        reg = PX_RREG(port, i);
+        reg = ahci_px_rreg(ahci, port, i);
         g_assert_cmphex(reg, ==, 0);
     }
 
     /* [28 -- 31] Vendor-Specific */
     for (i = AHCI_PX_VS; i < 32; ++i) {
-        reg = PX_RREG(port, i);
+        reg = ahci_px_rreg(ahci, port, i);
         if (reg) {
             g_test_message("INFO: Vendor register %u non-empty", i);
         }
@@ -918,7 +905,7 @@ static void ahci_test_identify(AHCIQState *ahci)
      */
 
     /* Pick the first implemented and running port */
-    ports = AHCI_RREG(AHCI_PI);
+    ports = ahci_rreg(ahci, AHCI_PI);
     for (i = 0; i < 32; ports >>= 1, ++i) {
         if (ports == 0) {
             i = 32;
@@ -928,7 +915,7 @@ static void ahci_test_identify(AHCIQState *ahci)
             continue;
         }
 
-        reg = PX_RREG(i, AHCI_PX_CMD);
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_CMD);
         if (BITSET(reg, AHCI_PX_CMD_ST)) {
             break;
         }
@@ -937,12 +924,12 @@ static void ahci_test_identify(AHCIQState *ahci)
     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);
+    reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
+    ahci_px_wreg(ahci, i, AHCI_PX_IS, reg);
+    g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
 
     /* Wipe the FIS-Receive Buffer */
-    fb = PX_RREG(i, AHCI_PX_FB);
+    fb = ahci_px_rreg(ahci, i, AHCI_PX_FB);
     g_assert_cmphex(fb, !=, 0);
     qmemset(fb, 0x00, 0x100);
 
@@ -957,7 +944,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     g_assert(data_ptr);
 
     /* Grab the Command List Buffer pointer */
-    clb = PX_RREG(i, AHCI_PX_CLB);
+    clb = ahci_px_rreg(ahci, i, AHCI_PX_CLB);
     g_assert(clb);
 
     /* Copy the existing Command #0 structure from the CLB into local memory,
@@ -985,7 +972,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     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);
+    g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
 
     /* Commit the Command FIS to the Command Table */
     memwrite(table, &fis, sizeof(fis));
@@ -997,29 +984,30 @@ static void ahci_test_identify(AHCIQState *ahci)
     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);
+    g_assert_cmphex(ahci_px_rreg(ahci, 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)) {
+    ahci_px_wreg(ahci, i, AHCI_PX_CI, (1 << 0));
+    while (BITSET(ahci_px_rreg(ahci, i, AHCI_PX_TFD), AHCI_PX_TFD_STS_BSY)) {
         usleep(50);
     }
 
     /* Check for expected interrupts */
-    reg = PX_RREG(i, AHCI_PX_IS);
+    reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
     ASSERT_BIT_SET(reg, AHCI_PX_IS_DHRS);
     ASSERT_BIT_SET(reg, AHCI_PX_IS_PSS);
     /* BUG: we expect AHCI_PX_IS_DPS to be set. */
     ASSERT_BIT_CLEAR(reg, AHCI_PX_IS_DPS);
 
     /* Clear expected interrupts and assert all interrupts now cleared. */
-    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);
+    ahci_px_wreg(ahci, i, AHCI_PX_IS,
+                 AHCI_PX_IS_DHRS | AHCI_PX_IS_PSS | AHCI_PX_IS_DPS);
+    g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
 
     /* Check for errors. */
-    reg = PX_RREG(i, AHCI_PX_SERR);
+    reg = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
     g_assert_cmphex(reg, ==, 0);
-    reg = PX_RREG(i, AHCI_PX_TFD);
+    reg = ahci_px_rreg(ahci, i, AHCI_PX_TFD);
     ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_ERR);
     ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_ERR);
 
@@ -1036,7 +1024,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     g_assert_cmphex(pio->status, ==, d2h->status);
     g_assert_cmphex(pio->error, ==, d2h->error);
 
-    reg = PX_RREG(i, AHCI_PX_TFD);
+    reg = ahci_px_rreg(ahci, i, AHCI_PX_TFD);
     g_assert_cmphex((reg & AHCI_PX_TFD_ERR), ==, pio->error);
     g_assert_cmphex((reg & AHCI_PX_TFD_STS), ==, pio->status);
     /* The PIO Setup FIS contains a "bytes read" field, which is a
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 14/15] qtest/ahci: Bookmark FB and CLB pointers
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (12 preceding siblings ...)
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 13/15] qtest/ahci: remove getter/setter macros John Snow
@ 2015-01-19 20:16 ` John Snow
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 15/15] libqos/ahci: create libqos/ahci.c John Snow
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-19 20:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

Instead of re-querying the AHCI device for the FB and CLB buffers, save
the pointer we gave to the device during initialization and reference
these values instead.

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

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index aa1f66f..b527e13 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -206,7 +206,7 @@ static void ahci_hba_enable(AHCIQState *ahci)
      * PxCMD.FR   "FIS Receive Running"
      * PxCMD.CR   "Command List Running"
      */
-    uint32_t reg, ports_impl, clb, fb;
+    uint32_t reg, ports_impl;
     uint16_t i;
     uint8_t num_cmd_slots;
 
@@ -255,16 +255,20 @@ static void ahci_hba_enable(AHCIQState *ahci)
 
         /* Allocate Memory for the Command List Buffer & FIS Buffer */
         /* PxCLB space ... 0x20 per command, as in 4.2.2 p 36 */
-        clb = ahci_alloc(ahci, num_cmd_slots * 0x20);
-        g_test_message("CLB: 0x%08x", clb);
-        ahci_px_wreg(ahci, i, AHCI_PX_CLB, clb);
-        g_assert_cmphex(clb, ==, ahci_px_rreg(ahci, i, AHCI_PX_CLB));
+        ahci->port[i].clb = ahci_alloc(ahci, num_cmd_slots * 0x20);
+        qmemset(ahci->port[i].clb, 0x00, 0x100);
+        g_test_message("CLB: 0x%08lx", ahci->port[i].clb);
+        ahci_px_wreg(ahci, i, AHCI_PX_CLB, ahci->port[i].clb);
+        g_assert_cmphex(ahci->port[i].clb, ==,
+                        ahci_px_rreg(ahci, i, AHCI_PX_CLB));
 
         /* PxFB space ... 0x100, as in 4.2.1 p 35 */
-        fb = ahci_alloc(ahci, 0x100);
-        g_test_message("FB: 0x%08x", fb);
-        ahci_px_wreg(ahci, i, AHCI_PX_FB, fb);
-        g_assert_cmphex(fb, ==, ahci_px_rreg(ahci, i, AHCI_PX_FB));
+        ahci->port[i].fb = ahci_alloc(ahci, 0x100);
+        qmemset(ahci->port[i].fb, 0x00, 0x100);
+        g_test_message("FB: 0x%08lx", ahci->port[i].fb);
+        ahci_px_wreg(ahci, i, AHCI_PX_FB, ahci->port[i].fb);
+        g_assert_cmphex(ahci->port[i].fb, ==,
+                        ahci_px_rreg(ahci, i, AHCI_PX_FB));
 
         /* Clear PxSERR, PxIS, then IS.IPS[x] by writing '1's. */
         ahci_px_wreg(ahci, i, AHCI_PX_SERR, 0xFFFFFFFF);
@@ -883,7 +887,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     RegH2DFIS fis;
     AHCICommand cmd;
     PRD prd;
-    uint32_t ports, reg, clb, table, fb, data_ptr;
+    uint32_t ports, reg, table, data_ptr;
     uint16_t buff[256];
     unsigned i;
     int rc;
@@ -929,9 +933,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
 
     /* Wipe the FIS-Receive Buffer */
-    fb = ahci_px_rreg(ahci, i, AHCI_PX_FB);
-    g_assert_cmphex(fb, !=, 0);
-    qmemset(fb, 0x00, 0x100);
+    qmemset(ahci->port[i].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.    */
@@ -943,13 +945,9 @@ static void ahci_test_identify(AHCIQState *ahci)
     data_ptr = ahci_alloc(ahci, 512);
     g_assert(data_ptr);
 
-    /* Grab the Command List Buffer pointer */
-    clb = ahci_px_rreg(ahci, 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));
+    memread(ahci->port[i].clb, &cmd, sizeof(cmd));
     cmd.b1 = 5;    /* reg_h2d_fis is 5 double-words long */
     cmd.b2 = 0x04; /* clear PxTFD.STS.BSY when done */
     cmd.prdtl = cpu_to_le16(1); /* One PRD table entry. */
@@ -981,7 +979,7 @@ static void ahci_test_identify(AHCIQState *ahci)
     memwrite(table + 0x80, &prd, sizeof(prd));
 
     /* Commit Command #0, pointing to the Table, to the Command List Buffer. */
-    memwrite(clb, &cmd, sizeof(cmd));
+    memwrite(ahci->port[i].clb, &cmd, sizeof(cmd));
 
     /* Everything is in place, but we haven't given the go-ahead yet. */
     g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_IS), ==, 0);
@@ -1012,12 +1010,12 @@ static void ahci_test_identify(AHCIQState *ahci)
     ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_ERR);
 
     /* Investigate CMD #0, assert that we read 512 bytes */
-    memread(clb, &cmd, sizeof(cmd));
+    memread(ahci->port[i].clb, &cmd, sizeof(cmd));
     g_assert_cmphex(512, ==, le32_to_cpu(cmd.prdbc));
 
     /* Investigate FIS responses */
-    memread(fb + 0x20, pio, 0x20);
-    memread(fb + 0x40, d2h, 0x20);
+    memread(ahci->port[i].fb + 0x20, pio, 0x20);
+    memread(ahci->port[i].fb + 0x40, d2h, 0x20);
     g_assert_cmphex(pio->fis_type, ==, 0x5f);
     g_assert_cmphex(d2h->fis_type, ==, 0x34);
     g_assert_cmphex(pio->flags, ==, d2h->flags);
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 645f05b..72c39bc 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -245,6 +245,11 @@
 
 /*** Structures ***/
 
+typedef struct AHCIPortQState {
+    uint64_t fb;
+    uint64_t clb;
+} AHCIPortQState;
+
 typedef struct AHCIQState {
     QOSState *parent;
     QPCIDevice *dev;
@@ -253,6 +258,7 @@ typedef struct AHCIQState {
     uint32_t fingerprint;
     uint32_t cap;
     uint32_t cap2;
+    AHCIPortQState port[32];
 } AHCIQState;
 
 /**
-- 
1.9.3

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

* [Qemu-devel] [PATCH v2 15/15] libqos/ahci: create libqos/ahci.c
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (13 preceding siblings ...)
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 14/15] qtest/ahci: Bookmark FB and CLB pointers John Snow
@ 2015-01-19 20:16 ` John Snow
  2015-01-28 10:26   ` Paolo Bonzini
  2015-01-27 21:05 ` [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
  2015-02-12 15:51 ` Stefan Hajnoczi
  16 siblings, 1 reply; 21+ messages in thread
From: John Snow @ 2015-01-19 20:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini,
	jsnow

With global state removed, code responsible for booting up,
verifying, and initializing the AHCI HBA is extracted and
inserted into libqos/ahci.c, which would allow for other
qtests in the future to quickly grab a meaningfully initialized
reference to an AHCI HBA.

Even without other users, functionalizing and isolating the code
assists future AHCI tests that exercise Q35 migration.

For now, libqos/ahci.o will be PC-only, but can be expanded into
something arch-agnostic in the future, if needed.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/Makefile      |   1 +
 tests/ahci-test.c   | 225 -------------------------------------------
 tests/libqos/ahci.c | 269 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/ahci.h |  11 ++-
 4 files changed, 280 insertions(+), 226 deletions(-)
 create mode 100644 tests/libqos/ahci.c

diff --git a/tests/Makefile b/tests/Makefile
index d8ef64f..0c056ec 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -302,6 +302,7 @@ libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
 libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
+libqos-pc-obj-y += tests/libqos/ahci.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
 libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o
 libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index b527e13..fca33d2 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -47,10 +47,6 @@ static char tmp_path[] = "/tmp/qtest.XXXXXX";
 static bool ahci_pedantic;
 
 /*** Function Declarations ***/
-static QPCIDevice *get_ahci_device(uint32_t *fingerprint);
-static void start_ahci_device(AHCIQState *ahci);
-static void free_ahci_device(QPCIDevice *dev);
-
 static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port);
 static void ahci_test_pci_spec(AHCIQState *ahci);
 static void ahci_test_pci_caps(AHCIQState *ahci, uint16_t header,
@@ -72,51 +68,6 @@ static void string_bswap16(uint16_t *s, size_t bytes)
     }
 }
 
-static uint64_t ahci_alloc(AHCIQState *ahci, size_t bytes)
-{
-    return qmalloc(ahci->parent, bytes);
-}
-
-/**
- * Locate, verify, and return a handle to the AHCI device.
- */
-static QPCIDevice *get_ahci_device(uint32_t *fingerprint)
-{
-    QPCIDevice *ahci;
-    uint32_t ahci_fingerprint;
-    QPCIBus *pcibus;
-
-    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);
-
-    ahci_fingerprint = qpci_config_readl(ahci, PCI_VENDOR_ID);
-
-    switch (ahci_fingerprint) {
-    case AHCI_INTEL_ICH9:
-        break;
-    default:
-        /* Unknown device. */
-        g_assert_not_reached();
-    }
-
-    if (fingerprint) {
-        *fingerprint = ahci_fingerprint;
-    }
-    return ahci;
-}
-
-static void free_ahci_device(QPCIDevice *dev)
-{
-    QPCIBus *pcibus = dev ? dev->bus : NULL;
-
-    /* libqos doesn't have a function for this, so free it manually */
-    g_free(dev);
-    qpci_free_pc(pcibus);
-}
-
 /*** Test Setup & Teardown ***/
 
 /**
@@ -153,182 +104,6 @@ static void ahci_shutdown(AHCIQState *ahci)
     qtest_shutdown(qs);
 }
 
-/*** Logical Device Initialization ***/
-
-/**
- * Start the PCI device and sanity-check default operation.
- */
-static void ahci_pci_enable(AHCIQState *ahci)
-{
-    uint8_t reg;
-
-    start_ahci_device(ahci);
-
-    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->dev, 0x92);
-        reg |= 0x3F;
-        qpci_config_writeb(ahci->dev, 0x92, reg);
-        /* 0...0111111b -- bit significant, ports 0-5 enabled. */
-        ASSERT_BIT_SET(qpci_config_readb(ahci->dev, 0x92), 0x3F);
-        break;
-    }
-
-}
-
-/**
- * Map BAR5/ABAR, and engage the PCI device.
- */
-static void start_ahci_device(AHCIQState *ahci)
-{
-    /* Map AHCI's ABAR (BAR5) */
-    ahci->hba_base = qpci_iomap(ahci->dev, 5, &ahci->barsize);
-
-    /* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */
-    qpci_device_enable(ahci->dev);
-}
-
-/**
- * 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(AHCIQState *ahci)
-{
-    /* 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"
-     */
-    uint32_t reg, ports_impl;
-    uint16_t i;
-    uint8_t num_cmd_slots;
-
-    g_assert(ahci != NULL);
-
-    /* Set GHC.AE to 1 */
-    ahci_set(ahci, AHCI_GHC, AHCI_GHC_AE);
-    reg = ahci_rreg(ahci, AHCI_GHC);
-    ASSERT_BIT_SET(reg, AHCI_GHC_AE);
-
-    /* Cache CAP and CAP2. */
-    ahci->cap = ahci_rreg(ahci, AHCI_CAP);
-    ahci->cap2 = ahci_rreg(ahci, AHCI_CAP2);
-
-    /* Read CAP.NCS, how many command slots do we have? */
-    num_cmd_slots = ((ahci->cap & 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, AHCI_PI);
-
-    for (i = 0; ports_impl; ports_impl >>= 1, ++i) {
-        if (!(ports_impl & 0x01)) {
-            continue;
-        }
-
-        g_test_message("Initializing port %u", i);
-
-        reg = ahci_px_rreg(ahci, 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");
-            ahci_px_clr(ahci, i, AHCI_PX_CMD,
-                        (AHCI_PX_CMD_ST | AHCI_PX_CMD_FRE));
-            /* The port has 500ms to disengage. */
-            usleep(500000);
-            reg = ahci_px_rreg(ahci, 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 */
-        ahci->port[i].clb = ahci_alloc(ahci, num_cmd_slots * 0x20);
-        qmemset(ahci->port[i].clb, 0x00, 0x100);
-        g_test_message("CLB: 0x%08lx", ahci->port[i].clb);
-        ahci_px_wreg(ahci, i, AHCI_PX_CLB, ahci->port[i].clb);
-        g_assert_cmphex(ahci->port[i].clb, ==,
-                        ahci_px_rreg(ahci, i, AHCI_PX_CLB));
-
-        /* PxFB space ... 0x100, as in 4.2.1 p 35 */
-        ahci->port[i].fb = ahci_alloc(ahci, 0x100);
-        qmemset(ahci->port[i].fb, 0x00, 0x100);
-        g_test_message("FB: 0x%08lx", ahci->port[i].fb);
-        ahci_px_wreg(ahci, i, AHCI_PX_FB, ahci->port[i].fb);
-        g_assert_cmphex(ahci->port[i].fb, ==,
-                        ahci_px_rreg(ahci, i, AHCI_PX_FB));
-
-        /* Clear PxSERR, PxIS, then IS.IPS[x] by writing '1's. */
-        ahci_px_wreg(ahci, i, AHCI_PX_SERR, 0xFFFFFFFF);
-        ahci_px_wreg(ahci, i, AHCI_PX_IS, 0xFFFFFFFF);
-        ahci_wreg(ahci, AHCI_IS, (1 << i));
-
-        /* Verify Interrupts Cleared */
-        reg = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
-        g_assert_cmphex(reg, ==, 0);
-
-        reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
-        g_assert_cmphex(reg, ==, 0);
-
-        reg = ahci_rreg(ahci, AHCI_IS);
-        ASSERT_BIT_CLEAR(reg, (1 << i));
-
-        /* Enable All Interrupts: */
-        ahci_px_wreg(ahci, i, AHCI_PX_IE, 0xFFFFFFFF);
-        reg = ahci_px_rreg(ahci, i, AHCI_PX_IE);
-        g_assert_cmphex(reg, ==, ~((uint32_t)AHCI_PX_IE_RESERVED));
-
-        /* Enable the FIS Receive Engine. */
-        ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_FRE);
-        reg = ahci_px_rreg(ahci, 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 = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
-        if (BITSET(reg, AHCI_PX_SERR_DIAG_X)) {
-            ahci_px_set(ahci, i, AHCI_PX_SERR, AHCI_PX_SERR_DIAG_X);
-        }
-
-        reg = ahci_px_rreg(ahci, i, AHCI_PX_TFD);
-        if (BITCLR(reg, AHCI_PX_TFD_STS_BSY | AHCI_PX_TFD_STS_DRQ)) {
-            reg = ahci_px_rreg(ahci, i, AHCI_PX_SSTS);
-            if ((reg & AHCI_PX_SSTS_DET) == SSTS_DET_ESTABLISHED) {
-                /* Device Found: set PxCMD.ST := 1 */
-                ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_ST);
-                ASSERT_BIT_SET(ahci_px_rreg(ahci, 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, AHCI_GHC, AHCI_GHC_IE);
-    reg = ahci_rreg(ahci, 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 ***/
 
 /**
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
new file mode 100644
index 0000000..3056fb1
--- /dev/null
+++ b/tests/libqos/ahci.c
@@ -0,0 +1,269 @@
+/*
+ * libqos AHCI functions
+ *
+ * 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 <glib.h>
+
+#include "libqtest.h"
+#include "libqos/ahci.h"
+#include "libqos/pci-pc.h"
+
+#include "qemu-common.h"
+#include "qemu/host-utils.h"
+
+#include "hw/pci/pci_ids.h"
+#include "hw/pci/pci_regs.h"
+
+/**
+ * Allocate space in the guest using information in the AHCIQState object.
+ */
+uint64_t ahci_alloc(AHCIQState *ahci, size_t bytes)
+{
+    g_assert(ahci);
+    g_assert(ahci->parent);
+    return qmalloc(ahci->parent, bytes);
+}
+
+void ahci_free(AHCIQState *ahci, uint64_t addr)
+{
+    g_assert(ahci);
+    g_assert(ahci->parent);
+    qfree(ahci->parent, addr);
+}
+
+/**
+ * Locate, verify, and return a handle to the AHCI device.
+ */
+QPCIDevice *get_ahci_device(uint32_t *fingerprint)
+{
+    QPCIDevice *ahci;
+    uint32_t ahci_fingerprint;
+    QPCIBus *pcibus;
+
+    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);
+
+    ahci_fingerprint = qpci_config_readl(ahci, PCI_VENDOR_ID);
+
+    switch (ahci_fingerprint) {
+    case AHCI_INTEL_ICH9:
+        break;
+    default:
+        /* Unknown device. */
+        g_assert_not_reached();
+    }
+
+    if (fingerprint) {
+        *fingerprint = ahci_fingerprint;
+    }
+    return ahci;
+}
+
+void free_ahci_device(QPCIDevice *dev)
+{
+    QPCIBus *pcibus = dev ? dev->bus : NULL;
+
+    /* libqos doesn't have a function for this, so free it manually */
+    g_free(dev);
+    qpci_free_pc(pcibus);
+}
+
+/*** Logical Device Initialization ***/
+
+/**
+ * Start the PCI device and sanity-check default operation.
+ */
+void ahci_pci_enable(AHCIQState *ahci)
+{
+    uint8_t reg;
+
+    start_ahci_device(ahci);
+
+    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->dev, 0x92);
+        reg |= 0x3F;
+        qpci_config_writeb(ahci->dev, 0x92, reg);
+        /* 0...0111111b -- bit significant, ports 0-5 enabled. */
+        ASSERT_BIT_SET(qpci_config_readb(ahci->dev, 0x92), 0x3F);
+        break;
+    }
+
+}
+
+/**
+ * Map BAR5/ABAR, and engage the PCI device.
+ */
+void start_ahci_device(AHCIQState *ahci)
+{
+    /* Map AHCI's ABAR (BAR5) */
+    ahci->hba_base = qpci_iomap(ahci->dev, 5, &ahci->barsize);
+    g_assert(ahci->hba_base);
+
+    /* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */
+    qpci_device_enable(ahci->dev);
+}
+
+/**
+ * Test and initialize the AHCI's HBA memory areas.
+ * Initialize and start any ports with devices attached.
+ * Bring the HBA into the idle state.
+ */
+void ahci_hba_enable(AHCIQState *ahci)
+{
+    /* 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"
+     */
+    uint32_t reg, ports_impl;
+    uint16_t i;
+    uint8_t num_cmd_slots;
+
+    g_assert(ahci != NULL);
+
+    /* Set GHC.AE to 1 */
+    ahci_set(ahci, AHCI_GHC, AHCI_GHC_AE);
+    reg = ahci_rreg(ahci, AHCI_GHC);
+    ASSERT_BIT_SET(reg, AHCI_GHC_AE);
+
+    /* Cache CAP and CAP2. */
+    ahci->cap = ahci_rreg(ahci, AHCI_CAP);
+    ahci->cap2 = ahci_rreg(ahci, AHCI_CAP2);
+
+    /* Read CAP.NCS, how many command slots do we have? */
+    num_cmd_slots = ((ahci->cap & 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, AHCI_PI);
+
+    for (i = 0; ports_impl; ports_impl >>= 1, ++i) {
+        if (!(ports_impl & 0x01)) {
+            continue;
+        }
+
+        g_test_message("Initializing port %u", i);
+
+        reg = ahci_px_rreg(ahci, 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");
+            ahci_px_clr(ahci, i, AHCI_PX_CMD,
+                        (AHCI_PX_CMD_ST | AHCI_PX_CMD_FRE));
+            /* The port has 500ms to disengage. */
+            usleep(500000);
+            reg = ahci_px_rreg(ahci, 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 */
+        ahci->port[i].clb = ahci_alloc(ahci, num_cmd_slots * 0x20);
+        qmemset(ahci->port[i].clb, 0x00, 0x100);
+        g_test_message("CLB: 0x%08lx", ahci->port[i].clb);
+        ahci_px_wreg(ahci, i, AHCI_PX_CLB, ahci->port[i].clb);
+        g_assert_cmphex(ahci->port[i].clb, ==,
+                        ahci_px_rreg(ahci, i, AHCI_PX_CLB));
+
+        /* PxFB space ... 0x100, as in 4.2.1 p 35 */
+        ahci->port[i].fb = ahci_alloc(ahci, 0x100);
+        qmemset(ahci->port[i].fb, 0x00, 0x100);
+        g_test_message("FB: 0x%08lx", ahci->port[i].fb);
+        ahci_px_wreg(ahci, i, AHCI_PX_FB, ahci->port[i].fb);
+        g_assert_cmphex(ahci->port[i].fb, ==,
+                        ahci_px_rreg(ahci, i, AHCI_PX_FB));
+
+        /* Clear PxSERR, PxIS, then IS.IPS[x] by writing '1's. */
+        ahci_px_wreg(ahci, i, AHCI_PX_SERR, 0xFFFFFFFF);
+        ahci_px_wreg(ahci, i, AHCI_PX_IS, 0xFFFFFFFF);
+        ahci_wreg(ahci, AHCI_IS, (1 << i));
+
+        /* Verify Interrupts Cleared */
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
+        g_assert_cmphex(reg, ==, 0);
+
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
+        g_assert_cmphex(reg, ==, 0);
+
+        reg = ahci_rreg(ahci, AHCI_IS);
+        ASSERT_BIT_CLEAR(reg, (1 << i));
+
+        /* Enable All Interrupts: */
+        ahci_px_wreg(ahci, i, AHCI_PX_IE, 0xFFFFFFFF);
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_IE);
+        g_assert_cmphex(reg, ==, ~((uint32_t)AHCI_PX_IE_RESERVED));
+
+        /* Enable the FIS Receive Engine. */
+        ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_FRE);
+        reg = ahci_px_rreg(ahci, 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 = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
+        if (BITSET(reg, AHCI_PX_SERR_DIAG_X)) {
+            ahci_px_set(ahci, i, AHCI_PX_SERR, AHCI_PX_SERR_DIAG_X);
+        }
+
+        reg = ahci_px_rreg(ahci, i, AHCI_PX_TFD);
+        if (BITCLR(reg, AHCI_PX_TFD_STS_BSY | AHCI_PX_TFD_STS_DRQ)) {
+            reg = ahci_px_rreg(ahci, i, AHCI_PX_SSTS);
+            if ((reg & AHCI_PX_SSTS_DET) == SSTS_DET_ESTABLISHED) {
+                /* Device Found: set PxCMD.ST := 1 */
+                ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_ST);
+                ASSERT_BIT_SET(ahci_px_rreg(ahci, 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, AHCI_GHC, AHCI_GHC_IE);
+    reg = ahci_rreg(ahci, 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. */
+}
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 72c39bc..77f2055 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -28,7 +28,7 @@
 #include <stdint.h>
 #include <stdlib.h>
 #include <stdbool.h>
-
+#include "libqos/libqos.h"
 #include "libqos/pci.h"
 #include "libqos/malloc-pc.h"
 
@@ -423,4 +423,13 @@ static inline void ahci_px_clr(AHCIQState *ahci, uint8_t port,
                  ahci_px_rreg(ahci, port, reg_num) & ~mask);
 }
 
+/*** Prototypes ***/
+uint64_t ahci_alloc(AHCIQState *ahci, size_t bytes);
+void ahci_free(AHCIQState *ahci, uint64_t addr);
+QPCIDevice *get_ahci_device(uint32_t *fingerprint);
+void free_ahci_device(QPCIDevice *dev);
+void ahci_pci_enable(AHCIQState *ahci);
+void start_ahci_device(AHCIQState *ahci);
+void ahci_hba_enable(AHCIQState *ahci);
+
 #endif
-- 
1.9.3

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

* Re: [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (14 preceding siblings ...)
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 15/15] libqos/ahci: create libqos/ahci.c John Snow
@ 2015-01-27 21:05 ` John Snow
  2015-02-12 15:51 ` Stefan Hajnoczi
  16 siblings, 0 replies; 21+ messages in thread
From: John Snow @ 2015-01-27 21:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, marc.mari.barcelo, armbru, mreitz, stefanha, pbonzini

Ping: Waiting for reviews on patches #6 (Marc?), and re-reviews for 
patches #12 and #15 (Paolo?)

Thanks
--JS

On 01/19/2015 03:15 PM, John Snow wrote:
> This series aims to do two main things:
>
> (1) Eliminate global state out of the ahci-test file so that
>      the tests are more functional. This will allow me to write
>      migration tests more easily. These tests are already written
>      and will be submitted upstream after these refactor series.
>
> (2) With global state removed from ahci-test, factor out functions
>      that would be useful to manipulate ahci devices into libqos/ahci.o,
>      to allow other testwriters to capitalize on the functional
>      refactoring.
>
> So, as an overview, we do a few things:
>
>   - Clean up the malloc interface to have a clear pc-specific interface
>     that uses an architecture independent core.
>
>   - Add some new structures to help keep track of AHCI and libqos state.
>
>   - Rewrite existing AHCI helpers and routines to use the new structures
>     and helper routines.
>
>   - Excise any commonly valuable code to libqos/ahci.h and libqos/ahci.c.
>
> This series therefore introduces no new functionality itself, but I was
> trying to keep the series small and reviewable. The real necessity here
> in jumbling malloc and ahci functions around is to create a functional
> interface that can be bundled into a single structure to help facilitate
> migration testing inside of qtests, for which I intend to export to all
> of qtests to be re-used for other tests.
>
>  From this point forward, the churn to my AHCI refactor series should be
> significantly reduced and more pleasant to read.
>
> Thanks,
> John.
>
> V2:
>   - #6  Made QGuestAllocator object opaque, in a new patch.
>     - Created page_size setter method.
>   - #7  Renamed "QOSOperations" to "QOSOps" and added Paolo's R-b.
>   - #12 Dropped GCC attributes from static inline helpers (ahci.h)
>   - #15 Adjusted assertion to not compare void ptr to int 0
>   - #15 Share ahci.o with libqos-obj-pc-y instead of only ahci-test
>     * ahci.o is pc-only for now.
>
> John Snow (15):
>    libqos: Split apart pc_alloc_init
>    qtest/ahci: Create ahci.h
>    libqos: create libqos.c
>    libqos: add qtest_vboot
>    libqos: add alloc_init_flags
>    libqos: Update QGuestAllocator to be opaque
>    libqos: add pc specific interface
>    qtest/ahci: Store hba_base in AHCIQState
>    qtest/ahci: finalize AHCIQState consolidation
>    qtest/ahci: remove pcibus global
>    qtest/ahci: remove guest_malloc global
>    libqos/ahci: Functional register helpers
>    qtest/ahci: remove getter/setter macros
>    qtest/ahci: Bookmark FB and CLB pointers
>    libqos/ahci: create libqos/ahci.c
>
>   tests/Makefile           |   5 +-
>   tests/ahci-test.c        | 894 ++++++++---------------------------------------
>   tests/libqos/ahci.c      | 269 ++++++++++++++
>   tests/libqos/ahci.h      | 435 +++++++++++++++++++++++
>   tests/libqos/libqos-pc.c |  24 ++
>   tests/libqos/libqos-pc.h |   9 +
>   tests/libqos/libqos.c    |  63 ++++
>   tests/libqos/libqos.h    |  33 ++
>   tests/libqos/malloc-pc.c |  20 +-
>   tests/libqos/malloc.c    |  86 ++++-
>   tests/libqos/malloc.h    |  25 +-
>   11 files changed, 1068 insertions(+), 795 deletions(-)
>   create mode 100644 tests/libqos/ahci.c
>   create mode 100644 tests/libqos/ahci.h
>   create mode 100644 tests/libqos/libqos-pc.c
>   create mode 100644 tests/libqos/libqos-pc.h
>   create mode 100644 tests/libqos/libqos.c
>   create mode 100644 tests/libqos/libqos.h
>

-- 
—js

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

* Re: [Qemu-devel] [PATCH v2 06/15] libqos: Update QGuestAllocator to be opaque
  2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 06/15] libqos: Update QGuestAllocator to be opaque John Snow
@ 2015-01-27 21:22   ` Marc Marí
  0 siblings, 0 replies; 21+ messages in thread
From: Marc Marí @ 2015-01-27 21:22 UTC (permalink / raw)
  To: John Snow; +Cc: kwolf, armbru, qemu-devel, mreitz, stefanha, pbonzini

El Mon, 19 Jan 2015 15:15:54 -0500
John Snow <jsnow@redhat.com> escribió:
> To avoid the architecture-specific implementations of the generic
> qtest allocator having to know about fields within the allocator, add
> a page_size setter method for users or arch specializations to use.
> The allocator will assume a default page_size for general use, but it
> can always be overridden.
> 
> Since this was the last instance of code directly using properties of
> the QGuestAllocator object directly, modify the type to be opaque and
> move the structure inside of malloc.c.
> 
> mlist_new, which was previously exported, is made static local to
> malloc.c, as it has no external users.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/libqos/malloc-pc.c |  2 +-
>  tests/libqos/malloc.c    | 61
> ++++++++++++++++++++++++++++++++++++------------
> tests/libqos/malloc.h    | 22 +++-------------- 3 files changed, 50
> insertions(+), 35 deletions(-)
> 
> diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
> index 6a5fdf3..6e253b6 100644
> --- a/tests/libqos/malloc-pc.c
> +++ b/tests/libqos/malloc-pc.c
> @@ -38,7 +38,7 @@ QGuestAllocator *pc_alloc_init_flags(QAllocOpts
> flags) 
>      ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
>      s = alloc_init_flags(flags, 1 << 20, MIN(ram_size, 0xE0000000));
> -    s->page_size = PAGE_SIZE;
> +    alloc_set_page_size(s, PAGE_SIZE);
>  
>      /* clean-up */
>      g_free(fw_cfg);
> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
> index 4ff260f..8cce1ba 100644
> --- a/tests/libqos/malloc.c
> +++ b/tests/libqos/malloc.c
> @@ -16,6 +16,26 @@
>  #include <inttypes.h>
>  #include <glib.h>
>  
> +typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
> +
> +typedef struct MemBlock {
> +    QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
> +    uint64_t size;
> +    uint64_t addr;
> +} MemBlock;
> +
> +typedef struct QGuestAllocator {
> +    QAllocOpts opts;
> +    uint64_t start;
> +    uint64_t end;
> +    uint32_t page_size;
> +
> +    MemList used;
> +    MemList free;
> +} QGuestAllocator;
> +
> +#define DEFAULT_PAGE_SIZE 4096
> +
>  static void mlist_delete(MemList *list, MemBlock *node)
>  {
>      g_assert(list && node);
> @@ -103,6 +123,21 @@ static void mlist_coalesce(MemList *head,
> MemBlock *node) } while (merge);
>  }
>  
> +static MemBlock *mlist_new(uint64_t addr, uint64_t size)
> +{
> +    MemBlock *block;
> +
> +    if (!size) {
> +        return NULL;
> +    }
> +    block = g_malloc0(sizeof(MemBlock));
> +
> +    block->addr = addr;
> +    block->size = size;
> +
> +    return block;
> +}
> +
>  static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode,
>                                                                  uint64_t
> size) {
> @@ -187,21 +222,6 @@ static void mlist_free(QGuestAllocator *s,
> uint64_t addr) mlist_coalesce(&s->free, node);
>  }
>  
> -MemBlock *mlist_new(uint64_t addr, uint64_t size)
> -{
> -    MemBlock *block;
> -
> -    if (!size) {
> -        return NULL;
> -    }
> -    block = g_malloc0(sizeof(MemBlock));
> -
> -    block->addr = addr;
> -    block->size = size;
> -
> -    return block;
> -}
> -
>  /*
>   * Mostly for valgrind happiness, but it does offer
>   * a chokepoint for debugging guest memory leaks, too.
> @@ -283,6 +303,8 @@ QGuestAllocator *alloc_init(uint64_t start,
> uint64_t end) node = mlist_new(s->start, s->end - s->start);
>      QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
>  
> +    s->page_size = DEFAULT_PAGE_SIZE;
> +
>      return s;
>  }
>  
> @@ -293,3 +315,12 @@ QGuestAllocator *alloc_init_flags(QAllocOpts
> opts, s->opts = opts;
>      return s;
>  }
> +
> +void alloc_set_page_size(QGuestAllocator *allocator, size_t
> page_size) +{
> +    /* Can't alter the page_size for an allocator in-use */
> +    g_assert(QTAILQ_EMPTY(&allocator->used));
> +
> +    g_assert(is_power_of_2(page_size));
> +    allocator->page_size = page_size;
> +}
> diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
> index 7b29547..a39dba4 100644
> --- a/tests/libqos/malloc.h
> +++ b/tests/libqos/malloc.h
> @@ -17,8 +17,6 @@
>  #include <sys/types.h>
>  #include "qemu/queue.h"
>  
> -#define MLIST_ENTNAME entries
> -
>  typedef enum {
>      ALLOC_NO_FLAGS    = 0x00,
>      ALLOC_LEAK_WARN   = 0x01,
> @@ -26,24 +24,8 @@ typedef enum {
>      ALLOC_PARANOID    = 0x04
>  } QAllocOpts;
>  
> -typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
> -typedef struct MemBlock {
> -    QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
> -    uint64_t size;
> -    uint64_t addr;
> -} MemBlock;
> +typedef struct QGuestAllocator QGuestAllocator;
>  
> -typedef struct QGuestAllocator {
> -    QAllocOpts opts;
> -    uint64_t start;
> -    uint64_t end;
> -    uint32_t page_size;
> -
> -    MemList used;
> -    MemList free;
> -} QGuestAllocator;
> -
> -MemBlock *mlist_new(uint64_t addr, uint64_t size);
>  void alloc_uninit(QGuestAllocator *allocator);
>  
>  /* Always returns page aligned values */
> @@ -53,4 +35,6 @@ void guest_free(QGuestAllocator *allocator,
> uint64_t addr); QGuestAllocator *alloc_init(uint64_t start, uint64_t
> end); QGuestAllocator *alloc_init_flags(QAllocOpts flags,
>                                    uint64_t start, uint64_t end);
> +void alloc_set_page_size(QGuestAllocator *allocator, size_t
> page_size); +
>  #endif

Reviewed-by: Marc Marí <marc.mari.barcelo@gmail.com>

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

* Re: [Qemu-devel] [PATCH v2 12/15] libqos/ahci: Functional register helpers
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 12/15] libqos/ahci: Functional register helpers John Snow
@ 2015-01-28 10:26   ` Paolo Bonzini
  0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-01-28 10:26 UTC (permalink / raw)
  To: John Snow, qemu-devel; +Cc: kwolf, marc.mari.barcelo, armbru, stefanha, mreitz



On 19/01/2015 21:16, John Snow wrote:
> Introduce a set of "static inline" register helpers that are intended to
> replace the current set of macros with more functional versions that are
> better suited to inclusion in libqos than porcelain macros.
> 
> As a stopgap measure before eliminating the porcelain macros, define them
> to use the new functions defined in the ahci.h header.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/ahci-test.c   | 25 ++++++++++-----------
>  tests/libqos/ahci.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index bb98968..25e54b8 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -47,22 +47,19 @@ static char tmp_path[] = "/tmp/qtest.XXXXXX";
>  static bool ahci_pedantic;
>  
>  /*** IO macros for the AHCI memory registers. ***/
> -#define AHCI_READ(OFST) qpci_io_readl(ahci->dev, ahci->hba_base + (OFST))
> -#define AHCI_WRITE(OFST, VAL) qpci_io_writel(ahci->dev,                 \
> -                                             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))
> +#define AHCI_READ(OFST)       ahci_mread(ahci, (OFST))
> +#define AHCI_WRITE(OFST, VAL) ahci_mwrite(ahci, (OFST), (VAL))
> +#define AHCI_RREG(regno)      ahci_rreg(ahci, (regno))
> +#define AHCI_WREG(regno, val) ahci_wreg(ahci, (regno), (val))
> +#define AHCI_SET(regno, mask) ahci_set(ahci, (regno), (mask))
> +#define AHCI_CLR(regno, mask) ahci_clr(ahci, (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));
> +#define PX_OFST(port, regno)      ahci_px_ofst((port), (regno))
> +#define PX_RREG(port, regno)      ahci_px_rreg(ahci, (port), (regno))
> +#define PX_WREG(port, regno, val) ahci_px_wreg(ahci, (port), (regno), (val))
> +#define PX_SET(port, reg, mask)   ahci_px_set(ahci, (port), (reg), (mask))
> +#define PX_CLR(port, reg, mask)   ahci_px_clr(ahci, (port), (reg), (mask))
>  
>  /*** Function Declarations ***/
>  static QPCIDevice *get_ahci_device(uint32_t *fingerprint);
> diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
> index 8e92385..645f05b 100644
> --- a/tests/libqos/ahci.h
> +++ b/tests/libqos/ahci.h
> @@ -354,4 +354,67 @@ typedef struct PRD {
>  /* For calculating how big the PRD table needs to be: */
>  #define CMD_TBL_SIZ(n) ((0x80 + ((n) * sizeof(PRD)) + 0x7F) & ~0x7F)
>  
> +/* Helpers for reading/writing AHCI HBA register values */
> +
> +static inline uint32_t ahci_mread(AHCIQState *ahci, size_t offset)
> +{
> +    return qpci_io_readl(ahci->dev, ahci->hba_base + offset);
> +}
> +
> +static inline void ahci_mwrite(AHCIQState *ahci, size_t offset, uint32_t value)
> +{
> +    qpci_io_writel(ahci->dev, ahci->hba_base + offset, value);
> +}
> +
> +static inline uint32_t ahci_rreg(AHCIQState *ahci, uint32_t reg_num)
> +{
> +    return ahci_mread(ahci, 4 * reg_num);
> +}
> +
> +static inline void ahci_wreg(AHCIQState *ahci, uint32_t reg_num, uint32_t value)
> +{
> +    ahci_mwrite(ahci, 4 * reg_num, value);
> +}
> +
> +static inline void ahci_set(AHCIQState *ahci, uint32_t reg_num, uint32_t mask)
> +{
> +    ahci_wreg(ahci, reg_num, ahci_rreg(ahci, reg_num) | mask);
> +}
> +
> +static inline void ahci_clr(AHCIQState *ahci, uint32_t reg_num, uint32_t mask)
> +{
> +    ahci_wreg(ahci, reg_num, ahci_rreg(ahci, reg_num) & ~mask);
> +}
> +
> +static inline size_t ahci_px_offset(uint8_t port, uint32_t reg_num)
> +{
> +    return AHCI_PORTS + (HBA_PORT_NUM_REG * port) + reg_num;
> +}
> +
> +static inline uint32_t ahci_px_rreg(AHCIQState *ahci, uint8_t port,
> +                                    uint32_t reg_num)
> +{
> +    return ahci_rreg(ahci, ahci_px_offset(port, reg_num));
> +}
> +
> +static inline void ahci_px_wreg(AHCIQState *ahci, uint8_t port,
> +                                uint32_t reg_num, uint32_t value)
> +{
> +    ahci_wreg(ahci, ahci_px_offset(port, reg_num), value);
> +}
> +
> +static inline void ahci_px_set(AHCIQState *ahci, uint8_t port,
> +                               uint32_t reg_num, uint32_t mask)
> +{
> +    ahci_px_wreg(ahci, port, reg_num,
> +                 ahci_px_rreg(ahci, port, reg_num) | mask);
> +}
> +
> +static inline void ahci_px_clr(AHCIQState *ahci, uint8_t port,
> +                               uint32_t reg_num, uint32_t mask)
> +{
> +    ahci_px_wreg(ahci, port, reg_num,
> +                 ahci_px_rreg(ahci, port, reg_num) & ~mask);
> +}
> +
>  #endif
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

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

* Re: [Qemu-devel] [PATCH v2 15/15] libqos/ahci: create libqos/ahci.c
  2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 15/15] libqos/ahci: create libqos/ahci.c John Snow
@ 2015-01-28 10:26   ` Paolo Bonzini
  0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-01-28 10:26 UTC (permalink / raw)
  To: John Snow, qemu-devel; +Cc: kwolf, marc.mari.barcelo, armbru, stefanha, mreitz



On 19/01/2015 21:16, John Snow wrote:
> With global state removed, code responsible for booting up,
> verifying, and initializing the AHCI HBA is extracted and
> inserted into libqos/ahci.c, which would allow for other
> qtests in the future to quickly grab a meaningfully initialized
> reference to an AHCI HBA.
> 
> Even without other users, functionalizing and isolating the code
> assists future AHCI tests that exercise Q35 migration.
> 
> For now, libqos/ahci.o will be PC-only, but can be expanded into
> something arch-agnostic in the future, if needed.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/Makefile      |   1 +
>  tests/ahci-test.c   | 225 -------------------------------------------
>  tests/libqos/ahci.c | 269 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/ahci.h |  11 ++-
>  4 files changed, 280 insertions(+), 226 deletions(-)
>  create mode 100644 tests/libqos/ahci.c
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index d8ef64f..0c056ec 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -302,6 +302,7 @@ libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
>  libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
>  libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
> +libqos-pc-obj-y += tests/libqos/ahci.o
>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>  libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o
>  libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index b527e13..fca33d2 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -47,10 +47,6 @@ static char tmp_path[] = "/tmp/qtest.XXXXXX";
>  static bool ahci_pedantic;
>  
>  /*** Function Declarations ***/
> -static QPCIDevice *get_ahci_device(uint32_t *fingerprint);
> -static void start_ahci_device(AHCIQState *ahci);
> -static void free_ahci_device(QPCIDevice *dev);
> -
>  static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port);
>  static void ahci_test_pci_spec(AHCIQState *ahci);
>  static void ahci_test_pci_caps(AHCIQState *ahci, uint16_t header,
> @@ -72,51 +68,6 @@ static void string_bswap16(uint16_t *s, size_t bytes)
>      }
>  }
>  
> -static uint64_t ahci_alloc(AHCIQState *ahci, size_t bytes)
> -{
> -    return qmalloc(ahci->parent, bytes);
> -}
> -
> -/**
> - * Locate, verify, and return a handle to the AHCI device.
> - */
> -static QPCIDevice *get_ahci_device(uint32_t *fingerprint)
> -{
> -    QPCIDevice *ahci;
> -    uint32_t ahci_fingerprint;
> -    QPCIBus *pcibus;
> -
> -    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);
> -
> -    ahci_fingerprint = qpci_config_readl(ahci, PCI_VENDOR_ID);
> -
> -    switch (ahci_fingerprint) {
> -    case AHCI_INTEL_ICH9:
> -        break;
> -    default:
> -        /* Unknown device. */
> -        g_assert_not_reached();
> -    }
> -
> -    if (fingerprint) {
> -        *fingerprint = ahci_fingerprint;
> -    }
> -    return ahci;
> -}
> -
> -static void free_ahci_device(QPCIDevice *dev)
> -{
> -    QPCIBus *pcibus = dev ? dev->bus : NULL;
> -
> -    /* libqos doesn't have a function for this, so free it manually */
> -    g_free(dev);
> -    qpci_free_pc(pcibus);
> -}
> -
>  /*** Test Setup & Teardown ***/
>  
>  /**
> @@ -153,182 +104,6 @@ static void ahci_shutdown(AHCIQState *ahci)
>      qtest_shutdown(qs);
>  }
>  
> -/*** Logical Device Initialization ***/
> -
> -/**
> - * Start the PCI device and sanity-check default operation.
> - */
> -static void ahci_pci_enable(AHCIQState *ahci)
> -{
> -    uint8_t reg;
> -
> -    start_ahci_device(ahci);
> -
> -    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->dev, 0x92);
> -        reg |= 0x3F;
> -        qpci_config_writeb(ahci->dev, 0x92, reg);
> -        /* 0...0111111b -- bit significant, ports 0-5 enabled. */
> -        ASSERT_BIT_SET(qpci_config_readb(ahci->dev, 0x92), 0x3F);
> -        break;
> -    }
> -
> -}
> -
> -/**
> - * Map BAR5/ABAR, and engage the PCI device.
> - */
> -static void start_ahci_device(AHCIQState *ahci)
> -{
> -    /* Map AHCI's ABAR (BAR5) */
> -    ahci->hba_base = qpci_iomap(ahci->dev, 5, &ahci->barsize);
> -
> -    /* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */
> -    qpci_device_enable(ahci->dev);
> -}
> -
> -/**
> - * 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(AHCIQState *ahci)
> -{
> -    /* 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"
> -     */
> -    uint32_t reg, ports_impl;
> -    uint16_t i;
> -    uint8_t num_cmd_slots;
> -
> -    g_assert(ahci != NULL);
> -
> -    /* Set GHC.AE to 1 */
> -    ahci_set(ahci, AHCI_GHC, AHCI_GHC_AE);
> -    reg = ahci_rreg(ahci, AHCI_GHC);
> -    ASSERT_BIT_SET(reg, AHCI_GHC_AE);
> -
> -    /* Cache CAP and CAP2. */
> -    ahci->cap = ahci_rreg(ahci, AHCI_CAP);
> -    ahci->cap2 = ahci_rreg(ahci, AHCI_CAP2);
> -
> -    /* Read CAP.NCS, how many command slots do we have? */
> -    num_cmd_slots = ((ahci->cap & 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, AHCI_PI);
> -
> -    for (i = 0; ports_impl; ports_impl >>= 1, ++i) {
> -        if (!(ports_impl & 0x01)) {
> -            continue;
> -        }
> -
> -        g_test_message("Initializing port %u", i);
> -
> -        reg = ahci_px_rreg(ahci, 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");
> -            ahci_px_clr(ahci, i, AHCI_PX_CMD,
> -                        (AHCI_PX_CMD_ST | AHCI_PX_CMD_FRE));
> -            /* The port has 500ms to disengage. */
> -            usleep(500000);
> -            reg = ahci_px_rreg(ahci, 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 */
> -        ahci->port[i].clb = ahci_alloc(ahci, num_cmd_slots * 0x20);
> -        qmemset(ahci->port[i].clb, 0x00, 0x100);
> -        g_test_message("CLB: 0x%08lx", ahci->port[i].clb);
> -        ahci_px_wreg(ahci, i, AHCI_PX_CLB, ahci->port[i].clb);
> -        g_assert_cmphex(ahci->port[i].clb, ==,
> -                        ahci_px_rreg(ahci, i, AHCI_PX_CLB));
> -
> -        /* PxFB space ... 0x100, as in 4.2.1 p 35 */
> -        ahci->port[i].fb = ahci_alloc(ahci, 0x100);
> -        qmemset(ahci->port[i].fb, 0x00, 0x100);
> -        g_test_message("FB: 0x%08lx", ahci->port[i].fb);
> -        ahci_px_wreg(ahci, i, AHCI_PX_FB, ahci->port[i].fb);
> -        g_assert_cmphex(ahci->port[i].fb, ==,
> -                        ahci_px_rreg(ahci, i, AHCI_PX_FB));
> -
> -        /* Clear PxSERR, PxIS, then IS.IPS[x] by writing '1's. */
> -        ahci_px_wreg(ahci, i, AHCI_PX_SERR, 0xFFFFFFFF);
> -        ahci_px_wreg(ahci, i, AHCI_PX_IS, 0xFFFFFFFF);
> -        ahci_wreg(ahci, AHCI_IS, (1 << i));
> -
> -        /* Verify Interrupts Cleared */
> -        reg = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
> -        g_assert_cmphex(reg, ==, 0);
> -
> -        reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
> -        g_assert_cmphex(reg, ==, 0);
> -
> -        reg = ahci_rreg(ahci, AHCI_IS);
> -        ASSERT_BIT_CLEAR(reg, (1 << i));
> -
> -        /* Enable All Interrupts: */
> -        ahci_px_wreg(ahci, i, AHCI_PX_IE, 0xFFFFFFFF);
> -        reg = ahci_px_rreg(ahci, i, AHCI_PX_IE);
> -        g_assert_cmphex(reg, ==, ~((uint32_t)AHCI_PX_IE_RESERVED));
> -
> -        /* Enable the FIS Receive Engine. */
> -        ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_FRE);
> -        reg = ahci_px_rreg(ahci, 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 = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
> -        if (BITSET(reg, AHCI_PX_SERR_DIAG_X)) {
> -            ahci_px_set(ahci, i, AHCI_PX_SERR, AHCI_PX_SERR_DIAG_X);
> -        }
> -
> -        reg = ahci_px_rreg(ahci, i, AHCI_PX_TFD);
> -        if (BITCLR(reg, AHCI_PX_TFD_STS_BSY | AHCI_PX_TFD_STS_DRQ)) {
> -            reg = ahci_px_rreg(ahci, i, AHCI_PX_SSTS);
> -            if ((reg & AHCI_PX_SSTS_DET) == SSTS_DET_ESTABLISHED) {
> -                /* Device Found: set PxCMD.ST := 1 */
> -                ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_ST);
> -                ASSERT_BIT_SET(ahci_px_rreg(ahci, 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, AHCI_GHC, AHCI_GHC_IE);
> -    reg = ahci_rreg(ahci, 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 ***/
>  
>  /**
> diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
> new file mode 100644
> index 0000000..3056fb1
> --- /dev/null
> +++ b/tests/libqos/ahci.c
> @@ -0,0 +1,269 @@
> +/*
> + * libqos AHCI functions
> + *
> + * 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 <glib.h>
> +
> +#include "libqtest.h"
> +#include "libqos/ahci.h"
> +#include "libqos/pci-pc.h"
> +
> +#include "qemu-common.h"
> +#include "qemu/host-utils.h"
> +
> +#include "hw/pci/pci_ids.h"
> +#include "hw/pci/pci_regs.h"
> +
> +/**
> + * Allocate space in the guest using information in the AHCIQState object.
> + */
> +uint64_t ahci_alloc(AHCIQState *ahci, size_t bytes)
> +{
> +    g_assert(ahci);
> +    g_assert(ahci->parent);
> +    return qmalloc(ahci->parent, bytes);
> +}
> +
> +void ahci_free(AHCIQState *ahci, uint64_t addr)
> +{
> +    g_assert(ahci);
> +    g_assert(ahci->parent);
> +    qfree(ahci->parent, addr);
> +}
> +
> +/**
> + * Locate, verify, and return a handle to the AHCI device.
> + */
> +QPCIDevice *get_ahci_device(uint32_t *fingerprint)
> +{
> +    QPCIDevice *ahci;
> +    uint32_t ahci_fingerprint;
> +    QPCIBus *pcibus;
> +
> +    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);
> +
> +    ahci_fingerprint = qpci_config_readl(ahci, PCI_VENDOR_ID);
> +
> +    switch (ahci_fingerprint) {
> +    case AHCI_INTEL_ICH9:
> +        break;
> +    default:
> +        /* Unknown device. */
> +        g_assert_not_reached();
> +    }
> +
> +    if (fingerprint) {
> +        *fingerprint = ahci_fingerprint;
> +    }
> +    return ahci;
> +}
> +
> +void free_ahci_device(QPCIDevice *dev)
> +{
> +    QPCIBus *pcibus = dev ? dev->bus : NULL;
> +
> +    /* libqos doesn't have a function for this, so free it manually */
> +    g_free(dev);
> +    qpci_free_pc(pcibus);
> +}
> +
> +/*** Logical Device Initialization ***/
> +
> +/**
> + * Start the PCI device and sanity-check default operation.
> + */
> +void ahci_pci_enable(AHCIQState *ahci)
> +{
> +    uint8_t reg;
> +
> +    start_ahci_device(ahci);
> +
> +    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->dev, 0x92);
> +        reg |= 0x3F;
> +        qpci_config_writeb(ahci->dev, 0x92, reg);
> +        /* 0...0111111b -- bit significant, ports 0-5 enabled. */
> +        ASSERT_BIT_SET(qpci_config_readb(ahci->dev, 0x92), 0x3F);
> +        break;
> +    }
> +
> +}
> +
> +/**
> + * Map BAR5/ABAR, and engage the PCI device.
> + */
> +void start_ahci_device(AHCIQState *ahci)
> +{
> +    /* Map AHCI's ABAR (BAR5) */
> +    ahci->hba_base = qpci_iomap(ahci->dev, 5, &ahci->barsize);
> +    g_assert(ahci->hba_base);
> +
> +    /* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */
> +    qpci_device_enable(ahci->dev);
> +}
> +
> +/**
> + * Test and initialize the AHCI's HBA memory areas.
> + * Initialize and start any ports with devices attached.
> + * Bring the HBA into the idle state.
> + */
> +void ahci_hba_enable(AHCIQState *ahci)
> +{
> +    /* 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"
> +     */
> +    uint32_t reg, ports_impl;
> +    uint16_t i;
> +    uint8_t num_cmd_slots;
> +
> +    g_assert(ahci != NULL);
> +
> +    /* Set GHC.AE to 1 */
> +    ahci_set(ahci, AHCI_GHC, AHCI_GHC_AE);
> +    reg = ahci_rreg(ahci, AHCI_GHC);
> +    ASSERT_BIT_SET(reg, AHCI_GHC_AE);
> +
> +    /* Cache CAP and CAP2. */
> +    ahci->cap = ahci_rreg(ahci, AHCI_CAP);
> +    ahci->cap2 = ahci_rreg(ahci, AHCI_CAP2);
> +
> +    /* Read CAP.NCS, how many command slots do we have? */
> +    num_cmd_slots = ((ahci->cap & 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, AHCI_PI);
> +
> +    for (i = 0; ports_impl; ports_impl >>= 1, ++i) {
> +        if (!(ports_impl & 0x01)) {
> +            continue;
> +        }
> +
> +        g_test_message("Initializing port %u", i);
> +
> +        reg = ahci_px_rreg(ahci, 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");
> +            ahci_px_clr(ahci, i, AHCI_PX_CMD,
> +                        (AHCI_PX_CMD_ST | AHCI_PX_CMD_FRE));
> +            /* The port has 500ms to disengage. */
> +            usleep(500000);
> +            reg = ahci_px_rreg(ahci, 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 */
> +        ahci->port[i].clb = ahci_alloc(ahci, num_cmd_slots * 0x20);
> +        qmemset(ahci->port[i].clb, 0x00, 0x100);
> +        g_test_message("CLB: 0x%08lx", ahci->port[i].clb);
> +        ahci_px_wreg(ahci, i, AHCI_PX_CLB, ahci->port[i].clb);
> +        g_assert_cmphex(ahci->port[i].clb, ==,
> +                        ahci_px_rreg(ahci, i, AHCI_PX_CLB));
> +
> +        /* PxFB space ... 0x100, as in 4.2.1 p 35 */
> +        ahci->port[i].fb = ahci_alloc(ahci, 0x100);
> +        qmemset(ahci->port[i].fb, 0x00, 0x100);
> +        g_test_message("FB: 0x%08lx", ahci->port[i].fb);
> +        ahci_px_wreg(ahci, i, AHCI_PX_FB, ahci->port[i].fb);
> +        g_assert_cmphex(ahci->port[i].fb, ==,
> +                        ahci_px_rreg(ahci, i, AHCI_PX_FB));
> +
> +        /* Clear PxSERR, PxIS, then IS.IPS[x] by writing '1's. */
> +        ahci_px_wreg(ahci, i, AHCI_PX_SERR, 0xFFFFFFFF);
> +        ahci_px_wreg(ahci, i, AHCI_PX_IS, 0xFFFFFFFF);
> +        ahci_wreg(ahci, AHCI_IS, (1 << i));
> +
> +        /* Verify Interrupts Cleared */
> +        reg = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
> +        g_assert_cmphex(reg, ==, 0);
> +
> +        reg = ahci_px_rreg(ahci, i, AHCI_PX_IS);
> +        g_assert_cmphex(reg, ==, 0);
> +
> +        reg = ahci_rreg(ahci, AHCI_IS);
> +        ASSERT_BIT_CLEAR(reg, (1 << i));
> +
> +        /* Enable All Interrupts: */
> +        ahci_px_wreg(ahci, i, AHCI_PX_IE, 0xFFFFFFFF);
> +        reg = ahci_px_rreg(ahci, i, AHCI_PX_IE);
> +        g_assert_cmphex(reg, ==, ~((uint32_t)AHCI_PX_IE_RESERVED));
> +
> +        /* Enable the FIS Receive Engine. */
> +        ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_FRE);
> +        reg = ahci_px_rreg(ahci, 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 = ahci_px_rreg(ahci, i, AHCI_PX_SERR);
> +        if (BITSET(reg, AHCI_PX_SERR_DIAG_X)) {
> +            ahci_px_set(ahci, i, AHCI_PX_SERR, AHCI_PX_SERR_DIAG_X);
> +        }
> +
> +        reg = ahci_px_rreg(ahci, i, AHCI_PX_TFD);
> +        if (BITCLR(reg, AHCI_PX_TFD_STS_BSY | AHCI_PX_TFD_STS_DRQ)) {
> +            reg = ahci_px_rreg(ahci, i, AHCI_PX_SSTS);
> +            if ((reg & AHCI_PX_SSTS_DET) == SSTS_DET_ESTABLISHED) {
> +                /* Device Found: set PxCMD.ST := 1 */
> +                ahci_px_set(ahci, i, AHCI_PX_CMD, AHCI_PX_CMD_ST);
> +                ASSERT_BIT_SET(ahci_px_rreg(ahci, 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, AHCI_GHC, AHCI_GHC_IE);
> +    reg = ahci_rreg(ahci, 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. */
> +}
> diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
> index 72c39bc..77f2055 100644
> --- a/tests/libqos/ahci.h
> +++ b/tests/libqos/ahci.h
> @@ -28,7 +28,7 @@
>  #include <stdint.h>
>  #include <stdlib.h>
>  #include <stdbool.h>
> -
> +#include "libqos/libqos.h"
>  #include "libqos/pci.h"
>  #include "libqos/malloc-pc.h"
>  
> @@ -423,4 +423,13 @@ static inline void ahci_px_clr(AHCIQState *ahci, uint8_t port,
>                   ahci_px_rreg(ahci, port, reg_num) & ~mask);
>  }
>  
> +/*** Prototypes ***/
> +uint64_t ahci_alloc(AHCIQState *ahci, size_t bytes);
> +void ahci_free(AHCIQState *ahci, uint64_t addr);
> +QPCIDevice *get_ahci_device(uint32_t *fingerprint);
> +void free_ahci_device(QPCIDevice *dev);
> +void ahci_pci_enable(AHCIQState *ahci);
> +void start_ahci_device(AHCIQState *ahci);
> +void ahci_hba_enable(AHCIQState *ahci);
> +
>  #endif
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

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

* Re: [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring
  2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
                   ` (15 preceding siblings ...)
  2015-01-27 21:05 ` [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
@ 2015-02-12 15:51 ` Stefan Hajnoczi
  16 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2015-02-12 15:51 UTC (permalink / raw)
  To: John Snow
  Cc: kwolf, marc.mari.barcelo, armbru, qemu-devel, mreitz, stefanha,
	pbonzini

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

On Mon, Jan 19, 2015 at 03:15:48PM -0500, John Snow wrote:
> This series aims to do two main things:
> 
> (1) Eliminate global state out of the ahci-test file so that
>     the tests are more functional. This will allow me to write
>     migration tests more easily. These tests are already written
>     and will be submitted upstream after these refactor series.
> 
> (2) With global state removed from ahci-test, factor out functions
>     that would be useful to manipulate ahci devices into libqos/ahci.o,
>     to allow other testwriters to capitalize on the functional
>     refactoring.
> 
> So, as an overview, we do a few things:
> 
>  - Clean up the malloc interface to have a clear pc-specific interface
>    that uses an architecture independent core.
> 
>  - Add some new structures to help keep track of AHCI and libqos state.
> 
>  - Rewrite existing AHCI helpers and routines to use the new structures
>    and helper routines.
> 
>  - Excise any commonly valuable code to libqos/ahci.h and libqos/ahci.c.
> 
> This series therefore introduces no new functionality itself, but I was
> trying to keep the series small and reviewable. The real necessity here
> in jumbling malloc and ahci functions around is to create a functional
> interface that can be bundled into a single structure to help facilitate
> migration testing inside of qtests, for which I intend to export to all
> of qtests to be re-used for other tests.
> 
> From this point forward, the churn to my AHCI refactor series should be
> significantly reduced and more pleasant to read.
> 
> Thanks,
> John.
> 
> V2:
>  - #6  Made QGuestAllocator object opaque, in a new patch.
>    - Created page_size setter method.
>  - #7  Renamed "QOSOperations" to "QOSOps" and added Paolo's R-b.
>  - #12 Dropped GCC attributes from static inline helpers (ahci.h)
>  - #15 Adjusted assertion to not compare void ptr to int 0
>  - #15 Share ahci.o with libqos-obj-pc-y instead of only ahci-test
>    * ahci.o is pc-only for now.
> 
> John Snow (15):
>   libqos: Split apart pc_alloc_init
>   qtest/ahci: Create ahci.h
>   libqos: create libqos.c
>   libqos: add qtest_vboot
>   libqos: add alloc_init_flags
>   libqos: Update QGuestAllocator to be opaque
>   libqos: add pc specific interface
>   qtest/ahci: Store hba_base in AHCIQState
>   qtest/ahci: finalize AHCIQState consolidation
>   qtest/ahci: remove pcibus global
>   qtest/ahci: remove guest_malloc global
>   libqos/ahci: Functional register helpers
>   qtest/ahci: remove getter/setter macros
>   qtest/ahci: Bookmark FB and CLB pointers
>   libqos/ahci: create libqos/ahci.c
> 
>  tests/Makefile           |   5 +-
>  tests/ahci-test.c        | 894 ++++++++---------------------------------------
>  tests/libqos/ahci.c      | 269 ++++++++++++++
>  tests/libqos/ahci.h      | 435 +++++++++++++++++++++++
>  tests/libqos/libqos-pc.c |  24 ++
>  tests/libqos/libqos-pc.h |   9 +
>  tests/libqos/libqos.c    |  63 ++++
>  tests/libqos/libqos.h    |  33 ++
>  tests/libqos/malloc-pc.c |  20 +-
>  tests/libqos/malloc.c    |  86 ++++-
>  tests/libqos/malloc.h    |  25 +-
>  11 files changed, 1068 insertions(+), 795 deletions(-)
>  create mode 100644 tests/libqos/ahci.c
>  create mode 100644 tests/libqos/ahci.h
>  create mode 100644 tests/libqos/libqos-pc.c
>  create mode 100644 tests/libqos/libqos-pc.h
>  create mode 100644 tests/libqos/libqos.c
>  create mode 100644 tests/libqos/libqos.h
> 
> -- 
> 1.9.3
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

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

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

end of thread, other threads:[~2015-02-12 15:51 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-19 20:15 [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 01/15] libqos: Split apart pc_alloc_init John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 02/15] qtest/ahci: Create ahci.h John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 03/15] libqos: create libqos.c John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 04/15] libqos: add qtest_vboot John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 05/15] libqos: add alloc_init_flags John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 06/15] libqos: Update QGuestAllocator to be opaque John Snow
2015-01-27 21:22   ` Marc Marí
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 07/15] libqos: add pc specific interface John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 08/15] qtest/ahci: Store hba_base in AHCIQState John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 09/15] qtest/ahci: finalize AHCIQState consolidation John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 10/15] qtest/ahci: remove pcibus global John Snow
2015-01-19 20:15 ` [Qemu-devel] [PATCH v2 11/15] qtest/ahci: remove guest_malloc global John Snow
2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 12/15] libqos/ahci: Functional register helpers John Snow
2015-01-28 10:26   ` Paolo Bonzini
2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 13/15] qtest/ahci: remove getter/setter macros John Snow
2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 14/15] qtest/ahci: Bookmark FB and CLB pointers John Snow
2015-01-19 20:16 ` [Qemu-devel] [PATCH v2 15/15] libqos/ahci: create libqos/ahci.c John Snow
2015-01-28 10:26   ` Paolo Bonzini
2015-01-27 21:05 ` [Qemu-devel] [PATCH v2 00/15] ahci-test preliminary refactoring John Snow
2015-02-12 15:51 ` Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).