linux-kernel - Re: [PATCH] irqchip/riscv-imsic: Start local sync timer on correct CPU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAhSdy2nb0siV-oE8gDQLVdAnMzw2TQqYOSK6SXZqxzYBCqM0Q@mail.gmail.com>
Date: Thu, 15 May 2025 12:09:22 +0530
From: Anup Patel <anup@...infault.org>
To: Andrew Bresticker <abrestic@...osinc.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Palmer Dabbelt <palmer@...belt.com>, 
	Alexandre Ghiti <alex@...ti.fr>, linux-riscv@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] irqchip/riscv-imsic: Start local sync timer on correct CPU

On Wed, May 14, 2025 at 10:43 PM Andrew Bresticker
<abrestic@...osinc.com> wrote:
>
> When starting the local sync timer to synchronize the state of a remote
> CPU it should be added on the CPU to be synchronized, not the initiating
> CPU. This results in interrupt delivery being delayed until the timer
> eventually runs (due to another mask/unmask/migrate operation) on the
> target CPU.
>
> Fixes: 0f67911e821c ("irqchip/riscv-imsic: Separate next and previous pointers in IMSIC vector")
> Signed-off-by: Andrew Bresticker <abrestic@...osinc.com>

My bad. Thanks for catching the issue.

Reviewed-by: Anup Patel <anup@...infault.org>

Regards,
Anup

> ---
>  drivers/irqchip/irq-riscv-imsic-state.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
> index bdf5cd2037f2..62f76950a113 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.c
> +++ b/drivers/irqchip/irq-riscv-imsic-state.c
> @@ -208,17 +208,17 @@ static bool __imsic_local_sync(struct imsic_local_priv *lpriv)
>  }
>
>  #ifdef CONFIG_SMP
> -static void __imsic_local_timer_start(struct imsic_local_priv *lpriv)
> +static void __imsic_local_timer_start(struct imsic_local_priv *lpriv, unsigned int cpu)
>  {
>         lockdep_assert_held(&lpriv->lock);
>
>         if (!timer_pending(&lpriv->timer)) {
>                 lpriv->timer.expires = jiffies + 1;
> -               add_timer_on(&lpriv->timer, smp_processor_id());
> +               add_timer_on(&lpriv->timer, cpu);
>         }
>  }
>  #else
> -static inline void __imsic_local_timer_start(struct imsic_local_priv *lpriv)
> +static inline void __imsic_local_timer_start(struct imsic_local_priv *lpriv, unsigned int cpu)
>  {
>  }
>  #endif
> @@ -233,7 +233,7 @@ void imsic_local_sync_all(bool force_all)
>         if (force_all)
>                 bitmap_fill(lpriv->dirty_bitmap, imsic->global.nr_ids + 1);
>         if (!__imsic_local_sync(lpriv))
> -               __imsic_local_timer_start(lpriv);
> +               __imsic_local_timer_start(lpriv, smp_processor_id());
>
>         raw_spin_unlock_irqrestore(&lpriv->lock, flags);
>  }
> @@ -278,7 +278,7 @@ static void __imsic_remote_sync(struct imsic_local_priv *lpriv, unsigned int cpu
>                                 return;
>                 }
>
> -               __imsic_local_timer_start(lpriv);
> +               __imsic_local_timer_start(lpriv, cpu);
>         }
>  }
>  #else
> --
> 2.43.0
>