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

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>, Jan Beulich <JBeulich@suse.com>,
	Xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH v2 7/7] x86/build: Use new .nop directive when available
Date: Mon, 26 Feb 2018 13:08:05 +0000	[thread overview]
Message-ID: <defe2950-f766-a077-2473-d68debb209b8@citrix.com> (raw)
In-Reply-To: <20180226123159.5cvidg3glrqe2syy@MacBook-Pro-de-Roger.local>

On 26/02/18 12:31, Roger Pau Monné wrote:
> On Mon, Feb 26, 2018 at 11:35:04AM +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@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>>
>> RFC until support is actually committed to binutils mainline.
> Since RFC has been dropped from the subject, is this now committed?

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=62a02d25b6e5d9f92c205260daa11355d0c62532

>
>> ---
>>  xen/arch/x86/Rules.mk                 |  4 ++++
>>  xen/include/asm-x86/alternative-asm.h | 14 ++++++++++++--
>>  xen/include/asm-x86/alternative.h     | 13 ++++++++++---
>>  3 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
>> index e169d67..bf5047f 100644
>> --- a/xen/arch/x86/Rules.mk
>> +++ b/xen/arch/x86/Rules.mk
>> @@ -28,6 +28,10 @@ $(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \
>>  $(call as-option-add,CFLAGS,CC,\
>>      ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE)
>>  
>> +# Check to see whether the assmbler supports the .nop directive.
>> +$(call as-option-add,CFLAGS,CC,\
>> +    ".L1: .L2: .nop (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE)
>> +
>>  CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
>>  
>>  # Xen doesn't use SSE interally.  If the compiler supports it, also skip the
>> diff --git a/xen/include/asm-x86/alternative-asm.h b/xen/include/asm-x86/alternative-asm.h
>> index 25f79fe..9e46bed 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_AS_NOP_DIRECTIVE
>> +    .nop \nr_bytes, ASM_NOP_MAX
> I'm not able to find any online document about the .nop directive, and
> I cannot really figure out the purpose of the second parameter.

Its a bit woolly, named "control".  In practice, it is the maximum
length of an individual nop.  Beyond 11 bytes (iirc), most pipelines
take a decode stall.

>
> Also, after this patch is applied it seems like the padding is not
> going to be 0x90, because as will already emit optimized nops. Are
> those nops more optimized than the ones added by the alternatives
> framework?

They are the same nops (by and large), although arranged differently. 
For example, GAS fills backwards rather than forwards, which is as
recommended in the Intel ORM.

> I would expect the nops added at runtime would be more optimized than
> the ones added at build time, because the runtime ones could take into
> account the specific CPU model Xen is running on.

There are only a very few 64-bit capable CPUs which prefer K8 nops over
P6 nops, and they are all old.  The build-time nops are correct for the
overwhelming majority of hardware.

~Andrew

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