Re: [PATCH 2/5] KVM: Add irqdevice object

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

Gregory Haskins wrote:
> Hi Avi,  Sorry for the delay.  I have been traveling this week.  See inline...      
>   
>>>> [do we actually need a virtual destructor?]
>>>>     
>>>>         
>>> I believe it is the right thing to do, yes.  The implementation of the 
>>>       
>> irqdevice destructor may be as simple as a kfree(), or could be arbitrarily 
>> complex (don't forget that we will have multiple models..we already have 
>> three: userint, kernint, and lapic.  There may also be i8259 and 
>> i8259_cascaded in the future).
>>     
>>>   
>>>       
>> Yes, but does it need to be a function pointer? IOW, is the point it is
>> called generic code or already irqdevice- specific?
>>     
>
> The code can-be/is irqdevice specific, thus the virtual.  In some cases, it will be as simple as a kfree().  In others, (kernint, for instance), it might need to drop references to the apic/ext devices and do other cleanup (which reminds me that I should look at this to make sure its done right today) ;)
>
>   

I mean, "called from generic code".  e.g. in C++ a destructor need not 
be virtual unless you're destroying the object via a pointer to the base 
class.

>> I meant, __kvm_vcpu_irq_pending is just reading stuff.
>>     
>
> Ah, I see.  I am not 100% sure about this, but I think you can make the same argument here as you can with that "double check locks are broken" article that you sent out.  If I got anything out of that article (it was very interesting, BTW), its that the locks do more than protect critical sections:  They are an implicit memory barrier also.  I am under the impression that we want that behavior here.  I can be convinced otherwise....
>
>   

There's  a whole bunch of memory barriers available (smp__rmb() seems to 
be indicated here, which is a noop on x86 IIRC)

>   
>> I guess it works because no OS is insane enough to page out
>> the IDT or GDT, so the only faults we can get are handled by kvm, not
>> the guest.
>>     
>
> This is my thinking as well.  The conditions that cause an injection failure are probably relatively light-weight w.r.t. the guests execution context.  Like for instance, maybe an NMI comes in during the VMENTRY and causes an immediate VMEXIT (e.g. the guest never made any forward progress, and therefore nothing else (e.g. TPR) has changed)
>
>   

That, or a shadow pagefault.

>> So it seems the correct description is not 'un- ack the interrupt', as we
>> have effectively acked it, but actually queue it pending host- only kvm
>> processing.
>>     
>
> This is exactly what I have done (if I understood what you were saying).  When the injection fails we push the vector to the irq.deferred entry which takes a higher priority in the queue than the backing irqdevice (since it believes the vector is already dispatched).  
>
>   

Okay.  Windows can demand page drivers, but probably not the IDT :)  We 
shall have to live with the knowledge that emulation is incorrect wrt 
taking exceptions while delivering external interrupts to the guest.

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


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