Zachary Amsden wrote:
> Jeremy Fitzhardinge wrote:
>> Zachary Amsden wrote:
>>
>>> Failing to patch because not enough space is available for a call or
>>> jump
>>> or because the site clobbers do not allow the target clobbers to fit is
>>> a fatal error; it means the kernel can not be properly virtualized.
>>>
>>
>> No, that doesn't follow. If the original site was:
>>
>> patchable_start:
>> push %eax
>> push %ecx
>> push %edx
>> call *paravirt_ops + thingy
>> pop
>> pop
>> pop
>> patchable_end:
>>
>> then its perfectly OK to leave it as-is, even if the direct call's
>> destination clobbers are mismatched. If the patcher wants to generate a
>> call to a C function in a context which can't deal with normal C calling
>> conventions, then it needs to also patch in appropriate save/restores.
>>
>
> The example is a bit misconstrued. In this case, the clobbers for the
> patchable region are CLBR_ALL - so there is no possibility of mismatch
> because of expanded clobber list.
In my example, it would be CLBR_NONE, meaning that the surrounding code
expects no registers to be clobbered. We have constructions like this
for paravirt_ops calls in entry.S, in contexts where no clobbers are
acceptable, let alone normal C calls.
> If the patchable region consisted of this, it would be bad:
>
> push %eax
> push %ecx
> patchable_start:
> push %edx
> call *paravirt_ops + thingy
> pop
> patchable_end: (note - site clobber EDX ok)
> pop
> pop
>
>
> But, why would you do that?
We have done that in the past, to give the inlined code free access to
some scratch registers, but without clobbering everything.
> Calls through paravirt_ops function pointers are C function calls.
> Failing to provide a patchable region which can make a C function call
> is a BUG().
That's not the case at the moment.
J
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization
