Re: [PATCH v2 03/10] x86: Add a type field to alt_instr

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Tue, Jan 16, 2018 at 03:28:28PM +0100, Peter Zijlstra wrote:
> Add a type field to the alternative description. For now this will be
> used to annotate static_cpu_has() but possible future uses include
> using it to implement alternative alignment and special NOP handling.
> 
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/include/asm/alternative-asm.h |    3 ++-
>  arch/x86/include/asm/alternative.h     |    6 +++++-
>  arch/x86/include/asm/cpufeature.h      |    2 ++
>  tools/objtool/special.c                |    2 +-
>  4 files changed, 10 insertions(+), 3 deletions(-)
> 
> --- a/arch/x86/include/asm/alternative-asm.h
> +++ b/arch/x86/include/asm/alternative-asm.h
> @@ -25,13 +25,14 @@
>   * enough information for the alternatives patching code to patch an
>   * instruction. See apply_alternatives().
>   */
> -.macro altinstruction_entry orig alt feature orig_len alt_len pad_len
> +.macro altinstruction_entry orig alt feature orig_len alt_len pad_len type=0
>  	.long \orig - .
>  	.long \alt - .
>  	.word \feature
>  	.byte \orig_len
>  	.byte \alt_len
>  	.byte \pad_len
> +	.byte \type
>  .endm
>  
>  /*
> --- a/arch/x86/include/asm/alternative.h
> +++ b/arch/x86/include/asm/alternative.h
> @@ -45,6 +45,8 @@
>  #define LOCK_PREFIX ""
>  #endif
>  
> +#define ALT_TYPE_DEFAULT	0
> +
>  struct alt_instr {
>  	s32 instr_offset;	/* original instruction */
>  	s32 repl_offset;	/* offset to replacement instruction */
> @@ -52,6 +54,7 @@ struct alt_instr {
>  	u8  instrlen;		/* length of original instruction */
>  	u8  replacementlen;	/* length of new instruction */
>  	u8  padlen;		/* length of build-time padding */
> +	u8  type;		/* type of alternative */
>  } __packed;

Should we put a comment above the struct with a reminder to also update
objtool ALT_ENTRY_SIZE (and possibly the ALT_*_OFFSET macros) when they
change the struct?

Unfortunately it's against policy to include kernel headers from the
tools subdirectory, so we generally have to either hard code things like
this, or have a duplicated header file which needs to be kept in sync.

-- 
Josh