Skip to content

Commit f903bdb

Browse files
author
Herton R. Krzesinski
committed
Merge: KVM: arm64: GICv4.1: Fix race with doorbell on VPE activation/deactivation
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/2051 Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2166453 Upstream: Yes Test: non regression testing only To save the vgic LPI pending state with GICv4.1, the VPEs must all be unmapped from the ITSs so that the sGIC caches can be flushed. The opposite is done once the state is saved. This is all done by using the activate/deactivate irqdomain callbacks directly from the vgic code. Crutially, this is done without holding the irqdesc lock for the interrupts that represent the VPE. And these callbacks are changing the state of the irqdesc. What could possibly go wrong? If a doorbell fires while we are messing with the irqdesc state, it will acquire the lock and change the interrupt state concurrently. Since we don't hole the lock, curruption occurs in on the interrupt state. Oh well. While acquiring the lock would fix this (and this was Shanker's initial approach), this is still a layering violation we could do without. A better approach is actually to free the VPE interrupt, do what we have to do, and re-request it. It is more work, but this usually happens only once in the lifetime of the VM and we don't really care about this sort of overhead. Signed-off-by: Eric Auger <eric.auger@redhat.com> Approved-by: Donald Dutile <ddutile@redhat.com> Approved-by: Cornelia Huck <cohuck@redhat.com> Approved-by: Gavin Shan <gshan@redhat.com> Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
2 parents 61a8ccd + c8cf5a5 commit f903bdb

File tree

3 files changed

+18
-16
lines changed

3 files changed

+18
-16
lines changed

arch/arm64/kvm/vgic/vgic-v3.c

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -350,26 +350,23 @@ int vgic_v3_lpi_sync_pending_status(struct kvm *kvm, struct vgic_irq *irq)
350350
* The deactivation of the doorbell interrupt will trigger the
351351
* unmapping of the associated vPE.
352352
*/
353-
static void unmap_all_vpes(struct vgic_dist *dist)
353+
static void unmap_all_vpes(struct kvm *kvm)
354354
{
355-
struct irq_desc *desc;
355+
struct vgic_dist *dist = &kvm->arch.vgic;
356356
int i;
357357

358-
for (i = 0; i < dist->its_vm.nr_vpes; i++) {
359-
desc = irq_to_desc(dist->its_vm.vpes[i]->irq);
360-
irq_domain_deactivate_irq(irq_desc_get_irq_data(desc));
361-
}
358+
for (i = 0; i < dist->its_vm.nr_vpes; i++)
359+
free_irq(dist->its_vm.vpes[i]->irq, kvm_get_vcpu(kvm, i));
362360
}
363361

364-
static void map_all_vpes(struct vgic_dist *dist)
362+
static void map_all_vpes(struct kvm *kvm)
365363
{
366-
struct irq_desc *desc;
364+
struct vgic_dist *dist = &kvm->arch.vgic;
367365
int i;
368366

369-
for (i = 0; i < dist->its_vm.nr_vpes; i++) {
370-
desc = irq_to_desc(dist->its_vm.vpes[i]->irq);
371-
irq_domain_activate_irq(irq_desc_get_irq_data(desc), false);
372-
}
367+
for (i = 0; i < dist->its_vm.nr_vpes; i++)
368+
WARN_ON(vgic_v4_request_vpe_irq(kvm_get_vcpu(kvm, i),
369+
dist->its_vm.vpes[i]->irq));
373370
}
374371

375372
/**
@@ -394,7 +391,7 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
394391
* and enabling of the doorbells have already been done.
395392
*/
396393
if (kvm_vgic_global_state.has_gicv4_1) {
397-
unmap_all_vpes(dist);
394+
unmap_all_vpes(kvm);
398395
vlpi_avail = true;
399396
}
400397

@@ -444,7 +441,7 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
444441

445442
out:
446443
if (vlpi_avail)
447-
map_all_vpes(dist);
444+
map_all_vpes(kvm);
448445

449446
return ret;
450447
}

arch/arm64/kvm/vgic/vgic-v4.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,11 @@ void vgic_v4_get_vlpi_state(struct vgic_irq *irq, bool *val)
222222
*val = !!(*ptr & mask);
223223
}
224224

225+
int vgic_v4_request_vpe_irq(struct kvm_vcpu *vcpu, int irq)
226+
{
227+
return request_irq(irq, vgic_v4_doorbell_handler, 0, "vcpu", vcpu);
228+
}
229+
225230
/**
226231
* vgic_v4_init - Initialize the GICv4 data structures
227232
* @kvm: Pointer to the VM being initialized
@@ -283,8 +288,7 @@ int vgic_v4_init(struct kvm *kvm)
283288
irq_flags &= ~IRQ_NOAUTOEN;
284289
irq_set_status_flags(irq, irq_flags);
285290

286-
ret = request_irq(irq, vgic_v4_doorbell_handler,
287-
0, "vcpu", vcpu);
291+
ret = vgic_v4_request_vpe_irq(vcpu, irq);
288292
if (ret) {
289293
kvm_err("failed to allocate vcpu IRQ%d\n", irq);
290294
/*

arch/arm64/kvm/vgic/vgic.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,5 +331,6 @@ int vgic_v4_init(struct kvm *kvm);
331331
void vgic_v4_teardown(struct kvm *kvm);
332332
void vgic_v4_configure_vsgis(struct kvm *kvm);
333333
void vgic_v4_get_vlpi_state(struct vgic_irq *irq, bool *val);
334+
int vgic_v4_request_vpe_irq(struct kvm_vcpu *vcpu, int irq);
334335

335336
#endif

0 commit comments

Comments
 (0)