Skip to content

Commit bbe17c6

Browse files
dwmw2bonzini
authored andcommitted
KVM: x86/xen: Fix potential deadlock in kvm_xen_update_runstate_guest()
The kvm_xen_update_runstate_guest() function can be called when the vCPU is being scheduled out, from a preempt notifier. It *opportunistically* updates the runstate area in the guest memory, if the gfn_to_pfn_cache which caches the appropriate address is still valid. If there is *contention* when it attempts to obtain gpc->lock, then locking inside the priority inheritance checks may cause a deadlock. Lockdep reports: [13890.148997] Chain exists of: &gpc->lock --> &p->pi_lock --> &rq->__lock [13890.149002] Possible unsafe locking scenario: [13890.149003] CPU0 CPU1 [13890.149004] ---- ---- [13890.149005] lock(&rq->__lock); [13890.149007] lock(&p->pi_lock); [13890.149009] lock(&rq->__lock); [13890.149011] lock(&gpc->lock); [13890.149013] *** DEADLOCK *** In the general case, if there's contention for a read lock on gpc->lock, that's going to be because something else is either invalidating or revalidating the cache. Either way, we've raced with seeing it in an invalid state, in which case we would have aborted the opportunistic update anyway. So in the 'atomic' case when called from the preempt notifier, just switch to using read_trylock() and avoid the PI handling altogether. Signed-off-by: David Woodhouse <[email protected]> Message-Id: <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
1 parent 23e6025 commit bbe17c6

File tree

1 file changed

+17
-2
lines changed

1 file changed

+17
-2
lines changed

arch/x86/kvm/xen.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,15 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
271271
* Attempt to obtain the GPC lock on *both* (if there are two)
272272
* gfn_to_pfn caches that cover the region.
273273
*/
274-
read_lock_irqsave(&gpc1->lock, flags);
274+
if (atomic) {
275+
local_irq_save(flags);
276+
if (!read_trylock(&gpc1->lock)) {
277+
local_irq_restore(flags);
278+
return;
279+
}
280+
} else {
281+
read_lock_irqsave(&gpc1->lock, flags);
282+
}
275283
while (!kvm_gpc_check(gpc1, user_len1)) {
276284
read_unlock_irqrestore(&gpc1->lock, flags);
277285

@@ -308,7 +316,14 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
308316
* gpc1 lock to make lockdep shut up about it.
309317
*/
310318
lock_set_subclass(&gpc1->lock.dep_map, 1, _THIS_IP_);
311-
read_lock(&gpc2->lock);
319+
if (atomic) {
320+
if (!read_trylock(&gpc2->lock)) {
321+
read_unlock_irqrestore(&gpc1->lock, flags);
322+
return;
323+
}
324+
} else {
325+
read_lock(&gpc2->lock);
326+
}
312327

313328
if (!kvm_gpc_check(gpc2, user_len2)) {
314329
read_unlock(&gpc2->lock);

0 commit comments

Comments
 (0)