From: Josh Poimboeuf <jpoimboe@redhat.com>
To: speck@linutronix.de
Subject: [MODERATED] Re: [PATCH v7 04/10] TAAv7 4
Date: Tue, 22 Oct 2019 16:44:15 -0500 [thread overview]
Message-ID: <20191022214415.5gy4q7l4fa6ln6l6@treble> (raw)
In-Reply-To: <00890cc3-818b-6fbc-4780-22364407e68c@citrix.com>
On Tue, Oct 22, 2019 at 10:35:12PM +0100, speck for Andrew Cooper wrote:
> On 22/10/2019 22:20, speck for Josh Poimboeuf wrote:
> > On Mon, Oct 21, 2019 at 01:26:02PM -0700, speck for Pawan Gupta wrote:
> >> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> >> index 6e0a3b43d027..999b85039128 100644
> >> --- a/arch/x86/include/asm/processor.h
> >> +++ b/arch/x86/include/asm/processor.h
> >> @@ -988,4 +988,11 @@ enum mds_mitigations {
> >> MDS_MITIGATION_VMWERV,
> >> };
> >>
> >> +enum taa_mitigations {
> >> + TAA_MITIGATION_OFF,
> >> + TAA_MITIGATION_UCODE_NEEDED,
> >> + TAA_MITIGATION_VERW,
> >> + TAA_MITIGATION_TSX_DISABLE,
> > I would rename "TSX_DISABLE" to "TSX_DISABLED", because
> >
> > a) it matches the verb tense of "UCODE_NEEDED", and
> > b) the past tense hopefully helps make it slightly clearer that TSX was
> > disabled separately, not as part of the mitigation code itself.
> >
> >> @@ -765,7 +871,7 @@ static void update_indir_branch_cond(void)
> >> #undef pr_fmt
> >> #define pr_fmt(fmt) fmt
> >>
> >> -/* Update the static key controlling the MDS CPU buffer clear in idle */
> >> +/* Update the static key controlling the MDS and TAA CPU buffer clear in idle */
> >> static void update_mds_branch_idle(void)
> >> {
> >> /*
> >> @@ -775,8 +881,11 @@ static void update_mds_branch_idle(void)
> >> * The other variants cannot be mitigated when SMT is enabled, so
> >> * clearing the buffers on idle just to prevent the Store Buffer
> >> * repartitioning leak would be a window dressing exercise.
> >> + *
> >> + * Apply idle buffer clearing to TAA affected CPUs also.
> >> */
> >> - if (!boot_cpu_has_bug(X86_BUG_MSBDS_ONLY))
> >> + if (!boot_cpu_has_bug(X86_BUG_MSBDS_ONLY) &&
> >> + !boot_cpu_has_bug(X86_BUG_TAA))
> >> return;
> > Sorry, I was out for most of the last two weeks, so I think I left you
> > hanging on this. For context, here's your answer to my previous
> > question about whether the X86_BUG_TAA check makes sense here:
> >
> >> It does provide protection against the "store buffer" leak. But the
> >> other buffers(fill buffer and load port) are still SMT vulnerable. Do
> >> you prefer it removed?
> > Yes, please remove it, for the same reason we used for MDS. There's not
> > much point in partially mitigating TAA here.
> >
> >> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> >> index 885d4ac2111a..86f22c1e5912 100644
> >> --- a/arch/x86/kernel/cpu/common.c
> >> +++ b/arch/x86/kernel/cpu/common.c
> >> @@ -1128,6 +1128,21 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
> >> if (!cpu_matches(NO_SWAPGS))
> >> setup_force_cpu_bug(X86_BUG_SWAPGS);
> >>
> >> + /*
> >> + * When processor is not mitigated for TAA (TAA_NO=0) set TAA bug when:
> >> + * - TSX is supported or
> >> + * - TSX_CTRL is supported
> >> + *
> >> + * TSX_CTRL check is needed for cases when TSX could be disabled before
> >> + * the kernel boot e.g. kexec
> >> + * TSX_CTRL check alone is not sufficient for cases when the microcode
> >> + * update is not present or running as guest that don't get TSX_CTRL.
> >> + */
> >> + if (!(ia32_cap & ARCH_CAP_TAA_NO) &&
> >> + (boot_cpu_has(X86_FEATURE_RTM) ||
> >> + (ia32_cap & ARCH_CAP_TSX_CTRL_MSR)))
> >> + setup_force_cpu_bug(X86_BUG_TAA);
> >> +
> > I'm finding this logic to be less than 100% convincing, or at least hard
> > to follow. And it's different from most of the other bug checks in this
> > function.
> >
> > Would the following work instead?
> >
> > if (!cpu_matches(NO_MDS) && !(ia32_cap & ARCH_CAP_TAA_NO))
> > setup_force_cpu_bug(X86_BUG_TAA);
> >
> > In other words I would presume all the NO_MDS CPUs listed in
> > 'cpu_vuln_whitelist' are also immune to TAA.
>
> From a leakage point of view, TAA is just another way to access stale
> date in the load ports, store buffer or fill buffers.
>
> In practice, the leakage mechanism exists on all MDS-vulnerable,
> TSX-enabled parts. Parts which are not vulnerable to MDS leakage in the
> first place will be similarly unaffected.
>
> On pre MDS_NO=1 parts, the existing VERW and no-smt mitigates TAA as well.
>
> On the current MDS_NO=1 parts, the silicon fix constituting MDS_NO took
> care of the known leakage cases by forcing a register file write-back of
> 0 rather than the stale data. However, the TSX Async Abort case was
> missed, which is why this generation of parts are still vulnerable.
>
> Therefore, the TAA vulnerability only really matters for TSX=1,
> MDS_NO=1, TAA_NO=0 parts, because these parts are the ones that are
> leaky, but have a lower overhead mitigation option than VERW and no-SMT.
I agree with all of that. So are you saying you agree or disagree with
my code snippet? Note that a point of confusion here might be that
NO_MDS in the above code is something altogether different from the
ARCH_CAP_MDS_NO CPU bit.
--
Josh
next prev parent reply other threads:[~2019-10-22 21:44 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-21 20:22 [MODERATED] [PATCH v7 00/10] TAAv7 0 Pawan Gupta
2019-10-21 20:23 ` [MODERATED] [PATCH v7 01/10] TAAv7 1 Pawan Gupta
2019-10-21 20:24 ` [MODERATED] [PATCH v7 02/10] TAAv7 2 Pawan Gupta
2019-10-21 20:25 ` [MODERATED] [PATCH v7 03/10] TAAv7 3 Pawan Gupta
2019-10-21 20:26 ` [MODERATED] [PATCH v7 04/10] TAAv7 4 Pawan Gupta
2019-10-21 20:27 ` [MODERATED] [PATCH v7 05/10] TAAv7 5 Pawan Gupta
2019-10-21 20:28 ` [MODERATED] [PATCH v7 06/10] TAAv7 6 Pawan Gupta
2019-10-21 20:29 ` [MODERATED] [PATCH v7 07/10] TAAv7 7 Pawan Gupta
2019-10-21 20:30 ` [MODERATED] [PATCH v7 08/10] TAAv7 8 Pawan Gupta
2019-10-21 20:31 ` [MODERATED] [PATCH v7 09/10] TAAv7 9 Michal Hocko
2019-10-21 20:32 ` [MODERATED] [PATCH v7 10/10] TAAv7 10 Pawan Gupta
2019-10-21 21:32 ` [MODERATED] Re: [PATCH v7 00/10] TAAv7 0 Andy Lutomirski
2019-10-21 23:06 ` Andrew Cooper
2019-10-22 0:34 ` Pawan Gupta
2019-10-22 4:10 ` [MODERATED] Jon Masters
2019-10-22 5:53 ` [MODERATED] Pawan Gupta
2019-10-22 7:58 ` [MODERATED] Re: ***UNCHECKED*** [PATCH v7 07/10] TAAv7 7 Michal Hocko
2019-10-22 16:55 ` [MODERATED] " Pawan Gupta
2019-10-22 8:00 ` [MODERATED] Re: ***UNCHECKED*** [PATCH v7 09/10] TAAv7 9 Michal Hocko
2019-10-22 8:15 ` [MODERATED] Re: ***UNCHECKED*** [PATCH v7 03/10] TAAv7 3 Michal Hocko
2019-10-22 14:42 ` Josh Poimboeuf
2019-10-22 16:48 ` [MODERATED] " Pawan Gupta
2019-10-22 17:01 ` [MODERATED] Re: ***UNCHECKED*** " Michal Hocko
2019-10-22 17:35 ` Josh Poimboeuf
2019-10-22 14:38 ` [MODERATED] " Borislav Petkov
2019-10-22 16:58 ` Pawan Gupta
2019-10-22 14:48 ` Borislav Petkov
2019-10-22 17:00 ` Pawan Gupta
2019-10-22 17:16 ` [MODERATED] " Borislav Petkov
2019-10-22 18:07 ` [MODERATED] " Pawan Gupta
2019-10-22 15:07 ` Borislav Petkov
2019-10-22 18:36 ` Pawan Gupta
2019-10-22 18:59 ` [MODERATED] " Borislav Petkov
2019-10-22 16:51 ` [MODERATED] Re: [PATCH v7 04/10] TAAv7 4 Borislav Petkov
2019-10-22 17:02 ` Borislav Petkov
2019-10-22 18:00 ` Pawan Gupta
2019-10-22 18:12 ` [MODERATED] " Borislav Petkov
2019-10-22 19:16 ` Luck, Tony
2019-10-22 19:28 ` [MODERATED] " Borislav Petkov
2019-10-22 20:02 ` Luck, Tony
2019-10-22 20:48 ` [MODERATED] Jon Masters
2019-10-22 20:54 ` [MODERATED] Re: [PATCH v7 04/10] TAAv7 4 Borislav Petkov
2019-10-22 21:38 ` Josh Poimboeuf
2019-10-22 21:46 ` Borislav Petkov
2019-10-22 22:06 ` Josh Poimboeuf
2019-10-22 22:13 ` Borislav Petkov
2019-10-22 17:44 ` Pawan Gupta
2019-10-22 19:04 ` [MODERATED] " Borislav Petkov
2019-10-22 21:29 ` [MODERATED] " Pawan Gupta
2019-10-22 21:53 ` Borislav Petkov
2019-10-22 22:05 ` Borislav Petkov
2019-10-23 0:27 ` Pawan Gupta
2019-10-23 5:25 ` Pawan Gupta
2019-10-23 6:46 ` Borislav Petkov
2019-10-23 13:28 ` Pawan Gupta
2019-10-23 14:39 ` Borislav Petkov
2019-10-23 1:33 ` Pawan Gupta
2019-10-23 6:48 ` Borislav Petkov
2019-10-22 17:25 ` [MODERATED] Re: [PATCH v7 01/10] TAAv7 1 Josh Poimboeuf
2019-10-23 9:26 ` Borislav Petkov
2019-10-22 17:26 ` Josh Poimboeuf
2019-10-22 20:44 ` [MODERATED] Jon Masters
2019-10-22 17:47 ` [MODERATED] Re: [PATCH v7 03/10] TAAv7 3 Josh Poimboeuf
2019-10-22 18:39 ` [MODERATED] Re: [PATCH v7 10/10] TAAv7 10 Josh Poimboeuf
2019-10-23 7:24 ` Borislav Petkov
2019-10-22 21:20 ` [MODERATED] Re: [PATCH v7 04/10] TAAv7 4 Josh Poimboeuf
2019-10-22 21:35 ` Andrew Cooper
2019-10-22 21:44 ` Josh Poimboeuf [this message]
2019-10-22 22:03 ` Andrew Cooper
2019-10-23 1:16 ` Josh Poimboeuf
2019-10-23 15:46 ` [MODERATED] Re: [PATCH v7 00/10] TAAv7 0 Borislav Petkov
2019-10-23 17:11 ` Josh Poimboeuf
2019-10-23 21:49 ` Borislav Petkov
2019-10-23 22:12 ` Pawan Gupta
2019-10-24 14:08 ` Borislav Petkov
[not found] ` <5dae165e.1c69fb81.4beee.e271SMTPIN_ADDED_BROKEN@mx.google.com>
2019-10-24 20:53 ` [MODERATED] Re: [PATCH v7 06/10] TAAv7 6 Paolo Bonzini
2019-10-24 21:00 ` Luck, Tony
2019-10-24 21:33 ` Paolo Bonzini
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=20191022214415.5gy4q7l4fa6ln6l6@treble \
--to=jpoimboe@redhat.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).