Re: [Xen-devel] [PATCH v3 08/11] hvm/hpet: Use signed divide in hpet_get_comparator.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.04.14 at 19:43, <dslutz@xxxxxxxxxxx> wrote:
> This is done so that when comparator is less then or equal to one
> period it does not change.
> 
> The code lines:
> 
>     elapsed = hpet_read_maincounter(h, guest_time) +
>         period - 1 - comparator;
>     comparator += (elapsed / period) * period;
> 
> are what matter here.  For almost all cases where
> "hpet_read_maincounter() + period - 1" is greater then
> "comparator", a signed divide will produce the same answer as an
> unsigned divide.  One of the problem areas is when
> "hpet_read_maincounter() + period - 1" needs more then 64 bits to
> represent it.  It includes cases where hpet_read_maincounter() has

"It includes" implies there are other cases, yet I can't see any.
(Also twice s/then/than/ I think.)

> wrapped (I.E. needs more then 64 bits to correctly represent it),
> but "comparator" has not wrapped.

But can this happen in practice? Main counter and comparator
shouldn't be very far apart, and hence other than
"hpet_read_maincounter() + period - 1",
"hpet_read_maincounter() + period - 1 - comparator" shouldn't
ever require more than 64 bits to correctly represent it. All you
really need to deal with is the case where
hpet_read_maincounter() + period == comparator, i.e. the 1
being subtracted makes the expression degenerate. So you could
simply make the computation and update dependent upon
hpet_read_maincounter() + period > comparator.

And then again I don't really see why the subtraction of 1 is needed
here in the first place.

Jan


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