From: mark gross <mgross@linux.intel.com>
To: speck@linutronix.de
Subject: [MODERATED] Re: [PATCH 2/3] V5 more sampling fun 2
Date: Wed, 8 Apr 2020 15:14:19 -0700 [thread overview]
Message-ID: <20200408221419.GA30223@mtg-dev.jf.intel.com> (raw)
In-Reply-To: <20200406220714.q2jzjftqlxqnhco7@treble>
On Mon, Apr 06, 2020 at 05:07:14PM -0500, speck for Josh Poimboeuf wrote:
> On Thu, Jan 16, 2020 at 02:16:07PM -0800, speck for mark gross wrote:
> > From: mark gross <mgross@linux.intel.com>
> > Subject: [PATCH 2/3] x86/speculation: Special Register Buffer Data Sampling
> > (SRBDS) mitigation control.
> > +enum srbds_mitigations {
> > + SRBDS_MITIGATION_OFF,
> > + SRBDS_MITIGATION_UCODE_NEEDED,
> > + SRBDS_MITIGATION_FULL,
> > + SRBDS_NOT_AFFECTED_TSX_OFF,
> > + SRBDS_HYPERVISOR,
>
> These should all be prefixed with "SRBDS_MITIGATION_".
>
> > +};
> > +
> > +enum srbds_mitigations srbds_mitigation __ro_after_init = SRBDS_MITIGATION_FULL;
>
> This can be static.
>
> > +static const char * const srbds_strings[] = {
> > + [SRBDS_MITIGATION_OFF] = "Vulnerable",
> > + [SRBDS_MITIGATION_UCODE_NEEDED] = "Vulnerable: no microcode",
>
> "no microcode" should be capitalized:
>
> "Vulnerable: No microcode"
>
> > + [SRBDS_MITIGATION_FULL] = "Mitigated",
>
> All the other mitigations say "Mitigation: <description of mitigation>".
>
> So to be consistent, and to not break dumb scripts which might rely on
> that, how about:
>
> "Mitigation: Microcode"
>
> > + [SRBDS_NOT_AFFECTED_TSX_OFF] = "Not affected (TSX disabled)",
>
> Is this actually two distinct states?
>
> 1) MDS_NO parts which support TSX, where the user has disabled TSX. In
> which case it should say:
>
> "Mitigation: TSX disabled"
>
> 2) MDS_NO CPUs which *don't* support TSX, which should say:
>
> "Not affected"
>
> (I presume the bug bit doesn't need to be set in this case)
I've been working addressing this issue. It is common for Intel to fuse off
features on low end SKU's so this could happen. After looking into how to
disambiguate a CFL part (Family 6, Model 158 stepping 13) with TSX fused off
from a CLF part where TSX is disabled using TSX_CNTRL MSR. I'd like to side
step the problem.
The solution is code for common.c that I have a hard time parsing even though I
wrote it. It makes my eyes bleed. I wonder if it would be better to simply
reword the string used for sysfs in the SRBDS_NOT_AFFECTED_TSX_OFF case to
report simply "Not affected" i.e. drop the "(TSX disbled)" part to hide the
ambiguous case?
FWIW this is what I'm thinking of adding to common.c if we choose to not hide
this ambiguity. We are checking with the uCode folks to confirm but
ARCH_CAP_TSX_CTRL_MSR will not be set if the part has TSX fused off and we
think we can use that to disambiguate:
1166 if (cpu_matches(SRBDS, cpu_vuln_blacklist)) {
1167 /*
1168 * Some low-end SKUs on the affected list do not support
1169 * RDRAND or RDSEED. Make sure they show as "Not affected".
1170 */
1171 if (cpu_has(c, X86_FEATURE_RDRAND) ||
1172 cpu_has(c, X86_FEATURE_RDSEED)) {
1173 /* Some low end SKU's of parts with F/M/S in the
1174 * blacklist that normally are only affected if TSX is
1175 * enabled could have TSX fused off. To avoid reporting
1176 * "Not affected (TSX disabled)" check !MDS_NO or available
1177 * TSX_CTRL_MSR as a check for parts not fused
1178 * off.
1179 */
1180 if (!(ia32_cap & ARCH_CAP_MDS_NO) ||.
1181 (ia32_cap & ARCH_CAP_TSX_CTRL_MSR))
1182 setup_force_cpu_bug(X86_BUG_SRBDS);
1183
1184 }
1185 }
Another option is to add back the SRBDS_MITIGATION_NOT_AFFECTED and do this TSX
fused off check of ARCH_CAP_TSX_CTRL_MSR in bugs.c when checking for
X86_FEATURE_RTM.
What do you or others think?
--mark
next prev parent reply other threads:[~2020-04-08 22:14 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-06 17:52 [MODERATED] [PATCH 0/3] V5 more sampling fun 0 mark gross
2020-01-16 22:16 ` [MODERATED] [PATCH 2/3] V5 more sampling fun 2 mark gross
2020-01-30 19:12 ` [MODERATED] [PATCH 3/3] V5 more sampling fun 3 mark gross
2020-03-17 0:56 ` [MODERATED] [PATCH 1/3] V5 more sampling fun 1 mark gross
[not found] ` <5e8b7166.1c69fb81.4c99a.3619SMTPIN_ADDED_BROKEN@mx.google.com>
2020-04-06 18:31 ` [MODERATED] " Kees Cook
[not found] ` <5e8b71d8.1c69fb81.64075.43abSMTPIN_ADDED_BROKEN@mx.google.com>
2020-04-06 18:34 ` [MODERATED] Re: [PATCH 2/3] V5 more sampling fun 2 Kees Cook
2020-04-06 18:37 ` Greg KH
2020-04-06 20:56 ` mark gross
2020-04-06 22:14 ` Luck, Tony
2020-04-07 7:51 ` Greg KH
2020-04-06 18:52 ` mark gross
[not found] ` <5e8b71af.1c69fb81.d8b8.ac6bSMTPIN_ADDED_BROKEN@mx.google.com>
2020-04-06 18:34 ` [MODERATED] Re: [PATCH 3/3] V5 more sampling fun 3 Kees Cook
2020-04-06 22:07 ` [MODERATED] Re: [PATCH 2/3] V5 more sampling fun 2 Josh Poimboeuf
2020-04-07 0:34 ` mark gross
2020-04-07 12:39 ` Josh Poimboeuf
2020-04-08 20:26 ` mark gross
2020-04-08 22:14 ` mark gross [this message]
2020-04-08 22:58 ` mark gross
2020-04-07 15:17 ` Thomas Gleixner
2020-04-08 20:33 ` [MODERATED] " mark gross
2020-04-08 23:21 ` Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200408221419.GA30223@mtg-dev.jf.intel.com \
--to=mgross@linux.intel.com \
--cc=speck@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).