Re: [Xen-devel] [PATCH v2 08/19] x86/emul: Rework emulator event injection

[Tim Deegan <tim@xxxxxxx>]

At 12:48 +0000 on 28 Nov (1480337304), Andrew Cooper wrote:
> On 28/11/16 12:04, Tim Deegan wrote:
> > At 11:13 +0000 on 28 Nov (1480331605), Andrew Cooper wrote:
> >> +    /*
> >>       * NB. We do not unshadow on X86EMUL_EXCEPTION. It's not clear that it
> >>       * would be a good unshadow hint. If we *do* decide to 
> >> unshadow-on-fault
> >>       * then it must be 'failable': we cannot require the unshadow to 
> >> succeed.
> >>       */
> >> -    if ( r == X86EMUL_UNHANDLEABLE )
> >> +    if ( r == X86EMUL_UNHANDLEABLE || emul_ctxt.ctxt.event_pending )
> > No thank you.  The comment there explains why we don't want to
> > unshadow for an injection; please let it stand.  Or if the new
> > semantics have changed, update the comment.
> 
> This addition is no functional behavioural change from before, which is
> the point I was trying to get across.

I can understand why you want to do it that way but it makes the
series (and this code!) read quite oddly.  If you keep this, then
please add a second comment above this line that explains why the code
temporarily disagrees with the first comment. :)  You can delete that
comment again in patch #13.

With that, Acked-by: Tim Deegan <tim@xxxxxxx>

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