[RFC] lapic calibration results fix - Kernel

This is a discussion on [RFC] lapic calibration results fix - Kernel ; Ingo, Maciej, it seems I found a bit strange code snippet in apic_32.c:setup_boot_APIC_clock. We set local_apic_timer_verify_ok = 1 before checking the results. I think the following patch make sense. Please take a look on. --- We set lapic flag that ...

+ Reply to Thread
Results 1 to 2 of 2

Thread: [RFC] lapic calibration results fix

  1. [RFC] lapic calibration results fix

    Ingo, Maciej,

    it seems I found a bit strange code snippet in apic_32.c:setup_boot_APIC_clock.
    We set local_apic_timer_verify_ok = 1 before checking the results. I think
    the following patch make sense. Please take a look on.

    ---
    We set lapic flag that clocksource calibration is
    fine too early. Fix it.

    Signed-off-by: Cyrill Gorcunov
    ---

    Index: linux-2.6.git/arch/x86/kernel/apic_32.c
    ================================================== =================
    --- linux-2.6.git.orig/arch/x86/kernel/apic_32.c 2008-07-13 15:25:20.000000000 +0400
    +++ linux-2.6.git/arch/x86/kernel/apic_32.c 2008-07-13 15:31:03.000000000 +0400
    @@ -489,8 +489,6 @@ void __init setup_boot_APIC_clock(void)
    calibration_result / (1000000 / HZ),
    calibration_result % (1000000 / HZ));

    - local_apic_timer_verify_ok = 1;
    -
    /*
    * Do a sanity check on the APIC calibration result
    */
    @@ -504,6 +502,8 @@ void __init setup_boot_APIC_clock(void)
    return;
    }

    + local_apic_timer_verify_ok = 1;
    +
    /* We trust the pm timer based calibration */
    if (!pm_referenced) {
    apic_printk(APIC_VERBOSE, "... verify APIC timer\n");
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [RFC] lapic calibration results fix

    [Cyrill Gorcunov - Sun, Jul 13, 2008 at 03:37:34PM +0400]
    | Ingo, Maciej,
    |
    | it seems I found a bit strange code snippet in apic_32.c:setup_boot_APIC_clock.
    | We set local_apic_timer_verify_ok = 1 before checking the results. I think
    | the following patch make sense. Please take a look on.
    |
    | ---
    | We set lapic flag that clocksource calibration is
    | fine too early. Fix it.
    |
    | Signed-off-by: Cyrill Gorcunov
    | ---
    |
    | Index: linux-2.6.git/arch/x86/kernel/apic_32.c
    | ================================================== =================
    | --- linux-2.6.git.orig/arch/x86/kernel/apic_32.c 2008-07-13 15:25:20.000000000 +0400
    | +++ linux-2.6.git/arch/x86/kernel/apic_32.c 2008-07-13 15:31:03.000000000 +0400
    | @@ -489,8 +489,6 @@ void __init setup_boot_APIC_clock(void)
    | calibration_result / (1000000 / HZ),
    | calibration_result % (1000000 / HZ));
    |
    | - local_apic_timer_verify_ok = 1;
    | -
    | /*
    | * Do a sanity check on the APIC calibration result
    | */
    | @@ -504,6 +502,8 @@ void __init setup_boot_APIC_clock(void)
    | return;
    | }
    |
    | + local_apic_timer_verify_ok = 1;
    | +
    | /* We trust the pm timer based calibration */
    | if (!pm_referenced) {
    | apic_printk(APIC_VERBOSE, "... verify APIC timer\n");

    Letme clarify a bit situation why I propose this path (it's not that
    clear from its description).

    While calibrating apic timer we use different 'flags' signaling if it
    was success/fail. On 64bit platform lapic_timer_setup() does check
    for CLOCK_EVT_FEAT_DUMMY bit which is set by default and cleared on
    successfully calibrated timer. So 64bit is fine now. On 32bit we
    use a different approach - local_apic_timer_verify_ok flag _BUT_
    as I see it being set too early _before_ calibration result is checked
    so we could have the following - kernel reports user that APIC timer
    is too slow and disabled but local_apic_timer_verify_ok remains set
    to 1 instead of 0.

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

+ Reply to Thread