Re: [RFC] TSC patch v2

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

Colin D Bennett wrote:
> On Fri, 04 Jul 2008 22:49:59 +0300
> Vesa Jääskeläinen <chaac@nic.fi> wrote:
> 
>> Hi Colin,
>>
>> Colin D Bennett wrote:
>>> Here is an updated version of the TSC patch.
>>>
>>> I addressed the issues in your comments with the exception of
>>> supporting x86 CPUs that don't have a TSC (386 and 486).
>>>
>>> I eliminated the grub_time_init() function and the call to it in
>>> grub_main() in favor of having grub_machine_init() simply call
>>> grub_tsc_calibrate().  The i386-pc and i386-efi platforms use the
>>> TSC.
>> Please still use grub_time_init(). Detect TSC, if it is there
>> calibrate and adapt for it. If it is not there, use RTC.
> 
> Should grub_time_init() be a CPU or machine independent function?
> Should it use conditional compilation to be usable on different CPUs?
> Or should grub_time_init() be specific to i386 kernels at this point?
> (Since we don't have anything to initialize for non-TSC-supporting
> platforms.)

In your patch you just change call of grub_tsc_calibrate() to 
grub_time_init(). And in grub_time_init() you check cpuflags if tsc is 
available or not. If tsc is available you call grub_tsc_calibrate(). And 
if you are planning to use function pointers then you set proper 
function pointers accordingly.

> If grub_time_init() is specific to the i386 CPU, then grub_time_init()
> should check whether the CPU has TSC support, and if so,
> grub_tsc_calibrate() should be called, and calls to grub_get_time_ms()
> should be directed to the TSC implementation.  If no TSC support is
> available, then no calibration is done, and calls to grub_get_time_ms()
> should be directed to the generic get_time_ms() implementation that
> uses grub_get_rtc() to do its job.

You can even make it something like:

- check for cpu flags (eg. detect method)
- if tsc, set function pointers rtc (init=tsc, getters=tsc)
- if no tsc set function pointers to rtc (init=rtc, getters=rtc)
- call function pointer init

> I want to make sure that I clearly understand what you want, since I
> have made major re-designs of the TSC support in GRUB in an attempt to
> do what you want.  Just in the past couple of days I have spent about 8
> hours getting the TSC patch where it is now. (I know that seems
> like a lot since it's not that much code, but I am just beginning to get
> familiar with the GRUB kernel organization, so simple changes have been
> taking me a long time.)

Sure... This is why it is good to discuss and design these things on 
mailing list :)