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
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization
