* [PATCH] nestedhvm: do not translate INVALID_GFN
@ 2012-07-31 15:37 Christoph Egger
2012-08-02 11:19 ` Tim Deegan
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Egger @ 2012-07-31 15:37 UTC (permalink / raw)
To: xen-devel@lists.xen.org; +Cc: Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 476 bytes --]
Do not translate INVALID_GFN as l2 guest gfn into l1 guest gfn.
Pass correct pfec for translation into l1 guest gfn.
Found with Hyper-V.
Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
CC: Tim Deegan <tim@xen.org>
--
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
[-- Attachment #2: xen_p2m.diff --]
[-- Type: text/plain, Size: 958 bytes --]
diff -r 8330198c3240 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c Fri Jul 27 12:24:03 2012 +0200
+++ b/xen/arch/x86/mm/p2m.c Tue Jul 31 16:49:54 2012 +0200
@@ -1582,12 +1582,19 @@ unsigned long paging_gva_to_gfn(struct v
struct p2m_domain *p2m;
const struct paging_mode *mode;
uint64_t ncr3 = nhvm_vcpu_hostcr3(v);
+ uint32_t pfec1 = *pfec;
/* translate l2 guest va into l2 guest gfn */
p2m = p2m_get_nestedp2m(v, ncr3);
mode = paging_get_nestedmode(v);
gfn = mode->gva_to_gfn(v, p2m, va, pfec);
+ /* if l1 guest maps its mmio pages into the
+ * l2 guest then we see this case here. */
+ if (gfn == INVALID_GFN)
+ return INVALID_GFN;
+ *pfec = pfec1;
+
/* translate l2 guest gfn into l1 guest gfn */
return hostmode->p2m_ga_to_gfn(v, hostp2m, ncr3,
gfn << PAGE_SHIFT, pfec, NULL);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] nestedhvm: do not translate INVALID_GFN 2012-07-31 15:37 [PATCH] nestedhvm: do not translate INVALID_GFN Christoph Egger @ 2012-08-02 11:19 ` Tim Deegan 2012-08-02 11:28 ` Christoph Egger 0 siblings, 1 reply; 6+ messages in thread From: Tim Deegan @ 2012-08-02 11:19 UTC (permalink / raw) To: Christoph Egger; +Cc: xen-devel@lists.xen.org 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); } return hostmode->gva_to_gfn(v, hostp2m, va, pfec); Cheers, Tim. > Found with Hyper-V. > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> > CC: Tim Deegan <tim@xen.org> > > -- > ---to satisfy European Law for business letters: > Advanced Micro Devices GmbH > Einsteinring 24, 85689 Dornach b. Muenchen > Geschaeftsfuehrer: Alberto Bozzo > Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen > Registergericht Muenchen, HRB Nr. 43632 Content-Description: xen_p2m.diff > diff -r 8330198c3240 xen/arch/x86/mm/p2m.c > --- a/xen/arch/x86/mm/p2m.c Fri Jul 27 12:24:03 2012 +0200 > +++ b/xen/arch/x86/mm/p2m.c Tue Jul 31 16:49:54 2012 +0200 > @@ -1582,12 +1582,19 @@ unsigned long paging_gva_to_gfn(struct v > struct p2m_domain *p2m; > const struct paging_mode *mode; > uint64_t ncr3 = nhvm_vcpu_hostcr3(v); > + uint32_t pfec1 = *pfec; > > /* translate l2 guest va into l2 guest gfn */ > p2m = p2m_get_nestedp2m(v, ncr3); > mode = paging_get_nestedmode(v); > gfn = mode->gva_to_gfn(v, p2m, va, pfec); > > + /* if l1 guest maps its mmio pages into the > + * l2 guest then we see this case here. */ > + if (gfn == INVALID_GFN) > + return INVALID_GFN; > + *pfec = pfec1; > + > /* translate l2 guest gfn into l1 guest gfn */ > return hostmode->p2m_ga_to_gfn(v, hostp2m, ncr3, > gfn << PAGE_SHIFT, pfec, NULL); > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nestedhvm: do not translate INVALID_GFN 2012-08-02 11:19 ` Tim Deegan @ 2012-08-02 11:28 ` Christoph Egger 2012-08-02 11:35 ` Tim Deegan 0 siblings, 1 reply; 6+ messages in thread From: Christoph Egger @ 2012-08-02 11:28 UTC (permalink / raw) To: Tim Deegan; +Cc: xen-devel@lists.xen.org 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. Christoph > } > > return hostmode->gva_to_gfn(v, hostp2m, va, pfec); > > Cheers, > > Tim. > >> Found with Hyper-V. >> >> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> >> CC: Tim Deegan <tim@xen.org> >> >> -- >> ---to satisfy European Law for business letters: >> Advanced Micro Devices GmbH >> Einsteinring 24, 85689 Dornach b. Muenchen >> Geschaeftsfuehrer: Alberto Bozzo >> Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen >> Registergericht Muenchen, HRB Nr. 43632 > > Content-Description: xen_p2m.diff >> diff -r 8330198c3240 xen/arch/x86/mm/p2m.c >> --- a/xen/arch/x86/mm/p2m.c Fri Jul 27 12:24:03 2012 +0200 >> +++ b/xen/arch/x86/mm/p2m.c Tue Jul 31 16:49:54 2012 +0200 >> @@ -1582,12 +1582,19 @@ unsigned long paging_gva_to_gfn(struct v >> struct p2m_domain *p2m; >> const struct paging_mode *mode; >> uint64_t ncr3 = nhvm_vcpu_hostcr3(v); >> + uint32_t pfec1 = *pfec; >> >> /* translate l2 guest va into l2 guest gfn */ >> p2m = p2m_get_nestedp2m(v, ncr3); >> mode = paging_get_nestedmode(v); >> gfn = mode->gva_to_gfn(v, p2m, va, pfec); >> >> + /* if l1 guest maps its mmio pages into the >> + * l2 guest then we see this case here. */ >> + if (gfn == INVALID_GFN) >> + return INVALID_GFN; >> + *pfec = pfec1; >> + >> /* translate l2 guest gfn into l1 guest gfn */ >> return hostmode->p2m_ga_to_gfn(v, hostp2m, ncr3, >> gfn << PAGE_SHIFT, pfec, NULL); > >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel > > -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nestedhvm: do not translate INVALID_GFN 2012-08-02 11:28 ` Christoph Egger @ 2012-08-02 11:35 ` Tim Deegan 2012-08-02 12:14 ` Christoph Egger 0 siblings, 1 reply; 6+ messages in thread From: Tim Deegan @ 2012-08-02 11:35 UTC (permalink / raw) To: Christoph Egger; +Cc: xen-devel@lists.xen.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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nestedhvm: do not translate INVALID_GFN 2012-08-02 11:35 ` Tim Deegan @ 2012-08-02 12:14 ` Christoph Egger 2012-08-02 13:45 ` Tim Deegan 0 siblings, 1 reply; 6+ messages in thread From: Christoph Egger @ 2012-08-02 12:14 UTC (permalink / raw) To: Tim Deegan; +Cc: xen-devel@lists.xen.org On 08/02/12 13:35, Tim Deegan wrote: > 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? Yes. > And more importantly, does it fix the Hyper-V problem you encountered? The one you mean is covered with the other patch. But I found this with Hyper-V when doing MMIO accesses. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nestedhvm: do not translate INVALID_GFN 2012-08-02 12:14 ` Christoph Egger @ 2012-08-02 13:45 ` Tim Deegan 0 siblings, 0 replies; 6+ messages in thread From: Tim Deegan @ 2012-08-02 13:45 UTC (permalink / raw) To: Christoph Egger; +Cc: xen-devel@lists.xen.org At 14:14 +0200 on 02 Aug (1343916891), Christoph Egger wrote: > > 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? > > Yes. Thanks; I've applied it. Tim. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-08-02 13:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-31 15:37 [PATCH] nestedhvm: do not translate INVALID_GFN Christoph Egger 2012-08-02 11:19 ` Tim Deegan 2012-08-02 11:28 ` Christoph Egger 2012-08-02 11:35 ` Tim Deegan 2012-08-02 12:14 ` Christoph Egger 2012-08-02 13:45 ` Tim Deegan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).