Re: [Qemu-devel] [PATCH] qtest: add rtc periodic timer test

[Paolo Bonzini]

On 26/05/2017 05:21, Xiao Guangrong wrote:
> On 05/26/2017 12:03 AM, Paolo Bonzini wrote:
>> On 25/05/2017 05:19, address@hidden wrote:
>> I'm not sure I understand.  Why would clock_step(1000) not be a good
>> replacement for nsleep(1000)?
> 
> We can not. As we use the real time to compare with the time that is
> passed in the VM, however, clock_step() is not a real time based clock
> source which immediately injects a time step to the VM regardless how
> much real time elapsed.

The trick lies in not using the real time for comparison, but the number
of time steps that were injected (or directly nanoseconds), like this:

--- a/tests/rtc-periodic-test.c
+++ b/tests/rtc-periodic-test.c
@@ -21,29 +21,29 @@
 
 #include "rtc-test.h"
 
-#define RTC_PERIOD_CODE1    13
-#define RTC_PERIOD_CODE2    15
+#define RTC_PERIOD_CODE1    13 /* 8 Hz */
+#define RTC_PERIOD_CODE2    15 /* 2 Hz */
 
 #define RTC_PERIOD_TEST_NR  50
 
-static void nsleep(int64_t nsecs)
-{
-    const struct timespec val = { .tv_nsec = nsecs };
-    nanosleep(&val, NULL);
-}
-
-static void wait_periodic_interrupt(void)
+static uint64_t wait_periodic_interrupt(void)
 {
-    while (1) {
-        if (get_irq(RTC_ISA_IRQ)) {
-            break;
-        }
-
-        /* 1 us.*/
-        nsleep(1000);
+    uint64_t real_time = 0;
+    while (!get_irq(RTC_ISA_IRQ)) {
+        /*
+        * about 2 ms.  It's more than enough given the period
+        * that we set above.
+        */
+       clock_step(NANOSECONDS_PER_SECOND / 512);
+       real_time += NANOSECONDS_PER_SECOND / 512;
     }
 
     g_assert((cmos_read(RTC_REG_C) & REG_C_PF) != 0);
+    return real_time;
 }
 
 static void periodic_timer(void)
@@ -58,17 +58,15 @@
     /* enable periodic interrupt after properly configure the period. */
     cmos_write(RTC_REG_B, cmos_read(RTC_REG_B) | REG_B_PIE);
 
-    real_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+    real_time = 0;
 
     for (i = 0; i < RTC_PERIOD_TEST_NR; i++) {
         cmos_write(RTC_REG_A, RTC_PERIOD_CODE1);
-        wait_periodic_interrupt();
+        real_time += wait_periodic_interrupt();
         cmos_write(RTC_REG_A, RTC_PERIOD_CODE2);
-        wait_periodic_interrupt();
+        real_time += wait_periodic_interrupt();
     }
 
-    real_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - real_time;
-
     period_clocks = periodic_period_to_clock(RTC_PERIOD_CODE1) +
                        periodic_period_to_clock(RTC_PERIOD_CODE2);
     period_clocks *= RTC_PERIOD_TEST_NR;
@@ -85,7 +83,7 @@
 
     g_test_init(&argc, &argv, NULL);
 
-    s = qtest_start("-rtc clock=host");
+    s = qtest_start("-rtc clock=vm");
     qtest_irq_intercept_in(s, "ioapic");
 
     qtest_add_func("/rtc/periodic/interrupt", periodic_timer);


Even better, the vm_clock time can be returned directly from
clock_step(), and you can even speed up the test by using
clock_step_next():

--- a/tests/rtc-periodic-test.c
+++ b/tests/rtc-periodic-test.c
@@ -21,29 +21,19 @@
 
 #include "rtc-test.h"
 
-#define RTC_PERIOD_CODE1    13
-#define RTC_PERIOD_CODE2    15
+#define RTC_PERIOD_CODE1    13 /* 8 Hz */
+#define RTC_PERIOD_CODE2    15 /* 2 Hz */
 
 #define RTC_PERIOD_TEST_NR  50
 
-static void nsleep(int64_t nsecs)
+static uint64_t wait_periodic_interrupt(uint64_t real_time)
 {
-    const struct timespec val = { .tv_nsec = nsecs };
-    nanosleep(&val, NULL);
-}
-
-static void wait_periodic_interrupt(void)
-{
-    while (1) {
-        if (get_irq(RTC_ISA_IRQ)) {
-            break;
-        }
-
-        /* 1 us.*/
-        nsleep(1000);
+    while (!get_irq(RTC_ISA_IRQ)) {
+       real_time = clock_step_next();
     }
 
     g_assert((cmos_read(RTC_REG_C) & REG_C_PF) != 0);
+    return real_time;
 }
 
 static void periodic_timer(void)
@@ -51,6 +41,8 @@
     int i;
     int64_t period_clocks, period_time, real_time;
 
+    real_time = clock_step(0);
+
     /* disable all interrupts. */
     cmos_write(RTC_REG_B, cmos_read(RTC_REG_B) &
                                    ~(REG_B_PIE | REG_B_AIE | REG_B_UIE));
@@ -58,17 +50,13 @@
     /* enable periodic interrupt after properly configure the period. */
     cmos_write(RTC_REG_B, cmos_read(RTC_REG_B) | REG_B_PIE);
 
-    real_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-
     for (i = 0; i < RTC_PERIOD_TEST_NR; i++) {
         cmos_write(RTC_REG_A, RTC_PERIOD_CODE1);
-        wait_periodic_interrupt();
+        real_time = wait_periodic_interrupt(real_time);
         cmos_write(RTC_REG_A, RTC_PERIOD_CODE2);
-        wait_periodic_interrupt();
+        real_time = wait_periodic_interrupt(real_time);
     }
 
-    real_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - real_time;
-
     period_clocks = periodic_period_to_clock(RTC_PERIOD_CODE1) +
                        periodic_period_to_clock(RTC_PERIOD_CODE2);
     period_clocks *= RTC_PERIOD_TEST_NR;
@@ -85,7 +73,7 @@
 
     g_test_init(&argc, &argv, NULL);
 
-    s = qtest_start("-rtc clock=host");
+    s = qtest_start("-rtc clock=vm");
     qtest_irq_intercept_in(s, "ioapic");
 
     qtest_add_func("/rtc/periodic/interrupt", periodic_timer);
@@ -98,57 +86,3 @@
 
     return ret;
 }


Can you send v2 that integrates this in rtc-test.c?

Thanks,

Paolo