Re: [Xen-devel] [PATCH] x86/HVM: don't retain emulated insn cache when exiting back to guest

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 05 December 2017 17:19
> 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: don't retain emulated insn cache when
> exiting back to guest
> 
> >>> On 05.12.17 at 17:44, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: 05 December 2017 16:14
> >> --- a/xen/arch/x86/hvm/emulate.c
> >> +++ b/xen/arch/x86/hvm/emulate.c
> >> @@ -2109,20 +2109,22 @@ static int _hvm_emulate_one(struct hvm_e
> >>
> >>      vio->mmio_retry = 0;
> >>
> >> -    rc = x86_emulate(&hvmemul_ctxt->ctxt, ops);
> >> -
> >> -    if ( rc == X86EMUL_OKAY && vio->mmio_retry )
> >> -        rc = X86EMUL_RETRY;
> >> -    if ( rc != X86EMUL_RETRY )
> >> +    switch ( rc = x86_emulate(&hvmemul_ctxt->ctxt, ops) )
> >>      {
> >> +    case X86EMUL_OKAY:
> >> +        if ( vio->mmio_retry )
> >> +            rc = X86EMUL_RETRY;
> >> +        /* fall through */
> >> +    default:
> >>          vio->mmio_cache_count = 0;
> >>          vio->mmio_insn_bytes = 0;
> >> -    }
> >> -    else
> >> -    {
> >> +        break;
> >> +
> >> +    case X86EMUL_RETRY:
> >>          BUILD_BUG_ON(sizeof(vio->mmio_insn) < sizeof(hvmemul_ctxt-
> >> >insn_buf));
> >>          vio->mmio_insn_bytes = hvmemul_ctxt->insn_buf_bytes;
> >>          memcpy(vio->mmio_insn, hvmemul_ctxt->insn_buf, vio-
> >> >mmio_insn_bytes);
> >> +        break;
> >
> > So, we have two distinct cases when X86EMUL_RETRY will be returned: the
> > former when we do want to return to guest part way through a rep
> operation,
> > and another when an MMIO has been sent for external emulation and we
> are
> > expecting a completion. The code looks correct so...
> >
> > Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> 
> Thanks.
> 
> > ...but I wonder there should be two distinct return codes for these two
> > cases.
> 
> The question is - does any of the callers care about the difference.
> I think the relevant information has been recorded in data structures
> by the time we get here.

Technically, no I don't think the callers need to know but I've certainly got a 
bit tied in knots while trying to remember how this stuff is supposed to work 
so I was just hoping something could be done to make the 'don't return to guest 
because we need to retry mmio' and the 'return to guest even though want to 
retry a string operation' cases more obviously distinct.

  Paul

> 
> Jan


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