Re: [PATCH] x86emul: minor cleanup

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] x86emul: minor cleanup
Date: Mon, 12 Jun 2017 13:41:03 +0100	[thread overview]
Message-ID: <d937289a-b6f8-6d1a-fe8b-ccac31a4b3f4@citrix.com> (raw)
In-Reply-To: <593E4F7E0200007800161AA3@prv-mh.provo.novell.com>

On 12/06/17 07:23, Jan Beulich wrote:
>>>> On 09.06.17 at 19:50, <andrew.cooper3@citrix.com> wrote:
>> On 08/06/17 16:49, Jan Beulich wrote:
>>> Drop a redundant input constraint, correct a comment, and (re)move
>>> fix.insn_bytes adjustments (these aren't needed for custom stub
>>> invocations when the instruction placed in the stub can't raise #XF)
>> I'm not sure these are wise to remove.  Even if we don't expect an
>> exception, should one occur, fpu_handle_exception() will fail to step
>> over the instruction, and will re-execute it.
> Ah, perhaps I shouldn't have split this off the remaining
> emulator series I have ready - you refer to a no longer
> existing function (in my code base).

So you have dropped the legacy FPU exception infrastructure in the series?

> Once there, do_trap()
> will panic() as usual in that case, which I think it is sort of
> appropriate if we receive an exception that shouldn't occur -
> after all we then don't really know what to do with it. This
> btw goes along the lines of me not really being happy about
> us handling all sorts of exceptions once an .ex_table entry
> is associated with an instruction, rather than just the ones
> we really mean to recover from. You may recall such a
> discussion from a few years back.

I don't follow what you mean here. 

>
> Would you be okay with temporarily adding a respective
> BUG_ON(!fic->insn_bytes) to fpu_handle_exception() to
> achieve the same effect?

That would be better than nothing, but is fic->insn_bytes a useful field
with the legacy handling removed?  As all recovery is return-address
based, the length of the instruction (so long as it fits within the
stub) isn't important.

>
> As a side note, I'm removing these here since the further
> SIMD emulation patches I have ready, but would prefer to
> post only once 4.9 is out, do not add respective code in the
> first place. Without knowing this in advance I'm not even
> sure this would be reliably spottable during review.

These what?  Again sorry, I don't understand what you mean.

~Andrew

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