fix possible deadlock with FPU sharing on ARM64 and RISC-V

arch/arm64/core/smp.c

@@ -225,6 +225,25 @@ void z_arm64_flush_fpu_ipi(unsigned int cpu)
 
 	gic_raise_sgi(SGI_FPU_IPI, mpidr, 1 << aff0);
 }
+
+/*
+ * Make sure there is no pending FPU flush request for this CPU while
+ * waiting for a contended spinlock to become available. This prevents
+ * a deadlock when the lock we need is already taken by another CPU
+ * that also wants its FPU content to be reinstated while such content
+ * is still live in this CPU's FPU.
+ */
+void arch_spin_relax(void)
+{
+	if (arm_gic_irq_is_pending(SGI_FPU_IPI)) {
+		arm_gic_irq_clear_pending(SGI_FPU_IPI);
+		/*
+		 * We may not be in IRQ context here hence cannot use
+		 * z_arm64_flush_local_fpu() directly.
+		 */
+		arch_float_disable(_current_cpu->arch.fpu_owner);
+	}
+}
 #endif
 
 static int arm64_smp_init(void)

arch/riscv/core/smp.c

@@ -66,8 +66,8 @@ void z_riscv_secondary_cpu_init(int hartid)
 #define MSIP(hartid) ((volatile uint32_t *)RISCV_MSIP_BASE)[hartid]
 
 static atomic_val_t cpu_pending_ipi[CONFIG_MP_MAX_NUM_CPUS];
