Re: [PATCH] Add irqdevice interface + userint implementation

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

Gregory Haskins wrote:
>> The NIC raises an interrupt, we have to interrupt the guest to see if its
>> current IF/cr8 permit interrupt injection, and if not,  we have to keep
>> the interrupt in the irqdevice and request an exit when IF/cr8 permit
>> injection.
>>     
>
> Exactly.  The INTR really just serves to kick the cpu into a VMEXIT so we can evaluate the IF/TPR.  In many cases this may reveal that the CPU might not be able to take the interrupt right then and there, so it will remain queued by the irqdevice model until it can.  If the interrupt cannot be dispatched, we would turn on any relevant EXIT feature (e.g., interrupt-window-exiting) if we dont do this already (I believe this is already in there)
>
>   
>> This means that - >pending() and - >read_vector() have to be called in a
>> critical section, otherwise the pending interrupt might be change
>> between the first and second call, resulting in an injected interrupt to
>> software that is not ready to accept it.  If we replace read_vector by
>> - >ack(this, int vector) we avoid this need (and also save a pointless
>> recalculation of the vector).
>>     
>
> I am a little confused as to what you are proposing.  The current design has pending only returning a boolean if there is something pending, and read_vector actually does the INTA cycle (i.e., returns the vector # and marks it accepted).  I can certainly see how a lack of critical section between pending and read_vector could allow a new interrupt to be injected, but I am not seeing why this is a problem.  If a new higher-priority interrupt comes in between the pending and read_vector, the new one will be returned, but I think this is desirable?
>
> And if we replace read_vector() with ack(), how do we learn of the vector in the first place?  I am guessing that you are thinking that both pending and read_vector return a vector and thus the confusion.  Otherwise, I am missing your point completely :)  Could you elaborate?
>   

That's what I thought, but it doesn't really matter:  what's important 
is that accepting an interrupt has several stages, and that they must 
act as a transaction.

The stages are:
1. determine which interrupt is to be injected (if not, abort)
2. check if IF and tpr allow injection
3. one of
     - inject the interrupt and mark it as in-service
     - request an exit on IF and/or tpr threshold

Simply indicating that an interrupt may be pending is already performed 
by the irq_sink callback.

I think it would be best to put all stages into one callback so that the 
locking is kept simple, but that's just handwaving.  Seeing the code 
would make my comments more concrete.

One way around the issue is to have a single callback, ->inject_irq(), 
that is responsible for all of the above.  It would call vcpu helper 
functions to check IF and do the actual injection or exit request.


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