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

[Dave Hansen <dave.hansen@intel.com>]

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.