Re: [PATCH] qemu-kvm: fix improper nmi emulation

[Jan Kiszka <jan.kiszka@web.de>]

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

On 2011-10-10 08:06, Lai Jiangshan wrote:
> From: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> 
> Currently, NMI interrupt is blindly sent to all the vCPUs when NMI
> button event happens. This doesn't properly emulate real hardware on
> which NMI button event triggers LINT1. Because of this, NMI is sent to
> the processor even when LINT1 is maskied in LVT. For example, this
> causes the problem that kdump initiated by NMI sometimes doesn't work
> on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.
> 
> With this patch, inject-nmi request is handled as follows.
> 
> - When in-kernel irqchip is disabled, inject LINT1 instead of NMI
>   interrupt.
> - When in-kernel irqchip is enabled, send nmi event to kernel as the
>   current code does. LINT1 should be emulated in kernel.
> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> Tested-by: Lai Jiangshan <laijs@cn.fujitsu.com>

This is targeting uq/master?

Please make sure your patch passes checkpatch.pl

> ---
>  hw/apic.c |   16 ++++++++++++++++
>  hw/apic.h |    1 +
>  monitor.c |    5 ++---
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> Index: qemu-kvm/hw/apic.c
> ===================================================================
> --- qemu-kvm.orig/hw/apic.c
> +++ qemu-kvm/hw/apic.c
> @@ -205,6 +205,22 @@ void apic_deliver_pic_intr(DeviceState *
>      }
>  }
>  
> +void apic_deliver_nmi(CPUState *env)
> +{
> +    APICState *apic;
> +
> +    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
> +        cpu_interrupt(env, CPU_INTERRUPT_NMI);
> +	return;
> +    }
> +
> +    apic = DO_UPCAST(APICState, busdev.qdev, env->apic_state);
> +    if (!apic)
> +        cpu_interrupt(env, CPU_INTERRUPT_NMI);

Testing for !apic and handling the non-APIC case here looks a bit
strange. Let's move the !env->apic_state test to the caller to make it
consistent with other APIC services.

The KVM case should be a separate qemu-kvm patch on top for now. (We may
implement calls into APIC models differently when pushing in-kernel
irqchip support upstream.)

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]