From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH] Search the LAPIC's for one that will accept a PIC interrupt. Date: Tue, 22 Jun 2010 14:49:03 +0300 Message-ID: <20100622114903.GW4689@redhat.com> References: <1277134180-9806-1-git-send-email-clalance@redhat.com> <20100622081021.GV4689@redhat.com> <4C209FC5.40201@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Chris Lalancette , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:60614 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753112Ab0FVLtF (ORCPT ); Tue, 22 Jun 2010 07:49:05 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5MBn5M5011521 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 22 Jun 2010 07:49:05 -0400 Content-Disposition: inline In-Reply-To: <4C209FC5.40201@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 > >>--- > >> 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.