| Message ID | 4E7B04DC.1030407@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 2011-09-22 11:50, Lai Jiangshan wrote: > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > Subject: [PATCH] Fix inject-nmi > > Now, inject-nmi sends NMI to all cpus...but this doesn't emulate > pc hardware 'NMI button', which triggers LINT1. > > So, now, LINT1 mask is ignored by inject-nmi and NMIs are sent to > all cpus without checking LINT1 mask. > > Because Linux masks LINT1 of cpus other than 0, this makes trouble. > For example, kdump cannot run sometimes. > --- > hw/apic.c | 7 +++++++ > hw/apic.h | 1 + > monitor.c | 4 ++-- > 3 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/hw/apic.c b/hw/apic.c > index 69d6ac5..020305b 100644 > --- a/hw/apic.c > +++ b/hw/apic.c > @@ -205,6 +205,13 @@ void apic_deliver_pic_intr(DeviceState *d, int level) > } > } > > +void apic_deliver_lint1_intr(DeviceState *d) > +{ > + APICState *s = DO_UPCAST(APICState, busdev.qdev, d); > + > + apic_local_deliver(s, APIC_LVT_LINT1); This will cause a qemu crash when apic_state is NULL (non-SMP 486 systems). Moreover: wrong indention. You know that this won't work for qemu-kvm with in-kernel irqchip? You may want to provide a patch for that tree, emulating the unavailable LINT1 injection via testing the APIC configration and then raising an NMI as before if it is accepted. Jan
On 09/22/2011 10:51 PM, Jan Kiszka wrote: > On 2011-09-22 11:50, Lai Jiangshan wrote: >> >> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> >> Subject: [PATCH] Fix inject-nmi >> >> Now, inject-nmi sends NMI to all cpus...but this doesn't emulate >> pc hardware 'NMI button', which triggers LINT1. >> >> So, now, LINT1 mask is ignored by inject-nmi and NMIs are sent to >> all cpus without checking LINT1 mask. >> >> Because Linux masks LINT1 of cpus other than 0, this makes trouble. >> For example, kdump cannot run sometimes. >> --- >> hw/apic.c | 7 +++++++ >> hw/apic.h | 1 + >> monitor.c | 4 ++-- >> 3 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/hw/apic.c b/hw/apic.c >> index 69d6ac5..020305b 100644 >> --- a/hw/apic.c >> +++ b/hw/apic.c >> @@ -205,6 +205,13 @@ void apic_deliver_pic_intr(DeviceState *d, int level) >> } >> } >> >> +void apic_deliver_lint1_intr(DeviceState *d) >> +{ >> + APICState *s = DO_UPCAST(APICState, busdev.qdev, d); >> + >> + apic_local_deliver(s, APIC_LVT_LINT1); > > This will cause a qemu crash when apic_state is NULL (non-SMP 486 > systems). Ouch, I see. What are the interrupt mode used for non-SMP 486 systems? > Moreover: wrong indention. > > You know that this won't work for qemu-kvm with in-kernel irqchip? You > may want to provide a patch for that tree, emulating the unavailable > LINT1 injection via testing the APIC configration and then raising an > NMI as before if it is accepted. > It works in my box but the NMI is not injected through the in-kernel irqchip, I will implement it as you suggested. Thanks, Lai
On 2011-09-23 11:31, Lai Jiangshan wrote: > On 09/22/2011 10:51 PM, Jan Kiszka wrote: >> On 2011-09-22 11:50, Lai Jiangshan wrote: >>> >>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> >>> Subject: [PATCH] Fix inject-nmi >>> >>> Now, inject-nmi sends NMI to all cpus...but this doesn't emulate >>> pc hardware 'NMI button', which triggers LINT1. >>> >>> So, now, LINT1 mask is ignored by inject-nmi and NMIs are sent to >>> all cpus without checking LINT1 mask. >>> >>> Because Linux masks LINT1 of cpus other than 0, this makes trouble. >>> For example, kdump cannot run sometimes. >>> --- >>> hw/apic.c | 7 +++++++ >>> hw/apic.h | 1 + >>> monitor.c | 4 ++-- >>> 3 files changed, 10 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/apic.c b/hw/apic.c >>> index 69d6ac5..020305b 100644 >>> --- a/hw/apic.c >>> +++ b/hw/apic.c >>> @@ -205,6 +205,13 @@ void apic_deliver_pic_intr(DeviceState *d, int level) >>> } >>> } >>> >>> +void apic_deliver_lint1_intr(DeviceState *d) >>> +{ >>> + APICState *s = DO_UPCAST(APICState, busdev.qdev, d); >>> + >>> + apic_local_deliver(s, APIC_LVT_LINT1); >> >> This will cause a qemu crash when apic_state is NULL (non-SMP 486 >> systems). > > Ouch, I see. > What are the interrupt mode used for non-SMP 486 systems? Not sure what you mean with interrupt mode. Those boxes used to have only the classic PIC, no APIC & IOAPIC. I would simply inject the NMI directly (in the old way) if there is no APIC available. Jan
On 09/23/2011 12:31 PM, Lai Jiangshan wrote: > > Moreover: wrong indention. > > > > You know that this won't work for qemu-kvm with in-kernel irqchip? You > > may want to provide a patch for that tree, emulating the unavailable > > LINT1 injection via testing the APIC configration and then raising an > > NMI as before if it is accepted. > > > > It works in my box but the NMI is not injected through the in-kernel irqchip, > I will implement it as you suggested. Somewhat hacky; isn't it better to test LINT1 in the kernel (and redefine the KVM_NMI ioctl as "toggle LINT1")?
On 2011-09-25 16:07, Avi Kivity wrote: > On 09/23/2011 12:31 PM, Lai Jiangshan wrote: >> > Moreover: wrong indention. >> > >> > You know that this won't work for qemu-kvm with in-kernel irqchip? You >> > may want to provide a patch for that tree, emulating the unavailable >> > LINT1 injection via testing the APIC configration and then raising an >> > NMI as before if it is accepted. >> > >> >> It works in my box but the NMI is not injected through the in-kernel >> irqchip, >> I will implement it as you suggested. > > Somewhat hacky; isn't it better to test LINT1 in the kernel (and > redefine the KVM_NMI ioctl as "toggle LINT1")? KVM_NMI is required for user space IRQ chip as well. Introducing some KVM_SET_LINT1 is an option though. But emulating it for the NMI button on older kernels sounds worthwhile nevertheless. Jan
On 09/25/2011 08:22 PM, Jan Kiszka wrote: > On 2011-09-25 16:07, Avi Kivity wrote: > > On 09/23/2011 12:31 PM, Lai Jiangshan wrote: > >> > Moreover: wrong indention. > >> > > >> > You know that this won't work for qemu-kvm with in-kernel irqchip? You > >> > may want to provide a patch for that tree, emulating the unavailable > >> > LINT1 injection via testing the APIC configration and then raising an > >> > NMI as before if it is accepted. > >> > > >> > >> It works in my box but the NMI is not injected through the in-kernel > >> irqchip, > >> I will implement it as you suggested. > > > > Somewhat hacky; isn't it better to test LINT1 in the kernel (and > > redefine the KVM_NMI ioctl as "toggle LINT1")? > > KVM_NMI is required for user space IRQ chip as well. We could define KVM_NMI as edging the core NMI input if !irqchip_in_kernel, and toggling LINT1 otherwise. Hardly nice though. The current KVM_NMI with irqchip_in_kernel is not meaningful, since it doesn't obey the rules of any NMI source. > Introducing some KVM_SET_LINT1 is an option though. But emulating it for > the NMI button on older kernels sounds worthwhile nevertheless. > Perhaps this is the best option to avoid confusion.
On 09/26/2011 04:21 PM, Avi Kivity wrote: > On 09/25/2011 08:22 PM, Jan Kiszka wrote: >> On 2011-09-25 16:07, Avi Kivity wrote: >> > On 09/23/2011 12:31 PM, Lai Jiangshan wrote: >> >> > Moreover: wrong indention. >> >> > >> >> > You know that this won't work for qemu-kvm with in-kernel irqchip? You >> >> > may want to provide a patch for that tree, emulating the unavailable >> >> > LINT1 injection via testing the APIC configration and then raising an >> >> > NMI as before if it is accepted. >> >> > >> >> >> >> It works in my box but the NMI is not injected through the in-kernel >> >> irqchip, >> >> I will implement it as you suggested. >> > >> > Somewhat hacky; isn't it better to test LINT1 in the kernel (and >> > redefine the KVM_NMI ioctl as "toggle LINT1")? >> >> KVM_NMI is required for user space IRQ chip as well. > > We could define KVM_NMI as edging the core NMI input if !irqchip_in_kernel, and toggling LINT1 otherwise. Hardly nice though. > > The current KVM_NMI with irqchip_in_kernel is not meaningful, since it doesn't obey the rules of any NMI source. > >> Introducing some KVM_SET_LINT1 is an option though. But emulating it for >> the NMI button on older kernels sounds worthwhile nevertheless. >> > > Perhaps this is the best option to avoid confusion. > (add cc: seabios@seabios.org) Hi, All, When I was implementing KVM_SET_LINT1, I found many places of the qemu-kvm code need to be changed, and it became nasty. And as Avi said KVM_NMI with irqchip_in_kernel is not meaningful, so KVM_NMI is not used anymore when KVM_SET_LINT1 & irqchip_in_kernel, it is dead. Now, we redefine KVM_NMI with more proper meaning, when irqchip_in_kernel, it is kernel/kvm's responsibility to simulate the NMI-injection and set LINT1. When !irqchip_in_kernel, it is userspace's responsibility. It results more real simulation and results simpler code, and it don't need to add new ioctl interface, and it can make use of existing KVM_NMI. Thanks, Lai
diff --git a/hw/apic.c b/hw/apic.c index 69d6ac5..020305b 100644 --- a/hw/apic.c +++ b/hw/apic.c @@ -205,6 +205,13 @@ void apic_deliver_pic_intr(DeviceState *d, int level) } } +void apic_deliver_lint1_intr(DeviceState *d) +{ + APICState *s = DO_UPCAST(APICState, busdev.qdev, d); + + apic_local_deliver(s, APIC_LVT_LINT1); +} + #define foreach_apic(apic, deliver_bitmask, code) \ {\ int __i, __j, __mask;\ diff --git a/hw/apic.h b/hw/apic.h index c857d52..7ccf214 100644 --- a/hw/apic.h +++ b/hw/apic.h @@ -10,6 +10,7 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t trigger_mode); int apic_accept_pic_intr(DeviceState *s); void apic_deliver_pic_intr(DeviceState *s, int level); +void apic_deliver_lint1_intr(DeviceState *s); int apic_get_interrupt(DeviceState *s); void apic_reset_irq_delivered(void); int apic_get_irq_delivered(void); diff --git a/monitor.c b/monitor.c index cb485bf..d740478 100644 --- a/monitor.c +++ b/monitor.c @@ -2614,9 +2614,9 @@ static void do_wav_capture(Monitor *mon, const QDict *qdict) static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data) { CPUState *env; - + /* This emulates hardware NMI button. So, trigger LINT1 */ for (env = first_cpu; env != NULL; env = env->next_cpu) { - cpu_interrupt(env, CPU_INTERRUPT_NMI); + apic_deliver_lint1_intr(env->apic_state); } return 0;
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Subject: [PATCH] Fix inject-nmi Now, inject-nmi sends NMI to all cpus...but this doesn't emulate pc hardware 'NMI button', which triggers LINT1. So, now, LINT1 mask is ignored by inject-nmi and NMIs are sent to all cpus without checking LINT1 mask. Because Linux masks LINT1 of cpus other than 0, this makes trouble. For example, kdump cannot run sometimes. --- hw/apic.c | 7 +++++++ hw/apic.h | 1 + monitor.c | 4 ++-- 3 files changed, 10 insertions(+), 2 deletions(-) -- 1.7.4.1