linux-kernel - Re: [PATCH v2] x86/speculation/srbds: do not try to turn mitigation off when not supported

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YkVn4wSDoTHl5Icd@zn.tnic>
Date:   Thu, 31 Mar 2022 10:35:47 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Ricardo Cañuelo <ricardo.canuelo@...labora.com>
Cc:     linux-kernel@...r.kernel.org,
        Thadeu Lima de Souza Cascardo <cascardo@...onical.com>,
        Mark Gross <mgross@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        John Johansen <john.johansen@...onical.com>,
        Steve Beattie <sbeattie@...ntu.com>, kernel@...labora.com,
        Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
Subject: Re: [PATCH v2] x86/speculation/srbds: do not try to turn mitigation
 off when not supported

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