linux-kernel - Re: [PATCH v4 19/26] arm64: Add support for GICv5 GSB barriers

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: <aDce4TGxUoGGONA3@lpieralisi>
Date: Wed, 28 May 2025 16:34:09 +0200
From: Lorenzo Pieralisi <lpieralisi@...nel.org>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: Marc Zyngier <maz@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, Arnd Bergmann <arnd@...db.de>,
	Sascha Bischoff <sascha.bischoff@....com>,
	Timothy Hayes <timothy.hayes@....com>,
	"Liam R. Howlett" <Liam.Howlett@...cle.com>,
	Mark Rutland <mark.rutland@....com>,
	Jiri Slaby <jirislaby@...nel.org>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH v4 19/26] arm64: Add support for GICv5 GSB barriers

On Wed, May 28, 2025 at 02:17:30PM +0100, Jonathan Cameron wrote:
> On Tue, 13 May 2025 19:48:12 +0200
> Lorenzo Pieralisi <lpieralisi@...nel.org> wrote:
> 
> > The GICv5 architecture introduces two barriers instructions
> > (GSB SYS, GSB ACK) that are used to manage interrupt effects.
> > 
> > Rework macro used to emit the SB barrier instruction and implement
> > the GSB barriers on top of it.
> > 
> > Suggested-by: Marc Zyngier <maz@...nel.org>
> > Signed-off-by: Lorenzo Pieralisi <lpieralisi@...nel.org>
> > Cc: Will Deacon <will@...nel.org>
> > Cc: Catalin Marinas <catalin.marinas@....com>
> > Cc: Marc Zyngier <maz@...nel.org>
> > ---
> >  arch/arm64/include/asm/barrier.h |  3 +++
> >  arch/arm64/include/asm/sysreg.h  | 10 +++++++---
> >  2 files changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> > index 1ca947d5c93963d33fe8fb02d6037fc71bd9fd7a..f5801b0ba9e9e7e0433f16ffedf0ec7dfb3e358e 100644
> > --- a/arch/arm64/include/asm/barrier.h
> > +++ b/arch/arm64/include/asm/barrier.h
> > @@ -44,6 +44,9 @@
> >  						 SB_BARRIER_INSN"nop\n",	\
> >  						 ARM64_HAS_SB))
> >  
> > +#define gsb_ack()	asm volatile(GSB_ACK_BARRIER_INSN : : : "memory")
> > +#define gsb_sys()	asm volatile(GSB_SYS_BARRIER_INSN : : : "memory")
> > +
> >  #ifdef CONFIG_ARM64_PSEUDO_NMI
> >  #define pmr_sync()						\
> >  	do {							\
> > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> > index 2639d3633073de10f5040a7efff059021f847530..e7734f90bb723bfbd8be99f16dd6d6fdc7fa57e8 100644
> > --- a/arch/arm64/include/asm/sysreg.h
> > +++ b/arch/arm64/include/asm/sysreg.h
> > @@ -112,10 +112,14 @@
> >  /* Register-based PAN access, for save/restore purposes */
> >  #define SYS_PSTATE_PAN			sys_reg(3, 0, 4, 2, 3)
> >  
> > -#define __SYS_BARRIER_INSN(CRm, op2, Rt) \
> > -	__emit_inst(0xd5000000 | sys_insn(0, 3, 3, (CRm), (op2)) | ((Rt) & 0x1f))
> > +#define __SYS_BARRIER_INSN(op0, op1, CRn, CRm, op2, Rt)		\
> > +	__emit_inst(0xd5000000				|	\
> > +	sys_insn((op0), (op1), (CRn), (CRm), (op2))	|	\
> > +	((Rt) & 0x1f))
> 
> Perhaps indent as something like the following for readbility?
> #define __SYS_BARRIER_INSN(op0, op1, CRn, CRm, op2, Rt)			\
> 	__emit_inst(0xd5000000 |					\
> 		    sys_insn((op0), (op1), (CRn), (CRm), (op2)) |	\
> 		    ((Rt) & 0x1f))
> 

I can do - even though readability is subjective, this looks nicer to
me but if possible I'd avoid the churn required if I change it and then
it is not readable for other people.

Noted.

Thanks,
Lorenzo

> >  
> > -#define SB_BARRIER_INSN			__SYS_BARRIER_INSN(0, 7, 31)
> > +#define SB_BARRIER_INSN			__SYS_BARRIER_INSN(0, 3, 3, 0, 7, 31)
> > +#define GSB_SYS_BARRIER_INSN		__SYS_BARRIER_INSN(1, 0, 12, 0, 0, 31)
> > +#define GSB_ACK_BARRIER_INSN		__SYS_BARRIER_INSN(1, 0, 12, 0, 1, 31)
> >  
> >  #define SYS_DC_ISW			sys_insn(1, 0, 7, 6, 2)
> >  #define SYS_DC_IGSW			sys_insn(1, 0, 7, 6, 4)
> > 
>