Re: [PATCH 4/5] KVM: Local-APIC interface cleanup

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

Gregory Haskins wrote:
>>>> On Sun, Apr 22, 2007 at  4:54 AM, in message <462B22AE.4090108-atKUWr5tajBWk0Htik3J/w@public.gmane.org>,
>>>>         
> Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote: 
>   
>> Gregory Haskins wrote:
>>     
>>> Adds an abstraction to the LAPIC logic so that we can later substitute it
>>> for an in- kernel model.
>>>
>>>   
>>>       
>> This is overly abstracted.  It's not like you can (on real hardware) 
>> wire your own lapic and plug it into the processor.  
>>     
>
> Agreed, but the key point is that under KVM, you *can* plug in more that one LAPIC (e.g. userint and kernint).  Notice that I did not try to completely abstract an LAPIC.  Rather, I tried to identify the common touchpoints between the userspace and kernel version. In short, that basically came down to CR8/TPR and the APIC_BASE_MSR handling.  The other functions of the APIC (e.g. kvm_apicbus_send()) which were not common between the two I simply defined as non-virtuals.  I figured that my minimalist abstraction was preferable to doing this all over the place:
>
>  if (!vcpu->kvm->enable_kernel_pic)
>       vcpu->cr8 = cr8;
>  else
>      apic_set_tpr(cr8)
>
> Instead, you can just do:
>
>   kvm_lapic_set_tpr(&vcpu->apic, cr8)
>   

You can put the if statement into kvm_lapic_set_tpr().

And please keep cr8 in the vcpu, people may want to read it.

> and let the model figure out the right action.  This is easily reversible if you prefer.  I just figured mine was a cleaner way of accomplishing the same thing.  Perhaps I am a bit overzealous ;)
>  
>   
>> The differentiation can be made by installing or not installing the mmio 
>> handler and the irqdevice stuff.
>>     
>
> Well, it's actually a bit more complicated than that.  Per my previous example, handing TPR is simple in the userspace case (just save it as part of the VCPU state), where its complex in the in-kernel state (if lowering TPR unmasks pending vectors, inject them).  However, the core code doesn't have to care.  It simply notes that TPR changed and the model handles the rest.
>   

#include "diatribes/complexity.h"

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


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