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.