Re: [PATCH 0/5] KVM paravirt_ops implementation

[Anthony Liguori <anthony-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org>]

Jeremy Fitzhardinge wrote:
> Anthony Liguori wrote:
>   
>> Hi Jeremy,
>>
>> Jeremy Fitzhardinge wrote:
>>     
>>> Anthony Liguori wrote:
>>>  
>>>       
>>>> 1) Not really sure what is needed for CONFIG_PREEMPT support.  I'm not
>>>> sure which paravirt_ops calls are actually re-entrant.
>>>>     
>>>>         
>>> I'm not sure that has specifically come up.  The main issue is whether a
>>> particular call can be preempted and whether that matters.  I guess the
>>> calls which affect a particular CPU's state will generally be called in
>>> a non-preemptable context, but I guess we can't assume that; the best
>>> approach is to assume that each call be atomic with respect to
>>> preemption.
>>>   
>>>       
>> So each call would need to disable preemption?  I'm not sure that
>> makes a whole lot of sense for something like CR reads/writes.  In
>> fact, without passing in a cpu parameter, I'm pretty sure that those
>> operations *have* to require preemption to be disabled.
>>     
>
> Yeah, its a little unclear to me.  If you're poking at a control
> register, then one presumes you've got a specific CPU's CRx in mind. 
> But in the Xen code I don't care about the preemption state for control
> register updates - except for write_cr3, which never makes any sense
> with preemption enabled.
>
>   
>> For something like MMU operations, preemption really doesn't have to
>> be disabled.
>>     
>
> Unless you're batching, since the lazy_mode is inherently per-cpu state.
>
>   
>>> Things like batching must be completed with preemption disabled over the
>>> whole batch.  I check that with BUG_ON in the Xen code.
>>>   
>>>       
>> Right now the KVM batching requires preemption to be disabled for
>> batching.
>>     
>
> I think that's probably overkill.  I had to put a few explicit
> preempt_disable/enables in the Xen code, but mostly the preempt state is
> reasonable for a given operation (ie, disabled for per-cpu state
> updates, enabled for memory/global state updates).
>
>   
>>   I don't think that's a hard requirement though since we could pass
>> the current batch PA as part of the flush hypercalls.
>>     
>
> PA?
>
>   
>> Things are a lot easier though if we can just assume preemption is
>> disabled :-)
>>
>> Are you aware of any paravirt_ops calls that are probably being called
>> in the kernel with preemption enabled?
>>     
>
> Erm, I haven't made a breakdown, but many are.  The descriptor updates
> generally are, for example.  Pagetable updates could be, but are
> generally done under a pagetable lock, and so are not preemptible anyway.
>
>   
>> I don't see a compelling reason to paravirtualize earlier although I
>> also don't see a compelling reason not too.  I noticed that VMI hooks
>> setup.c.  It wasn't immediately obvious why it was hooking there but
>> perhaps it worthwhile to have a common hook?  I suspect VMI and KVM
>> will have a similar model for startup.
>>     
>
> Well, I was suggesting we could print the banner later rather than
> forcing an earlier init.
>   

Perhaps we can just print the banner before batching occurs?  Then it's 
being printed at the last possible moment.

Regards,

Anthony Liguori

> The important part is that you set your pv_ops before patching occurs,
> since that will bake the function calls into the rest of the kernel, and
> it will ignore any further changes to the paravirt_ops structure.
>
> I think Zach was originally thinking of initializing VMI much later
> (even as a module load), but the subtleties of inveigling its way into
> the kernel at that late stage got too complex.
>
>     J
>
>   


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