Re: [PATCH 07/12] Rename kvm_apic_accept_pic_intr

[Gleb Natapov]

On Tue, Dec 01, 2009 at 03:36:36PM +0100, Chris Lalancette wrote:
> Call it kvm_apic_in_virtual_wire_mode, which is more
> correct.  Also change it to not only operate properly
> on the boot CPU, but on any CPU.
> 
Currently it is assumed that if kvm_cpu_has_interrupt() returns true
kvm_cpu_get_interrupt() will return valid interrupt. After this change
it may not be the case any more. Suppose there are more then one vcpus
in a virtual wire mode. Pic receives interrupt and multiple cpus try
to inject it simultaneously. They all check that interrupt is pending
by calling kvm_cpu_has_interrupt() and then try to get the vector by
calling kvm_cpu_get_interrupt(). Only one vcpu will get correct value,
others will get spurious interrupt.

--
Gleb.
--
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 07/12] Rename kvm_apic_accept_pic_intr

[Avi Kivity]

On 12/01/2009 04:36 PM, Chris Lalancette wrote:

Call it kvm_apic_in_virtual_wire_mode, which is more
correct.  Also change it to not only operate properly
on the boot CPU, but on any CPU.
   


I'm a little worried about the latter change.  It effectively modifies 
the platform from having the PIC wired to just one cpu to having it 
wired to all CPUs.  Let's leave that for now (but keep the rename).


--
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

[PATCH 07/12] Rename kvm_apic_accept_pic_intr

[Chris Lalancette]

Call it kvm_apic_in_virtual_wire_mode, which is more
correct.  Also change it to not only operate properly
on the boot CPU, but on any CPU.

Signed-off-by: Chris Lalancette 
---
 arch/x86/kvm/i8259.c |2 +-
 arch/x86/kvm/irq.c   |4 ++--
 arch/x86/kvm/lapic.c |   17 -
 arch/x86/kvm/lapic.h |2 +-
 4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 8285f0d..80d4705 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -263,7 +263,7 @@ static void kvm_pic_reset(struct kvm_kpic_state *s)
s->init4 = 0;
 
for (irq = 0; irq < PIC_NUM_PINS/2; irq++) {
-   if (vcpu0 && kvm_apic_accept_pic_intr(vcpu0))
+   if (vcpu0 && kvm_apic_in_virtual_wire_mode(vcpu0))
if (irr & (1 << irq) || isr & (1 << irq)) {
pic_clear_isr(s, irq);
}
diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
index 96dfbb6..05721fc 100644
--- a/arch/x86/kvm/irq.c
+++ b/arch/x86/kvm/irq.c
@@ -53,7 +53,7 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v)
return v->arch.interrupt.pending;
 
if (kvm_apic_has_interrupt(v) == -1) {  /* LAPIC */
-   if (kvm_apic_accept_pic_intr(v)) {
+   if (kvm_apic_in_virtual_wire_mode(v)) {
s = pic_irqchip(v->kvm);/* PIC */
return s->output;
} else
@@ -76,7 +76,7 @@ int kvm_cpu_get_interrupt(struct kvm_vcpu *v)
 
vector = kvm_get_apic_interrupt(v); /* APIC */
if (vector == -1) {
-   if (kvm_apic_accept_pic_intr(v)) {
+   if (kvm_apic_in_virtual_wire_mode(v)) {
s = pic_irqchip(v->kvm);
s->output = 0;  /* PIC */
vector = kvm_pic_read_irq(v->kvm);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index cd60c0b..44acf7c 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -435,7 +435,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
delivery_mode,
/*
 * Should only be called by kvm_apic_local_deliver() with LVT0,
 * before NMI watchdog was enabled. Already handled by
-* kvm_apic_accept_pic_intr().
+* kvm_apic_in_virtual_wire_mode().
 */
break;
 
@@ -1099,18 +1099,17 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
return highest_irr;
 }
 
-int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu)
+int kvm_apic_in_virtual_wire_mode(struct kvm_vcpu *vcpu)
 {
u32 lvt0 = apic_get_reg(vcpu->arch.apic, APIC_LVT0);
int r = 0;
 
-   if (kvm_vcpu_is_bsp(vcpu)) {
-   if (!apic_hw_enabled(vcpu->arch.apic))
-   r = 1;
-   if ((lvt0 & APIC_LVT_MASKED) == 0 &&
-   GET_APIC_DELIVERY_MODE(lvt0) == APIC_MODE_EXTINT)
-   r = 1;
-   }
+   if (!apic_hw_enabled(vcpu->arch.apic))
+   r = 1;
+   if ((lvt0 & APIC_LVT_MASKED) == 0 &&
+   GET_APIC_DELIVERY_MODE(lvt0) == APIC_MODE_EXTINT)
+   r = 1;
+
return r;
 }
 
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 40010b0..ce4cd2d 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -22,7 +22,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu);
 void kvm_free_lapic(struct kvm_vcpu *vcpu);
 
 int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu);
-int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu);
+int kvm_apic_in_virtual_wire_mode(struct kvm_vcpu *vcpu);
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu);
 void kvm_lapic_reset(struct kvm_vcpu *vcpu);
 u64 kvm_lapic_get_cr8(struct kvm_vcpu *vcpu);
-- 
1.6.5.2

--
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