Re: [PATCH v6 00/13] x86/bugs: Add a separate config for each mitigation

From: Ingo Molnar <mingo@kernel.org>
To: Breno Leitao <leitao@debian.org>
Cc: jpoimboe@kernel.org, mingo@redhat.com, tglx@linutronix.de,
	bp@alien8.de, x86@kernel.org, leit@meta.com,
	linux-kernel@vger.kernel.org, pawan.kumar.gupta@linux.intel.com,
	bpf@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v6 00/13] x86/bugs: Add a separate config for each mitigation
Date: Wed, 10 Jan 2024 10:56:46 +0100	[thread overview]
Message-ID: <ZZ5p3vdnTtU5TeJe@gmail.com> (raw)
In-Reply-To: <20231121160740.1249350-1-leitao@debian.org>

* Breno Leitao <leitao@debian.org> wrote:

> Currently, the CONFIG_SPECULATION_MITIGATIONS is halfway populated,
> where some mitigations have entries in Kconfig, and they could be
> modified, while others mitigations do not have Kconfig entries, and
> could not be controlled at build time.
> 
> The fact of having a fine grained control can help in a few ways:
> 
> 1) Users can choose and pick only mitigations that are important for
> their workloads.
> 
> 2) Users and developers can choose to disable mitigations that mangle
> the assembly code generation, making it hard to read.
> 
> 3) Separate configs for just source code readability,
> so that we see *which* butt-ugly piece of crap code is for what
> reason.
> 
> Important to say, if a mitigation is disabled at compilation time, it
> could be enabled at runtime using kernel command line arguments.
> 
> Discussion about this approach:
> https://lore.kernel.org/all/CAHk-=wjTHeQjsqtHcBGvy9TaJQ5uAm5HrCDuOD9v7qA9U1Xr4w@mail.gmail.com/
> and
> https://lore.kernel.org/lkml/20231011044252.42bplzjsam3qsasz@treble/
> 
> In order to get the missing mitigations, some clean up was done.
> 
> 1) Get a namespace for mitigations, prepending MITIGATION to the Kconfig
> entries.
> 
> 2) Adding the missing mitigations, so, the mitigations have entries in the
> Kconfig that could be easily configure by the user.
> 
> With this patchset applied, all configs have an individual entry under
> CONFIG_SPECULATION_MITIGATIONS, and all of them starts with CONFIG_MITIGATION.

Yeah, so:

 - I took this older series and updated it to current upstream, and made
   sure all renames were fully done: there were two new Kconfig option
   uses, which I integrated into the series. (Sorry about the delay, holiday & stuff.)

 - I also widened the renames to comments and messages, which were not
   always covered.

 - Then I took this cover letter and combined it with a more high level
   description of the reasoning behind this series I wrote up, and added it
   to patch #1. (see it below.)

 - Then I removed the changelog repetition from the other patches and just
   referred them back to patch #1.

 - Then I stuck the resulting updated series into tip:x86/bugs, without the 
   last 3 patches that modify behavior.

 - You might notice the somewhat weird extra whitespaces in the titles - 
   I've done that so that it all looks tidy in the shortlog:

   Breno Leitao (10):
      x86/bugs: Rename CONFIG_GDS_FORCE_MITIGATION => CONFIG_MITIGATION_GDS_FORCE
      x86/bugs: Rename CONFIG_CPU_IBPB_ENTRY       => CONFIG_MITIGATION_IBPB_ENTRY
      x86/bugs: Rename CONFIG_CALL_DEPTH_TRACKING  => CONFIG_MITIGATION_CALL_DEPTH_TRACKING
      x86/bugs: Rename CONFIG_PAGE_TABLE_ISOLATION => CONFIG_MITIGATION_PAGE_TABLE_ISOLATION
      x86/bugs: Rename CONFIG_RETPOLINE            => CONFIG_MITIGATION_RETPOLINE
      x86/bugs: Rename CONFIG_SLS                  => CONFIG_MITIGATION_SLS
      x86/bugs: Rename CONFIG_CPU_UNRET_ENTRY      => CONFIG_MITIGATION_UNRET_ENTRY
      x86/bugs: Rename CONFIG_CPU_IBRS_ENTRY       => CONFIG_MITIGATION_IBRS_ENTRY
      x86/bugs: Rename CONFIG_CPU_SRSO             => CONFIG_MITIGATION_SRSO
      x86/bugs: Rename CONFIG_RETHUNK              => CONFIG_MITIGATION_RETHUNK

