Re: [Xen-devel] [PATCH v8 1/2] x86emul: New return code for unimplemented instruction

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Petre Pircalabu [mailto:ppircalabu@xxxxxxxxxxxxxxx]
> Sent: 08 August 2017 19:07
> 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 v8 1/2] x86emul: New return code for unimplemented
> instruction
> 
> Enforce the distinction between an instruction not implemented by the
> emulator and the failure to emulate that instruction by defining a new
> return code, X86EMUL_UNIMPLEMENTED.
> 
> This value should only be used by the core emulator if it fails to decode
> the current instruction, and not by any of the x86_emulate_ops
> callbacks.
> 
> Signed-off-by: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
>  xen/arch/x86/hvm/emulate.c             | 4 ++++
>  xen/arch/x86/hvm/io.c                  | 2 ++
>  xen/arch/x86/hvm/vmx/realmode.c        | 2 +-
>  xen/arch/x86/mm/shadow/multi.c         | 2 +-
>  xen/arch/x86/x86_emulate/x86_emulate.c | 8 ++++----
>  xen/arch/x86/x86_emulate/x86_emulate.h | 6 ++++++
>  6 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 3a8db21..28133c0 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -2044,6 +2044,8 @@ int hvm_emulate_one_mmio(unsigned long mfn,
> unsigned long gla)
>      switch ( rc )
>      {
>      case X86EMUL_UNHANDLEABLE:
> +        /* fall-through */
> +    case X86EMUL_UNIMPLEMENTED:
>          hvm_dump_emulation_state(XENLOG_G_WARNING, "MMCFG",
> &ctxt);
>          break;
>      case X86EMUL_EXCEPTION:
> @@ -2113,6 +2115,8 @@ void hvm_emulate_one_vm_event(enum
> emul_kind kind, unsigned int trapnr,
>           * consistent with X86EMUL_RETRY.
>           */
>          return;
> +    case X86EMUL_UNIMPLEMENTED:
> +        /* fall-through */
>      case X86EMUL_UNHANDLEABLE:
>          hvm_dump_emulation_state(XENLOG_G_DEBUG, "Mem event", &ctx);
>          hvm_inject_hw_exception(trapnr, errcode);
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 214ab30..af4e1dc 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -96,6 +96,8 @@ bool hvm_emulate_one_insn(hvm_emulate_validate_t
> *validate, const char *descr)
>      switch ( rc )
>      {
>      case X86EMUL_UNHANDLEABLE:
> +        /* fall-through */
> +    case X86EMUL_UNIMPLEMENTED:
>          hvm_dump_emulation_state(XENLOG_G_WARNING, descr, &ctxt);
>          return false;
> 
> diff --git a/xen/arch/x86/hvm/vmx/realmode.c
> b/xen/arch/x86/hvm/vmx/realmode.c
> index 11bde58..fdbbee2 100644
> --- a/xen/arch/x86/hvm/vmx/realmode.c
> +++ b/xen/arch/x86/hvm/vmx/realmode.c
> @@ -106,7 +106,7 @@ 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 )
> +    if ( rc == X86EMUL_UNHANDLEABLE || rc == X86EMUL_UNIMPLEMENTED
> )
>      {
>          gdprintk(XENLOG_ERR, "Failed to emulate insn.\n");
>          goto fail;
> diff --git a/xen/arch/x86/mm/shadow/multi.c
> b/xen/arch/x86/mm/shadow/multi.c
> index c9c2252..85fb165 100644
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -3486,7 +3486,7 @@ static int sh_page_fault(struct vcpu *v,
>       * would be a good unshadow hint. If we *do* decide to unshadow-on-
> fault
>       * then it must be 'failable': we cannot require the unshadow to succeed.
>       */
> -    if ( r == X86EMUL_UNHANDLEABLE )
> +    if ( r == X86EMUL_UNHANDLEABLE || r == X86EMUL_UNIMPLEMENTED )
>      {
>          perfc_incr(shadow_fault_emulate_failed);
>  #if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index 2201852..480bad9 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -2577,7 +2577,7 @@ x86_decode(
>                          d = twobyte_table[0x3a].desc;
>                          break;
>                      default:
> -                        rc = X86EMUL_UNHANDLEABLE;
> +                        rc = X86EMUL_UNIMPLEMENTED;
>                          goto done;
>                      }
>                  }
> @@ -2591,7 +2591,7 @@ x86_decode(
>                  }
>                  else
>                  {
> -                    rc = X86EMUL_UNHANDLEABLE;
> +                    rc = X86EMUL_UNIMPLEMENTED;
>                      goto done;
>                  }
> 
> @@ -2871,7 +2871,7 @@ x86_decode(
> 
>      default:
>          ASSERT_UNREACHABLE();
> -        return X86EMUL_UNHANDLEABLE;
> +        return X86EMUL_UNIMPLEMENTED;
>      }
> 
>      if ( ea.type == OP_MEM )
> @@ -7717,7 +7717,7 @@ x86_emulate(
> 
>      default:
>      cannot_emulate:
> -        rc = X86EMUL_UNHANDLEABLE;
> +        rc = X86EMUL_UNIMPLEMENTED;
>          goto done;
>      }
> 
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h
> b/xen/arch/x86/x86_emulate/x86_emulate.h
> index 4ddf111..82812ca 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -133,6 +133,12 @@ struct x86_emul_fpu_aux {
>    * Undefined behavior when used anywhere else.
>    */
>  #define X86EMUL_DONE           4
> + /*
> +  * Current instruction is not implemented by the emulator.
> +  * This value should only be returned by the core emulator if decode fails
> +  * and not by any of the x86_emulate_ops callbacks.
> +  */
> +#define X86EMUL_UNIMPLEMENTED  5
> 
>  /* FPU sub-types which may be requested via ->get_fpu(). */
>  enum x86_emulate_fpu_type {
> --
> 2.7.4


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