linux-kernel - Re: [PATCH 1/4] x86/asm: inline constant inputs in rdpkru(), wrpkru()

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] [day] [month] [year] [list]

Message-ID: <be984bc9-33d7-4695-95a1-ec7a3ee50824@intel.com>
Date: Fri, 7 Mar 2025 08:29:52 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: Alexey Dobriyan <adobriyan@...il.com>, tglx@...utronix.de,
 mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com
Cc: x86@...nel.org, hpa@...or.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] x86/asm: inline constant inputs in rdpkru(), wrpkru()

On 3/6/25 22:12, Alexey Dobriyan wrote:
>  static inline u32 rdpkru(void)
>  {
> -	u32 ecx = 0;
>  	u32 edx, pkru;
>  
>  	/*
> @@ -88,20 +87,18 @@ static inline u32 rdpkru(void)
>  	 */
>  	asm volatile(".byte 0x0f,0x01,0xee\n\t"
>  		     : "=a" (pkru), "=d" (edx)
> -		     : "c" (ecx));
> +		     : "c" (0));
>  	return pkru;
>  }

Hey Alexey,

Again, thanks for the patch. I'll explain for a sec why I wrote it this way.

If you're looking at the RDPKRU spec, it literally says "ECX must be 0".
If you see "ecx = 0" in the code, any old dummy can tell that the
_intent_ is to set ecx=0. Anybody that can read C can at least
understand the intent.

But '"c" (0)', on the other hand, requires knowing inline asm and
specifically how x86 inline asm names its registers. It's not utterly
and blatantly obvious that "c" means "ecx".

I don't expect you to totally agree with me on this one. But I don't
think we can take your patch. Two reasons: we can't just be taking
patches for deeply personal style preferences. If we did, the code
history would just be filled with thrash. Second, I kinda like the code
as-is.