Re: [Xen-devel] [PATCH 1/2] x86/svm: Improve diagnostics when __get_instruction_length_from_list() fails

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 30 November 2018 17:07
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Roger Pau Monne
> <roger.pau@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Boris
> Ostrovsky <boris.ostrovsky@xxxxxxxxxx>; Suravee Suthikulpanit
> <suravee.suthikulpanit@xxxxxxx>; Brian Woods <brian.woods@xxxxxxx>
> Subject: [PATCH 1/2] x86/svm: Improve diagnostics when
> __get_instruction_length_from_list() fails
> 
> Sadly, a lone:
> 
>   (XEN) emulate.c:156:d2v0 __get_instruction_length_from_list: 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.
> 
> Reported-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> CC: Brian Woods <brian.woods@xxxxxxx>
> 
> RFC: __get_instruction_length_from_list() tries to cope with VMEXIT_IOIO,
> but
> IN/OUT instructions aren't in the decode list and I can't spot an entry
> point
> from the IOIO path.  Am I missing something?


Yes, odd. IOIO are handled in the ifdef NDEBUG blocks but an IOIO exit does 
indeed not call into __get_instruction_length() but does the same calculation 
inline. 

> 
> Also, I'm not entirely convinced that making modrm an annonymous union is
> going to work with older CentOS compilers, and therefore am not sure
> whether
> that part of the change is worth it.  The instruction in question can be
> obtained from the printed INSN_ constant alone.

You could just dump the bitfield values individually.

  Paul

> ---
>  xen/arch/x86/hvm/svm/emulate.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/svm/emulate.c
> b/xen/arch/x86/hvm/svm/emulate.c
> index 3d04af0..71a1b6e 100644
> --- a/xen/arch/x86/hvm/svm/emulate.c
> +++ b/xen/arch/x86/hvm/svm/emulate.c
> @@ -56,11 +56,14 @@ static unsigned long svm_nextrip_insn_length(struct
> vcpu *v)
> 
>  static const struct {
>      unsigned int opcode;
> -    struct {
> -        unsigned int rm:3;
> -        unsigned int reg:3;
> -        unsigned int mod:2;
> -#define MODRM(mod, reg, rm) { rm, reg, mod }
> +    union {
> +        struct {
> +            unsigned int rm:3;
> +            unsigned int reg:3;
> +            unsigned int mod:2;
> +        };
> +        unsigned int raw;
> +#define MODRM(mod, reg, rm) {{ rm, reg, mod }}
>      } modrm;
>  } opc_tab[INSTR_MAX_COUNT] = {
>      [INSTR_PAUSE]   = { X86EMUL_OPC_F3(0, 0x90) },
> @@ -152,8 +155,17 @@ int __get_instruction_length_from_list(struct vcpu
> *v,
>      }
> 
>      gdprintk(XENLOG_WARNING,
> -             "%s: Mismatch between expected and actual instruction: "
> -             "eip = %lx\n",  __func__, (unsigned long)vmcb->rip);
> +             "%s: Mismatch between expected and actual instruction:\n",
> +             __func__);
> +    gdprintk(XENLOG_WARNING,
> +             "  list[0] val %d, { opc %#x, modrm %#x }, list entries:
> %u\n",
> +             list[0], opc_tab[list[0]].opcode,
> opc_tab[list[0]].modrm.raw,
> +             list_count);
> +    gdprintk(XENLOG_WARNING, "  rip 0x%lx, nextrip 0x%lx, len %lu\n",
> +             vmcb->rip, vmcb->nextrip, vmcb->nextrip - vmcb->rip);
> +    hvm_dump_emulation_state(XENLOG_G_WARNING, "Insn_len",
> +                             &ctxt, X86EMUL_UNHANDLEABLE);
> +
>      hvm_inject_hw_exception(TRAP_gp_fault, 0);
>      return 0;
>  }
> --
> 2.1.4

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