x86/mm: slightly simplify mod_l1_entry()

Commit Message

Jan Beulich Feb. 17, 2016, 4:10 p.m. UTC

Re-order code to simplify error cleanup.

Signed-off-by: Jan Beulich <[e-mail]>
x86/mm: slightly simplify mod_l1_entry()

Re-order code to simplify error cleanup.

Signed-off-by: Jan Beulich <[e-mail]>

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1873,38 +1873,32 @@ static int mod_l1_entry(l1_pgentry_t *pl
     {
         /* Translate foreign guest addresses. */
         struct page_info *page = NULL;
-        if ( paging_mode_translate(pg_dom) )
-        {
-            page = get_page_from_gfn(pg_dom, l1e_get_pfn(nl1e), NULL, P2M_ALLOC);
-            if ( !page )
-                return -EINVAL;
-            nl1e = l1e_from_pfn(page_to_mfn(page), l1e_get_flags(nl1e));
-        }
 
         if ( unlikely(l1e_get_flags(nl1e) & l1_disallow_mask(pt_dom)) )
         {
             MEM_LOG("Bad L1 flags %x",
                     l1e_get_flags(nl1e) & l1_disallow_mask(pt_dom));
-            if ( page )
-                put_page(page);
             return -EINVAL;
         }
 
+        if ( paging_mode_translate(pg_dom) )
+        {
+            page = get_page_from_gfn(pg_dom, l1e_get_pfn(nl1e), NULL, P2M_ALLOC);
+            if ( !page )
+                return -EINVAL;
+            nl1e = l1e_from_pfn(page_to_mfn(page), l1e_get_flags(nl1e));
+        }
+
         /* Fast path for identical mapping, r/w, presence, and cachability. */
         if ( !l1e_has_changed(ol1e, nl1e,
                               PAGE_CACHE_ATTRS | _PAGE_RW | _PAGE_PRESENT) )
         {
             adjust_guest_l1e(nl1e, pt_dom);
-            if ( UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
-                              preserve_ad) )
-            {
-                if ( page )
-                    put_page(page);
-                return 0;
-            }
+            rc = UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
+                              preserve_ad);
             if ( page )
                 put_page(page);
-            return -EBUSY;
+            return rc ? 0 : -EBUSY;
         }
 
         switch ( rc = get_page_from_l1e(nl1e, pt_dom, pg_dom) )

Comments

Andrew Cooper Feb. 17, 2016, 5:41 p.m. UTC | #1

On 17/02/16 16:10, Jan Beulich wrote:
> Re-order code to simplify error cleanup.
>
> Signed-off-by: Jan Beulich <[e-mail]>

Reviewed-by: Andrew Cooper <[e-mail]>

Patch

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1873,38 +1873,32 @@  static int mod_l1_entry(l1_pgentry_t *pl
     {
         /* Translate foreign guest addresses. */
         struct page_info *page = NULL;
-        if ( paging_mode_translate(pg_dom) )
-        {
-            page = get_page_from_gfn(pg_dom, l1e_get_pfn(nl1e), NULL, P2M_ALLOC);
-            if ( !page )
-                return -EINVAL;
-            nl1e = l1e_from_pfn(page_to_mfn(page), l1e_get_flags(nl1e));
-        }
 
         if ( unlikely(l1e_get_flags(nl1e) & l1_disallow_mask(pt_dom)) )
         {
             MEM_LOG("Bad L1 flags %x",
                     l1e_get_flags(nl1e) & l1_disallow_mask(pt_dom));
-            if ( page )
-                put_page(page);
             return -EINVAL;
         }
 
+        if ( paging_mode_translate(pg_dom) )
+        {
+            page = get_page_from_gfn(pg_dom, l1e_get_pfn(nl1e), NULL, P2M_ALLOC);
+            if ( !page )
+                return -EINVAL;
+            nl1e = l1e_from_pfn(page_to_mfn(page), l1e_get_flags(nl1e));
+        }
+
         /* Fast path for identical mapping, r/w, presence, and cachability. */
         if ( !l1e_has_changed(ol1e, nl1e,
                               PAGE_CACHE_ATTRS | _PAGE_RW | _PAGE_PRESENT) )
         {
             adjust_guest_l1e(nl1e, pt_dom);
-            if ( UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
-                              preserve_ad) )
-            {
-                if ( page )
-                    put_page(page);
-                return 0;
-            }
+            rc = UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
+                              preserve_ad);
             if ( page )
                 put_page(page);
-            return -EBUSY;
+            return rc ? 0 : -EBUSY;
         }
 
         switch ( rc = get_page_from_l1e(nl1e, pt_dom, pg_dom) )