@@ -227,14 +227,8 @@ struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int virq)
static int vgic_get_virq_priority(struct vcpu *v, unsigned int virq)
{
struct vgic_irq_rank *rank = vgic_rank_irq(v, virq);
- unsigned long flags;
- int priority;
-
- vgic_lock_rank(v, rank, flags);
- priority = rank->priority[virq & INTERRUPT_RANK_MASK];
- vgic_unlock_rank(v, rank, flags);
- return priority;
+ return read_atomic(&rank->priority[virq & INTERRUPT_RANK_MASK]);
}
bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq)
When reading the priority value of a virtual interrupt, we were taking the respective rank lock so far. However for forwarded interrupts (Dom0 only so far) this may lead to a deadlock with the following call chain: - MMIO access to change the IRQ affinity, calling the ITARGETSR handler - this handler takes the appropriate rank lock and calls vgic_store_itargetsr() - vgic_store_itargetsr() will eventually call vgic_migrate_irq() - if this IRQ is already in-flight, it will remove it from the old VCPU and inject it into the new one, by calling vgic_vcpu_inject_irq() - vgic_vcpu_inject_irq will call vgic_get_virq_priority() - vgic_get_virq_priority() tries to take the rank lock - again! It seems like this code path has never been exercised before. Fix this by avoiding taking the lock in vgic_get_virq_priority() (like we do in vgic_get_target_vcpu()). Actually we are just reading one byte, and priority changes while interrupts are handled are a benign race that can happen on real hardware too. So it looks safe to just use read_atomic() instead. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/vgic.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)