diff --git a/arch/x86_64/src/intel64/intel64_hpet.c b/arch/x86_64/src/intel64/intel64_hpet.c index 83ff189b1eb80..36512ac6c7175 100644 --- a/arch/x86_64/src/intel64/intel64_hpet.c +++ b/arch/x86_64/src/intel64/intel64_hpet.c @@ -223,6 +223,7 @@ static void intel64_hpet_cmpset(struct intel64_tim_dev_s *dev, uint8_t timer, uint64_t cmp) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + DEBUGASSERT(timer < hpet->timers); intel64_hpet_putreg(hpet, HPET_TCOMP_OFFSET(timer), cmp); } @@ -239,6 +240,7 @@ static uint64_t intel64_hpet_cmpget(struct intel64_tim_dev_s *dev, uint8_t timer) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + DEBUGASSERT(timer < hpet->timers); return intel64_hpet_getreg(hpet, HPET_TCOMP_OFFSET(timer)); } @@ -255,6 +257,7 @@ static uint64_t intel64_hpet_intget(struct intel64_tim_dev_s *dev, uint8_t timer) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return (intel64_hpet_getreg(hpet, HPET_GISR_OFFSET) & HPET_GISR_TINT(timer)); } @@ -271,6 +274,7 @@ static void intel64_hpet_intack(struct intel64_tim_dev_s *dev, uint8_t timer) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + intel64_hpet_putreg(hpet, HPET_GISR_OFFSET, HPET_GISR_TINT(timer)); } @@ -285,6 +289,7 @@ static void intel64_hpet_intack(struct intel64_tim_dev_s *dev, static uint64_t intel64_hpet_cntget(struct intel64_tim_dev_s *dev) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return intel64_hpet_getreg(hpet, HPET_MCNTR_OFFSET); } @@ -300,6 +305,7 @@ static void intel64_hpet_cntset(struct intel64_tim_dev_s *dev, uint64_t cntr) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return intel64_hpet_putreg(hpet, HPET_MCNTR_OFFSET, cntr); } @@ -314,6 +320,7 @@ static void intel64_hpet_cntset(struct intel64_tim_dev_s *dev, static uint32_t intel64_hpet_perget(struct intel64_tim_dev_s *dev) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return hpet->clk_per_fs; } @@ -383,9 +390,12 @@ static int intel64_hpet_setisr(struct intel64_tim_dev_s *dev, uint8_t timer, if (handler == NULL) { - /* Disable interrupt */ + /* Disable the interrupt but keep the ISR attached. Detaching it here + * installs irq_unexpected_isr(), and an HPET interrupt that is already + * in flight to another CPU then panics the system. A stray interrupt + * is handled as spurious by the oneshot ISR instead. + */ - irq_attach(irq, handler, arg); up_disable_irq(irq); } else diff --git a/arch/x86_64/src/intel64/intel64_oneshot.c b/arch/x86_64/src/intel64/intel64_oneshot.c index a98b8cea55502..27168a329f88b 100644 --- a/arch/x86_64/src/intel64/intel64_oneshot.c +++ b/arch/x86_64/src/intel64/intel64_oneshot.c @@ -100,18 +100,21 @@ static int intel64_oneshot_handler(int irg_num, void * context, void *arg) INTEL64_TIM_ACKINT(oneshot->tch, oneshot->chan); #endif - /* The timer is no longer running */ + /* The timer is no longer running. Only pick up the handler here; + * it is owned by intel64_oneshot_start()/cancel(), which may be + * re-arming the timer on another CPU right now. Clearing it from + * the ISR could leave a re-armed timer without a handler, and the + * next expiry would then jump through a NULL pointer. + */ oneshot->running = false; - - /* Forward the event, clearing out any vestiges */ - oneshot_handler = (oneshot_handler_t)oneshot->handler; - oneshot->handler = NULL; oneshot_arg = (void *)oneshot->arg; - oneshot->arg = NULL; - oneshot_handler(oneshot_arg); + if (oneshot_handler != NULL) + { + oneshot_handler(oneshot_arg); + } } else { @@ -301,10 +304,21 @@ int intel64_oneshot_start(struct intel64_oneshot_s *oneshot, flags = spin_lock_irqsave(&g_oneshot_spin); if (oneshot->running) { - /* Yes.. then cancel it */ + /* Yes.. then stop it. Do NOT call intel64_oneshot_cancel() here: + * it takes g_oneshot_spin, which we already hold, and spinlocks are + * not recursive, so that deadlocks the CPU. Everything else that + * cancel would do (ISR, comparator, interrupt enable) is + * reprogrammed below anyway. + */ tmrinfo("Already running... cancelling\n"); - intel64_oneshot_cancel(oneshot, NULL); + +#ifndef CONFIG_INTEL64_HPET_FSB + INTEL64_TIM_DISABLEINT(oneshot->tch, oneshot->chan); + INTEL64_TIM_SETISR(oneshot->tch, oneshot->chan, NULL, NULL, false); +#endif + + oneshot->running = false; } /* Save the new handler and its argument */