From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [kvm-commits] KVM: Fix ioapic level-triggered interrupt redelivery Date: Wed, 19 Sep 2007 15:41:39 +0200 Message-ID: <46F12713.9030004@qumranet.com> References: <20070918122732.3F11D250D4F@il.qumranet.com> <10EA09EFD8728347A513008B6B0DA77A014E8AF2@pdsmsx411.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel To: "Dong, Eddie" Return-path: In-Reply-To: <10EA09EFD8728347A513008B6B0DA77A014E8AF2-wq7ZOvIWXbNpB2pF5aRoyrfspsVTdybXVpNB7YpNyf8@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Dong, Eddie wrote: >> diff --git a/drivers/kvm/ioapic.c b/drivers/kvm/ioapic.c >> index 3ee13c3..b8c7da4 100644 >> --- a/drivers/kvm/ioapic.c >> +++ b/drivers/kvm/ioapic.c >> @@ -243,17 +243,10 @@ void kvm_ioapic_set_irq(struct >> kvm_ioapic *ioapic, int irq, int level) >> entry = ioapic->redirtbl[irq]; >> if (!level) >> ioapic->irr &= ~mask; >> - if (entry.fields.trig_mode) { /* level triggered */ >> - if (level && !entry.fields.remote_irr) { >> - ioapic->irr |= mask; >> - ioapic_service(ioapic, irq); >> - } >> - } else if (level && !(ioapic->irr & mask)) { >> - /* >> - * edge triggered >> - */ >> + else { >> ioapic->irr |= mask; >> - ioapic_service(ioapic, irq); >> + if (!entry.fields.trig_mode || >> !entry.fields.remote_irr) >> + ioapic_service(ioapic, irq); >> > > Good fix for ioapic->irr to indicate the line state. But > I think this one will cause a redunadant edge trigger IRQ. A device > model may repeat call SET_IRQ_LINE state even w/o state change > (thus no IRQ), with this logic, a redunadnat IRQ will happen. > > if (!entry.fields.trig_mode || =====> > if ((!entry.fields.trig_mode && (old_irr & mask ==0)) || > > Thanks, I'll do that. > Also architectually level = 0 may also mean an IRQ to IOAPIC > if the polarity is negative though today we may not see this. > But this change will expose the risk, and the propose of pass-through > hardware device will change the polarity. > Sure, if you run Xen + pci passthrough with the polarity reversal on kvm :) We do want a correct polarity implementation -- I'll do that later on. I certainly won't say no to patches... -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/