From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
linux-kernel@vger.kernel.org,
"Kenneth R. Crudup" <kenny@panix.com>,
Jessica Yu <jeyu@kernel.org>,
Rasmus Villemoes <rasmus.villemoes@prevas.dk>,
Paolo Bonzini <pbonzini@redhat.com>,
Fenghua Yu <fenghua.yu@intel.com>,
Xiaoyao Li <xiaoyao.li@intel.com>,
Nadav Amit <nadav.amit@gmail.com>,
Thomas Hellstrom <thellstrom@vmware.com>,
Tony Luck <tony.luck@intel.com>,
Steven Rostedt <rostedt@goodmis.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jann Horn <jannh@google.com>, Kees Cook <keescook@chromium.org>,
David Laight <David.Laight@ACULAB.COM>,
Doug Covelli <dcovelli@vmware.com>
Subject: Re: [RFC PATCH] x86/split_lock: Disable SLD if an unaware (out-of-tree) module enables VMX
Date: Fri, 3 Apr 2020 10:20:18 -0700 [thread overview]
Message-ID: <20200403172018.GD2701@linux.intel.com> (raw)
In-Reply-To: <20200403164244.GZ20730@hirez.programming.kicks-ass.net>
On Fri, Apr 03, 2020 at 06:42:44PM +0200, Peter Zijlstra wrote:
> On Fri, Apr 03, 2020 at 09:30:07AM -0700, Sean Christopherson wrote:
> > Hook into native CR4 writes to disable split-lock detection if CR4.VMXE
> > is toggled on by an SDL-unaware entity, e.g. an out-of-tree hypervisor
> > module. Most/all VMX-based hypervisors blindly reflect #AC exceptions
> > into the guest, or don't intercept #AC in the first place. With SLD
> > enabled, this results in unexpected #AC faults in the guest, leading to
> > crashes in the guest and other undesirable behavior.
> >
> > Reported-by: "Kenneth R. Crudup" <kenny@panix.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Jessica Yu <jeyu@kernel.org>
> > Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> > Cc: Kenneth R. Crudup <kenny@panix.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Fenghua Yu <fenghua.yu@intel.com>
> > Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> > Cc: Nadav Amit <namit@vmware.com>
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Jann Horn <jannh@google.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: David Laight <David.Laight@ACULAB.COM>
> > Cc: Doug Covelli <dcovelli@vmware.com>
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> >
> > A bit ugly, but on the plus side the code is largely contained to intel.c.
> > I think forgoing the on_all_cpus() remote kill is safe?
>
> How would it be safe? You can't control where the module text will be
> ran, or how quickly.
Ugh, I forgot about the stupid core scope behavior.
CR4.VMXE needs to be set on every logical CPU before that CPU can do VMXON
and enter a guest, so every CPU will come through this code and locally
disable SLD.
But, a SMT sibling could race on the WRMSR and re-enable SLD on the CPU
that just killed SLD. Waiting until other CPUs stop enabling SLD should
work. Something like this? Disclaimer, memory ordering isn't my forte.
static atomic_t enabling_sld = ATOMIC_INIT(0);
static void sld_update_msr(bool on)
{
u64 test_ctrl_val = msr_test_ctrl_cache;
if (on && !sld_killed)
test_ctrl_val |= MSR_TEST_CTRL_SPLIT_LOCK_DETECT;
if (test_ctrl_val & MSR_TEST_CTRL_SPLIT_LOCK_DETECT)
atomic_inc(&enabling_sld);
wrmsrl(MSR_TEST_CTRL, test_ctrl_val);
if (test_ctrl_val & MSR_TEST_CTRL_SPLIT_LOCK_DETECT)
atomic_dec(&enabling_sld);
}
void split_lock_cr4_write(unsigned long val)
{
u64 ctrl;
/*
* Out-of-tree hypervisors that aren't aware of split-lock will blindly
* reflect split-lock #AC into their guests. Kill split-lock detection
* if an unaware entity enables VMX.
*/
if (!static_cpu_has(X86_FEATURE_VMX) ||
!static_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT) ||
!(val & X86_CR4_VMXE) || atomic_read(&cr4_vmxe_split_lock_safe) ||
(native_read_cr4() & X86_CR4_VMXE))
return;
WARN_ON_ONCE(1);
/*
* Set the global kill flag to prevent re-enabling SLD, e.g. via
* switch_to_sld().
*/
WRITE_ONCE(sld_killed, true);
/*
* No need to forcefully disable SLD on other CPUs, they'll come here
* if/when they set CR4.VMXE. But, wait until no other threads are
* enabling SLD, i.e. have seen sld_killed, as the MSR may be shared
* by SMT siblings.
*/
while (atomic_read(&enabling_sld));
sld_update_msr(false);
}
next prev parent reply other threads:[~2020-04-03 17:20 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-03 16:30 [RFC PATCH] x86/split_lock: Disable SLD if an unaware (out-of-tree) module enables VMX Sean Christopherson
2020-04-03 16:42 ` Peter Zijlstra
2020-04-03 17:20 ` Sean Christopherson [this message]
2020-04-06 12:50 ` Christoph Hellwig
2020-04-06 14:04 ` Peter Zijlstra
2020-04-06 14:34 ` Peter Zijlstra
2020-04-06 15:24 ` Christoph Hellwig
2020-04-06 15:39 ` Christoph Hellwig
2020-04-06 16:01 ` Peter Zijlstra
2020-04-06 17:10 ` Christoph Hellwig
2020-04-06 18:39 ` Peter Zijlstra
2020-04-06 22:54 ` Andy Lutomirski
2020-04-08 9:12 ` Peter Zijlstra
2020-04-08 11:02 ` Christoph Hellwig
2020-04-06 21:37 ` 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=20200403172018.GD2701@linux.intel.com \
--to=sean.j.christopherson@intel.com \
--cc=David.Laight@ACULAB.COM \
--cc=bp@alien8.de \
--cc=dcovelli@vmware.com \
--cc=fenghua.yu@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=hpa@zytor.com \
--cc=jannh@google.com \
--cc=jeyu@kernel.org \
--cc=keescook@chromium.org \
--cc=kenny@panix.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=nadav.amit@gmail.com \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=rasmus.villemoes@prevas.dk \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=thellstrom@vmware.com \
--cc=tony.luck@intel.com \
--cc=x86@kernel.org \
--cc=xiaoyao.li@intel.com \
/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).