linux-kernel - 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: <a7d9b0e5-7ef6-457e-ace6-c56e745c3150@app.fastmail.com>
Date: Mon, 27 Jan 2025 17:21:46 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Xu Lu" <luxu.kernel@...edance.com>,
 "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>
Cc: lihangjing@...edance.com, "Xie Yongji" <xieyongji@...edance.com>,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND] irqchip: riscv: Order normal writes and IPI writes

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.

     Arnd