Re: [Xen-devel] [RFC] x86/vm_event: Allow returning i-cache for emulation

[Tamas Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Sep 12, 2016 08:59, "George Dunlap" <george.dunlap@xxxxxxxxxx> wrote:
>
> On 12/09/16 15:56, Jan Beulich wrote:
> >>>> On 09.09.16 at 17:41, <tamas.lengyel@xxxxxxxxxxxx> wrote:
> >> When emulating instructions the emulator maintains a small i-cache fetched
> >> from the guest memory. Under certain scenarios this memory region may contain
> >> instructions that a monitor subscriber would prefer to hide, namely INT3, and
> >> instead would prefer to emulate a different instruction in-place.
> >>
> >> This patch extends the vm_event interface to allow returning this i-cache via
> >> the vm_event response.
> >
> > Please try to explain better what this change is about, perhaps along
> > the lines of what George used as description, which you then
> > confirmed.
> >
> >> @@ -1783,7 +1786,14 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
> >>          pfec |= PFEC_user_mode;
> >>
> >>      hvmemul_ctxt->insn_buf_eip = regs->eip;
> >> -    if ( !vio->mmio_insn_bytes )
> >> +
> >> +    if ( unlikely(hvmemul_ctxt->set_context_insn) )
> >> +    {
> >> +        memcpy(hvmemul_ctxt->insn_buf, curr->arch.vm_event->emul_data.data,
> >> +               curr->arch.vm_event->emul_data.size);
> >> +        hvmemul_ctxt->insn_buf_bytes = curr->arch.vm_event->emul_data.size;
> >> +    }
> >> +    else if ( !vio->mmio_insn_bytes )
> >
> > I'm not sure about this ordering: Do you really mean to allow an
> > external entity to override insn bytes e.g. in an MMIO retry, i.e.
> > allowing the retried insn to be different from the original one?
> >
> > And additionally I think you need to deal with the case of the
> > supplied data not being a full insn. There shouldn't be any
> > fetching from guest memory in that case imo, emulation should
> > just fail.
>
> This patch was marked "RFC", which to me means, "This isn't ready for a
> full review, but what do you think of the basic idea"?
>
> I would wait until Tamas submits a non-RFC patch before doing a detailed
> review, as it's quite possible this was just a prototype he didn't want
> to bother fixing up until he knew it would be accepted.
>
>  -George

Or that it even makes sense for others too :) I still appreciate the review though.

Tamas

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