plat/kvm/x86: Fix lost IRQs

There is a race condition in `ukplat_lcpu_halt_irq()`, where an IRQ
may fire in between the short window after which interrupts are
enabled and before the halt is done. This may halt the caller
forever (or much longer until the next interrupt).

As this issue is platform and architecture specific, this commit
moves `ukplat_lcpu_halt_irq()` from the common code to the
plat/arch-specific implementation. For x86 the race condition
is avoided by ensuring that there are no instructions between
`STI` and `HLT`.

Signed-off-by: Marc Rittinghaus <[email protected]>
Reviewed-by: Simon Kuenzer <[email protected]>
Tested-by: Unikraft CI <[email protected]>
GitHub-Pull-Request: unikraft#257

Author: marcrittinghaus

plat/common/include/x86/irq.h

@@ -49,6 +49,11 @@
 	asm volatile("sti" : : : "memory"); \
 })
 
+#define __sti_hlt() \
+({ \
+	asm volatile("sti; hlt" : : : "memory"); \
+})
+
 #define __save_flags(x) \
 	do { \
 		unsigned long __f; \
@@ -83,6 +88,7 @@ static inline int irqs_disabled(void)
 #define local_save_flags(x)      __save_flags(x)
 #define local_irq_disable()      __cli()
 #define local_irq_enable()       __sti()
+#define local_irq_enable_halt()  __sti_hlt()
 
 #define __MAX_IRQ	16
 

plat/common/lcpu.c

@@ -34,20 +34,12 @@
 #include <uk/plat/common/cpu.h>
 #include <uk/plat/common/_time.h>
 
-
 void ukplat_lcpu_halt(void)
 {
 	ukplat_lcpu_disable_irq();
 	halt();
 }
 
-void ukplat_lcpu_halt_irq(void)
-{
-	ukplat_lcpu_enable_irq();
-	halt();
-	ukplat_lcpu_disable_irq();
-}
-
 void ukplat_lcpu_halt_to(__snsec until)
 {
 	unsigned long flags;

plat/kvm/arm/lcpu.c

@@ -43,6 +43,13 @@ void ukplat_lcpu_disable_irq(void)
 	local_irq_disable();
 }
 
+void ukplat_lcpu_halt_irq(void)
+{
+	ukplat_lcpu_enable_irq();
+	halt();
+	ukplat_lcpu_disable_irq();
+}
+
 unsigned long ukplat_lcpu_save_irqf(void)
 {
 	unsigned long flags;

plat/kvm/x86/lcpu.c

@@ -35,7 +35,6 @@
 #include <uk/plat/lcpu.h>
 #include <x86/irq.h>
 
-
 void ukplat_lcpu_enable_irq(void)
 {
 	local_irq_enable();
@@ -46,6 +45,21 @@ void ukplat_lcpu_disable_irq(void)
 	local_irq_disable();
 }
 
+void ukplat_lcpu_halt_irq(void)
+{
+	/*
+	 * We have to be careful when enabling interrupts before entering a
+	 * halt state. If we want to wait for an interrupt (e.g., a timer)
+	 * the interrupt may fire in the short window between sti and hlt and
+	 * we are going to halt forever. As sti only enables interrupts after
+	 * the following instruction, we can avoid the race condition by
+	 * ensuring that hlt immediately follows sti. There must be no
+	 * instruction in between.
+	 */
+	local_irq_enable_halt();
+	local_irq_disable();
+}
+
 unsigned long ukplat_lcpu_save_irqf(void)
 {
 	unsigned long flags;

plat/linuxu/irq.c

@@ -79,6 +79,13 @@ void ukplat_lcpu_disable_irq(void)
 	irq_enabled = 0;
 }
 
+void ukplat_lcpu_halt_irq(void)
+{
+	ukplat_lcpu_enable_irq();
+	halt();
+	ukplat_lcpu_disable_irq();
+}
+
 int ukplat_lcpu_irqs_disabled(void)
 {
 	return (irq_enabled == 0);

plat/xen/lcpu.c

@@ -51,6 +51,13 @@ void ukplat_lcpu_disable_irq(void)
 	local_irq_disable();
 }
 
+void ukplat_lcpu_halt_irq(void)
+{
+	ukplat_lcpu_enable_irq();
+	halt();
+	ukplat_lcpu_disable_irq();
+}
+
 unsigned long ukplat_lcpu_save_irqf(void)
 {
 	unsigned long flags;