Re: [PATCH 4/4] KVM: Add support for in-kernel LAPIC model

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

Gregory Haskins wrote:
>   
>>> +
>>> +static struct kvm_irqdevice *get_irq_dev(struct kvm_kernint *s)
>>> +{
>>> +	struct kvm_irqdevice *dev;
>>> +
>>> +	if (kvm_lapic_enabled(s- >vcpu))
>>> +		dev = &s- >apic_irq;
>>> +	else
>>> +		dev = s- >ext_irq;
>>> +
>>> +	if (!dev)
>>> +		kvm_crash_guest(s- >vcpu- >kvm);
>>>   
>>>       
>> Can this happen? Doesn't there always have to be an external interrupt 
>> controller when the lapic is disabled?
>>     
>
> Well, if a non-BSP processor calls this, ext_irq could be null.  Its a pathological condition, thus the harsh punishment to the guest.  If its an AP, it *better* be using its LAPIC.  If its not, something is busted. 
>
>
>   

Yes, I agree.

>>>   
>>>       
>> ... for example:
>> #define hrtimer timer_list
>>
>> I'd just drop it for now; it makes the code hard to read. We can re- add 
>> it later.
>>     
>
> By "drop it", I assume you mean drop the abstraction and just use native HRTIMER directly for now?  
>
>   

Yes, we can worry about compatibility when this merged.

>> [I didn't audit the lapic code]
>>
>> Where's vcpu- >cr8 gone?
>>     
>
> I know you requested that this entry remain in the VCPU structure.  However, I couldnt make this work reasonably.  The APIC wants to maintain this value itself so the two can very easily get out of sync.  I suppose there could be ways to make the APIC use the vcpu->cr8 variable as storage for TPR (albeit messy), but this idea falls apart when we start looking at optimizations like TPR shadowing.
>
> Based on all that, I felt it was best to just maintain CR8 as the TPR register in the model.
>
>   

I don't understand.  Isn't the tpr read-only from the point of view of 
the lapic?

A simple set_cr8 helper should do the trick.


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