nestedhvm: do not translate INVALID_GFN

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 <[email protected]>

CC Tim Deegan <[email protected]>

Signature · 6 lines

---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

diff -r 8330198c3240 xen/arch/x86/mm/p2m.c

xen/arch/x86/mm/p2m.c +7 -0

@@ -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
[email protected]
http://lists.xen.org/xen-devel

Hi,

Quoted 1 line · Christoph Egger

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.

Quoted 1 line

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

xen/arch/x86/mm/p2m.c +1 -0

@@ -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 */

xen/arch/x86/mm/p2m.c +0 -0

     }
 
     return hostmode->gva_to_gfn(v, hostp2m, va, pfec);

Cheers,

Tim.

Quoted 12 lines

Found with Hyper-V.

Signed-off-by Christoph Egger <[email protected]>

CC Tim Deegan <[email protected]>

Signature · 6 lines

---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

Quoted 28 lines

diff -r 8330198c3240 xen/arch/x86/mm/p2m.c

xen/arch/x86/mm/p2m.c +7 -0

@@ -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
[email protected]
http://lists.xen.org/xen-devel

Quoted 29 lines · Tim Deegan

Hi,

Quoted 1 line · Christoph Egger

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.

Quoted 1 line

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

xen/arch/x86/mm/p2m.c +1 -0

@@ -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
 

xen/arch/x86/mm/p2m.c unsigned long paging_gva_to_gfn(struct v · line 1594

Quoted 4 lines

xen/arch/x86/mm/p2m.c +1 -1

         /* 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.

Quoted 31 lines · Christoph Egger

Quoted 29 lines · Tim Deegan

Hi,

Quoted 1 line · Christoph Egger

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.

Quoted 1 line

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

xen/arch/x86/mm/p2m.c +1 -0

@@ -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
 

Quoted 8 lines

xen/arch/x86/mm/p2m.c unsigned long paging_gva_to_gfn(struct v · line 1594

Quoted 4 lines

xen/arch/x86/mm/p2m.c +1 -1

         /* 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?

Quoted 4 lines

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.