Re: [PATCH] x86/HVM: fix interaction between internal and extern emulation

From: Paul Durrant <Paul.Durrant@citrix.com>
To: 'Jan Beulich' <JBeulich@suse.com>
Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] x86/HVM: fix interaction between internal and extern emulation
Date: Tue, 28 Nov 2017 11:01:12 +0000	[thread overview]
Message-ID: <5919a70347b244f2b4c99997fdb26e05@AMSPEX02CL03.citrite.net> (raw)
In-Reply-To: <5A1D4B1B0200007800192AD7@prv-mh.provo.novell.com>

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 28 November 2017 10:40
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; xen-devel <xen-
> devel@lists.xenproject.org>
> Subject: RE: [PATCH] x86/HVM: fix interaction between internal and extern
> emulation
> 
> >>> On 28.11.17 at 11:22, <Paul.Durrant@citrix.com> wrote:
> > It would definitely be good to only reset io_completion when it is clear
> > that handle_hvm_io_completion() is not going to be called (i.e. for
> > internally handled I/O)
> 
> Where would you suggest to do that? These two ...
> 
> > and perhaps even add ASSERTs in _hvm_emulate_one()
> > and handle_pio().
> 
> ... sit down the call tree from handle_hvm_io_completion(). Plus
> internal vs external isn't distinguishable in _hvm_emulate_one()
> afaict (neither on the way in nor on the way out).

Whether the emulation is being handed internally or externally should be apparent on the way out because that's what hvm_vcpu_io_need_completion() is testing for after the call to hvm_emulate_one() in hvm_emulate_one_insn(). The problem is completion being requested if mmio_retry is set even if the former test fails, and I can't remember why that is. On the face of it, it looks wrong.

> Adding
> ASSERT()s there suggests the distinction would need to be done
> up the call stack, yet up the call stack may only be the VM exit
> handler. I don't think the state reset should be done in vendor-
> specific code.
> 

I was hoping that an argument could be passed into the call stack by handle_hvm_io_completion() so that the lower layers would be able to distinguish a re-emulation from an initial call and thus be able to verify state. Maybe that is not practical though.

  Paul

> Jan

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