On Tue, Jun 22, 2010 at 02:34:29PM +0300, Avi Kivity wrote:
> On 06/22/2010 11:10 AM, Gleb Natapov wrote:
> >On Mon, Jun 21, 2010 at 11:29:40AM -0400, Chris Lalancette wrote:
> >>Older versions of 32-bit linux have a "Checking 'hlt' instruction"
> >>test where they repeatedly call the 'hlt' instruction, and then
> >>expect a timer interrupt to kick the CPU out of halt. This happens
> >>before any LAPIC or IOAPIC setup happens, which means that all of
> >>the APIC's are in virtual wire mode at this point. Unfortunately,
> >>the current implementation of virtual wire mode is hardcoded to
> >>only kick the BSP, so if a crash+kexec occurs on a different
> >>vcpu, it will never get kicked.
> >>
> >>This patch makes pic_unlock() do the equivalent of
> >>kvm_irq_delivery_to_apic() for the IOAPIC code. That is, it runs
> >>through all of the vcpus looking for one that is in virtual wire
> >>mode. In the normal case where LAPICs and IOAPICs are configured,
> >>this won't be used at all. In the bootstrap phase of a modern
> >>OS, before the LAPICs and IOAPICs are configured, this will have
> >>exactly the same behavior as today; VCPU0 is always looked at
> >>first, so it will always get out of the loop after the first
> >>iteration. This will only go through the loop more than once
> >>during a kexec/kdump, in which case it will only do it a few times
> >>until the kexec'ed kernel programs the LAPIC and IOAPIC.
> >>
> >>Signed-off-by: Chris Lalancette<clalance@xxxxxxxxxx>
> >>---
> >> arch/x86/kvm/i8259.c | 17 +++++++++++++----
> >> 1 files changed, 13 insertions(+), 4 deletions(-)
> >>
> >>diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> >>index 2c73f44..85ecabc 100644
> >>--- a/arch/x86/kvm/i8259.c
> >>+++ b/arch/x86/kvm/i8259.c
> >>@@ -44,16 +44,25 @@ static void pic_unlock(struct kvm_pic *s)
> >> __releases(&s->lock)
> >> {
> >> bool wakeup = s->wakeup_needed;
> >>- struct kvm_vcpu *vcpu;
> >>+ struct kvm_vcpu *vcpu, *found = NULL;
> >>+ int i;
> >>
> >> s->wakeup_needed = false;
> >>
> >> raw_spin_unlock(&s->lock);
> >>
> >> if (wakeup) {
> >>- vcpu = s->kvm->bsp_vcpu;
> >>- if (vcpu)
> >>- kvm_vcpu_kick(vcpu);
> >>+ kvm_for_each_vcpu(i, vcpu, s->kvm) {
> >>+ if (kvm_apic_accept_pic_intr(vcpu)) {
> >>+ found = vcpu;
> >>+ break;
> >>+ }
> >>+ }
> >Shouldn't we kick all vcpus that are in virtual write mode, not just
> >first one found?
>
> If two lapics are in ExtInt mode, both will perform the IntAck cycle
> and the PIC might get confused. I don't think it's a valid
> configuration. So I think the patch is fine.
>
May be, interesting what would happen on real HW. How kdump kernel knows
that other cpu's lapics configured correctly?
> There's a slight issue in that if an interrupt happens while a vcpu
> is turning off LVT0.ExtInt, the interrupt gets lost. But this is
> better than what we have now.
>
We can check pic output after LVT0.ExtInt is configured.
> btw, I think virtual wire refers to:
>
> pic -> ioapic(ExtInt) -> (apic bus) -> lapic
>
> (virtual wire since the interrupt is passed over the apic bus, not a
> real wire)
>
> while our configuration is
>
> pic -> lint0 -> lapic lvt0 (ExtInt)
>
I saw both referred as virtual wire, may be erroneous.
How is the mode where lapic is disabled and pic interrupts are delivered
directly to cpu is called?
--
Gleb.
--
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
