Re: [Xen-devel] [PATCH 7/7] x86/build: Use new .nop directive when available

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Feb 12, 2018 at 11:23:07AM +0000, Andrew Cooper wrote:
> Newer versions of binutils are capable of emitting an exact number bytes worth
> of optimised nops.  Use this in preference to .skip when available.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
> RFC until support is actually committed to binutils mainline.

Do you think you could reference the documentation? I can guess what
the arguments are, but would be better if I can have a reference.

> ---
>  xen/arch/x86/Rules.mk                 |  1 +
>  xen/include/asm-x86/alternative-asm.h | 16 +++++++++++++---
>  xen/include/asm-x86/alternative.h     | 17 ++++++++++++-----
>  3 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
> index 56b2ea8..c3b726c 100644
> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -20,6 +20,7 @@ $(call as-insn-check,CFLAGS,CC,"invept 
> (%rax)$$(comma)%rax",-DHAVE_GAS_EPT)
>  $(call as-insn-check,CFLAGS,CC,"rdrand %eax",-DHAVE_GAS_RDRAND)
>  $(call as-insn-check,CFLAGS,CC,"rdfsbase %rax",-DHAVE_GAS_FSGSBASE)
>  $(call as-insn-check,CFLAGS,CC,"rdseed %eax",-DHAVE_GAS_RDSEED)
> +$(call as-insn-check,CFLAGS,CC,".nop 0$$(comma)9",-DHAVE_GAS_LONG_NOPS)

Since .nop is going to be used with labels, might be better to have a
test like:

.L1:
.L2:
.nop (.L2 - .L1),9

At least with clang assembler some expressions used in directives
allow labels, while others not (ie: .skip and .fill allow labels,
.rept doesn't)

>  $(call as-insn-check,CFLAGS,CC,".equ \"x\"$$(comma)1", \
>                       -U__OBJECT_LABEL__ -DHAVE_GAS_QUOTED_SYM \
>                       '-D__OBJECT_LABEL__=$(subst 
> $(BASEDIR)/,,$(CURDIR))/$$@')
> diff --git a/xen/include/asm-x86/alternative-asm.h 
> b/xen/include/asm-x86/alternative-asm.h
> index f7e37cb..a4893e7 100644
> --- a/xen/include/asm-x86/alternative-asm.h
> +++ b/xen/include/asm-x86/alternative-asm.h
> @@ -1,6 +1,8 @@
>  #ifndef _ASM_X86_ALTERNATIVE_ASM_H_
>  #define _ASM_X86_ALTERNATIVE_ASM_H_
>  
> +#include <asm/nops.h>
> +
>  #ifdef __ASSEMBLY__
>  
>  /*
> @@ -18,6 +20,14 @@
>      .byte \pad_len
>  .endm
>  
> +.macro mknops nr_bytes
> +#ifdef HAVE_GAS_LONG_NOPS
> +    .nop \nr_bytes, ASM_NOP_MAX
> +#else
> +    .skip \nr_bytes, 0x90
> +#endif
> +.endm

Out of curiosity, is there any reason this is an assembly macro
instead of a define?

Easier to integrate with existing code?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel