On 06/07/2010 06:35 PM, Chris Lalancette wrote:
+static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
+{
+ int restart_timer = 0;
+ struct kvm_vcpu *vcpu, *this;
+ int i;
+ struct kvm_timer *ktimer = container_of(data, struct kvm_timer, timer);
+
+ if (!ktimer->vcpu)
+ return HRTIMER_NORESTART;
+
+ this = NULL;
+ kvm_for_each_vcpu(i, vcpu, ktimer->kvm) {
+ if (kvm_lapic_enabled(vcpu)) {
+ this = vcpu;
+ break;
+ }
+ }
+ if (this == NULL)
+ this = ktimer->kvm->bsp_vcpu;
If lapics are active, 'this' is chosen as the highest numbered lapic.
Hm, I'm not sure I understand this comment. From my reading
"kvm_for_each_vcpu" starts at vcpu 0 and iterates up from there. So in the
normal case with all LAPICs enabled, we choose vcpu 0 exclusively unless
it's lapic is disabled. What am I missing?
Sorry, I misread the code. It will select vcpu 0.
Maybe we should check whether the bsp's lapic is enabled, and if not,
choose the first enabled vcpu.
Dealing with a related problem, that code is still wrong. It should be
possible to have multiple local APICs enabled for LINT0 delivery (say
with DM_NMI). But one thing at a time, let's start with getting one
thing fixed.
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index 9fd5b3e..c492bae 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
if (r< 0)
r = 0;
r += kvm_apic_set_irq(vcpu, irq);
- } else {
+ } else if (kvm_lapic_enabled(vcpu)){
if (!lowest)
lowest = vcpu;
else if (kvm_apic_compare_prio(vcpu, lowest)< 0)
Unrelated fix?
Actually, no, this is also required for kdump to work (and I should have put
a comment here explaining).
I don't want a patch that fixes kdump. I want a series of patches that
each fix a single problem with the emulated hardware. If the effect of
the set is to fix kdump, that's a pleasant side effect.
So from my POV this hunk fixes a different problem from the
timer-delivery-to-non-bsp problem.
What happens duing a kdump is that the apic of
the crashing cpu (say vcpu 1) is re-written to have the same APIC id as vcpu 0.
Wow, doesn't that confuse the APIC bus on real hardware?
So what happens during this loop is that we still always deliver the interrupt
to vcpu 0 because we are DM_LOWEST mode and it's the first one that matches.
By gating it on kvm_lapic_enabled() we ensure that we skip vcpu 0 and choose
vcpu 1 as the one to deliver to.
Perhaps having the local APIC disabled prevents it from talking on the
APIC bus. Will need to rereread the docs.
--
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
