arm-clock low precision potential issues

[wkozaczuk]

Unlike the APIC clock on x86_64, which operates at 1GHz precision and allows setting time to expire at the exact number of nanoseconds, the arm clock typically operates at a much lower frequency—on QEMU, it is 24MHz (stored in the freq_hz variable). So the time on ARM is measured in ticks, which in the QEMU case lasts ~ 41ns.

APIC clock

void apic_clock_events::set(std::chrono::nanoseconds nanos)
{
    if (nanos.count() <= 0) {
        _callback->fired();
    } else {
        // FIXME: handle overflow
        apic->write(apicreg::TMICT, nanos.count());
    }
}

vs

ARM clock

void arm_clock_events::set(std::chrono::nanoseconds nanos)
{
    if (nanos.count() <= 0) {
        _callback->fired();
    } else {
        u64 tval = nanos.count();
        class arm_clock *c = static_cast<arm_clock *>(clock::get());
        tval = ((__uint128_t)tval * c->freq_hz) / NANO_PER_SEC;

        if (tval) {
            //This 'if (tval)' optimization is not likely to save us much
            //as the tiny 'nanos' values are rare but is so cheap
            //to do it, so why not?
            u32 ctl = this->read_ctl();
            ctl |= TIMER_CTL_ENABLE_BIT; // set enable
            ctl &= ~TIMER_CTL_IMASK_BIT; // unmask timer interrupt
            this->write_tval(tval);
            this->write_ctl(ctl);
        } else {
            //No point to set timer if tval is 0
            _callback->fired();
        }
    }
}

I am concerned this may potentially cause issues in the thread scheduler that uses a preemption timer and timer-related code in general (sleep(), etc). More specifically, when the arm_clock_events::set() is called to set up a timer event in the future in a specified number of nanoseconds since "now", it never does it precisely - it rounds it down to the number of ~41 ns ticks. So, it is theoretically possible that when the timer interrupt for this arrives, it may be too early. The code that is used to determine "now" is as you can see below, which also gives a rounded number of nanoseconds:

s64 arm_clock::uptime()
{
    //Please note we read CNTVCT cpu system register which provides
    //the accross-system consistent value of the virtual system counter.
    //This means that uptime() should return the exact same value
    //on each cpu.
    u64 cntvct;
    asm volatile ("isb; mrs %0, cntvct_el0; isb; " : "=r"(cntvct) :: "memory");

    cntvct = ((__uint128_t)cntvct * NANO_PER_SEC) / this->freq_hz;
    return cntvct;
}

s64 arm_clock::time()
{
    //Given that _boot_time_in_ns came from RTC which gives only 1 second
    //precision the resulting time() value is only accurate up to a second
    //On top of this, this implementation does not account for any host
    //clock drifts
    return _boot_time_in_ns + uptime();
}

So, I am not sure if OSv scheduler and other timer-related code can deal with this type of granularity. Maybe we should adjust the set() method to round up when calculating the number of ticks. I also wonder if my changes in cdb0ab6 to call _callback->fired() when the calculated number of ticks is 0 is actually correct.

There have been some arm clock-related shortcomings I have addressed:

• a0c1a5a
• cdb0ab6
• 9734d92
• bc37fbd

PS. ARM v8.6 is supposed to standardize the clock frequency at 1GHz - https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/arm-architecture-developments-armv8-6-a