x86/pagewalk: Consistently use guest_walk_*() helpers for translation

hap_p2m_ga_to_gfn() and sh_page_fault() currently use guest_l1e_get_gfn() to
obtain the translation of a pagewalk.  This is conceptually wrong (the
semantics of gw.l1e is an internal detail), and will actually be wrong when
PSE36 superpage support is fixed.  Switch them to using guest_walk_to_gfn().

guest_walk_tables() also uses guest_l1e_get_gfn(), and is updated for
consistency.

Take the opportunity to const-correct the walk_t parameter of the
guest_walk_to_*() helpers, and implement guest_walk_to_gpa() in terms of
guest_walk_to_gfn() to avoid duplicating the actual translation calculation.

While editing guest_walk_to_gpa(), fix a latent bug by causing it to return
INVALID_PADDR rather than 0 for a failed translation, as 0 is also a valid
successful result.  The sole caller, sh_page_fault(), has already confirmed
that the translation is valid, so this doesn't cause a behavioural change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
Acked-by: George Dunlap <george.dunlap@citrix.com>

diff --git a/xen/arch/x86/mm/guest_walk.c b/xen/arch/x86/mm/guest_walk.c
index 9d11e3b0789ce1852e63634491d15cf972c681de..8187226e621c7c8667db5a35b17e9d5974bfa116 100644 (file)
--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -438,7 +438,7 @@ set_ad:
 
     /* If this guest has a restricted physical address space then the
      * target GFN must fit within it. */
-    if ( !(rc & _PAGE_PRESENT) && !gfn_valid(d, guest_l1e_get_gfn(gw->l1e)) )
+    if ( !(rc & _PAGE_PRESENT) && !gfn_valid(d, guest_walk_to_gfn(gw)) )
         rc |= _PAGE_INVALID_BITS;
 
     return rc;

diff --git a/xen/arch/x86/mm/hap/guest_walk.c b/xen/arch/x86/mm/hap/guest_walk.c
index 569a4951047469f2acea367934f4d47825f434c1..313f82f0b0c8c11abbd053d2af0223f86682f520 100644 (file)
--- a/xen/arch/x86/mm/hap/guest_walk.c
+++ b/xen/arch/x86/mm/hap/guest_walk.c
@@ -98,7 +98,7 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)(
     /* Interpret the answer */
     if ( missing == 0 )
     {
-        gfn_t gfn = guest_l1e_get_gfn(gw.l1e);
+        gfn_t gfn = guest_walk_to_gfn(&gw);
         struct page_info *page;
         page = get_page_from_gfn_p2m(p2m->domain, p2m, gfn_x(gfn), &p2mt,
                                      NULL, P2M_ALLOC | P2M_UNSHARE);

diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 68ef35c97b07ecdaa93fdcbe572ce2c4d43dbcfc..7ea9d81bf528bdec09544ec7285863322517cd65 100644 (file)
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3104,7 +3104,7 @@ static int sh_page_fault(struct vcpu *v,
     }
 
     /* What mfn is the guest trying to access? */
-    gfn = guest_l1e_get_gfn(gw.l1e);
+    gfn = guest_walk_to_gfn(&gw);
     gmfn = get_gfn(d, gfn, &p2mt);
 
     if ( shadow_mode_refcounts(d) &&

diff --git a/xen/include/asm-x86/guest_pt.h b/xen/include/asm-x86/guest_pt.h
index 0bf6cf93c56848ab1a6e134be25f8ba9e87c9b88..6a06ba037fc4c798ba0d2843b254ecc078dfc651 100644 (file)
--- a/xen/include/asm-x86/guest_pt.h
+++ b/xen/include/asm-x86/guest_pt.h
@@ -241,8 +241,7 @@ struct guest_pagetable_walk
 
 /* Given a walk_t, translate the gw->va into the guest's notion of the
  * corresponding frame number. */
-static inline gfn_t
-guest_walk_to_gfn(walk_t *gw)
+static inline gfn_t guest_walk_to_gfn(const walk_t *gw)
 {
     if ( !(guest_l1e_get_flags(gw->l1e) & _PAGE_PRESENT) )
         return INVALID_GFN;
@@ -251,19 +250,19 @@ guest_walk_to_gfn(walk_t *gw)
 
 /* Given a walk_t, translate the gw->va into the guest's notion of the
  * corresponding physical address. */
-static inline paddr_t
-guest_walk_to_gpa(walk_t *gw)
+static inline paddr_t guest_walk_to_gpa(const walk_t *gw)
 {
-    if ( !(guest_l1e_get_flags(gw->l1e) & _PAGE_PRESENT) )
-        return 0;
-    return ((paddr_t)gfn_x(guest_l1e_get_gfn(gw->l1e)) << PAGE_SHIFT) +
-           (gw->va & ~PAGE_MASK);
+    gfn_t gfn = guest_walk_to_gfn(gw);
+
+    if ( gfn_eq(gfn, INVALID_GFN) )
+        return INVALID_PADDR;
+
+    return (gfn_x(gfn) << PAGE_SHIFT) | (gw->va & ~PAGE_MASK);
 }
 
 /* Given a walk_t from a successful walk, return the page-order of the 
  * page or superpage that the virtual address is in. */
-static inline unsigned int 
-guest_walk_to_page_order(walk_t *gw)
+static inline unsigned int guest_walk_to_page_order(const walk_t *gw)
 {
     /* This is only valid for successful walks - otherwise the 
      * PSE bits might be invalid. */