From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (193.142.43.55:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 26 Feb 2020 11:46:56 -0000 Received: from mx2.suse.de ([195.135.220.15]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1j6v9T-00060O-Bb for speck@linutronix.de; Wed, 26 Feb 2020 12:46:55 +0100 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 3FCF0ACB9 for ; Wed, 26 Feb 2020 11:46:49 +0000 (UTC) Date: Wed, 26 Feb 2020 12:46:41 +0100 From: Borislav Petkov Subject: [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 Message-ID: <20200226114641.GC17448@zn.tnic> References: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable To: speck@linutronix.de List-ID: On Thu, Jan 16, 2020 at 02:16:07PM -0800, speck for mark gross wrote: > From: mark gross > Subject: [PATCH v2 2/2] WIP SRBDS mitigation enabling. >=20 > From: mark gross > Subject: [PATCH v2 2/2] WIP SRBDS mitigation enabling. In addition to the notes in the previous mail, your subject needs a verb: "Add ... mitigation" or so. See git log -p arch/x86/kernel/cpu/bugs.c output for inspiration. > SRBDS is an MDS-like speculative side channel that can leak bits from > the RNG across cores and threads. New microcode serializes the processor > access during the execution of RDRAND and RDSEED ensures that the shared > buffer is overwritten before it is released for reuse. >=20 > We subdivide processors that are vulnerable to SRBDS into two classes: "We" is? > X86_BUG_SRBDS: models that are vulnerable > X86_BUG_SRBDS_TSX: models only vulnerable when TSX is enabled. >=20 > The latter are not vulnerable to SRBDS if TSX is disabled on all cores. >=20 > The mitigation is activated by default on affected processors and it slows > down /dev/urandom. The latency of RDRAND and RDSEED instructions is > increased by 10x. We don't expect this to be noticeable in most cases. This "We" sounds like you mean Intel...? > This patch: Avoid having "This patch" or "This commit" in the commit message. It is tautologically useless. Also, do $ git grep 'This patch' Documentation/process for more details. > @@ -397,6 +399,69 @@ static int __init tsx_async_abort_parse_cmdline(char *= str) > } > early_param("tsx_async_abort", tsx_async_abort_parse_cmdline); > =20 > +#undef pr_fmt > +#define pr_fmt(fmt) "SRBDS: " fmt > + > +enum srbds_mitigations srbds_mitigation __ro_after_init =3D SRBDS_MITIGATI= ON_FULL; > +static const char * const srbds_strings[] =3D { > + [SRBDS_MITIGATION_OFF] =3D "Vulnerable", > + [SRBDS_MITIGATION_UCODE_NEEDED] =3D "Vulnerable: no microcode", > + [SRBDS_MITIGATION_FULL] =3D "Mitigation: bus lock when using RDRAND or R= DSEED", > +}; > + > +void srbds_configure_mitigation(void) > +{ > + u64 mcu_ctrl; > + > + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && !boot_cpu_has_bug(X86_BUG_SRBDS_T= SX)) > + return; > + > + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) > + return; > + > + rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); What's up with virtualization? Is that MSR going to be exported or what? If so, you need the _safe() accessors here. > + if (srbds_mitigation =3D=3D SRBDS_MITIGATION_FULL) > + mcu_ctrl &=3D ~SRBDS_MITG_DIS; > + else if (srbds_mitigation =3D=3D SRBDS_MITIGATION_OFF) > + mcu_ctrl |=3D SRBDS_MITG_DIS; > + > + if (boot_cpu_has_bug(X86_BUG_SRBDS_TSX) && !boot_cpu_has(X86_FEATURE_RTM)) > + mcu_ctrl |=3D SRBDS_MITG_DIS; > + > + wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > +} > + > +static void __init srbds_select_mitigation(void) > +{ > + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && > + !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) > + return; Why is that check here again if you do it in srbds_configure_mitigation() above? I'm guessing you can remove the one above... > + > + if (cpu_mitigations_off()) { > + srbds_mitigation =3D SRBDS_MITIGATION_OFF; > + goto out; > + } > + > + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) > + srbds_mitigation =3D SRBDS_MITIGATION_UCODE_NEEDED; > + > +out: > + srbds_configure_mitigation(); > +} > + > +static int __init srbds_parse_cmdline(char *str) > +{ > + if (!str) > + return -EINVAL; > + > + if (!strcmp(str, "off")) > + srbds_mitigation =3D SRBDS_MITIGATION_OFF; > + > + return 0; > +} > + > +early_param("srbds_mitigation", srbds_parse_cmdline); "srb_sampling=3D" or something more readable pls. --=20 Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imend=C3=B6rffer, HRB 36809, = AG N=C3=BCrnberg --=20