[Xen-devel] [PATCH] xen: Improvements to domain_crash_sync()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

The use of __LINE__ in a printk() is problematic for livepatching, as it
causes unnecessary binary differences.

Furthermore, diagnostic information around calls is inconsistent and
occasionally unhelpful.  (e.g. diagnosing logs from the field which might be
release builds, or likely without exact source code).

Take the opportunity to improve things.  Shorten the name to
domain_crash_sync() and require the user to pass a print message in.

Internally, the calling function is identified, and the message is emitted as
a non-debug guest error.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>
---
 xen/arch/x86/hvm/vmx/vmcs.c |  2 +-
 xen/arch/x86/traps.c        |  5 +----
 xen/common/domain.c         |  2 +-
 xen/common/wait.c           | 16 ++++------------
 xen/include/xen/sched.h     | 11 ++++++-----
 5 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index e7818ca..5370ffa 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1669,7 +1669,7 @@ void vmx_vmentry_failure(void)
          error == VMX_INSN_INVALID_HOST_STATE )
         vmcs_dump_vcpu(curr);
 
-    domain_crash_synchronous();
+    domain_crash_sync("\n"); /* Nothing further interesting to print. */
 }
 
 void vmx_do_resume(struct vcpu *v)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 1187fd9..3c11582 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2111,10 +2111,7 @@ void asm_domain_crash_synchronous(unsigned long addr)
     if ( addr == 0 )
         addr = this_cpu(last_extable_addr);
 
-    printk("domain_crash_sync called from entry.S: fault at %p %pS\n",
-           _p(addr), _p(addr));
-
-    __domain_crash_synchronous();
+    domain_crash_sync("entry.S fault at %pS [%p]\n", _p(addr), _p(addr));
 }
 
 /*
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 4567773..d3ba2c2 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -685,7 +685,7 @@ void __domain_crash(struct domain *d)
 }
 
 
-void __domain_crash_synchronous(void)
+void __domain_crash_sync(void)
 {
     __domain_crash(current->domain);
 
diff --git a/xen/common/wait.c b/xen/common/wait.c
index a57bc10..153a59e 100644
--- a/xen/common/wait.c
+++ b/xen/common/wait.c
@@ -133,10 +133,7 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
     wqv->wakeup_cpu = smp_processor_id();
     cpumask_copy(&wqv->saved_affinity, curr->cpu_hard_affinity);
     if ( vcpu_set_hard_affinity(curr, cpumask_of(wqv->wakeup_cpu)) )
-    {
-        gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n");
-        domain_crash_synchronous();
-    }
+        domain_crash_sync("Unable to set vcpu affinity\n");
 
     /* Hand-rolled setjmp(). */
     asm volatile (
@@ -164,10 +161,7 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
         : "memory" );
 
     if ( unlikely(wqv->esp == 0) )
-    {
-        gdprintk(XENLOG_ERR, "Stack too large in %s\n", __func__);
-        domain_crash_synchronous();
-    }
+        domain_crash_sync("Stack too large\n");
 
     cpu_info->guest_cpu_user_regs.entry_vector = entry_vector;
 }
@@ -194,10 +188,8 @@ void check_wakeup_from_wait(void)
         struct vcpu *curr = current;
         cpumask_copy(&wqv->saved_affinity, curr->cpu_hard_affinity);
         if ( vcpu_set_hard_affinity(curr, cpumask_of(wqv->wakeup_cpu)) )
-        {
-            gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n");
-            domain_crash_synchronous();
-        }
+            domain_crash_sync("Unable to set vcpu affinity\n");
+
         wait(); /* takes us back into the scheduler */
     }
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 39f9386..1da93aa 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -627,11 +627,12 @@ void __domain_crash(struct domain *d);
  * Mark current domain as crashed and synchronously deschedule from the local
  * processor. This function never returns.
  */
-void noreturn __domain_crash_synchronous(void);
-#define domain_crash_synchronous() do {                                   \
-    printk("domain_crash_sync called from %s:%d\n", __FILE__, __LINE__);  \
-    __domain_crash_synchronous();                                         \
-} while (0)
+void noreturn __domain_crash_sync(void);
+#define domain_crash_sync(fmt, args...) do {                            \
+        printk(XENLOG_G_ERR "domain_crash_sync called from %s: " fmt,   \
+               __func__, ## args);                                      \
+        __domain_crash_sync();                                          \
+    } while (0)
 
 /*
  * Called from assembly code, with an optional address to help indicate why
-- 
2.1.4


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