linux-kernel - Re: [External] Re: [PATCH RESEND] irqchip: riscv: 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: <CAPYmKFtx6GNE5r-R33aiHeF-qgFESH0sbsorttjJ88wTL8yr9w@mail.gmail.com>
Date: Tue, 28 Jan 2025 00:37:49 +0800
From: Xu Lu <luxu.kernel@...edance.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: Thomas Gleixner <tglx@...utronix.de>, Anup Patel <anup@...infault.org>, 
	Charlie Jenkins <charlie@...osinc.com>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, lihangjing@...edance.com, 
	Xie Yongji <xieyongji@...edance.com>, linux-riscv@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH RESEND] irqchip: riscv: Order normal writes
 and IPI writes

On Tue, Jan 28, 2025 at 12:23 AM Arnd Bergmann <arnd@...db.de> wrote:
>
> On Mon, Jan 27, 2025, at 10:38, Xu Lu wrote:
> > RISC-V distinguishes between normal memory accesses and device I/O and
> > uses FENCE instruction to order them as viewed by othe RISC-V harts and
> > external devices or coprocessors. The FENCE instruction can order any
> > combination of device input(I), device output(O), memory reads(R) and
> > memory writes(W). For example, 'fence w, o' can be used to ensure all
> > memory writes from instructions preceding the FENCE instruction appear
> > earlier in the global memory order than device output writes from
> > instructions after the FENCE instruction.
>
> There is nothing risc-v specific in here really, it's just a bug
> in the driver: writel() means access the mmio register with appropriate
> barriers, while writel_relaxed() is a special case that should
> only ever be used if a particular function is sensitive to
> performance and never needs to be serialized.
>
> > diff --git a/drivers/irqchip/irq-thead-c900-aclint-sswi.c
> > b/drivers/irqchip/irq-thead-c900-aclint-sswi.c
> > index b0e366ade427..8ff6e7a1363b 100644
> > --- a/drivers/irqchip/irq-thead-c900-aclint-sswi.c
> > +++ b/drivers/irqchip/irq-thead-c900-aclint-sswi.c
> > @@ -31,7 +31,7 @@ static DEFINE_PER_CPU(void __iomem *, sswi_cpu_regs);
> >
> >  static void thead_aclint_sswi_ipi_send(unsigned int cpu)
> >  {
> > -     writel_relaxed(0x1, per_cpu(sswi_cpu_regs, cpu));
> > +     writel(0x1, per_cpu(sswi_cpu_regs, cpu));
> >  }
> >
> >  static void thead_aclint_sswi_ipi_clear(void)
> > --
>
> thead_aclint_sswi_ipi_clear() seems to have the same bug,
> it also uses the _relaxed() version for no apparent reason.

Hi Arnd,

There seems no need to modify thead_aclint_sswi_ipi_clear() as it only
clears pending IPI on current hart. No other harts require to see
strict order between preceding memory writes and this ACLINT MMIO
write. Please correct me if I missed anything.

Thanks,

Xu Lu

>
>      Arnd