kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: stsp <stsp2@yandex.ru>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/6] KVM: x86: KVM_SET_SREGS.CR4 bug fixes and cleanup
Date: Fri, 9 Oct 2020 08:30:55 -0700	[thread overview]
Message-ID: <20201009153053.GA16234@linux.intel.com> (raw)
In-Reply-To: <5dfa55f3-ecdf-9f8d-2d45-d2e6e54f2daa@yandex.ru>

On Fri, Oct 09, 2020 at 05:11:51PM +0300, stsp wrote:
> 09.10.2020 07:04, Sean Christopherson пишет:
> >>Hmm. But at least it was lying
> >>similarly on AMD and Intel CPUs. :)
> >>So I was able to reproduce the problems
> >>myself.
> >>Do you mean, any AMD tests are now useless, and we need to proceed with Intel
> >>tests only?
> >For anything VMXE related, yes.
> 
> What would be the expected behaviour on Intel, if it is set? Any difference
> with AMD?

On Intel, userspace should be able to stuff CR4.VMXE=1 via KVM_SET_SREGS if
the 'nested' module param is 1, e.g. if 'modprobe kvm_intel nested=1'.  Note,
'nested' is enabled by default on kernel 5.0 and later.

With AMD, setting CR4.VMXE=1 is never allowed as AMD doesn't support VMX,
AMD's virtualization solution is called SVM (Secure Virtual Machine).  KVM
doesn't support nesting VMX within SVM and vice versa.

> >>Then additional question.
> >>On old Intel CPUs we needed to set VMXE in guest to make it to work in
> >>nested-guest mode.
> >>Is it still needed even with your patches?
> >>Or the nested-guest mode will work now even on older Intel CPUs and KVM will
> >>set VMXE for us itself, when needed?
> >I'm struggling to even come up with a theory as to how setting VMXE from
> >userspace would have impacted KVM with unrestricted_guest=n, let alone fixed
> >anything.
> >
> >CR4.VMXE must always be 1 in _hardware_ when VMX is on, including when running
> >the guest.  But KVM forces vmcs.GUEST_CR4.VMXE=1 at all times, regardless of
> >the guest's actual value (the guest sees a shadow value when it reads CR4).
> >
> >And unless I grossly misunderstand dosemu2, it's not doing anything related to
> >nested virtualization, i.e. the stuffing VMXE=1 for the guest's shadow value
> >should have absolutely zero impact.
> >
> >More than likely, VMXE was a red herring.
> 
> Yes, it was. :( (as you can see from the end of the github thread)
> 
> 
> >   Given that the reporter is also
> >seeing the same bug on bare metal after moving to kernel 5.4, odds are good
> >the issue is related to unrestricted_guest=n and has nothing to do with nVMX.
> 
> But we do not use unrestricted guest.
> We use v86 under KVM.

Unrestricted guest can kick in even if CR0.PG=1 && CR0.PE=1, e.g. there are
segmentation checks that apply if and only if unrestricted_guest=0.  Long story
short, without a deep audit, it's basically impossible to rule out a dependency
on unrestricted guest since you're playing around with v86.
 
> The only other effect of setting VMXE was clearing VME. Which shouldn't
> affect anything either, right?

Hmm, clearing VME would mean that exceptions/interrupts within the guest would
trigger a switch out of v86 and into vanilla protected mode.  v86 and PM have
different consistency checks, particularly for segmentation, so it's plausible
that clearing CR4.VME inadvertantly worked around the bug by avoiding invalid
guest state for v86.

  reply	other threads:[~2020-10-09 15:31 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07  1:44 [PATCH 0/6] KVM: x86: KVM_SET_SREGS.CR4 bug fixes and cleanup Sean Christopherson
2020-10-07  1:44 ` [PATCH 1/6] KVM: VMX: Drop guest CPUID check for VMXE in vmx_set_cr4() Sean Christopherson
2020-10-07  1:44 ` [PATCH 2/6] KVM: VMX: Drop explicit 'nested' check from vmx_set_cr4() Sean Christopherson
2020-10-07  1:44 ` [PATCH 3/6] KVM: SVM: Drop VMXE check from svm_set_cr4() Sean Christopherson
2020-10-07  1:44 ` [PATCH 4/6] KVM: x86: Move vendor CR4 validity check to dedicated kvm_x86_ops hook Sean Christopherson
2020-10-07  1:44 ` [PATCH 5/6] KVM: x86: Return bool instead of int for CR4 and SREGS validity checks Sean Christopherson
2020-10-07  1:44 ` [PATCH 6/6] KVM: selftests: Verify supported CR4 bits can be set before KVM_SET_CPUID2 Sean Christopherson
2020-10-08 16:00 ` [PATCH 0/6] KVM: x86: KVM_SET_SREGS.CR4 bug fixes and cleanup stsp
2020-10-08 17:59   ` Sean Christopherson
2020-10-08 18:18     ` stsp
2020-10-09  4:04       ` Sean Christopherson
2020-10-09 14:11         ` stsp
2020-10-09 15:30           ` Sean Christopherson [this message]
2020-10-09 15:48             ` stsp
2020-10-09 16:11               ` Sean Christopherson
2020-12-07 11:19             ` KVM_SET_CPUID doesn't check supported bits (was Re: [PATCH 0/6] KVM: x86: KVM_SET_SREGS.CR4 bug fixes and cleanup) stsp
2020-12-07 11:24             ` stsp
2020-12-07 11:29               ` Paolo Bonzini
2020-12-07 11:47                 ` stsp
     [not found]                   ` <CABgObfYS57_ez-t=eu9+3S2bhSXC_9DTj=64Sna2jnYEMYo2Ag@mail.gmail.com>
2020-12-07 14:03                     ` stsp
     [not found]                       ` <CABgObfb_4r=k_qakd+48hPar8rzc-P50+dgdoYvQaL2H-po6+g@mail.gmail.com>
2020-12-07 14:29                         ` stsp
     [not found]                           ` <CABgObfYN7Okdt+YfHtsd3M_00iuWf=UyKPmbQhhYBhoiMtdXuw@mail.gmail.com>
2020-12-07 14:41                             ` stsp
2020-12-07 23:59                   ` Jim Mattson
2020-11-13 11:36 ` [PATCH 0/6] KVM: x86: KVM_SET_SREGS.CR4 bug fixes and cleanup 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=20201009153053.GA16234@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=stsp2@yandex.ru \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.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).