Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
MIME-Version: 1.0
References: <20230628142129.2468174-1-leitao@debian.org> <ZRV1bIuSXjZ+uPKB@gmail.com>
 <20231005162545.GFZR7jiUNyNkscijUl@fat_crate.local>
In-Reply-To: <20231005162545.GFZR7jiUNyNkscijUl@fat_crate.local>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu, 5 Oct 2023 11:29:02 -0700
Message-ID: <CAHk-=wjTHeQjsqtHcBGvy9TaJQ5uAm5HrCDuOD9v7qA9U1Xr4w@mail.gmail.com>
Subject: Re: [PATCH v3] x86/bugs: Add a separate config for each mitigation
To:     Borislav Petkov <bp@alien8.de>
Cc:     Breno Leitao <leitao@debian.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Pawan Gupta <pawan.kumar.gupta@linux.intel.com>, leit@meta.com,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 5 Oct 2023 at 09:26, Borislav Petkov <bp@alien8.de> wrote:
>
> I happen to know that Linus wanted those per mitigation, perhaps to be
> able to disable only a subset of them.

Partly for that - some of them are more obnoxious than others, and
cause more changes to code generation.

And partly I want separate configs for just source code readability,
so that we see *which* butt-ugly piece of crap code is for what
reason.

> Linus, what are you thoughts on it, should we continue with a Kconfig
> option per mitigation or should we hide them all behind a single Kconfig
> option - which would be a lot simpler and easier?
>
> Apparently people want to completely remove the mitigations crap for
> some configurations at build time already.

I'd be perfectly happy with a top-level Kconfig question for "enable
mitigations", which could be a config with three values ("all", "none"
and "finegrained").

But see above (particularly the second thing) on why I want us to
still have individual config options for each individual issue. I'm
not convinced a lot of people *care* about the "finegrained" case of
enabling/disabling each mitigation at build time), but I do use it
myself because some of the mitigations end up changing code generation
*so* much that it gets hard to even read the generated assembly (ie
all the retpoline crap looks *horrendous* if what you actually want to
do is check that some change helps code generation and want to
actually look at the resulting *.s files).

Same goes for some perf runs, so this is not *just* "I do a build and
look at the result" - I want to be able to run it too.

So I'll keep the mitigations that don't f*ck up my system too much,
and then because I actually do look at the generated code when I match
up source and result (ie the whole "annotate" thing in perf), things
like retpoline really do end up screwing things up horrendously, in
ways that some of the other mitigations don't.

Maybe I'm odd in the above "disable some mitigations to see the code
generation", but the source-level "readability", and the "this crazy
code is because of this mitigation" is still important, I feel.

I do *not* want to live in a world where we have random crazy code and
just a "#ifdef CONFIG_MITIGATIONS" around it. Those

    IS_ENABLED(CONFIG_RETPOLINE) || IS_ENABLED(CONFIG_SLS))

"complex" conditionals may also be annoying, but dammit, they are
important documentation about why we do those things, and unlike just
comments that will inevitably bit-rot, they have semantics and get
tested.

                  Linus