From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH 1/8] KVM: MMU: combine unsync and unsync_children Date: Fri, 23 Dec 2011 12:11:22 +0800 Message-ID: <4EF3FF6A.8030505@linux.vnet.ibm.com> References: <4EEB19AF.5070501@linux.vnet.ibm.com> <4EEB19D6.60101@linux.vnet.ibm.com> <20111222160641.GA7125@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , LKML , KVM To: Marcelo Tosatti Return-path: Received: from e28smtp09.in.ibm.com ([122.248.162.9]:40450 "EHLO e28smtp09.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753794Ab1LWELh (ORCPT ); Thu, 22 Dec 2011 23:11:37 -0500 Received: from /spool/local by e28smtp09.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 23 Dec 2011 09:41:35 +0530 In-Reply-To: <20111222160641.GA7125@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On 12/23/2011 12:06 AM, Marcelo Tosatti wrote: > On Fri, Dec 16, 2011 at 06:13:42PM +0800, Xiao Guangrong wrote: >> unsync is only used for the page of level == 1 and unsync_children is only >> used in the upper page, so combine them to reduce the size of shadow page >> structure >> >> Signed-off-by: Xiao Guangrong >> --- >> Documentation/virtual/kvm/mmu.txt | 5 ++- >> arch/x86/include/asm/kvm_host.h | 14 +++++++++++- >> arch/x86/kvm/mmu.c | 39 +++++++++++++++++++++++++----------- >> arch/x86/kvm/mmu_audit.c | 6 ++-- >> arch/x86/kvm/mmutrace.h | 3 +- >> arch/x86/kvm/paging_tmpl.h | 2 +- >> 6 files changed, 48 insertions(+), 21 deletions(-) >> >> diff --git a/Documentation/virtual/kvm/mmu.txt b/Documentation/virtual/kvm/mmu.txt >> index 5dc972c..4a5fedd 100644 >> --- a/Documentation/virtual/kvm/mmu.txt >> +++ b/Documentation/virtual/kvm/mmu.txt >> @@ -205,14 +205,15 @@ Shadow pages contain the following information: >> this page's spt. Otherwise, parent_ptes points at a data structure >> with a list of parent_ptes. >> unsync: >> + It is used when role.level == 1, only level 1 shadow pages can be unsync. >> If true, then the translations in this page may not match the guest's >> translation. This is equivalent to the state of the tlb when a pte is >> changed but before the tlb entry is flushed. Accordingly, unsync ptes >> are synchronized when the guest executes invlpg or flushes its tlb by >> other means. Valid for leaf pages. >> unsync_children: >> - How many sptes in the page point at pages that are unsync (or have >> - unsynchronized children). >> + It is used when role.level > 1 and indicates how many sptes in the page >> + point at unsync pages or unsynchronized children. >> unsync_child_bitmap: >> A bitmap indicating which sptes in spt point (directly or indirectly) at >> pages that may be unsynchronized. Used to quickly locate all unsychronized >> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >> index 52d6640..c0a89cd 100644 >> --- a/arch/x86/include/asm/kvm_host.h >> +++ b/arch/x86/include/asm/kvm_host.h >> @@ -233,9 +233,19 @@ struct kvm_mmu_page { >> * in this shadow page. >> */ >> DECLARE_BITMAP(slot_bitmap, KVM_MEM_SLOTS_NUM); >> - bool unsync; >> int root_count; /* Currently serving as active root */ >> - unsigned int unsync_children; >> + >> + /* >> + * If role.level == 1, unsync indicates whether the sp is >> + * unsync, otherwise unsync_children indicates the number >> + * of sptes which point at unsync sp or unsychronized children. >> + * See sp_is_unsync() / sp_unsync_children_num. >> + */ >> + union { >> + bool unsync; >> + unsigned int unsync_children; >> + }; >> + >> unsigned long parent_ptes; /* Reverse mapping for parent_pte */ >> DECLARE_BITMAP(unsync_child_bitmap, 512); >> >> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c >> index 2a2a9b4..6913a16 100644 >> --- a/arch/x86/kvm/mmu.c >> +++ b/arch/x86/kvm/mmu.c >> @@ -151,6 +151,21 @@ module_param(dbg, bool, 0644); >> >> #define SHADOW_PT_INDEX(addr, level) PT64_INDEX(addr, level) >> >> +static bool sp_is_unsync(struct kvm_mmu_page *sp) >> +{ >> + return sp->role.level == PT_PAGE_TABLE_LEVEL && sp->unsync; >> +} >> + >> +static unsigned int sp_unsync_children_num(struct kvm_mmu_page *sp) >> +{ >> + unsigned int num = 0; >> + >> + if (sp->role.level != PT_PAGE_TABLE_LEVEL) >> + num = sp->unsync_children; >> + >> + return num; >> +} > > IIRC there are windows where unsync_children is not accurate. Have you > verified this function is not called inside one of those windows? > Actually, this function is only used to see whether the sp has unsync children in current code. And this patch did not change the logic, it just use sp_unsync_children_num instead of using sp->unsync_children directly.