[Xen-changelog] [xen staging] x86/mm: Clean up p2m_finish_type_change return value

[patchbot@xxxxxxx]

commit 0c07431cf712d990b3c60341b0b435c903bbf4f4
Author:     Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>
AuthorDate: Wed Apr 10 11:08:39 2019 +0100
Commit:     George Dunlap <george.dunlap@xxxxxxxxxx>
CommitDate: Wed Apr 10 11:08:39 2019 +0100

    x86/mm: Clean up p2m_finish_type_change return value
    
    In the case of any errors, finish_type_change() passes values returned
    from p2m->recalc() up the stack (with some exceptions in the case where
    an error is expected); this eventually ends up being returned to the
    XEN_DOMOP_map_mem_type_to_ioreq_server hypercall.
    
    However, on Intel processors (but not on AMD processor), p2m->recalc()
    can also return '1' as well as '0'.  This case is handled very
    inconsistently: finish_type_change() will return the value of the final
    entry it attempts, discarding results for other entries;
    p2m_finish_type_change() will attempt to accumulate '1's, so that it
    returns '1' if any of the calls to finish_type_change() returns '1'; and
    dm_op() will again return '1' only if the very last call to
    p2m_finish_type_change() returns '1'.  The result is that the
    XEN_DMOP_map_mem_type_to_ioreq_server() hypercall will sometimes return
    0 and sometimes return 1 on success, in an unpredictable manner.
    
    The hypercall documentation doesn't mention return values; but it's not
    clear what the caller could do with the information about whether
    entries had been changed or not.  At the moment it's always 0 on AMD
    boxes, and *usually* 1 on Intel boxes; so nothing can be relying on a
    '1' return value for correctness (or if it is, it's broken).
    
    Make the return value on success consistently '0' by only returning
    0/-ERROR from finish_type_change().  Also remove the accumulation code
    from p2m_finish_type_change().
    
    Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
    Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/mm/p2m.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index b9bbb8f485..9e81a30cc4 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1158,7 +1158,7 @@ void p2m_change_type_range(struct domain *d,
  * Finish p2m type change for gfns which are marked as need_recalc in a range.
  * Uses the current p2m's max_mapped_pfn to further clip the invalidation
  * range for alternate p2ms.
- * Returns: 0/1 for success, negative for failure
+ * Returns: 0 for success, negative for failure
  */
 static int finish_type_change(struct p2m_domain *p2m,
                               gfn_t first_gfn, unsigned long max_nr)
@@ -1174,9 +1174,9 @@ static int finish_type_change(struct p2m_domain *p2m,
         /*
          * ept->recalc could return 0/1/-ENOMEM. pt->recalc could return
          * 0/-ENOMEM/-ENOENT, -ENOENT isn't an error as we are looping
-         * gfn here.
+         * gfn here. If rc is 1 we need to have it 0 for success.
          */
-        if ( rc == -ENOENT )
+        if ( rc == -ENOENT || rc > 0 )
             rc = 0;
         else if ( rc < 0 )
         {
@@ -1213,19 +1213,13 @@ int p2m_finish_type_change(struct domain *d,
             if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             {
                 struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
-                int rc1;
 
                 p2m_lock(altp2m);
-                rc1 = finish_type_change(altp2m, first_gfn, max_nr);
+                rc = finish_type_change(altp2m, first_gfn, max_nr);
                 p2m_unlock(altp2m);
 
-                if ( rc1 < 0 )
-                {
-                    rc = rc1;
+                if ( rc < 0 )
                     goto out;
-                }
-
-                rc |= rc1;
             }
     }
 #endif
--
generated by git-patchbot for /home/xen/git/xen.git#staging

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog