From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH] nestedhvm: do not translate INVALID_GFN Date: Thu, 2 Aug 2012 12:35:00 +0100 Message-ID: <20120802113500.GG11437@ocelot.phlegethon.org> References: <5017FBB0.7060500@amd.com> <20120802111915.GF11437@ocelot.phlegethon.org> <501A6478.8070605@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <501A6478.8070605@amd.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Christoph Egger Cc: "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org At 13:28 +0200 on 02 Aug (1343914136), Christoph Egger wrote: > On 08/02/12 13:19, Tim Deegan wrote: > > > Hi, > > > > At 17:37 +0200 on 31 Jul (1343756240), Christoph Egger wrote: > >> Do not translate INVALID_GFN as l2 guest gfn into l1 guest gfn. > > > > Why not? l2 gfns don't have any special meaning that we can > > dictate from inside Xen. > > > >> Pass correct pfec for translation into l1 guest gfn. > > > > This seems like a good idea, but probably should happen for all > > entries, not just INVALID_GFN ones -- we shouldn't be returning a PFEC > > to the guest that comes from translations outside his control. > > > > How about this: > > > > diff -r fdd4b7b36959 xen/arch/x86/mm/p2m.c > > --- a/xen/arch/x86/mm/p2m.c Thu Aug 02 12:04:31 2012 +0100 > > +++ b/xen/arch/x86/mm/p2m.c Thu Aug 02 12:17:48 2012 +0100 > > @@ -1581,6 +1581,7 @@ unsigned long paging_gva_to_gfn(struct v > > unsigned long gfn; > > struct p2m_domain *p2m; > > const struct paging_mode *mode; > > + uint32_t pfec_21 = *pfec; > > uint64_t ncr3 = nhvm_vcpu_hostcr3(v); > > > > /* translate l2 guest va into l2 guest gfn */ > > @@ -1590,7 +1591,7 @@ unsigned long paging_gva_to_gfn(struct v > > > > /* translate l2 guest gfn into l1 guest gfn */ > > return hostmode->p2m_ga_to_gfn(v, hostp2m, ncr3, > > - gfn << PAGE_SHIFT, pfec, NULL); > > + gfn << PAGE_SHIFT, &pfec_21, NULL); > > > The caller will see the return value of pfec and not from pfec_21. > If this is what the caller expects then this is fine with me. Yes, I think that is what the caller expects -- the error code is made up from the pagetable walk rather than from the p2m table. Can I take that as an ack? And more importantly, does it fix the Hyper-V problem you encountered? Cheers, Tim