From 5dfc0663d5a98c050f5263cbcf2c265efb93274c Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Thu, 21 May 2026 15:01:17 -0400 Subject: [PATCH] kernel/x86: Avoid using any initial values in APIC timer registers. We should just be zeroing any unknown bits, not trying to preserve them. If the BIOS or other mechanisms left the timer in some other mode besides "one-shot" like we need, then we might incorrectly preserve those bits. This also has the advantage of avoiding config reads in set_hardware_timer. --- headers/private/kernel/arch/x86/apic.h | 3 - .../kernel/arch/x86/timers/x86_apic.cpp | 64 +++++++++---------- 2 files changed, 29 insertions(+), 38 deletions(-) diff --git a/headers/private/kernel/arch/x86/apic.h b/headers/private/kernel/arch/x86/apic.h index 0a3ef8d547..e88509329c 100644 --- a/headers/private/kernel/arch/x86/apic.h +++ b/headers/private/kernel/arch/x86/apic.h @@ -68,9 +68,6 @@ /* Local Vector Table defines */ #define APIC_LVT_MASKED (1 << 16) -// timer defines -#define APIC_LVT_TIMER_MASK 0xfffcef00 - // LINT0/1 defines #define APIC_LVT_LINT_MASK 0xfffe0800 #define APIC_LVT_LINT_INPUT_POLARITY (1 << 13) diff --git a/src/system/kernel/arch/x86/timers/x86_apic.cpp b/src/system/kernel/arch/x86/timers/x86_apic.cpp index 4c104be7b4..bb871e1860 100644 --- a/src/system/kernel/arch/x86/timers/x86_apic.cpp +++ b/src/system/kernel/arch/x86/timers/x86_apic.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2025, Haiku, Inc. All rights reserved. + * Copyright 2025-2026, Haiku, Inc. All rights reserved. * Copyright 2008, Dustin Howett, dustin.howett@gmail.com. All rights reserved. * Copyright 2002-2010, Axel Dörfler, axeld@pinc-software.de. All rights reserved. * Distributed under the terms of the MIT License. @@ -28,21 +28,8 @@ #endif -/* Method Prototypes */ -static int apic_timer_get_priority(); -static status_t apic_timer_set_hardware_timer(bigtime_t relativeTimeout); -static status_t apic_timer_clear_hardware_timer(); -static status_t apic_timer_init(struct kernel_args *args); - static uint32 sApicTicsPerSec = 0; - -struct timer_info gAPICTimer = { - "APIC", - &apic_timer_get_priority, - &apic_timer_set_hardware_timer, - &apic_timer_clear_hardware_timer, - &apic_timer_init -}; +static uint32 sApicTimerConfigBase = 0; static int @@ -72,13 +59,13 @@ apic_timer_set_hardware_timer(bigtime_t relativeTimeout) cpu_status state = disable_interrupts(); - uint32 config = apic_lvt_timer() | APIC_LVT_MASKED; // mask the timer - apic_set_lvt_timer(config); + // mask the timer + apic_set_lvt_timer(sApicTimerConfigBase | APIC_LVT_MASKED); apic_set_lvt_initial_timer_count(0); // zero out the timer - config &= ~APIC_LVT_MASKED; // unmask the timer - apic_set_lvt_timer(config); + // unmask the timer + apic_set_lvt_timer(sApicTimerConfigBase); TRACE("arch_smp_set_apic_timer: config 0x%" B_PRIx32 ", timeout %" B_PRIdBIGTIME ", tics/sec %" B_PRIu32 ", tics %" B_PRId64 "\n", config, relativeTimeout, @@ -97,11 +84,11 @@ apic_timer_clear_hardware_timer() { cpu_status state = disable_interrupts(); - uint32 config = apic_lvt_timer() | APIC_LVT_MASKED; - // mask the timer - apic_set_lvt_timer(config); + // mask the timer + apic_set_lvt_timer(sApicTimerConfigBase | APIC_LVT_MASKED); - apic_set_lvt_initial_timer_count(0); // zero out the timer + // zero out the timer + apic_set_lvt_initial_timer_count(0); restore_interrupts(state); return B_OK; @@ -115,13 +102,11 @@ static uint32 calculate_apic_timer_conversion_factor() { // setup the timer - uint32 config = apic_lvt_timer() & APIC_LVT_TIMER_MASK; - config |= APIC_LVT_MASKED; + uint32 config = APIC_LVT_MASKED; // timer masked, vector 0 apic_set_lvt_timer(config); - config = (apic_lvt_timer_divide_config() & ~0xf); - apic_set_lvt_timer_divide_config(config | APIC_TIMER_DIVIDE_CONFIG_1); + apic_set_lvt_timer_divide_config(APIC_TIMER_DIVIDE_CONFIG_1); // divide clock by one apic_set_lvt_initial_timer_count(UINT32_MAX); // start the counter @@ -181,6 +166,8 @@ apic_timer_init(struct kernel_args *args) install_io_interrupt_handler(0xfb - ARCH_INTERRUPT_BASE, &apic_timer_interrupt, NULL, B_NO_LOCK_VECTOR); + sApicTimerConfigBase = 0xfb; // one-shot mode, vector 0xfb + apic_timer_per_cpu_init(args, 0); // will be called on non-boot CPUs by apic_per_cpu_init() @@ -191,16 +178,23 @@ apic_timer_init(struct kernel_args *args) status_t apic_timer_per_cpu_init(struct kernel_args *args, int32 cpu) { - /* setup timer */ - uint32 config = apic_lvt_timer() & APIC_LVT_TIMER_MASK; - config |= 0xfb | APIC_LVT_MASKED; // vector 0xfb, timer masked - apic_set_lvt_timer(config); + // setup timer + apic_set_lvt_timer(sApicTimerConfigBase | APIC_LVT_MASKED); - apic_set_lvt_initial_timer_count(0); // zero out the clock + // zero out the clock + apic_set_lvt_initial_timer_count(0); - config = apic_lvt_timer_divide_config() & 0xfffffff0; - config |= APIC_TIMER_DIVIDE_CONFIG_1; // clock division by 1 - apic_set_lvt_timer_divide_config(config); + // clock division by 1 + apic_set_lvt_timer_divide_config(APIC_TIMER_DIVIDE_CONFIG_1); return B_OK; } + + +struct timer_info gAPICTimer = { + "APIC", + &apic_timer_get_priority, + &apic_timer_set_hardware_timer, + &apic_timer_clear_hardware_timer, + &apic_timer_init +};