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

From: Paul Durrant <Paul.Durrant@citrix.com>
To: Paul Durrant <Paul.Durrant@citrix.com>,
	'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:06:36 +0000	[thread overview]
Message-ID: <e349bb1d89a841e3aa0b7eac9692b9b4@AMSPEX02CL03.citrite.net> (raw)
In-Reply-To: <5919a70347b244f2b4c99997fdb26e05@AMSPEX02CL03.citrite.net>

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Paul Durrant
> Sent: 28 November 2017 11:01
> 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: [Xen-devel] [PATCH] x86/HVM: fix interaction between internal
> and extern emulation
> 
> > -----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.

Yes, it appears that mmio_retry is only set when the underlying emulation returned X86EMUL_OKAY but not all reps were completed. If the underlying emulation did not return X86EMUL_RETRY then I can't figure out why vio->io_completion should need to be set to anything other than HVMIO_no_completion since any other return value indicates there should be nothing pending.

  Paul

> 
> > 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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel