All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yu Zhang <yu.c.zhang@linux.intel.com>
To: Sean Christopherson <seanjc@google.com>
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] Revert "KVM: x86: WARN and reject loading KVM if NX is supported but not enabled"
Date: Tue, 13 Jul 2021 11:59:44 +0800	[thread overview]
Message-ID: <20210713035944.l7qa7q4qsmqywg6u@linux.intel.com> (raw)
In-Reply-To: <YOxThZrKeyONVe4i@google.com>

On Mon, Jul 12, 2021 at 02:36:53PM +0000, Sean Christopherson wrote:
> On Mon, Jul 12, 2021, Yu Zhang wrote:
> > On Fri, Jul 09, 2021 at 05:21:52PM +0000, Sean Christopherson wrote:
> > > On Thu, Jul 08, 2021, Paolo Bonzini wrote:
> > > > So do we want this or "depends on X86_64 || X86_PAE"?
> > > 
> > > Hmm, I'm leaning towards keeping !PAE support purely for testing the !PAE<->PAE
> > > MMU transitions for nested virtualization.  It's not much coverage, and the !PAE
> > 
> > May I ask what "!PAE<->PAE MMU transition for nested virtualization" means?
> > Running L1 KVM with !PAE and L0 in PAE? I had thought KVM can only function
> > with PAE set(though I did not see any check of CR4 in kvm_arch_init()). Did
> > I miss something?
> 
> When L1 uses shadow paging, L0 KVM's uses a single MMU instance for both L1 and
> L2, and relies on the MMU role to differentiate between L1 and L2.  KVM requires
> PAE for shadow paging, but does not require PAE in the host kernel.  So when L1
> KVM uses shadow paging, it can effectively use !PAE paging for L1 and PAE paging
> for L2.  L0 KVM needs to handle that the !PAE<->PAE transitions when switching
> between L1 and L2, e.g. needs to correctly reinitialize the MMU context.

Hah... Actually, I do have a misunderstanding here. The host does not need to be
PAE. Thanks for the explanation! :)

> 
> > > NPT horror is a much bigger testing gap (because KVM doesn't support it), but on
> > > the other hand setting EFER.NX for !PAE kernels appears to be trivial, e.g.
> > > 
> > > diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
> > > index 67f590425d90..bfbea25a9fe8 100644
> > > --- a/arch/x86/kernel/head_32.S
> > > +++ b/arch/x86/kernel/head_32.S
> > > @@ -214,12 +214,6 @@ SYM_FUNC_START(startup_32_smp)
> > >         andl $~1,%edx                   # Ignore CPUID.FPU
> > >         jz .Lenable_paging              # No flags or only CPUID.FPU = no CR4
> > > 
> > > -       movl pa(mmu_cr4_features),%eax
> > > -       movl %eax,%cr4
> > > -
> > > -       testb $X86_CR4_PAE, %al         # check if PAE is enabled
> > > -       jz .Lenable_paging
> > > -
> > >         /* Check if extended functions are implemented */
> > >         movl $0x80000000, %eax
> > >         cpuid
> > > 
> > > My only hesitation is the risk of somehow breaking ancient CPUs by falling into
> > > the NX path.  Maybe try forcing EFER.NX=1 for !PAE, and fall back to requiring
> > > PAE if that gets NAK'd or needs to be reverted for whatever reason?
> > > 
> > 
> > One more dumb question: are you planning to set NX for linux with !PAE?
> 
> Yep.
> 
> > Why do we need EFER in that case? Thanks! :)
> 
> Because as you rightly remembered above, KVM always uses PAE paging for the guest,
> even when the host is !PAE.  And KVM also requires EFER.NX=1 for the guest when
> using shadow paging to handle a potential SMEP and !WP case.  
> 

Just saw this in update_transition_efer(), which now enables efer.nx in shadow
unconditionally. But I guess the host kernel still needs to set efer.nx for
!PAE(e.g. in head_32.S), because the guest may not touch efer at all. Is this
correct?

B.R.
Yu

  reply	other threads:[~2021-07-13  3:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-25  0:18 [PATCH] Revert "KVM: x86: WARN and reject loading KVM if NX is supported but not enabled" Sean Christopherson
2021-07-08 16:17 ` Paolo Bonzini
2021-07-09 17:21   ` Sean Christopherson
2021-07-09 17:32     ` Paolo Bonzini
2021-07-12  7:52     ` Yu Zhang
2021-07-12 14:36       ` Sean Christopherson
2021-07-13  3:59         ` Yu Zhang [this message]
2021-07-21 21:28           ` Sean Christopherson

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=20210713035944.l7qa7q4qsmqywg6u@linux.intel.com \
    --to=yu.c.zhang@linux.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=seanjc@google.com \
    --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 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.