From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH 1/1 v2] KVM: MMU: Use ptep_user for cmpxchg_gpte() Date: Wed, 4 May 2011 08:16:11 -0300 Message-ID: <20110504111611.GA26538@amt.cnet> References: <20110501143026.9eb3c875.takuya.yoshikawa@gmail.com> <20110501143307.1bcfd375.takuya.yoshikawa@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: avi@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]:48173 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751212Ab1EDLXp (ORCPT ); Wed, 4 May 2011 07:23:45 -0400 Content-Disposition: inline In-Reply-To: <20110501143307.1bcfd375.takuya.yoshikawa@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On Sun, May 01, 2011 at 02:33:07PM +0900, Takuya Yoshikawa wrote: > From: Takuya Yoshikawa > > The address of the gpte was already calculated and stored in ptep_user > before entering cmpxchg_gpte(). > > This patch makes cmpxchg_gpte() to use that to make it clear that we > are using the same address during walk_addr_generic(). > > Note that the unlikely annotations are used to show that the conditions > are something unusual rather than for performance. > > Signed-off-by: Takuya Yoshikawa > --- > arch/x86/kvm/paging_tmpl.h | 26 ++++++++++++-------------- > 1 files changed, 12 insertions(+), 14 deletions(-) Hi Takuya, > > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > index 52450a6..f9d9af1 100644 > --- a/arch/x86/kvm/paging_tmpl.h > +++ b/arch/x86/kvm/paging_tmpl.h > @@ -79,21 +79,19 @@ static gfn_t gpte_to_gfn_lvl(pt_element_t gpte, int lvl) > } > > static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > - gfn_t table_gfn, unsigned index, > - pt_element_t orig_pte, pt_element_t new_pte) > + pt_element_t __user *ptep_user, unsigned index, > + pt_element_t orig_pte, pt_element_t new_pte) > { > + int npages; > pt_element_t ret; > pt_element_t *table; > struct page *page; > - gpa_t gpa; > > - gpa = mmu->translate_gpa(vcpu, table_gfn << PAGE_SHIFT, > - PFERR_USER_MASK|PFERR_WRITE_MASK); > - if (gpa == UNMAPPED_GVA) > + npages = get_user_pages_fast((unsigned long)ptep_user, 1, 1, &page); > + /* Check if the user is doing something meaningless. */ > + if (unlikely(npages != 1)) > return -EFAULT; > > - page = gfn_to_page(vcpu->kvm, gpa_to_gfn(gpa)); > - gfn_to_page is the interface for mapping guest pages inside KVM, and you're bypassing it for IMO no good reason (i doubt there's any performance improvement by skipping the translation).