[Xen-devel] [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

No functional change (as confirmed by diffing the assembly).

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>

For some reason best known to GCC, there is one single change:

@@ -145835,7 +145835,7 @@
 ffff82d0802864f8:      85 c0                   test   %eax,%eax
 ffff82d0802864fa:      75 e0                   jne    ffff82d0802864dc 
<new_guest_cr3+0x7c>
 ffff82d0802864fc:      4c 8b ad 08 0b 00 00    mov    0xb08(%rbp),%r13
-ffff82d080286503:      49 39 dd                cmp    %rbx,%r13
+ffff82d080286503:      4c 39 eb                cmp    %r13,%rbx
 ffff82d080286506:      0f 84 74 01 00 00       je     ffff82d080286680 
<new_guest_cr3+0x220>
 ffff82d08028650c:      41 f6 84 24 19 06 00    testb  $0x8,0x619(%r12)
 ffff82d080286513:      00 08

This is from the mfn_eq() alteration, and must be a side effect from using an
inline function.  The net result is still correct, as only the zero flag is
checked.
---
 xen/arch/x86/mm.c              | 35 ++++++++++++++++++-----------------
 xen/arch/x86/pv/emul-priv-op.c |  2 +-
 xen/include/asm-x86/mm.h       |  2 +-
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 1f23470..dc07b4f 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2772,23 +2772,23 @@ int vcpu_destroy_pagetables(struct vcpu *v)
     return rc != -EINTR ? rc : -ERESTART;
 }
 
-int new_guest_cr3(unsigned long mfn)
+int new_guest_cr3(mfn_t mfn)
 {
     struct vcpu *curr = current;
     struct domain *d = curr->domain;
     int rc;
-    unsigned long old_base_mfn;
+    mfn_t old_base_mfn;
 
     if ( is_pv_32bit_domain(d) )
     {
-        unsigned long gt_mfn = pagetable_get_pfn(curr->arch.guest_table);
-        l4_pgentry_t *pl4e = map_domain_page(_mfn(gt_mfn));
+        mfn_t mmfn = pagetable_get_mfn(curr->arch.guest_table);
+        l4_pgentry_t *pl4e = map_domain_page(mmfn);
 
         rc = mod_l4_entry(pl4e,
-                          l4e_from_pfn(mfn,
+                          l4e_from_mfn(mfn,
                                        (_PAGE_PRESENT | _PAGE_RW |
                                         _PAGE_USER | _PAGE_ACCESSED)),
-                          gt_mfn, 0, curr);
+                          mfn_x(mmfn), 0, curr);
         unmap_domain_page(pl4e);
         switch ( rc )
         {
@@ -2800,7 +2800,7 @@ int new_guest_cr3(unsigned long mfn)
         default:
             gdprintk(XENLOG_WARNING,
                      "Error while installing new compat baseptr %" PRI_mfn 
"\n",
-                     mfn);
+                     mfn_x(mfn));
             return rc;
         }
 
@@ -2814,20 +2814,20 @@ int new_guest_cr3(unsigned long mfn)
     if ( unlikely(rc) )
         return rc;
 
-    old_base_mfn = pagetable_get_pfn(curr->arch.guest_table);
+    old_base_mfn = pagetable_get_mfn(curr->arch.guest_table);
     /*
      * This is particularly important when getting restarted after the
      * previous attempt got preempted in the put-old-MFN phase.
      */
-    if ( old_base_mfn == mfn )
+    if ( mfn_eq(old_base_mfn, mfn) )
     {
         write_ptbase(curr);
         return 0;
     }
 
     rc = paging_mode_refcounts(d)
-         ? (get_page_from_mfn(_mfn(mfn), d) ? 0 : -EINVAL)
-         : get_page_and_type_from_mfn(_mfn(mfn), PGT_root_page_table, d, 0, 1);
+         ? (get_page_from_mfn(mfn, d) ? 0 : -EINVAL)
+         : get_page_and_type_from_mfn(mfn, PGT_root_page_table, d, 0, 1);
     switch ( rc )
     {
     case 0:
@@ -2837,22 +2837,23 @@ int new_guest_cr3(unsigned long mfn)
         return -ERESTART;
     default:
         gdprintk(XENLOG_WARNING,
-                 "Error while installing new baseptr %" PRI_mfn "\n", mfn);
+                 "Error while installing new baseptr %" PRI_mfn "\n",
+                 mfn_x(mfn));
         return rc;
     }
 
     invalidate_shadow_ldt(curr, 0);
 
     if ( !VM_ASSIST(d, m2p_strict) && !paging_mode_refcounts(d) )
-        fill_ro_mpt(_mfn(mfn));
-    curr->arch.guest_table = pagetable_from_pfn(mfn);
+        fill_ro_mpt(mfn);
+    curr->arch.guest_table = pagetable_from_mfn(mfn);
     update_cr3(curr);
 
     write_ptbase(curr);
 
-    if ( likely(old_base_mfn != 0) )
+    if ( likely(mfn_x(old_base_mfn) != 0) )
     {
-        struct page_info *page = mfn_to_page(_mfn(old_base_mfn));
+        struct page_info *page = mfn_to_page(old_base_mfn);
 
         if ( paging_mode_refcounts(d) )
             put_page(page);
@@ -3180,7 +3181,7 @@ long do_mmuext_op(
             else if ( unlikely(paging_mode_translate(currd)) )
                 rc = -EINVAL;
             else
-                rc = new_guest_cr3(op.arg1.mfn);
+                rc = new_guest_cr3(_mfn(op.arg1.mfn));
             break;
 
         case MMUEXT_NEW_USER_BASEPTR: {
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index af1624a..54a63c2 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -774,7 +774,7 @@ static int priv_op_write_cr(unsigned int reg, unsigned long 
val,
         page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC);
         if ( !page )
             break;
-        rc = new_guest_cr3(mfn_x(page_to_mfn(page)));
+        rc = new_guest_cr3(page_to_mfn(page));
         put_page(page);
 
         switch ( rc )
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index ec7ce3c..4c03a33 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -539,7 +539,7 @@ void audit_domains(void);
 
 #endif
 
-int new_guest_cr3(unsigned long pfn);
+int new_guest_cr3(mfn_t mfn);
 void make_cr3(struct vcpu *v, unsigned long mfn);
 void update_cr3(struct vcpu *v);
 int vcpu_destroy_pagetables(struct vcpu *);
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel