Re: [PATCH v2 10/10] hvm/hpet: handle 1st period special

[Don Slutz <dslutz@verizon.com>]

On 04/14/14 11:27, Jan Beulich wrote:
>>>> On 08.04.14 at 16:24, <dslutz@verizon.com> wrote:
>> The software-developers-hpet-spec-1-0a.pdf says that the 1st
>> interrupt is based on the setting of comparator.  After that it will
>> be on each period.  Add code that checks for this special case.
> The way hpet_set_timer() call create_periodic_time() to me seems to
> do exactly that. What am I missing?

Since hpet_get_comparator() will always adjust
the current value of comparator  to the current value
of the master clock, the call to create_periodic_time()
which uses diff which is:


     tn_cmp   = hpet_get_comparator(h, tn);
     cur_tick = hpet_read_maincounter(h);
     ...
     diff = tn_cmp - cur_tick;



So using the manual test code and asking for 1 and 3 starting
periods:

dcs-xen-54:~/xen/tools/tests/vhpet>./test_vhpet 0 1 0 3 3 >foo
hpet: T0 Error: Set 0x1fcbd(130,237) != 0x1e848(125,000)
...

Which is not the default mode, and the important arguments are:

hvm_clock_cost=0 hpet_mult=1 hpet_add=0

(I.E. the 1st 3)

create_periodic_time: pt=0 delta=916,208 period=1,000,000 - 1,000.00 Hz irq=0


and

dcs-xen-54:~/xen/tools/tests/vhpet>./test_vhpet 0 3 0 3 3 >foo
hpet: T0 Error: Set 0x3e505(255,237) != 0x1e848(125,000)
...
(hvm_clock_cost=0 hpet_mult=3 hpet_add=0)


create_periodic_time: pt=0 delta=916,208 period=1,000,000 - 1,000.00 Hz irq=0


Notice that the cmp jumps back to the same value.  The master clock is not at
the period (it is 0x10899(67,737)) and so cmp is adjusted "backwards" in this case.

This output is with no changes.  It is not until patch #6:
hvm/hpet: comparator can only change when master clock is enabled.

That the unit test stops printing the error.   However the hpet_save output still
shows the changed value.

It is not till after this patch that:

dcs-xen-54:~/xen/tools/tests/vhpet>./test_vhpet 0 1 0 3 0x13 >foo1
dcs-xen-54:~/xen/tools/tests/vhpet>./test_vhpet 0 3 0 3 0x13 >foo3
dcs-xen-54:~/xen>diff -suwp tools/tests/vhpet/foo1 tools/tests/vhpet/foo3 | head -30
--- tools/tests/vhpet/foo1      2014-04-14 20:17:51.923684610 -0400
+++ tools/tests/vhpet/foo3      2014-04-14 20:19:02.148251889 -0400
@@ -1,5 +1,5 @@
  test_vhpet 1.0
-hvm_clock_cost=0 hpet_mult=1 hpet_add=0 tick_count=3 debug=0x13
+hvm_clock_cost=0 hpet_mult=3 hpet_add=0 tick_count=3 debug=0x13

  skip_load=1
      HPET: capability 0xf424008086a201 config 0()
@@ -56,19 +56,19 @@ skip_load
            timer1 period 0(0) fsb 0
            timer2 config 0xf0000000000030() cmp 0xffffffffffffffff(-1)
            timer2 period 0(0) fsb 0
-hpet_writel: HPET_TN_SETVAL cmp=0x1fccc(130,252) timer=0
+hpet_writel: HPET_TN_SETVAL cmp=0x3e514(255,252) timer=0
  hpet_writel: period=0xf424(62,500) timer=0
  create_periodic_time: mc64=0x108a8(67,752) mc_offset=0x108a7(67,751)
-                 [0] cmp64=0x1fccc(130,252) cmp=0x1fccc(130,252)
+                 [0] cmp64=0x3e514(255,252) cmp=0x3e514(255,252)
                   [1] cmp64=0xffffffffffffffff(-1) cmp=0xffffffffffffffff(-1)
                   [2] cmp64=0xffffffffffffffff(-1) cmp=0xffffffffffffffff(-1)
-create_periodic_time: pt=0 delta=1,000,000 period=1,000,000 - 1,000.00 Hz irq=0
+create_periodic_time: pt=0 delta=3,000,000 period=1,000,000 - 1,000.00 Hz irq=0
  hpet: hpet_set_mode(402):
  hpet: ID: 0x8086a201, PERIOD: 0xf42400
  hpet: CFG: 0x1, STATUS: 0x0
  hpet: COUNTER_l: 0x108a8, COUNTER_h: 0x0
  hpet: T0: CFG_l: 0x13c, CFG_h: 0xf00000
-hpet: T0: CMP_l: 0x1fccc, CMP_h: 0x0
+hpet: T0: CMP_l: 0x3e514, CMP_h: 0x0


that the 1st delta is actually the request one in both cases.


    -Don Slutz


> Jan
>