Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 3 Nov 2020 11:57:57 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     "Anand K. Mistry" <amistry@google.com>
Cc:     Tom Lendacky <thomas.lendacky@amd.com>, x86@kernel.org,
        Joel Fernandes <joelaf@google.com>,
        Anthony Steinhauser <asteinhauser@google.com>,
        tglx@linutronix.de, "H. Peter Anvin" <hpa@zytor.com>,
        Ingo Molnar <mingo@redhat.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Mark Gross <mgross@linux.intel.com>,
        Mike Rapoport <rppt@kernel.org>,
        Pawan Gupta <pawan.kumar.gupta@linux.intel.com>,
        Tony Luck <tony.luck@intel.com>,
        Vineela Tummalapalli <vineela.tummalapalli@intel.com>,
        Waiman Long <longman@redhat.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] x86/speculation: Allow IBPB to be conditionally
 enabled on CPUs with always-on STIBP
Message-ID: <20201103105757.GC6310@zn.tnic>
References: <20201029065133.3027749-1-amistry@google.com>
 <20201029175120.1.Ifd7243cd3e2c2206a893ad0a5b9a4f19549e22c6@changeid>
 <839fad53-4377-592a-a0da-2cf18b5c6027@amd.com>
 <CAATStaOTMrdserLepxkSdFErrjhMKyvd_g_GNBqOo_4p932ikw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CAATStaOTMrdserLepxkSdFErrjhMKyvd_g_GNBqOo_4p932ikw@mail.gmail.com>
Precedence: bulk

On Mon, Nov 02, 2020 at 11:02:10AM +1100, Anand K. Mistry wrote:
> > I like the idea of passing in the mode you want to check, but it appears
> > they are never used independently. The ibpb and stibp modes are always
> > checked together in one of the if statements below, so you could make this
> > a function that checks both modes and just have a single call. I'll leave
> > that up to the maintainers to see what is preferred.
> 
> I can see both sides to this. Personally, I think I prefer it as-is
> since I think it improves readability a bit by making the conditions
> less complicated whilst not hiding too many details. I'll wait to see
> what others say before changing this one.

Yes, but if you make it a single function with a descriptive name, you'd
make the call sites even more readable:

	if (!is_spec_ib_conditional(..))
		bla;

or

	if (!is_spec_ib_user_controlled(..))
		blu;

and that function should simply check both spectre_v2_user_ibpb *and*
spectre_v2_user_stibp in one go.

Why should we do that?

Exactly because you both got your brains twisted just from looking at
this. Because this mitigation crap is such an ugly and complex maze that
we would take even the smallest simplification any day of the week!

Welcome to my life since meltdown. Brain twist feels good, doesn't it?

:-)))

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette