Jeremy Fitzhardinge wrote:
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.
Yes, we still do that, but..
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.
All of the patch sites can make C function calls, perhaps with fewer
clobbers. If the implementation cannot fit a proper save / restore and
call to the C function into the allotted space, it still is a BUG(). If
it has unique clobber requirements which go beyond the patch site
clobbers, surely it can't use the generic patch code for those
operations. But it still must have the space.
Zach
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization
