Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-29 Thread Gleb Natapov 
On Tue, Jul 27, 2010 at 04:19:35PM +0300, Avi Kivity wrote:
 Instead of blindly attempting to inject an event before each guest entry,
 check for a possible event first in vcpu-requests.  Sites that can trigger
 event injection are modified to set KVM_REQ_EVENT:
 
 - interrupt, nmi window opening
 - ppr updates
 - i8259 output changes
 - local apic irr changes
 - rflags updates
 - gif flag set
 - event set on exit
 
What about userspace irq chip? Does it work with this patch? I don't see
that you set KVM_REQ_EVENT on ioctl(KVM_INTERRUPT) for instance and
vcpu-run-request_interrupt_window should be probably checked out of
if (KVM_REQ_EVEN). It looks like with this approach we scatter irq
injection logic all over the code instead of having it in one place.

 This improves non-injecting entry performance, and sets the stage for
 non-atomic injection.
 
 Signed-off-by: Avi Kivity a...@redhat.com
 ---
  arch/x86/kvm/i8259.c |1 +
  arch/x86/kvm/lapic.c |   12 ++--
  arch/x86/kvm/svm.c   |8 +++-
  arch/x86/kvm/vmx.c   |6 ++
  arch/x86/kvm/x86.c   |   35 ++-
  include/linux/kvm_host.h |1 +
  6 files changed, 51 insertions(+), 12 deletions(-)
 
 diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
 index 8d10c06..9f7ab44 100644
 --- a/arch/x86/kvm/i8259.c
 +++ b/arch/x86/kvm/i8259.c
 @@ -64,6 +64,7 @@ static void pic_unlock(struct kvm_pic *s)
   if (!found)
   found = s-kvm-bsp_vcpu;
  
 + kvm_make_request(KVM_REQ_EVENT, found);
   kvm_vcpu_kick(found);
   }
  }
 diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
 index 77d8c0f..e83d203 100644
 --- a/arch/x86/kvm/lapic.c
 +++ b/arch/x86/kvm/lapic.c
 @@ -259,9 +259,10 @@ static inline int apic_find_highest_isr(struct kvm_lapic 
 *apic)
  
  static void apic_update_ppr(struct kvm_lapic *apic)
  {
 - u32 tpr, isrv, ppr;
 + u32 tpr, isrv, ppr, old_ppr;
   int isr;
  
 + old_ppr = apic_get_reg(apic, APIC_PROCPRI);
   tpr = apic_get_reg(apic, APIC_TASKPRI);
   isr = apic_find_highest_isr(apic);
   isrv = (isr != -1) ? isr : 0;
 @@ -274,7 +275,10 @@ static void apic_update_ppr(struct kvm_lapic *apic)
   apic_debug(vlapic %p, ppr 0x%x, isr 0x%x, isrv 0x%x,
  apic, ppr, isr, isrv);
  
 - apic_set_reg(apic, APIC_PROCPRI, ppr);
 + if (old_ppr != ppr) {
 + apic_set_reg(apic, APIC_PROCPRI, ppr);
 + kvm_make_request(KVM_REQ_EVENT, apic-vcpu);
 + }
  }
  
  static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
 @@ -391,6 +395,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
 delivery_mode,
   break;
   }
  
 + kvm_make_request(KVM_REQ_EVENT, vcpu);
   kvm_vcpu_kick(vcpu);
   break;
  
 @@ -416,6 +421,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
 delivery_mode,
  INIT on a runnable vcpu %d\n,
  vcpu-vcpu_id);
   vcpu-arch.mp_state = KVM_MP_STATE_INIT_RECEIVED;
 + kvm_make_request(KVM_REQ_EVENT, vcpu);
   kvm_vcpu_kick(vcpu);
   } else {
   apic_debug(Ignoring de-assert INIT to vcpu %d\n,
 @@ -430,6 +436,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
 delivery_mode,
   result = 1;
   vcpu-arch.sipi_vector = vector;
   vcpu-arch.mp_state = KVM_MP_STATE_SIPI_RECEIVED;
 + kvm_make_request(KVM_REQ_EVENT, vcpu);
   kvm_vcpu_kick(vcpu);
   }
   break;
 @@ -475,6 +482,7 @@ static void apic_set_eoi(struct kvm_lapic *apic)
   trigger_mode = IOAPIC_EDGE_TRIG;
   if (!(apic_get_reg(apic, APIC_SPIV)  APIC_SPIV_DIRECTED_EOI))
   kvm_ioapic_update_eoi(apic-vcpu-kvm, vector, trigger_mode);
 + kvm_make_request(KVM_REQ_EVENT, apic-vcpu);
  }
  
  static void apic_send_ipi(struct kvm_lapic *apic)
 diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
 index 56c9b6b..a51e067 100644
 --- a/arch/x86/kvm/svm.c
 +++ b/arch/x86/kvm/svm.c
 @@ -2258,6 +2258,7 @@ static int stgi_interception(struct vcpu_svm *svm)
  
   svm-next_rip = kvm_rip_read(svm-vcpu) + 3;
   skip_emulated_instruction(svm-vcpu);
 + kvm_make_request(KVM_REQ_EVENT, svm-vcpu);
  
   enable_gif(svm);
  
 @@ -2644,6 +2645,7 @@ static int interrupt_window_interception(struct 
 vcpu_svm *svm)
  {
   struct kvm_run *kvm_run = svm-vcpu.run;
  
 + kvm_make_request(KVM_REQ_EVENT, svm-vcpu);
   svm_clear_vintr(svm);
   svm-vmcb-control.int_ctl = ~V_IRQ_MASK;
   /*
 @@ -3089,8 +3091,10 @@ static void svm_complete_interrupts(struct vcpu_svm 
 *svm)
  
   svm-int3_injected = 0;
  
 - if (svm-vcpu.arch.hflags  HF_IRET_MASK)
 + if

Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-29 Thread Avi Kivity 

 On 07/28/2010 08:22 PM, Marcelo Tosatti wrote:



that needs reinjection (or an exception).

That can happen event now, no?  A pending exception, interrupt comes
along, injection picks up the exception but leaves the interrupt.

Now the situation can be more complicated:

- pending exception
- injection
- interrupt, sets KVM_REQ_EVENT
- notices KVM_REQ_EVENT
- drops KVM_REQ_EVENT, cancels exception (made pending again)
- goes back
- injection (injects exception again, interrupt is pending)

as far as I can tell, this is all fine.

But you cleared KVM_REQ_EVENT. Which means you're not going to inject
the pending interrupt on the next entry.

Doh.  So we need to set KVM_REQ_EVENT again, after the final check
for vcpu-requests, to make sure we redo injection again.

So we can make inject_pending_event() return true if there's more in
the queue, and if it did, re-raise KVM_REQ_EVENT just before entry?

Yeah, that would do it.


On second and third thoughts, that is unneeded.  If an interrupt or nmi 
is still pending after event injection, we will request an interrupt or 
nmi window which will set KVM_REQ_EVENT.  An exception cannot be pending 
after an event injection since it is the highest priority event.


Yes?

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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-29 Thread Avi Kivity 

 On 07/29/2010 09:51 AM, Gleb Natapov wrote:

On Tue, Jul 27, 2010 at 04:19:35PM +0300, Avi Kivity wrote:

Instead of blindly attempting to inject an event before each guest entry,
check for a possible event first in vcpu-requests.  Sites that can trigger
event injection are modified to set KVM_REQ_EVENT:

- interrupt, nmi window opening
- ppr updates
- i8259 output changes
- local apic irr changes
- rflags updates
- gif flag set
- event set on exit


What about userspace irq chip? Does it work with this patch? I don't see
that you set KVM_REQ_EVENT on ioctl(KVM_INTERRUPT) for instance and
vcpu-run-request_interrupt_window should be probably checked out of
if (KVM_REQ_EVEN).


Right.


It looks like with this approach we scatter irq
injection logic all over the code instead of having it in one place.


One place is better, but it means we have to poll all event types on 
every entry.  We can go back to one place by having a mini-API for 
events (extending the kvm_queue_exception family) that would take care 
of the details.


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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-29 Thread Marcelo Tosatti 
On Thu, Jul 29, 2010 at 11:49:31AM +0300, Avi Kivity wrote:
  On 07/28/2010 08:22 PM, Marcelo Tosatti wrote:
 
 that needs reinjection (or an exception).
 That can happen event now, no?  A pending exception, interrupt comes
 along, injection picks up the exception but leaves the interrupt.
 
 Now the situation can be more complicated:
 
 - pending exception
 - injection
 - interrupt, sets KVM_REQ_EVENT
 - notices KVM_REQ_EVENT
 - drops KVM_REQ_EVENT, cancels exception (made pending again)
 - goes back
 - injection (injects exception again, interrupt is pending)
 
 as far as I can tell, this is all fine.
 But you cleared KVM_REQ_EVENT. Which means you're not going to inject
 the pending interrupt on the next entry.
 Doh.  So we need to set KVM_REQ_EVENT again, after the final check
 for vcpu-requests, to make sure we redo injection again.
 
 So we can make inject_pending_event() return true if there's more in
 the queue, and if it did, re-raise KVM_REQ_EVENT just before entry?
 Yeah, that would do it.
 
 On second and third thoughts, that is unneeded.  If an interrupt or
 nmi is still pending after event injection, we will request an
 interrupt or nmi window which will set KVM_REQ_EVENT.  An exception
 cannot be pending after an event injection since it is the highest
 priority event.
 
 Yes?

Yep. Userspace irqchip is still broken though. Can't see whats wrong
with svm.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-28 Thread Marcelo Tosatti 
On Tue, Jul 27, 2010 at 04:19:35PM +0300, Avi Kivity wrote:
 Instead of blindly attempting to inject an event before each guest entry,
 check for a possible event first in vcpu-requests.  Sites that can trigger
 event injection are modified to set KVM_REQ_EVENT:
 
 - interrupt, nmi window opening
 - ppr updates
 - i8259 output changes
 - local apic irr changes
 - rflags updates
 - gif flag set
 - event set on exit
 
 This improves non-injecting entry performance, and sets the stage for
 non-atomic injection.
 
 Signed-off-by: Avi Kivity a...@redhat.com
 ---
  arch/x86/kvm/i8259.c |1 +
  arch/x86/kvm/lapic.c |   12 ++--
  arch/x86/kvm/svm.c   |8 +++-
  arch/x86/kvm/vmx.c   |6 ++
  arch/x86/kvm/x86.c   |   35 ++-
  include/linux/kvm_host.h |1 +
  6 files changed, 51 insertions(+), 12 deletions(-)
 

 @@ -4731,17 +4737,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
   goto out;
   }
  
 - inject_pending_event(vcpu);
 + if (kvm_check_request(KVM_REQ_EVENT, vcpu)) {
 + inject_pending_event(vcpu);
  
 - /* enable NMI/IRQ window open exits if needed */
 - if (vcpu-arch.nmi_pending)
 - kvm_x86_ops-enable_nmi_window(vcpu);
 - else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
 - kvm_x86_ops-enable_irq_window(vcpu);
 + /* enable NMI/IRQ window open exits if needed */
 + if (vcpu-arch.nmi_pending)
 + kvm_x86_ops-enable_nmi_window(vcpu);
 + else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
 + kvm_x86_ops-enable_irq_window(vcpu);

Problem is it might not be possible to inject the event signalled by
KVM_REQ_EVENT, say an interrupt from an irqchip, if there is an event
that needs reinjection (or an exception).

Perhaps moving atomic_set(vcpu-guest_mode, 1) up to preemptible
section is safe, because kvm_vcpu_kick can only IPI stale vcpu-cpu
while preemption is enabled. In that case, it will hit

if (!atomic_read(vcpu-guest_mode)

later.

Although the KVM_REQ_EVENT idea is nice. Can you think of a way 
to fix the issue?
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-28 Thread Avi Kivity 

 On 07/28/2010 07:21 PM, Marcelo Tosatti wrote:

On Tue, Jul 27, 2010 at 04:19:35PM +0300, Avi Kivity wrote:

Instead of blindly attempting to inject an event before each guest entry,
check for a possible event first in vcpu-requests.  Sites that can trigger
event injection are modified to set KVM_REQ_EVENT:

- interrupt, nmi window opening
- ppr updates
- i8259 output changes
- local apic irr changes
- rflags updates
- gif flag set
- event set on exit

This improves non-injecting entry performance, and sets the stage for
non-atomic injection.

Signed-off-by: Avi Kivitya...@redhat.com
---
  arch/x86/kvm/i8259.c |1 +
  arch/x86/kvm/lapic.c |   12 ++--
  arch/x86/kvm/svm.c   |8 +++-
  arch/x86/kvm/vmx.c   |6 ++
  arch/x86/kvm/x86.c   |   35 ++-
  include/linux/kvm_host.h |1 +
  6 files changed, 51 insertions(+), 12 deletions(-)

@@ -4731,17 +4737,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
goto out;
}

-   inject_pending_event(vcpu);
+   if (kvm_check_request(KVM_REQ_EVENT, vcpu)) {
+   inject_pending_event(vcpu);

-   /* enable NMI/IRQ window open exits if needed */
-   if (vcpu-arch.nmi_pending)
-   kvm_x86_ops-enable_nmi_window(vcpu);
-   else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
-   kvm_x86_ops-enable_irq_window(vcpu);
+   /* enable NMI/IRQ window open exits if needed */
+   if (vcpu-arch.nmi_pending)
+   kvm_x86_ops-enable_nmi_window(vcpu);
+   else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
+   kvm_x86_ops-enable_irq_window(vcpu);

Problem is it might not be possible to inject the event signalled by
KVM_REQ_EVENT, say an interrupt from an irqchip, if there is an event
that needs reinjection (or an exception).


That can happen event now, no?  A pending exception, interrupt comes 
along, injection picks up the exception but leaves the interrupt.


Now the situation can be more complicated:

- pending exception
- injection
- interrupt, sets KVM_REQ_EVENT
- notices KVM_REQ_EVENT
- drops KVM_REQ_EVENT, cancels exception (made pending again)
- goes back
- injection (injects exception again, interrupt is pending)

as far as I can tell, this is all fine.


Perhaps moving atomic_set(vcpu-guest_mode, 1) up to preemptible
section is safe, because kvm_vcpu_kick can only IPI stale vcpu-cpu
while preemption is enabled. In that case, it will hit

if (!atomic_read(vcpu-guest_mode)

later.



I don't really follow.


Although the KVM_REQ_EVENT idea is nice. Can you think of a way
to fix the issue?




--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-28 Thread Marcelo Tosatti 
On Wed, Jul 28, 2010 at 07:31:03PM +0300, Avi Kivity wrote:
  On 07/28/2010 07:21 PM, Marcelo Tosatti wrote:
 On Tue, Jul 27, 2010 at 04:19:35PM +0300, Avi Kivity wrote:
 Instead of blindly attempting to inject an event before each guest entry,
 check for a possible event first in vcpu-requests.  Sites that can trigger
 event injection are modified to set KVM_REQ_EVENT:
 
 - interrupt, nmi window opening
 - ppr updates
 - i8259 output changes
 - local apic irr changes
 - rflags updates
 - gif flag set
 - event set on exit
 
 This improves non-injecting entry performance, and sets the stage for
 non-atomic injection.
 
 Signed-off-by: Avi Kivitya...@redhat.com
 ---
   arch/x86/kvm/i8259.c |1 +
   arch/x86/kvm/lapic.c |   12 ++--
   arch/x86/kvm/svm.c   |8 +++-
   arch/x86/kvm/vmx.c   |6 ++
   arch/x86/kvm/x86.c   |   35 ++-
   include/linux/kvm_host.h |1 +
   6 files changed, 51 insertions(+), 12 deletions(-)
 
 @@ -4731,17 +4737,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 goto out;
 }
 
 -   inject_pending_event(vcpu);
 +   if (kvm_check_request(KVM_REQ_EVENT, vcpu)) {
 +   inject_pending_event(vcpu);
 
 -   /* enable NMI/IRQ window open exits if needed */
 -   if (vcpu-arch.nmi_pending)
 -   kvm_x86_ops-enable_nmi_window(vcpu);
 -   else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
 -   kvm_x86_ops-enable_irq_window(vcpu);
 +   /* enable NMI/IRQ window open exits if needed */
 +   if (vcpu-arch.nmi_pending)
 +   kvm_x86_ops-enable_nmi_window(vcpu);
 +   else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
 +   kvm_x86_ops-enable_irq_window(vcpu);
 Problem is it might not be possible to inject the event signalled by
 KVM_REQ_EVENT, say an interrupt from an irqchip, if there is an event
 that needs reinjection (or an exception).
 
 That can happen event now, no?  A pending exception, interrupt comes
 along, injection picks up the exception but leaves the interrupt.
 
 Now the situation can be more complicated:
 
 - pending exception
 - injection
 - interrupt, sets KVM_REQ_EVENT
 - notices KVM_REQ_EVENT
 - drops KVM_REQ_EVENT, cancels exception (made pending again)
 - goes back
 - injection (injects exception again, interrupt is pending)
 
 as far as I can tell, this is all fine.

But you cleared KVM_REQ_EVENT. Which means you're not going to inject
the pending interrupt on the next entry.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-28 Thread Avi Kivity 

 On 07/28/2010 07:37 PM, Marcelo Tosatti wrote:

On Wed, Jul 28, 2010 at 07:31:03PM +0300, Avi Kivity wrote:

  On 07/28/2010 07:21 PM, Marcelo Tosatti wrote:

On Tue, Jul 27, 2010 at 04:19:35PM +0300, Avi Kivity wrote:

Instead of blindly attempting to inject an event before each guest entry,
check for a possible event first in vcpu-requests.  Sites that can trigger
event injection are modified to set KVM_REQ_EVENT:

- interrupt, nmi window opening
- ppr updates
- i8259 output changes
- local apic irr changes
- rflags updates
- gif flag set
- event set on exit

This improves non-injecting entry performance, and sets the stage for
non-atomic injection.

Signed-off-by: Avi Kivitya...@redhat.com
---
  arch/x86/kvm/i8259.c |1 +
  arch/x86/kvm/lapic.c |   12 ++--
  arch/x86/kvm/svm.c   |8 +++-
  arch/x86/kvm/vmx.c   |6 ++
  arch/x86/kvm/x86.c   |   35 ++-
  include/linux/kvm_host.h |1 +
  6 files changed, 51 insertions(+), 12 deletions(-)

@@ -4731,17 +4737,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
goto out;
}

-   inject_pending_event(vcpu);
+   if (kvm_check_request(KVM_REQ_EVENT, vcpu)) {
+   inject_pending_event(vcpu);

-   /* enable NMI/IRQ window open exits if needed */
-   if (vcpu-arch.nmi_pending)
-   kvm_x86_ops-enable_nmi_window(vcpu);
-   else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
-   kvm_x86_ops-enable_irq_window(vcpu);
+   /* enable NMI/IRQ window open exits if needed */
+   if (vcpu-arch.nmi_pending)
+   kvm_x86_ops-enable_nmi_window(vcpu);
+   else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
+   kvm_x86_ops-enable_irq_window(vcpu);

Problem is it might not be possible to inject the event signalled by
KVM_REQ_EVENT, say an interrupt from an irqchip, if there is an event
that needs reinjection (or an exception).

That can happen event now, no?  A pending exception, interrupt comes
along, injection picks up the exception but leaves the interrupt.

Now the situation can be more complicated:

- pending exception
- injection
- interrupt, sets KVM_REQ_EVENT
- notices KVM_REQ_EVENT
- drops KVM_REQ_EVENT, cancels exception (made pending again)
- goes back
- injection (injects exception again, interrupt is pending)

as far as I can tell, this is all fine.

But you cleared KVM_REQ_EVENT. Which means you're not going to inject
the pending interrupt on the next entry.


Doh.  So we need to set KVM_REQ_EVENT again, after the final check for 
vcpu-requests, to make sure we redo injection again.


So we can make inject_pending_event() return true if there's more in the 
queue, and if it did, re-raise KVM_REQ_EVENT just before entry?


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-28 Thread Marcelo Tosatti 
On Wed, Jul 28, 2010 at 07:53:32PM +0300, Avi Kivity wrote:
  On 07/28/2010 07:37 PM, Marcelo Tosatti wrote:
 On Wed, Jul 28, 2010 at 07:31:03PM +0300, Avi Kivity wrote:
   On 07/28/2010 07:21 PM, Marcelo Tosatti wrote:
 On Tue, Jul 27, 2010 at 04:19:35PM +0300, Avi Kivity wrote:
 Instead of blindly attempting to inject an event before each guest entry,
 check for a possible event first in vcpu-requests.  Sites that can 
 trigger
 event injection are modified to set KVM_REQ_EVENT:
 
 - interrupt, nmi window opening
 - ppr updates
 - i8259 output changes
 - local apic irr changes
 - rflags updates
 - gif flag set
 - event set on exit
 
 This improves non-injecting entry performance, and sets the stage for
 non-atomic injection.
 
 Signed-off-by: Avi Kivitya...@redhat.com
 ---
   arch/x86/kvm/i8259.c |1 +
   arch/x86/kvm/lapic.c |   12 ++--
   arch/x86/kvm/svm.c   |8 +++-
   arch/x86/kvm/vmx.c   |6 ++
   arch/x86/kvm/x86.c   |   35 ++-
   include/linux/kvm_host.h |1 +
   6 files changed, 51 insertions(+), 12 deletions(-)
 
 @@ -4731,17 +4737,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
   goto out;
   }
 
 - inject_pending_event(vcpu);
 + if (kvm_check_request(KVM_REQ_EVENT, vcpu)) {
 + inject_pending_event(vcpu);
 
 - /* enable NMI/IRQ window open exits if needed */
 - if (vcpu-arch.nmi_pending)
 - kvm_x86_ops-enable_nmi_window(vcpu);
 - else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
 - kvm_x86_ops-enable_irq_window(vcpu);
 + /* enable NMI/IRQ window open exits if needed */
 + if (vcpu-arch.nmi_pending)
 + kvm_x86_ops-enable_nmi_window(vcpu);
 + else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
 + kvm_x86_ops-enable_irq_window(vcpu);
 Problem is it might not be possible to inject the event signalled by
 KVM_REQ_EVENT, say an interrupt from an irqchip, if there is an event
 that needs reinjection (or an exception).
 That can happen event now, no?  A pending exception, interrupt comes
 along, injection picks up the exception but leaves the interrupt.
 
 Now the situation can be more complicated:
 
 - pending exception
 - injection
 - interrupt, sets KVM_REQ_EVENT
 - notices KVM_REQ_EVENT
 - drops KVM_REQ_EVENT, cancels exception (made pending again)
 - goes back
 - injection (injects exception again, interrupt is pending)
 
 as far as I can tell, this is all fine.
 But you cleared KVM_REQ_EVENT. Which means you're not going to inject
 the pending interrupt on the next entry.
 
 Doh.  So we need to set KVM_REQ_EVENT again, after the final check
 for vcpu-requests, to make sure we redo injection again.
 
 So we can make inject_pending_event() return true if there's more in
 the queue, and if it did, re-raise KVM_REQ_EVENT just before entry?

Yeah, that would do it.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] KVM: Check for pending events before attempting injection

2010-07-27 Thread Avi Kivity 
Instead of blindly attempting to inject an event before each guest entry,
check for a possible event first in vcpu-requests.  Sites that can trigger
event injection are modified to set KVM_REQ_EVENT:

- interrupt, nmi window opening
- ppr updates
- i8259 output changes
- local apic irr changes
- rflags updates
- gif flag set
- event set on exit

This improves non-injecting entry performance, and sets the stage for
non-atomic injection.

Signed-off-by: Avi Kivity a...@redhat.com
---
 arch/x86/kvm/i8259.c |1 +
 arch/x86/kvm/lapic.c |   12 ++--
 arch/x86/kvm/svm.c   |8 +++-
 arch/x86/kvm/vmx.c   |6 ++
 arch/x86/kvm/x86.c   |   35 ++-
 include/linux/kvm_host.h |1 +
 6 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 8d10c06..9f7ab44 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -64,6 +64,7 @@ static void pic_unlock(struct kvm_pic *s)
if (!found)
found = s-kvm-bsp_vcpu;
 
+   kvm_make_request(KVM_REQ_EVENT, found);
kvm_vcpu_kick(found);
}
 }
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 77d8c0f..e83d203 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -259,9 +259,10 @@ static inline int apic_find_highest_isr(struct kvm_lapic 
*apic)
 
 static void apic_update_ppr(struct kvm_lapic *apic)
 {
-   u32 tpr, isrv, ppr;
+   u32 tpr, isrv, ppr, old_ppr;
int isr;
 
+   old_ppr = apic_get_reg(apic, APIC_PROCPRI);
tpr = apic_get_reg(apic, APIC_TASKPRI);
isr = apic_find_highest_isr(apic);
isrv = (isr != -1) ? isr : 0;
@@ -274,7 +275,10 @@ static void apic_update_ppr(struct kvm_lapic *apic)
apic_debug(vlapic %p, ppr 0x%x, isr 0x%x, isrv 0x%x,
   apic, ppr, isr, isrv);
 
-   apic_set_reg(apic, APIC_PROCPRI, ppr);
+   if (old_ppr != ppr) {
+   apic_set_reg(apic, APIC_PROCPRI, ppr);
+   kvm_make_request(KVM_REQ_EVENT, apic-vcpu);
+   }
 }
 
 static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
@@ -391,6 +395,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
delivery_mode,
break;
}
 
+   kvm_make_request(KVM_REQ_EVENT, vcpu);
kvm_vcpu_kick(vcpu);
break;
 
@@ -416,6 +421,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
delivery_mode,
   INIT on a runnable vcpu %d\n,
   vcpu-vcpu_id);
vcpu-arch.mp_state = KVM_MP_STATE_INIT_RECEIVED;
+   kvm_make_request(KVM_REQ_EVENT, vcpu);
kvm_vcpu_kick(vcpu);
} else {
apic_debug(Ignoring de-assert INIT to vcpu %d\n,
@@ -430,6 +436,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
delivery_mode,
result = 1;
vcpu-arch.sipi_vector = vector;
vcpu-arch.mp_state = KVM_MP_STATE_SIPI_RECEIVED;
+   kvm_make_request(KVM_REQ_EVENT, vcpu);
kvm_vcpu_kick(vcpu);
}
break;
@@ -475,6 +482,7 @@ static void apic_set_eoi(struct kvm_lapic *apic)
trigger_mode = IOAPIC_EDGE_TRIG;
if (!(apic_get_reg(apic, APIC_SPIV)  APIC_SPIV_DIRECTED_EOI))
kvm_ioapic_update_eoi(apic-vcpu-kvm, vector, trigger_mode);
+   kvm_make_request(KVM_REQ_EVENT, apic-vcpu);
 }
 
 static void apic_send_ipi(struct kvm_lapic *apic)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 56c9b6b..a51e067 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2258,6 +2258,7 @@ static int stgi_interception(struct vcpu_svm *svm)
 
svm-next_rip = kvm_rip_read(svm-vcpu) + 3;
skip_emulated_instruction(svm-vcpu);
+   kvm_make_request(KVM_REQ_EVENT, svm-vcpu);
 
enable_gif(svm);
 
@@ -2644,6 +2645,7 @@ static int interrupt_window_interception(struct vcpu_svm 
*svm)
 {
struct kvm_run *kvm_run = svm-vcpu.run;
 
+   kvm_make_request(KVM_REQ_EVENT, svm-vcpu);
svm_clear_vintr(svm);
svm-vmcb-control.int_ctl = ~V_IRQ_MASK;
/*
@@ -3089,8 +3091,10 @@ static void svm_complete_interrupts(struct vcpu_svm *svm)
 
svm-int3_injected = 0;
 
-   if (svm-vcpu.arch.hflags  HF_IRET_MASK)
+   if (svm-vcpu.arch.hflags  HF_IRET_MASK) {
svm-vcpu.arch.hflags = ~(HF_NMI_MASK | HF_IRET_MASK);
+   kvm_make_request(KVM_REQ_EVENT, svm-vcpu);
+   }
 
svm-vcpu.arch.nmi_injected = false;
kvm_clear_exception_queue(svm-vcpu);
@@ -3099,6 +3103,8 @@ static void svm_complete_interrupts(struct vcpu_svm *svm)
if (!(exitintinfo  SVM_EXITINTINFO_VALID))
return;
 
+