Re: [RFC] nmi watchdog in kvm

Avi Kivity
2008-01-30T10:43:58Z

Balaji Rao wrote:
> Hello,
>
> I was trying to enable the use of nmi watchdog within a linux guest running in kvm. I have done it
> by allowing direct access to perfmon msrs using the MSR_BITMAP field in vmcs region.
>
> Most of the times the NMI Watchdog Test in the guest fails, but with a finite number of NMI's
> received by the guest. But randomly it does work! Whenever it fails, i get this vmwrite error :
>
> vmwrite error: reg 4016 value 80000202 (err 164061)
>
> I have a few questions.
>
> 1. How are NMI's supposed to be delivered to the guest ? I did this by adding a new op to
> kvm_x86_ops.
>

That's fine. ops are there to be extended.

> 2. How am I supposed to handle perfmon MSRs ? Direct access may pose problems during migration. But
> am not sure how costly emulation by abstraction would be..
> I have not yet considered saving the MSRS upon vmexits to allow multiple VMs use the MSRs. I think i
> can do them easily when i get this working.
>

I don't think there's a sane way to emulate the non-architectural
perfmon counters. It may be possible to do so for the architectural
ones, but I'm not sure. So pass-through (with saving and restoring) is
the only option.

> Here's the code. Please tell me what dumb mistake I am doing.
>
>
> diff --git a/arch/x86/kernel/cpu/perfctr-watchdog.c b/arch/x86/kernel/cpu/perfctr-watchdog.c
> index c02541e..276048a 100644
> --- a/arch/x86/kernel/cpu/perfctr-watchdog.c
> +++ b/arch/x86/kernel/cpu/perfctr-watchdog.c
> @@ -342,7 +342,7 @@ static const struct wd_ops k7_wd_ops = {
> #define P6_EVNTSEL_INT (1 << 20)
> #define P6_EVNTSEL_OS (1 << 17)
> #define P6_EVNTSEL_USR (1 << 16)
> -#define P6_EVENT_CPU_CLOCKS_NOT_HALTED 0x79
> +#define P6_EVENT_CPU_CLOCKS_NOT_HALTED 0x3C
> #define P6_NMI_EVENT P6_EVENT_CPU_CLOCKS_NOT_HALTED
>

What's this?

>
> static int setup_p6_watchdog(unsigned nmi_hz)
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 2cbee94..73e9361 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -25,6 +25,8 @@
> #include <linux/hrtimer.h>
> #include <linux/io.h>
> #include <linux/module.h>
> +#include <linux/kdebug.h>
> +#include <linux/notifier.h>
> #include <asm/processor.h>
> #include <asm/msr.h>
> #include <asm/page.h>
> @@ -740,9 +742,12 @@ static void apic_mmio_write(struct kvm_io_device *this,
> apic_set_reg(apic, APIC_ICR2, val & 0xff000000);
> break;
>
> + case APIC_LVTPC:
> + /* Enable PC NMI*/
> + if (val == APIC_DM_NMI)
> + apic_write(APIC_LVTPC,val);
>

You're writing to the precious host apic here.

- need to disallow if the host is using it
- need to prevent illegal values
- need to use some sort of perfmon api rather than directly banging on
the apic

> @@ -790,6 +795,18 @@ static int apic_mmio_range(struct kvm_io_device *this, gpa_t addr)
> return ret;
> }
>
> +static int nmi_notify(struct notifier_block *self,unsigned long val, void *data) {
> +
> + struct kvm *kvm;
> + kvm = list_entry(vm_list.next, struct kvm, vm_list);
> + kvm_x86_ops->inject_nmi(kvm->vcpus[0]);
> + return NOTIFY_STOP;
> +}
>

You're not guaranteed to be in vcpu context here, that's what's causing
the vmwrite errors.

Enabling on guest entry and disabling on guest exit is critical, both
for accuracy and correctness.

--
error compiling committee.c: too many arguments to function