All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Xiaoyao Li <xiaoyao.li@intel.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	x86@kernel.org, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Arvind Sankar <nivedita@alum.mit.edu>
Subject: Re: [PATCH v8 4/4] kvm: vmx: virtualize split lock detection
Date: Mon, 4 May 2020 20:07:36 -0700	[thread overview]
Message-ID: <20200505030736.GA20916@linux.intel.com> (raw)
In-Reply-To: <20200415214318.GH30627@linux.intel.com>

On Wed, Apr 15, 2020 at 02:43:18PM -0700, Sean Christopherson wrote:
> On Wed, Apr 15, 2020 at 11:22:11PM +0200, Thomas Gleixner wrote:
> > Sean Christopherson <sean.j.christopherson@intel.com> writes:
> > > I don't see any way to avoid having KVM differentiate between sld_warn and
> > > sld_fatal.  Even if KVM is able to virtualize SLD in sld_fatal mode, e.g.
> > > by telling the guest it must not try to disable SLD, KVM would still need
> > > to know the kernel is sld_fatal so that it can forward that information to
> > > the guest.
> > 
> > Huch? There is absolutely zero code like that. The only place where
> > sld_state is used is:
> > 
> > + static inline void vmx_update_sld(struct kvm_vcpu *vcpu, bool on)
> > + {
> > +	if (sld_state == sld_warn && guest_cpu_has_feature_sld(vcpu) &&
> > +	    on == test_thread_flag(TIF_SLD)) {
> > +		    sld_update_msr(on);
> > +		    update_thread_flag(TIF_SLD, !on);
> > +	}
> > 
> > You might have some faint memories from the previous trainwrecks :)
> 
> Yeah, I was thinking SLD was only being exposed if the host is sld_warn.
> I'll work with Xiaoyao to figure out a cleaner interface for this code.

...

> > So we can go with the proposed mode of allowing the write but not
> > propagating it. If the resulting split lock #AC originates from CPL != 3
> > then the guest will be killed with SIGBUS. If it originates from CPL ==
> > 3 and the guest has user #AC disabled then it will be killed as well.
> 
> An idea that's been floated around to avoid killing the guest on a CPL==3
> split-lock #AC is to add a STICKY bit to MSR_TEST_CTRL that KVM can
> virtualize to tell the guest that attempting to disable SLD is futile,
> e.g. so that the guest can kill its misbehaving userspace apps instead of
> trying to disable SLD and getting killed by the host.

Circling back to this.  KVM needs access to sld_state in one form or another
if we want to add a KVM hint when the host is in fatal mode.  Three options
I've come up with:

  1. Bite the bullet and export sld_state.  

  2. Add an is_split_fatal_wrapper().  Ugly since it needs to be non-inline
     to avoid triggering (1).

  3. Add a synthetic feature flag, e.g. X86_FEATURE_SLD_FATAL, and drop
     sld_state altogether.

I like (3) because it requires the least amount of code when all is said
and done, doesn't require more exports, and as a bonus it'd probably be nice
for userspace to see sld_fatal in /proc/cpuinfo.

Thoughts?

  reply	other threads:[~2020-05-05  3:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-14  6:31 [PATCH v8 0/4] KVM: Add virtualization support of split lock detection Xiaoyao Li
2020-04-14  6:31 ` [PATCH v8 1/4] kvm: x86: Emulate MSR IA32_CORE_CAPABILITIES Xiaoyao Li
2020-04-14  6:31 ` [PATCH v8 2/4] kvm: vmx: Enable MSR TEST_CTRL for guest Xiaoyao Li
2020-04-14  6:31 ` [PATCH v8 3/4] x86/split_lock: Export sld_update_msr() and sld_state Xiaoyao Li
2020-04-14  6:31 ` [PATCH v8 4/4] kvm: vmx: virtualize split lock detection Xiaoyao Li
2020-04-15 17:43   ` Thomas Gleixner
2020-04-15 19:18     ` Sean Christopherson
2020-04-15 21:22       ` Thomas Gleixner
2020-04-15 21:43         ` Sean Christopherson
2020-05-05  3:07           ` Sean Christopherson [this message]
2020-05-06 12:12             ` Thomas Gleixner
2020-04-16 12:34         ` Xiaoyao Li
2020-04-16 13:33           ` Thomas Gleixner
2020-04-15 19:47   ` Thomas Gleixner
2020-04-16  2:12     ` Xiaoyao Li

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=20200505030736.GA20916@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=bp@alien8.de \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=nivedita@alum.mit.edu \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.