Re: [PATCH v3 3/3] x86/svm: Improve diagnostics when svm_get_insn_len() fails

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: 31 December 2018 11:38
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich
> <JBeulich@suse.com>; Wei Liu <wei.liu2@citrix.com>; Roger Pau Monne
> <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; Boris
> Ostrovsky <boris.ostrovsky@oracle.com>; Suravee Suthikulpanit
> <suravee.suthikulpanit@amd.com>; Brian Woods <brian.woods@amd.com>
> Subject: [PATCH v3 3/3] x86/svm: Improve diagnostics when
> svm_get_insn_len() fails
> 
> Sadly, a lone:
> 
>   (XEN) emulate.c:156:d2v0 svm_get_insn_len: Mismatch between expected and
> actual instruction: eip = fffff804564139c0
> 
> on the console is of no use trying to identify what went wrong.  Dump as
> much
> state as we can to help identify what went wrong.
> 
>   (XEN) Insn mismatch: Expected opcode 0xf0031, modrm 0, got nrip_len 3,
> emul_len 3
>   (XEN) SVM Insn len emulation failed (1): d1v0 64bit @ 0008:0010475f ->
> 0f 01 f9 0f 31 5b 31 ff 31 c0 e9 c2 db ff ff 00
> 
> Drop the debug-only early exit if the sources of length disagree, because
> the
> only effect it has it to avoid the more detailed analysis of what went
> wrong.
> 
> Reported-by: Paul Durrant <paul.durrant@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Brian Woods <brian.woods@amd.com>

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

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> CC: Brian Woods <brian.woods@amd.com>
> 
> v2:
>  * Drop anonymous union
>  * Rebase
> v3:
>  * Rework yet again, over the removal of enum instruction_index
> ---
>  xen/arch/x86/hvm/svm/emulate.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/svm/emulate.c
> b/xen/arch/x86/hvm/svm/emulate.c
> index 827cfc8..4000087 100644
> --- a/xen/arch/x86/hvm/svm/emulate.c
> +++ b/xen/arch/x86/hvm/svm/emulate.c
> @@ -65,7 +65,6 @@ static unsigned long svm_nextrip_insn_length(struct vcpu
> *v)
>   */
>  unsigned int svm_get_insn_len(struct vcpu *v, unsigned int instr_enc)
>  {
> -    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
>      struct hvm_emulate_ctxt ctxt;
>      struct x86_emulate_state *state;
>      unsigned long nrip_len, emul_len;
> @@ -93,15 +92,6 @@ unsigned int svm_get_insn_len(struct vcpu *v, unsigned
> int instr_enc)
>      modrm_mod = x86_insn_modrm(state, &modrm_rm, &modrm_reg);
>      x86_emulate_free_state(state);
> 
> -#ifndef NDEBUG
> -    if ( nrip_len && nrip_len != emul_len )
> -    {
> -        gprintk(XENLOG_WARNING, "insn-len[%02x]=%lu (exp %lu)\n",
> -                ctxt.ctxt.opcode, nrip_len, emul_len);
> -        return nrip_len;
> -    }
> -#endif
> -
>      /* Extract components from instr_enc. */
>      instr_modrm  = instr_enc & 0xff;
>      instr_opcode = instr_enc >> 8;
> @@ -117,9 +107,12 @@ unsigned int svm_get_insn_len(struct vcpu *v,
> unsigned int instr_enc)
>              return emul_len;
>      }
> 
> -    gdprintk(XENLOG_WARNING,
> -             "%s: Mismatch between expected and actual instruction: "
> -             "eip = %lx\n",  __func__, (unsigned long)vmcb->rip);
> +    printk(XENLOG_G_WARNING
> +           "Insn mismatch: Expected opcode %#x, modrm %#x, got nrip_len
> %lu, emul_len %lu\n",
> +           instr_opcode, instr_modrm, nrip_len, emul_len);
> +    hvm_dump_emulation_state(XENLOG_G_WARNING, "SVM Insn len",
> +                             &ctxt, X86EMUL_UNHANDLEABLE);
> +
>      hvm_inject_hw_exception(TRAP_gp_fault, 0);
>      return 0;
>  }
> --
> 2.1.4

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel