Skip to content

Commit

Permalink
Kernel: Fix APIC timer calibration to be more accurate
Browse files Browse the repository at this point in the history
We were calibrating it to 260 instead of 250 ticks per second (being
off by one for the 1/10th second calibration time), resulting in
ticks of only ~3.6 ms instead of ~4ms. This gets us closer to ~4ms,
but because the APIC isn't nearly as precise as e.g. HPET, it will
only be a best effort. Then, use the higher precision reference
timer to more accurately calculate how many ticks we actually get
each second.

Also the frequency calculation was off, causing a "Frequency too slow"
error with VMware.

Fixes some problems observed in SerenityOS#5539
  • Loading branch information
tomuta authored and awesomekling committed Mar 1, 2021
1 parent f66adbd commit cdbd878
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 23 deletions.
2 changes: 1 addition & 1 deletion Kernel/Random.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ UNMAP_AFTER_INIT KernelRng::KernelRng()
klog() << "KernelRng: Using HPET as entropy source";

for (size_t i = 0; i < resource().pool_count * resource().reseed_threshold; ++i) {
u64 hpet_time = HPET::the().read_main_counter();
u64 hpet_time = HPET::the().read_main_counter_unsafe();
this->resource().add_random_event(hpet_time, i % 32);
}
} else {
Expand Down
33 changes: 23 additions & 10 deletions Kernel/Time/APICTimer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,28 +65,33 @@ UNMAP_AFTER_INIT bool APICTimer::calibrate(HardwareTimerBase& calibration_source

// temporarily replace the timer callbacks
const size_t ticks_in_100ms = calibration_source.ticks_per_second() / 10;
Atomic<size_t> calibration_ticks = 0;
Atomic<size_t, AK::memory_order_relaxed> calibration_ticks = 0;
#ifdef APIC_TIMER_MEASURE_CPU_CLOCK
volatile u64 start_tsc = 0, end_tsc = 0;
#endif
volatile u64 start_reference = 0, end_reference = 0;
volatile u32 start_apic_count = 0, end_apic_count = 0;
bool query_reference = calibration_source.can_query_raw();
auto original_source_callback = calibration_source.set_callback([&](const RegisterState&) {
u32 current_timer_count = apic.get_timer_current_count();
#ifdef APIC_TIMER_MEASURE_CPU_CLOCK
u64 current_tsc = supports_tsc ? read_tsc() : 0;
#endif
u64 current_reference = query_reference ? calibration_source.current_raw() : 0;

auto prev_tick = calibration_ticks.fetch_add(1, AK::memory_order_acq_rel);
auto prev_tick = calibration_ticks.fetch_add(1);
if (prev_tick == 0) {
#ifdef APIC_TIMER_MEASURE_CPU_CLOCK
start_tsc = current_tsc;
#endif
start_apic_count = current_timer_count;
} else if (prev_tick + 1 == ticks_in_100ms) {
start_reference = current_reference;
} else if (prev_tick + 1 == ticks_in_100ms + 1) {
#ifdef APIC_TIMER_MEASURE_CPU_CLOCK
end_tsc = current_tsc;
#endif
end_apic_count = current_timer_count;
end_reference = current_reference;
}
});

Expand All @@ -102,7 +107,7 @@ UNMAP_AFTER_INIT bool APICTimer::calibrate(HardwareTimerBase& calibration_source

sti();
// Loop for about 100 ms
while (calibration_ticks.load(AK::memory_order_relaxed) < ticks_in_100ms)
while (calibration_ticks.load() <= ticks_in_100ms)
;
cli();

Expand All @@ -112,25 +117,33 @@ UNMAP_AFTER_INIT bool APICTimer::calibrate(HardwareTimerBase& calibration_source

disable_local_timer();

if (query_reference) {
u64 one_tick_ns = calibration_source.raw_to_ns((end_reference - start_reference) / ticks_in_100ms);
m_frequency = (u32)(1000000000ull / one_tick_ns);
klog() << "APICTimer: Ticks per second: " << m_frequency << " (" << (one_tick_ns / 1000000) << "." << (one_tick_ns % 1000000) << "ms)";
} else {
// For now, assume the frequency is exactly the same
m_frequency = calibration_source.ticks_per_second();
klog() << "APICTimer: Ticks per second: " << m_frequency << " (assume same frequency as reference clock)";
}

auto delta_apic_count = start_apic_count - end_apic_count; // The APIC current count register decrements!
m_timer_period = (delta_apic_count * apic.get_timer_divisor()) / ticks_in_100ms;

auto apic_freq = (delta_apic_count * apic.get_timer_divisor()) / apic.get_timer_divisor();
u64 apic_freq = delta_apic_count * apic.get_timer_divisor() * 10;
klog() << "APICTimer: Bus clock speed: " << (apic_freq / 1000000) << "." << (apic_freq % 1000000) << " MHz";
if (apic_freq < 1000000) {
klog() << "APICTimer: Frequency too slow!";
return false;
}
klog() << "APICTimer: Bus clock speed: " << (apic_freq / 1000000) << "." << (apic_freq % 1000000) << " MHz";

#ifdef APIC_TIMER_MEASURE_CPU_CLOCK
if (supports_tsc) {
auto delta_tsc = end_tsc - start_tsc;
auto delta_tsc = (end_tsc - start_tsc) * 10;
klog() << "APICTimer: CPU clock speed: " << (delta_tsc / 1000000) << "." << (delta_tsc % 1000000) << " MHz";
}
#endif

// TODO: measure rather than assuming it matches?
m_frequency = calibration_source.ticks_per_second();

enable_local_timer();
return true;
}
Expand Down
31 changes: 20 additions & 11 deletions Kernel/Time/HPET.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace Kernel {

#define ABSOLUTE_MAXIMUM_COUNTER_TICK_PERIOD 0x05F5E100
#define NANOSECOND_PERIOD_TO_HERTZ(x) 1000000000 / x
#define MEGAHERTZ_TO_HERTZ(x) (x / 1000000)
#define HERTZ_TO_MEGAHERTZ(x) (x / 1000000)

namespace HPETFlags {
enum class Attributes {
Expand Down Expand Up @@ -220,7 +220,7 @@ void HPET::update_periodic_comparator_value()
// way to resume periodic timers properly because we reset the main counter
// and we can only write the period into the comparator value...
timer.capabilities = timer.capabilities | (u32)HPETFlags::TimerConfiguration::ValueSet;
u64 value = frequency() / comparator.ticks_per_second();
u64 value = ns_to_raw_counter_ticks(1000000000ull / comparator.ticks_per_second());
dbgln_if(HPET_DEBUG, "HPET: Update periodic comparator {} comparator value to {} main value was: {}",
comparator.comparator_number(),
value,
Expand Down Expand Up @@ -250,19 +250,18 @@ void HPET::update_non_periodic_comparator_value(const HPETComparator& comparator
VERIFY_INTERRUPTS_DISABLED();
VERIFY(!comparator.is_periodic());
VERIFY(comparator.comparator_number() <= m_comparators.size());
auto& regs = registers();
auto& timer = regs.get_timer_by_index(comparator.comparator_number());
auto& timer = registers().get_timer_by_index(comparator.comparator_number());
u64 value = frequency() / comparator.ticks_per_second();
// NOTE: If the main counter passes this new value before we finish writing it, we will never receive an interrupt!
u64 new_counter_value = read_register_safe64(regs.main_counter_value) + value;
u64 new_counter_value = read_main_counter() + value;
timer.comparator_value.high = (u32)(new_counter_value >> 32);
timer.comparator_value.low = (u32)new_counter_value;
}

u64 HPET::update_time(u64& seconds_since_boot, u32& ticks_this_second, bool query_only)
{
// Should only be called by the time keeper interrupt handler!
u64 current_value = read_register_safe64(registers().main_counter_value);
u64 current_value = read_main_counter();
u64 delta_ticks = m_main_counter_drift;
if (current_value >= m_main_counter_last_read)
delta_ticks += current_value - m_main_counter_last_read;
Expand All @@ -286,12 +285,17 @@ u64 HPET::update_time(u64& seconds_since_boot, u32& ticks_this_second, bool quer
return (delta_ticks * 1000000000ull) / ticks_per_second;
}

u64 HPET::read_main_counter() const
u64 HPET::read_main_counter_unsafe() const
{
auto& main_counter = registers().main_counter_value;
return ((u64)main_counter.high << 32) | (u64)main_counter.low;
}

u64 HPET::read_main_counter() const
{
return read_register_safe64(registers().main_counter_value);
}

void HPET::enable_periodic_interrupt(const HPETComparator& comparator)
{
#if HPET_DEBUG
Expand Down Expand Up @@ -405,10 +409,15 @@ HPETRegistersBlock& HPET::registers()
return *(HPETRegistersBlock*)m_hpet_mmio_region->vaddr().offset(m_physical_acpi_hpet_registers.offset_in_page()).as_ptr();
}

u64 HPET::calculate_ticks_in_nanoseconds() const
u64 HPET::raw_counter_ticks_to_ns(u64 raw_ticks) const
{
// ABSOLUTE_MAXIMUM_COUNTER_TICK_PERIOD == 100 nanoseconds
return ((u64)registers().capabilities.main_counter_tick_period * 100ull) / ABSOLUTE_MAXIMUM_COUNTER_TICK_PERIOD;
return (raw_ticks * (u64)registers().capabilities.main_counter_tick_period * 100ull) / ABSOLUTE_MAXIMUM_COUNTER_TICK_PERIOD;
}

u64 HPET::ns_to_raw_counter_ticks(u64 ns) const
{
return (ns * 1000000ull) / (u64)registers().capabilities.main_counter_tick_period;
}

UNMAP_AFTER_INIT HPET::HPET(PhysicalAddress acpi_hpet)
Expand Down Expand Up @@ -438,8 +447,8 @@ UNMAP_AFTER_INIT HPET::HPET(PhysicalAddress acpi_hpet)

global_disable();

m_frequency = NANOSECOND_PERIOD_TO_HERTZ(calculate_ticks_in_nanoseconds());
klog() << "HPET: frequency " << m_frequency << " Hz (" << MEGAHERTZ_TO_HERTZ(m_frequency) << " MHz) resolution: " << calculate_ticks_in_nanoseconds() << "ns";
m_frequency = NANOSECOND_PERIOD_TO_HERTZ(raw_counter_ticks_to_ns(1));
klog() << "HPET: frequency " << m_frequency << " Hz (" << HERTZ_TO_MEGAHERTZ(m_frequency) << " MHz) resolution: " << raw_counter_ticks_to_ns(1) << "ns";
VERIFY(regs.capabilities.main_counter_tick_period <= ABSOLUTE_MAXIMUM_COUNTER_TICK_PERIOD);

// Reset the counter, just in case... (needs to match m_main_counter_last_read)
Expand Down
5 changes: 4 additions & 1 deletion Kernel/Time/HPET.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class HPET {
static HPET& the();

u64 frequency() const { return m_frequency; }
u64 raw_counter_ticks_to_ns(u64) const;
u64 ns_to_raw_counter_ticks(u64) const;

const NonnullRefPtrVector<HPETComparator>& comparators() const { return m_comparators; }
void disable(const HPETComparator&);
Expand All @@ -60,6 +62,7 @@ class HPET {
void disable_periodic_interrupt(const HPETComparator& comparator);

u64 update_time(u64& seconds_since_boot, u32& ticks_this_second, bool query_only);
u64 read_main_counter_unsafe() const;
u64 read_main_counter() const;

Vector<unsigned> capable_interrupt_numbers(u8 comparator_number);
Expand All @@ -75,7 +78,7 @@ class HPET {
bool is_periodic_capable(u8 comparator_number);
void set_comparators_to_optimal_interrupt_state(size_t timers_count);

u64 calculate_ticks_in_nanoseconds() const;
u64 nanoseconds_to_raw_ticks() const;

PhysicalAddress find_acpi_hpet_registers_block();
explicit HPET(PhysicalAddress acpi_hpet);
Expand Down
10 changes: 10 additions & 0 deletions Kernel/Time/HPETComparator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,14 @@ size_t HPETComparator::calculate_nearest_possible_frequency(size_t frequency) co
return frequency;
}

u64 HPETComparator::current_raw() const
{
return HPET::the().read_main_counter();
}

u64 HPETComparator::raw_to_ns(u64 raw_delta) const
{
return HPET::the().raw_counter_ticks_to_ns(raw_delta);
}

}
3 changes: 3 additions & 0 deletions Kernel/Time/HPETComparator.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ class HPETComparator final : public HardwareTimer<IRQHandler> {
virtual void set_periodic() override;
virtual void set_non_periodic() override;
virtual void disable() override;
virtual bool can_query_raw() const override { return true; }
virtual u64 current_raw() const override;
virtual u64 raw_to_ns(u64) const override;

virtual void reset_to_default_ticks_per_second() override;
virtual bool try_to_set_frequency(size_t frequency) override;
Expand Down
3 changes: 3 additions & 0 deletions Kernel/Time/HardwareTimer.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ class HardwareTimerBase
virtual void set_non_periodic() = 0;
virtual void disable() = 0;
virtual u32 frequency() const = 0;
virtual bool can_query_raw() const { return false; }
virtual u64 current_raw() const { return 0; }
virtual u64 raw_to_ns(u64) const { return 0; }

virtual size_t ticks_per_second() const = 0;

Expand Down

0 comments on commit cdbd878

Please sign in to comment.