Re: [PATCH 3/3] x86/alternative: Patch a single alternative location only once

[Jürgen Groß]

On 30.09.25 09:39, Peter Zijlstra wrote:
> On Tue, Sep 30, 2025 at 09:33:25AM +0200, Jürgen Groß wrote:
> > On 30.09.25 09:26, Peter Zijlstra wrote:
>
> > > > @@ -692,14 +698,19 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start,
> > > >    		 * - feature not present but ALT_FLAG_NOT is set to mean,
> > > >    		 *   patch if feature is *NOT* present.
> > > >    		 */
> > > > -		if (!boot_cpu_has(a->cpuid) == !(a->flags & ALT_FLAG_NOT)) {
> > > > -			memcpy(insn_buff, instr, a->instrlen);
> > > > -			optimize_nops(instr, insn_buff, a->instrlen);
> > > > -		} else {
> > > > +		if (!boot_cpu_has(a->cpuid) != !(a->flags & ALT_FLAG_NOT)) {
> > > >    			apply_one_alternative(instr, insn_buff, a);
> > > > +			patched = true;
> > > >    		}
> > > > -		text_poke_early(instr, insn_buff, a->instrlen);
> > > > +		instances--;
> > > > +		if (!instances) {
> > > > +			if (!patched) {
> > > > +				memcpy(insn_buff, instr, a->instrlen);
> > > > +				optimize_nops(instr, insn_buff, a->instrlen);
> > > > +			}
> > > > +			text_poke_early(instr, insn_buff, a->instrlen);
> > > > +		}
> > > >    	}
> > > >    	kasan_enable_current();
> > >
> > > I think you lost the optimize_nops() call for the patched case.
> > >
> > > That is, note how apply_one_alternative() does 0x90 padding, but then
> > > you only do optimize_nops() when !patched.
> >
> > The call of optimize_nops() is part of text_poke_apply_relocation() when
> > patching, like without my series.
>
> Indeed it is. Clearly I need more wakeup juice :-)
>
> Therefore the patches seem fine. I'll try and hold onto them until the
> merge window closes and then stick them in x86/core or thereabout.

Thanks.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature