Re: [PATCH 1/3] x86: add support for computing the instruction length

["Mihai Donțu" <mdontu@bitdefender.com>]

On Tuesday 09 September 2014 18:46:30 Mihai Donțu wrote:
> On Tuesday 09 September 2014 19:13:26 Masami Hiramatsu wrote:
> > Hello,
> > 
> > (2014/09/09 18:44), Mihai Donțu wrote:
> > > On Tue, 9 Sep 2014 11:47:05 +0300 Mihai Donțu wrote:
> > >> On Tuesday 09 September 2014 05:28:02 Mihai Donțu wrote:
> > >>> This patch adds support for computing the length of an instruction. The entire
> > >>> logic relies on a set of opcode tables generated by gen-insn-attr-x86.awk from
> > >>> x86-opcode-map.txt and a number of small helper functions. It originated in
> > >>> Linux, where it was added by Masami Hiramatsu. Since it's an almost identical
> > >>> copy, it's separated from the x86 emulator, simplifying future updates.
> > >>>
> > >>> ---
> > >>> Changed since v1:
> > >>>   * adjusted the coding style to match the rest of xen
> > >>>   * moved the source files into x86/x86_emulate
> > >>>   * replaced inat-tables.c with x86-opcode-map.txt and gen-insn-attr-x86.awk
> > >>>     that are used to generate it
> > >>>
> > >>> Signed-off-by: Mihai Donțu <mdontu@bitdefender.com>
> > >>
> > >> There are a couple of design issues with this code which Jan pointed
> > >> out and which I overlooked. I'm sorry, it was not intentional. I'll do
> > >> my best to address them in a following email.
> > > 
> > > I'd like to give it a try at saving some time by asking the author
> > > behind the Linux code what is his opinion on this.
> > > 
> > > Masami, I'm trying to bring to xen the code you wrote for Linux a while
> > > back and which uses an opcode map, a script and some helper functions
> > > (insn.c, inat.c) to compute an instruction's length. Jan has made some
> > > observations which I believe would be of interest to the kernel
> > > developers as well. Please see:
> > > 
> > > http://lists.xenproject.org/archives/html/xen-devel/2014-07/msg00375.html
> > 
> > Ah, thanks for the comments!
> > 
> > > 
> > > While valid, would they affect the overall goal of insn_get_length() or
> > > is that function tuned for a specific use case? I can't yet tell for
> > > sure if we can get away with it.
> > 
> > Actually, in the Linux kernel, currently we use this just for decoding the
> > length and searching branches. Thus, strict decoding is not needed (moreover,
> > I have a request to optimize it only for that purpose for speeding up).
> > But I think it is better to integrate several different decoders in kernel too.
> > So your comments are very useful for me! :)
> 
> Thank you Masami, that's the exact same use case we plan for it too: to
> solely compute the instruction length. This, I hope, dispels any
> concerns regarding its functionality and whether it does what it's
> supposed to do correctly.

I've opted to send a new mail so I can remove Masami from CC, as he's
probably not interested in the rest of the conversation.

Right now we have two patches which work around x86/emulator
limitations:

 * one computes the instruction length;
 * the other uses single stepping to jump over unsupported instructions;

Adding support for the complete x86(_64) instruction set to the
existent emulator in Xen would make those two unneeded and while I
would like to try my hand at it, I'm not sure the effort would be pay
off. Not to mention that I would very much like to _somehow_ catch the
4.5 deadline. I wonder if it's possible to do this in iterations: take
this (or a decent derivation of it) in, while Răzvan and I work on doing
a better work for 4.6. Am I pushing it? :-)

-- 
Mihai Donțu

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel