lkml.org 
[lkml]   [2008]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [Q] Is 64bit LVTT screwed
[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 -


\
 
 \ /
  Last update: 2008-07-08 21:13    [W:0.041 / U:21.096 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site