Re: [PATCH 1/3] x86: Move RETPOLINE*_CFLAGS to arch Makefile

[Nick Desaulniers]

On Fri, Nov 19, 2021 at 8:57 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> Currently RETPOLINE*_CFLAGS are defined in the top-level Makefile but
> only x86 makes use of them, move them there. If ever another
> architecture finds the need, we can reconsider.
>
> Suggested-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> ---
>  Makefile          |   11 -----------
>  arch/x86/Makefile |   11 +++++++++++
>  2 files changed, 11 insertions(+), 11 deletions(-)
>
> --- a/Makefile
> +++ b/Makefile
> @@ -688,17 +688,6 @@ ifdef CONFIG_FUNCTION_TRACER
>    CC_FLAGS_FTRACE := -pg
>  endif
>
> -ifdef CONFIG_CC_IS_GCC
> -RETPOLINE_CFLAGS       := $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
> -RETPOLINE_VDSO_CFLAGS  := $(call cc-option,-mindirect-branch=thunk-inline -mindirect-branch-register)
> -endif
> -ifdef CONFIG_CC_IS_CLANG
> -RETPOLINE_CFLAGS       := -mretpoline-external-thunk
> -RETPOLINE_VDSO_CFLAGS  := -mretpoline
> -endif
> -export RETPOLINE_CFLAGS
> -export RETPOLINE_VDSO_CFLAGS
> -
>  include $(srctree)/arch/$(SRCARCH)/Makefile
>
>  ifdef need-config
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -12,6 +12,17 @@ else
>          KBUILD_DEFCONFIG := $(ARCH)_defconfig
>  endif
>
> +ifdef CONFIG_CC_IS_GCC
> +RETPOLINE_CFLAGS       := $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
> +RETPOLINE_VDSO_CFLAGS  := $(call cc-option,-mindirect-branch=thunk-inline -mindirect-branch-register)
> +endif
> +ifdef CONFIG_CC_IS_CLANG
> +RETPOLINE_CFLAGS       := -mretpoline-external-thunk
> +RETPOLINE_VDSO_CFLAGS  := -mretpoline
> +endif
> +export RETPOLINE_CFLAGS
> +export RETPOLINE_VDSO_CFLAGS

I appreciate the cleanup.
Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>

If we wanted to be more aggressive here, RETPOLINE_VDSO_CFLAGS is only
used in arch/x86/entry/vdso/Makefile. If the definition of it was
moved there, then we wouldn't need to export it.  Though it is perhaps
descriptive to contrast how the flags used differs between the kernel
and vdso; having them together does demonstrate that clearly and all
in one place.

> +
>  # For gcc stack alignment is specified with -mpreferred-stack-boundary,
>  # clang has the option -mstack-alignment for that purpose.
>  ifneq ($(call cc-option, -mpreferred-stack-boundary=4),)
>
>


-- 
Thanks,
~Nick Desaulniers