[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 04/14/14 11:07, Jan Beulich wrote:
On 08.04.14 at 16:24, <dslutz@xxxxxxxxxxx> wrote:
This is based on software-developers-hpet-spec-1-0a.pdf saying:

When the main counter value matches the value in the timer's
comparator register, an interrupt can be generated.  The hardware
will then automatically increase the value in the compare register
by the last value written to that register.

And since interrupt can only be generated when the master clock
is enabled.
This last sentence looks truncated.

It is. I think I had:

And since interrupt can only be generated when the master clock
is enabled, the compare register should also change only then.

Which looks too complex to me now.

From the spec:


ENABLE_CNF

Overall Enable:

This bit must be set to enable any of the timers to generate interrupts. If
this bit is 0, then the main counter will halt (will not increment) and no 
interrupts will be
caused by any of these timers.

• 0 – Halt main count and disable all timer interrupts
• 1 – allow main counter to run, and allow timer interrupts if enabled




I think a better statement would be:

When the overall enable is off (the main count is halted), none of
the compare registers should change.


So I will change this in the commit message.

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).

-Don Slutz


Jan



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.