Re: [Xen-devel] [PATCH 2/2] Add vlapic timer checks

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/03/17 14:38, Anthony PERARD wrote:

Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
---
 tests/vlapic-timer/Makefile |   9 +++
 tests/vlapic-timer/main.c   | 131 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 140 insertions(+)
 create mode 100644 tests/vlapic-timer/Makefile
 create mode 100644 tests/vlapic-timer/main.c

diff --git a/tests/vlapic-timer/Makefile b/tests/vlapic-timer/Makefile
new file mode 100644
index 0000000..01fa4ea
--- /dev/null
+++ b/tests/vlapic-timer/Makefile
@@ -0,0 +1,9 @@
+include $(ROOT)/build/common.mk
+
+NAME      := vlapic-timer
+CATEGORY  := functional
+TEST-ENVS := $(HVM_ENVIRONMENTS)

Do you really need all HVM environments?  APIC emulation doesn't have any interaction with paging or operating modes.

Speaking of this test, I should really see about upstreaming some of my ad-hoc lapic tests, which have existed almost as long as XTF has.  I am also going to have to see about getting the test revision logic working, because I'd prefer to have one "apic functionaltiy test" than a large number of individual tests each testing a different part of behaviour.

+
+obj-perenv += main.o
+
+include $(ROOT)/build/gen.mk
diff --git a/tests/vlapic-timer/main.c b/tests/vlapic-timer/main.c
new file mode 100644
index 0000000..b081661
--- /dev/null
+++ b/tests/vlapic-timer/main.c
@@ -0,0 +1,131 @@
+/**
+ * @file tests/vlapic-timer/main.c
+ * @ref test-vlapic-timer - LAPIC Timer Emulation
+ *
+ * @page test-vlapic-timer LAPIC Timer Emulation
+ *
+ * Tests the behavior of the vlapic timer emulation by Xen.
+ *
+ * This tests should work on baremetal.
+ *
+ * It is testing switch between different mode, one-shot and periodic.
+ *
+ * @see tests/vlapic-timer/main.c
+ */
+#include <stdbool.h>

No need for this include.  It will come in via xtf.h

+#include <xtf.h>
+#include <arch/apicdef.h>
+
+const char test_title[] = "Test vlapic-timer";
+
+static inline void apic_write(unsigned long reg, uint32_t v)
+{
+    *((volatile uint32_t *)(APIC_BASE + reg)) = v;
+}
+
+static inline uint32_t apic_read(unsigned long reg)
+{
+    return *((volatile uint32_t *)(APIC_BASE + reg));
+}

These should be in an piece of common apic library, along with an initialisation function to probe and enable the lapic.  (after all, restricted PVH environments one have one at all).

+
+static inline void change_mode(unsigned long new_mode)
+{
+    uint32_t lvtt;
+
+    lvtt = apic_read(APIC_LVTT);
+    apic_write(APIC_LVTT, (lvtt & ~APIC_TIMER_MODE_MASK) | new_mode);
+}
+
+void wait_until_tmcct_is_zero(uint32_t initial_count, bool stop_when_half)
+{
+    uint32_t tmcct = apic_read(APIC_TMCCT);
+
+    if ( tmcct )
+    {
+        while ( tmcct > (initial_count / 2) )
+            tmcct = apic_read(APIC_TMCCT);
+
+        if ( stop_when_half )
+            return;
+
+        /* Wait until the counter reach 0 or wrap-around */
+        while ( tmcct <= (initial_count / 2) && tmcct > 0 )
+            tmcct = apic_read(APIC_TMCCT);
+    }
+}
+
+void test_main(void)
+{
+    uint32_t tmict = 0x999999;
+
+    apic_write(APIC_TMICT, tmict);
+    /*
+     * Assuming that the initial mode is one-shot, change it to periodic. TMICT
+     * should not be reset.

It is not generally a good idea make assumptions like this.  The only way it would be safe, is to state that one-shot is the reset value of the APIC state.

+     */
+    change_mode(APIC_TIMER_MODE_PERIODIC);
+    if ( apic_read(APIC_TMICT) != tmict )
+        xtf_failure("Fail: TMICT value reset\n");
+
+    /* Testing one-shot */
+    printk("Testing one-shot mode\n");
+    change_mode(APIC_TIMER_MODE_ONESHOT);
+    apic_write(APIC_TMICT, tmict);
+    if ( !apic_read(APIC_TMCCT) )
+        xtf_failure("Fail: TMCCT should have a non-zero value\n");
+    wait_until_tmcct_is_zero(tmict, false);
+    if ( apic_read(APIC_TMCCT) )
+        xtf_failure("Fail: TMCCT should have reached 0\n");

Please spread this logic out with newlines, to make it easier to read.

~Andrew

+
+    /*
+     * Write TMICT before changing mode from one-shot to periodic TMCCT should
+     * be reset to TMICT periodicly
+     */
+    apic_write(APIC_TMICT, tmict);
+    wait_until_tmcct_is_zero(tmict, true);
+    printk("Testing periodic mode\n");
+    change_mode(APIC_TIMER_MODE_PERIODIC);
+    if ( !apic_read(APIC_TMCCT) )
+        xtf_failure("Fail: TMCCT should have a non-zero value\n");
+    /*
+     * After the change of mode, the counter should not be reset and continue
+     * counting down from where it was
+     */
+    if ( apic_read(APIC_TMCCT) > (tmict / 2) )
+        xtf_failure("Fail: TMCCT should not be reset to TMICT value\n");
+    wait_until_tmcct_is_zero(tmict, false);
+    if ( apic_read(APIC_TMCCT) < (tmict / 2) )
+        xtf_failure("Fail: TMCCT should be reset to the initial-count\n");
+
+    wait_until_tmcct_is_zero(tmict, true);
+    /*
+     * Keep the same TMICT and change timer mode to one-shot
+     * TMCCT should be > 0 and count-down to 0
+     */
+    printk("Testing one-shot after periodic (no tmict reset)\n");
+    change_mode(APIC_TIMER_MODE_ONESHOT);
+    if ( !apic_read(APIC_TMCCT) )
+        xtf_failure("Fail: TMCCT should have a non-zero value\n");
+    if ( apic_read(APIC_TMCCT) > (tmict / 2) )
+        xtf_failure("Fail: TMCCT should not be reset to init\n");
+    wait_until_tmcct_is_zero(tmict, false);
+    if ( apic_read(APIC_TMCCT) )
+        xtf_failure("Fail: TMCCT should have reach zero\n");
+
+    /* now tmcct == 0 and tmict != 0 */
+    change_mode(APIC_TIMER_MODE_PERIODIC);
+    if ( apic_read(APIC_TMCCT) )
+        xtf_failure("Fail: TMCCT should stay at zero\n");
+
+    xtf_success(NULL);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel