Re: [PATCH 1/3] x86: Add definitions for NOP operation

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 26/05/2014 08:27, Feng Wu wrote:
> This patch adds definitions for different length of NOP operation.
>
> Signed-off-by: Feng Wu <feng.wu@intel.com>
> ---
>  xen/include/asm-x86/nops.h | 71 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 xen/include/asm-x86/nops.h
>
> diff --git a/xen/include/asm-x86/nops.h b/xen/include/asm-x86/nops.h
> new file mode 100644
> index 0000000..f38df58
> --- /dev/null
> +++ b/xen/include/asm-x86/nops.h
> @@ -0,0 +1,71 @@
> +#ifndef __X86_ASM_NOPS_H__
> +#define __X86_ASM_NOPS_H__
> +
> +#include <xen/stringify.h>

It appears that we have two competing ways of performing preprocessor
stringification.  __stringify() from stringify.h, and STR() from config.h

As neither are used very much, it would be nice to consolidate down to
one, although I am not fussed which.

> +
> +/*
> + * Define nops for use with alternative().
> + */
> +
> +/*
> + * Opteron 64bit nops
> + * 1: nop
> + * 2: osp nop
> + * 3: osp osp nop
> + * 4: osp osp osp nop
> + */
> +#define K8_NOP1 0x90
> +#define K8_NOP2 0x66,K8_NOP1
> +#define K8_NOP3 0x66,K8_NOP2
> +#define K8_NOP4 0x66,K8_NOP3
> +#define K8_NOP5 K8_NOP3,K8_NOP2
> +#define K8_NOP6 K8_NOP3,K8_NOP3
> +#define K8_NOP7 K8_NOP4,K8_NOP3
> +#define K8_NOP8 K8_NOP4,K8_NOP4
> +
> +/*
> + * P6 nops
> + * uses eax dependencies (Intel-recommended choice)
> + * 1: nop
> + * 2: osp nop
> + * 3: nopl (%eax)
> + * 4: nopl 0x00(%eax)
> + * 5: nopl 0x00(%eax,%eax,1)
> + * 6: osp nopl 0x00(%eax,%eax,1)
> + * 7: nopl 0x00000000(%eax)
> + * 8: nopl 0x00000000(%eax,%eax,1)
> + *    Note: All the above are assumed to be a single instruction.
> + *          There is kernel code that depends on this.
> + */
> +#define P6_NOP1 0x90
> +#define P6_NOP2 0x66,0x90
> +#define P6_NOP3 0x0f,0x1f,0x00
> +#define P6_NOP4 0x0f,0x1f,0x40,0
> +#define P6_NOP5 0x0f,0x1f,0x44,0x00,0
> +#define P6_NOP6 0x66,0x0f,0x1f,0x44,0x00,0
> +#define P6_NOP7 0x0f,0x1f,0x80,0,0,0,0
> +#define P6_NOP8 0x0f,0x1f,0x84,0x00,0,0,0,0
> +
> +#ifdef __ASSEMBLY__
> +#define _ASM_MK_NOP(x) .byte x
> +#else
> +#define _ASM_MK_NOP(x) ".byte " __stringify(x) "\n"
> +#endif
> +
> +#define ASM_NOP1 _ASM_MK_NOP(K8_NOP1)
> +#define ASM_NOP2 _ASM_MK_NOP(K8_NOP2)
> +#define ASM_NOP3 _ASM_MK_NOP(K8_NOP3)
> +#define ASM_NOP4 _ASM_MK_NOP(K8_NOP4)
> +#define ASM_NOP5 _ASM_MK_NOP(K8_NOP5)
> +#define ASM_NOP6 _ASM_MK_NOP(K8_NOP6)
> +#define ASM_NOP7 _ASM_MK_NOP(K8_NOP7)
> +#define ASM_NOP8 _ASM_MK_NOP(K8_NOP8)
> +
> +#define ASM_NOP_MAX 8
> +
> +#ifndef __ASSEMBLY__
> +extern const unsigned char * const *ideal_nops;
> +extern void arch_init_ideal_nops(void);

No need for extern on a function declaration.

~Andrew

> +#endif
> +
> +#endif /* __X86_ASM_NOPS_H__ */