[Qemu-devel] [PATCH] Rework local IRQ delivery for APICs (was:[PATCH] APIC: add NMI and SMI IPI support)

[Jan Kiszka]

Jan Kiszka wrote:
> However, find some experimental watchdog-enabler patch below. It allows
> to use nmi_watchdog=1 (i.e. the IO-APIC variant) with Linux, without any
> visible regression of normal IRQ delivery (but I only lightly tested it
> on top of the kvm-userspace qemu, that's why "experimental").

As I was lacking NMIs on CPUs > 0, I thought about IRQ delivery again
and came to the conclusion, that we may better broadcast the PIC IRQ to
all APICs. After fixing the current APIC initialization, this actually
works as desired. Updated patch below.

Jan

---
 hw/apic.c |   63 ++++++++++++++++++++++++++++++++++++++++++--------------------
 hw/pc.c   |   15 ++++++++++----
 hw/pc.h   |    3 ++
 3 files changed, 57 insertions(+), 24 deletions(-)

Index: b/hw/apic.c
===================================================================
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -166,6 +166,37 @@ static inline void reset_bit(uint32_t *t
     tab[i] &= ~mask;
 }
 
+void apic_local_deliver(CPUState *env, int vector)
+{
+    APICState *s = env->apic_state;
+    uint32_t lvt = s->lvt[vector];
+    int trigger_mode;
+
+    if (lvt & APIC_LVT_MASKED)
+        return;
+
+    switch ((lvt >> 8) & 7) {
+    case APIC_DM_SMI:
+        cpu_interrupt(env, CPU_INTERRUPT_SMI);
+        break;
+
+    case APIC_DM_NMI:
+        cpu_interrupt(env, CPU_INTERRUPT_NMI);
+        break;
+
+    case APIC_DM_EXTINT:
+        cpu_interrupt(env, CPU_INTERRUPT_HARD);
+        break;
+
+    case APIC_DM_FIXED:
+        trigger_mode = APIC_TRIGGER_EDGE;
+        if ((vector == APIC_LVT_LINT0 || vector == APIC_LVT_LINT1) &&
+            (lvt & APIC_LVT_LEVEL_TRIGGER))
+            trigger_mode = APIC_TRIGGER_LEVEL;
+        apic_set_irq(s, lvt & 0xff, trigger_mode);
+    }
+}
+
 #define foreach_apic(apic, deliver_bitmask, code) \
 {\
     int __i, __j, __mask;\
@@ -502,10 +533,8 @@ int apic_accept_pic_intr(CPUState *env)
 
     lvt0 = s->lvt[APIC_LVT_LINT0];
 
-    if (s->id == 0 &&
-        ((s->apicbase & MSR_IA32_APICBASE_ENABLE) == 0 ||
-         ((lvt0 & APIC_LVT_MASKED) == 0 &&
-          ((lvt0 >> 8) & 0x7) == APIC_DM_EXTINT)))
+    if ((s->apicbase & MSR_IA32_APICBASE_ENABLE) == 0 ||
+        (lvt0 & APIC_LVT_MASKED) == 0)
         return 1;
 
     return 0;
@@ -556,9 +585,7 @@ static void apic_timer(void *opaque)
 {
     APICState *s = opaque;
 
-    if (!(s->lvt[APIC_LVT_TIMER] & APIC_LVT_MASKED)) {
-        apic_set_irq(s, s->lvt[APIC_LVT_TIMER] & 0xff, APIC_TRIGGER_EDGE);
-    }
+    apic_local_deliver(s->cpu_env, APIC_LVT_TIMER);
     apic_timer_update(s, s->next_time);
 }
 
@@ -818,12 +845,14 @@ static void apic_reset(void *opaque)
     APICState *s = opaque;
     apic_init_ipi(s);
 
-    /*
-     * LINT0 delivery mode is set to ExtInt at initialization time
-     * typically by BIOS, so PIC interrupt can be delivered to the
-     * processor when local APIC is enabled.
-     */
-    s->lvt[APIC_LVT_LINT0] = 0x700;
+    if (s->id == 0) {
+        /*
+         * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
+         * time typically by BIOS, so PIC interrupt can be delivered to the
+         * processor when local APIC is enabled.
+         */
+        s->lvt[APIC_LVT_LINT0] = 0x700;
+    }
 }
 
 static CPUReadMemoryFunc *apic_mem_read[3] = {
@@ -848,19 +877,13 @@ int apic_init(CPUState *env)
     if (!s)
         return -1;
     env->apic_state = s;
-    apic_init_ipi(s);
     s->id = last_apic_id++;
     env->cpuid_apic_id = s->id;
     s->cpu_env = env;
     s->apicbase = 0xfee00000 |
         (s->id ? 0 : MSR_IA32_APICBASE_BSP) | MSR_IA32_APICBASE_ENABLE;
 
-    /*
-     * LINT0 delivery mode is set to ExtInt at initialization time
-     * typically by BIOS, so PIC interrupt can be delivered to the
-     * processor when local APIC is enabled.
-     */
-    s->lvt[APIC_LVT_LINT0] = 0x700;
+    apic_reset(s);
 
     /* XXX: mapping more APICs at the same memory location */
     if (apic_io_memory == 0) {
Index: b/hw/pc.c
===================================================================
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -113,9 +113,16 @@ int cpu_get_pic_interrupt(CPUState *env)
 
 static void pic_irq_request(void *opaque, int irq, int level)
 {
-    CPUState *env = opaque;
-    if (level && apic_accept_pic_intr(env))
-        cpu_interrupt(env, CPU_INTERRUPT_HARD);
+    CPUState *env = first_cpu;
+
+    if (!level)
+        return;
+
+    while (env) {
+        if (apic_accept_pic_intr(env))
+            apic_local_deliver(env, APIC_LINT0);
+        env = env->next_cpu;
+    }
 }
 
 /* PC cmos mappings */
@@ -841,7 +848,7 @@ static void pc_init1(int ram_size, int v
     if (linux_boot)
        load_linux(kernel_filename, initrd_filename, kernel_cmdline);
 
-    cpu_irq = qemu_allocate_irqs(pic_irq_request, first_cpu, 1);
+    cpu_irq = qemu_allocate_irqs(pic_irq_request, NULL, 1);
     i8259 = i8259_init(cpu_irq[0]);
     ferr_irq = i8259[13];
 
Index: b/hw/pc.h
===================================================================
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -39,8 +39,11 @@ void irq_info(void);
 /* APIC */
 typedef struct IOAPICState IOAPICState;
 
+#define APIC_LINT0     3
+
 int apic_init(CPUState *env);
 int apic_accept_pic_intr(CPUState *env);
+void apic_local_deliver(CPUState *env, int vector);
 int apic_get_interrupt(CPUState *env);
 IOAPICState *ioapic_init(void);
 void ioapic_set_irq(void *opaque, int vector, int level);