* [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).