Re: [Xen-devel] [PATCH v12 4/4] x86emul: Raise #UD when emulating an unrecognized instruction.

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Petre Pircalabu [mailto:ppircalabu@xxxxxxxxxxxxxxx]
> Sent: 21 September 2017 06:12
> To: xen-devel@xxxxxxxxxxxxx
> Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>;
> Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; jbeulich@xxxxxxxx; konrad.wilk@xxxxxxxxxx;
> sstabellini@xxxxxxxxxx; Tim (Xen.org) <tim@xxxxxxx>; Paul Durrant
> <Paul.Durrant@xxxxxxxxxx>; rcojocaru@xxxxxxxxxxxxxxx;
> tamas@xxxxxxxxxxxxx; jun.nakajima@xxxxxxxxx; Kevin Tian
> <kevin.tian@xxxxxxxxx>; Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
> Subject: [PATCH v12 4/4] x86emul: Raise #UD when emulating an
> unrecognized instruction.
> 
> Modified the behavior of hvm_emulate_one_insn and
> vmx_realmode_emulate_one to generate an Invalid Opcode trap when
> X86EMUL_UNRECOGNIZED is returned by the emulator instead of just
> crashing the domain.
> 
> Signed-off-by: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
>  xen/arch/x86/hvm/io.c           |  6 +++++-
>  xen/arch/x86/hvm/vmx/realmode.c | 11 ++++++++++-
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 7152c28..c7b1c53 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -96,10 +96,14 @@ bool
> hvm_emulate_one_insn(hvm_emulate_validate_t *validate, const char
> *descr)
>      switch ( rc )
>      {
>      case X86EMUL_UNHANDLEABLE:
> -    case X86EMUL_UNIMPLEMENTED:
>          hvm_dump_emulation_state(XENLOG_G_WARNING, descr, &ctxt, rc);
>          return false;
> 
> +    case X86EMUL_UNRECOGNIZED:
> +        hvm_dump_emulation_state(XENLOG_G_WARNING, descr, &ctxt, rc);
> +        hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
> +        break;
> +
>      case X86EMUL_EXCEPTION:
>          hvm_inject_event(&ctxt.ctxt.event);
>          break;
> diff --git a/xen/arch/x86/hvm/vmx/realmode.c
> b/xen/arch/x86/hvm/vmx/realmode.c
> index b93792d..03dea6c 100644
> --- a/xen/arch/x86/hvm/vmx/realmode.c
> +++ b/xen/arch/x86/hvm/vmx/realmode.c
> @@ -106,12 +106,21 @@ void vmx_realmode_emulate_one(struct
> hvm_emulate_ctxt *hvmemul_ctxt)
>      if ( hvm_vcpu_io_need_completion(vio) || vio->mmio_retry )
>          vio->io_completion = HVMIO_realmode_completion;
> 
> -    if ( rc == X86EMUL_UNHANDLEABLE || rc == X86EMUL_UNIMPLEMENTED
> )
> +    if ( rc == X86EMUL_UNHANDLEABLE )

I don't quite understand this change. Why has it become unnecessary to deal 
with X86EMUL_UNIMPLEMENTED? Patch #1 added this change so it seems odd that 
patch #4 would then revert it.

  Paul

>      {
>          gdprintk(XENLOG_ERR, "Failed to emulate insn.\n");
>          goto fail;
>      }
> 
> +    if ( rc == X86EMUL_UNRECOGNIZED )
> +    {
> +        gdprintk(XENLOG_ERR, "Unrecognized insn.\n");
> +        if ( curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE )
> +            goto fail;
> +
> +        realmode_deliver_exception(TRAP_invalid_op, 0, hvmemul_ctxt);
> +    }
> +
>      if ( rc == X86EMUL_EXCEPTION )
>      {
>          if ( unlikely(curr->domain->debugger_attached) &&
> --
> 2.7.4


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