Re: [Xen-devel] [PATCH v2 2/3] x86/svm: Improve diagnostics when svm_get_insn_len() fails

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.12.18 at 21:22, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/svm/emulate.c
> +++ b/xen/arch/x86/hvm/svm/emulate.c
> @@ -143,8 +143,17 @@ int svm_get_insn_len(struct vcpu *v, enum 
> instruction_index insn)
>      }
>  
>      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__);

Is __func__ really useful in a gdprintk()?

> +    gdprintk(XENLOG_WARNING,
> +             "  insn_index %d, opcode %#x modrm %#x\n",
> +             insn, opc_tab[insn].opcode, ((opc_tab[insn].modrm.rm  << 6) |
> +                                          (opc_tab[insn].modrm.reg << 3) |
> +                                          (opc_tab[insn].modrm.mod)));

This is going to be misleading when bit 3 of modrm.reg or modrm.rm
are set.

> +    gdprintk(XENLOG_WARNING, "  rip %#lx, nextrip %#lx, len %lu\n",
> +             vmcb->rip, vmcb->nextrip, vmcb->nextrip - vmcb->rip);

If you find it necessary to log the length here in the first place, is
there any reason not to use nrip_len here?

Jan



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