Re: [RFC, PATCH 5/5] Paravirt_ops export.patch

[Zachary Amsden <zach@vmware.com>]

Jeremy Fitzhardinge wrote:
> Zachary Amsden wrote:
>   
>> Doing nothing is a BUG, even before this change.  If you can't patch
>> in a properly virtualizable substitute for a non-virtualizable
>> sequence, the kernel will not work.  The only way for the patching to
>> fail is for lack of space or failure to meet clobber constraints, both
>> of which would be fatal even without the patching.
>>     
>
> Could you give a specific example?  Because the intent is that if you do
> nothing (ie, don't apply patching at all), then you'll just end up with
> indirect calls which will be a bit expensive but completely functional. 
> The idea is that every time there's a paravirt call, it must be
> surrounded with push/pops to make the C call compatible with the
> callsite's register usage.  Or are you talking about something else?
>   

There are two specific examples, both of which are real BUGs.  If there 
is not enough space to do generic patching to a direct function call, 
then it is a bug, since there would not have even been enough space to 
do an indirect call in the first place.  Obviously, with the i386 ISA, 
this is impossible as long as there was an indirect in the first place.  
But an attempt to place native "cli" inline with no padding, to use as a 
patch point, would fire this BUG and rightly so.

The second BUG I added is also justified.  If the patch point has a 
smaller than normal clobber list, then the original indirect call is 
incorrect unless the code has taken care to save and restore clobbered 
registers around the patch point.  That is the case today, but there is 
nothing validating that in the patching code.

We could make the patching code more intelligent and have it emit the 
proper push / pop sequence around the call, but it is difficult to do 
and does not work for more than 3 argument function calls.  Or we could 
record the EIP of the call instruction in the patch information, 
allowing the patcher to either overwrite the whole thing, or do indirect 
/ direct conversion.

asm volatile("
patch_begin:
    push %edx
    push %ecx
 patch_call:
    call *(paravirt_ops+op*4)
   pop %ecx
   pop %edx
patch_end:
.section .paravirt_patch
.long patch_begin
.long patch_end
.long patch_call
.byte CLBR_ANY
" :: "eax");

Maybe that is too complicated for just this special case.

>> 2) You must support dynamic re-linking - the kernel has to boot and
>> use builtin native style operations before switching over to the
>> virtualized operations.  So you have to have some kind of jettisonable
>> early binding support.
>>     
>
> I don't think there's any particular reason we can't do this very early,
> at the same time we currently populate paravirt_ops.  I think the idea
> is that if you do nothing, the calls will all point to the native
> versions, so if you do the late-paravirtualization (do you still do
> that?) then you'll get native ops initially.
>   

Yes, we still do late paravirtualization.  My point is that because of 
that, you can't just link once and be done with it - you must relink the 
paravirt-ops later, which is more complex than a one time pre-execution 
link.

Zach