[Xen-changelog] [xen master] x86/schedule: remove noreturn from schedule_tail() function pointer

[patchbot@xxxxxxx]

commit 7fb8e8b4e58cdd0a4ac457fd25342b35b6851521
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Mon Mar 10 11:18:05 2014 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Mon Mar 10 11:18:05 2014 +0100

    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@xxxxxxxxxx>
---
 xen/include/asm-x86/current.h |   10 +++++++++-
 xen/include/asm-x86/domain.h  |    2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index 4d1f20e..2081015 100644
--- 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 49f7c0c..4ff89f0 100644
--- 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 *);
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog