From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Gregory Haskins" Subject: Re: [PATCH 2/3] KVM: Add irqdevice object Date: Mon, 16 Apr 2007 11:17:58 -0400 Message-ID: <46235B51.BA47.005A.0@novell.com> References: <20070416121832.9824.40317.stgit@ghaskins-t60p.haskins.net> <20070416121936.9824.8889.stgit@ghaskins-t60p.haskins.net> <46238338.8070503@qumranet.com> <462352F1.BA47.005A.0@novell.com> <46238E55.80401@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: "Avi Kivity" Return-path: In-Reply-To: <46238E55.80401-atKUWr5tajBWk0Htik3J/w@public.gmane.org> Content-Disposition: inline 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 >>> On Mon, Apr 16, 2007 at 10:55 AM, in message <46238E55.80401-atKUWr5tajBWk0Htik3J/w@public.gmane.org>, Avi Kivity wrote: > Gregory Haskins wrote: >> >>>> >>>> if (vcpu- >rmode.active) { >>>> inject_rmode_irq(vcpu, irq); >>>> @@ - 1246,7 +1241,7 @@ static void do_interrupt_requests(struct kvm_vcpu >>>> >>> *vcpu, >>> >>>> (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0); >>>> >>>> if (vcpu- >interrupt_window_open && >>>> - vcpu- >irq_summary && >>>> + kvm_irqdevice_pending(&vcpu- >irq_dev, 0) && >>>> !(vmcs_read32(VM_ENTRY_INTR_INFO_FIELD) & INTR_INFO_VALID_MASK)) >>>> >>>> >>> What if an irq is made pending here? >>> >> >> The only race I see is related to what you pointed out previously: A > level- sensitive interrupt could be asserted when pending() is read, and > deasserted when read_vector() is read. Handling the irq == - 1 from > read_vector() should fix the race. Or are you pointing out something else? >> > > That one. > > I think there are probably a few more hiding in there. To reduce the > number of combinations, I'd suggest putting the irq and the inter- vcpu > communication things under the same lock, and to make sure - >pending() > and - >read_vector() are always called in the same critical section (or > even better, to unify them into one function). I think you will see that this has been addressed downstream in my series. Its just not polished up enough to share yet. Probably some time this week, however. Thanks for the review! -Greg ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/