Re: [PATCH v2 06/70] x86: Introduce support for CET-IBT

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.02.2022 13:50, Andrew Cooper wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -39,6 +39,11 @@ config HAS_AS_CET_SS
>       # binutils >= 2.29 or LLVM >= 6
>       def_bool $(as-instr,wrssq %rax$(comma)0;setssbsy)
>  
> +config HAS_CC_CET_IBT
> +     # GCC >= 9 and binutils >= 2.29
> +     # Retpoline check to work around 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93654
> +     def_bool $(cc-option,-fcf-protection=branch -mmanual-endbr 
> -mindirect-branch=thunk-extern) && $(as-instr,endbr64)

At the top of asm-defns.h we have a number of similarly operand-less
instructions expressed via .macro expanding to .byte. I don't see why
we couldn't do so here as well, eliminating the need for the
$(as-instr ...). In fact ...

> --- a/xen/arch/x86/include/asm/asm-defns.h
> +++ b/xen/arch/x86/include/asm/asm-defns.h
> @@ -57,6 +57,12 @@
>      INDIRECT_BRANCH jmp \arg
>  .endm
>  
> +#ifdef CONFIG_XEN_IBT
> +# define ENDBR64 endbr64
> +#else
> +# define ENDBR64
> +#endif

... it could also be this macro which ends up conditionally empty,
but would then want expressing as an assembler macro. Albeit no, the
lower case form would probably still be needed to deal with compiler
emitted insns, as the compiler doesn't appear to make recognition of
the command line option dependent on the underlying assembler's
capabilities.

> --- a/xen/arch/x86/include/asm/cpufeatures.h
> +++ b/xen/arch/x86/include/asm/cpufeatures.h
> @@ -39,6 +39,7 @@ XEN_CPUFEATURE(SC_VERW_PV,        X86_SYNTH(23)) /* VERW 
> used by Xen for PV */
>  XEN_CPUFEATURE(SC_VERW_HVM,       X86_SYNTH(24)) /* VERW used by Xen for HVM 
> */
>  XEN_CPUFEATURE(SC_VERW_IDLE,      X86_SYNTH(25)) /* VERW used by Xen for 
> idle */
>  XEN_CPUFEATURE(XEN_SHSTK,         X86_SYNTH(26)) /* Xen uses CET Shadow 
> Stacks */
> +XEN_CPUFEATURE(XEN_IBT,           X86_SYNTH(27)) /* Xen uses CET Indirect 
> Branch Tracking */

Is a feature flag actually warranted here, rather than a single
global boolean? You don't key any alternatives patching to this
bit, unlike was the case for XEN_SHSTK. And the only consumer is
cpu_has_xen_ibt, expanding to the boot CPU's instance of the bit.

Jan