[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen master] x86/paging: Delete update_cr3()'s do_locking parameter
commit e71157d1ac2a7fbf413130663cf0a93ff9fbcf7e Author: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> AuthorDate: Wed Sep 20 20:06:53 2023 +0100 Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> CommitDate: Fri Sep 22 16:00:17 2023 +0100 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> --- 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 8fad4cfc18..f291f2f9a2 100644 --- a/xen/arch/x86/include/asm/paging.h +++ b/xen/arch/x86/include/asm/paging.h @@ -118,8 +118,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); unsigned int guest_levels; @@ -296,7 +295,7 @@ static inline bool paging_flush_tlb(const unsigned long *vcpu_bitmap) * 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 e30f543d2c..9f964c1d87 100644 --- a/xen/arch/x86/mm/hap/hap.c +++ b/xen/arch/x86/mm/hap/hap.c @@ -707,8 +707,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); @@ -794,7 +793,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 8211e77cc7..8aa7b698f8 100644 --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -2510,7 +2510,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); } /* diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c index 447512870d..90cf0ceaa3 100644 --- a/xen/arch/x86/mm/shadow/multi.c +++ b/xen/arch/x86/mm/shadow/multi.c @@ -2475,7 +2475,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 @@ -3156,17 +3156,13 @@ sh_update_linear_entries(struct vcpu *v) sh_flush_local(d); } -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; @@ -3184,7 +3180,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 @@ -3412,8 +3412,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 9c4be4562f..2a4005a795 100644 --- a/xen/arch/x86/mm/shadow/none.c +++ b/xen/arch/x86/mm/shadow/none.c @@ -55,8 +55,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#master
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |