qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] APIC: add NMI and SMI IPI support
@ 2008-01-28 20:37 Jan Kiszka
  0 siblings, 0 replies; 3+ messages in thread
From: Jan Kiszka @ 2008-01-28 20:37 UTC (permalink / raw)
  To: qemu-devel

While testing KGDB (yeah, it actually seem to make it into mainline!)
under QEMU, I failed to get it running in SMP mode. Reason: NMI IPIs are
not correctly handled by QEMU's emulated APIC.

To overcome this, the patch below introduces a new interruption request,
CPU_INTERRUPT_NMI, so that a VCPU can cleanly send this special
interrupt to other VCPUs. It also introduces HF_NMI_MASK which shall
ensure that NMIs are not recursively triggered, but I must confess that
this particular property was not really tested yet.

CPU_INTERRUPT_NMI is then trivially exploited by apic_bus_deliver to
send out both NMIs and (for the sake of completeness - it's untested as
well SMIs).

With this patch applied, I'm finally able to run (and potentially debug)
KGDB for Linux SMP guests.

Jan

---
 cpu-all.h            |    1 +
 cpu-exec.c           |    6 ++++++
 hw/apic.c            |    8 +++++++-
 target-i386/cpu.h    |    2 ++
 target-i386/helper.c |    2 ++
 5 files changed, 18 insertions(+), 1 deletion(-)

Index: b/hw/apic.c
===================================================================
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -216,8 +216,14 @@ static void apic_bus_deliver(const uint3
             break;
 
         case APIC_DM_SMI:
+            foreach_apic(apic_iter, deliver_bitmask,
+                cpu_interrupt(apic_iter->cpu_env, CPU_INTERRUPT_SMI) );
+            return;
+
         case APIC_DM_NMI:
-            break;
+            foreach_apic(apic_iter, deliver_bitmask,
+                cpu_interrupt(apic_iter->cpu_env, CPU_INTERRUPT_NMI) );
+            return;
 
         case APIC_DM_INIT:
             /* normal INIT IPI sent to processors */
Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -748,6 +748,7 @@ extern int code_copy_enabled;
 #define CPU_INTERRUPT_SMI    0x40 /* (x86 only) SMI interrupt pending */
 #define CPU_INTERRUPT_DEBUG  0x80 /* Debug event occured.  */
 #define CPU_INTERRUPT_VIRQ   0x100 /* virtual interrupt pending.  */
+#define CPU_INTERRUPT_NMI    0x200 /* NMI pending. */
 
 void cpu_interrupt(CPUState *s, int mask);
 void cpu_reset_interrupt(CPUState *env, int mask);
Index: b/cpu-exec.c
===================================================================
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -505,6 +505,12 @@ int cpu_exec(CPUState *env1)
                         env->interrupt_request &= ~CPU_INTERRUPT_SMI;
                         do_smm_enter();
                         BREAK_CHAIN;
+                    } else if ((interrupt_request & CPU_INTERRUPT_NMI) &&
+                        !(env->hflags & HF_NMI_MASK)) {
+                        env->interrupt_request &= ~CPU_INTERRUPT_NMI;
+                        env->hflags |= HF_NMI_MASK;
+                        do_interrupt(EXCP02_NMI, 0, 0, 0, 1);
+                        BREAK_CHAIN;
                     } else if ((interrupt_request & CPU_INTERRUPT_HARD) &&
                         (env->eflags & IF_MASK || env->hflags & HF_HIF_MASK) &&
                         !(env->hflags & HF_INHIBIT_IRQ_MASK)) {
Index: b/target-i386/cpu.h
===================================================================
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -148,6 +148,7 @@
 #define HF_SMM_SHIFT        19 /* CPU in SMM mode */
 #define HF_GIF_SHIFT        20 /* if set CPU takes interrupts */
 #define HF_HIF_SHIFT        21 /* shadow copy of IF_MASK when in SVM */
+#define HF_NMI_SHIFT        22 /* CPU serving NMI */
 
 #define HF_CPL_MASK          (3 << HF_CPL_SHIFT)
 #define HF_SOFTMMU_MASK      (1 << HF_SOFTMMU_SHIFT)
@@ -167,6 +168,7 @@
 #define HF_SMM_MASK          (1 << HF_SMM_SHIFT)
 #define HF_GIF_MASK          (1 << HF_GIF_SHIFT)
 #define HF_HIF_MASK          (1 << HF_HIF_SHIFT)
+#define HF_NMI_MASK          (1 << HF_NMI_SHIFT)
 
 #define CR0_PE_MASK  (1 << 0)
 #define CR0_MP_MASK  (1 << 1)
Index: b/target-i386/helper.c
===================================================================
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -2382,6 +2382,7 @@ void helper_iret_real(int shift)
     if (shift == 0)
         eflags_mask &= 0xffff;
     load_eflags(new_eflags, eflags_mask);
+    env->hflags &= ~HF_NMI_MASK;
 }
 
 static inline void validate_seg(int seg_reg, int cpl)
