linux-kernel - Re: [PATCH 14/20] x86/barrier: Use alternative

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: <20250317200432.1a076d6a@pumpkin>
Date: Mon, 17 Mar 2025 20:04:32 +0000
From: David Laight <david.laight.linux@...il.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, x86@...nel.org,
 linux-kernel@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>,
 Borislav Petkov <bp@...en8.de>, "H. Peter Anvin" <hpa@...or.com>, Uros
 Bizjak <ubizjak@...il.com>, Andrew Cooper <andrew.cooper3@...rix.com>, Ingo
 Molnar <mingo@...nel.org>
Subject: Re: [PATCH 14/20] x86/barrier: Use alternative_io() in 32-bit
 barrier functions

On Fri, 14 Mar 2025 17:05:34 -0700
Josh Poimboeuf <jpoimboe@...nel.org> wrote:

> On Fri, Mar 14, 2025 at 01:49:48PM -1000, Linus Torvalds wrote:
> > So all of these patches look like good cleanups to me, but I do wonder
> > if we should
> > 
> >  (a) not use some naming *quite* as generic as 'ARG()'
> > 
> >  (b) make the asms use ARG_OUT/ARG_IN/ARG_CLOBBER() to clarify
> > 
> > because that ARG(), ARG(), ARGC() pattern looks odd to me.
> > 
> > Maybe it's just me.
> > 
> > Regardless, I do think the series looks like a nice improvement even
> > in the current form, even if that particular repeated pattern feels
> > strange.  
> 
> So originally I had ASM_OUTPUT/ASM_INPUT/ASM_CLOBBER, but I ended up
> going with ARG() due to its nice vertical alignment and conciseness:

But ARG() does look horrid.

Is the ARG() necessary just to handle the comma separated lists?
If so is it only actually needed if there is more than one item?

Another option is to just require () and add the ARG in the expansion.
So with:
#define __asm_call(qual, alt, out, in, clobber) \
	asm("zzz", ARG out, ARG in, ARG clobber)

__asm_call(qual, ALT(), \
		([var] "+m" (__my_cpu_var(_var)), "+a" (old__.low),	\
		    "+d" (old__.high)),					\
		("b" (new__.low), "c" (new__.high), "S" (&(_var))),	\
		("memory"));

would get expanded the same as the line below.

	David
	
> 
> 
> 	__asm_call(qual,						\
> 		ALTERNATIVE("call this_cpu_cmpxchg8b_emu",		\
> 			    "cmpxchg8b " __percpu_arg([var]),		\
> 			    X86_FEATURE_CX8),				\
> 		ARG([var] "+m" (__my_cpu_var(_var)), "+a" (old__.low),	\
> 		    "+d" (old__.high)),					\
> 		ARG("b" (new__.low), "c" (new__.high), "S" (&(_var))),	\
> 		ARG("memory"));						\
> 
> 
> Though ASM_OUTPUT/ASM_INPUT/ASM_CLOBBER isn't so bad either:
> 
> 	__asm_call(qual,						\
> 		ALTERNATIVE("call this_cpu_cmpxchg8b_emu",		\
> 			    "cmpxchg8b " __percpu_arg([var]),		\
> 			    X86_FEATURE_CX8),				\
> 		ASM_OUTPUT([var] "+m" (__my_cpu_var(_var)),		\
> 			   "+a" (old__.low), "+d" (old__.high)),	\
> 		ASM_INPUT("b" (new__.low), "c" (new__.high),		\
> 			  "S" (&(_var))),				\
> 		ASM_CLOBBER("memory"));					\
> 
> 
> That has the nice benefit of being more self-documenting, albeit more
> verbose and less vertically aligned.
> 
> So I could go either way, really.
>