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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf
> Of Paul Durrant
> Sent: 28 November 2017 11:01
> To: 'Jan Beulich' <JBeulich@xxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Julien Grall
> <julien.grall@xxxxxxx>; xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH] x86/HVM: fix interaction between internal
> and extern emulation
> 
> > -----Original Message-----
> > From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > Sent: 28 November 2017 10:40
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> > <Andrew.Cooper3@xxxxxxxxxx>; xen-devel <xen-
> > devel@xxxxxxxxxxxxxxxxxxxx>
> > Subject: RE: [PATCH] x86/HVM: fix interaction between internal and extern
> > emulation
> >
> > >>> On 28.11.17 at 11:22, <Paul.Durrant@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel