[PATCH v3 1/6] x86: Remove return value from UPDATE_ENTRY

[Kevin Lampis <kevin.lampis@xxxxxxxxxx>]

UPDATE_ENTRY/update_intpte always returns true since 1bc30c076a7f
"x86/mm: {paging, sh}_{cmpxchg, write}_guest_entry() cannot fault"

Signed-off-by: Kevin Lampis <kevin.lampis@xxxxxxxxxx>
---
Changes in v3:
- None

Changes in v2:
- New patch
---
 xen/arch/x86/mm.c               | 74 +++++++++------------------------
 xen/arch/x86/pv/grant_table.c   | 53 +++++++++++------------
 xen/arch/x86/pv/mm.h            |  6 +--
 xen/arch/x86/pv/ro-page-fault.c |  3 +-
 4 files changed, 46 insertions(+), 90 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index b1bbecc7d7f7..1ddb8b71469a 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2215,11 +2215,10 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, 
l1_pgentry_t nl1e,
         /* Fast path for sufficiently-similar mappings. */
         if ( !l1e_has_changed(ol1e, nl1e, ~FASTPATH_FLAG_WHITELIST) )
         {
-            rc = UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
-                              update_flags);
+            UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu, update_flags);
             if ( page )
                 put_page(page);
-            return rc ? 0 : -EBUSY;
+            return 0;
         }
 
         switch ( rc = get_page_from_l1e(nl1e, pt_dom, pg_dom) )
@@ -2239,20 +2238,12 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, 
l1_pgentry_t nl1e,
         if ( page )
             put_page(page);
 
-        if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
-                                    update_flags)) )
-        {
-            ol1e = nl1e;
-            rc = -EBUSY;
-        }
+        UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu, update_flags);
     }
     else if ( pv_l1tf_check_l1e(pt_dom, nl1e) )
         return -ERESTART;
-    else if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
-                                     update_flags)) )
-    {
-        return -EBUSY;
-    }
+    else
+        UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu, update_flags);
 
     put_page_from_l1e(ol1e, pt_dom);
     return rc;
@@ -2295,28 +2286,19 @@ static int mod_l2_entry(l2_pgentry_t *pl2e,
         /* Fast path for sufficiently-similar mappings. */
         if ( !l2e_has_changed(ol2e, nl2e, ~FASTPATH_PDE_FLAG_WHITELIST) )
         {
-            if ( UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu, update_flags) )
-                return 0;
-            return -EBUSY;
+            UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu, update_flags);
+            return 0;
         }
 
         if ( unlikely((rc = get_page_from_l2e(nl2e, mfn, d, 0)) < 0) )
             return rc;
 
-        if ( unlikely(!UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu,
-                                    update_flags)) )
-        {
-            ol2e = nl2e;
-            rc = -EBUSY;
-        }
+        UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu, update_flags);
     }
     else if ( pv_l1tf_check_l2e(d, nl2e) )
         return -ERESTART;
-    else if ( unlikely(!UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu,
-                                     update_flags)) )
-    {
-        return -EBUSY;
-    }
+    else
+        UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu, update_flags);
 
     put_page_from_l2e(ol2e, mfn, PTF_defer);
 
@@ -2357,8 +2339,8 @@ static int mod_l3_entry(l3_pgentry_t *pl3e,
         /* Fast path for sufficiently-similar mappings. */
         if ( !l3e_has_changed(ol3e, nl3e, ~FASTPATH_PDE_FLAG_WHITELIST) )
         {
-            rc = UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu, update_flags);
-            return rc ? 0 : -EFAULT;
+            UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu, update_flags);
+            return 0;
         }
 
         rc = get_page_from_l3e(nl3e, mfn, d, 0);
@@ -2366,20 +2348,12 @@ static int mod_l3_entry(l3_pgentry_t *pl3e,
             return rc;
         rc = 0;
 
-        if ( unlikely(!UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu,
-                                    update_flags)) )
-        {
-            ol3e = nl3e;
-            rc = -EFAULT;
-        }
+        UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu, update_flags);
     }
     else if ( pv_l1tf_check_l3e(d, nl3e) )
         return -ERESTART;
-    else if ( unlikely(!UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu,
-                                     update_flags)) )
-    {
-        return -EFAULT;
-    }
+    else
+        UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu, update_flags);
 
     put_page_from_l3e(ol3e, mfn, PTF_defer);
     return rc;
@@ -2419,8 +2393,8 @@ static int mod_l4_entry(l4_pgentry_t *pl4e,
         /* Fast path for sufficiently-similar mappings. */
         if ( !l4e_has_changed(ol4e, nl4e, ~FASTPATH_PDE_FLAG_WHITELIST) )
         {
-            rc = UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu, update_flags);
-            return rc ? 0 : -EFAULT;
+            UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu, update_flags);
+            return 0;
         }
 
         rc = get_page_from_l4e(nl4e, mfn, d, 0);
@@ -2428,20 +2402,12 @@ static int mod_l4_entry(l4_pgentry_t *pl4e,
             return rc;
         rc = 0;
 
-        if ( unlikely(!UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu,
-                                    update_flags)) )
-        {
-            ol4e = nl4e;
-            rc = -EFAULT;
-        }
+        UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu, update_flags);
     }
     else if ( pv_l1tf_check_l4e(d, nl4e) )
         return -ERESTART;
