[xen master] x86/shadow: streamline shadow_get_page_from_l1e()

[patchbot@xxxxxxx]

commit 972ba1d1d4bcb77018b50fd2bb63c0e628859ed3
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Tue Apr 27 14:36:13 2021 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Apr 27 14:36:13 2021 +0200

    x86/shadow: streamline shadow_get_page_from_l1e()
    
    Trying get_page_from_l1e() up to three times isn't helpful; in debug
    builds it may lead to log messages making things look as if there was a
    problem somewhere. And there's no need to have more than one try: The
    function can only possibly succeed for one domain passed as 3rd
    argument (unless the page is an MMIO one to which both have access,
    but MMIO pages should be "got" by specifying the requesting domain
    anyway). Re-arrange things so just the one call gets made which has a
    chance of succeeding.
    
    The code could in principle be arranged such that there's only a single
    call to get_page_from_l1e(), but the conditional would become pretty
    complex then and hence hard to follow / understand / adjust.
    
    The redundant (with shadow_mode_refcounts()) shadow_mode_translate()
    gets dropped.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Tim Deegan <tim@xxxxxxx>
---
 xen/arch/x86/mm/shadow/set.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/mm/shadow/set.c b/xen/arch/x86/mm/shadow/set.c
index 8ef37c6f11..87e9c6eeb2 100644
--- a/xen/arch/x86/mm/shadow/set.c
+++ b/xen/arch/x86/mm/shadow/set.c
@@ -109,40 +109,36 @@ shadow_get_page_from_l1e(shadow_l1e_t sl1e, struct domain 
*d, p2m_type_t type)
     if ( pg && page_refcounting_suppressed(pg) )
         return 0;
 
-    res = get_page_from_l1e(sl1e, d, d);
+    if ( owner == dom_io )
+        owner = NULL;
 
     /*
      * If a privileged domain is attempting to install a map of a page it does
      * not own, we let it succeed anyway.
      */
-    if ( unlikely(res < 0) &&
-         !shadow_mode_translate(d) &&
-         owner && (d != owner) )
+    if ( owner && (d != owner) &&
+         !(res = xsm_priv_mapping(XSM_TARGET, d, owner)) )
     {
-        res = xsm_priv_mapping(XSM_TARGET, d, owner);
-        if ( !res )
-        {
-            res = get_page_from_l1e(sl1e, d, owner);
-            SHADOW_PRINTK("privileged %pd installs map of mfn %"PRI_mfn" owned 
by %pd: %s\n",
-                           d, mfn_x(mfn), owner,
-                           res >= 0 ? "success" : "failed");
-        }
+        res = get_page_from_l1e(sl1e, d, owner);
+        SHADOW_PRINTK("privileged %pd installs map of %pd's mfn %"PRI_mfn": 
%s\n",
+                      d, owner, mfn_x(mfn),
+                      res >= 0 ? "success" : "failed");
     }
-
     /* Okay, it might still be a grant mapping PTE.  Try it. */
-    if ( unlikely(res < 0) &&
-         (type == p2m_grant_map_rw ||
-          (type == p2m_grant_map_ro &&
-           !(shadow_l1e_get_flags(sl1e) & _PAGE_RW))) )
+    else if ( owner &&
+              (type == p2m_grant_map_rw ||
+               (type == p2m_grant_map_ro &&
+                !(shadow_l1e_get_flags(sl1e) & _PAGE_RW))) )
     {
         /*
          * It's a grant mapping.  The grant table implementation will
          * already have checked that we're supposed to have access, so
          * we can just grab a reference directly.
          */
-        if ( owner )
-            res = get_page_from_l1e(sl1e, d, owner);
+        res = get_page_from_l1e(sl1e, d, owner);
     }
+    else
+        res = get_page_from_l1e(sl1e, d, d);
 
     if ( unlikely(res < 0) )
     {
--
generated by git-patchbot for /home/xen/git/xen.git#master