From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH v3 03/13] nEPT: Add EPT tables support to paging_tmpl.h Date: Tue, 18 Jun 2013 20:51:25 +0800 Message-ID: <51C057CD.6010907@linux.vnet.ibm.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> <20130611113254.GK4725@redhat.com> <51BEFCD7.70504@linux.vnet.ibm.com> <20130618105745.GF5832@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Jun Nakajima , kvm@vger.kernel.org, Paolo Bonzini To: Gleb Natapov Return-path: Received: from e28smtp02.in.ibm.com ([122.248.162.2]:41369 "EHLO e28smtp02.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754497Ab3FRMve (ORCPT ); Tue, 18 Jun 2013 08:51:34 -0400 Received: from /spool/local by e28smtp02.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 18 Jun 2013 18:13:51 +0530 Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by d28dlp01.in.ibm.com (Postfix) with ESMTP id 98080E004F for ; Tue, 18 Jun 2013 18:20:53 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r5ICpYou32047196 for ; Tue, 18 Jun 2013 18:21:34 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r5ICpSNm022702 for ; Tue, 18 Jun 2013 22:51:28 +1000 In-Reply-To: <20130618105745.GF5832@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 06/18/2013 06:57 PM, Gleb Natapov wrote: > On Mon, Jun 17, 2013 at 08:11:03PM +0800, Xiao Guangrong wrote: >> On 06/11/2013 07:32 PM, Gleb Natapov wrote: >>> 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 >> >> Yes. I totally agree with you, but... >> >>> misunderstood what you mean here? >> >> What confused me is that this patch defines "#define PT_MAX_FULL_LEVELS 2", so i asked the >> question: "Which kind of process supports page-walk length = 2". >> Sorry, there is a typo in my origin comments. "process" should be "processor" or "CPU". >> > That is how I understood it, but then the discussion moved to dropping > of nEPT support on 32-bit host. What's the connection? Even on 32bit If EPT supports "walk-level = 2" on 32bit host (maybe it is not true), i thought dropping 32bit support to reduce the complex is worthwhile, otherwise: a) we need to handle different page size between L0 and L2 and b) we need to carefully review the code due to lacking PDPT supporting on nept on L2. I remember that the origin version of NEPT did not support PAE-32bit L2 guest. I have found it out: http://comments.gmane.org/gmane.comp.emulators.kvm.devel/95395 It seems no changes in this version, I have no idea how it was fixed in this version. > host the walk is 4 levels. Doesn't shadow page code support 4 levels on > 32bit host? Yes, it does. 4 levels is fine on 32bit host. If EPT only supports 4 levels on both 32bit and 64bit hosts, there is no big difference to support nept on 32bit L2 and 64bit L2.