mirror of
https://github.com/xen-project/xen.git
synced 2026-10-28 02:01:24 +01:00
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: fb0ff49fe9 ("x86/shadow: defer releasing of PV's top-level shadow reference")
Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
This commit is contained in:
1 parent
e282067cf2
commit
e71157d1ac
5 files changed
+14
-18
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
/*
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||