[PATCH v4 0/5] Improvements with noreturn

[PATCH v4 0/5] Improvements with noreturn

[Andrew Cooper]

Make better use of noreturn.  It allows optimising compilers to produce more
efficient code.

Each patch is compile tested on each architecture, and the result is
functionally tested on x86 and compile tested on GCC 4.1.1 to verify that
older compilers are happy.

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

[PATCH v4 1/5] xen/compiler: Replace opencoded __attribute__((noreturn))

[Andrew Cooper]

Make a formal define for noreturn in compiler.h, and fix up opencoded uses of
__attribute__((noreturn)).  This includes removing redundant uses with
function definitions which have a public declaration.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>

---

Changes in v4:
 * Standardise on noreturn before the function name
---
 xen/arch/arm/early_printk.c        |    2 +-
 xen/arch/x86/efi/boot.c            |    4 ++--
 xen/arch/x86/shutdown.c            |    2 +-
 xen/include/asm-arm/early_printk.h |    4 ++--
 xen/include/xen/compiler.h         |    2 ++
 xen/include/xen/lib.h              |    2 +-
 xen/include/xen/sched.h            |    4 ++--
 7 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index 41938bb..2870a30 100644
--- a/xen/arch/arm/early_printk.c
+++ b/xen/arch/arm/early_printk.c
@@ -52,7 +52,7 @@ void __init early_printk(const char *fmt, ...)
     va_end(args);
 }
 
-void __attribute__((noreturn)) __init
+void __init
 early_panic(const char *fmt, ...)
 {
     va_list args;
diff --git a/xen/arch/x86/efi/boot.c b/xen/arch/x86/efi/boot.c
index 0dd935c..a26e0af 100644
--- a/xen/arch/x86/efi/boot.c
+++ b/xen/arch/x86/efi/boot.c
@@ -183,7 +183,7 @@ static bool_t __init match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2)
            !memcmp(guid1->Data4, guid2->Data4, sizeof(guid1->Data4));
 }
 
-static void __init __attribute__((__noreturn__)) blexit(const CHAR16 *str)
+static void __init noreturn blexit(const CHAR16 *str)
 {
     if ( str )
         PrintStr((CHAR16 *)str);
@@ -762,7 +762,7 @@ static void __init relocate_trampoline(unsigned long phys)
         *(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
 }
 
-void EFIAPI __init __attribute__((__noreturn__))
+void EFIAPI __init noreturn
 efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 {
     static EFI_GUID __initdata loaded_image_guid = LOADED_IMAGE_PROTOCOL;
diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index 6eba271..6143c40 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -85,7 +85,7 @@ static inline void kb_wait(void)
             break;
 }
 
-static void __attribute__((noreturn)) __machine_halt(void *unused)
+static void noreturn __machine_halt(void *unused)
 {
     local_irq_disable();
     for ( ; ; )
diff --git a/xen/include/asm-arm/early_printk.h b/xen/include/asm-arm/early_printk.h
index 707bbf7..3cb8dab 100644
--- a/xen/include/asm-arm/early_printk.h
+++ b/xen/include/asm-arm/early_printk.h
@@ -26,7 +26,7 @@
 
 void early_printk(const char *fmt, ...)
     __attribute__((format (printf, 1, 2)));
-void early_panic(const char *fmt, ...) __attribute__((noreturn))
+void early_panic(const char *fmt, ...) noreturn
     __attribute__((format (printf, 1, 2)));
 
 #else
@@ -35,7 +35,7 @@ static inline  __attribute__((format (printf, 1, 2))) void
 early_printk(const char *fmt, ...)
 {}
 
-static inline void  __attribute__((noreturn))
+static inline void noreturn
 __attribute__((format (printf, 1, 2))) early_panic(const char *fmt, ...)
 {while(1);}
 
diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index 7d6805c..c80398d 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -14,6 +14,8 @@
 #define always_inline __inline__ __attribute__ ((always_inline))
 #define noinline      __attribute__((noinline))
 
+#define noreturn      __attribute__((noreturn))
+
 #if (!defined(__clang__) && (__GNUC__ == 4) && (__GNUC_MINOR__ < 5))
 #define unreachable() do {} while (1)
 #else
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 5b258fd..0d1a5d3 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -8,7 +8,7 @@
 #include <xen/string.h>
 #include <asm/bug.h>
 
-void __bug(char *file, int line) __attribute__((noreturn));
+void noreturn __bug(char *file, int line);
 void __warn(char *file, int line);
 
 #define BUG_ON(p)  do { if (unlikely(p)) BUG();  } while (0)
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index fb8bd36..00f0eba 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -580,7 +580,7 @@ void __domain_crash(struct domain *d);
  * Mark current domain as crashed and synchronously deschedule from the local
  * processor. This function never returns.
  */
-void __domain_crash_synchronous(void) __attribute__((noreturn));
+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();                                         \
@@ -591,7 +591,7 @@ void __domain_crash_synchronous(void) __attribute__((noreturn));
  * the crash occured.  If addr is 0, look up address from last extable
  * redirection.
  */
-void asm_domain_crash_synchronous(unsigned long addr) __attribute__((noreturn));
+void noreturn asm_domain_crash_synchronous(unsigned long addr);
 
 #define set_current_state(_s) do { current->state = (_s); } while (0)
 void scheduler_init(void);
-- 
1.7.10.4

[PATCH v4 2/5] x86/crash: Fix up declaration of do_nmi_crash()

[Andrew Cooper]

... so it can correctly be annotated as noreturn.  Move the declaration of
nmi_crash() to be effectivly private in crash.c

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>

---

Changes in v4:
 * Standardise on noreturn before the function name
---
 xen/arch/x86/crash.c            |    3 ++-
 xen/include/asm-x86/processor.h |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/crash.c b/xen/arch/x86/crash.c
index 01fd906..ec586bd 100644
--- a/xen/arch/x86/crash.c
+++ b/xen/arch/x86/crash.c
@@ -36,7 +36,7 @@ static unsigned int crashing_cpu;
 static DEFINE_PER_CPU_READ_MOSTLY(bool_t, crash_save_done);
 
 /* This becomes the NMI handler for non-crashing CPUs, when Xen is crashing. */
-void __attribute__((noreturn)) do_nmi_crash(struct cpu_user_regs *regs)
+void do_nmi_crash(struct cpu_user_regs *regs)
 {
     int cpu = smp_processor_id();
 
@@ -113,6 +113,7 @@ void __attribute__((noreturn)) do_nmi_crash(struct cpu_user_regs *regs)
         halt();
 }
 
+void nmi_crash(void);
 static void nmi_shootdown_cpus(void)
 {
     unsigned long msecs;
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index c120460..1d1dee6 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -529,7 +529,6 @@ void do_ ## _name(struct cpu_user_regs *regs)
 DECLARE_TRAP_HANDLER(divide_error);
 DECLARE_TRAP_HANDLER(debug);
 DECLARE_TRAP_HANDLER(nmi);
-DECLARE_TRAP_HANDLER(nmi_crash);
 DECLARE_TRAP_HANDLER(int3);
 DECLARE_TRAP_HANDLER(overflow);
 DECLARE_TRAP_HANDLER(bounds);
@@ -550,6 +549,7 @@ DECLARE_TRAP_HANDLER(spurious_interrupt_bug);
 
 void trap_nop(void);
 void enable_nmis(void);
+void noreturn do_nmi_crash(struct cpu_user_regs *regs);
 
 void syscall_enter(void);
 void sysenter_entry(void);
-- 
1.7.10.4

[PATCH v4 3/5] xen: Identify panic and reboot/halt functions as noreturn

[Andrew Cooper]

On an x86 build (GCC Debian 4.7.2-5), this substantially reduces the size of
.text and .init.text sections.

Experimentally, even in a non-debug build, GCC uses `call` rather than `jmp`
so there should be no impact on any stack trace generation.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>

---

Changes in v4:
 * Standardise on noreturn before the function name
---
 xen/arch/arm/shutdown.c         |    2 +-
 xen/arch/x86/cpu/mcheck/mce.h   |    2 +-
 xen/arch/x86/shutdown.c         |    2 +-
 xen/common/shutdown.c           |    2 +-
 xen/include/asm-arm/smp.h       |    2 +-
 xen/include/asm-x86/processor.h |    2 +-
 xen/include/xen/lib.h           |    2 +-
 xen/include/xen/shutdown.h      |    8 +++++---
 8 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/shutdown.c b/xen/arch/arm/shutdown.c
index 767cc12..adc0529 100644
--- a/xen/arch/arm/shutdown.c
+++ b/xen/arch/arm/shutdown.c
@@ -11,7 +11,7 @@ static void raw_machine_reset(void)
     platform_reset();
 }
 
-static void halt_this_cpu(void *arg)
+static void noreturn halt_this_cpu(void *arg)
 {
     stop_cpu();
 }
diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h
index cbd123d..33bd1ab 100644
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -57,7 +57,7 @@ int mce_available(struct cpuinfo_x86 *c);
 unsigned int mce_firstbank(struct cpuinfo_x86 *c);
 /* Helper functions used for collecting error telemetry */
 struct mc_info *x86_mcinfo_getptr(void);
-void mc_panic(char *s);
+void noreturn mc_panic(char *s);
 void x86_mc_get_cpu_info(unsigned, uint32_t *, uint16_t *, uint16_t *,
 			 uint32_t *, uint32_t *, uint32_t *, uint32_t *);
 
diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index 6143c40..827515d 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -452,7 +452,7 @@ static int __init reboot_init(void)
 }
 __initcall(reboot_init);
 
-static void __machine_restart(void *pdelay)
+static void noreturn __machine_restart(void *pdelay)
 {
     machine_restart(*(unsigned int *)pdelay);
 }
diff --git a/xen/common/shutdown.c b/xen/common/shutdown.c
index 9bccd34..fadb69b 100644
--- a/xen/common/shutdown.c
+++ b/xen/common/shutdown.c
@@ -17,7 +17,7 @@
 bool_t __read_mostly opt_noreboot;
 boolean_param("noreboot", opt_noreboot);
 
-static void maybe_reboot(void)
+static void noreturn maybe_reboot(void)
 {
     if ( opt_noreboot )
     {
diff --git a/xen/include/asm-arm/smp.h b/xen/include/asm-arm/smp.h
index a1de03c..91b1e52 100644
--- a/xen/include/asm-arm/smp.h
+++ b/xen/include/asm-arm/smp.h
@@ -15,7 +15,7 @@ DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask);
 
 #define raw_smp_processor_id() (get_processor_id())
 
-extern void stop_cpu(void);
+extern void noreturn stop_cpu(void);
 
 extern int arch_smp_init(void);
 extern int arch_cpu_init(int cpu, struct dt_device_node *dn);
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 1d1dee6..58fc917 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -514,7 +514,7 @@ void show_registers(struct cpu_user_regs *regs);
 void show_execution_state(struct cpu_user_regs *regs);
 #define dump_execution_state() run_in_exception_handler(show_execution_state)
 void show_page_walk(unsigned long addr);
-void fatal_trap(int trapnr, struct cpu_user_regs *regs);
+void noreturn fatal_trap(int trapnr, struct cpu_user_regs *regs);
 
 void compat_show_guest_stack(struct vcpu *, struct cpu_user_regs *, int lines);
 
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 0d1a5d3..1369b2b 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -87,7 +87,7 @@ extern void printk(const char *format, ...)
     __attribute__ ((format (printf, 1, 2)));
 extern void guest_printk(const struct domain *d, const char *format, ...)
     __attribute__ ((format (printf, 2, 3)));
-extern void panic(const char *format, ...)
+extern void noreturn panic(const char *format, ...)
     __attribute__ ((format (printf, 1, 2)));
 extern long vm_assist(struct domain *, unsigned int, unsigned int);
 extern int __printk_ratelimit(int ratelimit_ms, int ratelimit_burst);
diff --git a/xen/include/xen/shutdown.h b/xen/include/xen/shutdown.h
index 2bee748..f04905b 100644
--- a/xen/include/xen/shutdown.h
+++ b/xen/include/xen/shutdown.h
@@ -1,13 +1,15 @@
 #ifndef __XEN_SHUTDOWN_H__
 #define __XEN_SHUTDOWN_H__
 
+#include <xen/compiler.h>
+
 /* opt_noreboot: If true, machine will need manual reset on error. */
 extern bool_t opt_noreboot;
 
-void dom0_shutdown(u8 reason);
+void noreturn dom0_shutdown(u8 reason);
 
-void machine_restart(unsigned int delay_millisecs);
-void machine_halt(void);
+void noreturn machine_restart(unsigned int delay_millisecs);
+void noreturn machine_halt(void);
 void machine_power_off(void);
 
 #endif /* __XEN_SHUTDOWN_H__ */
-- 
1.7.10.4

[PATCH v4 4/5] xen: Misc cleanup as a result of the previous patches

[Andrew Cooper]

This includes:
 * A stale comment in sh_skip_sync()
 * A dead for ever loop in __bug()
 * A prototype for machine_power_off() which unimplemented in any architecture
 * Replacing a for(;;); loop with unreachable()

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/efi/boot.c         |    2 +-
 xen/arch/x86/mm/shadow/common.c |    1 -
 xen/drivers/char/console.c      |    1 -
 xen/include/xen/shutdown.h      |    1 -
 4 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/xen/arch/x86/efi/boot.c b/xen/arch/x86/efi/boot.c
index a26e0af..62c4812 100644
--- a/xen/arch/x86/efi/boot.c
+++ b/xen/arch/x86/efi/boot.c
@@ -201,7 +201,7 @@ static void __init noreturn blexit(const CHAR16 *str)
         efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size));
 
     efi_bs->Exit(efi_ih, EFI_SUCCESS, 0, NULL);