@@ -2633,6 +2634,7 @@ void helper_iret_protected(int shift, in
     } else {
         helper_ret_protected(shift, 1, 0);
     }
+    env->hflags &= ~HF_NMI_MASK;
 #ifdef USE_KQEMU
     if (kqemu_is_ok(env)) {
         CC_OP = CC_OP_EFLAGS;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] APIC: add NMI and SMI IPI support
@ 2008-02-19 13:17 Robi Yagel
  2008-02-20 17:23 ` Jan Kiszka
  0 siblings, 1 reply; 3+ messages in thread
From: Robi Yagel @ 2008-02-19 13:17 UTC (permalink / raw)
  To: qemu-devel, jan.kiszka

[-- Attachment #1: Type: text/plain, Size: 4840 bytes --]

Hello,
Thanks for the patch, if you can, please advice on the proper place to add
periodic generation of SMI/NMIs in order to simulate, e.g., a watchdog (and the
needed parameters - except for CPU_INTERRUPT_NMI...)
Thanks in advance, Robi



>> original message from Jan Kiszka on Mon, 28 Jan 2008 12:38:19 -0800

While testing KGDB (yeah, it actually seem to make it into mainline!)
under QEMU, I failed to get it running in SMP mode. Reason: NMI IPIs are
not correctly handled by QEMU's emulated APIC.

To overcome this, the patch below introduces a new interruption request,
CPU_INTERRUPT_NMI, so that a VCPU can cleanly send this special
interrupt to other VCPUs. It also introduces HF_NMI_MASK which shall
ensure that NMIs are not recursively triggered, but I must confess that
this particular property was not really tested yet.

CPU_INTERRUPT_NMI is then trivially exploited by apic_bus_deliver to
send out both NMIs and (for the sake of completeness - it's untested as
well SMIs).

With this patch applied, I'm finally able to run (and potentially debug)
KGDB for Linux SMP guests.

Jan

---
 cpu-all.h            |    1 +
 cpu-exec.c           |    6 ++++++
 hw/apic.c            |    8 +++++++-
 target-i386/cpu.h    |    2 ++
 target-i386/helper.c |    2 ++
 5 files changed, 18 insertions(+), 1 deletion(-)

Index: b/hw/apic.c
===================================================================
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -216,8 +216,14 @@ static void apic_bus_deliver(const uint3
             break;

         case APIC_DM_SMI:
+            foreach_apic(apic_iter, deliver_bitmask,
+                cpu_interrupt(apic_iter->cpu_env, CPU_INTERRUPT_SMI) );
+            return;
+
         case APIC_DM_NMI:
-            break;
+            foreach_apic(apic_iter, deliver_bitmask,
+                cpu_interrupt(apic_iter->cpu_env, CPU_INTERRUPT_NMI) );
+            return;

         case APIC_DM_INIT:
             /* normal INIT IPI sent to processors */
Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -748,6 +748,7 @@ extern int code_copy_enabled;
 #define CPU_INTERRUPT_SMI    0x40 /* (x86 only) SMI interrupt pending */
 #define CPU_INTERRUPT_DEBUG  0x80 /* Debug event occured.  */
 #define CPU_INTERRUPT_VIRQ   0x100 /* virtual interrupt pending.  */
+#define CPU_INTERRUPT_NMI    0x200 /* NMI pending. */

 void cpu_interrupt(CPUState *s, int mask);
 void cpu_reset_interrupt(CPUState *env, int mask);
Index: b/cpu-exec.c
===================================================================
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -505,6 +505,12 @@ int cpu_exec(CPUState *env1)
                         env->interrupt_request &= ~CPU_INTERRUPT_SMI;
                         do_smm_enter();
                         BREAK_CHAIN;
+                    } else if ((interrupt_request & CPU_INTERRUPT_NMI) &&
+                        !(env->hflags & HF_NMI_MASK)) {
+                        env->interrupt_request &= ~CPU_INTERRUPT_NMI;
+                        env->hflags |= HF_NMI_MASK;
+                        do_interrupt(EXCP02_NMI, 0, 0, 0, 1);
+                        BREAK_CHAIN;
                     } else if ((interrupt_request & CPU_INTERRUPT_HARD) &&
                         (env->eflags & IF_MASK || env->hflags & HF_HIF_MASK) &&
                         !(env->hflags & HF_INHIBIT_IRQ_MASK)) {
Index: b/target-i386/cpu.h
===================================================================
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -148,6 +148,7 @@
 #define HF_SMM_SHIFT        19 /* CPU in SMM mode */
 #define HF_GIF_SHIFT        20 /* if set CPU takes interrupts */
 #define HF_HIF_SHIFT        21 /* shadow copy of IF_MASK when in SVM */
+#define HF_NMI_SHIFT        22 /* CPU serving NMI */

 #define HF_CPL_MASK          (3 << HF_CPL_SHIFT)
 #define HF_SOFTMMU_MASK      (1 << HF_SOFTMMU_SHIFT)
@@ -167,6 +168,7 @@
 #define HF_SMM_MASK          (1 << HF_SMM_SHIFT)
 #define HF_GIF_MASK          (1 << HF_GIF_SHIFT)
 #define HF_HIF_MASK          (1 << HF_HIF_SHIFT)
+#define HF_NMI_MASK          (1 << HF_NMI_SHIFT)

 #define CR0_PE_MASK  (1 << 0)
 #define CR0_MP_MASK  (1 << 1)
Index: b/target-i386/helper.c
===================================================================
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -2382,6 +2382,7 @@ void helper_iret_real(int shift)
     if (shift == 0)
         eflags_mask &= 0xffff;
     load_eflags(new_eflags, eflags_mask);
+    env->hflags &= ~HF_NMI_MASK;
 }

 static inline void validate_seg(int seg_reg, int cpl)
@@ -2633,6 +2634,7 @@ void helper_iret_protected(int shift, in
     } else {
         helper_ret_protected(shift, 1, 0);
     }
+    env->hflags &= ~HF_NMI_MASK;
 #ifdef USE_KQEMU
     if (kqemu_is_ok(env)) {
         CC_OP = CC_OP_EFLAGS;

[-- Attachment #2: Type: text/html, Size: 5479 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] APIC: add NMI and SMI IPI support
  2008-02-19 13:17 Robi Yagel
@ 2008-02-20 17:23 ` Jan Kiszka
  0 siblings, 0 replies; 3+ messages in thread
