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

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

On 15/01/18 10:14, Jan Beulich wrote:
>>>> On 12.01.18 at 19:00, <andrew.cooper3@citrix.com> 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 <asm/asm_defns.h>
>> +
>> +.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