On Sep 12, 2016 08:59, "George Dunlap" <george.dun...@citrix.com> wrote: > > On 12/09/16 15:56, Jan Beulich wrote: > >>>> On 09.09.16 at 17:41, <tamas.leng...@zentific.com> 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@lists.xen.org https://lists.xen.org/xen-devel