xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: "Mihai Donțu" <mdontu@bitdefender.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	keir@xen.org, Razvan Cojocaru <rcojocaru@bitdefender.com>,
	Jan Beulich <JBeulich@suse.com>
Subject: Re: [PATCH 1/3] x86: add support for computing the instruction length
Date: Tue, 9 Sep 2014 18:14:21 +0100	[thread overview]
Message-ID: <540F356D.5000807@citrix.com> (raw)
In-Reply-To: <540F46360200007800032D55@mail.emea.novell.com>

On 09/09/14 17:25, Jan Beulich wrote:
>>>> On 09.09.14 at 18:01, <mdontu@bitdefender.com> wrote:
>> 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? :-)
> Personally I don't think this makes sense to push for 4.5, but in the
> end it'll be Konrad's call. We already have enough other half-way
> reviewed patch series that need finalizing, so I don't think this series
> (which was posted just once many weeks ago) is a candidate.
> Furthermore I'm rather unconvinced of this being code useful to
> other than just your product. And finally, we had (with other
> submitters) some bad experience in the past taking what they
> promised they would clean up later.

I would agree that pushing for 4.5 is unlikely to happen.  I would
suggest that your efforts would be be better spent getting a high
quality series for 4.6 sorted.

As for the instruction length itself, that could be sorted in a more
generic way.

One issue with the current x86_emulate() is that it mixes instruction
decode with instruction emulation.  The AMD SVM code already has a
partial instruction length checker for certain cases where hardware
support is lacking.

It seems plausible to split instruction decode and instruction emulate
into two distinct steps.  This would allow easier unit testing of each
step (always a good thing), and for the instruction decode to be used
independently of emulation.

~Andrew


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

  reply	other threads:[~2014-09-09 17:14 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 [this message]
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
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=540F356D.5000807@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=keir@xen.org \
    --cc=mdontu@bitdefender.com \
    --cc=rcojocaru@bitdefender.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).