[PATCH] irqchip/riscv-imsic: Synchronize remote state with IRQ work

[Anup Patel]

On Tue, Nov 3, 2026 at 1:34 PM Troy Mitchell <troy.mitchell at linux.dev> wrote:
>
> Remote IMSIC state updates are deferred to a timer on the target CPU.
> If that CPU has stopped its local clockevent and is waiting for a
> broadcast interrupt, enabling the broadcast IRQ can depend on delivery
> of that same interrupt. This can stall timer delivery when the broadcast
> IRQ is unmasked or moved to another CPU, including during CPU hotplug.
>
> Queue hard IRQ work to synchronize remote state independently of timer
> delivery. Keep synchronous updates on the local CPU and the timer retry
> for an unfinished vector move. Hard IRQ work also avoids depending on a
> schedulable worker when PREEMPT_RT is enabled.

Using timer retry for local CPU and IRQ work queue for remote CPUs looks
strange. Why not use IRQ work queue even for local CPU ??

>
> Fixes: 21a8f8a0eb35 ("irqchip: Add RISC-V incoming MSI controller early driver")
> Signed-off-by: Troy Mitchell <troy.mitchell at linux.dev>
> ---
> This was found while bringing up the SoC timer on SpacemiT K3 as a tick
> broadcast device.
>
> Without a real broadcast device, the tick code falls back to the hrtimer
> based broadcast, which keeps one CPU's local timer running. Once the SoC
> timer is registered it replaces the hrtimer broadcast, and every CPU may
> stop its local timer in idle.
>
> On K3 the SoC timer interrupt goes through APLIC in MSI mode to IMSIC.
> When the CPU owning it goes offline, the interrupt is moved to a CPU
> that may already be idle with its local timer stopped. IMSIC leaves the
> enable on the new target to a timer on that CPU, and that timer can only
> fire through the broadcast interrupt, which is not enabled yet. The
> system stalls.
>
> Tested on a K3 COM260 board by offlining the CPU owning the broadcast
> interrupt, and then each CPU in turn, under a timerfd load:
> - v7.4-rc1 with the SpacemiT SoC timer series and local timers forced
>   to stop in idle
> - a vendor 6.18 kernel with SBI cpuidle states that stop the local timer
>
> Both stall without this patch and pass with it.
> ---
>  drivers/irqchip/Kconfig                 |  1 +
>  drivers/irqchip/irq-riscv-imsic-state.c | 19 +++++++++++++------
>  drivers/irqchip/irq-riscv-imsic-state.h |  4 ++++
>  3 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 136bab41f801..5cdc3f52198c 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -651,6 +651,7 @@ config RISCV_IMSIC
>         bool
>         depends on RISCV
>         select IRQ_DOMAIN_HIERARCHY
> +       select IRQ_WORK if SMP
>         select GENERIC_IRQ_MATRIX_ALLOCATOR
>         select GENERIC_MSI_IRQ
>         select IRQ_MSI_LIB
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
> index 01620373b43a..f6373e4019a2 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.c
> +++ b/drivers/irqchip/irq-riscv-imsic-state.c
> @@ -252,6 +252,11 @@ void imsic_local_delivery(bool enable)
>  }
>
>  #ifdef CONFIG_SMP
> +static void imsic_local_sync_work(struct irq_work *work)
> +{
> +       imsic_local_sync_all(false);
> +}
> +
>  static void imsic_local_timer_callback(struct timer_list *timer)
>  {
>         imsic_local_sync_all(false);
> @@ -268,18 +273,19 @@ static void __imsic_remote_sync(struct imsic_local_priv *lpriv, unsigned int cpu
>          */
>
>         /*
> -        * We schedule a timer on the target CPU if the target CPU is not
> -        * same as the current CPU. An offline CPU will unconditionally
> -        * synchronize IDs through imsic_starting_cpu() when the
> -        * CPU is brought up.
> +        * A remote CPU may have stopped its local timer and be waiting for
> +        * the broadcast interrupt whose IMSIC state needs updating. Use an
> +        * IPI-backed work item so enabling that interrupt does not depend on
> +        * its delivery. An offline CPU synchronizes IDs in imsic_starting_cpu().
>          */
>         if (cpu_online(cpu)) {
>                 if (cpu == smp_processor_id()) {
>                         if (__imsic_local_sync(lpriv))
>                                 return;
> +                       __imsic_local_timer_start(lpriv, cpu);
> +               } else {
> +                       irq_work_queue_on(&lpriv->sync_work, cpu);
>                 }
> -
> -               __imsic_local_timer_start(lpriv, cpu);
>         }
>  }
>  #else
> @@ -510,6 +516,7 @@ static int __init imsic_local_init(void)
>  #ifdef CONFIG_SMP
>                 /* Setup lazy timer for synchronization */
>                 timer_setup(&lpriv->timer, imsic_local_timer_callback, TIMER_PINNED);
> +               lpriv->sync_work = IRQ_WORK_INIT_HARD(imsic_local_sync_work);
>  #endif
>
>                 /* Allocate vector array */
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.h b/drivers/irqchip/irq-riscv-imsic-state.h
> index 189829b8673a..a36b9f0ef840 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.h
> +++ b/drivers/irqchip/irq-riscv-imsic-state.h
> @@ -9,6 +9,7 @@
>
>  #include <linux/irqchip/riscv-imsic.h>
>  #include <linux/irqdomain.h>
> +#include <linux/irq_work.h>
>  #include <linux/fwnode.h>
>  #include <linux/timer.h>
>
> @@ -34,6 +35,9 @@ struct imsic_local_priv {
>         unsigned long                           *dirty_bitmap;
>
>  #ifdef CONFIG_SMP
> +       /* Remote synchronization must not depend on a timer interrupt. */
> +       struct irq_work                         sync_work;
> +
>         /* Local timer for synchronization */
>         struct timer_list                       timer;
>  #endif
>
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20261103-imsic-remote-sync-irqwork-9cfec6c410de
>
> Best regards,
> --
> Troy Mitchell <troy.mitchell at linux.dev>
>

Regards,
Anup