Re: [RFC] High resolution time support using x86 TSC

["Vesa Jääskeläinen" <chaac@nic.fi>]

Colin D Bennett wrote:
> On Thu, 03 Jul 2008 20:52:53 +0200
> Marco Gerards <mgerards@xs4all.nl> wrote:
> 
>> Hi Colin,
>>
>> Colin D Bennett <colin@gibibit.com> writes:
>>
>>> +	* kern/i386/linuxbios/init.c (grub_get_time_ms):
>>> +	Define grub_get_time_ms() to always return 0.  This should
>>> be fixed
>>> +	when RTC functionality is implemented.
>>> +	(grub_time_init): Define this required function as a
>>> no-op. Inserted
>>> +	a comment to remind us delete this function when proper
>>> time support 
>>> +	is added to i386-linuxbios.
>>> +
>>> +	* kern/main.c (grub_main): Call grub_time_init() right
>>> after
>>> +	grub_machine_init().
>> I think this should go into grub_machine_init?  Why didn't you just
>> add it there?
> 
> Originally, I had grub_machine_init() call grub_tsc_calibrate(), but
> these are comments Vesa made to me about that code:
> 
> Vesa Jääskeläinen <dachaac@gmail.com> wrote
> on Mon, 16 Jun 2008 20:34:54 +0300:
> 
>> Colin D Bennett wrote:
>>> This week I implemented high resolution timer support using the x86
>>> TSC (see attached patch <grub_tsc_2008-06-10.patch>).  It is
>>> calibrated at GRUB startup using the RTC as a reference. The core
>>> TSC function is ``grub_get_tsc()`` -- corresponding to
>>> ``grub_get_rtc()``, but returning a uint64 value representing the
>>> number of CPU cycles elapsed since boot.
>>>
>>> In most situations, you will want to use ``grub_get_time_ms()`` to
>>> get the system time in milliseconds based on the TSC value.  The
>>> calibration function ``grub_tsc_calibrate()``, calculates the
>>> proper scale factor and absolute offset so that the millisecond
>>> value represents real time.
>>>
>>> The ``grub_get_time_ms()`` function is implemented for non-x86
>>> platforms to simply call ``grub_get_time_ms_generic()`` (defined in
>>> kern/misc.c), which uses the RTC to get the time in milliseconds.  
>> We would probably want to leave that generic out from kernel, and let 
>> every platform either use this generic code or implement their own 
>> mechanism to do the job. Perhaps we should make own folder for
>> generic stuff that can be included for arch specific build if there
>> is no better replacement.
>>
>> How about calling function like grub_time_init() which would then be 
>> platform specific? Then platform can implement whatever way to
>> calibrate (if needed) as long as it provides this grub_get_time_ms()
>> function (also being platform specific). This would make
>> initialization non-specific to arch while leaving more room for
>> implementation.
> 
> Therefore, I thought this was the right way to do it.  Do you want me
> to instead call grub_time_init() from grub_machine_init()?

Hi Colin,

That is Ok.

My comment was more of calling directly tsc_calibrate which is not 
generic function at all. You can call grub_time_init() and in there you 
decide that you use tsc (and then call tsc_calibrate) or something else 
depending what is available. This way time handling stuff can be reside 
easily on one module and it is easy to provide generic function to other 
platforms.

Thanks,
Vesa Jääskeläinen