-    for( ; ; ); /* not reached */
+    unreachable(); /* not reached */
 }
 
 /* generic routine for printing error messages */
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 11c6b62..b400ccb 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -875,7 +875,6 @@ static int sh_skip_sync(struct vcpu *v, mfn_t gl1mfn)
     SHADOW_ERROR("gmfn %#lx was OOS but not shadowed as an l1.\n",
                  mfn_x(gl1mfn));
     BUG();
-    return 0; /* BUG() is no longer __attribute__((noreturn)). */
 }
 
 
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 532c426..7d4383c 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1089,7 +1089,6 @@ void __bug(char *file, int line)
     printk("Xen BUG at %s:%d\n", file, line);
     dump_execution_state();
     panic("Xen BUG at %s:%d", file, line);
-    for ( ; ; ) ;
 }
 
 void __warn(char *file, int line)
diff --git a/xen/include/xen/shutdown.h b/xen/include/xen/shutdown.h
index f04905b..a00bfef 100644
--- a/xen/include/xen/shutdown.h
+++ b/xen/include/xen/shutdown.h
@@ -10,6 +10,5 @@ void noreturn dom0_shutdown(u8 reason);
 
 void noreturn machine_restart(unsigned int delay_millisecs);
 void noreturn machine_halt(void);
-void machine_power_off(void);
 
 #endif /* __XEN_SHUTDOWN_H__ */
