Re: [Xen-devel] [PATCH RFC] x86/lapic: remove the PIT usage to calibrate the lapic timer

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.09.18 at 18:25, <roger.pau@xxxxxxxxxx> wrote:
> And instead use NOW which is based on the TSC. This was already used
> when running in shim mode, since there's likely no PIT in that
> environment.
> 
> Remove printing the CPU frequency, since it's already printed earlier
> at boot, and getting the CPU frequency against the TSC without any
> external reference timer is pointless.
> 
> The motivation behind this change is to allow Xen to boot on HyperV
> gen2 instances, which lack a PIT.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> I'm not sure about the reason behind the usage of the PIT instead of
> the TSC, maybe this was done because the TSC wasn't available until
> the Pentium? Xen certainly doesn't care about such hardware anymore,
> and the TSC is already used unconditionally in Xen.

How good are the chances that on at least some hardware LAPIC
and TSC frequencies derive from the same oscillator? If they do,
you'd now basically calibrate a clock against itself, ...

> Linux seems to prefer to calibrate the lapic timer against the PM
> timer and has already dropped PIT usage for that.

... while this is almost certainly using a completely different timer,
with the possible (hypothetical?) exception of SoC-s.

Also, if we were to change the default against which clock to
calibrate, I think we'd at least want to have a command line option
controllable fallback to the current mode, just in case of unforeseen
problems. This could be dropped a couple of releases later, if we
really want to get rid of that code.

> @@ -1176,10 +1117,6 @@ static int __init calibrate_APIC_clock(void)
>  
>      result = (tt1-tt2)*APIC_DIVISOR/LOOPS;
>  
> -    apic_printk(APIC_VERBOSE, "..... CPU clock speed is %ld.%04ld MHz.\n",
> -                ((long)(t2 - t1) / LOOPS) / (1000000 / HZ),
> -                ((long)(t2 - t1) / LOOPS) % (1000000 / HZ));
> -

I think I dislike this removal, despite the apparent redundancy, and
in particular as part of an unrelated patch. It sitting next to

>      apic_printk(APIC_VERBOSE, "..... host bus clock speed is %ld.%04ld 
> MHz.\n",
>                  result / (1000000 / HZ), result % (1000000 / HZ));

is quite helpful imo, and APIC_VERBOSE mode is off by default
anyway.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel