Re: [PATCH] Add irqdevice interface + userint implementation

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Gregory Haskins wrote:
>   
>> 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?
>   

My suggestion is basically to give up on having well separated layers of 
interrupts and vcpu but hack as necessary.  While this may sound 
heretical, it actually follows the hardware.  Note how cr8 (a cpu 
register) is aliased to the tpr (an irqdevice entity), and how VT and 
SVM (which are supposed to virtualize the cpu) actually provide some 
apic emulation capabilities.

My point is that the hardware is a tangled mess, and any attempt to 
pretend otherwise in software will lead to a brittle model that breaks 
often/is unnatural/is clumsy (like the switch above).

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
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