-- 
1.7.10.4

[PATCH v4 5/5] xen/x86: Identify reset_stack_and_jump() as noreturn

[Andrew Cooper]

reset_stack_and_jump() is actually a macro, but can effectivly become noreturn
by giving it an unreachable() declaration.

Propagate the 'noreturn-ness' up through the direct and indirect callers.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>

---

Changes in v4:
 * Standardise on noreturn before the function name
---
 xen/arch/x86/domain.c             |    6 ++----
 xen/arch/x86/hvm/svm/svm.c        |    2 +-
 xen/arch/x86/setup.c              |    2 +-
 xen/include/asm-x86/current.h     |   11 +++++++----
 xen/include/asm-x86/domain.h      |    2 +-
 xen/include/asm-x86/hvm/vmx/vmx.h |    2 +-
 6 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 6618ae6..c42a079 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -133,12 +133,12 @@ void startup_cpu_idle_loop(void)
     reset_stack_and_jump(idle_loop);
 }
 
-static void continue_idle_domain(struct vcpu *v)
+static void noreturn continue_idle_domain(struct vcpu *v)
 {
     reset_stack_and_jump(idle_loop);
 }
 
-static void continue_nonidle_domain(struct vcpu *v)
+static void noreturn continue_nonidle_domain(struct vcpu *v)
 {
     check_wakeup_from_wait();
     mark_regs_dirty(guest_cpu_user_regs());
@@ -1521,13 +1521,11 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
     update_vcpu_system_time(next);
 
     schedule_tail(next);
-    BUG();
 }
 
 void continue_running(struct vcpu *same)
 {
     schedule_tail(same);
-    BUG();
 }
 
 int __sync_local_execstate(void)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 406d394..a1d9320 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -911,7 +911,7 @@ static void svm_ctxt_switch_to(struct vcpu *v)
         wrmsrl(MSR_TSC_AUX, hvm_msr_tsc_aux(v));
 }
 
