From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH v4 11/13] nEPT: Advertise EPT to L1
Date: Mon, 29 Jul 2013 14:11:16 +0300
Message-ID: <20130729111115.GE18009@redhat.com>
References: <1374750001-28527-1-git-send-email-gleb@redhat.com>
 <1374750001-28527-12-git-send-email-gleb@redhat.com>
 <51F63422.4020406@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org,
	Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Yang Zhang <yang.z.zhang@intel.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:7619 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751112Ab3G2LLT (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 29 Jul 2013 07:11:19 -0400
Content-Disposition: inline
In-Reply-To: <51F63422.4020406@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Jul 29, 2013 at 11:21:38AM +0200, Paolo Bonzini wrote:
> Il 25/07/2013 12:59, Gleb Natapov ha scritto:
> > +	if (enable_ept) {
> > +		/* nested EPT: emulate EPT also to L1 */
> > +		nested_vmx_secondary_ctls_high |= SECONDARY_EXEC_ENABLE_EPT;
> > +		nested_vmx_ept_caps = VMX_EPT_PAGE_WALK_4_BIT;
> > +		nested_vmx_ept_caps |=
> > +			VMX_EPT_INVEPT_BIT | VMX_EPT_EXTENT_GLOBAL_BIT |
> > +			VMX_EPT_EXTENT_CONTEXT_BIT |
> > +			VMX_EPT_EXTENT_INDIVIDUAL_BIT;
> 
> What version of the Intel manual defines bit 24?  Mine is January 2013
> and only has 20/25/26.
> 
Good question. Looks like this crept in while Jun rebased Nadavs patch
on more resent master. This bit was defined by initial EPT implementation,
but was dropped by commit 2b3c5cbc0d814437fe4d70cc11ed60550b95b29f not
long time ago.

> > +		nested_vmx_ept_caps &= vmx_capability.ept;
> 
> This is always missing VMX_EPT_EXECUTE_ONLY_BIT, should it be added
> before the "&=".
I am not at all sure our current shadow implementation can support
execute only pages. Best to leave it off for now.

> 
> Also, the three extent bits should always be fine for the MSR,
> independent of the host support, because the processor will do the
> INVEPT vmexit before checking the INVEPT type against the processor
> capabilities.  So they can be added after the "&=".
> 
Good point.

> Related to this, I suppose enable_ept should depend on
> VMX_EPT_INVEPT_BIT too (it currently doesn't) since vmx_flush_tlb does
> an unconditional invept under "if (enable_ept)".  This is really just
> nitpicking though.
> 
> Paolo
> 
> > +	} else
> > +		nested_vmx_ept_caps = 0;

--
			Gleb.