arch/inte64: fix various issues for HPET

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

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 */