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

["Jürgen Groß" <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 1670 bytes --]

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 #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]