Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 22 Jun 2023 14:32:53 -0700
In-Reply-To: <20230622081953.jc4tw6cwczl7bc6j@linux.intel.com>
Mime-Version: 1.0
References: <20230613203037.1968489-1-seanjc@google.com> <20230613203037.1968489-2-seanjc@google.com>
 <20230622081953.jc4tw6cwczl7bc6j@linux.intel.com>
Message-ID: <ZJS+BdDFg+qd1SyA@google.com>
Subject: Re: [PATCH 1/3] KVM: x86: Disallow KVM_SET_SREGS{2} if incoming CR0
 is invalid
From:   Sean Christopherson <seanjc@google.com>
To:     Yu Zhang <yu.c.zhang@linux.intel.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        syzbot+5feef0b9ee9c8e9e5689@syzkaller.appspotmail.com,
        Jim Mattson <jmattson@google.com>
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Thu, Jun 22, 2023, Yu Zhang wrote:
> On Tue, Jun 13, 2023 at 01:30:35PM -0700, Sean Christopherson wrote:
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 0ecf4be2c6af..355b0e8c9b00 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -3037,6 +3037,15 @@ static void enter_rmode(struct kvm_vcpu *vcpu)
> >  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> >  	struct kvm_vmx *kvm_vmx = to_kvm_vmx(vcpu->kvm);
> >  
> > +	/*
> > +	 * KVM should never use VM86 to virtualize Real Mode when L2 is active,
> > +	 * as using VM86 is unnecessary if unrestricted guest is enabled, and
> > +	 * if unrestricted guest is disabled, VM-Enter (from L1) with CR0.PG=0
> > +	 * should VM-Fail and KVM should reject userspace attempts to stuff
> 
> VM Enry shall fail(with CR0.PG=0), because SECONDARY_EXEC_UNRESTRICTED_GUEST
> will be cleared in L1's secondary_ctls_high MSR, and hence in its VMCS12?

Yep.

> 
> When will an unrestricted L1 run L2 as a restricted one? Shadow on EPT(L0
> uses EPT for L1 and L1 uses shadow for L2)?

Ya, the L1 VMM/hypervisor disabling EPT is the most likely scenario, i.e. the only
thing I would expect to encounter outside of testing.  Other than testing, e.g. to
ensure compatibility with Nehalem CPUs (the only Intel CPUs with EPT but not URG),
I don't know of any reason to disable URG but not EPT.