Re: [PATCH v2 06/10] hvm/hpet: comparator can only change when master clock is enabled.

["Jan Beulich" <JBeulich@suse.com>]

>>> On 15.04.14 at 17:53, <dslutz@verizon.com> wrote:
> On 04/15/14 03:05, Jan Beulich wrote:
>>>>> On 14.04.14 at 21:50, <dslutz@verizon.com> wrote:
>>> On 04/14/14 11:07, Jan Beulich wrote:
>>>> As to the change - I'm not sure: The quoted description from the
>>>> specification cal also be read to mean that interrupt generation is
>>>> optional, but comparator increment will always happen. As long as
>>>> this can't be clarified, I'd prefer to stay with the code as is.
>>> I think the code needs to change to match the spec.
>>>
>>> #define timer_enabled(h, n) (timer_config(h, n) & HPET_TN_ENABLE)
>>>
>>> vs
>>>
>>> #define hpet_enabled(h) (h->hpet.config & HPET_CFG_ENABLE)
>>>
>>>
>>> The change uses hpet_enabled() (I.E. Overall Enable).
>> Correct. But do we really need this? When the HPET is globally
>> disabled, hpet_read_maincounter() returns a fixed value, and
>> hence - due to the comparators not changing either -
>> hpet_get_comparator() will too even without the addition. So if
>> at all, the change would be mostly for documentation purposes.
> 
> We do need this.  hpet_get_comparator() does not return a
> fixed value.  Without this change it will always adjust to
> hpet_read_maincounter().

But as said, hpet_read_maincounter() returns a fixed value in that
case too (or at least it is supposed to be).

And no, I'm not fully trusting the test program, so please explain
things relative to the source code rather than by pointing at the
test program showing something.

Jan