Re: [PATCH v1 05/12] x86, alternatives: Use optimized NOPs for padding

[Andy Lutomirski <luto@amacapital.net>]

On 02/03/2015 10:16 AM, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
>
> Alternatives allow now for empty old instruction. In this case we go
> and pad the space with NOPs at assembly time. However, there are the
> optimal, longer NOPs which should be used. Do that at patching time.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>   arch/x86/kernel/alternative.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 715af37bf008..dd0cdb6b179c 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -323,6 +323,21 @@ done:
>   		n_dspl, (unsigned long)orig_insn + n_dspl + repl_len);
>   }
>
> +static void __init_or_module optimize_nops(u8 *instr, u8 max_len)
> +{
> +	int i = 0;
> +
> +	while (instr[i] == 0x90 && i < max_len)
> +		i++;
> +
> +	if (!i)
> +		return;
> +
> +	add_nops(instr, i);
> +
> +	DUMP_BYTES(instr, i, "%p: optimized NOPs: ", instr);
> +}
> +
>   /*
>    * Replace instructions with better alternatives for this CPU type. This runs
>    * before SMP is initialized to avoid SMP problems with self modifying code.
> @@ -354,8 +369,11 @@ void __init_or_module apply_alternatives(struct alt_instr *start,
>   		replacement = (u8 *)&a->repl_offset + a->repl_offset;
>   		BUG_ON(a->instrlen > sizeof(insnbuf));
>   		BUG_ON(a->cpuid >= (NCAPINTS + NBUGINTS) * 32);
> -		if (!boot_cpu_has(a->cpuid))
> +		if (!boot_cpu_has(a->cpuid)) {
> +			if (instr[0] == 0x90)
> +				optimize_nops(instr, a->instrlen);
>   			continue;
> +		}

I'm a bit confused here.  Shouldn't NOPs after a non-NOP in the old 
instruction also be optimized?

--Andy