On Mon, Apr 04, 2022 at 01:55:56PM +0200, Jan Beulich wrote:
> On 04.04.2022 13:46, Roger Pau Monné wrote:
> > On Thu, Mar 31, 2022 at 11:29:44AM +0200, Jan Beulich wrote:
> >> +uint64_t __init calibrate_apic_timer(void)
> >> +{
> >> + uint32_t start, end;
> >> + uint64_t count = read_pt_and_tmcct(&start), elapsed;
> >> + uint64_t target = CALIBRATE_VALUE(plt_src.frequency), actual;
> >> + uint64_t mask = (uint64_t)~0 >> (64 - plt_src.counter_bits);
> >> +
> >> + /*
> >> + * PIT cannot be used here as it requires the timer interrupt to
> >> maintain
> >> + * its 32-bit software counter, yet here we run with IRQs disabled.
> >> + */
> >> + if ( using_pit )
> >> + return 0;
> >> +
> >> + while ( ((plt_src.read_counter() - count) & mask) < target )
> >> + continue;
> >> +
> >> + actual = read_pt_and_tmcct(&end) - count;
> >
> > Don't you need to apply the pt mask here?
>
> Oh, yes, of course. I guess I did clone an earlier mistake from
> calibrate_tsc().
>
> >> + elapsed = start - end;
> >> +
> >> + if ( likely(actual > target) )
> >> + {
> >> + /* See the comment in calibrate_tsc(). */
> >
> > I would maybe add that the counters used here might have > 32 bits,
> > and hence we need to trim the values for muldiv64 to scale properly.
>
> Sure - I've added "But first scale down values to actually fit
> muldiv64()'s input range."
With those taken care of:
Reviewed-by: Roger Pau Monné <roger....@citrix.com>
Thanks, Roger.
