Re: [PATCH v2 0/3] retpoline: add clang support + Kconfig selectable

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 16/02/2022 16:21, Roger Pau Monne wrote:
> Hello,
>
> The following series adds retpoline support for clang builds, and also
> allows the user to select whether to enable retpoline support at build
> time via a new Kconfig option.
>
> I've tried adding a suitable description to the Kconfig option, but I'm
> sure there's room for improvement.
>
> Thanks, Roger.
>
> Roger Pau Monne (3):
>   x86/retpoline: rename retpoline Kconfig check to include GCC prefix
>   x86/clang: add retpoline support
>   x86/Kconfig: introduce option to select retpoline usage

I don't particularly want to nitpick, but IMO this would be a lot easier
to follow if we ended up with

config CC_HAS_RETPOLINE
    def_bool $(cc-option,-mindirect-branch-register) ||
$(cc-option,-mretpoline-external-thunk)

config INDIRECT_THUNK
    depends on CC_HAS_RETPOLINE

and

ifeq ($(CONFIG_INDIRECT_THUNK),y)
CFLAGS-$(CONFIG_CC_IS_GCC) += ...
CFLAGS-$(CONFIG_CC_IS_CLANG) += ...
endif

because this reduces the number of CONFIG_* options involved.

Thoughts?

On substantially more minor points, INDIRECT_THUNK wants to be first in
the speculative hardening list, seeing as it is by far and away the most
important one, and I think we should stop writing things like "If
unsure, ..." in the help because it's just parroting the default which
is also rendered to people reading this message.  Our audience here are
developers, and I think we can depend on them to intuit the stated default.

~Andrew