I think the resulting tree is all mostly good, but still I'd like to see 
just the 10 pure low-risk renames done in this first step, to not carry too 
much of this around unnecessarily - maybe even send it Linuswards in this 
cycle if it's problem-free - without any real regression risk to upstream.

Thanks,

	Ingo

=============================>
commit be83e809ca67bca98fde97ad6b9344237963220b
Author: Breno Leitao <leitao@debian.org>
Date:   Tue Nov 21 08:07:28 2023 -0800

    x86/bugs: Rename CONFIG_GDS_FORCE_MITIGATION => CONFIG_MITIGATION_GDS_FORCE

    So the CPU mitigations Kconfig entries - there's 10 meanwhile - are named
    in a historically idiosyncratic and hence rather inconsistent fashion
    and have become hard to relate with each other over the years:

       https://lore.kernel.org/lkml/20231011044252.42bplzjsam3qsasz@treble/

    When they were introduced we never expected that we'd eventually have
    about a dozen of them, and that more organization would be useful,
    especially for Linux distributions that want to enable them in an
    informed fashion, and want to make sure all mitigations are configured
    as expected.

    For example, the current CONFIG_SPECULATION_MITIGATIONS namespace is only
    halfway populated, where some mitigations have entries in Kconfig, and
    they could be modified, while others mitigations do not have Kconfig entries,
    and can not be controlled at build time.

    Fine-grained control over these Kconfig entries can help in a number of ways:

      1) Users can choose and pick only mitigations that are important for
         their workloads.

      2) Users and developers can choose to disable mitigations that mangle
         the assembly code generation, making it hard to read.

      3) Separate Kconfigs for just source code readability,
         so that we see *which* butt-ugly piece of crap code is for what
         reason...

    In most cases, if a mitigation is disabled at compilation time, it
    can still be enabled at runtime using kernel command line arguments.

    This is the first patch of an initial series that renames various
    mitigation related Kconfig options, unifying them under a single
    CONFIG_MITIGATION_* namespace:

        CONFIG_GDS_FORCE_MITIGATION => CONFIG_MITIGATION_GDS_FORCE
        CONFIG_CPU_IBPB_ENTRY       => CONFIG_MITIGATION_IBPB_ENTRY
        CONFIG_CALL_DEPTH_TRACKING  => CONFIG_MITIGATION_CALL_DEPTH_TRACKING
        CONFIG_PAGE_TABLE_ISOLATION => CONFIG_MITIGATION_PAGE_TABLE_ISOLATION
        CONFIG_RETPOLINE            => CONFIG_MITIGATION_RETPOLINE
        CONFIG_SLS                  => CONFIG_MITIGATION_SLS
        CONFIG_CPU_UNRET_ENTRY      => CONFIG_MITIGATION_UNRET_ENTRY
        CONFIG_CPU_IBRS_ENTRY       => CONFIG_MITIGATION_IBRS_ENTRY
        CONFIG_CPU_SRSO             => CONFIG_MITIGATION_SRSO
        CONFIG_RETHUNK              => CONFIG_MITIGATION_RETHUNK

    Implement step 1/10 of the namespace unification of CPU mitigations related
    Kconfig options and rename CONFIG_GDS_FORCE_MITIGATION to
    CONFIG_MITIGATION_GDS_FORCE.

    [ mingo: Rewrote changelog for clarity. ]

    Suggested-by: Josh Poimboeuf <jpoimboe@kernel.org>
    Signed-off-by: Breno Leitao <leitao@debian.org>
    Signed-off-by: Ingo Molnar <mingo@kernel.org>
    Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Link: https://lore.kernel.org/r/20231121160740.1249350-2-leitao@debian.org