Re: [PATCH 6/9] KVM: Adds support for real NMI injection onVMX processors

Re: [PATCH 6/9] KVM: Adds support for real NMI injection onVMX processors

[Gregory Haskins]

>>> On Thu, May 31, 2007 at 10:43 PM, in message
<B30DA1341B0CFA4893EF8A36B40B5C5D0134E080-wq7ZOvIWXbNpB2pF5aRoyrfspsVTdybXVpNB7YpNyf8@public.gmane.org>, "Li,
Xin B" <xin.b.li-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote: 
> I don't think this patch is complete to add NMIP support to KVM

Could you be more specific on whats left to be done?  From what I could read in the doc, this seemed complete.

> and can you pls exclude it from current APIC patchsets? We can revisit it later
> with more thoughts.

I don't have a problem with excluding it.  Honestly, I primarily wanted a working demonstration of how I envisioned the kvm_irqpin mechanism to work more than anything.  The woodcrest I have doesn't seem to support VNMI, so I am not able to take advantage of this patch anyway.  Since the woodcrest is pretty new, I assume most VMX systems would run into similar limitations.  I haven't tried it yet on my merom laptop, but I would guess it has the same problem since they are both C2D.

Regards,
-Greg

> 
> -  Xin 
> 
>>-----  Original Message-----
>>From: kvm-  devel-  bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org 
>>[mailto:kvm-  devel-  bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of 
>>Gregory Haskins
>>Sent: Friday, June 01, 2007 2:09 AM
>>To: kvm-  devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>>Subject: [kvm-  devel] [PATCH 6/9] KVM: Adds support for real 
>>NMI injection onVMX processors
>>
>>Signed-  off-  by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
>>---
>>
>> drivers/kvm/vmx.c |   56 
>>+++++++++++++++++++++++++++++++++++++++++++++++++----
>> drivers/kvm/vmx.h |    3 +++
>> 2 files changed, 55 insertions(+), 4 deletions(-  )
>>
>>diff --  git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
>>index 82e40c9..7923a42 100644
>>---   a/drivers/kvm/vmx.c
>>+++ b/drivers/kvm/vmx.c
>>@@ -  1301,7 +1301,14 @@ static int vmx_vcpu_setup(struct kvm_vcpu *vcpu)
>> 			       PIN_BASED_VM_EXEC_CONTROL,
>> 			       PIN_BASED_EXT_INTR_MASK   /* 20.6.1 */
>> 			       | PIN_BASED_NMI_EXITING   /* 20.6.1 */
>>+			       | PIN_BASED_VIRTUAL_NMI   /* 20.6.1 */
>> 			);
>>+
>>+	if (!(vmcs_read32(PIN_BASED_VM_EXEC_CONTROL) & 
>>PIN_BASED_VIRTUAL_NMI))
>>+		printk(KERN_DEBUG "KVM: Warning -   Host 
>>processor does " \
>>+		       "not support virtual-  NMI injection.  
>>Using IRQ " \
>>+		       "method\n");
>>+ 
>> 	vmcs_write32_fixedbits(MSR_IA32_VMX_PROCBASED_CTLS,
>> 			       CPU_BASED_VM_EXEC_CONTROL,
>> 			       CPU_BASED_HLT_EXITING         /* 
>>20.6.2 */
>>@@ -  1450,6 +1457,37 @@ static void inject_rmode_irq(struct 
>>kvm_vcpu *vcpu, int irq)
>> 	vmcs_writel(GUEST_RSP, (vmcs_readl(GUEST_RSP) & 
>>~0xffff) | (sp -   6));
>> }
>> 
>>+static void do_nmi_requests(struct kvm_vcpu *vcpu)
>>+{
>>+	int nmi_window = 0;
>>+
>>+	BUG_ON(!(test_bit(kvm_irqpin_nmi, &vcpu-  >irq.pending)));
>>+
>>+	nmi_window =
>>+		(((vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 0xb) == 0)
>>+		 && (vmcs_read32(VM_ENTRY_INTR_INFO_FIELD)
>>+		     & INTR_INFO_VALID_MASK));
>>+
>>+	if (nmi_window) {
>>+		if (vcpu-  >rmode.active)
>>+			inject_rmode_irq(vcpu, 2);
>>+		else
>>+			vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
>>+				     2 |
>>+				     INTR_TYPE_NMI |
>>+				     INTR_INFO_VALID_MASK);
>>+
>>+		__clear_bit(kvm_irqpin_nmi, &vcpu-  >irq.pending);
>>+	} else {
>>+		/*
>>+		 * NMIs blocked.  Wait for unblock.
>>+		 */
>>+		u32 cbvec = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
>>+		cbvec |= CPU_BASED_NMI_EXITING;
>>+		vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);	
>>+	}
>>+}
>>+
>> static void do_intr_requests(struct kvm_vcpu *vcpu,
>> 			    struct kvm_run *kvm_run,
>> 			    kvm_irqpin_t pin)
>>@@ -  1482,9 +1520,11 @@ static void do_intr_requests(struct 
>>kvm_vcpu *vcpu,
>> 			break;
>> 		case kvm_irqpin_nmi:
>> 			/*
>>-			 * FIXME: Someday we will handle this using the
>>-			 * specific VMX NMI features.  For now, 
>>just inject
>>-			 * the NMI as a standard interrupt on vector 2
>>+			 * We should only get here if the processor does
>>+			 * not support virtual NMIs.  Inject 
>>the NMI as a
>>+			 * standard interrupt on vector 2.  The 
>>implication is
>>+			 * that NMIs are going to be subject to 
>>RFLAGS.IF
>>+			 * masking, unfortunately.
>> 			 */
>> 			ack.flags |= KVM_IRQACKDATA_VECTOR_VALID;
>> 			ack.vector = 2;
>>@@ -  1534,6 +1574,8 @@ static void 
>>clear_pending_controls(struct kvm_vcpu *vcpu,
>>  	else
>>  		cbvec &= ~CPU_BASED_VIRTUAL_INTR_PENDING;
>> 
>>+	cbvec &= ~CPU_BASED_NMI_EXITING;
>>+
>> 	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);
>> }
>> 
>>@@ -  1550,7 +1592,6 @@ static void do_interrupt_requests(struct 
>>kvm_vcpu *vcpu,
>> 		switch (pin) {
>> 		case kvm_irqpin_localint:
>> 		case kvm_irqpin_extint:
>>-		case kvm_irqpin_nmi:
>> 			do_intr_requests(vcpu, kvm_run, pin);
>> 			break;
>> 		case kvm_irqpin_smi:
>>@@ -  1558,6 +1599,13 @@ static void 
>>do_interrupt_requests(struct kvm_vcpu *vcpu,
>> 			printk(KERN_WARNING "KVM: dropping 
>>unhandled SMI\n");
>> 			__clear_bit(pin, &vcpu-  >irq.pending);
>> 			break;
>>+		case kvm_irqpin_nmi:
>>+			if (vmcs_read32(PIN_BASED_VM_EXEC_CONTROL)
>>+			    & PIN_BASED_VIRTUAL_NMI)
>>+				do_nmi_requests(vcpu);
>>+			else
>>+				do_intr_requests(vcpu, kvm_run, pin);	
>>+			break;
>> 		case kvm_irqpin_invalid:
>> 			/* drop */
>> 			break;
>>diff --  git a/drivers/kvm/vmx.h b/drivers/kvm/vmx.h
>>index d0dc93d..d3fe017 100644
>>---   a/drivers/kvm/vmx.h
>>+++ b/drivers/kvm/vmx.h
>>@@ -  35,6 +35,7 @@
>> #define CPU_BASED_CR8_LOAD_EXITING      0x00080000
>> #define CPU_BASED_CR8_STORE_EXITING     0x00100000
>> #define CPU_BASED_TPR_SHADOW            0x00200000
>>+#define CPU_BASED_NMI_EXITING           0x00400000
>> #define CPU_BASED_MOV_DR_EXITING        0x00800000
>> #define CPU_BASED_UNCOND_IO_EXITING     0x01000000
>> #define CPU_BASED_ACTIVATE_IO_BITMAP    0x02000000
>>@@ -  44,6 +45,7 @@
>> 
>> #define PIN_BASED_EXT_INTR_MASK 0x1
>> #define PIN_BASED_NMI_EXITING   0x8
>>+#define PIN_BASED_VIRTUAL_NMI   0x20
>> 
>> #define VM_EXIT_ACK_INTR_ON_EXIT        0x00008000
>> #define VM_EXIT_HOST_ADD_SPACE_SIZE     0x00000200
>>@@ -  221,6 +223,7 @@ enum vmcs_field {
>> #define VECTORING_INFO_VALID_MASK       	INTR_INFO_VALID_MASK
>> 
>> #define INTR_TYPE_EXT_INTR              (0 << 8) /* external 
>>interrupt */
>>+#define INTR_TYPE_NMI                   (2 << 8) /* 
>>non-  maskable interrupt */
>> #define INTR_TYPE_EXCEPTION             (3 << 8) /* processor 
>>exception */
>> 
>> /*
>>
>>
>>---------------------------------------------------------------
>>----------
>>This SF.net email is sponsored by DB2 Express
>>Download DB2 Express C -   the FREE version of DB2 express and take
>>control of your XML. No limits. Just data. Click to get it now.
>>http://sourceforge.net/powerbar/db2/
>>_______________________________________________
>>kvm-  devel mailing list
>>kvm-  devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>>https://lists.sourceforge.net/lists/listinfo/kvm-  devel
>>




-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 6/9] KVM: Adds support for real NMIinjection onVMX processors

[Li, Xin B]

>> I don't think this patch is complete to add NMIP support to KVM
>
>Could you be more specific on whats left to be done?  From 
>what I could read in the doc, this seemed complete.
>

This is a good start, and we also need to add a vmexit handler to handle
exit reason 8, NMI pending.
And the major issue, I think, is how guest uses NMI, although from
virtualization view, we should not think from that, but we need make
sure all possible guest NMI usage model works well.
Thanks.
-Xin



>> and can you pls exclude it from current APIC patchsets? We 
>can revisit it later with more thoughts.
>
>I don't have a problem with excluding it.  Honestly, I 
>primarily wanted a working demonstration of how I envisioned 
>the kvm_irqpin mechanism to work more than anything.  The 
>woodcrest I have doesn't seem to support VNMI, so I am not 
>able to take advantage of this patch anyway.  Since the 
>woodcrest is pretty new, I assume most VMX systems would run 
>into similar limitations.  I haven't tried it yet on my merom 
>laptop, but I would guess it has the same problem since they 
>are both C2D.
>
>Regards,
>-Greg
>
>> 
>> -  Xin 
>> 
>>>-----  Original Message-----
>>>From: kvm-  devel-  bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org 
>>>[mailto:kvm-  devel-  bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of 
>>>Gregory Haskins
>>>Sent: Friday, June 01, 2007 2:09 AM
>>>To: kvm-  devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>>>Subject: [kvm-  devel] [PATCH 6/9] KVM: Adds support for real 
>>>NMI injection onVMX processors
>>>
>>>Signed-  off-  by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
>>>---
>>>
>>> drivers/kvm/vmx.c |   56 
>>>+++++++++++++++++++++++++++++++++++++++++++++++++----
>>> drivers/kvm/vmx.h |    3 +++
>>> 2 files changed, 55 insertions(+), 4 deletions(-  )
>>>
>>>diff --  git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
>>>index 82e40c9..7923a42 100644
>>>---   a/drivers/kvm/vmx.c
>>>+++ b/drivers/kvm/vmx.c
>>>@@ -  1301,7 +1301,14 @@ static int vmx_vcpu_setup(struct 
>kvm_vcpu *vcpu)
>>> 			       PIN_BASED_VM_EXEC_CONTROL,
>>> 			       PIN_BASED_EXT_INTR_MASK   /* 20.6.1 */
>>> 			       | PIN_BASED_NMI_EXITING   /* 20.6.1 */
>>>+			       | PIN_BASED_VIRTUAL_NMI   /* 20.6.1 */
>>> 			);
>>>+
>>>+	if (!(vmcs_read32(PIN_BASED_VM_EXEC_CONTROL) & 
>>>PIN_BASED_VIRTUAL_NMI))
>>>+		printk(KERN_DEBUG "KVM: Warning -   Host 
>>>processor does " \
>>>+		       "not support virtual-  NMI injection.  
>>>Using IRQ " \
>>>+		       "method\n");
>>>+ 
>>> 	vmcs_write32_fixedbits(MSR_IA32_VMX_PROCBASED_CTLS,
>>> 			       CPU_BASED_VM_EXEC_CONTROL,
>>> 			       CPU_BASED_HLT_EXITING         /* 
>>>20.6.2 */
>>>@@ -  1450,6 +1457,37 @@ static void inject_rmode_irq(struct 
>>>kvm_vcpu *vcpu, int irq)
>>> 	vmcs_writel(GUEST_RSP, (vmcs_readl(GUEST_RSP) & 
>>>~0xffff) | (sp -   6));
>>> }
>>> 
>>>+static void do_nmi_requests(struct kvm_vcpu *vcpu)
>>>+{
>>>+	int nmi_window = 0;
>>>+
>>>+	BUG_ON(!(test_bit(kvm_irqpin_nmi, &vcpu-  >irq.pending)));
>>>+
>>>+	nmi_window =
>>>+		(((vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 0xb) == 0)
>>>+		 && (vmcs_read32(VM_ENTRY_INTR_INFO_FIELD)
>>>+		     & INTR_INFO_VALID_MASK));
>>>+
>>>+	if (nmi_window) {
>>>+		if (vcpu-  >rmode.active)
>>>+			inject_rmode_irq(vcpu, 2);
>>>+		else
>>>+			vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
>>>+				     2 |
>>>+				     INTR_TYPE_NMI |
>>>+				     INTR_INFO_VALID_MASK);
>>>+
>>>+		__clear_bit(kvm_irqpin_nmi, &vcpu-  >irq.pending);
>>>+	} else {
>>>+		/*
>>>+		 * NMIs blocked.  Wait for unblock.
>>>+		 */
>>>+		u32 cbvec = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
>>>+		cbvec |= CPU_BASED_NMI_EXITING;
>>>+		vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);	
>>>+	}
>>>+}
>>>+
>>> static void do_intr_requests(struct kvm_vcpu *vcpu,
>>> 			    struct kvm_run *kvm_run,
>>> 			    kvm_irqpin_t pin)
>>>@@ -  1482,9 +1520,11 @@ static void do_intr_requests(struct 
>>>kvm_vcpu *vcpu,
>>> 			break;
>>> 		case kvm_irqpin_nmi:
>>> 			/*
>>>-			 * FIXME: Someday we will handle this using the
>>>-			 * specific VMX NMI features.  For now, 
>>>just inject
>>>-			 * the NMI as a standard interrupt on vector 2
>>>+			 * We should only get here if the processor does
>>>+			 * not support virtual NMIs.  Inject 
>>>the NMI as a
>>>+			 * standard interrupt on vector 2.  The 
>>>implication is
>>>+			 * that NMIs are going to be subject to 
>>>RFLAGS.IF
>>>+			 * masking, unfortunately.
>>> 			 */
>>> 			ack.flags |= KVM_IRQACKDATA_VECTOR_VALID;
>>> 			ack.vector = 2;
>>>@@ -  1534,6 +1574,8 @@ static void 
>>>clear_pending_controls(struct kvm_vcpu *vcpu,
>>>  	else
>>>  		cbvec &= ~CPU_BASED_VIRTUAL_INTR_PENDING;
>>> 
>>>+	cbvec &= ~CPU_BASED_NMI_EXITING;
>>>+
>>> 	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);
>>> }
>>> 
>>>@@ -  1550,7 +1592,6 @@ static void do_interrupt_requests(struct 
>>>kvm_vcpu *vcpu,
>>> 		switch (pin) {
>>> 		case kvm_irqpin_localint:
>>> 		case kvm_irqpin_extint:
>>>-		case kvm_irqpin_nmi:
>>> 			do_intr_requests(vcpu, kvm_run, pin);
>>> 			break;
>>> 		case kvm_irqpin_smi:
>>>@@ -  1558,6 +1599,13 @@ static void 
>>>do_interrupt_requests(struct kvm_vcpu *vcpu,
>>> 			printk(KERN_WARNING "KVM: dropping 
>>>unhandled SMI\n");
>>> 			__clear_bit(pin, &vcpu-  >irq.pending);
>>> 			break;
>>>+		case kvm_irqpin_nmi:
>>>+			if (vmcs_read32(PIN_BASED_VM_EXEC_CONTROL)
>>>+			    & PIN_BASED_VIRTUAL_NMI)
>>>+				do_nmi_requests(vcpu);
>>>+			else
>>>+				do_intr_requests(vcpu, kvm_run, pin);	
>>>+			break;
>>> 		case kvm_irqpin_invalid:
>>> 			/* drop */
>>> 			break;
>>>diff --  git a/drivers/kvm/vmx.h b/drivers/kvm/vmx.h
>>>index d0dc93d..d3fe017 100644
>>>---   a/drivers/kvm/vmx.h
>>>+++ b/drivers/kvm/vmx.h
>>>@@ -  35,6 +35,7 @@
>>> #define CPU_BASED_CR8_LOAD_EXITING      0x00080000
>>> #define CPU_BASED_CR8_STORE_EXITING     0x00100000
>>> #define CPU_BASED_TPR_SHADOW            0x00200000
>>>+#define CPU_BASED_NMI_EXITING           0x00400000
>>> #define CPU_BASED_MOV_DR_EXITING        0x00800000
>>> #define CPU_BASED_UNCOND_IO_EXITING     0x01000000
>>> #define CPU_BASED_ACTIVATE_IO_BITMAP    0x02000000
>>>@@ -  44,6 +45,7 @@
>>> 
>>> #define PIN_BASED_EXT_INTR_MASK 0x1
>>> #define PIN_BASED_NMI_EXITING   0x8
>>>+#define PIN_BASED_VIRTUAL_NMI   0x20
>>> 
>>> #define VM_EXIT_ACK_INTR_ON_EXIT        0x00008000
>>> #define VM_EXIT_HOST_ADD_SPACE_SIZE     0x00000200
>>>@@ -  221,6 +223,7 @@ enum vmcs_field {
>>> #define VECTORING_INFO_VALID_MASK       	INTR_INFO_VALID_MASK
>>> 
>>> #define INTR_TYPE_EXT_INTR              (0 << 8) /* external 
>>>interrupt */
>>>+#define INTR_TYPE_NMI                   (2 << 8) /* 
>>>non-  maskable interrupt */
>>> #define INTR_TYPE_EXCEPTION             (3 << 8) /* processor 
>>>exception */
>>> 
>>> /*
>>>
>>>
>>>---------------------------------------------------------------
>>>----------
>>>This SF.net email is sponsored by DB2 Express
>>>Download DB2 Express C -   the FREE version of DB2 express and take
>>>control of your XML. No limits. Just data. Click to get it now.
>>>http://sourceforge.net/powerbar/db2/
>>>_______________________________________________
>>>kvm-  devel mailing list
>>>kvm-  devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>>>https://lists.sourceforge.net/lists/listinfo/kvm-  devel
>>>
>
>

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 6/9] KVM: Adds support for real NMIinjection onVMX processors

[Gregory Haskins]

>>> On Fri, Jun 1, 2007 at 12:29 AM, in message
<B30DA1341B0CFA4893EF8A36B40B5C5D0134E12F-wq7ZOvIWXbNpB2pF5aRoyrfspsVTdybXVpNB7YpNyf8@public.gmane.org>, "Li,
Xin B" <xin.b.li-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote: 
>> > I don't think this patch is complete to add NMIP support to KVM
>>
>>Could you be more specific on whats left to be done?  From 
>>what I could read in the doc, this seemed complete.
>>
> 
> This is a good start, and we also need to add a vmexit handler to handle
> exit reason 8, NMI pending.

Ah, yes.  Thank you for finding that.

> And the major issue, I think, is how guest uses NMI, although from
> virtualization view, we should not think from that, but we need make
> sure all possible guest NMI usage model works well.

Note that the guest can (and does) use NMI today independent of this patch and it works fine.  In fact, some version of linux do this on a regular basis (they map LINT0 to NMI).  

This patch only affects whether NMIs are injected using the standard event-injection (INTR_TYPE_EXT_INTR) technique on vector 2, or using the VNMI facility (INTR_TYPE_NMI) instead.  My impression is that the guest dispatches the IRQ-2 to the IDT in the same way in either case.  The only difference is that VNMI method allows the event to be injected even if RFLAGS.IF is masking normal interrupts, thus more closely modeling the real world counterpart.

So that being said, if you think there is a problem with injecting NMIs to guests in general then we have a bigger problem. This patch really only affects *how* they are delivered, not *if* they are delivered.  The counterpoint to all this is that I have been testing with plenty of guests that take NMIs and they seem to be working normally.  Please advise if you think there is a bigger problem we need to address.

Regards
-Greg

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 6/9] KVM: Adds support for real NMIinjectiononVMX processors

[Dong, Eddie]

kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org wrote:
>>> Could you be more specific on whats left to be done?  From
>>> what I could read in the doc, this seemed complete.
>>> 
>> 
>> This is a good start, and we also need to add a vmexit handler to
>> handle exit reason 8, NMI pending.
> 
> Ah, yes.  Thank you for finding that.
> 
>> And the major issue, I think, is how guest uses NMI, although from
>> virtualization view, we should not think from that, but we need make
>> sure all possible guest NMI usage model works well.
> 
> Note that the guest can (and does) use NMI today independent
> of this patch and it works fine.  In fact, some version of
> linux do this on a regular basis (they map LINT0 to NMI).
> 
> This patch only affects whether NMIs are injected using the
> standard event-injection (INTR_TYPE_EXT_INTR) technique on
> vector 2, or using the VNMI facility (INTR_TYPE_NMI) instead.
> My impression is that the guest dispatches the IRQ-2 to the
> IDT in the same way in either case.  The only difference is
> that VNMI method allows the event to be injected even if
> RFLAGS.IF is masking normal interrupts, thus more closely
> modeling the real world counterpart.
> 
NMI can be injected even RFLAGS.IF=0, but it can be blocked by 
previous NMI. If we want to handle NMI in this patch, we need to
add the logic of NMI block and unblock. Like you said, this needs 
new hardware support and may not in your box yet :-(
BTW, We have a close to workable patch for NMI support in Xen 
now, but still debug on that, I would image 
we can reuse that patch in KVM easily, so maybe temply remove it is 
a good idea :-)
Eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[PATCH 0/9] in-kernel APIC v9 (kernel side)

[Gregory Haskins]

Incorporates v8 plus the following changes:

1) Fix for hang on AMD
2) Fixes issue where irq-windows are inaccurately reported to userspace
3) Fixed issue where irq-window-exiting requests can be ignored in some cases

Note that we no longer need the backlog.patch to handle a corner cases now.

As before, this has been tested on 32 bit XP w/ACPI and 64 bit windows.  It
offers a 17% performance improvement over git HEAD in my testing.  Note that I
am not able to fully verify that this works on AMD, as even git-head does not
work on my system.  I am able to verify that it no longer hangs the kernel
hard.  The guest hangs, but it hangs without my patches as well.  Perhaps
someone with a known good environment on AMD can verify for me?

I am being pulled off of my KVM work for a little while, so I will not be able
to contribute again until further notice. If there are any remaining issues
that need to be addressed and someone wants to carry the torch, feel free to
do so.  Otherwise, I will pick up the effort to get this merged in when I am
able to return to KVM.

Thanks all for the feedback/comments/suggestions through all of this.  It has
been very fun and quite a learning experience.

-Greg
 

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[PATCH 6/9] KVM: Adds support for real NMI injection on VMX processors

[Gregory Haskins]

Signed-off-by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
---

 drivers/kvm/vmx.c |   56 +++++++++++++++++++++++++++++++++++++++++++++++++----
 drivers/kvm/vmx.h |    3 +++
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 82e40c9..7923a42 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -1301,7 +1301,14 @@ static int vmx_vcpu_setup(struct kvm_vcpu *vcpu)
 			       PIN_BASED_VM_EXEC_CONTROL,
 			       PIN_BASED_EXT_INTR_MASK   /* 20.6.1 */
 			       | PIN_BASED_NMI_EXITING   /* 20.6.1 */
+			       | PIN_BASED_VIRTUAL_NMI   /* 20.6.1 */
 			);
+
+	if (!(vmcs_read32(PIN_BASED_VM_EXEC_CONTROL) & PIN_BASED_VIRTUAL_NMI))
+		printk(KERN_DEBUG "KVM: Warning - Host processor does " \
+		       "not support virtual-NMI injection.  Using IRQ " \
+		       "method\n");
+ 
 	vmcs_write32_fixedbits(MSR_IA32_VMX_PROCBASED_CTLS,
 			       CPU_BASED_VM_EXEC_CONTROL,
 			       CPU_BASED_HLT_EXITING         /* 20.6.2 */
@@ -1450,6 +1457,37 @@ static void inject_rmode_irq(struct kvm_vcpu *vcpu, int irq)
 	vmcs_writel(GUEST_RSP, (vmcs_readl(GUEST_RSP) & ~0xffff) | (sp - 6));
 }
 
+static void do_nmi_requests(struct kvm_vcpu *vcpu)
+{
+	int nmi_window = 0;
+
+	BUG_ON(!(test_bit(kvm_irqpin_nmi, &vcpu->irq.pending)));
+
+	nmi_window =
+		(((vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 0xb) == 0)
+		 && (vmcs_read32(VM_ENTRY_INTR_INFO_FIELD)
+		     & INTR_INFO_VALID_MASK));
+
+	if (nmi_window) {
+		if (vcpu->rmode.active)
+			inject_rmode_irq(vcpu, 2);
+		else
+			vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
+				     2 |
+				     INTR_TYPE_NMI |
+				     INTR_INFO_VALID_MASK);
+
+		__clear_bit(kvm_irqpin_nmi, &vcpu->irq.pending);
+	} else {
+		/*
+		 * NMIs blocked.  Wait for unblock.
+		 */
+		u32 cbvec = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
+		cbvec |= CPU_BASED_NMI_EXITING;
+		vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);	
+	}
+}
+
 static void do_intr_requests(struct kvm_vcpu *vcpu,
 			    struct kvm_run *kvm_run,
 			    kvm_irqpin_t pin)
@@ -1482,9 +1520,11 @@ static void do_intr_requests(struct kvm_vcpu *vcpu,
 			break;
 		case kvm_irqpin_nmi:
 			/*
-			 * FIXME: Someday we will handle this using the
-			 * specific VMX NMI features.  For now, just inject
-			 * the NMI as a standard interrupt on vector 2
+			 * We should only get here if the processor does
+			 * not support virtual NMIs.  Inject the NMI as a
+			 * standard interrupt on vector 2.  The implication is
+			 * that NMIs are going to be subject to RFLAGS.IF
+			 * masking, unfortunately.
 			 */
 			ack.flags |= KVM_IRQACKDATA_VECTOR_VALID;
 			ack.vector = 2;
@@ -1534,6 +1574,8 @@ static void clear_pending_controls(struct kvm_vcpu *vcpu,
  	else
  		cbvec &= ~CPU_BASED_VIRTUAL_INTR_PENDING;
 
+	cbvec &= ~CPU_BASED_NMI_EXITING;
+
 	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);
 }
 
@@ -1550,7 +1592,6 @@ static void do_interrupt_requests(struct kvm_vcpu *vcpu,
 		switch (pin) {
 		case kvm_irqpin_localint:
 		case kvm_irqpin_extint:
-		case kvm_irqpin_nmi:
 			do_intr_requests(vcpu, kvm_run, pin);
 			break;
 		case kvm_irqpin_smi:
@@ -1558,6 +1599,13 @@ static void do_interrupt_requests(struct kvm_vcpu *vcpu,
 			printk(KERN_WARNING "KVM: dropping unhandled SMI\n");
 			__clear_bit(pin, &vcpu->irq.pending);
 			break;
+		case kvm_irqpin_nmi:
+			if (vmcs_read32(PIN_BASED_VM_EXEC_CONTROL)
+			    & PIN_BASED_VIRTUAL_NMI)
+				do_nmi_requests(vcpu);
+			else
+				do_intr_requests(vcpu, kvm_run, pin);	
+			break;
 		case kvm_irqpin_invalid:
 			/* drop */
 			break;
diff --git a/drivers/kvm/vmx.h b/drivers/kvm/vmx.h
index d0dc93d..d3fe017 100644
--- a/drivers/kvm/vmx.h
+++ b/drivers/kvm/vmx.h
@@ -35,6 +35,7 @@
 #define CPU_BASED_CR8_LOAD_EXITING      0x00080000
 #define CPU_BASED_CR8_STORE_EXITING     0x00100000
 #define CPU_BASED_TPR_SHADOW            0x00200000
+#define CPU_BASED_NMI_EXITING           0x00400000
 #define CPU_BASED_MOV_DR_EXITING        0x00800000
 #define CPU_BASED_UNCOND_IO_EXITING     0x01000000
 #define CPU_BASED_ACTIVATE_IO_BITMAP    0x02000000
@@ -44,6 +45,7 @@
 
 #define PIN_BASED_EXT_INTR_MASK 0x1
 #define PIN_BASED_NMI_EXITING   0x8
+#define PIN_BASED_VIRTUAL_NMI   0x20
 
 #define VM_EXIT_ACK_INTR_ON_EXIT        0x00008000
 #define VM_EXIT_HOST_ADD_SPACE_SIZE     0x00000200
@@ -221,6 +223,7 @@ enum vmcs_field {
 #define VECTORING_INFO_VALID_MASK       	INTR_INFO_VALID_MASK
 
 #define INTR_TYPE_EXT_INTR              (0 << 8) /* external interrupt */
+#define INTR_TYPE_NMI                   (2 << 8) /* non-maskable interrupt */
 #define INTR_TYPE_EXCEPTION             (3 << 8) /* processor exception */
 
 /*


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 6/9] KVM: Adds support for real NMI injection onVMX processors

[Li, Xin B]

I don't think this patch is complete to add NMIP support to KVM, and can
you pls exclude it from current APIC patchsets? We can revisit it later
with more thoughts.

-Xin 

>-----Original Message-----
>From: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org 
>[mailto:kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of 
>Gregory Haskins
>Sent: Friday, June 01, 2007 2:09 AM
>To: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>Subject: [kvm-devel] [PATCH 6/9] KVM: Adds support for real 
>NMI injection onVMX processors
>
>Signed-off-by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
>---
>
> drivers/kvm/vmx.c |   56 
>+++++++++++++++++++++++++++++++++++++++++++++++++----
> drivers/kvm/vmx.h |    3 +++
> 2 files changed, 55 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
>index 82e40c9..7923a42 100644
>--- a/drivers/kvm/vmx.c
>+++ b/drivers/kvm/vmx.c
>@@ -1301,7 +1301,14 @@ static int vmx_vcpu_setup(struct kvm_vcpu *vcpu)
> 			       PIN_BASED_VM_EXEC_CONTROL,
> 			       PIN_BASED_EXT_INTR_MASK   /* 20.6.1 */
> 			       | PIN_BASED_NMI_EXITING   /* 20.6.1 */
>+			       | PIN_BASED_VIRTUAL_NMI   /* 20.6.1 */
> 			);
>+
>+	if (!(vmcs_read32(PIN_BASED_VM_EXEC_CONTROL) & 
>PIN_BASED_VIRTUAL_NMI))
>+		printk(KERN_DEBUG "KVM: Warning - Host 
>processor does " \
>+		       "not support virtual-NMI injection.  
>Using IRQ " \
>+		       "method\n");
>+ 
> 	vmcs_write32_fixedbits(MSR_IA32_VMX_PROCBASED_CTLS,
> 			       CPU_BASED_VM_EXEC_CONTROL,
> 			       CPU_BASED_HLT_EXITING         /* 
>20.6.2 */
>@@ -1450,6 +1457,37 @@ static void inject_rmode_irq(struct 
>kvm_vcpu *vcpu, int irq)
> 	vmcs_writel(GUEST_RSP, (vmcs_readl(GUEST_RSP) & 
>~0xffff) | (sp - 6));
> }
> 
>+static void do_nmi_requests(struct kvm_vcpu *vcpu)
>+{
>+	int nmi_window = 0;
>+
>+	BUG_ON(!(test_bit(kvm_irqpin_nmi, &vcpu->irq.pending)));
>+
>+	nmi_window =
>+		(((vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 0xb) == 0)
>+		 && (vmcs_read32(VM_ENTRY_INTR_INFO_FIELD)
>+		     & INTR_INFO_VALID_MASK));
>+
>+	if (nmi_window) {
>+		if (vcpu->rmode.active)
>+			inject_rmode_irq(vcpu, 2);
>+		else
>+			vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
>+				     2 |
>+				     INTR_TYPE_NMI |
>+				     INTR_INFO_VALID_MASK);
>+
>+		__clear_bit(kvm_irqpin_nmi, &vcpu->irq.pending);
>+	} else {
>+		/*
>+		 * NMIs blocked.  Wait for unblock.
>+		 */
>+		u32 cbvec = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
>+		cbvec |= CPU_BASED_NMI_EXITING;
>+		vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);	
>+	}
>+}
>+
> static void do_intr_requests(struct kvm_vcpu *vcpu,
> 			    struct kvm_run *kvm_run,
> 			    kvm_irqpin_t pin)
>@@ -1482,9 +1520,11 @@ static void do_intr_requests(struct 
>kvm_vcpu *vcpu,
> 			break;
> 		case kvm_irqpin_nmi:
> 			/*
>-			 * FIXME: Someday we will handle this using the
>-			 * specific VMX NMI features.  For now, 
>just inject
>-			 * the NMI as a standard interrupt on vector 2
>+			 * We should only get here if the processor does
>+			 * not support virtual NMIs.  Inject 
>the NMI as a
>+			 * standard interrupt on vector 2.  The 
>implication is
>+			 * that NMIs are going to be subject to 
>RFLAGS.IF
>+			 * masking, unfortunately.
> 			 */
> 			ack.flags |= KVM_IRQACKDATA_VECTOR_VALID;
> 			ack.vector = 2;
>@@ -1534,6 +1574,8 @@ static void 
>clear_pending_controls(struct kvm_vcpu *vcpu,
>  	else
>  		cbvec &= ~CPU_BASED_VIRTUAL_INTR_PENDING;
> 
>+	cbvec &= ~CPU_BASED_NMI_EXITING;
>+
> 	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cbvec);
> }
> 
>@@ -1550,7 +1592,6 @@ static void do_interrupt_requests(struct 
>kvm_vcpu *vcpu,
> 		switch (pin) {
> 		case kvm_irqpin_localint:
> 		case kvm_irqpin_extint:
>-		case kvm_irqpin_nmi:
> 			do_intr_requests(vcpu, kvm_run, pin);
> 			break;
> 		case kvm_irqpin_smi:
>@@ -1558,6 +1599,13 @@ static void 
>do_interrupt_requests(struct kvm_vcpu *vcpu,
> 			printk(KERN_WARNING "KVM: dropping 
>unhandled SMI\n");
> 			__clear_bit(pin, &vcpu->irq.pending);
> 			break;
>+		case kvm_irqpin_nmi:
>+			if (vmcs_read32(PIN_BASED_VM_EXEC_CONTROL)
>+			    & PIN_BASED_VIRTUAL_NMI)
>+				do_nmi_requests(vcpu);
>+			else
>+				do_intr_requests(vcpu, kvm_run, pin);	
>+			break;
> 		case kvm_irqpin_invalid:
> 			/* drop */
> 			break;
>diff --git a/drivers/kvm/vmx.h b/drivers/kvm/vmx.h
>index d0dc93d..d3fe017 100644
>--- a/drivers/kvm/vmx.h
>+++ b/drivers/kvm/vmx.h
>@@ -35,6 +35,7 @@
> #define CPU_BASED_CR8_LOAD_EXITING      0x00080000
> #define CPU_BASED_CR8_STORE_EXITING     0x00100000
> #define CPU_BASED_TPR_SHADOW            0x00200000
>+#define CPU_BASED_NMI_EXITING           0x00400000
> #define CPU_BASED_MOV_DR_EXITING        0x00800000
> #define CPU_BASED_UNCOND_IO_EXITING     0x01000000
> #define CPU_BASED_ACTIVATE_IO_BITMAP    0x02000000
>@@ -44,6 +45,7 @@
> 
> #define PIN_BASED_EXT_INTR_MASK 0x1
> #define PIN_BASED_NMI_EXITING   0x8
>+#define PIN_BASED_VIRTUAL_NMI   0x20
> 
> #define VM_EXIT_ACK_INTR_ON_EXIT        0x00008000
> #define VM_EXIT_HOST_ADD_SPACE_SIZE     0x00000200
>@@ -221,6 +223,7 @@ enum vmcs_field {
> #define VECTORING_INFO_VALID_MASK       	INTR_INFO_VALID_MASK
> 
> #define INTR_TYPE_EXT_INTR              (0 << 8) /* external 
>interrupt */
>+#define INTR_TYPE_NMI                   (2 << 8) /* 
>non-maskable interrupt */
> #define INTR_TYPE_EXCEPTION             (3 << 8) /* processor 
>exception */
> 
> /*
>
>
>---------------------------------------------------------------
>----------
>This SF.net email is sponsored by DB2 Express
>Download DB2 Express C - the FREE version of DB2 express and take
>control of your XML. No limits. Just data. Click to get it now.
>http://sourceforge.net/powerbar/db2/
>_______________________________________________
>kvm-devel mailing list
>kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>https://lists.sourceforge.net/lists/listinfo/kvm-devel
>

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/