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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/02/18 16:22, Jan Beulich wrote:
>>>> On 26.02.18 at 12:35, <andrew.cooper3@xxxxxxxxxx> 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>
> In principle
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> However, ...
>
>> RFC until support is actually committed to binutils mainline.
> ... upstream looks to have switched to .nops now, so the prereq
> to the R-b is that you consistently switch over.

Yes.  I need to pull, rebuild and retest.  Even if this patch gets
committed, it is still semi-RFC and up for changes until the next
binutils release.

That said, given the explicit probe, it is at least safe to have a stale
ABI in use, and we can change it later if needs be.

>
>> --- 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)
> Do you really need the (arbitrary) second argument here?

I want the check to match the form we actually use.

>
>> --- 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
> And do you really need to specify ASM_NOP_MAX here? What's
> wrong with letting the assembler pick what it wants as the longest
> NOP?

I don't want a toolchain change to cause us to go beyond 11 byte nops,
because of the associated decode stall on almost all hardware.  Using
ASM_NOP_MAX seemed like the easiest way to keep the end result
consistent, irrespective of toolchain support.

~Andrew

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