[xen stable-4.17] x86/paging: Delete update_cr3()'s do_locking parameter

[patchbot@xxxxxxx]

commit bf70ce8b3449c49eb828d5b1f4934a49b00fef35
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Sep 20 20:06:53 2023 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Mar 12 16:13:03 2024 +0000

    x86/paging: Delete update_cr3()'s do_locking parameter
    
    Nicola reports that the XSA-438 fix introduced new MISRA violations because 
of
    some incidental tidying it tried to do.  The parameter is useless, so 
resolve
    the MISRA regression by removing it.
    
    hap_update_cr3() discards the parameter entirely, while sh_update_cr3() uses
    it to distinguish internal and external callers and therefore whether the
    paging lock should be taken.
    
    However, we have paging_lock_recursive() for this purpose, which also avoids
    the ability for the shadow internal callers to accidentally not hold the 
lock.
    
    Fixes: fb0ff49fe9f7 ("x86/shadow: defer releasing of PV's top-level shadow 
reference")
    Reported-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    Release-acked-by: Henry Wang <Henry.Wang@xxxxxxx>
    (cherry picked from commit e71157d1ac2a7fbf413130663cf0a93ff9fbcf7e)
---
 xen/arch/x86/include/asm/paging.h |  5 ++---
 xen/arch/x86/mm/hap/hap.c         |  5 ++---
 xen/arch/x86/mm/shadow/common.c   |  2 +-
 xen/arch/x86/mm/shadow/multi.c    | 17 ++++++++---------
 xen/arch/x86/mm/shadow/none.c     |  3 +--
 5 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/include/asm/paging.h 
b/xen/arch/x86/include/asm/paging.h
index 94c590f31a..809ff35d9a 100644
--- a/xen/arch/x86/include/asm/paging.h
+++ b/xen/arch/x86/include/asm/paging.h
@@ -138,8 +138,7 @@ struct paging_mode {
                                             paddr_t ga, uint32_t *pfec,
                                             unsigned int *page_order);
 #endif
-    pagetable_t   (*update_cr3            )(struct vcpu *v, bool do_locking,
-                                            bool noflush);
+    pagetable_t   (*update_cr3            )(struct vcpu *v, bool noflush);
     void          (*update_paging_modes   )(struct vcpu *v);
     bool          (*flush_tlb             )(const unsigned long *vcpu_bitmap);
 
@@ -312,7 +311,7 @@ static inline unsigned long paging_ga_to_gfn_cr3(struct 
vcpu *v,
  * as the value to load into the host CR3 to schedule this vcpu */
 static inline pagetable_t paging_update_cr3(struct vcpu *v, bool noflush)
 {
-    return paging_get_hostmode(v)->update_cr3(v, 1, noflush);
+    return paging_get_hostmode(v)->update_cr3(v, noflush);
 }
 
 /* Update all the things that are derived from the guest's CR0/CR3/CR4.
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 57a19c3d59..3ad39a7dd7 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -739,8 +739,7 @@ static bool cf_check hap_invlpg(struct vcpu *v, unsigned 
long linear)
     return 1;
 }
 
-static pagetable_t cf_check hap_update_cr3(
-    struct vcpu *v, bool do_locking, bool noflush)
+static pagetable_t cf_check hap_update_cr3(struct vcpu *v, bool noflush)
 {
     v->arch.hvm.hw_cr[3] = v->arch.hvm.guest_cr[3];
     hvm_update_guest_cr3(v, noflush);
@@ -826,7 +825,7 @@ static void cf_check hap_update_paging_modes(struct vcpu *v)
     }
 
     /* CR3 is effectively updated by a mode change. Flush ASIDs, etc. */
-    hap_update_cr3(v, 0, false);
+    hap_update_cr3(v, false);
 
  unlock:
     paging_unlock(d);
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index c0940f939e..18714dbd02 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -2579,7 +2579,7 @@ static void sh_update_paging_modes(struct vcpu *v)
     }
 #endif /* OOS */
 
-    v->arch.paging.mode->update_cr3(v, 0, false);
+    v->arch.paging.mode->update_cr3(v, false);
 }
 
 void cf_check shadow_update_paging_modes(struct vcpu *v)
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index c92b354a78..e54a507b54 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -2506,7 +2506,7 @@ static int cf_check sh_page_fault(
          * In any case, in the PAE case, the ASSERT is not true; it can
          * happen because of actions the guest is taking. */
 #if GUEST_PAGING_LEVELS == 3
-        v->arch.paging.mode->update_cr3(v, 0, false);
+        v->arch.paging.mode->update_cr3(v, false);
 #else
         ASSERT(d->is_shutting_down);
 #endif
@@ -3224,17 +3224,13 @@ static void cf_check sh_detach_old_tables(struct vcpu 
*v)
     }
 }
 
-static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool do_locking,
-                                          bool noflush)
+static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool noflush)
 /* Updates vcpu->arch.cr3 after the guest has changed CR3.
  * Paravirtual guests should set v->arch.guest_table (and guest_table_user,
  * if appropriate).
  * HVM guests should also make sure hvm_get_guest_cntl_reg(v, 3) works;
  * this function will call hvm_update_guest_cr(v, 3) to tell them where the
  * shadow tables are.
- * If do_locking != 0, assume we are being called from outside the
- * shadow code, and must take and release the paging lock; otherwise
- * that is the caller's responsibility.
  */
 {
     struct domain *d = v->domain;
@@ -3252,7 +3248,11 @@ static pagetable_t cf_check sh_update_cr3(struct vcpu 
*v, bool do_locking,
         return old_entry;
     }
 
-    if ( do_locking ) paging_lock(v->domain);
+    /*
+     * This is used externally (with the paging lock not taken) and internally
+     * by the shadow code (with the lock already taken).
+     */
+    paging_lock_recursive(v->domain);
 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
     /* Need to resync all the shadow entries on a TLB flush.  Resync
@@ -3480,8 +3480,7 @@ static pagetable_t cf_check sh_update_cr3(struct vcpu *v, 
bool do_locking,
     shadow_sync_other_vcpus(v);
 #endif
 
-    /* Release the lock, if we took it (otherwise it's the caller's problem) */
-    if ( do_locking ) paging_unlock(v->domain);
+    paging_unlock(v->domain);
 
     return old_entry;
 }
diff --git a/xen/arch/x86/mm/shadow/none.c b/xen/arch/x86/mm/shadow/none.c
index 743c0ffb85..7e4e386cd0 100644
--- a/xen/arch/x86/mm/shadow/none.c
+++ b/xen/arch/x86/mm/shadow/none.c
@@ -52,8 +52,7 @@ static unsigned long cf_check _gva_to_gfn(
 }
 #endif
 
-static pagetable_t cf_check _update_cr3(struct vcpu *v, bool do_locking,
-                                        bool noflush)
+static pagetable_t cf_check _update_cr3(struct vcpu *v, bool noflush)
 {
     ASSERT_UNREACHABLE();
     return pagetable_null();
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.17