From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Roedel, Joerg" Subject: Re: [PATCH 2/3] KVM: MMU: Introduce a helper to read guest pte Date: Wed, 20 Apr 2011 11:35:15 +0200 Message-ID: <20110420093515.GX2192@amd.com> References: <20110419033220.e527bcae.takuya.yoshikawa@gmail.com> <20110419033453.ffa856c9.takuya.yoshikawa@gmail.com> <4DAEA240.9020104@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: Takuya Yoshikawa , "mtosatti@redhat.com" , "kvm@vger.kernel.org" , "yoshikawa.takuya@oss.ntt.co.jp" To: Avi Kivity Return-path: Received: from ch1ehsobe002.messaging.microsoft.com ([216.32.181.182]:11188 "EHLO ch1outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750792Ab1DTJfY (ORCPT ); Wed, 20 Apr 2011 05:35:24 -0400 Content-Disposition: inline In-Reply-To: <4DAEA240.9020104@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Apr 20, 2011 at 05:07:12AM -0400, Avi Kivity wrote: > On 04/18/2011 09:34 PM, Takuya Yoshikawa wrote: > > From: Takuya Yoshikawa > > > > This will be optimized later. > > > > Signed-off-by: Takuya Yoshikawa > > --- > > arch/x86/kvm/paging_tmpl.h | 12 +++++++++--- > > 1 files changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > > index 74f8567..109939a 100644 > > --- a/arch/x86/kvm/paging_tmpl.h > > +++ b/arch/x86/kvm/paging_tmpl.h > > @@ -109,6 +109,14 @@ static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, pt_element_t gpte) > > return access; > > } > > > > +static int FNAME(read_guest_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > > + gfn_t table_gfn, int offset, pt_element_t *ptep) > > +{ > > + return kvm_read_guest_page_mmu(vcpu, mmu, table_gfn, ptep, > > + offset, sizeof(*ptep), > > + PFERR_USER_MASK | PFERR_WRITE_MASK); > > +} > > + > > /* > > * Fetch a guest pte for a guest virtual address > > */ > > @@ -160,9 +168,7 @@ walk: > > walker->table_gfn[walker->level - 1] = table_gfn; > > walker->pte_gpa[walker->level - 1] = pte_gpa; > > > > - if (kvm_read_guest_page_mmu(vcpu, mmu, table_gfn,&pte, > > - offset, sizeof(pte), > > - PFERR_USER_MASK|PFERR_WRITE_MASK)) { > > + if (FNAME(read_guest_pte)(vcpu, mmu, table_gfn, offset,&pte)) { > > present = false; > > break; > > } > > > I think it's better to avoid a separate function for this. The reason > is I'd like to use ptep_user for cmpxchg_gpte() later on in > walk_addr_generic(), so we use the same calculation for both read and > write. So please just inline the new code in walk_addr_generic(). > > In fact there's probably a bug there for nested npt - we use > gfn_to_page(table_gfn), but table_gfn is actually an ngfn, not a gfn. > Joerg, am I right here? This patch seems only to introduce another wrapper around kvm_read_guest_page_mmu(), so I don't see a problem in this patch. The kvm_read_guest_page_mmu takes care whether it gets a l1-gfn or l2-gfn (by calling mmu->translate_gpa). Regards, Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632