Re: [Xen-devel] [PATCH v3 11/11] hvm/hpet: handle 1st period special

[Tim Deegan <tim@xxxxxxx>]

At 13:32 +0100 on 25 Apr (1398429157), Jan Beulich wrote:
> >>> On 17.04.14 at 19:43, <dslutz@xxxxxxxxxxx> wrote:
> > v3:
> >   Better commit message.
> >   More setting of first_mc64 & first_enabled when needed.
> >   Switch to bool_t.
> > 
> >  xen/arch/x86/hvm/hpet.c       | 83 
> > ++++++++++++++++++++++++++++++++++++-------
> >  xen/include/asm-x86/hvm/vpt.h |  2 ++
> >  2 files changed, 73 insertions(+), 12 deletions(-)
> 
> I still can't help myself thinking that this must be achievable with less
> special casing - I just can't imagine that hardware also has this huge
> an amount of special case logic just for the first period.

+1.  I didn't follow the explanation on v1 very clearly, so perhaps
you can correct my understanding, but I _think_ the issue you're
describing is:

 when we read the comparator, we try to wind it forward from its
 current value towards the main counter value, by adding as many
 periods as will fit.

 If the guest sets the comparator >1 period away, we'll incorrectly
 warp the comparator to some foolish value as soon as we read it.

So, you're adding mechanism to detect the first interrupt after a
comparator set (or when the main counter is being started, or on hpet
load -- both of those rely on 'first_enabled' being essentialy
harmless if set when it's _not_ the first interrupt, which suggests
that maybe that's not the best name for the flag).

Couldn't it be fixed more simply by detecting comparator values in the
past in hpet_get_comparator()?

Tim.

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