On Mon, Jun 25, 2012 at 06:50:06PM +0300, Avi Kivity wrote:
> >> >> Later we can extend x86_decode_insn() and the other
> >> >> functions to follow the same rule.
> >> >>
> >> > What rule? We cannot not initialize a context. You can reduce things
> >> > that should be initialized to minimum (getting GP registers on demand,
> >> > etc), but still some initialization is needed since ctxt holds emulation
> >> > state and it needs to be reset before each emulation.
> >>
> >> An alternative is to use two contexts, the base context only holds ops
> >> and is the parameter to all the callbacks on the non-state APIs, the
> >> derived context holds the state:
> >>
> >> struct x86_emulation_ctxt {
> >> struct x86_ops *ops;
> >> /* state that always needs to be initialized, preferablt none */
> >> };
> >>
> >> struct x86_insn_ctxt {
> >> struct x86_emulation_ctxt em;
> >> /* instruction state */
> >> }
> >>
> >> and so we have a compile-time split between users of the state and
> >> non-users.
> >>
> > I do not understand how you will divide current ctxt structure between
> > those two.
> >
> > Where will you put those for instance: interruptibility, have_exception,
> > perm_ok, only_vendor_specific_insn and how can they not be initialized
> > before each instruction emulation?
>
> x86_emulate_ops::get_interruptibility()
> x86_emulate_ops::set_interruptibility()
> x86_emulate_ops::exception()
>
They do not remove the need for initialization before instruction
execution, they just move things that need to be initialized somewhere
else (to kvm_arch_vcpu likely).
> x86_decode_insn(struct x86_insn_ctxt *ctxt, unsigned flags)
> {
> ctxt->flags = flags;
> ctxt->perm_ok = false;
> }
>
> In short, instruction emulation state is only seen by instruction
> emulation functions, the others don't get to see it.
>
So you want to divide emulator.c to two types of function: those without
side effect, that do some kind of calculations on vcpu state according
to weird x86 rules, and those that change vcpu state and write it back
eventually. I do not see the justification for that complication really.
emulator.c is complicated enough already and the line between two may be
blurred.
If you dislike linearize() callback so much I can make
kvm_linearize_address() to do calculation base on its parameters only.
It is almost there, only cpl and seg base/desc are missing from
parameter list. I can put it into header and x86.c/emulator.c will both
be able to use it.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