-    else if ( unlikely(!UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu,
-                                     update_flags)) )
-    {
-        return -EFAULT;
-    }
+    else
+        UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu, update_flags);
 
     put_page_from_l4e(ol4e, mfn, PTF_defer);
     return rc;
diff --git a/xen/arch/x86/pv/grant_table.c b/xen/arch/x86/pv/grant_table.c
index 1df68440a24a..38767826f5d8 100644
--- a/xen/arch/x86/pv/grant_table.c
+++ b/xen/arch/x86/pv/grant_table.c
@@ -98,18 +98,16 @@ int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
         goto out_unlock;
 
     ol1e = *pl1e;
-    if ( UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, curr, 0) )
-    {
-        /*
-         * We always create mappings in this path.  However, our caller,
-         * map_grant_ref(), only passes potentially non-zero cache_flags for
-         * MMIO frames, so this path doesn't create non-coherent mappings of
-         * RAM frames and there's no need to calculate PGT_non_coherent.
-         */
-        ASSERT(!cache_flags || is_iomem_page(frame));
-
-        rc = GNTST_okay;
-    }
+    UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, curr, 0);
+    /*
+     * We always create mappings in this path.  However, our caller,
+     * map_grant_ref(), only passes potentially non-zero cache_flags for
+     * MMIO frames, so this path doesn't create non-coherent mappings of
+     * RAM frames and there's no need to calculate PGT_non_coherent.
+     */
+    ASSERT(!cache_flags || is_iomem_page(frame));
+
+    rc = GNTST_okay;
 
  out_unlock:
     page_unlock(page);
@@ -165,10 +163,9 @@ static bool steal_linear_address(unsigned long linear, 
l1_pgentry_t *out)
         goto out_unlock;
 
     ol1e = *pl1e;
-    okay = UPDATE_ENTRY(l1, pl1e, ol1e, l1e_empty(), gl1mfn, curr, 0);
-
-    if ( okay )
-        *out = ol1e;
+    UPDATE_ENTRY(l1, pl1e, ol1e, l1e_empty(), gl1mfn, curr, 0);
+    *out = ol1e;
+    okay = true;
 
  out_unlock:
     page_unlock(page);
@@ -293,19 +290,17 @@ int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
                  "PTE flags %x for %"PRIx64" don't match grant (%x)\n",
                  l1e_get_flags(ol1e), addr, grant_pte_flags);
 
-    if ( UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, curr, 0) )
-    {
-        /*
-         * Generally, replace_grant_pv_mapping() is used to destroy mappings
-         * (n1le = l1e_empty()), but it can be a present mapping on the
-         * GNTABOP_unmap_and_replace path.
-         *
-         * In such cases, the PTE is fully transplanted from its old location
-         * via steal_linear_addr(), so we need not perform PGT_non_coherent
-         * checking here.
-         */
-        rc = GNTST_okay;
-    }
+    UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, curr, 0);
+    /*
+     * Generally, replace_grant_pv_mapping() is used to destroy mappings
+     * (n1le = l1e_empty()), but it can be a present mapping on the
+     * GNTABOP_unmap_and_replace path.
+     *
+     * In such cases, the PTE is fully transplanted from its old location
+     * via steal_linear_addr(), so we need not perform PGT_non_coherent
+     * checking here.
+     */
+    rc = GNTST_okay;
 
  out_unlock:
     page_unlock(page);
diff --git a/xen/arch/x86/pv/mm.h b/xen/arch/x86/pv/mm.h
index 3aac0f3309a4..bfee0feb7b21 100644
--- a/xen/arch/x86/pv/mm.h
+++ b/xen/arch/x86/pv/mm.h
@@ -67,13 +67,10 @@ static inline intpte_t paging_cmpxchg_guest_entry(
 
 /*
  * How to write an entry to the guest pagetables.
- * Returns false for failure (pointer not valid), true for success.
  */
-static inline bool update_intpte(intpte_t *p, intpte_t old, intpte_t new,
+static inline void update_intpte(intpte_t *p, intpte_t old, intpte_t new,
                                  mfn_t mfn, struct vcpu *v, unsigned int flags)
 {
-    bool rv = true;
-
 #ifndef PTE_UPDATE_WITH_CMPXCHG
     if ( !(flags & PTE_UPDATE_PRESERVE_AD) )
         paging_write_guest_entry(v, p, new, mfn);
@@ -98,7 +95,6 @@ static inline bool update_intpte(intpte_t *p, intpte_t old, 
intpte_t new,
             old = t;
         }
     }
-    return rv;
 }
 
 /*
diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/ro-page-fault.c
index 34349e9437eb..f6baf371926b 100644
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/ro-page-fault.c
@@ -200,8 +200,7 @@ static int ptwr_emulated_update(unsigned long addr, 
intpte_t *p_old,
     else
     {
         ol1e = *pl1e;
-        if ( !UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, mfn, v, 0) )
-            BUG();
+        UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, mfn, v, 0);
     }
 
     trace_ptwr_emulation(addr, nl1e);
-- 
2.52.0