linux-kernel - Re: [PATCH v2 09/12] x86/msr: Use the alternatives mechanism for WRMSR

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250930085044.GK3245006@noisy.programming.kicks-ass.net>
Date: Tue, 30 Sep 2025 10:50:44 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Jürgen Groß <jgross@...e.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org, llvm@...ts.linux.dev,
	xin@...or.com, "H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Nathan Chancellor <nathan@...nel.org>,
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>
Subject: Re: [PATCH v2 09/12] x86/msr: Use the alternatives mechanism for
 WRMSR

On Tue, Sep 30, 2025 at 10:46:23AM +0200, Jürgen Groß wrote:
> On 30.09.25 10:31, Peter Zijlstra wrote:
> > On Tue, Sep 30, 2025 at 09:03:53AM +0200, Juergen Gross wrote:
> > 
> > > +static __always_inline bool __wrmsrq_constant(u32 msr, u64 val, int type)
> > > +{
> > > +	BUILD_BUG_ON(!__builtin_constant_p(msr));
> > > +
> > > +	asm_inline volatile goto(
> > > +		"1:\n"
> > > +		ALTERNATIVE_2(PREPARE_RCX_RDX_FOR_WRMSR
> > > +			      "2: ds wrmsr",
> > > +			      PREPARE_RCX_RDX_FOR_WRMSR
> > > +			      ASM_WRMSRNS,
> > > +			      X86_FEATURE_WRMSRNS,
> > > +			      ASM_WRMSRNS_IMM,
> > > +			      X86_FEATURE_MSR_IMM)
> > > +		_ASM_EXTABLE_TYPE(1b, %l[badmsr], %c[type])	/* For WRMSRNS immediate */
> > > +		_ASM_EXTABLE_TYPE(2b, %l[badmsr], %c[type])	/* For WRMSR(NS) */
> > > +
> > > +		:
> > > +		: [val] "a" (val), [msr] "i" (msr), [type] "i" (type)
> > > +		: "memory", "ecx", "rdx"
> > > +		: badmsr);
> > > +
> > > +	return false;
> > > +
> > > +badmsr:
> > > +	return true;
> > > +}
> > 
> > Just wondering, would something this work?
> > 
> > 	asm_inline volatile goto(
> > 		"1:\n"
> > 		ALTERNATIVE(PREPARE_RCX_RDX_FOR_WRMSR
> > 			    "2:\n"
> > 			    ALTERNATIVE("ds wrmsr",
> > 			                ASM_WRMSRNS, X86_FEATURE_WRMSRNS),
> > 			    ASM_WRMSRNS_IMM, X86_FEATURE_MSR_IMM);
> > 		_ASM_EXTABLE_TYPE(1b, %l[badmsr], %c[type])	/* For WRMSRNS immediate */
> > 		_ASM_EXTABLE_TYPE(2b, %l[badmsr], %c[type])	/* For WRMSR(NS) */
> > 
> > 		:
> > 		: [val] "a" (val), [msr] "i" (msr), [type] "i" (type)
> > 		: "memory", "ecx", "rdx"
> > 		: badmsr);
> > 
> > Its a bit weird because the nested alternative isn't for the exact same
> > position I suppose. But I find it a more readable form.
> 
> I don't think it would work. Nested ALTERNATIVE()s do work only with
> all of them starting at the same location. Have a look at the
> ALTERNATIVE() macro, which is defining the label "771" via OLDINSTR()
> and then referring to this label via ALTINSTR_ENTRY(). In your case
> the ALTINSTR_ENTRY() of the outer ALTERNATIVE() invocation would find
> the wrong "771" label (the one of the inner ALTERNATIVE()).
> 
> Allowing such constructs would probably require switching from preprocessor
> macros to assembler macros.

Right, I was looking at the asm macros. 

As long as the inner comes first in the apply list it should all just
work, except you get the double patching back.

Using the asm macros isn't going to make it more readable though. 
Oh well, lets forget about this :-)