Re: [PATCH v10 2/3] x86emul: New return code for unimplemented instruction

[Petre Ovidiu PIRCALABU <ppircalabu@bitdefender.com>]

On Jo, 2017-09-07 at 09:08 -0600, Jan Beulich wrote:
> > 
> > > 
> > > > 
> > > > On 07.09.17 at 16:26, <JBeulich@suse.com> wrote:
> > After discussing with Andrew I'm willing to agree with the changes
> > you do here, with one extra requirement: At least on non-debug
> > builds X86EMUL_UNIMPLEMENTED should always result in #UD being
> > raised by the final consumer of it. It should never, like would be
> > the case with the changes you do to vmx/realmode.c, result in
> > the domain being crashed. Please change that one and check
> > carefully whether there are any other similar cases.

Hi Jan,

Changing the way we handle X86EMUL_UNIMPLEMENTED in some of the
functions will modify the existing behavior, and I'm a little bit wary
of making so many changes unrelated to the current patchset'a purpose
without a thourough way of testing them.

e.g.: "hvm_descriptor_access_intercept".
The current behavior is to return false if X86EMUL_UNHANDLEABLE is
returned by hvm_emulate_one. Up until now, this return code covered
also the "unimplemented instruction" case.
If X86EMUL_UNIMPLEMENTED will be handled separately (e.g. by calling
hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC),
hvm_emulate_writeback, and finally returning true) some of the
scenarios where the domain got crashed will result only in an UD being
injected.

The same reasoning applies also to vmx/realmode. My patch didn't change
the current behavior, the domain crash logic was added by patch
3502a233e0132cb2facfe90c5c4872c823a5cb69.

However, in the end the decision if yours to take. I can add those
changes, but I will require a little help in order to make sure I don't
break anything.

Many thanks,
Petre

> Oh, and please make the comment next to the definition of
> X86EMUL_UNIMPLEMENTED also say so.
> 
> Jan
> 
> 
> ________________________
> This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel