[Qemu-devel] [PATCH 0/4] Re-enable and extend the m48t59 test

[Qemu-devel] [PATCH 0/4] Re-enable and extend the m48t59 test

[Thomas Huth]

This patch series fixes the currently disabled m48t59 test and
enables it for ppc, too.

Thomas Huth (4):
  tests/m48t59: Fix and re-enable the test for sparc
  tests/m48t59: Make the test independent of global_qtest
  tests/Makefile: Derive check-qtest-ppc64-y from check-qtest-ppc-y
  tests/m48t59: Add the m48t59 test on ppc, too

 tests/Makefile.include |  19 ++++----
 tests/m48t59-test.c    | 123 +++++++++++++++++++++++++++----------------------
 2 files changed, 76 insertions(+), 66 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/4] tests/m48t59: Fix and re-enable the test for sparc

[Thomas Huth]

The m48t59 test has been disabled in commit baeddded5fe6fa37d13fb94bf8d
("sparc: disable qtest in make check"), likely due to some timing issues
in the bcd_check_time tests which might fail if it gets interrupted for
too long. It should be OK to re-enable this test if we make sure that we
do not run it on timing-sensitive machines, thus it should be OK if we only
run it in the g_test_slow() mode.

Additionally, there are two other issues:

First, the test can not run so easily on sparc64 anymore, since commit
f3b18f35a23c60edbda6420cd ("sun4u: switch m48t59 NVRAM to MMIO access")
moved the m48t59 device to the ebus instead, and for this you first
have to set up the corresponding PCI device (which is currently not
possible from within the m48t59 test). So we can only re-enable this
test on sparc, but not the sparc64 target.

Second, the fuzzing test is executed before the bcd-check-time test
(due to the naming of the tests), without having the base address set
up properly, so the fuzzing test does not really check anything at all.
Fix it by setting up the base address from the main function already
and by moving the qtest_start() to the tests themselves, so that each
test starts with a clean environment (since after the fuzzing, the clock
is unusable for the bcd-check-time test).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include |  6 ++----
 tests/m48t59-test.c    | 58 ++++++++++++++++++++++++++++----------------------
 2 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 5d430e5..78bd163 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -350,13 +350,11 @@ check-qtest-sh4-y = tests/endianness-test$(EXESUF)
 check-qtest-sh4eb-y = tests/endianness-test$(EXESUF)
 
 check-qtest-sparc-y = tests/prom-env-test$(EXESUF)
-#check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
-#gcov-files-sparc-y = hw/timer/m48t59.c
+check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
+gcov-files-sparc-y = hw/timer/m48t59.c
 check-qtest-sparc-y += tests/boot-serial-test$(EXESUF)
 
 check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
-#check-qtest-sparc64-y += tests/m48t59-test$(EXESUF)
-#gcov-files-sparc64-y += hw/timer/m48t59.c
 check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
 check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
 
diff --git a/tests/m48t59-test.c b/tests/m48t59-test.c
index 0f921ef..a85f84d 100644
--- a/tests/m48t59-test.c
+++ b/tests/m48t59-test.c
@@ -143,11 +143,18 @@ static void cmos_get_date_time(struct tm *date)
     ts = mktime(date);
 }
 
-static void check_time(int wiggle)
+static QTestState *m48t59_qtest_start(void)
+{
+    return qtest_start("-rtc clock=vm");
+}
+
+static void bcd_check_time(void)
 {
     struct tm start, date[4], end;
     struct tm *datep;
     time_t ts;
+    const int wiggle = 2;
+    QTestState *s = m48t59_qtest_start();
 
     /*
      * This check assumes a few things.  First, we cannot guarantee that we get
@@ -198,30 +205,15 @@ static void check_time(int wiggle)
 
         g_assert_cmpint(ABS(t - s), <=, wiggle);
     }
-}
-
-static int wiggle = 2;
 
-static void bcd_check_time(void)
-{
-    if (strcmp(qtest_get_arch(), "sparc64") == 0) {
-        base = 0x74;
-        base_year = 1900;
-        use_mmio = false;
-    } else if (strcmp(qtest_get_arch(), "sparc") == 0) {
-        base = 0x71200000;
-        base_year = 1968;
-        use_mmio = true;
-    } else { /* PPC: need to map macio in PCI */
-        g_assert_not_reached();
-    }
-    check_time(wiggle);
+    qtest_quit(s);
 }
 
 /* success if no crash or abort */
 static void fuzz_registers(void)
 {
     unsigned int i;
+    QTestState *s = m48t59_qtest_start();
 
     for (i = 0; i < 1000; i++) {
         uint8_t reg, val;
@@ -237,24 +229,38 @@ static void fuzz_registers(void)
         cmos_write(reg, val);
         cmos_read(reg);
     }
+
+    qtest_quit(s);
+}
+
+static void base_setup(void)
+{
+    const char *arch = qtest_get_arch();
+
+    if (g_str_equal(arch, "sparc")) {
+        /* Note: For sparc64, we'd need to map-in the PCI bridge memory first */
+        base = 0x71200000;
+        base_year = 1968;
+        use_mmio = true;
+    } else {
+        g_assert_not_reached();
+    }
 }
 
 int main(int argc, char **argv)
 {
-    QTestState *s = NULL;
     int ret;
 
-    g_test_init(&argc, &argv, NULL);
+    base_setup();
 
-    s = qtest_start("-rtc clock=vm");
+    g_test_init(&argc, &argv, NULL);
 
-    qtest_add_func("/rtc/bcd/check-time", bcd_check_time);
+    if (g_test_slow()) {
+        /* Do not run this in timing-sensitive environments */
+        qtest_add_func("/rtc/bcd-check-time", bcd_check_time);
+    }
     qtest_add_func("/rtc/fuzz-registers", fuzz_registers);
     ret = g_test_run();
 
-    if (s) {
-        qtest_quit(s);
-    }
-
     return ret;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/4] tests/m48t59: Make the test independent of global_qtest

[Thomas Huth]

Stop using the functions that require global_qtest here and pass
around the QTestState instead (global_qtest should finally get
removed since this causes problems with tests running in parallel).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/m48t59-test.c | 60 ++++++++++++++++++++++++++---------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/tests/m48t59-test.c b/tests/m48t59-test.c
index a85f84d..8c25467 100644
--- a/tests/m48t59-test.c
+++ b/tests/m48t59-test.c
@@ -30,45 +30,45 @@ static uint16_t reg_base = 0x1ff0; /* 0x7f0 for m48t02 */
 static int base_year;
 static bool use_mmio;
 
-static uint8_t cmos_read_mmio(uint8_t reg)
+static uint8_t cmos_read_mmio(QTestState *s, uint8_t reg)
 {
-    return readb(base + (uint32_t)reg_base + (uint32_t)reg);
+    return qtest_readb(s, base + (uint32_t)reg_base + (uint32_t)reg);
 }
 
-static void cmos_write_mmio(uint8_t reg, uint8_t val)
+static void cmos_write_mmio(QTestState *s, uint8_t reg, uint8_t val)
 {
     uint8_t data = val;
 
-    writeb(base + (uint32_t)reg_base + (uint32_t)reg, data);
+    qtest_writeb(s, base + (uint32_t)reg_base + (uint32_t)reg, data);
 }
 
-static uint8_t cmos_read_ioio(uint8_t reg)
+static uint8_t cmos_read_ioio(QTestState *s, uint8_t reg)
 {
-    outw(base + 0, reg_base + (uint16_t)reg);
-    return inb(base + 3);
+    qtest_outw(s, base + 0, reg_base + (uint16_t)reg);
+    return qtest_inb(s, base + 3);
 }
 
