| Message ID | 20160922185420.6100-1-tamas.lengyel@zentific.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 22.09.16 at 20:54, <tamas.lengyel@zentific.com> wrote: > When emulating instructions Xen's emulator maintains a small i-cache fetched > from the guest memory. This patch extends the vm_event interface to allow > overwriting this i-cache via a buffer returned in the vm_event response. > > When responding to a SOFTWARE_BREAKPOINT event (INT3) the monitor subscriber > normally has to remove the INT3 from memory - singlestep - place back INT3 > to allow the guest to continue execution. This routine however is > susceptible > to a race-condition on multi-vCPU guests. By allowing the subscriber to return > the i-cache to be used for emulation it can side-step the problem by returning > a clean buffer without the INT3 present. > > As part of this patch we rename hvm_mem_access_emulate_one to > hvm_emulate_one_vm_event to better reflect that it is used in various > vm_event > scenarios now, not just in response to mem_access events. > > Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com> > Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com> Non-VM-event specific code: Reviewed-by: Jan Beulich <jbeulich@suse.com> One question though: > --- a/xen/arch/x86/vm_event.c > +++ b/xen/arch/x86/vm_event.c > @@ -209,11 +209,20 @@ void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp) > if ( p2m_mem_access_emulate_check(v, rsp) ) > { > if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA ) > - v->arch.vm_event->emul_read_data = rsp->data.emul_read_data; > + v->arch.vm_event->emul.read = rsp->data.emul.read; > > v->arch.vm_event->emulate_flags = rsp->flags; > } > break; > + > + case VM_EVENT_REASON_SOFTWARE_BREAKPOINT: > + if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) > + { > + v->arch.vm_event->emul.insn = rsp->data.emul.insn; > + v->arch.vm_event->emulate_flags = rsp->flags; > + } > + break; Is this intentionally different from the case above (where the setting of ->emulate_flags is outside the inner if()? Jan
On 09/26/2016 01:33 PM, Jan Beulich wrote: >>>> On 22.09.16 at 20:54, <tamas.lengyel@zentific.com> wrote: >> When emulating instructions Xen's emulator maintains a small i-cache fetched >> from the guest memory. This patch extends the vm_event interface to allow >> overwriting this i-cache via a buffer returned in the vm_event response. >> >> When responding to a SOFTWARE_BREAKPOINT event (INT3) the monitor subscriber >> normally has to remove the INT3 from memory - singlestep - place back INT3 >> to allow the guest to continue execution. This routine however is >> susceptible >> to a race-condition on multi-vCPU guests. By allowing the subscriber to return >> the i-cache to be used for emulation it can side-step the problem by returning >> a clean buffer without the INT3 present. >> >> As part of this patch we rename hvm_mem_access_emulate_one to >> hvm_emulate_one_vm_event to better reflect that it is used in various >> vm_event >> scenarios now, not just in response to mem_access events. >> >> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com> >> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com> > > Non-VM-event specific code: > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > One question though: > >> --- a/xen/arch/x86/vm_event.c >> +++ b/xen/arch/x86/vm_event.c >> @@ -209,11 +209,20 @@ void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp) >> if ( p2m_mem_access_emulate_check(v, rsp) ) >> { >> if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA ) >> - v->arch.vm_event->emul_read_data = rsp->data.emul_read_data; >> + v->arch.vm_event->emul.read = rsp->data.emul.read; >> >> v->arch.vm_event->emulate_flags = rsp->flags; >> } >> break; >> + >> + case VM_EVENT_REASON_SOFTWARE_BREAKPOINT: >> + if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) >> + { >> + v->arch.vm_event->emul.insn = rsp->data.emul.insn; >> + v->arch.vm_event->emulate_flags = rsp->flags; >> + } >> + break; > > Is this intentionally different from the case above (where the setting > of ->emulate_flags is outside the inner if()? Good point. The case below should follow suit of the one above unless there's a corner case Tamas is aware of that I'm missing. Otherwise, a comment would be nice to explain the difference (perhaps for VM_EVENT_REASON_SOFTWARE_BREAKPOINT only VM_EVENT_FLAG_SET_EMUL_INSN_DATA ever makes sense - never a simple emulation). Thanks, Razvan
On Sep 26, 2016 08:29, "Razvan Cojocaru" <rcojocaru@bitdefender.com> wrote: > > On 09/26/2016 01:33 PM, Jan Beulich wrote: > >>>> On 22.09.16 at 20:54, <tamas.lengyel@zentific.com> wrote: > >> When emulating instructions Xen's emulator maintains a small i-cache fetched > >> from the guest memory. This patch extends the vm_event interface to allow > >> overwriting this i-cache via a buffer returned in the vm_event response. > >> > >> When responding to a SOFTWARE_BREAKPOINT event (INT3) the monitor subscriber > >> normally has to remove the INT3 from memory - singlestep - place back INT3 > >> to allow the guest to continue execution. This routine however is > >> susceptible > >> to a race-condition on multi-vCPU guests. By allowing the subscriber to return > >> the i-cache to be used for emulation it can side-step the problem by returning > >> a clean buffer without the INT3 present. > >> > >> As part of this patch we rename hvm_mem_access_emulate_one to > >> hvm_emulate_one_vm_event to better reflect that it is used in various > >> vm_event > >> scenarios now, not just in response to mem_access events. > >> > >> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com> > >> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com> > > > > Non-VM-event specific code: > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > > > One question though: > > > >> --- a/xen/arch/x86/vm_event.c > >> +++ b/xen/arch/x86/vm_event.c > >> @@ -209,11 +209,20 @@ void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp) > >> if ( p2m_mem_access_emulate_check(v, rsp) ) > >> { > >> if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA ) > >> - v->arch.vm_event->emul_read_data = rsp->data.emul_read_data; > >> + v->arch.vm_event->emul.read = rsp->data.emul.read; > >> > >> v->arch.vm_event->emulate_flags = rsp->flags; > >> } > >> break; > >> + > >> + case VM_EVENT_REASON_SOFTWARE_BREAKPOINT: > >> + if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) > >> + { > >> + v->arch.vm_event->emul.insn = rsp->data.emul.insn; > >> + v->arch.vm_event->emulate_flags = rsp->flags; > >> + } > >> + break; > > > > Is this intentionally different from the case above (where the setting > > of ->emulate_flags is outside the inner if()? Yes, it's intentional. > Good point. The case below should follow suit of the one above unless > there's a corner case Tamas is aware of that I'm missing. Otherwise, a > comment would be nice to explain the difference (perhaps for > VM_EVENT_REASON_SOFTWARE_BREAKPOINT only > VM_EVENT_FLAG_SET_EMUL_INSN_DATA ever makes sense - never a simple > emulation). > That's exactly the case here as the commit text explains. Emulating in response to an int3 event only makes sense if you return the insn buffer. I can add a comment to that effect if that helps, though I think it's pretty straight forward. Tamas
On 09/26/2016 05:55 PM, Tamas K Lengyel wrote: > On Sep 26, 2016 08:29, "Razvan Cojocaru" <rcojocaru@bitdefender.com > <mailto:rcojocaru@bitdefender.com>> wrote: >> >> On 09/26/2016 01:33 PM, Jan Beulich wrote: >> >>>> On 22.09.16 at 20:54, <tamas.lengyel@zentific.com > <mailto:tamas.lengyel@zentific.com>> wrote: >> >> When emulating instructions Xen's emulator maintains a small > i-cache fetched >> >> from the guest memory. This patch extends the vm_event interface to > allow >> >> overwriting this i-cache via a buffer returned in the vm_event > response. >> >> >> >> When responding to a SOFTWARE_BREAKPOINT event (INT3) the monitor > subscriber >> >> normally has to remove the INT3 from memory - singlestep - place > back INT3 >> >> to allow the guest to continue execution. This routine however is >> >> susceptible >> >> to a race-condition on multi-vCPU guests. By allowing the > subscriber to return >> >> the i-cache to be used for emulation it can side-step the problem > by returning >> >> a clean buffer without the INT3 present. >> >> >> >> As part of this patch we rename hvm_mem_access_emulate_one to >> >> hvm_emulate_one_vm_event to better reflect that it is used in various >> >> vm_event >> >> scenarios now, not just in response to mem_access events. >> >> >> >> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com > <mailto:tamas.lengyel@zentific.com>> >> >> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com > <mailto:rcojocaru@bitdefender.com>> >> > >> > Non-VM-event specific code: >> > Reviewed-by: Jan Beulich <jbeulich@suse.com <mailto:jbeulich@suse.com>> >> > >> > One question though: >> > >> >> --- a/xen/arch/x86/vm_event.c >> >> +++ b/xen/arch/x86/vm_event.c >> >> @@ -209,11 +209,20 @@ void vm_event_emulate_check(struct vcpu *v, > vm_event_response_t *rsp) >> >> if ( p2m_mem_access_emulate_check(v, rsp) ) >> >> { >> >> if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA ) >> >> - v->arch.vm_event->emul_read_data = > rsp->data.emul_read_data; >> >> + v->arch.vm_event->emul.read = rsp->data.emul.read; >> >> >> >> v->arch.vm_event->emulate_flags = rsp->flags; >> >> } >> >> break; >> >> + >> >> + case VM_EVENT_REASON_SOFTWARE_BREAKPOINT: >> >> + if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) >> >> + { >> >> + v->arch.vm_event->emul.insn = rsp->data.emul.insn; >> >> + v->arch.vm_event->emulate_flags = rsp->flags; >> >> + } >> >> + break; >> > >> > Is this intentionally different from the case above (where the setting >> > of ->emulate_flags is outside the inner if()? > > Yes, it's intentional. > >> Good point. The case below should follow suit of the one above unless >> there's a corner case Tamas is aware of that I'm missing. Otherwise, a >> comment would be nice to explain the difference (perhaps for >> VM_EVENT_REASON_SOFTWARE_BREAKPOINT only >> VM_EVENT_FLAG_SET_EMUL_INSN_DATA ever makes sense - never a simple >> emulation). >> > > That's exactly the case here as the commit text explains. Emulating in > response to an int3 event only makes sense if you return the insn > buffer. I can add a comment to that effect if that helps, though I think > it's pretty straight forward. It might help dispel future confusion, but the commit text should suffice for now - I won't hold the patch hostage because of this. Thanks, Razvan
> -----Original Message----- > From: Tamas K Lengyel [mailto:tamas.lengyel@zentific.com] > Sent: 22 September 2016 19:54 > To: xen-devel@lists.xenproject.org > Cc: Tamas K Lengyel <tamas.lengyel@zentific.com>; Paul Durrant > <Paul.Durrant@citrix.com>; Jan Beulich <jbeulich@suse.com>; Andrew > Cooper <Andrew.Cooper3@citrix.com>; Jun Nakajima > <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>; George > Dunlap <George.Dunlap@citrix.com>; Stefano Stabellini > <sstabellini@kernel.org>; Julien Grall <julien.grall@arm.com> > Subject: [PATCH v4] x86/vm_event: Allow overwriting Xen's i-cache used for > emulation > > When emulating instructions Xen's emulator maintains a small i-cache > fetched from the guest memory. This patch extends the vm_event interface > to allow overwriting this i-cache via a buffer returned in the vm_event > response. > > When responding to a SOFTWARE_BREAKPOINT event (INT3) the monitor > subscriber normally has to remove the INT3 from memory - singlestep - place > back INT3 to allow the guest to continue execution. This routine however is > susceptible to a race-condition on multi-vCPU guests. By allowing the > subscriber to return the i-cache to be used for emulation it can side-step the > problem by returning a clean buffer without the INT3 present. > > As part of this patch we rename hvm_mem_access_emulate_one to > hvm_emulate_one_vm_event to better reflect that it is used in various > vm_event scenarios now, not just in response to mem_access events. > > Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com> > Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com> > --- > Cc: Paul Durrant <paul.durrant@citrix.com> I/O emulation code: Reviewed-by: Paul Durrant <paul.durrant@citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Jun Nakajima <jun.nakajima@intel.com> > Cc: Kevin Tian <kevin.tian@intel.com> > Cc: George Dunlap <george.dunlap@eu.citrix.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien.grall@arm.com> > > v4: Copy insn buffer into mmio buffer to avoid more login in > hvm_emulate_one > Add comment in hvm_do_resume to preserve order as described in > vm_event.h > --- > xen/arch/x86/hvm/emulate.c | 27 +++++++++++++++++++++------ > xen/arch/x86/hvm/hvm.c | 13 ++++++++++--- > xen/arch/x86/vm_event.c | 11 ++++++++++- > xen/include/asm-x86/hvm/emulate.h | 5 +++-- > xen/include/asm-x86/vm_event.h | 5 ++++- > xen/include/public/vm_event.h | 17 ++++++++++++++++- > 6 files changed, 64 insertions(+), 14 deletions(-) > > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c > index cc25676..17f7f0d 100644 > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -76,9 +76,9 @@ static int set_context_data(void *buffer, unsigned int > size) > if ( curr->arch.vm_event ) > { > unsigned int safe_size = > - min(size, curr->arch.vm_event->emul_read_data.size); > + min(size, curr->arch.vm_event->emul.read.size); > > - memcpy(buffer, curr->arch.vm_event->emul_read_data.data, > safe_size); > + memcpy(buffer, curr->arch.vm_event->emul.read.data, safe_size); > memset(buffer + safe_size, 0, size - safe_size); > return X86EMUL_OKAY; > } > @@ -1931,7 +1931,7 @@ int hvm_emulate_one_mmio(unsigned long mfn, > unsigned long gla) > return rc; > } > > -void hvm_mem_access_emulate_one(enum emul_kind kind, unsigned int > trapnr, > +void hvm_emulate_one_vm_event(enum emul_kind kind, unsigned int > trapnr, > unsigned int errcode) > { > struct hvm_emulate_ctxt ctx = {{ 0 }}; @@ -1944,10 +1944,25 @@ void > hvm_mem_access_emulate_one(enum emul_kind kind, unsigned int trapnr, > case EMUL_KIND_NOWRITE: > rc = hvm_emulate_one_no_write(&ctx); > break; > - case EMUL_KIND_SET_CONTEXT: > - ctx.set_context = 1; > - /* Intentional fall-through. */ > + case EMUL_KIND_SET_CONTEXT_INSN: { > + struct vcpu *curr = current; > + struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; > + > + BUILD_BUG_ON(sizeof(vio->mmio_insn) != > + sizeof(curr->arch.vm_event->emul.insn.data)); > + ASSERT(!vio->mmio_insn_bytes); > + > + /* > + * Stash insn buffer into mmio buffer here instead of ctx > + * to avoid having to add more logic to hvm_emulate_one. > + */ > + vio->mmio_insn_bytes = sizeof(vio->mmio_insn); > + memcpy(vio->mmio_insn, curr->arch.vm_event->emul.insn.data, > + vio->mmio_insn_bytes); > + } > + /* Fall-through */ > default: > + ctx.set_context = (kind == EMUL_KIND_SET_CONTEXT_DATA); > rc = hvm_emulate_one(&ctx); > } > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index > 7bad845..b06e4d5 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -487,15 +487,22 @@ void hvm_do_resume(struct vcpu *v) > { > enum emul_kind kind = EMUL_KIND_NORMAL; > > + /* > + * Please observ the order here to match the flag descriptions > + * provided in public/vm_event.h > + */ > if ( v->arch.vm_event->emulate_flags & > VM_EVENT_FLAG_SET_EMUL_READ_DATA ) > - kind = EMUL_KIND_SET_CONTEXT; > + kind = EMUL_KIND_SET_CONTEXT_DATA; > else if ( v->arch.vm_event->emulate_flags & > VM_EVENT_FLAG_EMULATE_NOWRITE ) > kind = EMUL_KIND_NOWRITE; > + else if ( v->arch.vm_event->emulate_flags & > + VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) > + kind = EMUL_KIND_SET_CONTEXT_INSN; > > - hvm_mem_access_emulate_one(kind, TRAP_invalid_op, > - HVM_DELIVER_NO_ERROR_CODE); > + hvm_emulate_one_vm_event(kind, TRAP_invalid_op, > + HVM_DELIVER_NO_ERROR_CODE); > > v->arch.vm_event->emulate_flags = 0; > } > diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c index > 343b9c8..1e88d67 100644 > --- a/xen/arch/x86/vm_event.c > +++ b/xen/arch/x86/vm_event.c > @@ -209,11 +209,20 @@ void vm_event_emulate_check(struct vcpu *v, > vm_event_response_t *rsp) > if ( p2m_mem_access_emulate_check(v, rsp) ) > { > if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA ) > - v->arch.vm_event->emul_read_data = rsp->data.emul_read_data; > + v->arch.vm_event->emul.read = rsp->data.emul.read; > > v->arch.vm_event->emulate_flags = rsp->flags; > } > break; > + > + case VM_EVENT_REASON_SOFTWARE_BREAKPOINT: > + if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) > + { > + v->arch.vm_event->emul.insn = rsp->data.emul.insn; > + v->arch.vm_event->emulate_flags = rsp->flags; > + } > + break; > + > default: > break; > }; > diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm- > x86/hvm/emulate.h > index 3aabcbe..96d8f0b 100644 > --- a/xen/include/asm-x86/hvm/emulate.h > +++ b/xen/include/asm-x86/hvm/emulate.h > @@ -40,14 +40,15 @@ struct hvm_emulate_ctxt { enum emul_kind { > EMUL_KIND_NORMAL, > EMUL_KIND_NOWRITE, > - EMUL_KIND_SET_CONTEXT > + EMUL_KIND_SET_CONTEXT_DATA, > + EMUL_KIND_SET_CONTEXT_INSN > }; > > int hvm_emulate_one( > struct hvm_emulate_ctxt *hvmemul_ctxt); int > hvm_emulate_one_no_write( > struct hvm_emulate_ctxt *hvmemul_ctxt); -void > hvm_mem_access_emulate_one(enum emul_kind kind, > +void hvm_emulate_one_vm_event(enum emul_kind kind, > unsigned int trapnr, > unsigned int errcode); > void hvm_emulate_prepare( > diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm- > x86/vm_event.h index ebb5d88..ca73f99 100644 > --- a/xen/include/asm-x86/vm_event.h > +++ b/xen/include/asm-x86/vm_event.h > @@ -27,7 +27,10 @@ > */ > struct arch_vm_event { > uint32_t emulate_flags; > - struct vm_event_emul_read_data emul_read_data; > + union { > + struct vm_event_emul_read_data read; > + struct vm_event_emul_insn_data insn; > + } emul; > struct monitor_write_data write_data; }; > > diff --git a/xen/include/public/vm_event.h > b/xen/include/public/vm_event.h index f756126..ba8e387 100644 > --- a/xen/include/public/vm_event.h > +++ b/xen/include/public/vm_event.h > @@ -97,6 +97,14 @@ > * Requires the vCPU to be paused already (synchronous events only). > */ > #define VM_EVENT_FLAG_SET_REGISTERS (1 << 8) > +/* > + * Instruction cache is being sent back to the hypervisor in the event > +response > + * to be used by the emulator. This flag is only useful when combined > +with > + * VM_EVENT_FLAG_EMULATE and does not take presedence if combined > with > + * VM_EVENT_FLAG_EMULATE_NOWRITE or > VM_EVENT_FLAG_SET_EMUL_READ_DATA, (i.e. > + * if any of those flags are set, only those will be honored). > + */ > +#define VM_EVENT_FLAG_SET_EMUL_INSN_DATA (1 << 9) > > /* > * Reasons for the vm event request > @@ -265,6 +273,10 @@ struct vm_event_emul_read_data { > uint8_t data[sizeof(struct vm_event_regs_x86) - sizeof(uint32_t)]; }; > > +struct vm_event_emul_insn_data { > + uint8_t data[16]; /* Has to be completely filled */ }; > + > typedef struct vm_event_st { > uint32_t version; /* VM_EVENT_INTERFACE_VERSION */ > uint32_t flags; /* VM_EVENT_FLAG_* */ > @@ -291,7 +303,10 @@ typedef struct vm_event_st { > struct vm_event_regs_arm arm; > } regs; > > - struct vm_event_emul_read_data emul_read_data; > + union { > + struct vm_event_emul_read_data read; > + struct vm_event_emul_insn_data insn; > + } emul; > } data; > } vm_event_request_t, vm_event_response_t; > > -- > 2.9.3
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index cc25676..17f7f0d 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -76,9 +76,9 @@ static int set_context_data(void *buffer, unsigned int size) if ( curr->arch.vm_event ) { unsigned int safe_size = - min(size, curr->arch.vm_event->emul_read_data.size); + min(size, curr->arch.vm_event->emul.read.size); - memcpy(buffer, curr->arch.vm_event->emul_read_data.data, safe_size); + memcpy(buffer, curr->arch.vm_event->emul.read.data, safe_size); memset(buffer + safe_size, 0, size - safe_size); return X86EMUL_OKAY; } @@ -1931,7 +1931,7 @@ int hvm_emulate_one_mmio(unsigned long mfn, unsigned long gla) return rc; } -void hvm_mem_access_emulate_one(enum emul_kind kind, unsigned int trapnr, +void hvm_emulate_one_vm_event(enum emul_kind kind, unsigned int trapnr, unsigned int errcode) { struct hvm_emulate_ctxt ctx = {{ 0 }}; @@ -1944,10 +1944,25 @@ void hvm_mem_access_emulate_one(enum emul_kind kind, unsigned int trapnr, case EMUL_KIND_NOWRITE: rc = hvm_emulate_one_no_write(&ctx); break; - case EMUL_KIND_SET_CONTEXT: - ctx.set_context = 1; - /* Intentional fall-through. */ + case EMUL_KIND_SET_CONTEXT_INSN: { + struct vcpu *curr = current; + struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; + + BUILD_BUG_ON(sizeof(vio->mmio_insn) != + sizeof(curr->arch.vm_event->emul.insn.data)); + ASSERT(!vio->mmio_insn_bytes); + + /* + * Stash insn buffer into mmio buffer here instead of ctx + * to avoid having to add more logic to hvm_emulate_one. + */ + vio->mmio_insn_bytes = sizeof(vio->mmio_insn); + memcpy(vio->mmio_insn, curr->arch.vm_event->emul.insn.data, + vio->mmio_insn_bytes); + } + /* Fall-through */ default: + ctx.set_context = (kind == EMUL_KIND_SET_CONTEXT_DATA); rc = hvm_emulate_one(&ctx); } diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 7bad845..b06e4d5 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -487,15 +487,22 @@ void hvm_do_resume(struct vcpu *v) { enum emul_kind kind = EMUL_KIND_NORMAL; + /* + * Please observ the order here to match the flag descriptions + * provided in public/vm_event.h + */ if ( v->arch.vm_event->emulate_flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA ) - kind = EMUL_KIND_SET_CONTEXT; + kind = EMUL_KIND_SET_CONTEXT_DATA; else if ( v->arch.vm_event->emulate_flags & VM_EVENT_FLAG_EMULATE_NOWRITE ) kind = EMUL_KIND_NOWRITE; + else if ( v->arch.vm_event->emulate_flags & + VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) + kind = EMUL_KIND_SET_CONTEXT_INSN; - hvm_mem_access_emulate_one(kind, TRAP_invalid_op, - HVM_DELIVER_NO_ERROR_CODE); + hvm_emulate_one_vm_event(kind, TRAP_invalid_op, + HVM_DELIVER_NO_ERROR_CODE); v->arch.vm_event->emulate_flags = 0; } diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c index 343b9c8..1e88d67 100644 --- a/xen/arch/x86/vm_event.c +++ b/xen/arch/x86/vm_event.c @@ -209,11 +209,20 @@ void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp) if ( p2m_mem_access_emulate_check(v, rsp) ) { if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA ) - v->arch.vm_event->emul_read_data = rsp->data.emul_read_data; + v->arch.vm_event->emul.read = rsp->data.emul.read; v->arch.vm_event->emulate_flags = rsp->flags; } break; + + case VM_EVENT_REASON_SOFTWARE_BREAKPOINT: + if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) + { + v->arch.vm_event->emul.insn = rsp->data.emul.insn; + v->arch.vm_event->emulate_flags = rsp->flags; + } + break; + default: break; }; diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h index 3aabcbe..96d8f0b 100644 --- a/xen/include/asm-x86/hvm/emulate.h +++ b/xen/include/asm-x86/hvm/emulate.h @@ -40,14 +40,15 @@ struct hvm_emulate_ctxt { enum emul_kind { EMUL_KIND_NORMAL, EMUL_KIND_NOWRITE, - EMUL_KIND_SET_CONTEXT + EMUL_KIND_SET_CONTEXT_DATA, + EMUL_KIND_SET_CONTEXT_INSN }; int hvm_emulate_one( struct hvm_emulate_ctxt *hvmemul_ctxt); int hvm_emulate_one_no_write( struct hvm_emulate_ctxt *hvmemul_ctxt); -void hvm_mem_access_emulate_one(enum emul_kind kind, +void hvm_emulate_one_vm_event(enum emul_kind kind, unsigned int trapnr, unsigned int errcode); void hvm_emulate_prepare( diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h index ebb5d88..ca73f99 100644 --- a/xen/include/asm-x86/vm_event.h +++ b/xen/include/asm-x86/vm_event.h @@ -27,7 +27,10 @@ */ struct arch_vm_event { uint32_t emulate_flags; - struct vm_event_emul_read_data emul_read_data; + union { + struct vm_event_emul_read_data read; + struct vm_event_emul_insn_data insn; + } emul; struct monitor_write_data write_data; }; diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h index f756126..ba8e387 100644 --- a/xen/include/public/vm_event.h +++ b/xen/include/public/vm_event.h @@ -97,6 +97,14 @@ * Requires the vCPU to be paused already (synchronous events only). */ #define VM_EVENT_FLAG_SET_REGISTERS (1 << 8) +/* + * Instruction cache is being sent back to the hypervisor in the event response + * to be used by the emulator. This flag is only useful when combined with + * VM_EVENT_FLAG_EMULATE and does not take presedence if combined with + * VM_EVENT_FLAG_EMULATE_NOWRITE or VM_EVENT_FLAG_SET_EMUL_READ_DATA, (i.e. + * if any of those flags are set, only those will be honored). + */ +#define VM_EVENT_FLAG_SET_EMUL_INSN_DATA (1 << 9) /* * Reasons for the vm event request @@ -265,6 +273,10 @@ struct vm_event_emul_read_data { uint8_t data[sizeof(struct vm_event_regs_x86) - sizeof(uint32_t)]; }; +struct vm_event_emul_insn_data { + uint8_t data[16]; /* Has to be completely filled */ +}; + typedef struct vm_event_st { uint32_t version; /* VM_EVENT_INTERFACE_VERSION */ uint32_t flags; /* VM_EVENT_FLAG_* */ @@ -291,7 +303,10 @@ typedef struct vm_event_st { struct vm_event_regs_arm arm; } regs; - struct vm_event_emul_read_data emul_read_data; + union { + struct vm_event_emul_read_data read; + struct vm_event_emul_insn_data insn; + } emul; } data; } vm_event_request_t, vm_event_response_t;