xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Mihai Donțu" <mdontu@bitdefender.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel@lists.xenproject.org
Subject: Re: [PATCH 0/3] xen: add support for skipping the current instruction
Date: Tue, 9 Sep 2014 20:00:31 +0300	[thread overview]
Message-ID: <20140909200031.764b5228@bitdefender.com> (raw)
In-Reply-To: <540EE8DF02000078000327B1@mail.emea.novell.com>

On Tuesday 09 September 2014 10:47:43 Jan Beulich wrote:
> >>> On 09.09.14 at 04:22, <mdontu@bitdefender.com> wrote:
> > Răzvan Cojocaru has posted a patch in July that added support for
> > computing the length of the instruction that generated the VMEXIT:
> > 
> > http://lists.xen.org/archives/html/xen-devel/2014-07/msg00314.html 
> > 
> > which he then split into two in order to give himself more time to
> > think about Andrew's proposal to extend the existent x86 emulator. I
> > offered to take that off his hands as he was already working to get
> > several patches in.
> > 
> > The initial code was an almost verbatim copy from Linux, with a very
> > small change to make it compile within xen (replaced 'asm/string.h'
> > with 'xen/string.h').
> > 
> > I spent some time to determine what the code save would be if we were
> > to make use of existent xen functionality and my first impression is
> > that the increase in complexity of an already very complex machinery
> > (caused by both the nature of the architecture and by the design of the
> > emulator itself) would not make it worthwhile.
> > 
> > The starting point would be x86_emulate() which uses a carefully coded
> > switch() to walk through the opcode groups, but it only covers a subset
> > of those and, obviously, expects one to add not just instruction length
> > calculation (ie. next eip/rip) but also _actual_ emulation to which I
> > don't think anyone can commit given the architecture complexity (think
> > MMX, SSEn, AVXn etc). The opportunity for bugs makes this task rather
> > prohibitive.
> 
> I disagree, for two reasons: First, there's no reason to implement full
> emulation for everything as long as there's a way for the caller to
> know whether actual emulation (rather than just instruction boundary
> determination) did happen. And second, as recently also pointed out
> by Andrew, making full emulation (including very recent additions like
> AVX-512) a goal would likely be of more than just theoretical value
> (allowing emergency migration of guests to less capable hosts without
> prior CPU feature leveling). Plus - as long as those newer extensions
> could be emulated in ways similar by how the FPU instructions get
> dealt with, I think the scope for bugs can be meaningfully reduced.

I'm looking at emulate_fpu_insn(). Took me a bit to understand the
trick behind it: it doesn't do full emulation just advances the %rip.
Mmm.

> > Masami Hiramatsu, the author of the Linux code, most likely came to the
> > same realization and used a rather simple approach: created
> > x86-opcode-map.txt in which he simply lists the opcode classes in a
> > fairly human readable way. Then he uses an awk script to generate the
> > core of the machinery to which he added a couple of helper functions. I
> > decided to keep this part and see if I can replace some of the helpers,
> > but aside insn_fetch_bytes() which relies on previous calculations made
> > in x86_emulate(), I wasn't able to identify any other code-reuse
> > opportunities. Also, I don't yet have the courage to shuffle code
> > around in x86_emulate.c. :-)
> > 
> > In conclusion, I would opt to bring Masami's code in with very little
> > changes possible (other than coding style which can be handled by a
> > script) so that future updates can be easily ported to xen.
> > 
> > What follows are three patches:
> >   1. adds Masami's et al code (insn_get_length());
> >   2. adds a user in emulate.c;
> >   3. a clang-format script that can be used to do the bulk of the
> >   coding style work on the imported files (should someone else other
> >   than me were to do it).
> > 
> > The motivation behind the 'instruction skipping' idea is to allow
> > memory introspection technologies to instruct Xen to jump over
> > instructions that reside in memory areas of the guest that are marked
> > as NX in EPT (eg. heap/stack of user processes). In such situations,
> > maintaining the vCPU register state is not needed and, as a bonus,
> > expedites the termination of the in-guest process that attempted to
> > execute the code.
> 
> Leaving open why terminating the in-guest process requires advancing
> its IP then, if all other register updates are unnecessary. A huge chunk
> of source code like this needs - I think - a little more of a rationale than
> some exotic, only partially explained use case.

Essentially, instruction skipping is an alternative to
'emulate-no-write'. All it offers is a speed boost, which is noticeable
when, for example, the emulator is walking a piece of code located into
an NX-marked memory area (stack, for example). With emulation, it takes
a long time for an application which has been exploited to terminate
(some types of malware try in a forever-loop to write to the memory
areas they target).

Now, given the tight schedule for 4.5 feature merges of which both
you and Ian talked, I think I'm going to hold this patch for now. It is
not an essential part of memory introspection, but rather an
improvement and it can wait a bit longer.

Thanks,

-- 
Mihai Donțu

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

  reply	other threads:[~2014-09-09 17:00 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-09  2:22 [PATCH 0/3] xen: add support for skipping the current instruction Mihai Donțu
2014-09-09  2:28 ` [PATCH 1/3] x86: add support for computing the instruction length Mihai Donțu
2014-09-09  8:47   ` Mihai Donțu
2014-09-09  9:44     ` Mihai Donțu
2014-09-09 10:13       ` Masami Hiramatsu
2014-09-09 15:46         ` Mihai Donțu
2014-09-09 16:01           ` Mihai Donțu
2014-09-09 16:25             ` Jan Beulich
2014-09-09 17:14               ` Andrew Cooper
2014-09-09 17:27               ` Mihai Donțu
2014-09-09 17:57               ` Konrad Rzeszutek Wilk
2014-09-09  2:29 ` [PATCH 2/3] x86/hvm: implement hvm_get_insn_length() Mihai Donțu
2014-09-09  2:32 ` [PATCH 3/3] xen/tools: script for automatically adjusting the coding style to xen style Mihai Donțu
2014-09-09 14:50   ` Ian Campbell
2014-09-09 15:00     ` Andrew Cooper
2014-09-09 19:52     ` Tim Deegan
2014-09-10 10:59     ` Don Slutz
2014-09-10 14:21   ` Don Slutz
2014-09-09  9:47 ` [PATCH 0/3] xen: add support for skipping the current instruction Jan Beulich
2014-09-09 17:00   ` Mihai Donțu [this message]
2014-09-09 18:58     ` Tamas K Lengyel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140909200031.764b5228@bitdefender.com \
    --to=mdontu@bitdefender.com \
    --cc=JBeulich@suse.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).