The AArch64 Programmer's Guides Generic Timer (chapter 3.4, page 10) states following:
"The interrupts generated by the timer behave in a level-sensitive manner. This means that, once the timer firing condition is reached, the timer will continue to signal an interrupt until one of the following situations occurs: - IMASK is set to one, which masks the interrupt. - ENABLE is cleared to 0, which disables the timer. - TVAL or CVAL is written, so that firing condition is no longer met." This indicates that the timer interrupt handler needs to mask or disable the timer, otherwise the interrupt will be re-delivered and in some scenarios might cause underlying VMM (like QEMU in TCG mode) stop delivering subsequent interrupts. In any case per the description from the guide above, it is correct to mask the interrupt and/or disable the timer in the timer interrupt handler. This is also what Linux does - https://github.com/torvalds/linux/blob/edd7ab76847442e299af64a761febd180d71f98d/drivers/clocksource/arm_arch_timer.c#L638-L652. Besides the above, the patch also sligthly optimizes the method set() to make it set new timer only if calculated number of ticks (tval) is greater than 0. Fixes #1127 Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com> --- arch/aarch64/arm-clock.cc | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/arch/aarch64/arm-clock.cc b/arch/aarch64/arm-clock.cc index 7d11e454..d32f58b1 100644 --- a/arch/aarch64/arm-clock.cc +++ b/arch/aarch64/arm-clock.cc @@ -98,6 +98,10 @@ u64 arm_clock::processor_to_nano(u64 ticks) return cntvct; } +#define TIMER_CTL_ISTATUS_BIT 4 +#define TIMER_CTL_IMASK_BIT 2 +#define TIMER_CTL_ENABLE_BIT 1 + class arm_clock_events : public clock_event_driver { public: arm_clock_events(); @@ -120,7 +124,25 @@ arm_clock_events::arm_clock_events() res = 16 + 11; /* default PPI 11 */ } _irq.reset(new ppi_interrupt(gic::irq_type::IRQ_TYPE_EDGE, res, - [this] { this->_callback->fired(); })); + [this] { + /* From AArch64 Programmer's Guides Generic Timer (chapter 3.4, page 10): + * The interrupts generated by the timer behave in a level-sensitive manner. + * This means that, once the timer firing condition is reached, + * the timer will continue to signal an interrupt until one of the following situations occurs: + - IMASK is set to one, which masks the interrupt. + - ENABLE is cleared to 0, which disables the timer. + - TVAL or CVAL is written, so that firing condition is no longer met. + When writing an interrupt handler for the timers, it is important + that software clears the interrupt before deactivating the interrupt in the GIC. + Otherwise the GIC will re-signal the same interrupt again. */ + u32 ctl = this->read_ctl(); + if (ctl & TIMER_CTL_ISTATUS_BIT) { // check if timer condition met + ctl |= TIMER_CTL_IMASK_BIT; // mask timer interrupt + ctl &= ~TIMER_CTL_ENABLE_BIT; // disable timer + this->write_ctl(ctl); + this->_callback->fired(); + } + })); } arm_clock_events::~arm_clock_events() @@ -171,11 +193,16 @@ void arm_clock_events::set(std::chrono::nanoseconds nanos) class arm_clock *c = static_cast<arm_clock *>(clock::get()); tval = ((__uint128_t)tval * c->freq_hz) / NANO_PER_SEC; - u32 ctl = this->read_ctl(); - ctl |= 1; /* set enable */ - ctl &= ~2; /* unmask timer interrupt */ - this->write_tval(tval); - this->write_ctl(ctl); + if (tval) { + 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(); + } } } -- 2.30.2 -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20210319214955.93417-1-jwkozaczuk%40gmail.com.