-static void cmos_write_ioio(uint8_t reg, uint8_t val)
+static void cmos_write_ioio(QTestState *s, uint8_t reg, uint8_t val)
 {
-    outw(base + 0, reg_base + (uint16_t)reg);
-    outb(base + 3, val);
+    qtest_outw(s, base + 0, reg_base + (uint16_t)reg);
+    qtest_outb(s, base + 3, val);
 }
 
-static uint8_t cmos_read(uint8_t reg)
+static uint8_t cmos_read(QTestState *s, uint8_t reg)
 {
     if (use_mmio) {
-        return cmos_read_mmio(reg);
+        return cmos_read_mmio(s, reg);
     } else {
-        return cmos_read_ioio(reg);
+        return cmos_read_ioio(s, reg);
     }
 }
 
-static void cmos_write(uint8_t reg, uint8_t val)
+static void cmos_write(QTestState *s, uint8_t reg, uint8_t val)
 {
     if (use_mmio) {
-        cmos_write_mmio(reg, val);
+        cmos_write_mmio(s, reg, val);
     } else {
-        cmos_write_ioio(reg, val);
+        cmos_write_ioio(s, reg, val);
     }
 }
 
@@ -106,18 +106,18 @@ static void print_tm(struct tm *tm)
 }
 #endif
 
-static void cmos_get_date_time(struct tm *date)
+static void cmos_get_date_time(QTestState *s, struct tm *date)
 {
     int sec, min, hour, mday, mon, year;
     time_t ts;
     struct tm dummy;
 
-    sec = cmos_read(RTC_SECONDS);
-    min = cmos_read(RTC_MINUTES);
-    hour = cmos_read(RTC_HOURS);
-    mday = cmos_read(RTC_DAY_OF_MONTH);
-    mon = cmos_read(RTC_MONTH);
-    year = cmos_read(RTC_YEAR);
+    sec = cmos_read(s, RTC_SECONDS);
+    min = cmos_read(s, RTC_MINUTES);
+    hour = cmos_read(s, RTC_HOURS);
+    mday = cmos_read(s, RTC_DAY_OF_MONTH);
+    mon = cmos_read(s, RTC_MONTH);
+    year = cmos_read(s, RTC_YEAR);
 
     sec = bcd2dec(sec);
     min = bcd2dec(min);
@@ -145,7 +145,7 @@ static void cmos_get_date_time(struct tm *date)
 
 static QTestState *m48t59_qtest_start(void)
 {
-    return qtest_start("-rtc clock=vm");
+    return qtest_init("-rtc clock=vm");
 }
 
 static void bcd_check_time(void)
@@ -172,10 +172,10 @@ static void bcd_check_time(void)
     ts = time(NULL);
     gmtime_r(&ts, &start);
 
-    cmos_get_date_time(&date[0]);
-    cmos_get_date_time(&date[1]);
-    cmos_get_date_time(&date[2]);
-    cmos_get_date_time(&date[3]);
+    cmos_get_date_time(s, &date[0]);
+    cmos_get_date_time(s, &date[1]);
+    cmos_get_date_time(s, &date[2]);
+    cmos_get_date_time(s, &date[3]);
 
     ts = time(NULL);
     gmtime_r(&ts, &end);
@@ -226,8 +226,8 @@ static void fuzz_registers(void)
             continue;
         }
 
-        cmos_write(reg, val);
-        cmos_read(reg);
+        cmos_write(s, reg, val);
+        cmos_read(s, reg);
     }
 
     qtest_quit(s);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/4] tests/m48t59: Make the test independent of global_qtest

[Eric Blake]

On 02/12/2018 06:44 AM, Thomas Huth wrote:
> Stop using the functions that require global_qtest here and pass
> around the QTestState instead (global_qtest should finally get
> removed since this causes problems with tests running in parallel).
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/m48t59-test.c | 60 ++++++++++++++++++++++++++---------------------------
>   1 file changed, 30 insertions(+), 30 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH 3/4] tests/Makefile: Derive check-qtest-ppc64-y from check-qtest-ppc-y

[Thomas Huth]

ppc64 is a superset of ppc, so the ppc64 tests should include all
the ppc tests.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 78bd163..3ac874c 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -319,15 +319,12 @@ check-qtest-ppc-y += tests/prom-env-test$(EXESUF)
 check-qtest-ppc-y += tests/drive_del-test$(EXESUF)
 check-qtest-ppc-y += tests/boot-serial-test$(EXESUF)
 
-check-qtest-ppc64-y = tests/spapr-phb-test$(EXESUF)
-gcov-files-ppc64-y = ppc64-softmmu/hw/ppc/spapr_pci.c
-check-qtest-ppc64-y += tests/endianness-test$(EXESUF)
-check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
-check-qtest-ppc64-y += tests/prom-env-test$(EXESUF)
+check-qtest-ppc64-y = $(check-qtest-ppc-y)
+gcov-files-ppc64-y = $(subst ppc-softmmu/,ppc64-softmmu/,$(gcov-files-ppc-y))
+check-qtest-ppc64-y += tests/spapr-phb-test$(EXESUF)
+gcov-files-ppc64-y += ppc64-softmmu/hw/ppc/spapr_pci.c
 check-qtest-ppc64-y += tests/pnv-xscom-test$(EXESUF)
-check-qtest-ppc64-y += tests/drive_del-test$(EXESUF)
 check-qtest-ppc64-y += tests/migration-test$(EXESUF)
-check-qtest-ppc64-y += tests/boot-serial-test$(EXESUF)
 check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
 check-qtest-ppc64-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF)
 check-qtest-ppc64-y += tests/usb-hcd-ohci-test$(EXESUF)
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/4] tests/m48t59: Add the m48t59 test on ppc, too

[Thomas Huth]

The ref405ep machine has a memory-mapped m48t59 device, so
we can run the m48t59 test on this machine, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include | 2 ++
 tests/m48t59-test.c    | 9 ++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3ac874c..d46807a 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -318,6 +318,8 @@ check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc-y += tests/prom-env-test$(EXESUF)
 check-qtest-ppc-y += tests/drive_del-test$(EXESUF)
 check-qtest-ppc-y += tests/boot-serial-test$(EXESUF)
+check-qtest-ppc-y += tests/m48t59-test$(EXESUF)
+gcov-files-ppc-y += hw/timer/m48t59.c
 
 check-qtest-ppc64-y = $(check-qtest-ppc-y)
 gcov-files-ppc64-y = $(subst ppc-softmmu/,ppc64-softmmu/,$(gcov-files-ppc-y))
diff --git a/tests/m48t59-test.c b/tests/m48t59-test.c
index 8c25467..26af7d6 100644
--- a/tests/m48t59-test.c
+++ b/tests/m48t59-test.c
@@ -28,6 +28,7 @@
 static uint32_t base;
 static uint16_t reg_base = 0x1ff0; /* 0x7f0 for m48t02 */
 static int base_year;
+static const char *base_machine;
 static bool use_mmio;
 
 static uint8_t cmos_read_mmio(QTestState *s, uint8_t reg)
@@ -145,7 +146,7 @@ static void cmos_get_date_time(QTestState *s, struct tm *date)
 
 static QTestState *m48t59_qtest_start(void)
 {
-    return qtest_init("-rtc clock=vm");
+    return qtest_startf("-M %s -rtc clock=vm", base_machine);
 }
 
 static void bcd_check_time(void)
@@ -241,6 +242,12 @@ static void base_setup(void)
         /* Note: For sparc64, we'd need to map-in the PCI bridge memory first */
         base = 0x71200000;
         base_year = 1968;
+        base_machine = "SS-5";
+        use_mmio = true;
+    } else if (g_str_equal(arch, "ppc") || g_str_equal(arch, "ppc64")) {
+        base = 0xF0000000;
+        base_year = 1968;
+        base_machine = "ref405ep";
         use_mmio = true;
     } else {
         g_assert_not_reached();
-- 
1.8.3.1