x86/schedule: remove noreturn from schedule_tail() function pointer

XenServer has recently had a support case where this bugframe in
context_switch() was hit, presumably from a corrupt function pointer as the
vcpu pointer was fine.

On balance, it is better to leave the bugframe around for peace of mind in
exceptional circumstances, than to use the optimisations provided by noreturn.

At any meaningful levels of optimisation, the noreturn causes the bugframe to
be optimised out, meaning that any exceptional returns fall into unlikely
branches, which will result in very weird behaviour.

The unreachable() in BUG() does the useful part of noreturn for us, allowing
the compiler not to mess about restoring stack frames etc, but causes a ud2
instruction to be present.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index 4d1f20e16d391d7a66424da81029c8ca4cb7510e..2081015ff9cc9f00a07284943158ec58b3c2248d 100644 (file)
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -67,7 +67,15 @@ static inline struct cpu_info *get_cpu_info(void)
         unreachable();                                                  \
     })
 
-#define schedule_tail(vcpu) (((vcpu)->arch.schedule_tail)(vcpu))
+/*
+ * Schedule tail *should* be a terminal function pointer, but leave a bugframe
+ * around just incase it returns, to save going back into the context
+ * switching code and leaving a far more subtle crash to diagnose.
+ */
+#define schedule_tail(vcpu) do {                \
+        (((vcpu)->arch.schedule_tail)(vcpu));   \
+        BUG();                                  \
+    } while (0)
 
 /*
  * Which VCPU's state is currently running on each CPU?

diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 49f7c0c23112b3cba180b402d165eb6e93c46a39..4ff89f01eac52a4de58c32d564290d9ad93cf255 100644 (file)
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -395,7 +395,7 @@ struct arch_vcpu
 
     unsigned long      flags; /* TF_ */
 
-    void noreturn (*schedule_tail) (struct vcpu *);
+    void (*schedule_tail) (struct vcpu *);
 
     void (*ctxt_switch_from) (struct vcpu *);
     void (*ctxt_switch_to) (struct vcpu *);