-static void svm_do_resume(struct vcpu *v) 
+static void noreturn svm_do_resume(struct vcpu *v)
 {
     struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
     bool_t debug_state = v->domain->debugger_attached;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b49256d..addd071 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -541,7 +541,7 @@ static char * __init cmdline_cook(char *p, char *loader_name)
     return p;
 }
 
-void __init __start_xen(unsigned long mbi_p)
+void __init noreturn __start_xen(unsigned long mbi_p)
 {
     char *memmap_type = NULL;
     char *cmdline, *kextra, *loader;
diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index c2792ce..4d1f20e 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -59,10 +59,13 @@ static inline struct cpu_info *get_cpu_info(void)
     ((sp & (~(STACK_SIZE-1))) +                 \
      (STACK_SIZE - sizeof(struct cpu_info) - sizeof(unsigned long)))
 
-#define reset_stack_and_jump(__fn)              \
-    __asm__ __volatile__ (                      \
-        "mov %0,%%"__OP"sp; jmp %c1"            \
-        : : "r" (guest_cpu_user_regs()), "i" (__fn) : "memory" )
+#define reset_stack_and_jump(__fn)                                      \
+    ({                                                                  \
+        __asm__ __volatile__ (                                          \
+            "mov %0,%%"__OP"sp; jmp %c1"                                \
+            : : "r" (guest_cpu_user_regs()), "i" (__fn) : "memory" );   \
+        unreachable();                                                  \
+    })
 
 #define schedule_tail(vcpu) (((vcpu)->arch.schedule_tail)(vcpu))
 
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 4ff89f0..49f7c0c 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 (*schedule_tail) (struct vcpu *);
+    void noreturn (*schedule_tail) (struct vcpu *);
 
     void (*ctxt_switch_from) (struct vcpu *);
     void (*ctxt_switch_to) (struct vcpu *);
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h
index 6f6b672..827c97e 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -91,7 +91,7 @@ typedef enum {
 void vmx_asm_vmexit_handler(struct cpu_user_regs);
 void vmx_asm_do_vmentry(void);
 void vmx_intr_assist(void);
-void vmx_do_resume(struct vcpu *);
+void noreturn vmx_do_resume(struct vcpu *);
 void vmx_vlapic_msr_changed(struct vcpu *v);
 void vmx_realmode(struct cpu_user_regs *regs);
 void vmx_update_debug_state(struct vcpu *v);
-- 
1.7.10.4

Re: [PATCH v4 1/5] xen/compiler: Replace opencoded __attribute__((noreturn))

[Jan Beulich]

>>> On 25.02.14 at 13:23, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> Changes in v4:
>  * Standardise on noreturn before the function name

Almost:

> --- a/xen/include/asm-arm/early_printk.h
> +++ b/xen/include/asm-arm/early_printk.h
> @@ -26,7 +26,7 @@
>  
>  void early_printk(const char *fmt, ...)
>      __attribute__((format (printf, 1, 2)));
> -void early_panic(const char *fmt, ...) __attribute__((noreturn))
> +void early_panic(const char *fmt, ...) noreturn
>      __attribute__((format (printf, 1, 2)));

Nevertheless, no need to re-submit afaic.

Jan

Re: [PATCH v4 0/5] Improvements with noreturn

[Jan Beulich]

>>> On 25.02.14 at 13:23, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> Make better use of noreturn.  It allows optimising compilers to produce more
> efficient code.
> 
> Each patch is compile tested on each architecture, and the result is
> functionally tested on x86 and compile tested on GCC 4.1.1 to verify that
> older compilers are happy.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v4 1/5] xen/compiler: Replace opencoded __attribute__((noreturn))

[Andrew Cooper]

On 25/02/14 13:36, Jan Beulich wrote:
>>>> On 25.02.14 at 13:23, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> Changes in v4:
>>  * Standardise on noreturn before the function name
> Almost:
>
>> --- a/xen/include/asm-arm/early_printk.h
>> +++ b/xen/include/asm-arm/early_printk.h
>> @@ -26,7 +26,7 @@
>>  
>>  void early_printk(const char *fmt, ...)
>>      __attribute__((format (printf, 1, 2)));
>> -void early_panic(const char *fmt, ...) __attribute__((noreturn))
>> +void early_panic(const char *fmt, ...) noreturn
>>      __attribute__((format (printf, 1, 2)));
> Nevertheless, no need to re-submit afaic.
>
> Jan
>

Almost!  but as Juliens patch removes early_panic(), this is probably
safe to leave (and would prevent him needing to respin the series)

~Andrew

Re: [PATCH v4 0/5] Improvements with noreturn

[Keir Fraser]

[-- Attachment #1.1: Type: text/plain, Size: 610 bytes --]

On Tue, Feb 25, 2014 at 12:23 PM, Andrew Cooper
<andrew.cooper3@citrix.com>wrote:

> Make better use of noreturn.  It allows optimising compilers to produce
> more
> efficient code.
>
> Each patch is compile tested on each architecture, and the result is
> functionally tested on x86 and compile tested on GCC 4.1.1 to verify that
> older compilers are happy.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>

Nice.

Acked-by: Keir Fraser <keir@xen.org>


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

[-- Attachment #1.2: Type: text/html, Size: 1325 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v4 1/5] xen/compiler: Replace opencoded __attribute__((noreturn))

[Jan Beulich]

>>> On 25.02.14 at 13:23, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> Make a formal define for noreturn in compiler.h, and fix up opencoded uses of
> __attribute__((noreturn)).  This includes removing redundant uses with
> function definitions which have a public declaration.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> Acked-by: Tim Deegan <tim@xen.org>

I had already committed this, but it failed my pre-push build test:

> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -14,6 +14,8 @@
>  #define always_inline __inline__ __attribute__ ((always_inline))
>  #define noinline      __attribute__((noinline))
>  
> +#define noreturn      __attribute__((noreturn))

This collides with uses of __attribute__((noreturn)) elsewhere in
the tree. Did this really build for you without issue?

Jan

Re: [PATCH v4 1/5] xen/compiler: Replace opencoded __attribute__((noreturn))

[Andrew Cooper]

On 28/02/14 16:33, Jan Beulich wrote:
>>>> On 25.02.14 at 13:23, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> Make a formal define for noreturn in compiler.h, and fix up opencoded uses of
>> __attribute__((noreturn)).  This includes removing redundant uses with
>> function definitions which have a public declaration.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <JBeulich@suse.com>
>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
>> Acked-by: Tim Deegan <tim@xen.org>
> I had already committed this, but it failed my pre-push build test:
>
>> --- a/xen/include/xen/compiler.h
>> +++ b/xen/include/xen/compiler.h
>> @@ -14,6 +14,8 @@
>>  #define always_inline __inline__ __attribute__ ((always_inline))
>>  #define noinline      __attribute__((noinline))
>>  
>> +#define noreturn      __attribute__((noreturn))
> This collides with uses of __attribute__((noreturn)) elsewhere in
> the tree. Did this really build for you without issue?
>
> Jan
>

Hmm - I can see why.  I will respin the series and double check each
commit for compilation.  I think this was broken by splitting out the
changes to nmi_crash() in v3.

~Andrew