[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240818171625.3030584-1-maz@kernel.org>
Date: Sun, 18 Aug 2024 18:16:25 +0100
From: Marc Zyngier <maz@...nel.org>
To: linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Cc: Zhou Wang <wangzhou1@...ilicon.com>,
Thomas Gleixner <tglx@...utronix.de>
Subject: [PATCH v2] irqchip/gic-v4: Fix ordering between vmapp and vpe locks
The (recently established) lock ordering mandates that the per-VM
vmapp_lock is acquired before we take the per-VPE lock.
As it turns out, its_vpe_set_affinity() takes the VPE lock, and
then calls into its_send_vmovp(), which itself takes the vmapp
lock. Obviously, this isn't what we want.
As its_send_vmovp() is only called from its_vpe_set_affinity(),
hoist the vmapp locking from the former into the latter, restoring
the expected order.
Fixes: f0eb154c39471 ("irqchip/gic-v4: Substitute vmovp_lock for a per-VM lock")
Reported-by: Zhou Wang <wangzhou1@...ilicon.com>
Signed-off-by: Marc Zyngier <maz@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
---
Notes:
v2:
- Dropped flags and rely on the irq_data being locked with interrupts
disabled
- Add extra documentation to explain why taking the vmapp_lock is
dependent on the HW requiring the use of the ITS list
v1: https://lore.kernel.org/r/20240723175203.3193882-1-maz@kernel.org
drivers/irqchip/irq-gic-v3-its.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 9b34596b3542..fdec478ba5e7 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1329,12 +1329,6 @@ static void its_send_vmovp(struct its_vpe *vpe)
return;
}
- /*
- * Protect against concurrent updates of the mapping state on
- * individual VMs.
- */
- guard(raw_spinlock_irqsave)(&vpe->its_vm->vmapp_lock);
-
/*
* Yet another marvel of the architecture. If using the
* its_list "feature", we need to make sure that all ITSs
@@ -3824,7 +3818,14 @@ static int its_vpe_set_affinity(struct irq_data *d,
* protect us, and that we must ensure nobody samples vpe->col_idx
* during the update, hence the lock below which must also be
* taken on any vLPI handling path that evaluates vpe->col_idx.
+ *
+ * Finally, we must protect ourselves against concurrent updates of
+ * the mapping state on this VM should the ITS list be in use (see
+ * the shortcut in its_send_vmovp() otherewise).
*/
+ if (its_list_map)
+ raw_spin_lock(&vpe->its_vm->vmapp_lock);
+
from = vpe_to_cpuid_lock(vpe, &flags);
table_mask = gic_data_rdist_cpu(from)->vpe_table_mask;
@@ -3854,6 +3855,9 @@ static int its_vpe_set_affinity(struct irq_data *d,
irq_data_update_effective_affinity(d, cpumask_of(cpu));
vpe_to_cpuid_unlock(vpe, flags);
+ if (its_list_map)
+ raw_spin_unlock(&vpe->its_vm->vmapp_lock);
+
return IRQ_SET_MASK_OK_DONE;
}
--
2.39.2
Powered by blists - more mailing lists