1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
|
From bf70ce8b3449c49eb828d5b1f4934a49b00fef35 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Wed, 20 Sep 2023 20:06:53 +0100
Subject: [PATCH 46/67] 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@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>
(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();
--
2.44.0
|
GitWeb