Skip to content

Commit

Permalink
tests: Don't assume structure of PCI IO base in ahci-test
Browse files Browse the repository at this point in the history
In a couple of places ahci-test makes assumptions about how the tokens
returned from qpci_iomap() are formatted in ways it probably shouldn't.

First in verify_state() it uses a non-NULL token to indicate that the AHCI
device has been enabled (part of enabling is to iomap()).  This changes it
to use an explicit 'enabled' flag instead.

Second, it uses the fact that the token contains a PCI address, stored when
the BAR is mapped during initialization to check that the BAR has the same
value after a migration.  This changes it to explicitly read the BAR
register before and after the migration and compare.

Together, these changes will  make the test more robust against changes to
the internals of the libqos PCI layer.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
  • Loading branch information
dgibson committed Oct 27, 2016
1 parent 204e54b commit e7c8526
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 6 deletions.
13 changes: 7 additions & 6 deletions tests/ahci-test.c
Expand Up @@ -78,25 +78,23 @@ static void string_bswap16(uint16_t *s, size_t bytes)
/**
* Verify that the transfer did not corrupt our state at all.
*/
static void verify_state(AHCIQState *ahci)
static void verify_state(AHCIQState *ahci, uint64_t hba_old)
{
int i, j;
uint32_t ahci_fingerprint;
uint64_t hba_base;
uint64_t hba_stored;
AHCICommandHeader cmd;

ahci_fingerprint = qpci_config_readl(ahci->dev, PCI_VENDOR_ID);
g_assert_cmphex(ahci_fingerprint, ==, ahci->fingerprint);

/* If we haven't initialized, this is as much as can be validated. */
if (!ahci->hba_base) {
if (!ahci->enabled) {
return;
}

hba_base = (uint64_t)qpci_config_readl(ahci->dev, PCI_BASE_ADDRESS_5);
hba_stored = (uint64_t)(uintptr_t)ahci->hba_base;
g_assert_cmphex(hba_base, ==, hba_stored);
g_assert_cmphex(hba_base, ==, hba_old);

g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP), ==, ahci->cap);
g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP2), ==, ahci->cap2);
Expand All @@ -119,12 +117,15 @@ static void ahci_migrate(AHCIQState *from, AHCIQState *to, const char *uri)
QOSState *tmp = to->parent;
QPCIDevice *dev = to->dev;
char *uri_local = NULL;
uint64_t hba_old;

if (uri == NULL) {
uri_local = g_strdup_printf("%s%s", "unix:", mig_socket);
uri = uri_local;
}

hba_old = (uint64_t)qpci_config_readl(from->dev, PCI_BASE_ADDRESS_5);

/* context will be 'to' after completion. */
migrate(from->parent, to->parent, uri);

Expand All @@ -141,7 +142,7 @@ static void ahci_migrate(AHCIQState *from, AHCIQState *to, const char *uri)
from->parent = tmp;
from->dev = dev;

verify_state(to);
verify_state(to, hba_old);
g_free(uri_local);
}

Expand Down
1 change: 1 addition & 0 deletions tests/libqos/ahci.c
Expand Up @@ -351,6 +351,7 @@ void ahci_hba_enable(AHCIQState *ahci)
reg = ahci_rreg(ahci, AHCI_GHC);
ASSERT_BIT_SET(reg, AHCI_GHC_IE);

ahci->enabled = true;
/* 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. */
Expand Down
1 change: 1 addition & 0 deletions tests/libqos/ahci.h
Expand Up @@ -327,6 +327,7 @@ typedef struct AHCIQState {
uint32_t cap;
uint32_t cap2;
AHCIPortQState port[32];
bool enabled;
} AHCIQState;

/**
Expand Down

0 comments on commit e7c8526

Please sign in to comment.