From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Gregory Haskins" Subject: Re: [PATCH] Add irqdevice interface + userint implementation Date: Tue, 10 Apr 2007 11:39:17 -0400 Message-ID: <461B7755.BA47.005A.0@novell.com> References: <4614FF5C.BA47.005A.0@novell.com> <46189005.2090609@qumranet.com> <461A415C.BA47.005A.0@novell.com> <461B4241.5030101@qumranet.com> <461B3E61.BA47.005A.0@novell.com> <461B7C81.7040102@qumranet.com> <461B64E4.BA47.005A.0@novell.com> <461BA363.5080308@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: <461BA363.5080308-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 Tue, Apr 10, 2007 at 10:46 AM, in message <461BA363.5080308-atKUWr5tajBWk0Htik3J/w@public.gmane.org>, Avi Kivity wrote: > > Both the vcpu and the irqdevice are involved in the decision: > > - the vcpu holds the IF (or rather, the "interrupt window") > - the lapic holds the tpr > - but the vcpu contains the controls that allow exiting on tpr > modification (on 64- bit and with next- gen Intel processors) Agreed > > IIUC, you are suggesting (pseudocode): > > spin_lock(vcpu- >irq_lock) > nmi = !irq_window_open(vcpu) > if (vcpu- >irq- >pending(nmi)) > inject(vcpu, vcpu- >irq- >read_vector(nmi)) > spin_unlock(vcpu- >irq_lock) > Your psuedo-code is spot on. This would also be coupled with the locks inside the INTR callback, of course. > where - >pending() has to program a tpr exit if current tpr masks an > interrupt. I wasn't planning on the irqdevice having this type of control because I assumed we would always have these exit types set, but you have highlighted an interesting point. If we want to selectively enable these different types of exits (and I agree that it's probably a good idea), we need more intelligence from the irqdevice. Now the question is how to handle/detect the different *types* of masking that can occur. My current API is really just designed to return a boolean: true = "there are injectable vectors", false = "there are no injectable vectors (but there may be masked vectors). I don't like the idea of this programming being handled directly by the irqdevice because I think its potentially the wrong layer. However, we do need to account for this somehow. We could change the API to handle the pending condition as a tri-level: no-interrupts, masked-interrupts, injectable-interrupts? For "no" and "injectable", the operation is as your psuedo-code indicates. For the masked case, we could enumerate different classes of reasons (e.g. TPR, etc) that could be used to drive the programming. So the pseudo-code would now be: spin_lock(vcpu- >irq_lock) nmi = !irq_window_open(vcpu) switch (vcpu- >irq- >pending(nmi, &mask_reasons)) { case NONE_PENDING: break; case MASKED_PENDING: if (mask_reasons & MASKREASON_TPR) vcpu->exit_reasons &= TPR; /* etc */ break; case PENDING: inject(vcpu, vcpu- >irq- >read_vector(nmi)) break; } spin_unlock(vcpu- >irq_lock) Thoughts? > > The alternative I'm suggesting is to move all this mess into the > irqdevice code in a single callback. While this has less reuse > potential, in fact the hardware itself is messy and attempts to > generalize it may turn into a mess as well. > I cant quite wrap my head around how I could work out all of the issues as I (think) have done with the current proposal, but I am open to suggestion. Can you elaborate more? >> I will put together a follow- on patch that implements my thinking to help > illustrate the concept. >> > > That will be best. After all, we're not trying to design a generic > interface, we have a know an limited set of users. Seeing the users > will help evaluate the API. Will do. Regards, -Greg ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV