Re: [RFC] TSC patch v2
Colin D Bennett <[EMAIL PROTECTED]> writes: > 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. > > I also changed the grub_millisleep() generic function to use > grub_get_time_ms() instead of grub_get_rtc() to achieve better > precision when a higher resolution time function is available. > > Please take a look at it and give me your comments when you have a > chance. Thanks! Great! Sorry I kept you waiting... > === modified file 'ChangeLog' > --- ChangeLog 2008-07-04 02:26:10 + > +++ ChangeLog 2008-07-04 18:08:36 + > @@ -1,3 +1,70 @@ > +2008-07-04 Colin D Bennett <[EMAIL PROTECTED]> > + > + High resolution timer support. Implemented for i386 CPU using TSC. > + Extracted generic grub_millisleep() so it's linked in only as needed. > + This requires a Pentium compatible CPU; currently the code does not > + check for this (so it will fail on 386 and 486 machines). We really need to keep the current machines working. If you will drop support for something, please don't do it in a patch but in a separate discussion so no one will miss it. But please, I do not think it will be that hard to keep this. > + * conf/i386-efi.rmk: Added TSC high resolution time module, link with > + generic grub_millisleep() function. > + > + * conf/i386-pc.rmk: Likewise. > + > + * conf/sparc64-ieee1275.rmk: Add kern/generic/millisleep.c and > + kern/generic/get_time_ms.c to kernel, to use generic time functions. > + > + * conf/powerpc-ieee1275.rmk: Add kern/generic/millisleep.c to kernel, > + to use generic grub_millisleep() function. > + > + * conf/i386-linuxbios.rmk: Added kern/generic/get_time_ms.c to the > + kernel. Please describe where you change things. Just have a look at existing changelog entries how this was done. > + * kern/generic/get_time_ms.c (grub_get_time_ms): New file. Platform > + independent implementation of grub_get_time_ms() using the RTC that > + can be linked into a platform's kernel when it does not implement its > + own specialized grub_get_time_ms() function. "New file." is sufficient. > + * kern/generic/millisleep.c (grub_millisleep): New file. Extracted > + from grub_millisleep_generic() in kern/misc.c and renamed. Changed it > + to use grub_get_time_ms() instead of grub_get_rtc() for better > + precision on when high resolution time is available. Same here. > + * kern/misc.c (grub_millisleep_generic): Deleted. Moved to > + kern/generic/millisleep.c so that it is only included in the kernel > + image when a platform does not define a specialized version. "Deleted." is sufficient. Or better "Removed.", just because most people use it :-) > + * commands/sleep.c (grub_interruptible_millisleep): Uses > + grub_get_time_ms() instead of grub_get_rtc() to stay in sync with > + grub_millisleep() from kern/generic/millisleep.c. > + > + * include/grub/i386/tsc.h (grub_get_tsc): New file. Inline function > + grub_get_tsc() uses x86 RDTSC instruction (available on Pentium+ CPUs) > + to read the counter value for the TSC. > + (grub_tsc_calibrate): Declare this function for grub_machine_init(). Same as above. > + * kern/i386/tsc.c (grub_get_time_ms): x86 TSC support providing a high > + resolution clock. I would simply say: "New function." > + (grub_tsc_calibrate): New function to calibrate the TSC using RTC. No need to explain what it does. > + * include/grub/time.h (grub_get_time_ms): Added grub_get_time_ms() > + function to return the current time in millseconds since the epoch. > + This supports higher resolution time than grub_get_rtc() on some > + platforms such as i386-pc, where the RTC has only about 1/18 s > + precision but a higher precision timer such as the TSC is available. Perhaps: "New prototype." You forgot to mention you included . Please make sure you mention all other inclusions of header files you added as well. > + * kern/i386/efi/init.c (grub_millisleep): Deleted. Don't define > + grub_millisleep() -- it just called grub_millisleep_generic() but now > + it is linked to kern/generic/millisleep.c for the implementation. Only "Removed." will be sufficient. > + * kern/sparc64/ieee1275/init.c (grub_millisleep): Deleted. Hurray! ;-) > + * kern/i386/pc/init.c (grub_machine_init): Call grub_tsc_calibrate(). > + (grub_millisleep): Deleted. > + > + * kern/ieee1275/init.c (grub_millisleep): Deleted. > + (grub_get_rtc): Now calls grub_get_time_ms(), which does the real > + work. As you can see from my comments, you do not have to say what something
Re: [RFC] TSC patch v2
Colin D Bennett wrote: On Fri, 04 Jul 2008 22:49:59 +0300 Vesa Jääskeläinen <[EMAIL PROTECTED]> 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 :) ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC] TSC patch v2
On Fri, 04 Jul 2008 22:49:59 +0300 Vesa Jääskeläinen <[EMAIL PROTECTED]> 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.) 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. 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.) > grub_time_init() is just more clearer term than tsc. At least for > people that are not familiar with it. in grub_time_init() you can > have anything then. And people that are not interested on time stuff > do not need to bother their heads about it. Also modifying logic > afterwards do not change other parts and it will be less error prone. OK. Regards, Colin signature.asc Description: PGP signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC] TSC patch v2
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. grub_time_init() is just more clearer term than tsc. At least for people that are not familiar with it. in grub_time_init() you can have anything then. And people that are not interested on time stuff do not need to bother their heads about it. Also modifying logic afterwards do not change other parts and it will be less error prone. Thanks, Vesa Jääskeläinen ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC] TSC patch v2
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.
I also changed the grub_millisleep() generic function to use
grub_get_time_ms() instead of grub_get_rtc() to achieve better
precision when a higher resolution time function is available.
Please take a look at it and give me your comments when you have a
chance. Thanks!
Regards,
Colin
=== modified file 'ChangeLog'
--- ChangeLog 2008-07-04 02:26:10 +
+++ ChangeLog 2008-07-04 18:08:36 +
@@ -1,3 +1,70 @@
+2008-07-04 Colin D Bennett <[EMAIL PROTECTED]>
+
+ High resolution timer support. Implemented for i386 CPU using TSC.
+ Extracted generic grub_millisleep() so it's linked in only as needed.
+ This requires a Pentium compatible CPU; currently the code does not
+ check for this (so it will fail on 386 and 486 machines).
+
+ * conf/i386-efi.rmk: Added TSC high resolution time module, link with
+ generic grub_millisleep() function.
+
+ * conf/i386-pc.rmk: Likewise.
+
+ * conf/sparc64-ieee1275.rmk: Add kern/generic/millisleep.c and
+ kern/generic/get_time_ms.c to kernel, to use generic time functions.
+
+ * conf/powerpc-ieee1275.rmk: Add kern/generic/millisleep.c to kernel,
+ to use generic grub_millisleep() function.
+
+ * conf/i386-linuxbios.rmk: Added kern/generic/get_time_ms.c to the
+ kernel.
+
+ * kern/generic/get_time_ms.c (grub_get_time_ms): New file. Platform
+ independent implementation of grub_get_time_ms() using the RTC that
+ can be linked into a platform's kernel when it does not implement its
+ own specialized grub_get_time_ms() function.
+
+ * kern/generic/millisleep.c (grub_millisleep): New file. Extracted
+ from grub_millisleep_generic() in kern/misc.c and renamed. Changed it
+ to use grub_get_time_ms() instead of grub_get_rtc() for better
+ precision on when high resolution time is available.
+
+ * kern/misc.c (grub_millisleep_generic): Deleted. Moved to
+ kern/generic/millisleep.c so that it is only included in the kernel
+ image when a platform does not define a specialized version.
+
+ * commands/sleep.c (grub_interruptible_millisleep): Uses
+ grub_get_time_ms() instead of grub_get_rtc() to stay in sync with
+ grub_millisleep() from kern/generic/millisleep.c.
+
+ * include/grub/i386/tsc.h (grub_get_tsc): New file. Inline function
+ grub_get_tsc() uses x86 RDTSC instruction (available on Pentium+ CPUs)
+ to read the counter value for the TSC.
+ (grub_tsc_calibrate): Declare this function for grub_machine_init().
+
+ * kern/i386/tsc.c (grub_get_time_ms): x86 TSC support providing a high
+ resolution clock.
+ (grub_tsc_calibrate): New function to calibrate the TSC using RTC.
+
+ * include/grub/time.h (grub_get_time_ms): Added grub_get_time_ms()
+ function to return the current time in millseconds since the epoch.
+ This supports higher resolution time than grub_get_rtc() on some
+ platforms such as i386-pc, where the RTC has only about 1/18 s
+ precision but a higher precision timer such as the TSC is available.
+
+ * kern/i386/efi/init.c (grub_millisleep): Deleted. Don't define
+ grub_millisleep() -- it just called grub_millisleep_generic() but now
+ it is linked to kern/generic/millisleep.c for the implementation.
+
+ * kern/sparc64/ieee1275/init.c (grub_millisleep): Deleted.
+
+ * kern/i386/pc/init.c (grub_machine_init): Call grub_tsc_calibrate().
+ (grub_millisleep): Deleted.
+
+ * kern/ieee1275/init.c (grub_millisleep): Deleted.
+ (grub_get_rtc): Now calls grub_get_time_ms(), which does the real
+ work.
+
2008-07-04 Pavel Roskin <[EMAIL PROTECTED]>
* kern/i386/linuxbios/init.c (grub_machine_init): Cast addr to
=== modified file 'commands/sleep.c'
--- commands/sleep.c 2008-05-16 20:55:29 +
+++ commands/sleep.c 2008-07-04 16:55:48 +
@@ -43,15 +43,15 @@
grub_printf ("%d", n);
}
-/* Based on grub_millisleep() from kern/misc.c. */
+/* Based on grub_millisleep() from kern/generic/millisleep.c. */
static int
grub_interruptible_millisleep (grub_uint32_t ms)
{
- grub_uint32_t end_at;
-
- end_at = grub_get_rtc () + grub_div_roundup (ms * GRUB_TICKS_PER_SECOND, 1000);
-
- while (grub_get_rtc () < end_at)
+ grub_uint64_t start;
+
+ start = grub_get_time_ms ();
+
+ while (grub_get_time_ms () - start < ms)
if (grub_checkkey () >= 0 &&
GRUB_TERM_ASCII_CHAR (grub_getkey ()) == GRUB_TERM_ESC)
return 1;
=== modified file 'conf/i386-efi.rmk'
--- conf/i386-efi.rmk 2008-06-19 04:14:16 +
+++ conf/i386-efi.rmk 2008-07-03 04:19:16 +
@@ -84,7 +84,9 @@
kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
kern/i386/dl.c kern/i386/efi/init.c kern/parser.c kern/partition.c \
kern/env.c symlist.c kern/efi/efi.c kern/efi/init.c kern/efi/mm.c \
- term/efi/console.c
