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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Feb 18, 2022 at 01:58:31PM +0000, Andrew Cooper wrote:
> 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?

That would reduce one hidden Kconfig option. I don't mind
implementing it that way.

> 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.

OK, so let me put that one first on the list then, and drop the "If
unsure, "

Thanks, Roger.