[xen stable-4.12] x86/p2m: fix PoD accounting in guest_physmap_add_entry()

[patchbot@xxxxxxx]

commit 2f4cfe5a234b4f788aad93e08b9994c87d09343e
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Wed Aug 25 15:45:21 2021 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Aug 25 15:45:21 2021 +0200

    x86/p2m: fix PoD accounting in guest_physmap_add_entry()
    
    The initial observation was that the mfn_valid() check comes too late:
    Neither mfn_add() nor mfn_to_page() (let alone de-referencing the
    result of the latter) are valid for MFNs failing this check. Move it up
    and - noticing that there's no caller doing so - also add an assertion
    that this should never produce "false" here.
    
    In turn this would have meant that the "else" to that if() could now go
    away, which didn't seem right at all. And indeed, considering callers
    like memory_exchange() or various grant table functions, the PoD
    accounting should have been outside of that if() from the very
    beginning.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    master commit: aea270e3f7c0db696c88a0e94b1ece7abd339c84
    master date: 2020-02-21 17:14:38 +0100
---
 xen/arch/x86/mm/p2m.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 25ce19f20b..eb5e340577 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -864,6 +864,12 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t 
mfn,
     if ( p2m_is_foreign(t) )
         return -EINVAL;
 
+    if ( !mfn_valid(mfn) )
+    {
+        ASSERT_UNREACHABLE();
+        return -EINVAL;
+    }
+
     p2m_lock(p2m);
 
     P2M_DEBUG("adding gfn=%#lx mfn=%#lx\n", gfn_x(gfn), mfn_x(mfn));
@@ -963,12 +969,15 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, 
mfn_t mfn,
     }
 
     /* Now, actually do the two-way mapping */
-    if ( mfn_valid(mfn) )
+    rc = p2m_set_entry(p2m, gfn, mfn, page_order, t, p2m->default_access);
+    if ( rc == 0 )
     {
-        rc = p2m_set_entry(p2m, gfn, mfn, page_order, t,
-                           p2m->default_access);
-        if ( rc )
-            goto out; /* Failed to update p2m, bail without updating m2p. */
+#ifdef CONFIG_HVM
+        pod_lock(p2m);
+        p2m->pod.entry_count -= pod_count;
+        BUG_ON(p2m->pod.entry_count < 0);
+        pod_unlock(p2m);
+#endif
 
         if ( !p2m_is_grant(t) )
         {
@@ -977,24 +986,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t 
mfn,
                                   gfn_x(gfn_add(gfn, i)));
         }
     }
-    else
-    {
-        gdprintk(XENLOG_WARNING, "Adding bad mfn to p2m map (%#lx -> %#lx)\n",
-                 gfn_x(gfn), mfn_x(mfn));
-        rc = p2m_set_entry(p2m, gfn, INVALID_MFN, page_order,
-                           p2m_invalid, p2m->default_access);
-#ifdef CONFIG_HVM
-        if ( rc == 0 )
-        {
-            pod_lock(p2m);
-            p2m->pod.entry_count -= pod_count;
-            BUG_ON(p2m->pod.entry_count < 0);
-            pod_unlock(p2m);
-        }
-#endif
-    }
 
-out:
     p2m_unlock(p2m);
 
     return rc;
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.12