Re: [Xen-devel] [PATCH v8 01/17] x86: Support compiling with indirect branch thunks

2018-01-15 Thread Jan Beulich 
>>> On 15.01.18 at 11:40,  wrote:
> On 15/01/18 10:14, Jan Beulich wrote:
> On 12.01.18 at 19:00,  wrote:
>>> --- a/xen/arch/x86/Rules.mk
>>> +++ b/xen/arch/x86/Rules.mk
>>> @@ -30,3 +30,10 @@ CFLAGS += -fno-asynchronous-unwind-tables
>>>  ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
>>>  CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
>>>  endif
>>> +
>>> +# Compile with thunk-extern, indirect-branch-register if avaiable.
>>> +ifneq ($(call cc-option,$(CC),-mindirect-branch-register,n),n)
>>> +CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
>>> +CFLAGS += -DCONFIG_INDIRECT_THUNK
>>> +export CONFIG_INDIRECT_THUNK=y
>>> +endif
>> Still not a proper config option?
> 
> No, for backportability.
> 
> I'm preparing a post-SP2 series with some cleanup for staging.
> 
>>
>>> --- /dev/null
>>> +++ b/xen/arch/x86/indirect-thunk.S
>>> @@ -0,0 +1,38 @@
>>> +/*
>>> + * Implement __x86_indirect_thunk_* symbols for use with compatbile 
> compilers
>>> + * and the -mindirect-branch=thunk-extern -mindirect-branch-register 
> options.
>>> + *
>>> + * Copyright (c) 2017-2018 Citrix Systems Ltd.
>>> + *
>>> + * This source code is licensed under the GNU General Public License,
>>> + * Version 2.  See the file COPYING for more details.
>>> + */
>>> +.file __FILE__
>>> +
>>> +#include 
>>> +
>>> +.macro IND_THUNK_RETPOLINE reg:req
>>> +call 2f
>>> +1:
>>> +lfence
>>> +jmp 1b
>>> +2:
>> As noted in a couple of other places, I'd prefer if numeric labels
>> weren't used in macros (and especially new ones), in favor of
>> .L ones utilizing \@.
> 
> For macros in header files, I can see this rational.
> 
> However, this is a local macro which only gets expanded in this file,
> and isn't likely to move elsewhere.  I don't see the value in reducing
> the readability.
> 
>>
>>> +mov %\reg, (%rsp)
>>> +ret
>>> +.endm
>>> +
>>> +/*
>>> + * Build the __x86_indirect_thunk_* symbols.  Currently implement the
>>> + * retpoline thunk only.
>>> + */
>>> +.macro GEN_INDIRECT_THUNK reg:req
>>> +.section .text.__x86_indirect_thunk_\reg, "ax", @progbits
>>> +
>>> +ENTRY(__x86_indirect_thunk_\reg)
>>> +IND_THUNK_RETPOLINE \reg
>>> +.endm
>> Still unnecessary leading underscores in the section name?
> 
> Having the section names different to the symbols in them is far worse
> than using double underscores.

Well, okay then:
Acked-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH v8 01/17] x86: Support compiling with indirect branch thunks

2018-01-15 Thread Andrew Cooper 
On 15/01/18 10:14, Jan Beulich wrote:
 On 12.01.18 at 19:00,  wrote:
>> --- a/xen/arch/x86/Rules.mk
>> +++ b/xen/arch/x86/Rules.mk
>> @@ -30,3 +30,10 @@ CFLAGS += -fno-asynchronous-unwind-tables
>>  ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
>>  CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
>>  endif
>> +
>> +# Compile with thunk-extern, indirect-branch-register if avaiable.
>> +ifneq ($(call cc-option,$(CC),-mindirect-branch-register,n),n)
>> +CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
>> +CFLAGS += -DCONFIG_INDIRECT_THUNK
>> +export CONFIG_INDIRECT_THUNK=y
>> +endif
> Still not a proper config option?

No, for backportability.

I'm preparing a post-SP2 series with some cleanup for staging.

>
>> --- /dev/null
>> +++ b/xen/arch/x86/indirect-thunk.S
>> @@ -0,0 +1,38 @@
>> +/*
>> + * Implement __x86_indirect_thunk_* symbols for use with compatbile 
>> compilers
>> + * and the -mindirect-branch=thunk-extern -mindirect-branch-register 
>> options.
>> + *
>> + * Copyright (c) 2017-2018 Citrix Systems Ltd.
>> + *
>> + * This source code is licensed under the GNU General Public License,
>> + * Version 2.  See the file COPYING for more details.
>> + */
>> +.file __FILE__
>> +
>> +#include 
>> +
>> +.macro IND_THUNK_RETPOLINE reg:req
>> +call 2f
>> +1:
>> +lfence
>> +jmp 1b
>> +2:
> As noted in a couple of other places, I'd prefer if numeric labels
> weren't used in macros (and especially new ones), in favor of
> .L ones utilizing \@.

For macros in header files, I can see this rational.

However, this is a local macro which only gets expanded in this file,
and isn't likely to move elsewhere.  I don't see the value in reducing
the readability.

>
>> +mov %\reg, (%rsp)
>> +ret
>> +.endm
>> +
>> +/*
>> + * Build the __x86_indirect_thunk_* symbols.  Currently implement the
>> + * retpoline thunk only.
>> + */
>> +.macro GEN_INDIRECT_THUNK reg:req
>> +.section .text.__x86_indirect_thunk_\reg, "ax", @progbits
>> +
>> +ENTRY(__x86_indirect_thunk_\reg)
>> +IND_THUNK_RETPOLINE \reg
>> +.endm
> Still unnecessary leading underscores in the section name?

Having the section names different to the symbols in them is far worse
than using double underscores.

~Andrew

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

Re: [Xen-devel] [PATCH v8 01/17] x86: Support compiling with indirect branch thunks

2018-01-14 Thread Andrew Cooper 
On 14/01/2018 19:48, David Woodhouse wrote:
> On Fri, 2018-01-12 at 18:00 +, Andrew Cooper wrote:
>> +.macro IND_THUNK_RETPOLINE reg:req
>> +    call 2f
>> +1:
> Linux and GCC have now settled on 'pause;lfence;jmp' here.

And elsewhere.  It is probably worth taking, but it takes us to 17 bytes
which is frustratingly over one cache line.

~Andrew

>
>> +    lfence
>> +    jmp 1b
>> +2:
>> +    mov %\reg, (%rsp)
>> +    ret
>> +.endm
>> +


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

Re: [Xen-devel] [PATCH v8 01/17] x86: Support compiling with indirect branch thunks

2018-01-14 Thread David Woodhouse 
On Fri, 2018-01-12 at 18:00 +, Andrew Cooper wrote:
> 
> +.macro IND_THUNK_RETPOLINE reg:req
> +    call 2f
> +1:

Linux and GCC have now settled on 'pause;lfence;jmp' here.

> +    lfence
> +    jmp 1b
> +2:
> +    mov %\reg, (%rsp)
> +    ret
> +.endm
> +

smime.p7s
Description: S/MIME cryptographic signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel