Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 31 Mar 2022 10:35:47 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Ricardo =?utf-8?Q?Ca=C3=B1uelo?= <ricardo.canuelo@collabora.com>
Cc:     linux-kernel@vger.kernel.org,
        Thadeu Lima de Souza Cascardo <cascardo@canonical.com>,
        Mark Gross <mgross@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        John Johansen <john.johansen@canonical.com>,
        Steve Beattie <sbeattie@ubuntu.com>, kernel@collabora.com,
        Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Subject: Re: [PATCH v2] x86/speculation/srbds: do not try to turn mitigation
 off when not supported
Message-ID: <YkVn4wSDoTHl5Icd@zn.tnic>
References: <20220330082026.1549073-1-ricardo.canuelo@collabora.com>
 <YkS3OKLS1Cixs9up@zn.tnic>
 <87o81mzhoh.fsf@rcn-XPS-13-9305.i-did-not-set--mail-host-address--so-tickle-me>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <87o81mzhoh.fsf@rcn-XPS-13-9305.i-did-not-set--mail-host-address--so-tickle-me>
Precedence: bulk

On Thu, Mar 31, 2022 at 09:48:14AM +0200, Ricardo Cañuelo wrote:
> I agree that the more explicit the better, I'll give this a try. I saw
> Pawan's suggestion as well but that one is similar to the originally
> proposed patch in that the logic/checks are split between two functions,
> this solution based on clearing the bug flag seems clearer considering
> the comment just before the code block:

Yeah, and I have some reservations with clearing that flag because,
technically speaking, that CPU still has X86_BUG_SRBDS - it's just that
it hasn't been exposed due to TSX being disabled. And no SRBDS microcode
has been uploaded.

Btw this is exactly the reason I want this to be crystal clear -
the insane conditionals around those things just to salvage *some*
performance with a lot of "but but" make everyone who deals with bugs.c
cringe...

Anyway, Pawan's suggestion makes more sense with the aspect that, yes,
the CPU is affected but the MSR is not there. And we already have
similar logic when dealing with TSX so that no new territory.

So yeah, let's do his but *actually* document why and put it in a
separate line:

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 6296e1ebed1d..d879a6c93609 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -446,6 +446,13 @@ void update_srbds_msr(void)
 	if (srbds_mitigation == SRBDS_MITIGATION_UCODE_NEEDED)
 		return;
 
+	/*
+	 * A MDS_NO CPU for which SRBDS mitigation is not needed due to TSX
+	 * being disabled and it hasn't received the SRBDS MSR microcode.
+	 */
+	if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL))
+		return;
+
 	rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl);
 
 	switch (srbds_mitigation) {

---

Thx.

-- 
Regards/Gruss,
    Boris.

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