From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] Search the LAPIC's for one that will accept a PIC interrupt. Date: Tue, 22 Jun 2010 14:34:29 +0300 Message-ID: <4C209FC5.40201@redhat.com> References: <1277134180-9806-1-git-send-email-clalance@redhat.com> <20100622081021.GV4689@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Chris Lalancette , kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:12889 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750736Ab0FVLeb (ORCPT ); Tue, 22 Jun 2010 07:34:31 -0400 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5MBYV3o025051 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 22 Jun 2010 07:34:31 -0400 Received: from cleopatra.tlv.redhat.com (cleopatra.tlv.redhat.com [10.35.255.11]) by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o5MBYUvq015773 for ; Tue, 22 Jun 2010 07:34:30 -0400 In-Reply-To: <20100622081021.GV4689@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 >> --- >> 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. 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. 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) -- error compiling committee.c: too many arguments to function