From: Jan Kiszka @ 2008-02-20 17:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: jan.kiszka

Robi Yagel wrote:
> Hello,
> Thanks for the patch,

Nice to hear that there is interest in this. :)

> if you can, please advice on the proper place to add
> periodic generation of SMI/NMIs in order to simulate, e.g., a watchdog (and the
> needed parameters - except for CPU_INTERRUPT_NMI...)

Yeah, I also thought about NMI watchdog emulation while adding
CPU_INTERRUPT_NMI, but it took a bit more effort to understand what
pieces are missing. It should be a fairly useful feature for testing NMI
interaction with new kernel code and maybe even catching nasty races
that way (whenever you happen to work on such things, like I do ;) ).
Also, we need NMI delivery for custom hardware emulation here. So there
might be broader use for CPU_INTERRUPT_NMI in the future.

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").

A performance counter based NMI watchdog is surely doable as well, but
it takes a bit more effort. You would have to decide first what
perf-counter features should be emulated and how cleanly, even if you
only want to (mis-)use it for watchdog services. I'm lacking time for
this right now.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux


---
 hw/apic.c |   38 +++++++++++++++++++++++++++++++++-----
 hw/pc.c   |    2 +-
 hw/pc.h   |    3 +++
 3 files changed, 37 insertions(+), 6 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;\
@@ -504,8 +535,7 @@ int apic_accept_pic_intr(CPUState *env)
 
     if (s->id == 0 &&
         ((s->apicbase & MSR_IA32_APICBASE_ENABLE) == 0 ||
-         ((lvt0 & APIC_LVT_MASKED) == 0 &&
-          ((lvt0 >> 8) & 0x7) == APIC_DM_EXTINT)))
+         (lvt0 & APIC_LVT_MASKED) == 0))
         return 1;
 
     return 0;
@@ -556,9 +586,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);
 }
 
Index: b/hw/pc.c
===================================================================
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -115,7 +115,7 @@ static void pic_irq_request(void *opaque
 {
     CPUState *env = opaque;
     if (level && apic_accept_pic_intr(env))
-        cpu_interrupt(env, CPU_INTERRUPT_HARD);
+        apic_local_deliver(env, APIC_LINT0);
 }
 
 /* PC cmos mappings */
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);

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2008-02-20 17:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-28 20:37 [Qemu-devel] [PATCH] APIC: add NMI and SMI IPI support Jan Kiszka
  -- strict thread matches above, loose matches on Subject: below --
2008-02-19 13:17 Robi Yagel
2008-02-20 17:23 ` Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).