Re: [Q] Is 64bit LVTT screwed

[Cyrill Gorcunov]

[Maciej W. Rozycki - Mon, Jul 07, 2008 at 11:08:57PM +0100]
[...]
| > 
| > but in both cases we use "divide by 16" in divide register. The only
| > explanation I imagine - for 64bit mode we are required to 'stuck'
| > for a bit longer (by 16 times longer to be precise). Am I right?
| > Or there is another reason why we dont use APIC_DIVISOR here. Actually,
| > as I see it not fair to a caller. For 64bit mode APIC timer is requested
| > to count 250000000 ticks but in real it will count 250000000 * 16.
| > Not sure who is right there. I think the better would be to
| > use 4000000000 and APIC_DIVISOR in 64bit mode. How do you think?
| 
|  The APIC clock varies across systems so the timer setup includes
| calibration.  Which means both variations end up with the correct
| interrupt rate probably, except using different divisors.  We used to
| print the APIC clock rate and that would be off with one of the above, but
| I don't we do this anymore.  We had a similar problem with the 82489DX
| where the prescaler was bypassed altogether resulting in an incorrect 
| clock rate printed, but the rate of timer interrupt was correctly 
| calibrated regardless.
| 
|  This is of course merely an explanation why the code you quoted does not
| explode spectacularly.  It does not make it correct.  If the prescaler is
| indeed set to 16 for 64-bit systems, then APIC_DIVISOR should obviously be
| used in the calculation above too.  And if not, then I think the prescaler 
| should be set consistently across systems and then the setting reflected 
| in the calculations accordingly.  Please note that only values between 2 
| and 16 are supported in a uniform way across all the APIC models and using 
| low values risks an overflow as clock rates are in the GHz range these 
| days already.  The value of 16 seems a reasonable one.
| 
|   Maciej
|

Maciej, I think the right soulution is the next:

Index: linux-2.6.git/arch/x86/kernel/apic_64.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic_64.c	2008-07-08 22:52:24.000000000 +0400
+++ linux-2.6.git/arch/x86/kernel/apic_64.c	2008-07-08 23:00:24.000000000 +0400
@@ -165,6 +165,9 @@ int lapic_get_maxlvt(void)
 	return maxlvt;
 }
 
+/* Clock divisor is set to 16 */
+#define APIC_DIVISOR 16
+
 /*
  * This function sets up the local APIC timer, with a timeout of
  * 'clocks' APIC bus clock. During calibration we actually call
@@ -197,7 +200,7 @@ static void __setup_APIC_LVTT(unsigned i
 				| APIC_TDR_DIV_16);
 
 	if (!oneshot)
-		apic_write(APIC_TMICT, clocks);
+		apic_write(APIC_TMICT, clocks/APIC_DIVISOR);
 }
 
 /*
@@ -329,7 +332,7 @@ static void __init calibrate_APIC_clock(
 	 *
 	 * No interrupt enable !
 	 */
-	__setup_APIC_LVTT(250000000, 0, 0);
+	__setup_APIC_LVTT(4000000000, 0, 0);
 
 	apic_start = apic_read(APIC_TMCCT);
 #ifdef CONFIG_X86_PM_TIMER

So we remain behaviour the same (ie calibrating duration remains the same)
but it has touched lapic_timer_setup. Will take a look into this more closer
soon.

		- Cyrill -
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/