From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH v3 03/13] nEPT: Add EPT tables support to paging_tmpl.h Date: Tue, 11 Jun 2013 14:32:54 +0300 Message-ID: <20130611113254.GK4725@redhat.com> References: <1368939152-11406-1-git-send-email-jun.nakajima@intel.com> <1368939152-11406-3-git-send-email-jun.nakajima@intel.com> <519B27AC.6090903@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Jun Nakajima , kvm@vger.kernel.org, Paolo Bonzini To: Xiao Guangrong Return-path: Received: from mx1.redhat.com ([209.132.183.28]:53594 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754868Ab3FKLdA (ORCPT ); Tue, 11 Jun 2013 07:33:00 -0400 Content-Disposition: inline In-Reply-To: <519B27AC.6090903@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, May 21, 2013 at 03:52:12PM +0800, Xiao Guangrong wrote: > On 05/19/2013 12:52 PM, Jun Nakajima wrote: > > From: Nadav Har'El > > > > This is the first patch in a series which adds nested EPT support to KVM's > > nested VMX. Nested EPT means emulating EPT for an L1 guest so that L1 can use > > EPT when running a nested guest L2. When L1 uses EPT, it allows the L2 guest > > to set its own cr3 and take its own page faults without either of L0 or L1 > > getting involved. This often significanlty improves L2's performance over the > > previous two alternatives (shadow page tables over EPT, and shadow page > > tables over shadow page tables). > > > > This patch adds EPT support to paging_tmpl.h. > > > > paging_tmpl.h contains the code for reading and writing page tables. The code > > for 32-bit and 64-bit tables is very similar, but not identical, so > > paging_tmpl.h is #include'd twice in mmu.c, once with PTTTYPE=32 and once > > with PTTYPE=64, and this generates the two sets of similar functions. > > > > There are subtle but important differences between the format of EPT tables > > and that of ordinary x86 64-bit page tables, so for nested EPT we need a > > third set of functions to read the guest EPT table and to write the shadow > > EPT table. > > > > So this patch adds third PTTYPE, PTTYPE_EPT, which creates functions (prefixed > > with "EPT") which correctly read and write EPT tables. > > > > Signed-off-by: Nadav Har'El > > Signed-off-by: Jun Nakajima > > Signed-off-by: Xinhao Xu > > --- > > arch/x86/kvm/mmu.c | 5 +++++ > > arch/x86/kvm/paging_tmpl.h | 43 +++++++++++++++++++++++++++++++++++++++++-- > > 2 files changed, 46 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > > index 117233f..6c1670f 100644 > > --- a/arch/x86/kvm/mmu.c > > +++ b/arch/x86/kvm/mmu.c > > @@ -3397,6 +3397,11 @@ static inline bool is_last_gpte(struct kvm_mmu *mmu, unsigned level, unsigned gp > > return mmu->last_pte_bitmap & (1 << index); > > } > > > > +#define PTTYPE_EPT 18 /* arbitrary */ > > +#define PTTYPE PTTYPE_EPT > > +#include "paging_tmpl.h" > > +#undef PTTYPE > > + > > #define PTTYPE 64 > > #include "paging_tmpl.h" > > #undef PTTYPE > > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > > index df34d4a..4c45654 100644 > > --- a/arch/x86/kvm/paging_tmpl.h > > +++ b/arch/x86/kvm/paging_tmpl.h > > @@ -50,6 +50,22 @@ > > #define PT_LEVEL_BITS PT32_LEVEL_BITS > > #define PT_MAX_FULL_LEVELS 2 > > #define CMPXCHG cmpxchg > > +#elif PTTYPE == PTTYPE_EPT > > + #define pt_element_t u64 > > + #define guest_walker guest_walkerEPT > > + #define FNAME(name) EPT_##name > > + #define PT_BASE_ADDR_MASK PT64_BASE_ADDR_MASK > > + #define PT_LVL_ADDR_MASK(lvl) PT64_LVL_ADDR_MASK(lvl) > > + #define PT_LVL_OFFSET_MASK(lvl) PT64_LVL_OFFSET_MASK(lvl) > > + #define PT_INDEX(addr, level) PT64_INDEX(addr, level) > > + #define PT_LEVEL_BITS PT64_LEVEL_BITS > > + #ifdef CONFIG_X86_64 > > + #define PT_MAX_FULL_LEVELS 4 > > + #define CMPXCHG cmpxchg > > + #else > > + #define CMPXCHG cmpxchg64 > > CMPXHG is only used in FNAME(cmpxchg_gpte), but you commented it later. > Do we really need it? > > > + #define PT_MAX_FULL_LEVELS 2 > > And the SDM says: > > "It uses a page-walk length of 4, meaning that at most 4 EPT paging-structure > entriesare accessed to translate a guest-physical address.", Is My SDM obsolete? > Which kind of process supports page-walk length = 2? > > It seems your patch is not able to handle the case that the guest uses walk-lenght = 2 > which is running on the host with walk-lenght = 4. > (plrease refer to how to handle sp->role.quadrant in FNAME(get_level1_sp_gpa) in > the current code.) > But since EPT always has 4 levels on all existing cpus it is not an issue and the only case that we should worry about is guest walk-lenght == host walk-lenght == 4, or have I misunderstood what you mean here? -- Gleb.