linux-kernel - Re: [External] Re: [PATCH 3/5] irqchip/riscv-imsic: Use wmb() to order normal writes and IPI writes

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: <CAAhSdy3edVEus88NiTvxQDZqstsCxwGjO-H4XThZcFxCcPGqZA@mail.gmail.com>
Date: Tue, 14 Jan 2025 14:28:43 +0530
From: Anup Patel <anup@...infault.org>
To: Xu Lu <luxu.kernel@...edance.com>
Cc: daniel.lezcano@...aro.org, tglx@...utronix.de, paul.walmsley@...ive.com, 
	palmer@...belt.com, lihangjing@...edance.com, xieyongji@...edance.com, 
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH 3/5] irqchip/riscv-imsic: Use wmb() to
 order normal writes and IPI writes

On Tue, Jan 14, 2025 at 12:09 PM Xu Lu <luxu.kernel@...edance.com> wrote:
>
> Hi Anup,
>
> I want to ensure when the receiver CPU traps into interrupt, it can
> see all memory updates made by the sender before.
>
> The smp_mb__after_atomic() uses 'fence rw, rw' which orders normal
> memory reads/writes. The IMSIC write is actually IO writes and is not
> ordered by it.
>
> So I worry in extreme cases, the receiver CPU can receive the IPI
> first, see no updates to ipi_mux_pcpu->bits and return immediately.
> Then this IPI will not be handled until another IPI comes.

Using wmb() is still inappropriate because if you want to simply
ensure IO writes are not ordered before the normal memory writes
then just convert writel_relaxed() to write().

Regards,
Anup

>
> Best Regards,
>
> Xu Lu
>
> On Tue, Jan 14, 2025 at 12:34 PM Anup Patel <anup@...infault.org> wrote:
> >
> > On Mon, Jan 13, 2025 at 8:39 PM Xu Lu <luxu.kernel@...edance.com> wrote:
> > >
> > > During an IPI procedure, we need to ensure all previous write operations
> > > are visible to other CPUs before sending a real IPI. We use wmb() barrier
> > > to ensure this as IMSIC issues IPI via mmio writes.
> > >
> > > Signed-off-by: Xu Lu <luxu.kernel@...edance.com>
> > > ---
> > >  drivers/irqchip/irq-riscv-imsic-early.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/irqchip/irq-riscv-imsic-early.c b/drivers/irqchip/irq-riscv-imsic-early.c
> > > index 63097f2bbadf..c6317cb557fb 100644
> > > --- a/drivers/irqchip/irq-riscv-imsic-early.c
> > > +++ b/drivers/irqchip/irq-riscv-imsic-early.c
> > > @@ -29,6 +29,12 @@ static void imsic_ipi_send(unsigned int cpu)
> > >  {
> > >         struct imsic_local_config *local = per_cpu_ptr(imsic->global.local, cpu);
> > >
> > > +       /*
> > > +        * Ensure that stores to normal memory are visible to the other CPUs
> > > +        * before issuing IPI.
> > > +        */
> > > +       wmb();
> > > +
> >
> > The imsic_ipi_send() is called through ipi_mux_send_mask()
> > which does smp_mb__after_atomic() before calling so no need
> > for any barrier here. Also, barriers need to be in-pair so adding
> > a single barrier at random location is inappropriate.
> > (Refer, https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/irq/ipi-mux.c?h=v6.13-rc7#n78)
> >
> > Based on the above, this patch is not needed.
> >
> > Regards,
> > Anup