Re: [PATCH] Use UEFI Time Service to calibrate TSC

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.11.2015 09:03, Michael Chang wrote:
> On Mon, Nov 09, 2015 at 10:29:55AM +0300, Andrei Borzenkov wrote:
>> On Mon, Nov 9, 2015 at 10:07 AM, Michael Chang <address@hidden> wrote:
>>> This patch tries to detect PIT timer is broken and use UEFI Time Service
>>> to calibrate TSC.
>>
>> Second try :)
>>
>> https://lists.gnu.org/archive/html/grub-devel/2014-11/msg00079.html
>>
>> Although I think that this one is acceptable - it is used as fallback
>> only. Will it also catch the case when PIT is not present at all?
> 
> I think yes, actually this patch was tested and can fix the timeout
> problem on hyper-v Generation 2 VMs with UEFI, which is known to ship
> without PIT.
> 
> In addition to that, the linux kernel also calibrates the tsc via PIT as
> default on x86.
> 
> http://lxr.free-electrons.com/source/arch/x86/kernel/tsc.c#L441
> 
> But with some sanity checks to detect the SMI storm interfering the
> result and will fallback to other timer sources if the sanity check
> fails. This patch is inspired by that one of that check with much more
> restricted loopmin to "1" that's basically a insane condition. 
> 
I like some aspects of this patch, i.a. that it's unlikely to break
compatibility. ut I feel that we can do a bit better:
1) Returning through pointed variable is expensive in terms of binary
size. Just plain return is better.
2) More modern calibrations can calibrate in 1ms, not 55ms. This is one
that slows down coreboot boot (I think only USB init is slower).
3) We could have a cascade of methods. E.g.
on EFI: PIT -> PM -> Stall -> hardcoded 1GHz (putting PM as first would
be slightly more risk
on coreboot: PM -> PIT -> hardcoded 1GHz
rest: PIT -> hardcoded 1GHz (need to keep size down)

I'm going to prepare proof-of-concept patch
> Thanks,
> Michael
> 
>>
>>> ---
>>>  grub-core/kern/i386/tsc.c |   33 +++++++++++++++++++++++++++++----
>>>  1 files changed, 29 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/grub-core/kern/i386/tsc.c b/grub-core/kern/i386/tsc.c
>>> index bc441d0..bd24cea 100644
>>> --- a/grub-core/kern/i386/tsc.c
>>> +++ b/grub-core/kern/i386/tsc.c
>>> @@ -29,6 +29,10 @@
>>>  #include <grub/xen.h>
>>>  #else
>>>  #include <grub/i386/pit.h>
>>> +#ifdef GRUB_MACHINE_EFI
>>> +#include <grub/efi/efi.h>
>>> +#include <grub/efi/api.h>
>>> +#endif
>>>  #endif
>>>  #include <grub/cpu/io.h>
>>>
>>> @@ -72,7 +76,7 @@ grub_cpu_is_tsc_supported (void)
>>>  }
>>>
>>>  static void
>>> -grub_pit_wait (grub_uint16_t tics)
>>> +grub_pit_wait (grub_uint16_t tics, int *is_started)
>>>  {
>>>    /* Disable timer2 gate and speaker.  */
>>>    grub_outb (grub_inb (GRUB_PIT_SPEAKER_PORT)
>>> @@ -90,8 +94,17 @@ grub_pit_wait (grub_uint16_t tics)
>>>              | GRUB_PIT_SPK_TMR2,
>>>               GRUB_PIT_SPEAKER_PORT);
>>>
>>> -  /* Wait.  */
>>> -  while ((grub_inb (GRUB_PIT_SPEAKER_PORT) & GRUB_PIT_SPK_TMR2_LATCH) == 
>>> 0x00);
>>> +  if ((grub_inb (GRUB_PIT_SPEAKER_PORT) & GRUB_PIT_SPK_TMR2_LATCH))
>>> +    {
>>> +      /* The ticks have expired too fast to know the counting really 
>>> started or not */
>>> +      *is_started = 0;
>>> +    }
>>> +  else
>>> +    {
>>> +      *is_started = 1;
>>> +      /* Wait.  */
>>> +      while ((grub_inb (GRUB_PIT_SPEAKER_PORT) & GRUB_PIT_SPK_TMR2_LATCH) 
>>> == 0x00);
>>> +    }
>>>
>>>    /* Disable timer2 gate and speaker.  */
>>>    grub_outb (grub_inb (GRUB_PIT_SPEAKER_PORT)
>>> @@ -117,11 +130,23 @@ calibrate_tsc (void)
>>>  {
>>>    /* First calibrate the TSC rate (relative, not absolute time). */
>>>    grub_uint64_t end_tsc;
>>> +  int is_started;
>>>
>>>    tsc_boot_time = grub_get_tsc ();
>>> -  grub_pit_wait (0xffff);
>>> +  grub_pit_wait (0xffff, &is_started);
>>>    end_tsc = grub_get_tsc ();
>>>
>>> +#ifdef GRUB_MACHINE_EFI
>>> +  /* The PIT is broken as 55ms is too sufficent to any cpu to catch it */
>>> +  if (!is_started)
>>> +    {
>>> +      /* Use EFI Time Service to calibrate TSC */
>>> +      tsc_boot_time = grub_get_tsc ();
>>> +      efi_call_1 (grub_efi_system_table->boot_services->stall, 54925);
>>> +      end_tsc = grub_get_tsc ();
>>> +    }
>>> +#endif
>>> +
>>>    grub_tsc_rate = 0;
>>>    if (end_tsc > tsc_boot_time)
>>>      grub_tsc_rate = grub_divmod64 ((55ULL << 32), end_tsc - tsc_boot_time, 
>>> 0);
>>> --
>>> 1.7.3.4
>>>
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> address@hidden
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>> _______________________________________________
>> Grub-devel mailing list
>> address@hidden
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

signature.asc
Description: OpenPGP digital signature