[Xen-devel] [PATCH for-4.5 2/2] x86/hvm: Improve "Emulation failed @" error messages

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

* Introduce hvm_dump_emulation_state() to be a common implementation rather
  than having the printk() open-coded slightly differently in 3 separate
  places.
* Identify the vcpu operating mode to allow for unambiguous decoding of the
  instruction bytes.
* Identify the actual number of bytes in the instruction buffer, and print
  exactly the number of bytes fetched.  A valid instruction can be up to 15
  bytes long, but may also be shorter than the current arbitrary 10 bytes.

A sample new error message looks like:

(d1) MMIO emulation failed: d1v0 64bit mode, 7 bytes @ 0008:ffff82d080102fea: 
48 63 8d 40 ff ff ff

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Keir Fraser <keir@xxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Eddie Dong <eddie.dong@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>

---

Ideally, hvm_dump_emulation_state() would have a const context pointer, but
hvmemul_get_seg_reg() currently needs to set an accessed bit.  We should
probably differencitate between get_seg_reg() for the purposes of emulating,
and get_seg_reg() for the purpose of debug printing.
---
 xen/arch/x86/hvm/emulate.c        |   41 +++++++++++++++++++++++++++++--------
 xen/arch/x86/hvm/io.c             |   11 +---------
 xen/arch/x86/hvm/vmx/realmode.c   |    9 +-------
 xen/include/asm-x86/hvm/emulate.h |    3 +++
 4 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 6ab06e0..d42d251 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1390,15 +1390,7 @@ void hvm_mem_event_emulate_one(bool_t nowrite, unsigned 
int trapnr,
          */
         return;
     case X86EMUL_UNHANDLEABLE:
-        gdprintk(XENLOG_DEBUG, "Emulation failed @ %04x:%lx: "
-               "%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
-               hvmemul_get_seg_reg(x86_seg_cs, &ctx)->sel,
-               ctx.insn_buf_eip,
-               ctx.insn_buf[0], ctx.insn_buf[1],
-               ctx.insn_buf[2], ctx.insn_buf[3],
-               ctx.insn_buf[4], ctx.insn_buf[5],
-               ctx.insn_buf[6], ctx.insn_buf[7],
-               ctx.insn_buf[8], ctx.insn_buf[9]);
+        hvm_dump_emulation_state(XENLOG_G_DEBUG "Mem event", &ctx);
         hvm_inject_hw_exception(trapnr, errcode);
         break;
     case X86EMUL_EXCEPTION:
@@ -1449,6 +1441,37 @@ struct segment_register *hvmemul_get_seg_reg(
     return &hvmemul_ctxt->seg_reg[seg];
 }
 
+static const char *guest_x86_mode_to_str(int mode)
+{
+    switch ( mode )
+    {
+    case 0:
+        return "Real";
+    case 1:
+        return "v8086";
+    case 2:
+    case 4:
+        return "32bit";
+    case 8:
+        return "64bit";
+    default:
+        return "Unknown";
+    }
+}
+
+void hvm_dump_emulation_state(const char *prefix,
+                              struct hvm_emulate_ctxt *hvmemul_ctxt)
+{
+    struct vcpu *curr = current;
+    const char *mode_str = guest_x86_mode_to_str(hvm_guest_x86_mode(curr));
+    struct segment_register *cs = hvmemul_get_seg_reg(x86_seg_cs, 
hvmemul_ctxt);
+
+    printk("%s emulation failed: %pv %s mode, %u bytes @ %04x:%lx: %*ph\n",
+           prefix, curr, mode_str, hvmemul_ctxt->insn_buf_bytes,
+           cs->sel, hvmemul_ctxt->insn_buf_eip,
+           hvmemul_ctxt->insn_buf_bytes, hvmemul_ctxt->insn_buf);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 9f565d6..acbb69b 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -100,16 +100,7 @@ int handle_mmio(void)
     switch ( rc )
     {
     case X86EMUL_UNHANDLEABLE:
-        gdprintk(XENLOG_WARNING,
-                 "MMIO emulation failed @ %04x:%lx: "
-                 "%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
-                 hvmemul_get_seg_reg(x86_seg_cs, &ctxt)->sel,
-                 ctxt.insn_buf_eip,
-                 ctxt.insn_buf[0], ctxt.insn_buf[1],
-                 ctxt.insn_buf[2], ctxt.insn_buf[3],
-                 ctxt.insn_buf[4], ctxt.insn_buf[5],
-                 ctxt.insn_buf[6], ctxt.insn_buf[7],
-                 ctxt.insn_buf[8], ctxt.insn_buf[9]);
+        hvm_dump_emulation_state(XENLOG_G_WARNING "MMIO", &ctxt);
         return 0;
     case X86EMUL_EXCEPTION:
         if ( ctxt.exn_pending )
diff --git a/xen/arch/x86/hvm/vmx/realmode.c b/xen/arch/x86/hvm/vmx/realmode.c
index 45066b2..80d1543 100644
--- a/xen/arch/x86/hvm/vmx/realmode.c
+++ b/xen/arch/x86/hvm/vmx/realmode.c
@@ -157,14 +157,7 @@ static void realmode_emulate_one(struct hvm_emulate_ctxt 
*hvmemul_ctxt)
     return;
 
  fail:
-    gdprintk(XENLOG_ERR,
-             "Real-mode emulation failed @ %04x:%08lx: "
-             "%02x %02x %02x %02x %02x %02x\n",
-             hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt)->sel,
-             hvmemul_ctxt->insn_buf_eip,
-             hvmemul_ctxt->insn_buf[0], hvmemul_ctxt->insn_buf[1],
-             hvmemul_ctxt->insn_buf[2], hvmemul_ctxt->insn_buf[3],
-             hvmemul_ctxt->insn_buf[4], hvmemul_ctxt->insn_buf[5]);
+    hvm_dump_emulation_state(XENLOG_G_ERR "Real-mode", hvmemul_ctxt);
     domain_crash(curr->domain);
 }
 
diff --git a/xen/include/asm-x86/hvm/emulate.h 
b/xen/include/asm-x86/hvm/emulate.h
index efff97e..a510123 100644
--- a/xen/include/asm-x86/hvm/emulate.h
+++ b/xen/include/asm-x86/hvm/emulate.h
@@ -55,4 +55,7 @@ int hvmemul_do_pio(
     unsigned long port, unsigned long *reps, int size,
     paddr_t ram_gpa, int dir, int df, void *p_data);
 
+void hvm_dump_emulation_state(const char *situation,
+                              struct hvm_emulate_ctxt *hvmemul_ctxt);
+
 #endif /* __ASM_X86_HVM_EMULATE_H__ */
-- 
1.7.10.4


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