-#define IPI_SCHED	BIT(0)
-#define IPI_FPU_FLUSH	BIT(1)
+#define IPI_SCHED	0
+#define IPI_FPU_FLUSH	1
 
 void arch_sched_ipi(void)
 {
@@ -77,7 +77,7 @@ void arch_sched_ipi(void)
 
 	for (unsigned int i = 0; i < num_cpus; i++) {
 		if (i != id && _kernel.cpus[i].arch.online) {
-			atomic_or(&cpu_pending_ipi[i], IPI_SCHED);
+			atomic_set_bit(&cpu_pending_ipi[i], IPI_SCHED);
 			MSIP(_kernel.cpus[i].arch.hartid) = 1;
 		}
 	}
@@ -88,7 +88,7 @@ void arch_sched_ipi(void)
 #ifdef CONFIG_FPU_SHARING
 void z_riscv_flush_fpu_ipi(unsigned int cpu)
 {
-	atomic_or(&cpu_pending_ipi[cpu], IPI_FPU_FLUSH);
+	atomic_set_bit(&cpu_pending_ipi[cpu], IPI_FPU_FLUSH);
 	MSIP(_kernel.cpus[cpu].arch.hartid) = 1;
 }
 #endif
@@ -101,11 +101,11 @@ static void ipi_handler(const void *unused)
 
 	atomic_val_t pending_ipi = atomic_clear(&cpu_pending_ipi[_current_cpu->id]);
 
-	if (pending_ipi & IPI_SCHED) {
+	if (pending_ipi & ATOMIC_MASK(IPI_SCHED)) {
 		z_sched_ipi();
 	}
 #ifdef CONFIG_FPU_SHARING
-	if (pending_ipi & IPI_FPU_FLUSH) {
+	if (pending_ipi & ATOMIC_MASK(IPI_FPU_FLUSH)) {
 		/* disable IRQs */
 		csr_clear(mstatus, MSTATUS_IEN);
 		/* perform the flush */
@@ -118,6 +118,28 @@ static void ipi_handler(const void *unused)
 #endif
 }
 
+#ifdef CONFIG_FPU_SHARING
+/*
+ * Make sure there is no pending FPU flush request for this CPU while
+ * waiting for a contended spinlock to become available. This prevents
+ * a deadlock when the lock we need is already taken by another CPU
+ * that also wants its FPU content to be reinstated while such content
+ * is still live in this CPU's FPU.
+ */
+void arch_spin_relax(void)
+{
+	atomic_val_t *pending_ipi = &cpu_pending_ipi[_current_cpu->id];
+
+	if (atomic_test_and_clear_bit(pending_ipi, IPI_FPU_FLUSH)) {
+		/*
+		 * We may not be in IRQ context here hence cannot use
+		 * z_riscv_flush_local_fpu() directly.
+		 */
+		arch_float_disable(_current_cpu->arch.fpu_owner);
+	}
+}
+#endif
+
 static int riscv_smp_init(void)
 {
 

drivers/interrupt_controller/intc_gic.c

@@ -57,6 +57,29 @@ bool arm_gic_irq_is_enabled(unsigned int irq)
 	return (enabler & (1 << int_off)) != 0;
 }
 
+bool arm_gic_irq_is_pending(unsigned int irq)
+{
+	int int_grp, int_off;
+	unsigned int enabler;
+
+	int_grp = irq / 32;
+	int_off = irq % 32;
+
+	enabler = sys_read32(GICD_ISPENDRn + int_grp * 4);
+
+	return (enabler & (1 << int_off)) != 0;
+}
+
+void arm_gic_irq_clear_pending(unsigned int irq)
+{
+	int int_grp, int_off;
+
+	int_grp = irq / 32;
+	int_off = irq % 32;
+
+	sys_write32((1 << int_off), (GICD_ICPENDRn + int_grp * 4));
+}
+
 void arm_gic_irq_set_priority(
 	unsigned int irq, unsigned int prio, uint32_t flags)
 {

drivers/interrupt_controller/intc_gicv3.c

@@ -213,6 +213,25 @@ bool arm_gic_irq_is_enabled(unsigned int intid)
 	return (val & mask) != 0;
 }
 
+bool arm_gic_irq_is_pending(unsigned int intid)
+{
+	uint32_t mask = BIT(intid & (GIC_NUM_INTR_PER_REG - 1));
+	uint32_t idx = intid / GIC_NUM_INTR_PER_REG;
+	uint32_t val;
+
+	val = sys_read32(ISPENDR(GET_DIST_BASE(intid), idx));
+
+	return (val & mask) != 0;
+}
+
+void arm_gic_irq_clear_pending(unsigned int intid)
+{
+	uint32_t mask = BIT(intid & (GIC_NUM_INTR_PER_REG - 1));
+	uint32_t idx = intid / GIC_NUM_INTR_PER_REG;
+
+	sys_write32(mask, ICPENDR(GET_DIST_BASE(intid), idx));
+}
+
 unsigned int arm_gic_get_active(void)
 {
 	int intid;

include/zephyr/drivers/interrupt_controller/gic.h

@@ -294,6 +294,21 @@ void arm_gic_irq_disable(unsigned int irq);
  */
 bool arm_gic_irq_is_enabled(unsigned int irq);
 
+/**
+ * @brief Check if an interrupt is pending
+ *
+ * @param irq interrupt ID
+ * @return Returns true if interrupt is pending, false otherwise
+ */
+bool arm_gic_irq_is_pending(unsigned int irq);
+
+/**
+ * @brief Clear the pending irq
+ *
+ * @param irq interrupt ID
+ */
+void arm_gic_irq_clear_pending(unsigned int irq);
+
 /**
  * @brief Set interrupt priority
  *

include/zephyr/spinlock.h

@@ -153,6 +153,7 @@ static ALWAYS_INLINE k_spinlock_key_t k_spin_lock(struct k_spinlock *l)
 
 #ifdef CONFIG_SMP
 	while (!atomic_cas(&l->locked, 0, 1)) {
+		arch_spin_relax();
 	}
 #endif
 

include/zephyr/sys/arch_interface.h

@@ -1190,6 +1190,16 @@ bool arch_pcie_msi_vector_connect(msi_vector_t *vector,
 
 #endif /* CONFIG_PCIE_MSI_MULTI_VECTOR */
 
+/**
+ * @brief Perform architecture specific processing within spin loops
+ *
+ * This is invoked from busy loops with IRQs disabled such as the contended
+ * spinlock loop. The default implementation is a weak function that calls
+ * arch_nop(). Architectures may implement this function to perform extra
+ * checks or power management tricks if needed.
+ */
+void arch_spin_relax(void);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

kernel/idle.c

@@ -106,3 +106,11 @@ void idle(void *unused1, void *unused2, void *unused3)
 #endif
 	}
 }
+
+void __weak arch_spin_relax(void)
+{
+	__ASSERT(!arch_irq_unlocked(arch_irq_lock()),
+		 "this is meant to be called with IRQs disabled");
+
+	arch_nop();
+}