From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [RFC PATCH 3/3] KVM: MMU: Optimize guest page table walk Date: Wed, 20 Apr 2011 12:02:27 +0300 Message-ID: <4DAEA123.3020403@redhat.com> References: <20110419033220.e527bcae.takuya.yoshikawa@gmail.com> <20110419033814.3cc7ab5e.takuya.yoshikawa@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org, yoshikawa.takuya@oss.ntt.co.jp To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:25715 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752834Ab1DTJCe (ORCPT ); Wed, 20 Apr 2011 05:02:34 -0400 In-Reply-To: <20110419033814.3cc7ab5e.takuya.yoshikawa@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 04/18/2011 09:38 PM, Takuya Yoshikawa wrote: > From: Takuya Yoshikawa > > We optimize multi level guest page table walk as follows: > > 1. We cache the memslot which, probably, includes the next guest page > tables to avoid searching for it many times. > 2. We use get_user() instead of copy_from_user(). > > Note that this is kind of a restricted way of Xiao's more generic > work: "KVM: optimize memslots searching and cache GPN to GFN." Good optimization. copy_from_user() really isn't optimized for short buffers, I expect much of the improvement comes from that. > +/* > + * Read the guest PTE refered to by table_gfn and offset and put it into ptep. > + * > + * *slot_hint, if not NULL, should point to a memslot which probably includes > + * the guest PTE. The actual memslot will be put back into this so that > + * callers can cache it. > + */ Please drop the slot_hint optimization. First, it belongs in a separate patch. Second, I prefer to see a generic slot sort instead of an ad-hoc cache. > static int FNAME(read_guest_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > - gfn_t table_gfn, int offset, pt_element_t *ptep) > + gfn_t table_gfn, int offset, pt_element_t *ptep, > + struct kvm_memory_slot **slot_hint) > { > - return kvm_read_guest_page_mmu(vcpu, mmu, table_gfn, ptep, > - offset, sizeof(*ptep), > - PFERR_USER_MASK | PFERR_WRITE_MASK); > + unsigned long addr; > + pt_element_t __user *ptep_user; > + gfn_t real_gfn; > + > + real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn), > + PFERR_USER_MASK | PFERR_WRITE_MASK); > + if (real_gfn == UNMAPPED_GVA) > + return -EFAULT; > + > + real_gfn = gpa_to_gfn(real_gfn); > + > + if (!(*slot_hint) || !gfn_in_memslot(*slot_hint, real_gfn)) > + *slot_hint = gfn_to_memslot(vcpu->kvm, real_gfn); > + > + addr = gfn_to_hva_memslot(*slot_hint, real_gfn); > + if (kvm_is_error_hva(addr)) > + return -EFAULT; > + > + ptep_user = (pt_element_t __user *)((void *)addr + offset); > + return get_user(*ptep, ptep_user); > } > > /* > @@ -130,6 +155,7 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker, > gpa_t pte_gpa; > bool eperm, present, rsvd_fault; > int offset, write_fault, user_fault, fetch_fault; > + struct kvm_memory_slot *slot_cache = NULL; > > write_fault = access& PFERR_WRITE_MASK; > user_fault = access& PFERR_USER_MASK; > @@ -168,7 +194,8 @@ walk: > walker->table_gfn[walker->level - 1] = table_gfn; > walker->pte_gpa[walker->level - 1] = pte_gpa; > > - if (FNAME(read_guest_pte)(vcpu, mmu, table_gfn, offset,&pte)) { > + if (FNAME(read_guest_pte)(vcpu, mmu, table_gfn, > + offset,&pte,&slot_cache)) { > present = false; > break; > } -- error compiling committee.c: too many arguments to function