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

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 09/09/16 18:41, Tamas K Lengyel 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.
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
> ---
> Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> Cc: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/x86/hvm/emulate.c        | 47 
> +++++++++++++++++++++++++--------------
>  xen/arch/x86/hvm/hvm.c            |  2 +-
>  xen/arch/x86/hvm/vmx/vmx.c        |  1 +
>  xen/arch/x86/mm/p2m.c             |  7 ++++--
>  xen/arch/x86/vm_event.c           | 10 +++++++++
>  xen/common/vm_event.c             |  5 ++++-
>  xen/include/asm-arm/vm_event.h    |  6 +++++
>  xen/include/asm-x86/hvm/emulate.h |  6 +++--
>  xen/include/asm-x86/vm_event.h    |  4 +++-
>  xen/include/public/vm_event.h     | 11 +++++++--
>  10 files changed, 73 insertions(+), 26 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index c55ad7b..968fb7b 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -76,9 +76,12 @@ 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_data.size);
>  
> -        memcpy(buffer, curr->arch.vm_event->emul_read_data.data, safe_size);
> +        gdprintk(XENLOG_WARNING, "Got buffer of size: %u. Request is for %u. 
> Safe size: %u\n",
> +                 curr->arch.vm_event->emul_data.size, size, safe_size);
> +
> +        memcpy(buffer, curr->arch.vm_event->emul_data.data, safe_size);
>          memset(buffer + safe_size, 0, size - safe_size);
>          return X86EMUL_OKAY;
>      }
> @@ -825,7 +828,7 @@ static int hvmemul_read(
>      struct hvm_emulate_ctxt *hvmemul_ctxt =
>          container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>  
> -    if ( unlikely(hvmemul_ctxt->set_context) )
> +    if ( unlikely(hvmemul_ctxt->set_context_data) )
>          return set_context_data(p_data, bytes);
>  
>      return __hvmemul_read(
> @@ -1027,7 +1030,7 @@ static int hvmemul_cmpxchg(
>      struct hvm_emulate_ctxt *hvmemul_ctxt =
>          container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>  
> -    if ( unlikely(hvmemul_ctxt->set_context) )
> +    if ( unlikely(hvmemul_ctxt->set_context_data) )
>      {
>          int rc = set_context_data(p_new, bytes);
>  
> @@ -1120,7 +1123,7 @@ static int hvmemul_rep_outs(
>      p2m_type_t p2mt;
>      int rc;
>  
> -    if ( unlikely(hvmemul_ctxt->set_context) )
> +    if ( unlikely(hvmemul_ctxt->set_context_data) )
>          return hvmemul_rep_outs_set_context(src_seg, src_offset, dst_port,
>                                              bytes_per_rep, reps, ctxt);
>  
> @@ -1262,7 +1265,7 @@ static int hvmemul_rep_movs(
>      if ( buf == NULL )
>          return X86EMUL_UNHANDLEABLE;
>  
> -    if ( unlikely(hvmemul_ctxt->set_context) )
> +    if ( unlikely(hvmemul_ctxt->set_context_data) )
>      {
>          rc = set_context_data(buf, bytes);
>  
> @@ -1460,7 +1463,7 @@ static int hvmemul_read_io(
>  
>      *val = 0;
>  
> -    if ( unlikely(hvmemul_ctxt->set_context) )
> +    if ( unlikely(hvmemul_ctxt->set_context_data) )
>          return set_context_data(val, bytes);
>  
>      return hvmemul_do_pio_buffer(port, bytes, IOREQ_READ, val);
> @@ -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;
> +    }

All these places where we're working with curr->arch.vm_event should
check that it's not NULL. Other than that, I like the concept.


Thanks,
Razvan

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