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

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.11.2015 14:23, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> 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
Attached proof-of-concept. It's all in one file but different methods
should probably go in separate files.

pm.diff
Description: Text Data

signature.asc
Description: OpenPGP digital signature